fix(chain): Validate header versions when serializing blocks

[arya2]

Motivation

This PR updates the ZcashSerialize implementation for block headers to apply to same constraints as ZcashDeserialize.

Closes #497.

Solution

• Splits version constraint checks in zcash_deserialize to a check_version function
• Calls check_version() from zcash_serialize

Related changes:

• Removes LOW_31_BITS constant and masking of high bit when deserializing (a high bit causes an error anyways)

Review

Anyone can review.

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
  • Does it change concurrent code, unsafe code, or consensus rules?
• How do you know it works? Does it have tests?

[oxarbitrage]

It looks good, thank you for the fix.

wrong_checkpoint_hash_fail_test is failing. This seems to be because Zebra now panic in the serialization instead of an error in the verifier.

Can be fixed by changed the version to 5 at https://github.com/ZcashFoundation/zebra/blob/main/zebra-consensus/src/checkpoint/tests.rs#L507

[teor2345]

Looks good, I just want to check we handle the serialization error correctly.

In some cases, we panic because we assume serialization errors are not possible. But we only do this when we serialize to a vector or memory. (If we're using the disk or network, we can encounter an IO error like a full disk or a broken connection.)

[codecov]

Codecov Report

Merging #6475 (6b4ec1b) into main (044ecb0) will decrease coverage by 0.17%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6475      +/-   ##
==========================================
- Coverage   77.89%   77.72%   -0.17%     
==========================================
  Files         304      304              
  Lines       39678    39665      -13     
==========================================
- Hits        30908    30831      -77     
- Misses       8770     8834      +64     

[arya2]

Can be fixed by changed the version to 5 at https://github.com/ZcashFoundation/zebra/blob/main/zebra-consensus/src/checkpoint/tests.rs#L507

Thank you!! Applied in 86ec19b.

In some cases, we panic because we assume serialization errors are not possible. But we only do this when we serialize to a vector or memory. (If we're using the disk or network, we can encounter an IO error like a full disk or a broken connection.)

It looks like Headers are only created in production via ZcashDeserialize and never modified, so the check_version() call in ZcashSerialize should only ever return an error in tests?

[teor2345]

In some cases, we panic because we assume serialization errors are not possible. But we only do this when we serialize to a vector or memory. (If we're using the disk or network, we can encounter an IO error like a full disk or a broken connection.)

It looks like Headers are only created in production via ZcashDeserialize and never modified, so the check_version() call in ZcashSerialize should only ever return an error in tests?

That makes sense to me!

[teor2345]

Thanks for the extra docs and tests!