From 9c48f9a9a84469670fe4bd53497252f084d9df6b Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Fri, 8 Nov 2024 19:10:45 +0900 Subject: [PATCH 01/11] refactor period consistency check Signed-off-by: Jun Kimura --- crates/light-client-cli/src/state.rs | 37 ++--- crates/light-client-verifier/Cargo.toml | 1 + crates/light-client-verifier/src/consensus.rs | 137 ++++++++++++++++-- crates/light-client-verifier/src/errors.rs | 12 +- crates/light-client-verifier/src/mock.rs | 65 +++++---- crates/light-client-verifier/src/state.rs | 31 +++- crates/light-client-verifier/src/updates.rs | 17 --- 7 files changed, 204 insertions(+), 96 deletions(-) diff --git a/crates/light-client-cli/src/state.rs b/crates/light-client-cli/src/state.rs index 7c8edad..b509f7b 100644 --- a/crates/light-client-cli/src/state.rs +++ b/crates/light-client-cli/src/state.rs @@ -53,13 +53,6 @@ impl< self.latest_finalized_header.slot } - pub fn current_period( - &self, - ctx: &CC, - ) -> ethereum_consensus::sync_protocol::SyncCommitteePeriod { - compute_sync_committee_period_at_slot(ctx, self.current_slot()) - } - pub fn apply_light_client_update< CC: ChainConsensusVerificationContext, CU: ConsensusUpdate, @@ -120,25 +113,19 @@ impl< > LightClientStoreReader for LightClientStore { - fn get_sync_committee( + fn current_period( &self, ctx: &CC, - period: ethereum_consensus::sync_protocol::SyncCommitteePeriod, - ) -> Option> { - // https://github.com/ethereum/consensus-specs/blob/1b408e9354358cd7f883c170813e8bf93c922a94/specs/altair/light-client/sync-protocol.md#validate_light_client_update - // # Verify sync committee aggregate signature - // if update_signature_period == store_period: - // sync_committee = store.current_sync_committee - // else: - // sync_committee = store.next_sync_committee - let current_period = self.current_period(ctx); - if period == current_period { - Some(self.current_sync_committee.clone()) - } else if period == current_period + 1 { - self.next_sync_committee.clone() - } else { - None - } + ) -> ethereum_consensus::sync_protocol::SyncCommitteePeriod { + compute_sync_committee_period_at_slot(ctx, self.current_slot()) + } + + fn current_sync_committee(&self) -> Option> { + Some(self.current_sync_committee.clone()) + } + + fn next_sync_committee(&self) -> Option> { + self.next_sync_committee.clone() } fn ensure_relevant_update< @@ -149,8 +136,6 @@ impl< ctx: &CC, update: &C, ) -> Result<(), ethereum_light_client_verifier::errors::Error> { - update.ensure_consistent_update_period(ctx)?; - let store_period = compute_sync_committee_period_at_slot(ctx, self.current_slot()); let update_attested_period = compute_sync_committee_period_at_slot(ctx, update.attested_beacon_header().slot); diff --git a/crates/light-client-verifier/Cargo.toml b/crates/light-client-verifier/Cargo.toml index ce679df..30be366 100644 --- a/crates/light-client-verifier/Cargo.toml +++ b/crates/light-client-verifier/Cargo.toml @@ -19,6 +19,7 @@ rand = { version = "0.8.5", features = ["std", "std_rng"], optional = true} serde_json = "1.0.91" hex-literal = "0.3.4" rand = { version = "0.8.5", features = ["std", "std_rng"] } +ethereum-consensus = { path = "../consensus", default-features = false, features = ["prover"] } [features] default = ["std"] diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index 8025400..d1e0594 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -77,6 +77,14 @@ impl, @@ -86,10 +94,8 @@ impl Result<(), Error> { - consensus_update.validate_basic(ctx)?; - store.ensure_relevant_update(ctx, consensus_update)?; - let sync_committee = self.get_sync_committee(ctx, store, consensus_update)?; validate_light_client_update(ctx, store, consensus_update)?; + let sync_committee = self.get_sync_committee(ctx, store, consensus_update)?; verify_sync_committee_attestation(ctx, consensus_update, &sync_committee)?; Ok(()) } @@ -161,6 +167,7 @@ impl Result<(), Error> { + consensus_update.validate_basic(ctx)?; + + let current_period = store.current_period(ctx); + let signature_period = + compute_sync_committee_period_at_slot(ctx, consensus_update.signature_slot()); + // ensure that the update is relevant to the store + // the `store`` only has the current and next sync committee, so the signature period must match the current or next period + if current_period != signature_period && current_period + 1 != signature_period { + return Err(Error::StoreNotCoveredSignaturePeriod( + current_period, + signature_period, + )); + } + let finalized_period = + compute_sync_committee_period_at_slot(ctx, consensus_update.finalized_beacon_header().slot); + let attested_period = + compute_sync_committee_period_at_slot(ctx, consensus_update.attested_beacon_header().slot); + // the following condition must always be false + if finalized_period != attested_period && finalized_period + 1 != attested_period { + return Err(Error::InconsistentFinalizedPeriod( + finalized_period, + attested_period, + )); + } + + store.ensure_relevant_update(ctx, consensus_update)?; + // https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md#validate_light_client_update // Verify that the `finality_branch`, if present, confirms `finalized_header` // to match the finalized checkpoint root saved in the state of `attested_header`. @@ -958,10 +992,11 @@ mod tests { .as_secs() .into(), ); - let period_1 = U64(1) * ctx.slots_per_epoch() * ctx.epochs_per_sync_committee_period(); + let base_store_slot = + U64(1) * ctx.slots_per_epoch() * ctx.epochs_per_sync_committee_period(); let initial_header = BeaconBlockHeader { - slot: period_1, + slot: base_store_slot, ..Default::default() }; let current_sync_committee = scm.get_committee(1); @@ -970,9 +1005,10 @@ mod tests { current_sync_committee.to_committee(), Default::default(), ); - let base_signature_slot = period_1 + 11; - let base_attested_slot = base_signature_slot - 1; - let base_finalized_epoch = base_attested_slot / ctx.slots_per_epoch(); + let base_finalized_epoch = base_store_slot / ctx.slots_per_epoch() + 1; + let base_attested_slot = (base_finalized_epoch + 2) * ctx.slots_per_epoch(); + let base_signature_slot = base_attested_slot + 1; + let dummy_execution_state_root = [1u8; 32].into(); let dummy_execution_block_number = 1; @@ -1031,6 +1067,81 @@ mod tests { ); assert!(res.is_err(), "{:?}", res); } + { + // + // | + // +-----------+ | +-----------+ +-----------+ +-----------+ + // | store | <-- | finalized | <-- | attested | <-- | signature | + // +-----------+ | +-----------+ +-----------+ +-----------+ + // | + // | + // sync committee + // period boundary + // + let store_period = + compute_sync_committee_period_at_slot(&ctx, store.latest_finalized_header.slot); + let next_period = store_period + 1; + let finalized_epoch = next_period * ctx.epochs_per_sync_committee_period(); + let attested_slot = (finalized_epoch + 2) * ctx.slots_per_epoch(); + let signature_slot = attested_slot + 1; + let update = gen_light_client_update( + &ctx, + signature_slot, + attested_slot, + finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + &scm, + ); + let res = SyncProtocolVerifier::default() + .validate_consensus_update(&ctx, &store, &update); + assert!(res.is_err(), "{:?}", res); + + let store = MockStore { + next_sync_committee: Some(scm.get_committee(2).to_committee()), + ..store.clone() + }; + let res = SyncProtocolVerifier::default() + .validate_consensus_update(&ctx, &store, &update); + assert!(res.is_ok(), "{:?}", res); + } + { + // + // | | + // +-----------+ | | +-----------+ +-----------+ +-----------+ + // | store | <------ | finalized | <-- | attested | <-- | signature | + // +-----------+ | | +-----------+ +-----------+ +-----------+ + // | | + // | | + // sync committee + // period boundary + // + let store_period = + compute_sync_committee_period_at_slot(&ctx, store.latest_finalized_header.slot); + let next_next_period = store_period + 2; + let finalized_epoch = next_next_period * ctx.epochs_per_sync_committee_period(); + let attested_slot = (finalized_epoch + 2) * ctx.slots_per_epoch(); + let signature_slot = attested_slot + 1; + let update = gen_light_client_update( + &ctx, + signature_slot, + attested_slot, + finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + &scm, + ); + let res = SyncProtocolVerifier::default() + .validate_consensus_update(&ctx, &store, &update); + assert!(res.is_err(), "{:?}", res); + let store = MockStore { + next_sync_committee: Some(scm.get_committee(2).to_committee()), + ..store.clone() + }; + let res = SyncProtocolVerifier::default() + .validate_consensus_update(&ctx, &store, &update); + assert!(res.is_err(), "{:?}", res); + } { // // | @@ -1112,13 +1223,7 @@ mod tests { &store, &update_invalid_inconsistent_periods, ); - assert!(res.is_err(), "{:?}", res); - if let Some(Error::InconsistentUpdatePeriod(a, b)) = res.as_ref().err() { - assert_eq!(a, &1.into()); - assert_eq!(b, &2.into()); - } else { - panic!("unexpected error: {:?}", res); - } + assert!(res.is_ok(), "{:?}", res); } } diff --git a/crates/light-client-verifier/src/errors.rs b/crates/light-client-verifier/src/errors.rs index 7c94c17..8645989 100644 --- a/crates/light-client-verifier/src/errors.rs +++ b/crates/light-client-verifier/src/errors.rs @@ -13,12 +13,16 @@ type BoxedTrieError = Box>; #[derive(Debug, Display)] pub enum Error { - /// unexpected signature period: `signature={0} reason={1}` - UnexpectedSingaturePeriod(SyncCommitteePeriod, String), + /// unexpected signature period: `store={0} signature={1} reason={2}` + UnexpectedSingaturePeriod(SyncCommitteePeriod, SyncCommitteePeriod, String), /// unexpected attested period: `store={0} attested={1} reason={2}` UnexpectedAttestedPeriod(SyncCommitteePeriod, SyncCommitteePeriod, String), - /// inconsistent update period: `store={0} attested={1}` - InconsistentUpdatePeriod(SyncCommitteePeriod, SyncCommitteePeriod), + /// unexpected finalized period: `store={0} finalized={1} reason={2}` + UnexpectedFinalizedPeriod(SyncCommitteePeriod, SyncCommitteePeriod, String), + /// store does not cover the signature period: `store={0} signature={1}` + StoreNotCoveredSignaturePeriod(SyncCommitteePeriod, SyncCommitteePeriod), + /// inconsistent attested period: `finalized={0} attested={1}` + InconsistentFinalizedPeriod(SyncCommitteePeriod, SyncCommitteePeriod), /// cannot rotate to next sync committee: `store={0} finalized={1}` CannotRotateNextSyncCommittee(SyncCommitteePeriod, SyncCommitteePeriod), /// no next sync committee in store: `store_period={0} signature_period={1}` diff --git a/crates/light-client-verifier/src/mock.rs b/crates/light-client-verifier/src/mock.rs index 705e46e..286acae 100644 --- a/crates/light-client-verifier/src/mock.rs +++ b/crates/light-client-verifier/src/mock.rs @@ -31,10 +31,7 @@ impl MockStore { } } - pub fn current_period(&self, ctx: &CC) -> Slot { - compute_sync_committee_period_at_slot(ctx, self.latest_finalized_header.slot) - } - + /// CONTRACT: `apply_light_client_update` must be called after `SyncProtocolVerifier::validate_consensus_update()` pub fn apply_light_client_update< CC: ChainContext + ConsensusVerificationContext, CU: ConsensusUpdate, @@ -46,30 +43,47 @@ impl MockStore { let mut new_store = self.clone(); let store_period = compute_sync_committee_period_at_slot(ctx, self.latest_finalized_header.slot); - let attested_period = compute_sync_committee_period_at_slot( + let finalized_period = compute_sync_committee_period_at_slot( ctx, - consensus_update.attested_beacon_header().slot, + consensus_update.finalized_beacon_header().slot, ); - if store_period == attested_period { - if let Some(committee) = consensus_update.next_sync_committee() { - new_store.next_sync_committee = Some(committee.clone()); + if store_period == finalized_period { + // store_period == finalized_period <= attested_period <= signature_period + let attested_period = compute_sync_committee_period_at_slot( + ctx, + consensus_update.attested_beacon_header().slot, + ); + if finalized_period == attested_period { + if consensus_update.next_sync_committee().is_some() { + new_store.next_sync_committee = consensus_update.next_sync_committee().cloned(); + } + } else if finalized_period + 1 == attested_period { + // no-op: because `consensus_update` has already validated with the `store.next_sync_committee()` + } else { + return Err(crate::errors::Error::UnexpectedAttestedPeriod( + store_period, + attested_period, + "attested period must be equal to finalized_period or finalized_period+1" + .into(), + )); } - } else if store_period + 1 == attested_period { + } else if store_period + 1 == finalized_period { + // store_period + 1 == finalized_period == attested_period == signature_period if let Some(committee) = self.next_sync_committee.as_ref() { new_store.current_sync_committee = committee.clone(); new_store.next_sync_committee = consensus_update.next_sync_committee().cloned(); } else { return Err(crate::errors::Error::CannotRotateNextSyncCommittee( store_period, - attested_period, + finalized_period, )); } } else { - return Err(crate::errors::Error::UnexpectedAttestedPeriod( + return Err(crate::errors::Error::UnexpectedFinalizedPeriod( store_period, - attested_period, - "attested period must be equal to store_period or store_period+1".into(), + finalized_period, + "finalized period must be equal to store_period or store_period+1".into(), )); }; if consensus_update.finalized_beacon_header().slot > self.latest_finalized_header.slot { @@ -86,18 +100,15 @@ impl MockStore { impl LightClientStoreReader for MockStore { - fn get_sync_committee( - &self, - ctx: &CC, - period: ethereum_consensus::sync_protocol::SyncCommitteePeriod, - ) -> Option> { - let current_period = self.current_period(ctx); - if period == current_period { - Some(self.current_sync_committee.clone()) - } else if period == current_period + 1 { - self.next_sync_committee.clone() - } else { - None - } + fn current_period(&self, ctx: &CC) -> Slot { + compute_sync_committee_period_at_slot(ctx, self.latest_finalized_header.slot) + } + + fn current_sync_committee(&self) -> Option> { + Some(self.current_sync_committee.clone()) + } + + fn next_sync_committee(&self) -> Option> { + self.next_sync_committee.clone() } } diff --git a/crates/light-client-verifier/src/state.rs b/crates/light-client-verifier/src/state.rs index b4b9d8c..14c151a 100644 --- a/crates/light-client-verifier/src/state.rs +++ b/crates/light-client-verifier/src/state.rs @@ -5,21 +5,40 @@ use ethereum_consensus::{ }; pub trait LightClientStoreReader { - /// Returns the sync committee for the given period. + /// Returns the current sync committee period + fn current_period(&self, ctx: &CC) -> SyncCommitteePeriod; + + /// Returns the current sync committee corresponding to the current period if available + fn current_sync_committee(&self) -> Option>; + + /// Returns the next sync committee corresponding to the next period if available + fn next_sync_committee(&self) -> Option>; + + /// Returns the sync committee corresponding to the given signature period if available fn get_sync_committee( &self, ctx: &CC, - period: SyncCommitteePeriod, - ) -> Option>; + signature_period: SyncCommitteePeriod, + ) -> Option> { + let current_period = self.current_period(ctx); + let next_period = current_period + 1; + if signature_period == current_period { + self.current_sync_committee() + } else if signature_period == next_period { + self.next_sync_committee() + } else { + None + } + } /// Returns a error indicating whether the update is relevant to the light client store. /// /// This method should be used to determine whether the update should be applied to the store. fn ensure_relevant_update>( &self, - ctx: &CC, - update: &C, + _ctx: &CC, + _update: &C, ) -> Result<(), Error> { - update.ensure_consistent_update_period(ctx) + Ok(()) } } diff --git a/crates/light-client-verifier/src/updates.rs b/crates/light-client-verifier/src/updates.rs index 4f21d41..bf9c1e9 100644 --- a/crates/light-client-verifier/src/updates.rs +++ b/crates/light-client-verifier/src/updates.rs @@ -1,8 +1,6 @@ use crate::context::{ChainConsensusVerificationContext, ConsensusVerificationContext}; use crate::errors::Error; use crate::internal_prelude::*; -use ethereum_consensus::compute::compute_sync_committee_period_at_slot; -use ethereum_consensus::context::ChainContext; use ethereum_consensus::{ beacon::{BeaconBlockHeader, Slot}, merkle::is_valid_normalized_merkle_branch, @@ -43,21 +41,6 @@ pub trait ConsensusUpdate: fn sync_aggregate(&self) -> &SyncAggregate; fn signature_slot(&self) -> Slot; - fn ensure_consistent_update_period(&self, ctx: &C) -> Result<(), Error> { - let finalized_period = - compute_sync_committee_period_at_slot(ctx, self.finalized_beacon_header().slot); - let attested_period = - compute_sync_committee_period_at_slot(ctx, self.attested_beacon_header().slot); - if finalized_period == attested_period { - Ok(()) - } else { - Err(Error::InconsistentUpdatePeriod( - finalized_period, - attested_period, - )) - } - } - /// ref. https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md#is_valid_light_client_header /// NOTE: There are no validation for the execution payload, so you should implement it if the update contains the execution payload. fn is_valid_light_client_finalized_header( From ad900ad3fbbd8f6c3d7495b9c041b5781e4151b0 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Fri, 8 Nov 2024 20:02:28 +0900 Subject: [PATCH 02/11] fix `NextSyncCommitteeMisbehaviour` to check if each attested header has finalized next sync committee Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/errors.rs | 2 ++ .../light-client-verifier/src/misbehaviour.rs | 24 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/light-client-verifier/src/errors.rs b/crates/light-client-verifier/src/errors.rs index 8645989..ad7c897 100644 --- a/crates/light-client-verifier/src/errors.rs +++ b/crates/light-client-verifier/src/errors.rs @@ -55,6 +55,8 @@ pub enum Error { CommonError(ethereum_consensus::errors::Error), /// rlp decoder error: `{0:?}` RlpDecoderError(rlp::DecoderError), + /// the next sync committee is not finalized: `finalized={0} attested={1}` + NotFinalizedNextSyncCommittee(SyncCommitteePeriod, SyncCommitteePeriod), /// both updates of misbehaviour data must have same period: {0} != {1} DifferentPeriodInNextSyncCommitteeMisbehaviour(SyncCommitteePeriod, SyncCommitteePeriod), /// both updates of misbehaviour data must have next sync committee diff --git a/crates/light-client-verifier/src/misbehaviour.rs b/crates/light-client-verifier/src/misbehaviour.rs index 06f9f80..e08d69f 100644 --- a/crates/light-client-verifier/src/misbehaviour.rs +++ b/crates/light-client-verifier/src/misbehaviour.rs @@ -65,7 +65,7 @@ impl> /// NextSyncCommitteeMisbehaviour is a misbehaviour that satisfies the followings: /// 1. Two updates are valid with the consensus state of the client -/// 2. Each attested header in the two updates has a same period +/// 2. Each attested header in the two updates has a same period and a finalized next sync committee /// 3. The two next sync committees are different from each other #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct NextSyncCommitteeMisbehaviour< @@ -84,10 +84,32 @@ impl> ctx, self.consensus_update_1.attested_beacon_header().slot, ); + let finalized_period_1 = compute_sync_committee_period_at_slot( + ctx, + self.consensus_update_1.finalized_beacon_header().slot, + ); + if finalized_period_1 != attested_period_1 { + return Err(Error::NotFinalizedNextSyncCommittee( + finalized_period_1, + attested_period_1, + )); + } + let attested_period_2 = compute_sync_committee_period_at_slot( ctx, self.consensus_update_2.attested_beacon_header().slot, ); + let finalized_period_2 = compute_sync_committee_period_at_slot( + ctx, + self.consensus_update_2.finalized_beacon_header().slot, + ); + if finalized_period_2 != attested_period_2 { + return Err(Error::NotFinalizedNextSyncCommittee( + finalized_period_2, + attested_period_2, + )); + } + let next_1 = self.consensus_update_1.next_sync_committee(); let next_2 = self.consensus_update_2.next_sync_committee(); From 8e48737b791fcad43fbc7295fb7b213caa84a220 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Fri, 8 Nov 2024 20:36:55 +0900 Subject: [PATCH 03/11] improve misbehaviour tests Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index d1e0594..8249d7e 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -979,7 +979,7 @@ mod tests { }; #[test] - fn test_lc() { + fn test_consensus_update_validation() { let scm = MockSyncCommitteeManager::<32>::new(1, 4); let ctx = LightClientContext::new_with_config( config::minimal::get_config(), @@ -1228,7 +1228,7 @@ mod tests { } #[test] - fn test_lc_misbehaviour() { + fn test_misbehaviour_validation() { let scm = MockSyncCommitteeManager::<32>::new(1, 4); let current_sync_committee = scm.get_committee(1); let ctx = LightClientContext::new_with_config( @@ -1242,11 +1242,11 @@ mod tests { .as_secs() .into(), ); - let start_slot = + let base_store_slot = U64(1) * ctx.slots_per_epoch() * ctx.epochs_per_sync_committee_period(); let initial_header = BeaconBlockHeader { - slot: start_slot, + slot: base_store_slot, ..Default::default() }; let store = MockStore::new( @@ -1255,11 +1255,12 @@ mod tests { Default::default(), ); + let base_finalized_epoch = base_store_slot / ctx.slots_per_epoch() + 1; + let base_attested_slot = (base_finalized_epoch + 2) * ctx.slots_per_epoch(); + let base_signature_slot = base_attested_slot + 1; + let dummy_execution_state_root = [1u8; 32].into(); let dummy_execution_block_number = 1; - let base_signature_slot = start_slot + 11; - let base_attested_slot = base_signature_slot - 1; - let base_finalized_epoch = base_attested_slot / ctx.slots_per_epoch(); let update_1 = gen_light_client_update_with_params::<32, _>( &ctx, @@ -1363,6 +1364,32 @@ mod tests { ); assert!(res.is_err(), "{:?}", res); } + { + let attested_slot = (base_finalized_epoch + ctx.epochs_per_sync_committee_period()) + * ctx.slots_per_epoch(); + let signature_slot = attested_slot + 1; + let update_not_finalized_next_sync_committee = + gen_light_client_update_with_params::<32, _>( + &ctx, + signature_slot, + attested_slot, + base_finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + current_sync_committee, + scm.get_committee(3), + 32, + ); + let res = SyncProtocolVerifier::default().validate_misbehaviour( + &ctx, + &store, + Misbehaviour::NextSyncCommittee(NextSyncCommitteeMisbehaviour { + consensus_update_1: update_1.clone(), + consensus_update_2: update_not_finalized_next_sync_committee, + }), + ); + assert!(res.is_err(), "{:?}", res); + } { let different_dummy_execution_state_root = [2u8; 32].into(); let update_different_finalized_block = gen_light_client_update_with_params::<32, _>( From ba1a083217edc0fe9d88276c76b65b777f4ddf45 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sat, 9 Nov 2024 11:48:00 +0900 Subject: [PATCH 04/11] tests: fix period calculation Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 83 +++++++++++-------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index 8249d7e..2a32906 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -510,6 +510,12 @@ pub mod test_utils { pub fn get_committee(&self, period: u64) -> &MockSyncCommittee { let idx = period - self.base_period; + assert!( + idx < self.committees.len() as u64, + "idx: {}, len: {}", + idx, + self.committees.len() + ); &self.committees[idx as usize] } } @@ -980,7 +986,7 @@ mod tests { #[test] fn test_consensus_update_validation() { - let scm = MockSyncCommitteeManager::<32>::new(1, 4); + let scm = MockSyncCommitteeManager::<32>::new(1, 6); let ctx = LightClientContext::new_with_config( config::minimal::get_config(), Default::default(), @@ -992,23 +998,24 @@ mod tests { .as_secs() .into(), ); - let base_store_slot = - U64(1) * ctx.slots_per_epoch() * ctx.epochs_per_sync_committee_period(); + let base_store_period = 3u64; + let base_store_slot = U64(base_store_period) + * ctx.slots_per_epoch() + * ctx.epochs_per_sync_committee_period(); + let base_finalized_epoch = base_store_slot / ctx.slots_per_epoch() + 1; + let base_attested_slot = (base_finalized_epoch + 2) * ctx.slots_per_epoch(); + let base_signature_slot = base_attested_slot + 1; let initial_header = BeaconBlockHeader { slot: base_store_slot, ..Default::default() }; - let current_sync_committee = scm.get_committee(1); + let current_sync_committee = scm.get_committee(base_store_period); let store = MockStore::new( initial_header, current_sync_committee.to_committee(), Default::default(), ); - let base_finalized_epoch = base_store_slot / ctx.slots_per_epoch() + 1; - let base_attested_slot = (base_finalized_epoch + 2) * ctx.slots_per_epoch(); - let base_signature_slot = base_attested_slot + 1; - let dummy_execution_state_root = [1u8; 32].into(); let dummy_execution_block_number = 1; @@ -1038,7 +1045,7 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), + scm.get_committee(base_store_period + 1), 21, ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1057,7 +1064,7 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), + scm.get_committee(base_store_period + 1), 0, ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1078,9 +1085,7 @@ mod tests { // sync committee // period boundary // - let store_period = - compute_sync_committee_period_at_slot(&ctx, store.latest_finalized_header.slot); - let next_period = store_period + 1; + let next_period = U64(base_store_period) + 1; let finalized_epoch = next_period * ctx.epochs_per_sync_committee_period(); let attested_slot = (finalized_epoch + 2) * ctx.slots_per_epoch(); let signature_slot = attested_slot + 1; @@ -1098,7 +1103,7 @@ mod tests { assert!(res.is_err(), "{:?}", res); let store = MockStore { - next_sync_committee: Some(scm.get_committee(2).to_committee()), + next_sync_committee: Some(scm.get_committee(next_period.into()).to_committee()), ..store.clone() }; let res = SyncProtocolVerifier::default() @@ -1116,9 +1121,7 @@ mod tests { // sync committee // period boundary // - let store_period = - compute_sync_committee_period_at_slot(&ctx, store.latest_finalized_header.slot); - let next_next_period = store_period + 2; + let next_next_period = U64(base_store_period + 2); let finalized_epoch = next_next_period * ctx.epochs_per_sync_committee_period(); let attested_slot = (finalized_epoch + 2) * ctx.slots_per_epoch(); let signature_slot = attested_slot + 1; @@ -1135,7 +1138,9 @@ mod tests { .validate_consensus_update(&ctx, &store, &update); assert!(res.is_err(), "{:?}", res); let store = MockStore { - next_sync_committee: Some(scm.get_committee(2).to_committee()), + next_sync_committee: Some( + scm.get_committee(base_store_period + 1).to_committee(), + ), ..store.clone() }; let res = SyncProtocolVerifier::default() @@ -1172,7 +1177,9 @@ mod tests { assert!(res.is_err(), "{:?}", res); let store = MockStore { - next_sync_committee: Some(scm.get_committee(2).to_committee()), + next_sync_committee: Some( + scm.get_committee(base_store_period + 1).to_committee(), + ), ..store.clone() }; let update_valid = gen_light_client_update::<32, _>( @@ -1206,7 +1213,9 @@ mod tests { + ctx.slots_per_epoch() * ctx.epochs_per_sync_committee_period(); let next_period_attested_slot = next_period_signature_slot - 1; let store = MockStore { - next_sync_committee: Some(scm.get_committee(2).to_committee()), + next_sync_committee: Some( + scm.get_committee(base_store_period + 1).to_committee(), + ), ..store.clone() }; let update_invalid_inconsistent_periods = gen_light_client_update::<32, _>( @@ -1229,7 +1238,7 @@ mod tests { #[test] fn test_misbehaviour_validation() { - let scm = MockSyncCommitteeManager::<32>::new(1, 4); + let scm = MockSyncCommitteeManager::<32>::new(1, 5); let current_sync_committee = scm.get_committee(1); let ctx = LightClientContext::new_with_config( config::minimal::get_config(), @@ -1242,8 +1251,14 @@ mod tests { .as_secs() .into(), ); - let base_store_slot = - U64(1) * ctx.slots_per_epoch() * ctx.epochs_per_sync_committee_period(); + + let base_store_period = 3u64; + let base_store_slot = U64(base_store_period) + * ctx.slots_per_epoch() + * ctx.epochs_per_sync_committee_period(); + let base_finalized_epoch = base_store_slot / ctx.slots_per_epoch() + 1; + let base_attested_slot = (base_finalized_epoch + 2) * ctx.slots_per_epoch(); + let base_signature_slot = base_attested_slot + 1; let initial_header = BeaconBlockHeader { slot: base_store_slot, @@ -1255,10 +1270,6 @@ mod tests { Default::default(), ); - let base_finalized_epoch = base_store_slot / ctx.slots_per_epoch() + 1; - let base_attested_slot = (base_finalized_epoch + 2) * ctx.slots_per_epoch(); - let base_signature_slot = base_attested_slot + 1; - let dummy_execution_state_root = [1u8; 32].into(); let dummy_execution_block_number = 1; @@ -1270,7 +1281,7 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(2), + scm.get_committee(base_store_period + 1), 32, ); @@ -1283,7 +1294,7 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), + scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1305,7 +1316,7 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), + scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1327,8 +1338,8 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), - 21, // at least 22 is required + scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct + 21, // at least 22 is required ); let res = SyncProtocolVerifier::default().validate_misbehaviour( &ctx, @@ -1351,7 +1362,7 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), + scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1377,7 +1388,7 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), + scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1400,7 +1411,7 @@ mod tests { different_dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(2), + scm.get_committee(base_store_period + 1), 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1424,7 +1435,7 @@ mod tests { different_dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(2), + scm.get_committee(base_store_period + 1), 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( From f01aa3234fd5958c1335ed937ab159c238478e98 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sat, 9 Nov 2024 12:57:55 +0900 Subject: [PATCH 05/11] add more test cases Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index 2a32906..a0f9f0d 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -1147,6 +1147,40 @@ mod tests { .validate_consensus_update(&ctx, &store, &update); assert!(res.is_err(), "{:?}", res); } + { + // + // | + // +-----------+ | +-----------+ +-----------+ +-----------+ + // | finalized | <-- | store | <-- | attested | <-- | signature | + // +-----------+ | +-----------+ +-----------+ +-----------+ + // | + // | + // sync committee + // period boundary + // + let prev_period = U64(base_store_period - 1); + let finalized_epoch = prev_period * ctx.epochs_per_sync_committee_period(); + let attested_slot = + (U64(base_store_period) * ctx.epochs_per_sync_committee_period() + 2) + * ctx.slots_per_epoch(); + let signature_slot = attested_slot + 1; + let update = gen_light_client_update( + &ctx, + signature_slot, + attested_slot, + finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + &scm, + ); + let res = SyncProtocolVerifier::default() + .validate_consensus_update(&ctx, &store, &update); + assert!(res.is_ok(), "{:?}", res); + + // the store cannot apply the finalized header whose period is `store_period-1` + let res = store.apply_light_client_update(&ctx, &update); + assert!(res.is_err(), "{:?}", res); + } { // // | From 988d438d83bf77d6675eb59cc02a2ba9641a786f Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 10 Nov 2024 11:39:41 +0900 Subject: [PATCH 06/11] fix comments Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 38 +++++++++++++++++-- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index a0f9f0d..7338008 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -1020,6 +1020,7 @@ mod tests { let dummy_execution_block_number = 1; { + // valid update (store_period == finalized_period == signature_period) let update_valid = gen_light_client_update::<32, _>( &ctx, base_signature_slot, @@ -1046,7 +1047,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 1), - 21, + 21, // insufficient attestations. at least 22 is required. ); let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, @@ -1055,6 +1056,25 @@ mod tests { ); assert!(res.is_err(), "{:?}", res); } + { + let update_sufficient_attestations = gen_light_client_update_with_params( + &ctx, + base_signature_slot, + base_attested_slot, + base_finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + current_sync_committee, + scm.get_committee(base_store_period + 1), + 22, // sufficient attestations + ); + let res = SyncProtocolVerifier::default().validate_consensus_update( + &ctx, + &store, + &update_sufficient_attestations, + ); + assert!(res.is_ok(), "{:?}", res); + } { let update_zero_attestations = gen_light_client_update_with_params( &ctx, @@ -1065,7 +1085,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 1), - 0, + 0, // insufficient attestations. at least 22 is required. ); let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, @@ -1252,7 +1272,7 @@ mod tests { ), ..store.clone() }; - let update_invalid_inconsistent_periods = gen_light_client_update::<32, _>( + let update_not_finalized_next_sync_committee = gen_light_client_update::<32, _>( &ctx, next_period_signature_slot, next_period_attested_slot, @@ -1264,9 +1284,19 @@ mod tests { let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, &store, - &update_invalid_inconsistent_periods, + &update_not_finalized_next_sync_committee, ); assert!(res.is_ok(), "{:?}", res); + let res = store + .apply_light_client_update(&ctx, &update_not_finalized_next_sync_committee); + assert!(res.is_ok(), "{:?}", res); + let new_store = res.unwrap().unwrap(); + // committees not changed + assert_eq!( + store.current_sync_committee, + new_store.current_sync_committee + ); + assert_eq!(store.next_sync_committee, new_store.next_sync_committee); } } From ecae3d58ff3458ad1d44c268134e0a06a0ef6c24 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 10 Nov 2024 12:24:28 +0900 Subject: [PATCH 07/11] tests: add `is_update_contain_next_sync_committee` option Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index 7338008..3cfa120 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -543,6 +543,7 @@ pub mod test_utils { execution_block_number, scm.get_committee(signature_period.into()), scm.get_committee((attested_period + 1).into()), + true, SYNC_COMMITTEE_SIZE, ) } @@ -560,6 +561,7 @@ pub mod test_utils { execution_block_number: BlockNumber, sync_committee: &MockSyncCommittee, next_sync_committee: &MockSyncCommittee, + is_update_contain_next_sync_committee: bool, sign_num: usize, ) -> ConsensusUpdateInfo { assert!( @@ -579,7 +581,7 @@ pub mod test_utils { ctx, attested_slot, finalized_root, - sync_committee.to_committee(), + Default::default(), next_sync_committee.to_committee(), ); @@ -605,10 +607,14 @@ pub mod test_utils { attested_header, sign_num, ), - next_sync_committee: Some(( - next_sync_committee.to_committee(), - next_sync_committee_branch, - )), + next_sync_committee: if is_update_contain_next_sync_committee { + Some(( + next_sync_committee.to_committee(), + next_sync_committee_branch, + )) + } else { + None + }, }; ConsensusUpdateInfo { @@ -1047,6 +1053,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 1), + true, 21, // insufficient attestations. at least 22 is required. ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1066,6 +1073,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 1), + true, 22, // sufficient attestations ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1085,6 +1093,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 1), + true, 0, // insufficient attestations. at least 22 is required. ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1346,6 +1355,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 1), + true, 32, ); @@ -1359,6 +1369,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1381,6 +1392,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1403,7 +1415,8 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct - 21, // at least 22 is required + true, + 21, // at least 22 is required ); let res = SyncProtocolVerifier::default().validate_misbehaviour( &ctx, @@ -1427,6 +1440,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1453,6 +1467,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 2), // `base_store_period+1` is really correct + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1476,6 +1491,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 1), + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1500,6 +1516,7 @@ mod tests { dummy_execution_block_number.into(), current_sync_committee, scm.get_committee(base_store_period + 1), + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( From c51ab444e0ee7cebd4506a8bfb9925c0fb8b4a32 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 10 Nov 2024 13:45:28 +0900 Subject: [PATCH 08/11] improve tests Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 125 ++++++++++++++++-- 1 file changed, 111 insertions(+), 14 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index 3cfa120..b6b2851 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -154,7 +154,7 @@ impl>( &self, ctx: &CC, @@ -254,7 +254,7 @@ pub fn validate_light_client_update< let signature_period = compute_sync_committee_period_at_slot(ctx, consensus_update.signature_slot()); // ensure that the update is relevant to the store - // the `store`` only has the current and next sync committee, so the signature period must match the current or next period + // the `store` only has the current and next sync committee, so the signature period must match the current or next period if current_period != signature_period && current_period + 1 != signature_period { return Err(Error::StoreNotCoveredSignaturePeriod( current_period, @@ -520,6 +520,7 @@ pub mod test_utils { } } + #[allow(clippy::too_many_arguments)] pub fn gen_light_client_update< const SYNC_COMMITTEE_SIZE: usize, C: ChainConsensusVerificationContext, @@ -530,6 +531,7 @@ pub mod test_utils { finalized_epoch: Epoch, execution_state_root: H256, execution_block_number: BlockNumber, + is_update_contain_next_sync_committee: bool, scm: &MockSyncCommitteeManager, ) -> ConsensusUpdateInfo { let signature_period = compute_sync_committee_period_at_slot(ctx, signature_slot); @@ -543,7 +545,7 @@ pub mod test_utils { execution_block_number, scm.get_committee(signature_period.into()), scm.get_committee((attested_period + 1).into()), - true, + is_update_contain_next_sync_committee, SYNC_COMMITTEE_SIZE, ) } @@ -1027,21 +1029,50 @@ mod tests { { // valid update (store_period == finalized_period == signature_period) - let update_valid = gen_light_client_update::<32, _>( - &ctx, - base_signature_slot, - base_attested_slot, - base_finalized_epoch, - dummy_execution_state_root, - dummy_execution_block_number.into(), - &scm, - ); + for b in [false, true] { + let update_valid = gen_light_client_update::<32, _>( + &ctx, + base_signature_slot, + base_attested_slot, + base_finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + b, + &scm, + ); + let res = SyncProtocolVerifier::default().validate_consensus_update( + &ctx, + &store, + &update_valid, + ); + assert!(res.is_ok(), "{:?}", res); + } + } + { + // valid update has no next sync committee branch (store_period == finalized_period == signature_period) + let update_invalid_no_next_sync_committee_branch = { + let mut update = gen_light_client_update::<32, _>( + &ctx, + base_signature_slot, + base_attested_slot, + base_finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + true, + &scm, + ); + let (next_sync_committee, _) = + update.light_client_update.next_sync_committee.unwrap(); + update.light_client_update.next_sync_committee = + Some((next_sync_committee, vec![])); + update + }; let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, &store, - &update_valid, + &update_invalid_no_next_sync_committee_branch, ); - assert!(res.is_ok(), "{:?}", res); + assert!(res.is_err(), "{:?}", res); } { let update_insufficient_attestations = gen_light_client_update_with_params( @@ -1103,6 +1134,66 @@ mod tests { ); assert!(res.is_err(), "{:?}", res); } + { + let mut update_invalid_finalized_header_branch = gen_light_client_update::<32, _>( + &ctx, + base_signature_slot, + base_attested_slot, + base_finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + true, + &scm, + ); + // set invalid finalized header branch + update_invalid_finalized_header_branch + .light_client_update + .finalized_header + .1[2] = H256::default(); + let res = SyncProtocolVerifier::default().validate_consensus_update( + &ctx, + &store, + &update_invalid_finalized_header_branch, + ); + assert!(res.is_err(), "{:?}", res); + + update_invalid_finalized_header_branch + .light_client_update + .finalized_header + .1 = vec![]; + let res = SyncProtocolVerifier::default().validate_consensus_update( + &ctx, + &store, + &update_invalid_finalized_header_branch, + ); + assert!(res.is_err(), "{:?}", res); + } + { + let mut update_invalid_finality_branch = gen_light_client_update::<32, _>( + &ctx, + base_signature_slot, + base_attested_slot, + base_finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + true, + &scm, + ); + update_invalid_finality_branch.finalized_execution_branch[0] = H256::default(); + let res = SyncProtocolVerifier::default().validate_consensus_update( + &ctx, + &store, + &update_invalid_finality_branch, + ); + assert!(res.is_err(), "{:?}", res); + update_invalid_finality_branch.finalized_execution_branch = vec![]; // empty branch + let res = SyncProtocolVerifier::default().validate_consensus_update( + &ctx, + &store, + &update_invalid_finality_branch, + ); + assert!(res.is_err(), "{:?}", res); + } { // // | @@ -1125,6 +1216,7 @@ mod tests { finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default() @@ -1161,6 +1253,7 @@ mod tests { finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default() @@ -1200,6 +1293,7 @@ mod tests { finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default() @@ -1230,6 +1324,7 @@ mod tests { base_finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1252,6 +1347,7 @@ mod tests { base_finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1288,6 +1384,7 @@ mod tests { base_finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default().validate_consensus_update( From 8f0196cf1e272aba3fd13fda9b46dfe7959d0fb3 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 10 Nov 2024 15:43:51 +0900 Subject: [PATCH 09/11] remove unnecessary validation and add some comments Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 23 +++------ crates/light-client-verifier/src/errors.rs | 2 - crates/light-client-verifier/src/mock.rs | 32 ++++++------- crates/light-client-verifier/src/updates.rs | 48 ++++++++++++++----- 4 files changed, 58 insertions(+), 47 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index b6b2851..49d4b05 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -261,18 +261,6 @@ pub fn validate_light_client_update< signature_period, )); } - let finalized_period = - compute_sync_committee_period_at_slot(ctx, consensus_update.finalized_beacon_header().slot); - let attested_period = - compute_sync_committee_period_at_slot(ctx, consensus_update.attested_beacon_header().slot); - // the following condition must always be false - if finalized_period != attested_period && finalized_period + 1 != attested_period { - return Err(Error::InconsistentFinalizedPeriod( - finalized_period, - attested_period, - )); - } - store.ensure_relevant_update(ctx, consensus_update)?; // https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md#validate_light_client_update @@ -1169,7 +1157,7 @@ mod tests { assert!(res.is_err(), "{:?}", res); } { - let mut update_invalid_finality_branch = gen_light_client_update::<32, _>( + let mut update_invalid_finalized_execution_branch = gen_light_client_update::<32, _>( &ctx, base_signature_slot, base_attested_slot, @@ -1179,18 +1167,19 @@ mod tests { true, &scm, ); - update_invalid_finality_branch.finalized_execution_branch[0] = H256::default(); + update_invalid_finalized_execution_branch.finalized_execution_branch[0] = + H256::default(); let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, &store, - &update_invalid_finality_branch, + &update_invalid_finalized_execution_branch, ); assert!(res.is_err(), "{:?}", res); - update_invalid_finality_branch.finalized_execution_branch = vec![]; // empty branch + update_invalid_finalized_execution_branch.finalized_execution_branch = vec![]; // empty branch let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, &store, - &update_invalid_finality_branch, + &update_invalid_finalized_execution_branch, ); assert!(res.is_err(), "{:?}", res); } diff --git a/crates/light-client-verifier/src/errors.rs b/crates/light-client-verifier/src/errors.rs index ad7c897..219f387 100644 --- a/crates/light-client-verifier/src/errors.rs +++ b/crates/light-client-verifier/src/errors.rs @@ -21,8 +21,6 @@ pub enum Error { UnexpectedFinalizedPeriod(SyncCommitteePeriod, SyncCommitteePeriod, String), /// store does not cover the signature period: `store={0} signature={1}` StoreNotCoveredSignaturePeriod(SyncCommitteePeriod, SyncCommitteePeriod), - /// inconsistent attested period: `finalized={0} attested={1}` - InconsistentFinalizedPeriod(SyncCommitteePeriod, SyncCommitteePeriod), /// cannot rotate to next sync committee: `store={0} finalized={1}` CannotRotateNextSyncCommittee(SyncCommitteePeriod, SyncCommitteePeriod), /// no next sync committee in store: `store_period={0} signature_period={1}` diff --git a/crates/light-client-verifier/src/mock.rs b/crates/light-client-verifier/src/mock.rs index 286acae..421c812 100644 --- a/crates/light-client-verifier/src/mock.rs +++ b/crates/light-client-verifier/src/mock.rs @@ -50,26 +50,24 @@ impl MockStore { if store_period == finalized_period { // store_period == finalized_period <= attested_period <= signature_period - let attested_period = compute_sync_committee_period_at_slot( - ctx, - consensus_update.attested_beacon_header().slot, - ); - if finalized_period == attested_period { - if consensus_update.next_sync_committee().is_some() { - new_store.next_sync_committee = consensus_update.next_sync_committee().cloned(); - } - } else if finalized_period + 1 == attested_period { - // no-op: because `consensus_update` has already validated with the `store.next_sync_committee()` - } else { - return Err(crate::errors::Error::UnexpectedAttestedPeriod( - store_period, - attested_period, - "attested period must be equal to finalized_period or finalized_period+1" - .into(), - )); + if consensus_update.has_finalized_next_sync_committee(ctx) { + // finalized_period == attested_period + new_store.next_sync_committee = consensus_update.next_sync_committee().cloned(); } } else if store_period + 1 == finalized_period { // store_period + 1 == finalized_period == attested_period == signature_period + debug_assert_eq!( + compute_sync_committee_period_at_slot( + ctx, + consensus_update.attested_beacon_header().slot + ), + finalized_period + ); + debug_assert_eq!( + compute_sync_committee_period_at_slot(ctx, consensus_update.signature_slot()), + finalized_period + ); + if let Some(committee) = self.next_sync_committee.as_ref() { new_store.current_sync_committee = committee.clone(); new_store.next_sync_committee = consensus_update.next_sync_committee().cloned(); diff --git a/crates/light-client-verifier/src/updates.rs b/crates/light-client-verifier/src/updates.rs index bf9c1e9..8fa7e2f 100644 --- a/crates/light-client-verifier/src/updates.rs +++ b/crates/light-client-verifier/src/updates.rs @@ -1,6 +1,8 @@ use crate::context::{ChainConsensusVerificationContext, ConsensusVerificationContext}; use crate::errors::Error; use crate::internal_prelude::*; +use ethereum_consensus::compute::compute_sync_committee_period_at_slot; +use ethereum_consensus::context::ChainContext; use ethereum_consensus::{ beacon::{BeaconBlockHeader, Slot}, merkle::is_valid_normalized_merkle_branch, @@ -14,32 +16,41 @@ pub mod deneb; pub trait LightClientBootstrap: core::fmt::Debug + Clone + PartialEq + Eq { + /// finalized header fn beacon_header(&self) -> &BeaconBlockHeader; + /// current sync committee corresponding to `beacon_header.state_root` fn current_sync_committee(&self) -> &SyncCommittee; - /// length of the branch should be `CURRENT_SYNC_COMMITTEE_DEPTH` + /// merkle branch of `current_sync_committee` within `BeaconState` fn current_sync_committee_branch(&self) -> Vec; } /// ConsensusUpdate is an update info of the consensus layer corresponding to a specific light client update +/// +/// NOTE: The design is intended to minimise data type differences between forks. +/// ref. +/// - https://github.com/ethereum/consensus-specs/blob/a09d0c321550c5411557674a981e2b444a1178c0/specs/altair/light-client/sync-protocol.md#lightclientupdate +/// - https://github.com/ethereum/consensus-specs/blob/a09d0c321550c5411557674a981e2b444a1178c0/specs/capella/light-client/sync-protocol.md#modified-lightclientheader pub trait ConsensusUpdate: core::fmt::Debug + Clone + PartialEq + Eq { + /// header attested to by the sync committee fn attested_beacon_header(&self) -> &BeaconBlockHeader; - + /// next sync committee corresponding to `attested_beacon_header.state_root` fn next_sync_committee(&self) -> Option<&SyncCommittee>; - /// length of the branch should be `NEXT_SYNC_COMMITTEE_DEPTH` + /// merkle branch of `next_sync_committee` within `BeaconState` fn next_sync_committee_branch(&self) -> Option>; - + /// finalized header corresponding to `attested_beacon_header.state_root` fn finalized_beacon_header(&self) -> &BeaconBlockHeader; - /// length of the branch should be `FINALIZED_ROOT_DEPTH` + /// merkle branch of `finalized_checkpoint.root` within `BeaconState`. This is called `finality_branch` in the spec. fn finalized_beacon_header_branch(&self) -> Vec; - - fn finalized_execution_root(&self) -> H256; - /// length of the branch should be `EXECUTION_PAYLOAD_DEPTH` - fn finalized_execution_branch(&self) -> Vec; - + /// sync committee aggregate signature fn sync_aggregate(&self) -> &SyncAggregate; + /// slot at which the aggregate signature was created (untrusted) fn signature_slot(&self) -> Slot; + /// root of execution payload corresponding to `finalized_beacon_header.body_root` + fn finalized_execution_root(&self) -> H256; + /// merkle branch of `execution_payload` within `BeaconBlockBody` + fn finalized_execution_branch(&self) -> Vec; /// ref. https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md#is_valid_light_client_header /// NOTE: There are no validation for the execution payload, so you should implement it if the update contains the execution payload. @@ -57,6 +68,13 @@ pub trait ConsensusUpdate: .map_err(Error::InvalidFinalizedExecutionPayload) } + /// Returns whether the contained next sync committee is finalized + fn has_finalized_next_sync_committee(&self, ctx: &C) -> bool { + self.next_sync_committee().is_some() + && compute_sync_committee_period_at_slot(ctx, self.attested_beacon_header().slot) + == compute_sync_committee_period_at_slot(ctx, self.finalized_beacon_header().slot) + } + /// validate the basic properties of the update fn validate_basic(&self, ctx: &C) -> Result<(), Error> { // ensure that sync committee's aggreated key matches pubkeys @@ -85,11 +103,15 @@ pub trait ConsensusUpdate: /// ExecutionUpdate is an update info of the execution payload pub trait ExecutionUpdate: core::fmt::Debug + Clone + PartialEq + Eq { + /// `state_root` of the execution payload fn state_root(&self) -> H256; + /// merkle branch of `state_root` within `ExecutionPayload` fn state_root_branch(&self) -> Vec; + /// `block_number` of the execution payload fn block_number(&self) -> U64; + /// merkle branch of `block_number` within `ExecutionPayload` fn block_number_branch(&self) -> Vec; - + /// validate the basic properties of the update fn validate_basic(&self) -> Result<(), Error> { if self.state_root_branch().is_empty() { return Err(Error::EmptyExecutionPayloadStateRootBranch); @@ -101,13 +123,17 @@ pub trait ExecutionUpdate: core::fmt::Debug + Clone + PartialEq + Eq { } } +/// LightClientBootstrapInfo is a basic type for the light client bootstrap pub type LightClientBootstrapInfo = bellatrix::LightClientBootstrapInfo; +/// LightClientUpdate is a basic type for the light client update pub type LightClientUpdate = ethereum_consensus::fork::bellatrix::LightClientUpdate; +/// ConsensusUpdateInfo is a basic type for the consensus update pub type ConsensusUpdateInfo = bellatrix::ConsensusUpdateInfo; +/// ExecutionUpdateInfo is a basic type for the execution update pub type ExecutionUpdateInfo = bellatrix::ExecutionUpdateInfo; From 47c3ddd0a522cc88b80b47036e3082bb03277120 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 10 Nov 2024 17:09:24 +0900 Subject: [PATCH 10/11] remove default implementation of `ensure_relevant_update` Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/mock.rs | 9 +++++++++ crates/light-client-verifier/src/state.rs | 14 ++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/light-client-verifier/src/mock.rs b/crates/light-client-verifier/src/mock.rs index 421c812..9ac6592 100644 --- a/crates/light-client-verifier/src/mock.rs +++ b/crates/light-client-verifier/src/mock.rs @@ -109,4 +109,13 @@ impl LightClientStoreReader Option> { self.next_sync_committee.clone() } + + fn ensure_relevant_update>( + &self, + _ctx: &CC, + _update: &C, + ) -> Result<(), crate::errors::Error> { + // every update is relevant + Ok(()) + } } diff --git a/crates/light-client-verifier/src/state.rs b/crates/light-client-verifier/src/state.rs index 14c151a..e097104 100644 --- a/crates/light-client-verifier/src/state.rs +++ b/crates/light-client-verifier/src/state.rs @@ -8,10 +8,10 @@ pub trait LightClientStoreReader { /// Returns the current sync committee period fn current_period(&self, ctx: &CC) -> SyncCommitteePeriod; - /// Returns the current sync committee corresponding to the current period if available + /// Returns the current sync committee corresponding to the `current_period()` if available fn current_sync_committee(&self) -> Option>; - /// Returns the next sync committee corresponding to the next period if available + /// Returns the next sync committee corresponding to the `current_period() + 1` if available fn next_sync_committee(&self) -> Option>; /// Returns the sync committee corresponding to the given signature period if available @@ -31,14 +31,12 @@ pub trait LightClientStoreReader { } } - /// Returns a error indicating whether the update is relevant to the light client store. + /// Returns a error indicating whether the update is relevant to this store. /// /// This method should be used to determine whether the update should be applied to the store. fn ensure_relevant_update>( &self, - _ctx: &CC, - _update: &C, - ) -> Result<(), Error> { - Ok(()) - } + ctx: &CC, + update: &C, + ) -> Result<(), Error>; } From c0fba4759cd3f5116fdf6c75ffb1b2a62c76b0b6 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 10 Nov 2024 17:43:41 +0900 Subject: [PATCH 11/11] add `get_sync_committee_at_period` function Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 8 ++-- crates/light-client-verifier/src/state.rs | 39 +++++++++++-------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index 49d4b05..55f1204 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -2,7 +2,7 @@ use crate::context::{ChainConsensusVerificationContext, ConsensusVerificationCon use crate::errors::Error; use crate::internal_prelude::*; use crate::misbehaviour::Misbehaviour; -use crate::state::LightClientStoreReader; +use crate::state::{get_sync_committee_at_period, LightClientStoreReader}; use crate::updates::{ConsensusUpdate, ExecutionUpdate, LightClientBootstrap}; use core::marker::PhantomData; use ethereum_consensus::beacon::{BeaconBlockHeader, Root, DOMAIN_SYNC_COMMITTEE}; @@ -163,7 +163,7 @@ impl Result, Error> { let update_signature_period = compute_sync_committee_period_at_slot(ctx, update.signature_slot()); - if let Some(committee) = store.get_sync_committee(ctx, update_signature_period) { + if let Some(committee) = get_sync_committee_at_period(ctx, store, update_signature_period) { Ok(committee) } else { Err(Error::UnexpectedSingaturePeriod( @@ -331,7 +331,9 @@ pub fn validate_light_client_update< ctx, consensus_update.attested_beacon_header().slot, ); - if let Some(committee) = store.get_sync_committee(ctx, update_attested_period + 1) { + if let Some(committee) = + get_sync_committee_at_period(ctx, store, update_attested_period + 1) + { if committee != *update_next_sync_committee { return Err(Error::InconsistentNextSyncCommittee( committee.aggregate_pubkey.clone(), diff --git a/crates/light-client-verifier/src/state.rs b/crates/light-client-verifier/src/state.rs index e097104..fbea558 100644 --- a/crates/light-client-verifier/src/state.rs +++ b/crates/light-client-verifier/src/state.rs @@ -4,6 +4,7 @@ use ethereum_consensus::{ sync_protocol::{SyncCommittee, SyncCommitteePeriod}, }; +/// A trait for reading the light client store pub trait LightClientStoreReader { /// Returns the current sync committee period fn current_period(&self, ctx: &CC) -> SyncCommitteePeriod; @@ -14,23 +15,6 @@ pub trait LightClientStoreReader { /// Returns the next sync committee corresponding to the `current_period() + 1` if available fn next_sync_committee(&self) -> Option>; - /// Returns the sync committee corresponding to the given signature period if available - fn get_sync_committee( - &self, - ctx: &CC, - signature_period: SyncCommitteePeriod, - ) -> Option> { - let current_period = self.current_period(ctx); - let next_period = current_period + 1; - if signature_period == current_period { - self.current_sync_committee() - } else if signature_period == next_period { - self.next_sync_committee() - } else { - None - } - } - /// Returns a error indicating whether the update is relevant to this store. /// /// This method should be used to determine whether the update should be applied to the store. @@ -40,3 +24,24 @@ pub trait LightClientStoreReader { update: &C, ) -> Result<(), Error>; } + +/// Returns the sync committee corresponding to the given signature period if available +pub fn get_sync_committee_at_period< + CC: ChainContext, + const SYNC_COMMITTEE_SIZE: usize, + ST: LightClientStoreReader, +>( + ctx: &CC, + store: &ST, + signature_period: SyncCommitteePeriod, +) -> Option> { + let current_period = store.current_period(ctx); + let next_period = current_period + 1; + if signature_period == current_period { + store.current_sync_committee() + } else if signature_period == next_period { + store.next_sync_committee() + } else { + None + } +}