Skip to content
This repository was archived by the owner on Aug 15, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions bridges/snowbridge/pallets/ethereum-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ pub mod pallet {
InvalidExecutionHeaderProof,
InvalidAncestryMerkleProof,
InvalidBlockRootsRootMerkleProof,
/// The gap between the finalized headers is larger than the sync committee period,
/// rendering execution headers unprovable using ancestry proofs (blocks root size is
/// the same as the sync committee period slots).
InvalidFinalizedHeaderGap,
HeaderNotFinalized,
BlockBodyHashTreeRootFailed,
HeaderHashTreeRootFailed,
Expand Down Expand Up @@ -398,6 +402,15 @@ pub mod pallet {
Error::<T>::IrrelevantUpdate
);

// Verify the finalized header gap between the current finalized header and new imported
// header is not larger than the sync committee period, otherwise we cannot do
// ancestry proofs for execution headers in the gap.
let max_latency = config::EPOCHS_PER_SYNC_COMMITTEE_PERIOD * config::SLOTS_PER_EPOCH;
ensure!(
latest_finalized_state.slot + max_latency as u64 >= update.finalized_header.slot,
Error::<T>::InvalidFinalizedHeaderGap
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should preferably define the max gap as a constant in config.rs as its inputs are constants, and use saturating or checked arithmetic to guard against overflow. Luckily saturating_mul can be used at compile time.

pub const HEADER_IMPORT_MAX_GAP: usize = EPOCHS_PER_SYNC_COMMITTEE_PERIOD.saturating_mul(SLOTS_PER_EPOCH);
Suggested change
let max_latency = config::EPOCHS_PER_SYNC_COMMITTEE_PERIOD * config::SLOTS_PER_EPOCH;
ensure!(
latest_finalized_state.slot + max_latency as u64 >= update.finalized_header.slot,
Error::<T>::InvalidFinalizedHeaderGap
);
ensure!(
update.finalized_header.slot <= latest_finalized_state.slot + HEADER_IMPORT_MAX_GAP as u64,
Error::<T>::InvalidFinalizedHeaderGap
);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is a good suggestion, adding in c263214.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there is already a SLOTS_PER_HISTORICAL_ROOT constant, which has a value of 8192. Can we use reuse that here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, prefer to reuse SLOTS_PER_HISTORICAL_ROOT


// Verify that the `finality_branch`, if present, confirms `finalized_header` to match
// the finalized checkpoint root saved in the state of `attested_header`.
let finalized_block_root: H256 = update
Expand Down
56 changes: 56 additions & 0 deletions bridges/snowbridge/pallets/ethereum-client/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,62 @@ fn submit_execution_header_not_finalized() {
});
}

/// Check that a gap of more than 8192 slots between finalized headers is not allowed.
#[test]
fn submit_finalized_header_update_with_too_large_gap() {
let checkpoint = Box::new(load_checkpoint_update_fixture());
let update = Box::new(load_sync_committee_update_fixture());
let mut next_update = Box::new(load_next_sync_committee_update_fixture());

// Adds 8193 slots, so that the next update is still in the next sync committee, but the
// gap between the finalized headers is more than 8192 slots.
let slot_with_large_gap =
checkpoint.header.slot + ((EPOCHS_PER_SYNC_COMMITTEE_PERIOD * SLOTS_PER_EPOCH) as u64) + 1;

next_update.finalized_header.slot = slot_with_large_gap;
// Adding some slots to the attested header and signature slot since they need to be ahead
// of the finalized header.
next_update.attested_header.slot = slot_with_large_gap + 33;
next_update.signature_slot = slot_with_large_gap + 43;

new_tester().execute_with(|| {
assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint));
assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()));
assert!(<NextSyncCommittee<Test>>::exists());
assert_err!(
EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()),
Error::<Test>::InvalidFinalizedHeaderGap
);
});
}

/// Check that a gap of 8192 slots between finalized headers is allowed.
#[test]
fn submit_finalized_header_update_with_gap_at_limit() {
let checkpoint = Box::new(load_checkpoint_update_fixture());
let update = Box::new(load_sync_committee_update_fixture());
let mut next_update = Box::new(load_next_sync_committee_update_fixture());

let max_latency = (EPOCHS_PER_SYNC_COMMITTEE_PERIOD * SLOTS_PER_EPOCH) as u64;
next_update.finalized_header.slot = checkpoint.header.slot + max_latency;
// Adding some slots to the attested header and signature slot since they need to be ahead
// of the finalized header.
next_update.attested_header.slot = checkpoint.header.slot + max_latency + 33;
next_update.signature_slot = checkpoint.header.slot + max_latency + 43;

new_tester().execute_with(|| {
assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint));
assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()));
assert!(<NextSyncCommittee<Test>>::exists());
assert_err!(
EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()),
// The test should pass the InvalidFinalizedHeaderGap check, and will fail at the
// next check, the merkle proof, because we changed the next_update slots.
Error::<Test>::InvalidHeaderMerkleProof
);
});
}

/* IMPLS */

#[test]
Expand Down