Fix border condition in Snowbridge free consensus Updates

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

@@ -179,6 +179,10 @@ pub mod pallet {
 	#[pallet::storage]
 	pub type NextSyncCommittee<T: Config> = StorageValue<_, SyncCommitteePrepared, ValueQuery>;
 
+	/// The last period where the next sync committee was updated for free.
+	#[pallet::storage]
+	pub type LatestFreeSyncCommitteeUpdatePeriod<T: Config> = StorageValue<_, u64, ValueQuery>;
+
 	/// The current operating mode of the pallet.
 	#[pallet::storage]
 	#[pallet::getter(fn operating_mode)]
@@ -442,6 +446,13 @@ pub mod pallet {
 			let latest_finalized_state =
 				FinalizedBeaconState::<T>::get(LatestFinalizedBlockRoot::<T>::get())
 					.ok_or(Error::<T>::NotBootstrapped)?;
+
+			let pays_fee = Self::check_refundable(update, latest_finalized_state.slot);
+			let actual_weight = match update.next_sync_committee_update {
+				None => T::WeightInfo::submit(),
+				Some(_) => T::WeightInfo::submit_with_sync_committee(),
+			};
+
 			if let Some(next_sync_committee_update) = &update.next_sync_committee_update {
 				let store_period = compute_period(latest_finalized_state.slot);
 				let update_finalized_period = compute_period(update.finalized_header.slot);
@@ -465,17 +476,12 @@ pub mod pallet {
 					"💫 SyncCommitteeUpdated at period {}.",
 					update_finalized_period
 				);
+				<LatestFreeSyncCommitteeUpdatePeriod<T>>::set(update_finalized_period);
 				Self::deposit_event(Event::SyncCommitteeUpdated {
 					period: update_finalized_period,
 				});
 			};
 
-			let pays_fee = Self::check_refundable(update, latest_finalized_state.slot);
-			let actual_weight = match update.next_sync_committee_update {
-				None => T::WeightInfo::submit(),
-				Some(_) => T::WeightInfo::submit_with_sync_committee(),
-			};
-
 			if update.finalized_header.slot > latest_finalized_state.slot {
 				Self::store_finalized_header(update.finalized_header, update.block_roots_root)?;
 			}
@@ -657,7 +663,14 @@ pub mod pallet {
 		/// successful sync committee updates are free.
 		pub(super) fn check_refundable(update: &Update, latest_slot: u64) -> Pays {
 			// If the sync committee was successfully updated, the update may be free.
-			if update.next_sync_committee_update.is_some() {
+			let update_period = compute_period(update.finalized_header.slot);
+			let latest_free_update_period = LatestFreeSyncCommitteeUpdatePeriod::<T>::get();
+			// If the next sync committee is not known and this update sets it, the update is free.
+			// If the sync committee update is in a period that we have not received an update for,
+			// the update is free.
+			let refundable =
+				!<NextSyncCommittee<T>>::exists() || update_period > latest_free_update_period;
+			if update.next_sync_committee_update.is_some() && refundable {
 				return Pays::No;
 			}
 

bridges/snowbridge/pallets/ethereum-client/src/mock.rs

@@ -59,6 +59,13 @@ pub fn load_next_finalized_header_update_fixture() -> snowbridge_beacon_primitiv
 	load_fixture("next-finalized-header-update.json".to_string()).unwrap()
 }
 
+pub fn load_sync_committee_update_period_0_fixture() -> snowbridge_beacon_primitives::Update<
+	{ config::SYNC_COMMITTEE_SIZE },
+	{ config::SYNC_COMMITTEE_BITS_SIZE },
+> {
+	load_fixture("sync-committee-update-period-0.json".to_string()).unwrap()
+}
+
 pub fn get_message_verification_payload() -> (Log, Proof) {
 	let inbound_fixture = snowbridge_pallet_ethereum_client_fixtures::make_inbound_fixture();
 	(inbound_fixture.message.event_log, inbound_fixture.message.proof)

bridges/snowbridge/pallets/ethereum-client/src/tests.rs

@@ -1,19 +1,18 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: 2023 Snowfork <hello@snowfork.com>
+pub use crate::mock::*;
 use crate::{
+	config::{EPOCHS_PER_SYNC_COMMITTEE_PERIOD, SLOTS_PER_EPOCH, SLOTS_PER_HISTORICAL_ROOT},
 	functions::compute_period,
 	mock::{
 		get_message_verification_payload, load_checkpoint_update_fixture,
 		load_finalized_header_update_fixture, load_next_finalized_header_update_fixture,
 		load_next_sync_committee_update_fixture, load_sync_committee_update_fixture,
 	},
 	sync_committee_sum, verify_merkle_branch, BeaconHeader, CompactBeaconState, Error,
-	FinalizedBeaconState, LatestFinalizedBlockRoot, NextSyncCommittee, SyncCommitteePrepared,
+	FinalizedBeaconState, LatestFinalizedBlockRoot, LatestFreeSyncCommitteeUpdatePeriod,
+	NextSyncCommittee, SyncCommitteePrepared,
 };
-
-pub use crate::mock::*;
-
-use crate::config::{EPOCHS_PER_SYNC_COMMITTEE_PERIOD, SLOTS_PER_EPOCH, SLOTS_PER_HISTORICAL_ROOT};
 use frame_support::{assert_err, assert_noop, assert_ok, pallet_prelude::Pays};
 use hex_literal::hex;
 use snowbridge_beacon_primitives::{
@@ -374,7 +373,7 @@ fn submit_update_in_current_period() {
 		assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint));
 		let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone());
 		assert_ok!(result);
-		assert_eq!(result.unwrap().pays_fee, Pays::Yes);
+		assert_eq!(result.unwrap().pays_fee, Pays::No);
 		let block_root: H256 = update.finalized_header.hash_tree_root().unwrap();
 		assert!(<FinalizedBeaconState<Test>>::contains_key(block_root));
 	});
@@ -711,8 +710,40 @@ fn duplicate_sync_committee_updates_are_not_free() {
 		// Check that if the same update is submitted, the update is not free.
 		let second_result =
 			EthereumBeaconClient::submit(RuntimeOrigin::signed(1), sync_committee_update);
-		assert_err!(second_result, Error::<Test>::IrrelevantUpdate);
-		assert_eq!(second_result.unwrap_err().post_info.pays_fee, Pays::Yes);
+		assert_ok!(second_result);
+		assert_eq!(second_result.unwrap().pays_fee, Pays::Yes);
+	});
+}
+
+#[test]
+fn sync_committee_update_for_sync_committee_already_imported_are_not_free() {
+	let checkpoint = Box::new(load_checkpoint_update_fixture());
+	let sync_committee_update = Box::new(load_sync_committee_update_fixture());
+	let second_sync_committee_update = Box::new(load_sync_committee_update_period_0_fixture());
+	let third_sync_committee_update = Box::new(load_next_sync_committee_update_fixture());
+
+	new_tester().execute_with(|| {
+		assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint));
+		assert_eq!(<LatestFreeSyncCommitteeUpdatePeriod<Test>>::get(), 0);
+
+		// Check that setting the next sync committee for period 0 is free (it is not set yet).
+		let result =
+			EthereumBeaconClient::submit(RuntimeOrigin::signed(1), sync_committee_update.clone());
+		assert_ok!(result);
+		assert_eq!(result.unwrap().pays_fee, Pays::No);
+		assert_eq!(<LatestFreeSyncCommitteeUpdatePeriod<Test>>::get(), 0);
+
+		// Check that setting the next sync committee for period 0 again is not free.
+		let second_result =
+			EthereumBeaconClient::submit(RuntimeOrigin::signed(1), second_sync_committee_update);
+		assert_eq!(second_result.unwrap().pays_fee, Pays::Yes);
+		assert_eq!(<LatestFreeSyncCommitteeUpdatePeriod<Test>>::get(), 0);
+
+		// Check that setting the next sync committee for period 1 is free.
+		let third_result =
+			EthereumBeaconClient::submit(RuntimeOrigin::signed(1), third_sync_committee_update);
+		assert_eq!(third_result.unwrap().pays_fee, Pays::No);
+		assert_eq!(<LatestFreeSyncCommitteeUpdatePeriod<Test>>::get(), 1);
 	});
 }
 

