From 023ae6d95a7adb301a35ae4d94766b06aef5a5a3 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Fri, 25 Oct 2024 19:31:49 +0900 Subject: [PATCH 1/4] refactor verifier implementation Signed-off-by: Jun Kimura --- crates/light-client-cli/src/client.rs | 41 +++-- crates/light-client-cli/src/state.rs | 33 +--- crates/light-client-verifier/src/consensus.rs | 171 ++++++++++++------ crates/light-client-verifier/src/errors.rs | 7 + crates/light-client-verifier/src/mock.rs | 53 +++--- crates/light-client-verifier/src/state.rs | 59 ++---- crates/light-client-verifier/src/updates.rs | 25 ++- 7 files changed, 216 insertions(+), 173 deletions(-) diff --git a/crates/light-client-cli/src/client.rs b/crates/light-client-cli/src/client.rs index b8afef5..2d4868f 100644 --- a/crates/light-client-cli/src/client.rs +++ b/crates/light-client-cli/src/client.rs @@ -20,8 +20,8 @@ use ethereum_consensus::{ use ethereum_light_client_verifier::{ consensus::SyncProtocolVerifier, context::{ConsensusVerificationContext, Fraction, LightClientContext}, - state::apply_sync_committee_update, - updates::deneb::ConsensusUpdateInfo, + state::should_update_sync_committees, + updates::{deneb::ConsensusUpdateInfo, ConsensusUpdate}, }; use log::*; use std::time::SystemTime; @@ -267,23 +267,36 @@ impl< self.verifier .validate_updates(vctx, state, &updates.0, &updates.1)?; - self.verifier - .ensure_relevant_update(vctx, state, &updates.0)?; - let mut updated = false; + self.apply_light_client_update(state, updates.0) + } + + fn apply_light_client_update( + &self, + state: &LightClientStore, + consensus_update: ConsensusUpdateInfo< + SYNC_COMMITTEE_SIZE, + BYTES_PER_LOGS_BLOOM, + MAX_EXTRA_DATA_BYTES, + >, + ) -> Result< + Option>, + > { let mut new_state = state.clone(); - if apply_sync_committee_update(&self.ctx, &mut new_state, &updates.0)? { - updated = true; + let (current_committee, next_committee) = + should_update_sync_committees(&self.ctx, state, &consensus_update)?; + if let Some(current_committee) = current_committee { + new_state.current_sync_committee = current_committee.clone(); } - if updates.0.finalized_header.execution.block_number - > state.latest_execution_payload_header.block_number - { + if let Some(next_committee) = next_committee { + new_state.next_sync_committee = next_committee.cloned(); + } + if consensus_update.finalized_beacon_header().slot > state.latest_finalized_header.slot { + new_state.latest_finalized_header = consensus_update.finalized_beacon_header().clone(); new_state.latest_execution_payload_header = - updates.0.finalized_header.execution.clone(); - updated = true; + consensus_update.finalized_header.execution.clone(); } - - if updated { + if *state != new_state { self.ctx.store_light_client_state(&new_state)?; Ok(Some(new_state)) } else { diff --git a/crates/light-client-cli/src/state.rs b/crates/light-client-cli/src/state.rs index 37d462a..3b58536 100644 --- a/crates/light-client-cli/src/state.rs +++ b/crates/light-client-cli/src/state.rs @@ -4,10 +4,7 @@ use ethereum_consensus::{ sync_protocol::SyncCommittee, types::{H256, U64}, }; -use ethereum_light_client_verifier::{ - state::{SyncCommitteeKeeper, SyncCommitteeView}, - updates::ExecutionUpdate, -}; +use ethereum_light_client_verifier::{state::LightClientStoreReader, updates::ExecutionUpdate}; #[derive(Debug, Clone, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct LightClientStore< @@ -52,7 +49,7 @@ impl< const SYNC_COMMITTEE_SIZE: usize, const BYTES_PER_LOGS_BLOOM: usize, const MAX_EXTRA_DATA_BYTES: usize, - > SyncCommitteeView + > LightClientStoreReader for LightClientStore { fn current_slot(&self) -> Slot { @@ -68,32 +65,6 @@ impl< } } -impl< - const SYNC_COMMITTEE_SIZE: usize, - const BYTES_PER_LOGS_BLOOM: usize, - const MAX_EXTRA_DATA_BYTES: usize, - > SyncCommitteeKeeper - for LightClientStore -{ - fn set_finalized_header(&mut self, header: BeaconBlockHeader) { - self.latest_finalized_header = header; - } - - fn set_current_sync_committee( - &mut self, - current_sync_committee: SyncCommittee, - ) { - self.current_sync_committee = current_sync_committee; - } - - fn set_next_sync_committee( - &mut self, - next_sync_committee: Option>, - ) { - self.next_sync_committee = next_sync_committee; - } -} - #[derive(Debug, Clone, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct ExecutionUpdateInfo { pub state_root: H256, diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index 9cb8c62..3eb52f9 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -2,12 +2,10 @@ use crate::context::ConsensusVerificationContext; use crate::errors::{Error, MisbehaviourError}; use crate::internal_prelude::*; use crate::misbehaviour::Misbehaviour; -use crate::state::SyncCommitteeView; +use crate::state::LightClientStoreReader; use crate::updates::{ConsensusUpdate, ExecutionUpdate, LightClientBootstrap}; use core::marker::PhantomData; -use ethereum_consensus::beacon::{ - Root, BLOCK_BODY_EXECUTION_PAYLOAD_LEAF_INDEX, DOMAIN_SYNC_COMMITTEE, -}; +use ethereum_consensus::beacon::{BeaconBlockHeader, Root, DOMAIN_SYNC_COMMITTEE}; use ethereum_consensus::bls::{fast_aggregate_verify, BLSPublicKey, BLSSignature}; use ethereum_consensus::compute::{ compute_domain, compute_epoch_at_slot, compute_fork_version, compute_signing_root, @@ -20,8 +18,8 @@ use ethereum_consensus::execution::{ use ethereum_consensus::merkle::is_valid_merkle_branch; use ethereum_consensus::sync_protocol::{ SyncCommittee, CURRENT_SYNC_COMMITTEE_DEPTH, CURRENT_SYNC_COMMITTEE_SUBTREE_INDEX, - EXECUTION_PAYLOAD_DEPTH, FINALIZED_ROOT_DEPTH, FINALIZED_ROOT_SUBTREE_INDEX, - NEXT_SYNC_COMMITTEE_DEPTH, NEXT_SYNC_COMMITTEE_SUBTREE_INDEX, + FINALIZED_ROOT_DEPTH, FINALIZED_ROOT_SUBTREE_INDEX, NEXT_SYNC_COMMITTEE_DEPTH, + NEXT_SYNC_COMMITTEE_SUBTREE_INDEX, }; use ethereum_consensus::types::H256; @@ -30,13 +28,13 @@ use ethereum_consensus::types::H256; pub struct SyncProtocolVerifier< const SYNC_COMMITTEE_SIZE: usize, const EXECUTION_PAYLOAD_TREE_DEPTH: usize, - ST: SyncCommitteeView, + ST: LightClientStoreReader, >(PhantomData); impl< const SYNC_COMMITTEE_SIZE: usize, const EXECUTION_PAYLOAD_TREE_DEPTH: usize, - ST: SyncCommitteeView, + ST: LightClientStoreReader, > SyncProtocolVerifier { /// validates a LightClientBootstrap @@ -77,6 +75,7 @@ impl< consensus_update.validate_basic(ctx)?; execution_update.validate_basic()?; + self.ensure_relevant_update(ctx, store, consensus_update)?; self.validate_consensus_update(ctx, store, consensus_update)?; self.validate_execution_update( consensus_update.finalized_execution_root(), @@ -96,17 +95,9 @@ impl< store: &ST, update: &CU, ) -> Result<(), Error> { - let sync_committee = self.get_attestation_verifier(ctx, store, update)?; - verify_merkle_branches_with_attested_header(ctx, update)?; + let sync_committee = self.get_sync_committee(ctx, store, update)?; + validate_light_client_update(ctx, store, update)?; verify_sync_committee_attestation(ctx, update, &sync_committee)?; - is_valid_merkle_branch( - update.finalized_execution_root(), - &update.finalized_execution_branch(), - EXECUTION_PAYLOAD_DEPTH as u32, - BLOCK_BODY_EXECUTION_PAYLOAD_LEAF_INDEX as u64, - update.finalized_beacon_header().body_root.clone(), - ) - .map_err(Error::InvalidFinalizedExecutionPayload)?; Ok(()) } @@ -169,6 +160,8 @@ impl< let update_has_next_sync_committee = store.next_sync_committee().is_none() && (update.next_sync_committee().is_some() && update_attested_period == store_period); + // https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md#validate_light_client_update + // assert (update_attested_slot > store.finalized_header.beacon.slot or update_has_next_sync_committee) if !(update.attested_beacon_header().slot > store.current_slot() || update_has_next_sync_committee) { @@ -181,13 +174,24 @@ impl< ))); } + // https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md#process_light_client_update + // update_has_finalized_next_sync_committee = ( + // not is_next_sync_committee_known(store) + // and is_sync_committee_update(update) and is_finality_update(update) and ( + // compute_sync_committee_period_at_slot(update.finalized_header.beacon.slot) + // == compute_sync_committee_period_at_slot(update.attested_header.beacon.slot) + // ) + // ) let update_has_finalized_next_sync_committee = store.next_sync_committee().is_none() - && update.next_sync_committee().is_some() + && update.next_sync_committee().is_some() // equivalent to is_sync_committee_update(update) && compute_sync_committee_period_at_slot(ctx, update.finalized_beacon_header().slot) == update_attested_period; - if !(update.finalized_beacon_header().slot > store.current_slot() - || update_has_finalized_next_sync_committee) + // https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md#process_light_client_update + // update.finalized_header.beacon.slot > store.finalized_header.beacon.slot + // or update_has_finalized_next_sync_committee + if !(update_has_finalized_next_sync_committee + || update.finalized_beacon_header().slot > store.current_slot()) { return Err(Error::IrrelevantConsensusUpdates(format!( "finalized_beacon_header_slot={} store_slot={} update_has_finalized_next_sync_committee={}", @@ -195,26 +199,11 @@ impl< ))); } - // Verify that the `next_sync_committee`, if present, actually is the next sync committee saved in the - // state of the `attested_header` - if let Some(update_next_sync_committee) = update.next_sync_committee() { - if let Some(store_next_sync_committee) = store.next_sync_committee() { - if update_attested_period == store_period - && store_next_sync_committee != update_next_sync_committee - { - return Err(MisbehaviourError::InconsistentNextSyncCommittee( - store_next_sync_committee.aggregate_pubkey.clone(), - update_next_sync_committee.aggregate_pubkey.clone(), - ) - .into()); - } - } - } Ok(()) } - /// returns a committee that needs to verify the update - pub fn get_attestation_verifier>( + /// get the sync committee from the store + pub fn get_sync_committee>( &self, ctx: &CC, store: &ST, @@ -224,21 +213,30 @@ impl< let update_signature_period = compute_sync_committee_period_at_slot(ctx, update.signature_slot()); - // select sync committee as current view - let sync_committee = if update_signature_period == store_period { - store.current_sync_committee() - } else if update_signature_period == store_period + 1 - && store.next_sync_committee().is_some() - { - store.next_sync_committee().unwrap() + // https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/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 + if update_signature_period == store_period { + Ok(store.current_sync_committee().clone()) + } else if update_signature_period == store_period + 1 { + if let Some(next_sync_committee) = store.next_sync_committee() { + Ok(next_sync_committee.clone()) + } else { + Err(Error::NoNextSyncCommitteeInStore( + store_period.into(), + update_signature_period.into(), + )) + } } else { - return Err(Error::InvalidSingaturePeriod( + Err(Error::InvalidSingaturePeriod( store_period, update_signature_period, "signature period must be equal to store_period or store_period+1".into(), - )); - }; - Ok(sync_committee.clone()) + )) + } } } @@ -285,26 +283,59 @@ pub fn verify_sync_committee_attestation< ) } -/// verify inclusion proofs of finalized header and next sync committee -pub fn verify_merkle_branches_with_attested_header< +/// 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 +pub fn validate_light_client_update< const SYNC_COMMITTEE_SIZE: usize, CC: ChainContext, + ST: LightClientStoreReader, CU: ConsensusUpdate, >( ctx: &CC, + store: &ST, consensus_update: &CU, ) -> Result<(), Error> { + // 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`. // Note that the genesis finalized checkpoint root is represented as a zero hash. + // if not is_finality_update(update): + // assert update.finalized_header == LightClientHeader() + // else: + // if update_finalized_slot == GENESIS_SLOT: + // assert update.finalized_header == LightClientHeader() + // finalized_root = Bytes32() + // else: + // assert is_valid_light_client_header(update.finalized_header) + // finalized_root = hash_tree_root(update.finalized_header.beacon) + // assert is_valid_normalized_merkle_branch( + // leaf=finalized_root, + // branch=update.finality_branch, + // gindex=finalized_root_gindex_at_slot(update.attested_header.beacon.slot), + // root=update.attested_header.beacon.state_root, + // ) + + // we assume that the `finalized_beacon_header_branch`` must be non-empty + if consensus_update.finalized_beacon_header_branch().is_empty() { + return Err(Error::FinalizedHeaderNotFound); + } let finalized_root = if consensus_update.finalized_beacon_header().slot == ctx.fork_parameters().genesis_slot() { + if consensus_update.finalized_beacon_header() != &BeaconBlockHeader::default() { + return Err(Error::NonEmptyBeaconHeaderAtGenesisSlot( + ctx.fork_parameters().genesis_slot().into(), + )); + } Default::default() } else { + // ensure that the finalized header is non-empty + if consensus_update.finalized_beacon_header() == &BeaconBlockHeader::default() { + return Err(Error::FinalizedHeaderNotFound); + } + consensus_update.is_valid_light_client_finalized_header()?; hash_tree_root(consensus_update.finalized_beacon_header().clone())? }; - is_valid_merkle_branch( finalized_root, &consensus_update.finalized_beacon_header_branch(), @@ -314,7 +345,36 @@ pub fn verify_merkle_branches_with_attested_header< ) .map_err(Error::InvalidFinalizedBeaconHeaderMerkleBranch)?; + // # Verify that the `next_sync_committee`, if present, actually is the next sync committee saved in the + // # state of the `attested_header` + // if not is_sync_committee_update(update): + // assert update.next_sync_committee == SyncCommittee() + // else: + // if update_attested_period == store_period and is_next_sync_committee_known(store): + // assert update.next_sync_committee == store.next_sync_committee + // assert is_valid_normalized_merkle_branch( + // leaf=hash_tree_root(update.next_sync_committee), + // branch=update.next_sync_committee_branch, + // gindex=next_sync_committee_gindex_at_slot(update.attested_header.beacon.slot), + // root=update.attested_header.beacon.state_root, + // ) if let Some(update_next_sync_committee) = consensus_update.next_sync_committee() { + let update_attested_period = compute_sync_committee_period_at_slot( + ctx, + consensus_update.attested_beacon_header().slot, + ); + let store_period = compute_sync_committee_period_at_slot(ctx, store.current_slot()); + if let Some(store_next_sync_committee) = store.next_sync_committee() { + if update_attested_period == store_period + && store_next_sync_committee != update_next_sync_committee + { + return Err(MisbehaviourError::InconsistentNextSyncCommittee( + store_next_sync_committee.aggregate_pubkey.clone(), + update_next_sync_committee.aggregate_pubkey.clone(), + ) + .into()); + } + } is_valid_merkle_branch( hash_tree_root(update_next_sync_committee.clone())?, &consensus_update.next_sync_committee_branch().unwrap(), @@ -323,6 +383,10 @@ pub fn verify_merkle_branches_with_attested_header< consensus_update.attested_beacon_header().state_root.clone(), ) .map_err(Error::InvalidNextSyncCommitteeMerkleBranch)?; + } else { + if let Some(branch) = consensus_update.next_sync_committee_branch() { + return Err(Error::NonEmptyNextSyncCommittee(branch.to_vec())); + } } Ok(()) @@ -346,7 +410,6 @@ mod tests_bellatrix { use crate::{ context::{Fraction, LightClientContext}, mock::MockStore, - state::apply_sync_committee_update, updates::{ bellatrix::{ConsensusUpdateInfo, ExecutionUpdateInfo, LightClientBootstrapInfo}, LightClientBootstrap, @@ -448,7 +511,7 @@ mod tests_bellatrix { assert!(verifier .validate_updates(&ctx, &store, &consensus_update, &execution_update) .is_ok()); - let res = apply_sync_committee_update(&ctx, &mut store, &consensus_update); + let res = store.apply_light_client_update(&ctx, &consensus_update); assert!(res.is_ok() && res.unwrap()); } } diff --git a/crates/light-client-verifier/src/errors.rs b/crates/light-client-verifier/src/errors.rs index 4f34063..4d07c8e 100644 --- a/crates/light-client-verifier/src/errors.rs +++ b/crates/light-client-verifier/src/errors.rs @@ -5,6 +5,7 @@ use ethereum_consensus::{ bls::PublicKey, errors::MerkleError, sync_protocol::SyncCommitteePeriod, + types::H256, }; use trie_db::TrieError; @@ -20,6 +21,10 @@ pub enum Error { NotFinalizedUpdate(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}` + NoNextSyncCommitteeInStore(u64, u64), + /// the beacon header at genesis slot must be empty: `slot={0}` + NonEmptyBeaconHeaderAtGenesisSlot(u64), /// verify membership error VerifyMembershipError(), /// trusted root mismatch: `expected={0:?} actual={1:?}` @@ -66,6 +71,8 @@ pub enum Error { InvalidFinalizedExecutionPayload(MerkleError), /// invalid merkle branch of next sync committee: `error={0}` InvalidNextSyncCommitteeMerkleBranch(MerkleError), + /// next sync committee must be empty: `actual={0:?}` + NonEmptyNextSyncCommittee(Vec), /// invalid merkle branch of current sync committee: `error={0}` InvalidCurrentSyncCommitteeMerkleBranch(MerkleError), /// invalid merkle branch of execution state root: `error={0}` diff --git a/crates/light-client-verifier/src/mock.rs b/crates/light-client-verifier/src/mock.rs index fb93316..771913c 100644 --- a/crates/light-client-verifier/src/mock.rs +++ b/crates/light-client-verifier/src/mock.rs @@ -1,4 +1,7 @@ -use crate::state::{SyncCommitteeKeeper, SyncCommitteeView}; +use crate::context::ConsensusVerificationContext; +use crate::state::{should_update_sync_committees, LightClientStoreReader}; +use crate::updates::ConsensusUpdate; +use ethereum_consensus::context::ChainContext; use ethereum_consensus::sync_protocol::SyncCommittee; use ethereum_consensus::{ beacon::{BeaconBlockHeader, Slot}, @@ -26,9 +29,35 @@ impl MockStore { next_sync_committee: None, } } + + pub fn apply_light_client_update< + CC: ChainContext + ConsensusVerificationContext, + CU: ConsensusUpdate, + >( + &mut self, + ctx: &CC, + consensus_update: &CU, + ) -> Result { + let (current_committee, next_committee) = + should_update_sync_committees(ctx, self, consensus_update)?; + let mut updated = false; + if let Some(committee) = current_committee { + self.current_sync_committee = committee.clone(); + updated = true; + } + if let Some(committee) = next_committee { + self.next_sync_committee = committee.cloned(); + updated = true; + } + if consensus_update.finalized_beacon_header().slot > self.latest_finalized_header.slot { + self.latest_finalized_header = consensus_update.finalized_beacon_header().clone(); + updated = true; + } + Ok(updated) + } } -impl SyncCommitteeView +impl LightClientStoreReader for MockStore { fn current_slot(&self) -> Slot { @@ -43,23 +72,3 @@ impl SyncCommitteeView self.next_sync_committee.as_ref() } } - -impl SyncCommitteeKeeper - for MockStore -{ - fn set_finalized_header(&mut self, header: BeaconBlockHeader) { - self.latest_finalized_header = header; - } - fn set_current_sync_committee( - &mut self, - current_sync_committee: SyncCommittee, - ) { - self.current_sync_committee = current_sync_committee; - } - fn set_next_sync_committee( - &mut self, - next_sync_committee: Option>, - ) { - self.next_sync_committee = next_sync_committee; - } -} diff --git a/crates/light-client-verifier/src/state.rs b/crates/light-client-verifier/src/state.rs index e675ea8..3f52daa 100644 --- a/crates/light-client-verifier/src/state.rs +++ b/crates/light-client-verifier/src/state.rs @@ -1,63 +1,28 @@ use crate::{errors::Error, updates::ConsensusUpdate}; use ethereum_consensus::{ - beacon::{BeaconBlockHeader, Slot}, - compute::compute_sync_committee_period_at_slot, - context::ChainContext, + beacon::Slot, compute::compute_sync_committee_period_at_slot, context::ChainContext, sync_protocol::SyncCommittee, }; -pub trait SyncCommitteeView { +pub trait LightClientStoreReader { + /// Returns the finalized slot based on the light client update. fn current_slot(&self) -> Slot; + /// Returns the current sync committee based on the light client update. fn current_sync_committee(&self) -> &SyncCommittee; + /// Returns the next sync committee based on the light client update. fn next_sync_committee(&self) -> Option<&SyncCommittee>; } -pub trait SyncCommitteeKeeper { - fn set_finalized_header(&mut self, header: BeaconBlockHeader); - fn set_current_sync_committee( - &mut self, - current_sync_committee: SyncCommittee, - ); - fn set_next_sync_committee( - &mut self, - next_sync_committee: Option>, - ); -} - -pub fn apply_sync_committee_update< - const SYNC_COMMITTEE_SIZE: usize, - CC: ChainContext, - S: SyncCommitteeView + SyncCommitteeKeeper, - CU: ConsensusUpdate, ->( - ctx: &CC, - state: &mut S, - consensus_update: &CU, -) -> Result { - let mut updated = false; - let (update_current, update_next) = - should_update_sync_committees(ctx, state, consensus_update)?; - if let Some(update_current) = update_current { - state.set_current_sync_committee(update_current.clone()); - updated = true; - } - if let Some(update_next) = update_next { - state.set_next_sync_committee(update_next.map(|c| c.clone())); - updated = true; - } - if consensus_update.finalized_beacon_header().slot > state.current_slot() { - state.set_finalized_header(consensus_update.finalized_beacon_header().clone()); - updated = true; - } - Ok(updated) -} - -fn should_update_sync_committees< +/// Returns the new current and next sync committees based on the state and the consensus update. +/// If the current sync committee should be updated, the new current sync committee is returned. +/// If the next sync committee should be updated, the new next sync committee is returned. +/// ref. https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md#apply_light_client_update +pub fn should_update_sync_committees< 's, 'u, const SYNC_COMMITTEE_SIZE: usize, CC: ChainContext, - S: SyncCommitteeView, + S: LightClientStoreReader, CU: ConsensusUpdate, >( ctx: &CC, @@ -65,7 +30,9 @@ fn should_update_sync_committees< consensus_update: &'u CU, ) -> Result< ( + // new current sync committee Option<&'s SyncCommittee>, + // new next sync committee Option>>, ), Error, diff --git a/crates/light-client-verifier/src/updates.rs b/crates/light-client-verifier/src/updates.rs index d1431ec..ff1e635 100644 --- a/crates/light-client-verifier/src/updates.rs +++ b/crates/light-client-verifier/src/updates.rs @@ -2,7 +2,8 @@ use crate::context::ConsensusVerificationContext; use crate::errors::Error; use crate::internal_prelude::*; use ethereum_consensus::{ - beacon::{BeaconBlockHeader, Slot}, + beacon::{BeaconBlockHeader, Slot, BLOCK_BODY_EXECUTION_PAYLOAD_LEAF_INDEX}, + merkle::is_valid_merkle_branch, sync_protocol::{ SyncAggregate, SyncCommittee, CURRENT_SYNC_COMMITTEE_DEPTH, EXECUTION_PAYLOAD_DEPTH, FINALIZED_ROOT_DEPTH, NEXT_SYNC_COMMITTEE_DEPTH, @@ -39,18 +40,29 @@ pub trait ConsensusUpdate: fn sync_aggregate(&self) -> &SyncAggregate; fn signature_slot(&self) -> Slot; - fn validate_basic(&self, ctx: &C) -> Result<(), Error> { - // ensure that the finalized header is non-empty - if self.finalized_beacon_header() == &BeaconBlockHeader::default() { - return Err(Error::FinalizedHeaderNotFound); - } + /// 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(&self) -> Result<(), Error> { + is_valid_merkle_branch( + self.finalized_execution_root(), + &self.finalized_execution_branch(), + EXECUTION_PAYLOAD_DEPTH as u32, + BLOCK_BODY_EXECUTION_PAYLOAD_LEAF_INDEX as u64, + self.finalized_beacon_header().body_root.clone(), + ) + .map_err(Error::InvalidFinalizedExecutionPayload) + } + fn validate_basic(&self, ctx: &C) -> Result<(), Error> { // ensure that sync committee's aggreated key matches pubkeys if let Some(next_sync_committee) = self.next_sync_committee() { next_sync_committee.validate()?; } // ensure that the order of slots is consistent + // equivalent to: + // `assert current_slot >= update.signature_slot > update_attested_slot >= update_finalized_slot`` + // from the spec: https://github.com/ethereum/consensus-specs/blob/087e7378b44f327cdad4549304fc308613b780c3/specs/altair/light-client/sync-protocol.md?plain=1#L380 if !(ctx.current_slot() >= self.signature_slot() && self.signature_slot() > self.attested_beacon_header().slot && self.attested_beacon_header().slot >= self.finalized_beacon_header().slot) @@ -66,6 +78,7 @@ pub trait ConsensusUpdate: // ensure that suffienct participants exist let participants = self.sync_aggregate().count_participants(); + // from the spec: `assert sum(sync_aggregate.sync_committee_bits) >= MIN_SYNC_COMMITTEE_PARTICIPANTS` if participants < ctx.min_sync_committee_participants() { return Err(Error::LessThanMinimalParticipants( participants, From 68dc44dc8bedf8add08da31c7374580b0ddf7454 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 27 Oct 2024 01:33:17 +0900 Subject: [PATCH 2/4] fix to use correct fork version: https://github.com/ethereum/consensus-specs/pull/3284 Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index 3eb52f9..be6a4aa 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -259,10 +259,8 @@ pub fn verify_sync_committee_attestation< .map(|t| t.1.clone().try_into().unwrap()) .collect(); - let fork_version = compute_fork_version( - ctx, - compute_epoch_at_slot(ctx, consensus_update.signature_slot()), - )?; + let fork_version_slot = consensus_update.signature_slot().max(1.into()) - 1; + let fork_version = compute_fork_version(ctx, compute_epoch_at_slot(ctx, fork_version_slot))?; let domain = compute_domain( ctx, DOMAIN_SYNC_COMMITTEE, From 48c82dac032cddaacd8e8354d89b43ac5f7dd04d Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 27 Oct 2024 10:32:06 +0900 Subject: [PATCH 3/4] merge `MisbehaviourError` into `Error` Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 4 ++-- crates/light-client-verifier/src/errors.rs | 19 ++----------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index be6a4aa..f4f627b 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -1,5 +1,5 @@ use crate::context::ConsensusVerificationContext; -use crate::errors::{Error, MisbehaviourError}; +use crate::errors::Error; use crate::internal_prelude::*; use crate::misbehaviour::Misbehaviour; use crate::state::LightClientStoreReader; @@ -366,7 +366,7 @@ pub fn validate_light_client_update< if update_attested_period == store_period && store_next_sync_committee != update_next_sync_committee { - return Err(MisbehaviourError::InconsistentNextSyncCommittee( + return Err(Error::InconsistentNextSyncCommittee( store_next_sync_committee.aggregate_pubkey.clone(), update_next_sync_committee.aggregate_pubkey.clone(), ) diff --git a/crates/light-client-verifier/src/errors.rs b/crates/light-client-verifier/src/errors.rs index 4d07c8e..000a3ec 100644 --- a/crates/light-client-verifier/src/errors.rs +++ b/crates/light-client-verifier/src/errors.rs @@ -47,8 +47,6 @@ pub enum Error { CommonError(ethereum_consensus::errors::Error), /// rlp decoder error: `{0:?}` RlpDecoderError(rlp::DecoderError), - /// misbehaviour error: `{0}` - Misbehaviour(MisbehaviourError), /// both updates of misbehaviour data must have same period: {0} != {1} DifferentPeriodInNextSyncCommitteeMisbehaviour(SyncCommitteePeriod, SyncCommitteePeriod), /// both updates of misbehaviour data must have next sync committee @@ -79,6 +77,8 @@ pub enum Error { InvalidExecutionStateRootMerkleBranch(MerkleError), /// invalid merkle branch of execution block number: `error={0}` InvalidExecutionBlockNumberMerkleBranch(MerkleError), + /// inconsistent next sync committee: `store:{0:?}` != `update:{1:?}` + InconsistentNextSyncCommittee(PublicKey, PublicKey), /// other error: `{description}` Other { description: String }, } @@ -103,18 +103,3 @@ impl From for Error { Self::RlpDecoderError(value) } } - -#[derive(Debug, Display)] -pub enum MisbehaviourError { - /// next sync committee: `{0:?} != {1:?}` - InconsistentNextSyncCommittee(PublicKey, PublicKey), -} - -#[cfg(feature = "std")] -impl std::error::Error for MisbehaviourError {} - -impl From for Error { - fn from(value: MisbehaviourError) -> Self { - Self::Misbehaviour(value) - } -} From e8e0825432a8dc0d0e1693fdbfb1502f2c762129 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Sun, 27 Oct 2024 10:38:07 +0900 Subject: [PATCH 4/4] fix error name Signed-off-by: Jun Kimura --- crates/light-client-verifier/src/consensus.rs | 2 +- crates/light-client-verifier/src/errors.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/light-client-verifier/src/consensus.rs b/crates/light-client-verifier/src/consensus.rs index f4f627b..e78998f 100644 --- a/crates/light-client-verifier/src/consensus.rs +++ b/crates/light-client-verifier/src/consensus.rs @@ -231,7 +231,7 @@ impl< )) } } else { - Err(Error::InvalidSingaturePeriod( + Err(Error::UnexpectedSingaturePeriod( store_period, update_signature_period, "signature period must be equal to store_period or store_period+1".into(), diff --git a/crates/light-client-verifier/src/errors.rs b/crates/light-client-verifier/src/errors.rs index 000a3ec..2de8143 100644 --- a/crates/light-client-verifier/src/errors.rs +++ b/crates/light-client-verifier/src/errors.rs @@ -13,8 +13,8 @@ type BoxedTrieError = Box>; #[derive(Debug, Display)] pub enum Error { - /// invalid signature period: `store={0} signature={1} reason={2}` - InvalidSingaturePeriod(SyncCommitteePeriod, SyncCommitteePeriod, String), + /// unexpected signature period: `store={0} signature={1} reason={2}` + UnexpectedSingaturePeriod(SyncCommitteePeriod, SyncCommitteePeriod, String), /// invalid finalized period: `store={0} finalized={1} reason={2}` InvalidFinalizedPeriod(SyncCommitteePeriod, SyncCommitteePeriod, String), /// not finalized period: `finalized={0} attested={1}`