Fix for #7305: Verify blobs and data columns during backfill
Issue Addressed
#7305: Verify blobs and data columns during backfill
Proposed Changes
Compare signatures (equality check) for blob and data column sidecars Then, validate KZG commitments as described in the issue.
Additional Info
This will be further compared and leveraged with #7352 as some parts of this issue is also covered as part of the issue it is fixing. So, WIP and will let you know once it is ready under the comments.
Currently under debugging its test case. The test is: https://github.com/sigp/lighthouse/blob/c13e069c9c633535336bca6d6f5f7b30720df629/beacon_node/beacon_chain/tests/store_tests.rs#L2267
However, the test does not run properly when Deneb or Fulu is enabled, e.g. FORK_NAME=deneb cargo test weak_subjectivity_sync --features "fork_from_env".
Status: Ready for review. Tests run:
- pre-Deneb: as it is in store_tests.rs
- pre-Fulu, post-Deneb: + incorrect signature in blob header + incorrect KZG commitment in blob
- post-Fulu: + incorrect signature in data column header + incorrect KZG commitment in data column
Hey @dapplion, I much liked your implementation from https://github.com/sigp/lighthouse/pull/7352, where you've used existing functions I wasn't aware of and also made helper functions that share the same logics between range sync.
I tried to minimize the conflicts I hope this is okay with you. Please let me know for any comments!
There're a few parts that I changed from yours.
- I made minimal changes to sync_methods.rs as there are many more changes related to your PR (peer scoring itself).
- Didn't take assert_correct_historical_block_chain() because if import_historical_block_batch() bails during block signature verification, the oldest block parent is updated regardless. I think this shouldn't happen (I think you also mentioned about DB stuff in your comment?). However, I left as it is to preserve working code.
- Properly added timer metrics
I haven't made any changes to the range sync verification as it wasn't scope of the issue (I would've done it if I was aware of) and thought it would create more conflicts between your PR.
Restructured backfill importing such that
- it now clearly separates block validation and storing.
- it follows PeerDAS scoring order
@dapplion could you review the PR?
We are focusing on merging https://github.com/sigp/lighthouse/pull/7352 first, will port the remaining diff from here later
This is quite outdated and a lot of the code touched in this PR has changed. Closing this and we'll address this separately. Thanks for this @SunnysidedJ