From cf45b1477764612cb6c3bda933456eef41cac345 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Mon, 18 Mar 2024 08:16:40 +0200 Subject: [PATCH 1/5] adds beacon header age check --- .../pallets/ethereum-client/src/lib.rs | 15 + .../pallets/ethereum-client/src/tests.rs | 57 +++- .../ethereum-beacon-client/src/mock.rs | 259 ------------------ 3 files changed, 71 insertions(+), 260 deletions(-) delete mode 100644 bridges/snowbridge/parachain/pallets/ethereum-beacon-client/src/mock.rs diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index a54d4a05ac58..fc2ab2fbb588 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -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, @@ -398,6 +402,17 @@ pub mod pallet { Error::::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. + ensure!( + latest_finalized_state + .slot + .saturating_add(config::SLOTS_PER_HISTORICAL_ROOT as u64) >= + update.finalized_header.slot, + Error::::InvalidFinalizedHeaderGap + ); + // 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 diff --git a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs index 50b6a25c3428..4a7b7b458869 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/tests.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/tests.rs @@ -15,7 +15,7 @@ use crate::mock::{ pub use crate::mock::*; -use crate::config::{EPOCHS_PER_SYNC_COMMITTEE_PERIOD, SLOTS_PER_EPOCH}; +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 hex_literal::hex; use primitives::{ @@ -884,6 +884,61 @@ 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 + SLOTS_PER_HISTORICAL_ROOT 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!(>::exists()); + assert_err!( + EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()), + Error::::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()); + + next_update.finalized_header.slot = checkpoint.header.slot + SLOTS_PER_HISTORICAL_ROOT as u64; + // 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 + SLOTS_PER_HISTORICAL_ROOT as u64 + 33; + next_update.signature_slot = checkpoint.header.slot + SLOTS_PER_HISTORICAL_ROOT as u64 + 43; + + new_tester().execute_with(|| { + assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint)); + assert_ok!(EthereumBeaconClient::submit(RuntimeOrigin::signed(1), update.clone())); + assert!(>::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::::InvalidHeaderMerkleProof + ); + }); +} + /* IMPLS */ #[test] diff --git a/bridges/snowbridge/parachain/pallets/ethereum-beacon-client/src/mock.rs b/bridges/snowbridge/parachain/pallets/ethereum-beacon-client/src/mock.rs deleted file mode 100644 index d2cca373e92b..000000000000 --- a/bridges/snowbridge/parachain/pallets/ethereum-beacon-client/src/mock.rs +++ /dev/null @@ -1,259 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: 2023 Snowfork -use crate as ethereum_beacon_client; -use frame_support::parameter_types; -use pallet_timestamp; -use primitives::{Fork, ForkVersions}; -use sp_core::H256; -use sp_runtime::traits::{BlakeTwo256, IdentityLookup}; - -#[cfg(not(feature = "beacon-spec-mainnet"))] -pub mod minimal { - use super::*; - - use crate::config; - use frame_support::derive_impl; - use hex_literal::hex; - use primitives::CompactExecutionHeader; - use snowbridge_core::inbound::{Log, Proof}; - use sp_runtime::BuildStorage; - use std::{fs::File, path::PathBuf}; - - type Block = frame_system::mocking::MockBlock; - - frame_support::construct_runtime!( - pub enum Test { - System: frame_system::{Pallet, Call, Storage, Event}, - Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, - EthereumBeaconClient: ethereum_beacon_client::{Pallet, Call, Storage, Event}, - } - ); - - parameter_types! { - pub const BlockHashCount: u64 = 250; - pub const SS58Prefix: u8 = 42; - } - - #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] - impl frame_system::Config for Test { - type BaseCallFilter = frame_support::traits::Everything; - type RuntimeOrigin = RuntimeOrigin; - type RuntimeCall = RuntimeCall; - type RuntimeTask = RuntimeTask; - type Hash = H256; - type Hashing = BlakeTwo256; - type AccountId = u64; - type Lookup = IdentityLookup; - type RuntimeEvent = RuntimeEvent; - type BlockHashCount = BlockHashCount; - type PalletInfo = PalletInfo; - type SS58Prefix = SS58Prefix; - type Nonce = u64; - type Block = Block; - } - - impl pallet_timestamp::Config for Test { - type Moment = u64; - type OnTimestampSet = (); - type MinimumPeriod = (); - type WeightInfo = (); - } - - parameter_types! { - pub const ExecutionHeadersPruneThreshold: u32 = 10; - pub const ChainForkVersions: ForkVersions = ForkVersions{ - genesis: Fork { - version: [0, 0, 0, 1], // 0x00000001 - epoch: 0, - }, - altair: Fork { - version: [1, 0, 0, 1], // 0x01000001 - epoch: 0, - }, - bellatrix: Fork { - version: [2, 0, 0, 1], // 0x02000001 - epoch: 0, - }, - capella: Fork { - version: [3, 0, 0, 1], // 0x03000001 - epoch: 0, - }, - }; - } - - impl ethereum_beacon_client::Config for Test { - type RuntimeEvent = RuntimeEvent; - type ForkVersions = ChainForkVersions; - type MaxExecutionHeadersToKeep = ExecutionHeadersPruneThreshold; - type WeightInfo = (); - } - - // Build genesis storage according to the mock runtime. - pub fn new_tester() -> sp_io::TestExternalities { - let t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - let mut ext = sp_io::TestExternalities::new(t); - let _ = ext.execute_with(|| Timestamp::set(RuntimeOrigin::signed(1), 30_000)); - ext - } - - fn load_fixture(basename: &str) -> Result - where - T: for<'de> serde::Deserialize<'de>, - { - let filepath: PathBuf = - [env!("CARGO_MANIFEST_DIR"), "tests", "fixtures", basename].iter().collect(); - serde_json::from_reader(File::open(filepath).unwrap()) - } - - pub fn load_execution_header_update_fixture() -> primitives::ExecutionHeaderUpdate { - load_fixture("execution-header-update.minimal.json").unwrap() - } - - pub fn load_checkpoint_update_fixture( - ) -> primitives::CheckpointUpdate<{ config::SYNC_COMMITTEE_SIZE }> { - load_fixture("initial-checkpoint.minimal.json").unwrap() - } - - pub fn load_sync_committee_update_fixture( - ) -> primitives::Update<{ config::SYNC_COMMITTEE_SIZE }, { config::SYNC_COMMITTEE_BITS_SIZE }> { - load_fixture("sync-committee-update.minimal.json").unwrap() - } - - pub fn load_finalized_header_update_fixture( - ) -> primitives::Update<{ config::SYNC_COMMITTEE_SIZE }, { config::SYNC_COMMITTEE_BITS_SIZE }> { - load_fixture("finalized-header-update.minimal.json").unwrap() - } - - pub fn load_next_sync_committee_update_fixture( - ) -> primitives::Update<{ config::SYNC_COMMITTEE_SIZE }, { config::SYNC_COMMITTEE_BITS_SIZE }> { - load_fixture("next-sync-committee-update.minimal.json").unwrap() - } - - pub fn load_next_finalized_header_update_fixture( - ) -> primitives::Update<{ config::SYNC_COMMITTEE_SIZE }, { config::SYNC_COMMITTEE_BITS_SIZE }> { - load_fixture("next-finalized-header-update.minimal.json").unwrap() - } - - pub fn get_message_verification_payload() -> (Log, Proof) { - ( - Log { - address: hex!("ee9170abfbf9421ad6dd07f6bdec9d89f2b581e0").into(), - topics: vec![ - hex!("1b11dcf133cc240f682dab2d3a8e4cd35c5da8c9cf99adac4336f8512584c5ad").into(), - hex!("00000000000000000000000000000000000000000000000000000000000003e8").into(), - hex!("0000000000000000000000000000000000000000000000000000000000000001").into(), - ], - data: hex!("0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004b000f000000000000000100d184c103f7acc340847eee82a0b909e3358bc28d440edffa1352b13227e8ee646f3ea37456dec701345772617070656420457468657210574554481235003511000000000000000000000000000000000000000000").into(), - }, - Proof { - block_hash: hex!("05aaa60b0f27cce9e71909508527264b77ee14da7b5bf915fcc4e32715333213").into(), - tx_index: 0, - data: (vec![ - hex!("cf0d1c1ba57d1e0edfb59786c7e30c2b7e12bd54612b00cd21c4eaeecedf44fb").to_vec(), - hex!("d21fc4f68ab05bc4dcb23c67008e92c4d466437cdd6ed7aad0c008944c185510").to_vec(), - hex!("b9890f91ca0d77aa2a4adfaf9b9e40c94cac9e638b6d9797923865872944b646").to_vec(), - ], vec![ - hex!("f90131a0b601337b3aa10a671caa724eba641e759399979856141d3aea6b6b4ac59b889ba00c7d5dd48be9060221a02fb8fa213860b4c50d47046c8fa65ffaba5737d569e0a094601b62a1086cd9c9cb71a7ebff9e718f3217fd6e837efe4246733c0a196f63a06a4b0dd0aefc37b3c77828c8f07d1b7a2455ceb5dbfd3c77d7d6aeeddc2f7e8ca0d6e8e23142cdd8ec219e1f5d8b56aa18e456702b195deeaa210327284d42ade4a08a313d4c87023005d1ab631bbfe3f5de1e405d0e66d0bef3e033f1e5711b5521a0bf09a5d9a48b10ade82b8d6a5362a15921c8b5228a3487479b467db97411d82fa0f95cccae2a7c572ef3c566503e30bac2b2feb2d2f26eebf6d870dcf7f8cf59cea0d21fc4f68ab05bc4dcb23c67008e92c4d466437cdd6ed7aad0c008944c1855108080808080808080").to_vec(), - hex!("f851a0b9890f91ca0d77aa2a4adfaf9b9e40c94cac9e638b6d9797923865872944b646a060a634b9280e3a23fb63375e7bbdd9ab07fd379ab6a67e2312bbc112195fa358808080808080808080808080808080").to_vec(), - hex!("f9030820b9030402f90300018301d6e2b9010000000000000800000000000020040008000000000000000000000000400000008000000000000000000000000000000000000000000000000000000000042010000000001000000000000000000000000000000000040000000000000000000000000000000000000000000000008000000000000000002000000000000000000000000200000000000000200000000000100000000040000001000200008000000000000200000000000000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000000f901f5f87a942ffa5ecdbe006d30397c7636d3e015eee251369ff842a0c965575a00553e094ca7c5d14f02e107c258dda06867cbf9e0e69f80e71bbcc1a000000000000000000000000000000000000000000000000000000000000003e8a000000000000000000000000000000000000000000000000000000000000003e8f9011c94ee9170abfbf9421ad6dd07f6bdec9d89f2b581e0f863a01b11dcf133cc240f682dab2d3a8e4cd35c5da8c9cf99adac4336f8512584c5ada000000000000000000000000000000000000000000000000000000000000003e8a00000000000000000000000000000000000000000000000000000000000000001b8a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004b000f000000000000000100d184c103f7acc340847eee82a0b909e3358bc28d440edffa1352b13227e8ee646f3ea37456dec701345772617070656420457468657210574554481235003511000000000000000000000000000000000000000000f858948cf6147918a5cbb672703f879f385036f8793a24e1a01449abf21e49fd025f33495e77f7b1461caefdd3d4bb646424a3f445c4576a5ba0000000000000000000000000440edffa1352b13227e8ee646f3ea37456dec701").to_vec(), - ]), - } - ) - } - - pub fn get_message_verification_header() -> CompactExecutionHeader { - CompactExecutionHeader { - parent_hash: hex!("04a7f6ab8282203562c62f38b0ab41d32aaebe2c7ea687702b463148a6429e04") - .into(), - block_number: 55, - state_root: hex!("894d968712976d613519f973a317cb0781c7b039c89f27ea2b7ca193f7befdb3") - .into(), - receipts_root: hex!("cf0d1c1ba57d1e0edfb59786c7e30c2b7e12bd54612b00cd21c4eaeecedf44fb") - .into(), - } - } -} - -#[cfg(feature = "beacon-spec-mainnet")] -pub mod mainnet { - use super::*; - use frame_support::derive_impl; - - type Block = frame_system::mocking::MockBlock; - use sp_runtime::BuildStorage; - - frame_support::construct_runtime!( - pub enum Test { - System: frame_system::{Pallet, Call, Storage, Event}, - Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, - EthereumBeaconClient: ethereum_beacon_client::{Pallet, Call, Storage, Event}, - } - ); - - parameter_types! { - pub const BlockHashCount: u64 = 250; - pub const SS58Prefix: u8 = 42; - } - - #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] - impl frame_system::Config for Test { - type BaseCallFilter = frame_support::traits::Everything; - type RuntimeOrigin = RuntimeOrigin; - type RuntimeCall = RuntimeCall; - type RuntimeTask = RuntimeTask; - type Hash = H256; - type Hashing = BlakeTwo256; - type AccountId = u64; - type Lookup = IdentityLookup; - type RuntimeEvent = RuntimeEvent; - type BlockHashCount = BlockHashCount; - type PalletInfo = PalletInfo; - type SS58Prefix = SS58Prefix; - type Nonce = u64; - type Block = Block; - } - - impl pallet_timestamp::Config for Test { - type Moment = u64; - type OnTimestampSet = (); - type MinimumPeriod = (); - type WeightInfo = (); - } - - parameter_types! { - pub const ChainForkVersions: ForkVersions = ForkVersions{ - genesis: Fork { - version: [0, 0, 16, 32], // 0x00001020 - epoch: 0, - }, - altair: Fork { - version: [1, 0, 16, 32], // 0x01001020 - epoch: 36660, - }, - bellatrix: Fork { - version: [2, 0, 16, 32], // 0x02001020 - epoch: 112260, - }, - capella: Fork { - version: [3, 0, 16, 32], // 0x03001020 - epoch: 162304, - }, - }; - pub const ExecutionHeadersPruneThreshold: u32 = 10; - } - - impl ethereum_beacon_client::Config for Test { - type RuntimeEvent = RuntimeEvent; - type ForkVersions = ChainForkVersions; - type MaxExecutionHeadersToKeep = ExecutionHeadersPruneThreshold; - type WeightInfo = (); - } - - // Build genesis storage according to the mock runtime. - pub fn new_tester() -> sp_io::TestExternalities { - let t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - let mut ext = sp_io::TestExternalities::new(t); - let _ = ext.execute_with(|| Timestamp::set(RuntimeOrigin::signed(1), 30_000)); - ext - } -} From aebec13d84815eccc0a3b2937a9540ca4c867d18 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 21 Mar 2024 10:52:57 +0200 Subject: [PATCH 2/5] change bitwise or to logical or --- .../runtimes/assets/asset-hub-rococo/src/xcm_config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs index c438361cc177..71c0c6201287 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs @@ -286,8 +286,8 @@ impl Contains for SafeCallFilter { match call { RuntimeCall::System(frame_system::Call::set_storage { items }) if items.iter().all(|(k, _)| { - k.eq(&bridging::XcmBridgeHubRouterByteFee::key()) | - k.eq(&bridging::XcmBridgeHubRouterBaseFee::key()) | + k.eq(&bridging::XcmBridgeHubRouterByteFee::key()) || + k.eq(&bridging::XcmBridgeHubRouterBaseFee::key()) || k.eq(&bridging::to_ethereum::BridgeHubEthereumBaseFee::key()) }) => return true, From d33c52b0dcaf83d00946ca9ac65874fcad056e97 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 21 Mar 2024 10:55:46 +0200 Subject: [PATCH 3/5] taplo --- substrate/primitives/arithmetic/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/primitives/arithmetic/Cargo.toml b/substrate/primitives/arithmetic/Cargo.toml index 64c1025b5858..120edd06a660 100644 --- a/substrate/primitives/arithmetic/Cargo.toml +++ b/substrate/primitives/arithmetic/Cargo.toml @@ -43,7 +43,7 @@ std = [ "scale-info/std", "serde/std", "sp-crypto-hashing/std", - "sp-std/std", + "sp-std/std", ] # Serde support without relying on std features. serde = ["dep:serde", "scale-info/serde"] From 3e5124db344950fa55ba84f967d66280fd83bf45 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 21 Mar 2024 10:56:56 +0200 Subject: [PATCH 4/5] remove rustdoc describing removed extrinsic --- bridges/snowbridge/pallets/ethereum-client/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index fc2ab2fbb588..86b55e9d23f9 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -15,8 +15,6 @@ //! ## Consensus Updates //! //! * [`Call::submit`]: Submit a finalized beacon header with an optional sync committee update -//! * [`Call::submit_execution_header`]: Submit an execution header together with an ancestry proof -//! that can be verified against an already imported finalized beacon header. #![cfg_attr(not(feature = "std"), no_std)] pub mod config; From 27ecafd7698295cf6b8e0f67a556bc1f9a23afbb Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Fri, 22 Mar 2024 10:36:43 +0200 Subject: [PATCH 5/5] revert unrelated changes --- bridges/snowbridge/pallets/ethereum-client/src/lib.rs | 2 ++ .../runtimes/assets/asset-hub-rococo/src/xcm_config.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs index 86b55e9d23f9..fc2ab2fbb588 100644 --- a/bridges/snowbridge/pallets/ethereum-client/src/lib.rs +++ b/bridges/snowbridge/pallets/ethereum-client/src/lib.rs @@ -15,6 +15,8 @@ //! ## Consensus Updates //! //! * [`Call::submit`]: Submit a finalized beacon header with an optional sync committee update +//! * [`Call::submit_execution_header`]: Submit an execution header together with an ancestry proof +//! that can be verified against an already imported finalized beacon header. #![cfg_attr(not(feature = "std"), no_std)] pub mod config; diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs index 71c0c6201287..c438361cc177 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs @@ -286,8 +286,8 @@ impl Contains for SafeCallFilter { match call { RuntimeCall::System(frame_system::Call::set_storage { items }) if items.iter().all(|(k, _)| { - k.eq(&bridging::XcmBridgeHubRouterByteFee::key()) || - k.eq(&bridging::XcmBridgeHubRouterBaseFee::key()) || + k.eq(&bridging::XcmBridgeHubRouterByteFee::key()) | + k.eq(&bridging::XcmBridgeHubRouterBaseFee::key()) | k.eq(&bridging::to_ethereum::BridgeHubEthereumBaseFee::key()) }) => return true,