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..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}; @@ -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(()) } @@ -148,7 +154,7 @@ impl>( &self, ctx: &CC, @@ -157,10 +163,11 @@ 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( + store.current_period(ctx), update_signature_period, "store does not have the sync committee corresponding to the update signature period" .into(), @@ -230,7 +237,7 @@ pub fn verify_sync_committee_attestation< /// validate_light_client_update validates a light client update /// -/// NOTE: we can skip the validation of the attested header's execution payload here because we do not reference it in the light client protocol +/// NOTE: we can skip the validation of the attested header's execution payload inclusion here because we do not use it in our light client implementation. pub fn validate_light_client_update< const SYNC_COMMITTEE_SIZE: usize, CC: ChainConsensusVerificationContext, @@ -241,6 +248,21 @@ pub fn validate_light_client_update< store: &ST, consensus_update: &CU, ) -> 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, + )); + } + 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`. @@ -309,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(), @@ -476,10 +500,17 @@ 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] } } + #[allow(clippy::too_many_arguments)] pub fn gen_light_client_update< const SYNC_COMMITTEE_SIZE: usize, C: ChainConsensusVerificationContext, @@ -490,6 +521,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); @@ -503,6 +535,7 @@ pub mod test_utils { execution_block_number, scm.get_committee(signature_period.into()), scm.get_committee((attested_period + 1).into()), + is_update_contain_next_sync_committee, SYNC_COMMITTEE_SIZE, ) } @@ -520,6 +553,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!( @@ -539,7 +573,7 @@ pub mod test_utils { ctx, attested_slot, finalized_root, - sync_committee.to_committee(), + Default::default(), next_sync_committee.to_committee(), ); @@ -565,10 +599,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 { @@ -945,8 +983,8 @@ mod tests { }; #[test] - fn test_lc() { - let scm = MockSyncCommitteeManager::<32>::new(1, 4); + fn test_consensus_update_validation() { + let scm = MockSyncCommitteeManager::<32>::new(1, 6); let ctx = LightClientContext::new_with_config( config::minimal::get_config(), Default::default(), @@ -958,43 +996,96 @@ mod tests { .as_secs() .into(), ); - let period_1 = 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: period_1, + 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_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 dummy_execution_state_root = [1u8; 32].into(); let dummy_execution_block_number = 1; { - let update_valid = gen_light_client_update::<32, _>( + // valid update (store_period == finalized_period == signature_period) + 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_invalid_no_next_sync_committee_branch, + ); + assert!(res.is_err(), "{:?}", res); + } + { + let update_insufficient_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(), - &scm, + 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( &ctx, &store, - &update_valid, + &update_insufficient_attestations, ); - assert!(res.is_ok(), "{:?}", res); + assert!(res.is_err(), "{:?}", res); } { - let update_insufficient_attestations = gen_light_client_update_with_params( + let update_sufficient_attestations = gen_light_client_update_with_params( &ctx, base_signature_slot, base_attested_slot, @@ -1002,15 +1093,16 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), - 21, + scm.get_committee(base_store_period + 1), + true, + 22, // sufficient attestations ); let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, &store, - &update_insufficient_attestations, + &update_sufficient_attestations, ); - assert!(res.is_err(), "{:?}", res); + assert!(res.is_ok(), "{:?}", res); } { let update_zero_attestations = gen_light_client_update_with_params( @@ -1021,8 +1113,9 @@ mod tests { dummy_execution_state_root, dummy_execution_block_number.into(), current_sync_committee, - scm.get_committee(3), - 0, + scm.get_committee(base_store_period + 1), + true, + 0, // insufficient attestations. at least 22 is required. ); let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, @@ -1031,6 +1124,177 @@ 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_finalized_execution_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_finalized_execution_branch.finalized_execution_branch[0] = + H256::default(); + let res = SyncProtocolVerifier::default().validate_consensus_update( + &ctx, + &store, + &update_invalid_finalized_execution_branch, + ); + assert!(res.is_err(), "{:?}", res); + update_invalid_finalized_execution_branch.finalized_execution_branch = vec![]; // empty branch + let res = SyncProtocolVerifier::default().validate_consensus_update( + &ctx, + &store, + &update_invalid_finalized_execution_branch, + ); + assert!(res.is_err(), "{:?}", res); + } + { + // + // | + // +-----------+ | +-----------+ +-----------+ +-----------+ + // | store | <-- | finalized | <-- | attested | <-- | signature | + // +-----------+ | +-----------+ +-----------+ +-----------+ + // | + // | + // sync committee + // period boundary + // + 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; + let update = gen_light_client_update( + &ctx, + signature_slot, + attested_slot, + finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + true, + &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(next_period.into()).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 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; + let update = gen_light_client_update( + &ctx, + signature_slot, + attested_slot, + finalized_epoch, + dummy_execution_state_root, + dummy_execution_block_number.into(), + true, + &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(base_store_period + 1).to_committee(), + ), + ..store.clone() + }; + let res = SyncProtocolVerifier::default() + .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(), + true, + &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); + } { // // | @@ -1051,6 +1315,7 @@ mod tests { base_finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1061,7 +1326,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, _>( @@ -1071,6 +1338,7 @@ mod tests { base_finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default().validate_consensus_update( @@ -1095,36 +1363,43 @@ 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, _>( + let update_not_finalized_next_sync_committee = gen_light_client_update::<32, _>( &ctx, next_period_signature_slot, next_period_attested_slot, base_finalized_epoch, dummy_execution_state_root, dummy_execution_block_number.into(), + true, &scm, ); let res = SyncProtocolVerifier::default().validate_consensus_update( &ctx, &store, - &update_invalid_inconsistent_periods, + &update_not_finalized_next_sync_committee, ); - 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); + 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); } } #[test] - fn test_lc_misbehaviour() { - let scm = MockSyncCommitteeManager::<32>::new(1, 4); + fn test_misbehaviour_validation() { + 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(), @@ -1137,11 +1412,17 @@ mod tests { .as_secs() .into(), ); - let start_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: start_slot, + slot: base_store_slot, ..Default::default() }; let store = MockStore::new( @@ -1152,9 +1433,6 @@ mod tests { 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, @@ -1164,7 +1442,8 @@ 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), + true, 32, ); @@ -1177,7 +1456,8 @@ 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 + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1199,7 +1479,8 @@ 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 + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1221,7 +1502,8 @@ 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 + true, 21, // at least 22 is required ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1245,7 +1527,8 @@ 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 + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1258,6 +1541,33 @@ 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(base_store_period + 2), // `base_store_period+1` is really correct + true, + 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, _>( @@ -1268,7 +1578,8 @@ 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), + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( @@ -1292,7 +1603,8 @@ 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), + true, 32, ); let res = SyncProtocolVerifier::default().validate_misbehaviour( diff --git a/crates/light-client-verifier/src/errors.rs b/crates/light-client-verifier/src/errors.rs index 7c94c17..219f387 100644 --- a/crates/light-client-verifier/src/errors.rs +++ b/crates/light-client-verifier/src/errors.rs @@ -13,12 +13,14 @@ 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), /// 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}` @@ -51,6 +53,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(); diff --git a/crates/light-client-verifier/src/mock.rs b/crates/light-client-verifier/src/mock.rs index 705e46e..9ac6592 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,45 @@ 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 + 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 == attested_period { + } 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(); } 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 +98,24 @@ impl MockStore { impl LightClientStoreReader for MockStore { - fn get_sync_committee( + 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() + } + + fn ensure_relevant_update>( &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 - } + _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 b4b9d8c..fbea558 100644 --- a/crates/light-client-verifier/src/state.rs +++ b/crates/light-client-verifier/src/state.rs @@ -4,22 +4,44 @@ use ethereum_consensus::{ sync_protocol::{SyncCommittee, SyncCommitteePeriod}, }; +/// A trait for reading the light client store pub trait LightClientStoreReader { - /// Returns the sync committee for the given period. - fn get_sync_committee( - &self, - ctx: &CC, - period: SyncCommitteePeriod, - ) -> Option>; + /// 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 a error indicating whether the update is relevant to the light client store. + /// Returns the next sync committee corresponding to the `current_period() + 1` if available + fn next_sync_committee(&self) -> Option>; + + /// 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> { - update.ensure_consistent_update_period(ctx) + ) -> 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 } } diff --git a/crates/light-client-verifier/src/updates.rs b/crates/light-client-verifier/src/updates.rs index 4f21d41..8fa7e2f 100644 --- a/crates/light-client-verifier/src/updates.rs +++ b/crates/light-client-verifier/src/updates.rs @@ -16,47 +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; - - 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, - )) - } - } + /// 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. @@ -74,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 @@ -102,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); @@ -118,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;