From 5613dc7867d32a3aa28cfcf0053e13a1a22c2633 Mon Sep 17 00:00:00 2001 From: Clara van Staden Date: Thu, 16 May 2024 08:49:44 +0200 Subject: [PATCH 1/3] Reject finalized updates without a sync committee in next store period (#145) * sync committee handover fix * remove comment * Update bridges/snowbridge/pallets/ethereum-client/src/lib.rs Co-authored-by: Vincent Geddes <117534+vgeddes@users.noreply.github.com> * add extra check * move checks around * remove extra validation * fix compiler error * reject updates in next period without sync committee * fix test * polish * Update bridges/snowbridge/pallets/ethereum-client/src/lib.rs Co-authored-by: Vincent Geddes <117534+vgeddes@users.noreply.github.com> --------- Co-authored-by: Vincent Geddes <117534+vgeddes@users.noreply.github.com> --- .../pallets/ethereum-client/src/lib.rs | 7 +++++++ .../pallets/ethereum-client/src/tests.rs | 16 +++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index c1b9e19729bc..0ba1b8df4654 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -104,6 +104,7 @@ pub mod pallet { #[pallet::error] pub enum Error { SkippedSyncCommitteePeriod, + SyncCommitteeUpdateRequired, /// Attested header is older than latest finalized header. IrrelevantUpdate, NotBootstrapped, @@ -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 = !>::exists() && (update.next_sync_committee_update.is_some() && update_attested_period == store_period); @@ -395,6 +397,11 @@ pub mod pallet { ), Error::::InvalidSyncCommitteeMerkleProof ); + } else { + ensure!( + update_finalized_period == store_period, + Error::::SyncCommitteeUpdateRequired + ); } // Verify sync committee aggregate signature. diff --git a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs index 765958c12821..ae476d2464e6 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs @@ -362,13 +362,14 @@ 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)); @@ -376,9 +377,18 @@ fn submit_update_in_next_period() { 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::::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(); - assert!(>::contains_key(block_root)); }); } From 1371e76518fffd22eda8119c591708c953c56918 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 16 May 2024 09:16:27 +0200 Subject: [PATCH 2/3] prdoc --- prdoc/pr_4478.prdoc | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 prdoc/pr_4478.prdoc diff --git a/prdoc/pr_4478.prdoc b/prdoc/pr_4478.prdoc new file mode 100644 index 000000000000..22e2e43db4ca --- /dev/null +++ b/prdoc/pr_4478.prdoc @@ -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 From 9fdcef31dc12f42e6499e460899ab67b21961231 Mon Sep 17 00:00:00 2001 From: Clara van Staden Date: Thu, 16 May 2024 14:54:44 +0200 Subject: [PATCH 3/3] add finalized header in storage check back --- bridges/snowbridge/pallets/ethereum-client/src/tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs index ae476d2464e6..da762dc2fd80 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs @@ -389,6 +389,8 @@ fn reject_submit_update_in_next_period() { )); // 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(); + assert!(>::contains_key(block_root)); }); }