Skip to content

Commit

Permalink
Fix border condition in Snowbridge free consensus Updates (#5671)
Browse files Browse the repository at this point in the history
# Description

A fix for a border condition introduced with new feature
#5201. A malicious
relayer could spam the Ethereum client with sync committee updates that
have already been imported for the period. This PR adds a storage item
to track the last imported sync committee period, so that subsequent
irrelevant updates are not free.

Original PR: Snowfork#172

## Integration

Downstream projects are not affected. Relayers will not be able to spam
the Ethereum client with irrelevant sync committee updates for free.

## Review Notes

Adds a storage item to track the last free sync committee update period,
so that duplicate imports are not free.

---------

Co-authored-by: Adrian Catangiu <[email protected]>
(cherry picked from commit 1790e42)
  • Loading branch information
claravanstaden authored and github-actions[bot] committed Sep 24, 2024
1 parent b61075b commit 7f5c7ab
Show file tree
Hide file tree
Showing 8 changed files with 1,828 additions and 30 deletions.
27 changes: 20 additions & 7 deletions bridges/snowbridge/pallets/ethereum-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 LatestSyncCommitteeUpdatePeriod<T: Config> = StorageValue<_, u64, ValueQuery>;

/// The current operating mode of the pallet.
#[pallet::storage]
#[pallet::getter(fn operating_mode)]
Expand Down Expand Up @@ -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);
Expand All @@ -465,17 +476,12 @@ pub mod pallet {
"💫 SyncCommitteeUpdated at period {}.",
update_finalized_period
);
<LatestSyncCommitteeUpdatePeriod<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)?;
}
Expand Down Expand Up @@ -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 = LatestSyncCommitteeUpdatePeriod::<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;
}

Expand Down
27 changes: 27 additions & 0 deletions bridges/snowbridge/pallets/ethereum-client/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,33 @@ 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() -> Box<
snowbridge_beacon_primitives::Update<
{ config::SYNC_COMMITTEE_SIZE },
{ config::SYNC_COMMITTEE_BITS_SIZE },
>,
> {
Box::new(load_fixture("sync-committee-update-period-0.json".to_string()).unwrap())
}

pub fn load_sync_committee_update_period_0_older_fixture() -> Box<
snowbridge_beacon_primitives::Update<
{ config::SYNC_COMMITTEE_SIZE },
{ config::SYNC_COMMITTEE_BITS_SIZE },
>,
> {
Box::new(load_fixture("sync-committee-update-period-0-older.json".to_string()).unwrap())
}

pub fn load_sync_committee_update_period_0_newer_fixture() -> Box<
snowbridge_beacon_primitives::Update<
{ config::SYNC_COMMITTEE_SIZE },
{ config::SYNC_COMMITTEE_BITS_SIZE },
>,
> {
Box::new(load_fixture("sync-committee-update-period-0-newer.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)
Expand Down
63 changes: 55 additions & 8 deletions bridges/snowbridge/pallets/ethereum-client/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023 Snowfork <[email protected]>
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, LatestSyncCommitteeUpdatePeriod,
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::{
Expand Down Expand Up @@ -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));
});
Expand Down Expand Up @@ -711,8 +710,56 @@ 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()); // slot 129
let second_sync_committee_update = load_sync_committee_update_period_0(); // slot 128
let third_sync_committee_update = load_sync_committee_update_period_0_newer_fixture(); // slot 224
let fourth_sync_committee_update = load_sync_committee_update_period_0_older_fixture(); // slot 96
let fith_sync_committee_update = Box::new(load_next_sync_committee_update_fixture()); // slot 8259

new_tester().execute_with(|| {
assert_ok!(EthereumBeaconClient::process_checkpoint_update(&checkpoint));
assert_eq!(<LatestSyncCommitteeUpdatePeriod<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!(<LatestSyncCommitteeUpdatePeriod<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!(<LatestSyncCommitteeUpdatePeriod<Test>>::get(), 0);

// Check that setting an update with a sync committee that has already been set, but with a
// newer finalized header, 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!(<LatestSyncCommitteeUpdatePeriod<Test>>::get(), 0);

// Check that setting the next sync committee for period 0 again with an earlier slot is not
// free.
let fourth_result =
EthereumBeaconClient::submit(RuntimeOrigin::signed(1), fourth_sync_committee_update);
assert_err!(fourth_result, Error::<Test>::IrrelevantUpdate);
assert_eq!(fourth_result.unwrap_err().post_info.pays_fee, Pays::Yes);

// Check that setting the next sync committee for period 1 is free.
let fith_result =
EthereumBeaconClient::submit(RuntimeOrigin::signed(1), fith_sync_committee_update);
assert_eq!(fith_result.unwrap().pays_fee, Pays::No);
assert_eq!(<LatestSyncCommitteeUpdatePeriod<Test>>::get(), 1);
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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": [
Expand Down Expand Up @@ -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"
]
}
}
Loading

0 comments on commit 7f5c7ab

Please sign in to comment.