bridges/snowbridge/pallets/ethereum-client/tests/fixtures/initial-checkpoint.json

@@ -1,10 +1,10 @@
 {
   "header": {
-    "slot": 864,
+    "slot": 64,
     "proposer_index": 4,
-    "parent_root": "0x614e7672f991ac268cd841055973f55e1e42228831a211adef207bb7329be614",
-    "state_root": "0x5fa8dfca3d760e4242ab46d529144627aa85348a19173b6e081172c701197a4a",
-    "body_root": "0x0f34c083b1803666bb1ac5e73fa71582731a2cf37d279ff0a3b0cad5a2ff371e"
+    "parent_root": "0x88e5b7e0dd468b334caf9281e0665184d2d712d7ffe632123ea07631b714920c",
+    "state_root": "0x82771f834d4d896f4969abdaf45f28f49a7437ecfca7bf2f7db7bfac5ca7224f",
+    "body_root": "0x8b36f34ceba40a29c9c6fa6266564c7df30ea75fecf1a85e6ec1cb4aabf4dc68"
   },
   "current_sync_committee": {
     "pubkeys": [
@@ -525,18 +525,18 @@
   },
   "current_sync_committee_branch": [
     "0x3ade38d498a062b50880a9409e1ca3a7fd4315d91eeb3bb83e56ac6bfe8d6a59",
-    "0xa9e90f89e7f90fd5d79a6bbcaf40ba5cfc05ab1b561ac51c84867c32248d5b1e",
-    "0xbd1a76b03e02402bb24a627de1980a80ab17691980271f597b844b89b497ef75",
-    "0x07bbcd27c7cad089023db046eda17e8209842b7d97add8b873519e84fe6480e7",
-    "0x94c11eeee4cb6192bf40810f23486d8c75dfbc2b6f28d988d6f74435ede243b0"
+    "0x058baa5628d6156e55ab99da54244be4a071978528f2eb3b19a4f4d7ab36f870",
+    "0x5f89984c1068b616e99589e161d2bb73b92c68b3422ef309ace434894b4503ae",
+    "0x4f1c230cf2bbe39502171956421fbe4f1c0a71a9691944019047b84584b371d5",
+    "0xbf8d5f6021db16e9b50e639e5c489eb8dc06449bf4ed17045cb949cb89a58a04"
   ],
   "validators_root": "0x270d43e74ce340de4bca2b1936beca0f4f5408d9e78aec4850920baf659d5b69",
-  "block_roots_root": "0xb9aab9c388c4e4fcd899b71f62c498fc73406e38e8eb14aa440e9affa06f2a10",
+  "block_roots_root": "0x2c453665ba6fc024116daf5246126e36085c61257cfbcce69d0bdcf89c766dc0",
   "block_roots_branch": [
-    "0x733422bd810895dab74cbbe07c69dd440cbb51f573181ad4dddac30fcdd0f41f",
-    "0x9b9eca73ab01d14549c325ba1b4610bb20bf1f8ec2dbd649f9d8cc7f3cea75fa",
-    "0xbcc666ad0ad9f9725cbd682bc95589d35b1b53b2a615f1e6e8dd5e086336becf",
-    "0x3069b547a08f703a1715016e926cbd64e71f93f64fb68d98d8c8f1ab745c46e5",
-    "0xc2de7e1097239404e17b263cfa0473533cc41e903cb03440d633bc5c27314cb4"
+    "0xbd04f51e43f63b0be48034920e8f5976111b7717225abccedbc6bcb327b95d00",
+    "0x758319a3bad11ee10fde1036551d982583c0392f284de5cc429b67fbd74c25d5",
+    "0xb42179d040c2bec20fa0a2750baf225b8097b5c9e4e22af9250cc773f4259427",
+    "0x5340ad5877c72dca689ca04bc8fedb78d67a4801d99887937edd8ccd29f87e82",
+    "0x9f03be8e70f74fc6b51e6ed03c96aabb544b5c50e5cdb8c0ab5001d1249d55f0"
   ]
-}
+}