Skip to content

Commit 2da3987

Browse files
Snowbridge Ethereum client spec fix (#10793)
This PR fixes a minor discrepancy in the Snowbridge Ethereum cient pallet where the fork version for sync committee signature verification was derived from `signature_slot` instead of `signature_slot - 1` as required by the https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/light-client/sync-protocol.md#validate_light_client_update. This caused valid light client updates to be rejected at Ethereum hard-fork boundaries. The impact is low severity - only affecting liveness once or twice a year for a few minutes. Origin: bug bounty report. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent f154a34 commit 2da3987

File tree

6 files changed

+99
-6
lines changed

6 files changed

+99
-6
lines changed

bridges/snowbridge/pallets/ethereum-client/src/config/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ pub const MAX_FEE_RECIPIENT_SIZE: usize = 20;
1414
pub const MAX_BRANCH_PROOF_SIZE: usize = 20;
1515

1616
/// DomainType('0x07000000')
17-
/// <https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/beacon-chain.md#domain-types>
17+
/// <https://github.com/ethereum/consensus-specs/blob/master/specs/altair/beacon-chain.md#domains>
1818
pub const DOMAIN_SYNC_COMMITTEE: [u8; 4] = [7, 0, 0, 0];
1919
/// Validators public keys are 48 bytes.
2020
pub const PUBKEY_SIZE: usize = 48;

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ pub mod pallet {
460460
Ok(())
461461
}
462462

463-
/// Reference and strictly follows <https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/light-client/sync-protocol.md#apply_light_client_update
463+
/// Reference and strictly follows <https://github.com/ethereum/consensus-specs/blob/master/specs/altair/light-client/sync-protocol.md#apply_light_client_update>
464464
/// Applies a finalized beacon header update to the beacon client. If a next sync committee
465465
/// is present in the update, verify the sync committee by converting it to a
466466
/// SyncCommitteePrepared type. Stores the provided finalized header. Updates are free
@@ -675,8 +675,11 @@ pub mod pallet {
675675
validators_root: H256,
676676
signature_slot: u64,
677677
) -> Result<H256, DispatchError> {
678+
// Per Ethereum Altair light-client spec, fork version is derived from signature_slot -
679+
// 1. See: https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/light-client/sync-protocol.md#validate_light_client_update
680+
// In validate_light_client_update: fork_version_slot = max(signature_slot, 1) - 1
678681
let fork_version = Self::compute_fork_version(compute_epoch(
679-
signature_slot,
682+
signature_slot.saturating_sub(1),
680683
config::SLOTS_PER_EPOCH as u64,
681684
));
682685
let domain_type = config::DOMAIN_SYNC_COMMITTEE.to_vec();

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

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
pub use crate::mock::*;
44
use crate::{
55
config::{EPOCHS_PER_SYNC_COMMITTEE_PERIOD, SLOTS_PER_EPOCH, SLOTS_PER_HISTORICAL_ROOT},
6-
functions::compute_period,
6+
functions::{compute_epoch, compute_period},
77
mock::{
88
get_message_verification_payload, load_checkpoint_update_fixture,
99
load_finalized_header_update_fixture, load_next_finalized_header_update_fixture,
@@ -974,3 +974,83 @@ fn verify_message_invalid_topic() {
974974
);
975975
});
976976
}
977+
978+
#[test]
979+
fn signing_root_uses_previous_slot_for_fork_version() {
980+
new_tester().execute_with(|| {
981+
// Use a signature_slot at a fork boundary (first slot of the fulu epoch).
982+
// In mock.rs: electra.epoch = 0, fulu.epoch = 100000000
983+
let fulu_epoch = ChainForkVersions::get().fulu.epoch;
984+
let signature_slot: u64 = fulu_epoch * (SLOTS_PER_EPOCH as u64);
985+
986+
// Verify this is the first slot of the epoch
987+
assert_eq!(signature_slot % (SLOTS_PER_EPOCH as u64), 0);
988+
989+
let header = BeaconHeader {
990+
slot: signature_slot - 1,
991+
proposer_index: 0,
992+
parent_root: H256::repeat_byte(0x11),
993+
state_root: H256::repeat_byte(0x22),
994+
body_root: H256::repeat_byte(0x33),
995+
};
996+
997+
let validators_root = H256::repeat_byte(0x44);
998+
999+
// Get fork versions for comparison
1000+
let fork_version_at_signature_slot = EthereumBeaconClient::compute_fork_version(
1001+
compute_epoch(signature_slot, SLOTS_PER_EPOCH as u64),
1002+
);
1003+
let fork_version_at_previous_slot = EthereumBeaconClient::compute_fork_version(
1004+
compute_epoch(signature_slot.saturating_sub(1), SLOTS_PER_EPOCH as u64),
1005+
);
1006+
1007+
// At the fork boundary, these should differ
1008+
assert_ne!(
1009+
fork_version_at_signature_slot, fork_version_at_previous_slot,
1010+
"Test setup error: fork versions should differ at fork boundary"
1011+
);
1012+
1013+
// Compute signing roots using both fork versions
1014+
let domain_type = crate::config::DOMAIN_SYNC_COMMITTEE.to_vec();
1015+
1016+
let domain_with_previous_slot = EthereumBeaconClient::compute_domain(
1017+
domain_type.clone(),
1018+
fork_version_at_previous_slot,
1019+
validators_root,
1020+
)
1021+
.unwrap();
1022+
1023+
let signing_root_with_previous_slot =
1024+
EthereumBeaconClient::compute_signing_root(&header, domain_with_previous_slot).unwrap();
1025+
1026+
// The pallet's signing_root should use the previous slot's fork version (per spec)
1027+
let pallet_signing_root =
1028+
EthereumBeaconClient::signing_root(&header, validators_root, signature_slot).unwrap();
1029+
1030+
assert_eq!(
1031+
pallet_signing_root, signing_root_with_previous_slot,
1032+
"signing_root should use fork version from signature_slot - 1"
1033+
);
1034+
});
1035+
}
1036+
1037+
#[test]
1038+
fn signing_root_handles_signature_slot_zero() {
1039+
// Per spec: fork_version_slot = max(signature_slot, 1) - 1
1040+
// When signature_slot = 0, saturating_sub(1) = 0, which matches max(0, 1) - 1 = 0
1041+
new_tester().execute_with(|| {
1042+
let header = BeaconHeader {
1043+
slot: 0,
1044+
proposer_index: 0,
1045+
parent_root: H256::repeat_byte(0x11),
1046+
state_root: H256::repeat_byte(0x22),
1047+
body_root: H256::repeat_byte(0x33),
1048+
};
1049+
1050+
let validators_root = H256::repeat_byte(0x44);
1051+
1052+
// Should not panic and should use epoch 0 fork version
1053+
let result = EthereumBeaconClient::signing_root(&header, validators_root, 0);
1054+
assert!(result.is_ok(), "signing_root should handle signature_slot = 0");
1055+
});
1056+
}

bridges/snowbridge/primitives/beacon/src/merkle_proof.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use sp_core::H256;
44
use sp_io::hashing::sha2_256;
55

66
/// Specified by <https://github.com/ethereum/consensus-specs/blob/fe9c1a8cbf0c2da8a4f349efdcd77dd7ac8445c4/specs/phase0/beacon-chain.md?plain=1#L742>
7-
/// with improvements from <https://github.com/ethereum/consensus-specs/blob/dev/ssz/merkle-proofs.md>
7+
/// with improvements from <https://github.com/ethereum/consensus-specs/blob/master/ssz/merkle-proofs.md>
88
pub fn verify_merkle_branch(
99
leaf: H256,
1010
branch: &[H256],

bridges/snowbridge/primitives/beacon/src/types.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ pub mod deneb {
600600
use sp_std::prelude::*;
601601

602602
/// ExecutionPayloadHeader
603-
/// <https://github.com/ethereum/consensus-specs/blob/dev/specs/deneb/beacon-chain.md#executionpayloadheader>
603+
/// <https://github.com/ethereum/consensus-specs/blob/master/specs/deneb/beacon-chain.md#executionpayloadheader>
604604
#[derive(
605605
Default,
606606
Encode,

prdoc/pr_10793.prdoc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
title: 'Snowbridge: Fix fork version slot selection for sync committee signature verification'
2+
doc:
3+
- audience: Runtime Dev
4+
description: |
5+
Fixes fork version selection for sync committee signature verification to use `signature_slot - 1` per the Ethereum Altair light-client spec. This prevents valid light client updates from being rejected at fork activation boundaries.
6+
crates:
7+
- name: snowbridge-pallet-ethereum-client
8+
bump: patch
9+
- name: snowbridge-beacon-primitives
10+
bump: patch

0 commit comments

Comments
 (0)