From 3410dfb3929462da88be2da813f121d8b1cf46b3 Mon Sep 17 00:00:00 2001
From: Clara van Staden <claravanstaden64@gmail.com>
Date: Fri, 22 Mar 2024 11:25:28 +0200
Subject: [PATCH] Snowbridge Beacon header age check (#3727)
## Bug Explanation
Adds a check that prevents finalized headers with a gap larger than the
sync committee period being imported, which could cause execution
headers in the gap being unprovable. The current version of the Ethereum
client checks that there is a header at least every sync committee, but
it doesn't check that the headers are within a sync period of each
other. For example:
Header 100 (sync committee period 1)
Header 9000 (sync committee period 2)
(8900 blocks apart)
These headers are in adjacent sync committees, but more than the sync
committee period (8192 blocks) apart.
The reason we need a header every 8192 slots at least, is the header is
used to prove messages within the last 8192 blocks. If we import header
9000, and we receive a message to be verified at header 200, the
`block_roots` field of header 9000 won't contain the header in order to
do the ancestry check.
## Environment
While running in Rococo, this edge case was discovered after the relayer
was offline for a few days. It is unlikely, but not impossible, to
happen again and so it should be backported to polkadot-sdk 1.7.0 (so
that
[polkadot-fellows/runtimes](https://github.com/polkadot-fellows/runtimes)
can be updated with the fix).
Our Ethereum client has been operational on Rococo for the past few
months, and this been the only major issue discovered so far.
### Unrelated Change
An unrelated nit: Removes a left over file that should have been deleted
when the `parachain` directory was removed.
---------
Co-authored-by: claravanstaden <Cats 4 life!>
---
.../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 a54d4a05ac5..fc2ab2fbb58 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::<T>::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::<T>::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 50b6a25c342..4a7b7b45886 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!(<NextSyncCommittee<Test>>::exists());
+ assert_err!(
+ EthereumBeaconClient::submit(RuntimeOrigin::signed(1), next_update.clone()),
+ Error::<Test>::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!(<NextSyncCommittee<Test>>::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::<Test>::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 d2cca373e92..00000000000
--- 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 <hello@snowfork.com>
-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<Test>;
-
- frame_support::construct_runtime!(
- pub enum Test {
- System: frame_system::{Pallet, Call, Storage, Event<T>},
- Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent},
- EthereumBeaconClient: ethereum_beacon_client::{Pallet, Call, Storage, Event<T>},
- }
- );
-
- 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<Self::AccountId>;
- 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::<Test>::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<T>(basename: &str) -> Result<T, serde_json::Error>
- 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<Test>;
- use sp_runtime::BuildStorage;
-
- frame_support::construct_runtime!(
- pub enum Test {
- System: frame_system::{Pallet, Call, Storage, Event<T>},
- Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent},
- EthereumBeaconClient: ethereum_beacon_client::{Pallet, Call, Storage, Event<T>},
- }
- );
-
- 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<Self::AccountId>;
- 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::<Test>::default().build_storage().unwrap();
- let mut ext = sp_io::TestExternalities::new(t);
- let _ = ext.execute_with(|| Timestamp::set(RuntimeOrigin::signed(1), 30_000));
- ext
- }
-}
--
GitLab