Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snowbridge - Ethereum Client - Reject finalized updates without a sync committee in next store period #4478

Merged
merged 8 commits into from
May 16, 2024
7 changes: 7 additions & 0 deletions bridges/snowbridge/pallets/ethereum-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ pub mod pallet {
#[pallet::error]
pub enum Error<T> {
SkippedSyncCommitteePeriod,
SyncCommitteeUpdateRequired,
/// Attested header is older than latest finalized header.
IrrelevantUpdate,
NotBootstrapped,
Expand Down Expand Up @@ -320,6 +321,7 @@ pub mod pallet {

// Verify update is relevant.
let update_attested_period = compute_period(update.attested_header.slot);
let update_finalized_period = compute_period(update.finalized_header.slot);
let update_has_next_sync_committee = !<NextSyncCommittee<T>>::exists() &&
(update.next_sync_committee_update.is_some() &&
update_attested_period == store_period);
Expand Down Expand Up @@ -395,6 +397,11 @@ pub mod pallet {
),
Error::<T>::InvalidSyncCommitteeMerkleProof
);
} else {
ensure!(
update_finalized_period == store_period,
Error::<T>::SyncCommitteeUpdateRequired
);
}

// Verify sync committee aggregate signature.
Expand Down
14 changes: 13 additions & 1 deletion bridges/snowbridge/pallets/ethereum-client/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,20 +362,32 @@ fn submit_update_with_sync_committee_in_current_period() {
}

#[test]
fn submit_update_in_next_period() {
fn reject_submit_update_in_next_period() {
let checkpoint = Box::new(load_checkpoint_update_fixture());
let sync_committee_update = Box::new(load_sync_committee_update_fixture());
let update = Box::new(load_next_finalized_header_update_fixture());
let sync_committee_period = compute_period(sync_committee_update.finalized_header.slot);
let next_sync_committee_period = compute_period(update.finalized_header.slot);
assert_eq!(sync_committee_period + 1, next_sync_committee_period);
let next_sync_committee_update = Box::new(load_next_sync_committee_update_fixture());

new_tester().execute_with(|| {
assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint));
assert_ok!(EthereumBeaconClient::submit(
RuntimeOrigin::signed(1),
sync_committee_update.clone()
));
// check an update in the next period is rejected
assert_err!(
EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()),
Error::<Test>::SyncCommitteeUpdateRequired
);
// submit update with next sync committee
assert_ok!(EthereumBeaconClient::submit(
RuntimeOrigin::signed(1),
next_sync_committee_update
));
// check same header in the next period can now be submitted successfully
assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()));
let block_root: H256 = update.finalized_header.clone().hash_tree_root().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it an intended removal? It looks like it should be fine even after the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Added back in 9fdcef3.

assert!(<FinalizedBeaconState<Test>>::contains_key(block_root));
Expand Down
13 changes: 13 additions & 0 deletions prdoc/pr_4478.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Snowbridge - Ethereum Client - Reject finalized updates without a sync committee in next store period

doc:
- audience: Runtime Dev
description: |
Bug fix in the Ethereum light client that stalls the light client when an update in the next sync committee period is received without receiving the next sync committee update in the next period.

crates:
- name: snowbridge-pallet-ethereum-client
bump: patch
Loading