[Malleability] Header

[tim-barry]

Updates the Header type to make it non-malleable, by moving ProposerSigData field out of the Header to the new Proposal/BlockProposal types. Header.ID() will continue to return the same values as before this change.

Proposal is a new distinct type, used for proposing new blocks and receiving/verifying incoming proposed blocks from the network. It is an internal type corresponding to messages.BlockProposal/messages.ClusterBlockProposal, containing the header/block and associated ProposerSigData. The main changes associated with this are in the Compliance Core, consensus/collection Builder, and MessageHub; as well as adding some new conversion functions and unit test fixtures.

This PR preserves the "body" intermediate type used to calculate the Header ID, which converts the Timestamp to unix time (necessary because time.Time is not RLP-encodable and therefore malleable) and flattens LastViewTC into an ID, and splits it out into an explicit type so that it can be used in tests (TestHeaderFingerprint).
Various serialization methods and their tests are also preserved (TODO: discuss whether to remove JSON and/or Msgpack), as well as the Fingerprint method which is used to test RLP marshaling/unmarshaling.

Closes #6656

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 50.75075% with 164 lines in your changes missing coverage. Please review.

Project coverage is 41.15%. Comparing base (9ce4635) to head (349653b).
Report is 28 commits behind head on feature/malleability.

Files with missing lines	Patch %	Lines
consensus/hotstuff/model/proposal.go	0.00%	22 Missing ⚠️
module/buffer/pending_blocks.go	0.00%	22 Missing ⚠️
module/buffer/pending_cluster_blocks.go	0.00%	22 Missing ⚠️
utils/unittest/fixtures.go	0.00%	22 Missing ⚠️
module/mock/pending_block_buffer.go	0.00%	12 Missing ⚠️
module/mock/pending_cluster_block_buffer.go	0.00%	12 Missing ⚠️
consensus/hotstuff/notifications/telemetry.go	0.00%	8 Missing ⚠️
model/flow/header.go	30.00%	7 Missing ⚠️
module/mock/builder.go	0.00%	6 Missing ⚠️
cmd/consensus/main.go	0.00%	5 Missing ⚠️
... and 12 more

Additional details and impacted files

@@                   Coverage Diff                    @@
##           feature/malleability    #7100      +/-   ##
========================================================
+ Coverage                 41.12%   41.15%   +0.03%     
========================================================
  Files                      2097     2135      +38     
  Lines                    184820   188801    +3981     
========================================================
+ Hits                      76008    77709    +1701     
- Misses                   102473   104585    +2112     
- Partials                   6339     6507     +168     

Flag	Coverage Δ
unittests	41.15% <50.75%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tim-barry]

I've determined at least some of the failing tests are due to a missing proposer signature in the BlockResponse to a RangeRequest, in the synchronization engine.

flow-go/engine/common/synchronization/engine.go

Line 317 in 89dfb91

filteredBlocks = append(filteredBlocks, &messages.BlockProposal{Block: block})

The blocks that are requested and synced then go through all the same logic as new block proposals, so their proposer signature is checked and found to be missing.

Appears in logs as [...] "error":"invalid proposal {ID} at view 1: invalid proposer signature: invalid vote at view 1 for block {ID}: could not split signature for block {ID}: invalid sig data length 0: signature's binary format is invalid","time":"2025-03-14T18:17:22Z","message":"received invalid block from other node (potential slashing evidence?)"}

[durkmurder]

Very nice, great work. I think there are some open questions how do we deal with syncing of blocks and recovery, most likely we will need to store the signature either way but I think we should leave this for another PR and have a separate discussion about this.

[jordanschalm]

Just adding a comment so we don't lost the TODO in a big diff

[jordanschalm]

Removing the signature field is a breaking API change.

We should minimally mark it as deprecated in the API docs (here is an example PR doing that: onflow/flow#1558). May need a more considered plan on how to deal with this if there is any substantial use of this field in the API already. If we end up merging this PR without fully addressing this, could you make an issue to address this under the Malleability tracker please.

fyi @peterargue

[jordanschalm]

We have decided to store the proposer signature for all node roles, at least for this round of changes. Given that, we can simply include the proposer signature here again.