Repository navigation
Phase3 trusted block root - #9917
aarishnaiyer wants to merge 13 commits into
Conversation
|
Maybe we could combine this/alias this with the |
Thanks for the suggestion! The main difference is that --wss-checkpoint is used for ongoing verification after sync while this flag is for verifying the downloaded state before initializing. But you're right that they overlap. Would it make sense to reuse --wss-checkpoint's block root to also verify the checkpoint download when --checkpoint-sync-url is set? |
|
updated to reuse --wss-checkpoint's block root for this verification. removed the separate flag |
7e970e9 to
a6c6b28
Compare
|
Hey Michael, working on the BeaconStatePartsByRange endpoint for Phase 3 of the decentralized checkpoint sync project. To build ListSummary and BeaconStatePart from a live BeaconState I need the raw items root of each list before length gets mixed in (generalized index 2). Checked milhouse's public API and there's no accessor for it yet. Would a small public getter be the right approach, or is there another way to get at it? Also noticed PR #92 on milhouse which looks related. |
The previous uniform items-per-chunk stride didn't produce valid single-subtree Merkle proofs against the actual progressive-list tree structure, since levels start at non-round cumulative offsets (0, 1, 5, 21, 85, ...). As per Etan's message: the first chunk folds together every level that fits completely within items_per_chunk, and every chunk after that is a plain round items_per_chunk-sized slice, since each later level is an exact power-of-two multiple of items_per_chunk. Also fixes ChunkLayout::locate to clip the final chunk of a field to its actual length, which the old uniform-stride version never did.
etan-status
left a comment
etan-status
left a comment
There was a problem hiding this comment.
Given that you have a chunking implementation, would you mind measuring the size of the snapshot on a real mainnet BeaconState?
Depending on the size, it might be worth splitting up the summary some more if it's big (i.e., update to https://hackmd.io/@etan-status/decentralized-cl-sync based on the measurements).
| // If --wss-checkpoint was provided, verify the downloaded block root matches it | ||
| // before initializing the chain from the checkpoint state. | ||
| if let Some(wss_checkpoint) = config.chain.weak_subjectivity_checkpoint { | ||
| if block_root != wss_checkpoint.root { |
There was a problem hiding this comment.
Hmm, given that --wss-checkpoint was already used before, is this compatible with its earlier use?
e.g., if the user passes a checkpoint that's slightly older than the freshest one, would it still work?
There was a problem hiding this comment.
Also, in PR description --checkpoint-sync-url is mentioned, is that still applicable?
There was a problem hiding this comment.
No Im still using --wss-checkpoint as the source of the root, that part’s unchanged. The gap is in how I compare it: so my check does flat equality against the freshest downloaded block, so it would reject a checkpoint that’s slightly older than the tip, something the existing verify_weak_subjectivity_checkpoint already tolerates by walking backward through block roots when the checkpoint’s epoch is older than the current finalized one. I’ll rework the comparison to match that existing backward-walk behavior instead of a flat equality check.
| // If --wss-checkpoint was provided, verify the downloaded block root matches it | ||
| // before initializing the chain from the checkpoint state. |
There was a problem hiding this comment.
The checkpoint state also has to be validated. i.e., have to verify that the block header's state_root matches the state's root, and that both of them are referring to the same slot / block.
Given that you only add new logic here, I wonder if this is a problem related to the PR, or already there before.
There was a problem hiding this comment.
Yeah thats checked, this predates the PR. The state and block are fetched independently a few lines above with no cross-check between them already there. My change only adds the root-vs---wss-checkpoint comparison after that, I didn’t touch the fetch logic itself. It's definitely worth fixing, but it’s a separate, existing gap, not something I introduced. Happy to pick it up as a follow-up, or fold it into this PR if you’d rather bundle it.
| /// The fold itself is computed in leaf units (dividing out [`Self::packing_factor`]) and | ||
| /// converted back to items at the end, since the real tree's levels are leaf-sized, not | ||
| /// item-sized (see [`Self::packing_factor`]'s docs). For unpacked fields (`packing_factor == | ||
| /// 1`) this is a no-op and matches the original item-unit fold exactly. |
There was a problem hiding this comment.
Yep! multiple leaves can get packed into chunks, so the packing factor is important.
Adds a checkpoint-sync-url-block-root flag to Lighthouse's checkpoint sync. When provided alongside --checkpoint-sync-url, the downloaded checkpoint block's canonical root is verified against the trusted block root before the node initializes from it. If they don't match, the node refuses to start and logs an error.
This is the first step toward decentralized checkpoint sync in Lighthouse. Currently, checkpoint sync downloads a BeaconState and block from a single trusted URL with no verification. A user who knows the trusted block root out-of-band (from a friend, a block explorer, or their own LC sync) can now pass it via this flag to ensure the checkpoint provider is serving the correct chain.
The flag requires --checkpoint-sync-url to be set. The block root must be 0x-prefixed hex.
This is Phase 3 of the Decentralized CL Checkpoint Sync EPF project.