From bbfcb8e9e3665d84ea83ad1d98f981a05a8978ee Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 13:09:52 +0200 Subject: [PATCH 01/13] free finalized updates --- .../pallets/ethereum-client/src/lib.rs | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 367eadc50aaa..6e0c36adcacd 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -34,7 +34,7 @@ mod tests; mod benchmarking; use frame_support::{ - dispatch::DispatchResult, pallet_prelude::OptionQuery, traits::Get, transactional, + dispatch::{DispatchResult, PostDispatchInfo}, pallet_prelude::OptionQuery, traits::Get, transactional, }; use frame_system::ensure_signed; use primitives::{ @@ -84,6 +84,8 @@ pub mod pallet { #[pallet::constant] type ForkVersions: Get; type WeightInfo: WeightInfo; + #[pallet::constant] + type FreeHeadersInterval: Get>; } #[pallet::event] @@ -205,11 +207,10 @@ pub mod pallet { #[transactional] /// Submits a new finalized beacon header update. The update may contain the next /// sync committee. - pub fn submit(origin: OriginFor, update: Box) -> DispatchResult { + pub fn submit(origin: OriginFor, update: Box) -> DispatchResultWithPostInfo { ensure_signed(origin)?; ensure!(!Self::operating_mode().is_halted(), Error::::Halted); - Self::process_update(&update)?; - Ok(()) + Self::process_update(&update) } /// Halt or resume all pallet operations. May only be called by root. @@ -281,10 +282,20 @@ pub mod pallet { Ok(()) } - pub(crate) fn process_update(update: &Update) -> DispatchResult { + pub(crate) fn process_update(update: &Update) -> DispatchResultWithPostInfo { Self::verify_update(update)?; Self::apply_update(update)?; - Ok(()) + + let pays_fee = if Self::may_refund_call_fee(update.finalized_header.slot) { Pays::No } else { Pays::Yes }; + + log::info!(target: LOG_TARGET, "Finalized header import, pays?: {:?}",pays_fee,); + + let actual_weight = match update.next_sync_committee_update { + None => T::WeightInfo::submit(), + Some(_) => T::WeightInfo::submit_with_sync_committee(), + }; + + Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee }) } /// References and strictly follows @@ -641,5 +652,19 @@ pub mod pallet { let signing_root = Self::compute_signing_root(header, domain)?; Ok(signing_root) } + + fn may_refund_call_fee( + improved_by_slot: u64, + ) -> bool { + // if configuration allows free non-mandatory headers and the header + // matches criteria => refund + if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { + if improved_by_slot >= free_headers_interval.into() { + return true; + } + } + + false + } } } From 839c39214942fc0db60001f750994da1a5a39c4d Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 13:12:34 +0200 Subject: [PATCH 02/13] update comment --- bridges/snowbridge/pallets/ethereum-client/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 6e0c36adcacd..1e13b6df4463 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -656,8 +656,8 @@ pub mod pallet { fn may_refund_call_fee( improved_by_slot: u64, ) -> bool { - // if configuration allows free non-mandatory headers and the header - // matches criteria => refund + // If free headers are allowed and the latest finalized header is larger than the + // minimum slot interval, the header import transaction is free. if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { if improved_by_slot >= free_headers_interval.into() { return true; From b13c057d7449b8fa8974bfb49e34b5c33e8549bd Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 13:16:13 +0200 Subject: [PATCH 03/13] fix check --- .../pallets/ethereum-client/src/lib.rs | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 1e13b6df4463..0024c3a62259 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -34,7 +34,10 @@ mod tests; mod benchmarking; use frame_support::{ - dispatch::{DispatchResult, PostDispatchInfo}, pallet_prelude::OptionQuery, traits::Get, transactional, + dispatch::{DispatchResult, PostDispatchInfo}, + pallet_prelude::OptionQuery, + traits::Get, + transactional, }; use frame_system::ensure_signed; use primitives::{ @@ -286,7 +289,11 @@ pub mod pallet { Self::verify_update(update)?; Self::apply_update(update)?; - let pays_fee = if Self::may_refund_call_fee(update.finalized_header.slot) { Pays::No } else { Pays::Yes }; + let pays_fee = if Self::may_refund_call_fee(update.finalized_header.slot) { + Pays::No + } else { + Pays::Yes + }; log::info!(target: LOG_TARGET, "Finalized header import, pays?: {:?}",pays_fee,); @@ -653,13 +660,16 @@ pub mod pallet { Ok(signing_root) } - fn may_refund_call_fee( - improved_by_slot: u64, - ) -> bool { + fn may_refund_call_fee(improved_by_slot: u64) -> bool { + // Get the latest stored finalized state. + let latest_finalized_state = + FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) + .ok_or(Error::::NotBootstrapped)?; + // If free headers are allowed and the latest finalized header is larger than the // minimum slot interval, the header import transaction is free. if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { - if improved_by_slot >= free_headers_interval.into() { + if latest_finalized_state.slot + free_headers_interval.into() >= improved_by_slot { return true; } } From 7030318766ad03a89cbfa9b7e6d718b0afcb5bc3 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 14:47:02 +0200 Subject: [PATCH 04/13] fixes --- .../snowbridge/pallets/ethereum-client/src/lib.rs | 13 +++++++------ .../snowbridge/pallets/ethereum-client/src/mock.rs | 2 ++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 0024c3a62259..a2b5536ac898 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -86,9 +86,9 @@ pub mod pallet { type RuntimeEvent: From> + IsType<::RuntimeEvent>; #[pallet::constant] type ForkVersions: Get; - type WeightInfo: WeightInfo; #[pallet::constant] type FreeHeadersInterval: Get>; + type WeightInfo: WeightInfo; } #[pallet::event] @@ -660,16 +660,17 @@ pub mod pallet { Ok(signing_root) } - fn may_refund_call_fee(improved_by_slot: u64) -> bool { + pub(super) fn may_refund_call_fee(improved_by_slot: u64) -> bool { // Get the latest stored finalized state. - let latest_finalized_state = - FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) - .ok_or(Error::::NotBootstrapped)?; + let latest_finalized_state = match FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) { + Some(state) => state, + None => return false, // Unexpected, would have been checked before. + }; // If free headers are allowed and the latest finalized header is larger than the // minimum slot interval, the header import transaction is free. if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { - if latest_finalized_state.slot + free_headers_interval.into() >= improved_by_slot { + if latest_finalized_state.slot + free_headers_interval as u64 >= improved_by_slot { return true; } } diff --git a/bridges/snowbridge/pallets/ethereum-client/src/mock.rs b/bridges/snowbridge/pallets/ethereum-client/src/mock.rs index b29584a0da77..c6473ca3d9d4 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/mock.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/mock.rs @@ -16,6 +16,7 @@ use frame_support::{ traits::{OnFinalize, OnInitialize}, }; use sp_runtime::BuildStorage; +use frame_support::traits::ConstU32; fn load_fixture(basename: String) -> Result where @@ -102,6 +103,7 @@ parameter_types! { impl ethereum_beacon_client::Config for Test { type RuntimeEvent = RuntimeEvent; type ForkVersions = ChainForkVersions; + type FreeHeadersInterval = ConstU32<5>; type WeightInfo = (); } From a18b81b423600f63a5c5fab019e66abbaa9f37c2 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 15:33:53 +0200 Subject: [PATCH 05/13] tests --- .../pallets/ethereum-client/src/lib.rs | 11 +++--- .../pallets/ethereum-client/src/mock.rs | 5 ++- .../pallets/ethereum-client/src/tests.rs | 36 ++++++++++++++----- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index a2b5536ac898..7b1f8b7563ba 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -662,15 +662,16 @@ pub mod pallet { pub(super) fn may_refund_call_fee(improved_by_slot: u64) -> bool { // Get the latest stored finalized state. - let latest_finalized_state = match FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) { - Some(state) => state, - None => return false, // Unexpected, would have been checked before. - }; + let latest_finalized_state = + match FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) { + Some(state) => state, + None => return false, // Unexpected, would have been checked before. + }; // If free headers are allowed and the latest finalized header is larger than the // minimum slot interval, the header import transaction is free. if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { - if latest_finalized_state.slot + free_headers_interval as u64 >= improved_by_slot { + if improved_by_slot >= latest_finalized_state.slot + free_headers_interval as u64 { return true; } } diff --git a/bridges/snowbridge/pallets/ethereum-client/src/mock.rs b/bridges/snowbridge/pallets/ethereum-client/src/mock.rs index c6473ca3d9d4..6471a18725a8 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/mock.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/mock.rs @@ -13,10 +13,9 @@ use std::{fs::File, path::PathBuf}; type Block = frame_system::mocking::MockBlock; use frame_support::{ migrations::MultiStepMigrator, - traits::{OnFinalize, OnInitialize}, + traits::{ConstU32, OnFinalize, OnInitialize}, }; use sp_runtime::BuildStorage; -use frame_support::traits::ConstU32; fn load_fixture(basename: String) -> Result where @@ -103,7 +102,7 @@ parameter_types! { impl ethereum_beacon_client::Config for Test { type RuntimeEvent = RuntimeEvent; type ForkVersions = ChainForkVersions; - type FreeHeadersInterval = ConstU32<5>; + type FreeHeadersInterval = ConstU32<96>; type WeightInfo = (); } diff --git a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs index ae476d2464e6..a193f5018164 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs @@ -1,15 +1,14 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023 Snowfork use crate::{ - functions::compute_period, sync_committee_sum, verify_merkle_branch, BeaconHeader, - CompactBeaconState, Error, FinalizedBeaconState, LatestFinalizedBlockRoot, NextSyncCommittee, - SyncCommitteePrepared, -}; - -use crate::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, + 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, }; pub use crate::mock::*; @@ -129,6 +128,25 @@ pub fn compute_domain_bls() { }); } +#[test] +pub fn may_refund_call_fee() { + let checkpoint = Box::new(load_checkpoint_update_fixture()); + let update = Box::new(load_sync_committee_update_fixture()); + + new_tester().execute_with(|| { + assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); + assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); + + let last_finalized_state = + FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()).unwrap(); + + // Not free, smaller than the allowed free header interval + assert!(!EthereumBeaconClient::may_refund_call_fee(last_finalized_state.slot + 30)); + // Is free, larger than the minimum interval + assert!(EthereumBeaconClient::may_refund_call_fee(last_finalized_state.slot + 200)); + }); +} + #[test] pub fn verify_merkle_branch_for_finalized_root() { new_tester().execute_with(|| { From 63bb960e30cbcdec09e0cb7a35a906f7deb4c46e Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 16:43:34 +0200 Subject: [PATCH 06/13] fixes --- .../pallets/ethereum-client/src/lib.rs | 42 ++++++------------- .../pallets/ethereum-client/src/tests.rs | 33 ++++++++------- 2 files changed, 30 insertions(+), 45 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 7b1f8b7563ba..0273b18489ae 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -287,22 +287,7 @@ pub mod pallet { pub(crate) fn process_update(update: &Update) -> DispatchResultWithPostInfo { Self::verify_update(update)?; - Self::apply_update(update)?; - - let pays_fee = if Self::may_refund_call_fee(update.finalized_header.slot) { - Pays::No - } else { - Pays::Yes - }; - - log::info!(target: LOG_TARGET, "Finalized header import, pays?: {:?}",pays_fee,); - - let actual_weight = match update.next_sync_committee_update { - None => T::WeightInfo::submit(), - Some(_) => T::WeightInfo::submit_with_sync_committee(), - }; - - Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee }) + Self::apply_update(update) } /// References and strictly follows @@ -452,7 +437,7 @@ pub mod pallet { /// Applies a finalized beacon header update to the beacon client. If a next sync committee /// is present in the update, verify the sync committee by converting it to a /// SyncCommitteePrepared type. Stores the provided finalized header. - fn apply_update(update: &Update) -> DispatchResult { + fn apply_update(update: &Update) -> DispatchResultWithPostInfo { let latest_finalized_state = FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) .ok_or(Error::::NotBootstrapped)?; @@ -484,11 +469,17 @@ pub mod pallet { }); }; + let pays_fee = Self::may_refund_call_fee(latest_finalized_state.slot, update.finalized_header.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)?; } - Ok(()) + Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee }) } /// Computes the signing root for a given beacon header and domain. The hash tree root @@ -660,23 +651,16 @@ pub mod pallet { Ok(signing_root) } - pub(super) fn may_refund_call_fee(improved_by_slot: u64) -> bool { - // Get the latest stored finalized state. - let latest_finalized_state = - match FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) { - Some(state) => state, - None => return false, // Unexpected, would have been checked before. - }; - + pub(super) fn may_refund_call_fee(latest_slot: u64, improved_by_slot: u64) -> Pays { // If free headers are allowed and the latest finalized header is larger than the // minimum slot interval, the header import transaction is free. if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { - if improved_by_slot >= latest_finalized_state.slot + free_headers_interval as u64 { - return true; + if improved_by_slot >= latest_slot + free_headers_interval as u64 { + return Pays::No; } } - false + Pays::Yes } } } diff --git a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs index a193f5018164..a6301ddaa0c3 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs @@ -15,6 +15,7 @@ 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}; +use frame_support::pallet_prelude::Pays; use hex_literal::hex; use primitives::{ types::deneb, Fork, ForkVersions, NextSyncCommitteeUpdate, VersionedExecutionPayloadHeader, @@ -130,20 +131,11 @@ pub fn compute_domain_bls() { #[test] pub fn may_refund_call_fee() { - let checkpoint = Box::new(load_checkpoint_update_fixture()); - let update = Box::new(load_sync_committee_update_fixture()); - new_tester().execute_with(|| { - assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); - - let last_finalized_state = - FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()).unwrap(); - // Not free, smaller than the allowed free header interval - assert!(!EthereumBeaconClient::may_refund_call_fee(last_finalized_state.slot + 30)); + assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 100), Pays::Yes); // Is free, larger than the minimum interval - assert!(EthereumBeaconClient::may_refund_call_fee(last_finalized_state.slot + 200)); + assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 300), Pays::No); }); } @@ -357,7 +349,10 @@ fn submit_update_in_current_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); + let update_result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()); + assert_ok!(update_result); + let post_info = update_result.unwrap(); + assert_eq!(post_info.pays_fee, Pays::Yes); let block_root: H256 = update.finalized_header.hash_tree_root().unwrap(); assert!(>::contains_key(block_root)); }); @@ -391,20 +386,26 @@ fn reject_submit_update_in_next_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - assert_ok!(EthereumBeaconClient::submit( + let update_result = EthereumBeaconClient::submit( RuntimeOrigin::signed(1), sync_committee_update.clone() - )); + ); + assert_ok!(update_result); + let post_info = update_result.unwrap(); + assert_eq!(post_info.pays_fee, Pays::Yes); // 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( + let second_update_result = EthereumBeaconClient::submit( RuntimeOrigin::signed(1), next_sync_committee_update - )); + ); + assert_ok!(second_update_result); + let second_post_info = second_update_result.unwrap(); + assert_eq!(second_post_info.pays_fee, Pays::No); // check same header in the next period can now be submitted successfully assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); }); From e01bc3179325f2215474b2be80b72b16559775ff Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 16:44:04 +0200 Subject: [PATCH 07/13] fmt --- .../snowbridge/pallets/ethereum-client/src/lib.rs | 5 ++++- .../pallets/ethereum-client/src/tests.rs | 15 +++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 0273b18489ae..8f2f24317b86 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -469,7 +469,10 @@ pub mod pallet { }); }; - let pays_fee = Self::may_refund_call_fee(latest_finalized_state.slot, update.finalized_header.slot); + let pays_fee = Self::may_refund_call_fee( + latest_finalized_state.slot, + update.finalized_header.slot, + ); let actual_weight = match update.next_sync_committee_update { None => T::WeightInfo::submit(), Some(_) => T::WeightInfo::submit_with_sync_committee(), diff --git a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs index a6301ddaa0c3..afec9ac6a7e6 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs @@ -14,8 +14,7 @@ use crate::{ 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}; -use frame_support::pallet_prelude::Pays; +use frame_support::{assert_err, assert_noop, assert_ok, pallet_prelude::Pays}; use hex_literal::hex; use primitives::{ types::deneb, Fork, ForkVersions, NextSyncCommitteeUpdate, VersionedExecutionPayloadHeader, @@ -386,10 +385,8 @@ fn reject_submit_update_in_next_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - let update_result = EthereumBeaconClient::submit( - RuntimeOrigin::signed(1), - sync_committee_update.clone() - ); + let update_result = + EthereumBeaconClient::submit(RuntimeOrigin::signed(1), sync_committee_update.clone()); assert_ok!(update_result); let post_info = update_result.unwrap(); assert_eq!(post_info.pays_fee, Pays::Yes); @@ -399,10 +396,8 @@ fn reject_submit_update_in_next_period() { Error::::SyncCommitteeUpdateRequired ); // submit update with next sync committee - let second_update_result = EthereumBeaconClient::submit( - RuntimeOrigin::signed(1), - next_sync_committee_update - ); + let second_update_result = + EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_sync_committee_update); assert_ok!(second_update_result); let second_post_info = second_update_result.unwrap(); assert_eq!(second_post_info.pays_fee, Pays::No); From 4aee593304b61023ea2a73a2937311d1734159c5 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 19:38:40 +0200 Subject: [PATCH 08/13] finishing touches --- .../pallets/ethereum-client/src/lib.rs | 14 +- .../pallets/ethereum-client/src/tests.rs | 173 +++++++++++------- .../src/bridge_to_ethereum_config.rs | 2 + 3 files changed, 118 insertions(+), 71 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 8f2f24317b86..ab1bdcb1bb88 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -441,6 +441,7 @@ pub mod pallet { let latest_finalized_state = FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) .ok_or(Error::::NotBootstrapped)?; + let mut sync_committee_updated = false; 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); @@ -467,11 +468,13 @@ pub mod pallet { Self::deposit_event(Event::SyncCommitteeUpdated { period: update_finalized_period, }); + sync_committee_updated = true; }; let pays_fee = Self::may_refund_call_fee( latest_finalized_state.slot, update.finalized_header.slot, + sync_committee_updated, ); let actual_weight = match update.next_sync_committee_update { None => T::WeightInfo::submit(), @@ -654,7 +657,16 @@ pub mod pallet { Ok(signing_root) } - pub(super) fn may_refund_call_fee(latest_slot: u64, improved_by_slot: u64) -> Pays { + pub(super) fn may_refund_call_fee( + latest_slot: u64, + improved_by_slot: u64, + sync_committee_updated: bool, + ) -> Pays { + // If the sync committee was successfully updated, the update may be free. + if sync_committee_updated { + return Pays::No; + } + // If free headers are allowed and the latest finalized header is larger than the // minimum slot interval, the header import transaction is free. if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { diff --git a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs index afec9ac6a7e6..eda987d22f45 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs @@ -132,9 +132,11 @@ pub fn compute_domain_bls() { pub fn may_refund_call_fee() { new_tester().execute_with(|| { // Not free, smaller than the allowed free header interval - assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 100), Pays::Yes); + assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 100, false), Pays::Yes); // Is free, larger than the minimum interval - assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 300), Pays::No); + assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 300, false), Pays::No); + // Is free, valid sync committee update + assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 100, true), Pays::No); }); } @@ -348,10 +350,9 @@ fn submit_update_in_current_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - let update_result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()); - assert_ok!(update_result); - let post_info = update_result.unwrap(); - assert_eq!(post_info.pays_fee, Pays::Yes); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::Yes); let block_root: H256 = update.finalized_header.hash_tree_root().unwrap(); assert!(>::contains_key(block_root)); }); @@ -368,7 +369,9 @@ fn submit_update_with_sync_committee_in_current_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); assert!(!>::exists()); - assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update)); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::No); assert!(>::exists()); }); } @@ -385,22 +388,21 @@ fn reject_submit_update_in_next_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - let update_result = + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), sync_committee_update.clone()); - assert_ok!(update_result); - let post_info = update_result.unwrap(); - assert_eq!(post_info.pays_fee, Pays::Yes); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::No); + // check an update in the next period is rejected - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()), - Error::::SyncCommitteeUpdateRequired - ); + let second_result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()); + assert_err!(second_result, Error::::SyncCommitteeUpdateRequired); + assert_eq!(second_result.unwrap_err().post_info.pays_fee, Pays::Yes); + // submit update with next sync committee - let second_update_result = + let third_result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_sync_committee_update); - assert_ok!(second_update_result); - let second_post_info = second_update_result.unwrap(); - assert_eq!(second_post_info.pays_fee, Pays::No); + assert_ok!(third_result); + assert_eq!(third_result.unwrap().pays_fee, Pays::No); // check same header in the next period can now be submitted successfully assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); }); @@ -418,10 +420,9 @@ fn submit_update_with_invalid_header_proof() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); assert!(!>::exists()); - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update), - Error::::InvalidHeaderMerkleProof - ); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_err!(result, Error::::InvalidHeaderMerkleProof); + assert_eq!(result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -437,10 +438,9 @@ fn submit_update_with_invalid_block_roots_proof() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); assert!(!>::exists()); - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update), - Error::::InvalidBlockRootsRootMerkleProof - ); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_err!(result, Error::::InvalidBlockRootsRootMerkleProof); + assert_eq!(result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -458,10 +458,9 @@ fn submit_update_with_invalid_next_sync_committee_proof() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); assert!(!>::exists()); - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update), - Error::::InvalidSyncCommitteeMerkleProof - ); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_err!(result, Error::::InvalidSyncCommitteeMerkleProof); + assert_eq!(result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -475,14 +474,14 @@ fn submit_update_with_skipped_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - assert_ok!(EthereumBeaconClient::submit( - RuntimeOrigin::signed(1), - sync_committee_update.clone() - )); - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update), - Error::::SkippedSyncCommitteePeriod - ); + let result = + EthereumBeaconClient::submit(RuntimeOrigin::signed(1), sync_committee_update.clone()); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::No); + + let second_result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_err!(second_result, Error::::SkippedSyncCommitteePeriod); + assert_eq!(second_result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -498,9 +497,16 @@ fn submit_update_with_sync_committee_in_next_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); assert!(!>::exists()); - assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); + + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::No); assert!(>::exists()); - assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone())); + + let second_result = + EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()); + assert_ok!(second_result); + assert_eq!(second_result.unwrap().pays_fee, Pays::No); let last_finalized_state = FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()).unwrap(); let last_synced_period = compute_period(last_finalized_state.slot); @@ -516,13 +522,12 @@ fn submit_update_with_sync_committee_invalid_signature_slot() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - // makes a invalid update with signature_slot should be more than attested_slot + // makes an invalid update with signature_slot should be more than attested_slot update.signature_slot = update.attested_header.slot; - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update), - Error::::InvalidUpdateSlot - ); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_err!(result, Error::::InvalidUpdateSlot); + assert_eq!(result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -536,10 +541,9 @@ fn submit_update_with_skipped_sync_committee_period() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), finalized_update), - Error::::SkippedSyncCommitteePeriod - ); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), finalized_update); + assert_err!(result, Error::::SkippedSyncCommitteePeriod); + assert_eq!(result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -557,10 +561,9 @@ fn submit_irrelevant_update() { update.attested_header.slot = checkpoint.header.slot; update.signature_slot = checkpoint.header.slot + 1; - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update), - Error::::IrrelevantUpdate - ); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_err!(result, Error::::IrrelevantUpdate); + assert_eq!(result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -569,10 +572,9 @@ fn submit_update_with_missing_bootstrap() { let update = Box::new(load_next_finalized_header_update_fixture()); new_tester().execute_with(|| { - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update), - Error::::NotBootstrapped - ); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_err!(result, Error::::NotBootstrapped); + assert_eq!(result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -585,7 +587,9 @@ fn submit_update_with_invalid_sync_committee_update() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update)); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::No); // makes update with invalid next_sync_committee >::mutate(>::get(), |x| { @@ -597,10 +601,9 @@ fn submit_update_with_invalid_sync_committee_update() { let next_sync_committee = NextSyncCommitteeUpdate::default(); next_update.next_sync_committee_update = Some(next_sync_committee); - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update), - Error::::InvalidSyncCommitteeUpdate - ); + let second_result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update); + assert_err!(second_result, Error::::InvalidSyncCommitteeUpdate); + assert_eq!(second_result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -623,12 +626,15 @@ fn submit_finalized_header_update_with_too_large_gap() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::No); assert!(>::exists()); - assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()), - Error::::InvalidFinalizedHeaderGap - ); + + let second_result = + EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()); + assert_err!(second_result, Error::::InvalidFinalizedHeaderGap); + assert_eq!(second_result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -648,14 +654,41 @@ fn submit_finalized_header_update_with_gap_at_limit() { new_tester().execute_with(|| { assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); - assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); + + let result = EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone()); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::No); assert!(>::exists()); + + let second_result = + EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()); assert_err!( - EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()), + second_result, // 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::::InvalidHeaderMerkleProof ); + assert_eq!(second_result.unwrap_err().post_info.pays_fee, Pays::Yes); + }); +} + +#[test] +fn duplicate_sync_committee_updates_are_not_free() { + let checkpoint = Box::new(load_checkpoint_update_fixture()); + let sync_committee_update = Box::new(load_sync_committee_update_fixture()); + + new_tester().execute_with(|| { + assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); + let result = + EthereumBeaconClient::submit(RuntimeOrigin::signed(1), sync_committee_update.clone()); + assert_ok!(result); + assert_eq!(result.unwrap().pays_fee, Pays::No); + + // 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::::IrrelevantUpdate); + assert_eq!(second_result.unwrap_err().post_info.pays_fee, Pays::Yes); }); } diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_ethereum_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_ethereum_config.rs index 53a536655408..a5d798835ac8 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_ethereum_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_to_ethereum_config.rs @@ -162,6 +162,8 @@ parameter_types! { impl snowbridge_pallet_ethereum_client::Config for Runtime { type RuntimeEvent = RuntimeEvent; type ForkVersions = ChainForkVersions; + // Free consensus update every epoch. Works out to be 225 updates per day. + type FreeHeadersInterval = ConstU32<32>; type WeightInfo = crate::weights::snowbridge_pallet_ethereum_client::WeightInfo; } From 170cdbc5d7f21d4e6ad207f47d4d5f8bd1d8dbc1 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 19:51:54 +0200 Subject: [PATCH 09/13] comments --- bridges/snowbridge/pallets/ethereum-client/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index ab1bdcb1bb88..46132dff117f 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -436,7 +436,8 @@ pub mod pallet { /// Reference and strictly follows DispatchResultWithPostInfo { let latest_finalized_state = FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) @@ -657,6 +658,9 @@ pub mod pallet { Ok(signing_root) } + /// Updates are free if the update is successful and the interval between the latest + /// finalized header in storage and the newly imported header is large enough. All + /// successful sync committee updates are free. pub(super) fn may_refund_call_fee( latest_slot: u64, improved_by_slot: u64, From 2483147adf0d4b5d315b98eb181654fd5d81e413 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 18 Jul 2024 20:16:03 +0200 Subject: [PATCH 10/13] adds missing impl --- bridges/snowbridge/pallets/inbound-queue/src/mock.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/bridges/snowbridge/pallets/inbound-queue/src/mock.rs b/bridges/snowbridge/pallets/inbound-queue/src/mock.rs index 05481ca2f6b4..77a4a3183dc2 100644 --- a/bridges/snowbridge/pallets/inbound-queue/src/mock.rs +++ b/bridges/snowbridge/pallets/inbound-queue/src/mock.rs @@ -110,6 +110,7 @@ parameter_types! { impl snowbridge_pallet_ethereum_client::Config for Test { type RuntimeEvent = RuntimeEvent; type ForkVersions = ChainForkVersions; + type FreeHeadersInterval = ConstU32<32>; type WeightInfo = (); } From c82647fa8a4ebbbd7befa4a42e59295f99773691 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Tue, 30 Jul 2024 13:06:32 +0200 Subject: [PATCH 11/13] adds test --- .github/workflows/parachain.yml | 1 + .../snowbridge/runtime/test-common/src/lib.rs | 76 ++++++++++++++++++- .../bridge-hub-rococo/tests/tests.rs | 10 ++- .../src/bridge_to_ethereum_config.rs | 1 + 4 files changed, 80 insertions(+), 8 deletions(-) diff --git a/.github/workflows/parachain.yml b/.github/workflows/parachain.yml index 83cc69b1c481..0df1fe3872a0 100644 --- a/.github/workflows/parachain.yml +++ b/.github/workflows/parachain.yml @@ -66,6 +66,7 @@ jobs: -p snowbridge-ethereum -p snowbridge-router-primitives -p snowbridge-runtime-common + -p snowbridge-runtime-test-common -p bridge-hub-rococo-runtime -p bridge-hub-rococo-integration-tests -p asset-hub-rococo-integration-tests diff --git a/bridges/snowbridge/runtime/test-common/src/lib.rs b/bridges/snowbridge/runtime/test-common/src/lib.rs index 8f36313e360f..2b5bfb8ff431 100644 --- a/bridges/snowbridge/runtime/test-common/src/lib.rs +++ b/bridges/snowbridge/runtime/test-common/src/lib.rs @@ -12,7 +12,7 @@ use parachains_runtimes_test_utils::{ }; use snowbridge_core::{ChannelId, ParaId}; use snowbridge_pallet_ethereum_client_fixtures::*; -use sp_core::{H160, U256}; +use sp_core::{Get, H160, U256}; use sp_keyring::AccountKeyring::*; use sp_runtime::{traits::Header, AccountId32, DigestItem, SaturatedConversion, Saturating}; use xcm::{ @@ -466,23 +466,37 @@ pub fn ethereum_extrinsic( let initial_checkpoint = make_checkpoint(); let update = make_finalized_header_update(); let sync_committee_update = make_sync_committee_update(); + let mut invalid_update = make_finalized_header_update(); + let mut invalid_sync_committee_update = make_sync_committee_update(); + invalid_update.finalized_header.slot = 4354; + invalid_sync_committee_update.finalized_header.slot = 4354; let alice = Alice; let alice_account = alice.to_account_id(); >::mint_into( - &alice_account.into(), + &alice_account.clone().into(), 10_000_000_000_000_u128.saturated_into::>(), ) .unwrap(); + let balance_before = + >::free_balance(&alice_account.clone().into()); assert_ok!(>::force_checkpoint( RuntimeHelper::::root_origin(), - initial_checkpoint, + initial_checkpoint.clone(), )); + let balance_after_checkpoint = + >::free_balance(&alice_account.clone().into()); let update_call: ::RuntimeCall = snowbridge_pallet_ethereum_client::Call::::submit { - update: Box::new(*update), + update: Box::new(*update.clone()), + } + .into(); + + let invalid_update_call: ::RuntimeCall = + snowbridge_pallet_ethereum_client::Call::::submit { + update: Box::new(*invalid_update), } .into(); @@ -492,12 +506,66 @@ pub fn ethereum_extrinsic( } .into(); + let invalid_update_sync_committee_call: ::RuntimeCall = + snowbridge_pallet_ethereum_client::Call::::submit { + update: Box::new(*invalid_sync_committee_update), + } + .into(); + + // Finalized header update let update_outcome = construct_and_apply_extrinsic(alice, update_call.into()); assert_ok!(update_outcome); + let balance_after_update = + >::free_balance(&alice_account.clone().into()); + + // Invalid finalized header update + let invalid_update_outcome = + construct_and_apply_extrinsic(alice, invalid_update_call.into()); + assert_err!( + invalid_update_outcome, + snowbridge_pallet_ethereum_client::Error::::InvalidUpdateSlot + ); + let balance_after_invalid_update = + >::free_balance(&alice_account.clone().into()); + // Sync committee update let sync_committee_outcome = construct_and_apply_extrinsic(alice, update_sync_committee_call.into()); assert_ok!(sync_committee_outcome); + let balance_after_sync_com_update = + >::free_balance(&alice_account.clone().into()); + + // Invalid sync committee update + let invalid_sync_committee_outcome = + construct_and_apply_extrinsic(alice, invalid_update_sync_committee_call.into()); + assert_err!( + invalid_sync_committee_outcome, + snowbridge_pallet_ethereum_client::Error::::InvalidUpdateSlot + ); + let balance_after_invalid_sync_com_update = + >::free_balance(&alice_account.clone().into()); + + // Assert paid operations are charged and free operations are free + // Checkpoint is a free operation + assert!(balance_before == balance_after_checkpoint); + let gap = + ::FreeHeadersInterval::get(); + // Large enough header gap is free + if gap.is_some() && + update.finalized_header.slot >= + initial_checkpoint.header.slot + gap.unwrap() as u64 + { + assert!(balance_after_checkpoint == balance_after_update); + } else { + // Otherwise paid + assert!(balance_after_checkpoint > balance_after_update); + } + // An invalid update is paid + assert!(balance_after_update > balance_after_invalid_update); + // A successful sync committee update is free + assert!(balance_after_invalid_update == balance_after_sync_com_update); + // An invalid sync committee update is paid + assert!(balance_after_sync_com_update > balance_after_invalid_sync_com_update); }); } diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs index 1d3d9e55f7ee..5964dd241bca 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs @@ -18,11 +18,13 @@ use bp_polkadot_core::Signature; use bridge_hub_rococo_runtime::{ - bridge_common_config, bridge_to_bulletin_config, bridge_to_westend_config, + bridge_common_config, bridge_to_bulletin_config, + bridge_to_ethereum_config::EthereumGatewayAddress, + bridge_to_westend_config, xcm_config::{RelayNetwork, TokenLocation, XcmConfig}, - AllPalletsWithoutSystem, BridgeRejectObsoleteHeadersAndMessages, EthereumGatewayAddress, - Executive, ExistentialDeposit, ParachainSystem, PolkadotXcm, Runtime, RuntimeCall, - RuntimeEvent, RuntimeOrigin, SessionKeys, SignedExtra, TransactionPayment, UncheckedExtrinsic, + AllPalletsWithoutSystem, BridgeRejectObsoleteHeadersAndMessages, Executive, ExistentialDeposit, + ParachainSystem, PolkadotXcm, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, SessionKeys, + SignedExtra, TransactionPayment, UncheckedExtrinsic, }; use bridge_hub_test_utils::SlotDurations; use codec::{Decode, Encode}; diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs index 7922d3ed02b1..82651b11aa51 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs @@ -163,6 +163,7 @@ parameter_types! { impl snowbridge_pallet_ethereum_client::Config for Runtime { type RuntimeEvent = RuntimeEvent; type ForkVersions = ChainForkVersions; + type FreeHeadersInterval = ConstU32<32>; type WeightInfo = crate::weights::snowbridge_pallet_ethereum_client::WeightInfo; } From e3281f8f1504d092eb0cfca042d1dec4dea7ad33 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Tue, 30 Jul 2024 16:06:03 +0200 Subject: [PATCH 12/13] fixes --- .../pallets/ethereum-client/src/lib.rs | 27 ++++++------------- .../pallets/ethereum-client/src/tests.rs | 11 +++++--- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 418a8ca1cfa4..09c0bb54d8db 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -86,8 +86,9 @@ pub mod pallet { type RuntimeEvent: From> + IsType<::RuntimeEvent>; #[pallet::constant] type ForkVersions: Get; + /// Minimum gap between finalized headers for an update to be free. #[pallet::constant] - type FreeHeadersInterval: Get>; + type FreeHeadersInterval: Get; type WeightInfo: WeightInfo; } @@ -442,7 +443,6 @@ pub mod pallet { let latest_finalized_state = FinalizedBeaconState::::get(LatestFinalizedBlockRoot::::get()) .ok_or(Error::::NotBootstrapped)?; - let mut sync_committee_updated = false; 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); @@ -469,14 +469,9 @@ pub mod pallet { Self::deposit_event(Event::SyncCommitteeUpdated { period: update_finalized_period, }); - sync_committee_updated = true; }; - let pays_fee = Self::may_refund_call_fee( - latest_finalized_state.slot, - update.finalized_header.slot, - sync_committee_updated, - ); + 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(), @@ -651,7 +646,7 @@ pub mod pallet { config::SLOTS_PER_EPOCH as u64, )); let domain_type = config::DOMAIN_SYNC_COMMITTEE.to_vec(); - // Domains are used for for seeds, for signatures, and for selecting aggregators. + // Domains are used for seeds, for signatures, and for selecting aggregators. let domain = Self::compute_domain(domain_type, fork_version, validators_root)?; // Hash tree root of SigningData - object root + domain let signing_root = Self::compute_signing_root(header, domain)?; @@ -661,22 +656,16 @@ pub mod pallet { /// Updates are free if the update is successful and the interval between the latest /// finalized header in storage and the newly imported header is large enough. All /// successful sync committee updates are free. - pub(super) fn may_refund_call_fee( - latest_slot: u64, - improved_by_slot: u64, - sync_committee_updated: bool, - ) -> Pays { + pub(super) fn check_refundable(update: &Update, latest_slot: u64) -> Pays { // If the sync committee was successfully updated, the update may be free. - if sync_committee_updated { + if update.next_sync_committee_update.is_some() { return Pays::No; } // If free headers are allowed and the latest finalized header is larger than the // minimum slot interval, the header import transaction is free. - if let Some(free_headers_interval) = T::FreeHeadersInterval::get() { - if improved_by_slot >= latest_slot + free_headers_interval as u64 { - return Pays::No; - } + if update.finalized_header.slot >= latest_slot + T::FreeHeadersInterval::get() as u64 { + return Pays::No; } Pays::Yes diff --git a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs index d969c6724dea..bc0ecaed674a 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs @@ -130,13 +130,18 @@ pub fn compute_domain_bls() { #[test] pub fn may_refund_call_fee() { + let finalized_update = Box::new(load_next_finalized_header_update_fixture()); + let sync_committee_update = Box::new(load_sync_committee_update_fixture()); new_tester().execute_with(|| { // Not free, smaller than the allowed free header interval - assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 100, false), Pays::Yes); + assert_eq!( + EthereumBeaconClient::check_refundable(&finalized_update.clone(), 8190), + Pays::Yes + ); // Is free, larger than the minimum interval - assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 300, false), Pays::No); + assert_eq!(EthereumBeaconClient::check_refundable(&finalized_update, 8000), Pays::No); // Is free, valid sync committee update - assert_eq!(EthereumBeaconClient::may_refund_call_fee(96, 100, true), Pays::No); + assert_eq!(EthereumBeaconClient::check_refundable(&sync_committee_update, 8190), Pays::No); }); } From d034cb89ebec59c2685d9f87fe71f5a3fe9503a9 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Tue, 30 Jul 2024 19:36:28 +0200 Subject: [PATCH 13/13] fix --- bridges/snowbridge/runtime/test-common/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bridges/snowbridge/runtime/test-common/src/lib.rs b/bridges/snowbridge/runtime/test-common/src/lib.rs index 2b5bfb8ff431..b157ad4356bd 100644 --- a/bridges/snowbridge/runtime/test-common/src/lib.rs +++ b/bridges/snowbridge/runtime/test-common/src/lib.rs @@ -551,10 +551,7 @@ pub fn ethereum_extrinsic( let gap = ::FreeHeadersInterval::get(); // Large enough header gap is free - if gap.is_some() && - update.finalized_header.slot >= - initial_checkpoint.header.slot + gap.unwrap() as u64 - { + if update.finalized_header.slot >= initial_checkpoint.header.slot + gap as u64 { assert!(balance_after_checkpoint == balance_after_update); } else { // Otherwise paid