From 3c5cbd8e600d972b19f498bdf49922953900f456 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Wed, 13 Nov 2024 23:07:18 +0900 Subject: [PATCH 1/6] add comments Signed-off-by: Jun Kimura --- crates/ibc/src/client_state.rs | 23 ++++++++++++++++++----- crates/ibc/src/header.rs | 5 +++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/crates/ibc/src/client_state.rs b/crates/ibc/src/client_state.rs index a4dd004..67ff574 100644 --- a/crates/ibc/src/client_state.rs +++ b/crates/ibc/src/client_state.rs @@ -43,29 +43,42 @@ pub const ETHEREUM_ACCOUNT_STORAGE_ROOT_INDEX: usize = 2; #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct ClientState { - /// Chain parameters + // Verification parameters + /// `genesis_validators_root` of the target beacon chain's BeaconState pub genesis_validators_root: Root, + /// https://github.com/ethereum/consensus-specs/blob/a09d0c321550c5411557674a981e2b444a1178c0/specs/altair/light-client/sync-protocol.md#misc pub min_sync_committee_participants: U64, + /// `genesis_time` of the target beacon chain's BeaconState pub genesis_time: U64, + /// fork parameters of the target beacon chain pub fork_parameters: ForkParameters, + /// https://github.com/ethereum/consensus-specs/blob/a09d0c321550c5411557674a981e2b444a1178c0/configs/mainnet.yaml#L69 pub seconds_per_slot: U64, + /// https://github.com/ethereum/consensus-specs/blob/a09d0c321550c5411557674a981e2b444a1178c0/presets/mainnet/phase0.yaml#L36 pub slots_per_epoch: Slot, + /// https://github.com/ethereum/consensus-specs/blob/a09d0c321550c5411557674a981e2b444a1178c0/presets/mainnet/altair.yaml#L18 pub epochs_per_sync_committee_period: Epoch, - /// IBC Solidity parameters + /// An address of IBC contract on execution layer pub ibc_address: Address, + /// The IBC contract's base storage location for storing commitments + /// https://github.com/hyperledger-labs/yui-ibc-solidity/blob/0e83dc7aadf71380dae6e346492e148685510663/docs/architecture.md#L46 pub ibc_commitments_slot: H256, - /// Light Client parameters + /// `trust_level` is threshold of sync committee participants to consider the attestation as valid. Highly recommended to be 2/3. pub trust_level: Fraction, + /// `trusting_period` is the period in which the consensus state is considered trusted pub trusting_period: Duration, + /// `max_clock_drift` defines how much new finalized header's time can drift into the future pub max_clock_drift: Duration, - /// State + // State + /// The latest block number of the stored consensus state pub latest_execution_block_number: U64, + /// `frozen_height` is the height at which the client is considered frozen. If `None`, the client is unfrozen. pub frozen_height: Option, - /// Verifier + // Verifiers #[serde(skip)] pub consensus_verifier: SyncProtocolVerifier>, diff --git a/crates/ibc/src/header.rs b/crates/ibc/src/header.rs index ba7fe33..f0e17e1 100644 --- a/crates/ibc/src/header.rs +++ b/crates/ibc/src/header.rs @@ -62,10 +62,15 @@ impl From> #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct Header { + /// trusted sync committee corresponding to the period of the signature slot of the `consensus_update` pub trusted_sync_committee: TrustedSyncCommittee, + /// consensus update attested by the `trusted_sync_committee` pub consensus_update: ConsensusUpdateInfo, + /// execution update based on the `consensus_update.finalized_header` pub execution_update: ExecutionUpdateInfo, + /// account update based on the `execution_update.state_root` pub account_update: AccountUpdateInfo, + /// timestamp of the `consensus_update.finalized_header` pub timestamp: Timestamp, } From ef0ec3f9c4d7840439b4259d104ba6b57c7ea869 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Wed, 13 Nov 2024 23:07:40 +0900 Subject: [PATCH 2/6] fix to check if `client_id` of misbehaviour matches `client_id` argument Signed-off-by: Jun Kimura --- crates/ibc/src/client_state.rs | 6 ++++++ crates/ibc/src/errors.rs | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/client_state.rs b/crates/ibc/src/client_state.rs index 67ff574..417bcd7 100644 --- a/crates/ibc/src/client_state.rs +++ b/crates/ibc/src/client_state.rs @@ -437,6 +437,12 @@ impl Ics2ClientState for ClientState::try_from(misbehaviour)?; misbehaviour.validate()?; + if misbehaviour.client_id != client_id { + return Err( + Error::UnexpectedClientIdInMisbehaviour(client_id, misbehaviour.client_id).into(), + ); + } + let consensus_state = match maybe_consensus_state( ctx, &ClientConsensusStatePath::new(&client_id, &misbehaviour.trusted_sync_committee.height), diff --git a/crates/ibc/src/errors.rs b/crates/ibc/src/errors.rs index cd99f19..04a1485 100644 --- a/crates/ibc/src/errors.rs +++ b/crates/ibc/src/errors.rs @@ -8,7 +8,11 @@ use ethereum_consensus::{ types::{H256, U64}, }; use ibc::{ - core::{ics02_client::error::ClientError, ics24_host::error::ValidationError, ContextError}, + core::{ + ics02_client::error::ClientError, + ics24_host::{error::ValidationError, identifier::ClientId}, + ContextError, + }, timestamp::{ParseTimestampError, Timestamp, TimestampOverflowError}, Height, }; @@ -115,6 +119,8 @@ pub enum Error { UnknownMessageType(String), /// cannot initialize frozen client CannotInitializeFrozenClient, + /// unexpected client ID in misbehaviour: expected={0} got={1} + UnexpectedClientIdInMisbehaviour(ClientId, ClientId), } impl Error { From dbdb71c1946ea13e6e420eed84899421789fb694 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Wed, 13 Nov 2024 23:22:44 +0900 Subject: [PATCH 3/6] add comments Signed-off-by: Jun Kimura --- crates/ibc/src/misbehaviour.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ibc/src/misbehaviour.rs b/crates/ibc/src/misbehaviour.rs index b95dddb..37970ce 100644 --- a/crates/ibc/src/misbehaviour.rs +++ b/crates/ibc/src/misbehaviour.rs @@ -31,8 +31,11 @@ pub const ETHEREUM_NEXT_SYNC_COMMITTEE_MISBEHAVIOUR_TYPE_URL: &str = #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Misbehaviour { + /// The client identifier pub client_id: ClientId, + /// The sync committee related to the misbehaviour pub trusted_sync_committee: TrustedSyncCommittee, + /// The misbehaviour data pub data: MisbehaviourData>, } From c7514cf370ba0bdb559e0cf56a64ad4ae6aa6de2 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Wed, 13 Nov 2024 23:28:42 +0900 Subject: [PATCH 4/6] remove unused code Signed-off-by: Jun Kimura --- crates/ibc/src/commitment.rs | 24 +----------------------- crates/ibc/src/errors.rs | 10 ---------- 2 files changed, 1 insertion(+), 33 deletions(-) diff --git a/crates/ibc/src/commitment.rs b/crates/ibc/src/commitment.rs index 31b25e4..36122d3 100644 --- a/crates/ibc/src/commitment.rs +++ b/crates/ibc/src/commitment.rs @@ -1,14 +1,10 @@ use crate::errors::Error; use crate::internal_prelude::*; -use ethereum_consensus::types::{Address, H256}; +use ethereum_consensus::types::H256; use ibc::core::ics24_host::Path; use rlp::Rlp; use tiny_keccak::{Hasher, Keccak}; -pub fn calculate_account_path(ibc_address: &Address) -> H256 { - keccak_256(&ibc_address.0).into() -} - pub fn calculate_ibc_commitment_storage_key(ibc_commitments_slot: &H256, path: Path) -> H256 { keccak_256( &[ @@ -37,24 +33,6 @@ pub fn decode_eip1184_rlp_proof(proof: Vec) -> Result>, Error> { } } -pub fn extract_storage_root_from_account(account_rlp: &[u8]) -> Result { - let r = Rlp::new(account_rlp); - if !r.is_list() { - let items: Vec> = r.as_list()?; - if items.len() != 4 { - Err(Error::InvalidProofFormatError( - "proof must be rlp list".into(), - )) - } else { - Ok(H256::from_slice(items.get(2).unwrap())) - } - } else { - Err(Error::InvalidProofFormatError( - "proof must be rlp list".into(), - )) - } -} - fn keccak_256(input: &[u8]) -> [u8; 32] { let mut out = [0u8; 32]; let mut k = Keccak::v256(); diff --git a/crates/ibc/src/errors.rs b/crates/ibc/src/errors.rs index 04a1485..70f08ce 100644 --- a/crates/ibc/src/errors.rs +++ b/crates/ibc/src/errors.rs @@ -38,12 +38,8 @@ pub enum Error { InvalidNextSyncCommitteeKeys(PublicKey, PublicKey), /// invalid proof format error: {0} InvalidProofFormatError(String), - /// rlp decode error: {0} - RLPDecodeError(rlp::DecoderError), /// account storage root mismatch: expected={0} actual={1} state_root={2} address={3} account_proof={4:?} AccountStorageRootMismatch(H256, H256, H256, String, Vec), - /// invalid account storage root: {0:?} - InvalidAccountStorageRoot(Vec), /// store does not support the finalized_period: store_period={0} finalized_period={1} StoreNotSupportedFinalizedPeriod(U64, U64), /// both updates of misbehaviour data must have same period: {0} != {1} @@ -143,12 +139,6 @@ impl From for ContextError { } } -impl From for Error { - fn from(value: rlp::DecoderError) -> Self { - Error::RLPDecodeError(value) - } -} - impl From for Error { fn from(value: ethereum_consensus::errors::Error) -> Self { Error::EthereumConsensusError(value) From 2f13fa532f0d16acbd62b90f4e36c6864fb248b6 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Wed, 13 Nov 2024 23:45:03 +0900 Subject: [PATCH 5/6] fix Signed-off-by: Jun Kimura --- crates/ibc/src/client_state.rs | 8 +++++--- crates/ibc/src/commitment.rs | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/ibc/src/client_state.rs b/crates/ibc/src/client_state.rs index 417bcd7..abbd465 100644 --- a/crates/ibc/src/client_state.rs +++ b/crates/ibc/src/client_state.rs @@ -1,4 +1,4 @@ -use crate::commitment::{calculate_ibc_commitment_storage_key, decode_eip1184_rlp_proof}; +use crate::commitment::{calculate_ibc_commitment_storage_location, decode_eip1184_rlp_proof}; use crate::consensus_state::{ConsensusState, TrustedConsensusState}; use crate::errors::Error; use crate::header::Header; @@ -201,7 +201,8 @@ impl ClientState { ), }); } - let key = calculate_ibc_commitment_storage_key(&self.ibc_commitments_slot, path.clone()); + let key = + calculate_ibc_commitment_storage_location(&self.ibc_commitments_slot, path.clone()); self.execution_verifier .verify_membership( root, @@ -239,7 +240,8 @@ impl ClientState { ), }); } - let key = calculate_ibc_commitment_storage_key(&self.ibc_commitments_slot, path.clone()); + let key = + calculate_ibc_commitment_storage_location(&self.ibc_commitments_slot, path.clone()); self.execution_verifier .verify_non_membership(root, key.as_bytes(), proof.clone()) .map_err(|e| ClientError::ClientSpecific { diff --git a/crates/ibc/src/commitment.rs b/crates/ibc/src/commitment.rs index 36122d3..33374fc 100644 --- a/crates/ibc/src/commitment.rs +++ b/crates/ibc/src/commitment.rs @@ -5,7 +5,10 @@ use ibc::core::ics24_host::Path; use rlp::Rlp; use tiny_keccak::{Hasher, Keccak}; -pub fn calculate_ibc_commitment_storage_key(ibc_commitments_slot: &H256, path: Path) -> H256 { +/// Calculate the storage location for the commitment stored in the IBC contract +/// +/// The spec is here: https://github.com/hyperledger-labs/yui-ibc-solidity/blob/0e83dc7aadf71380dae6e346492e148685510663/docs/architecture.md#L46 +pub fn calculate_ibc_commitment_storage_location(ibc_commitments_slot: &H256, path: Path) -> H256 { keccak_256( &[ &keccak_256(&path.into_bytes()), From b7867e304ebd523dc23b93c877e95a23bf18ac61 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Wed, 13 Nov 2024 23:55:43 +0900 Subject: [PATCH 6/6] add `verify_delay_passed` Signed-off-by: Jun Kimura --- crates/ibc/src/client_state.rs | 65 +++++++++++++++++++++++++++++++--- crates/ibc/src/errors.rs | 14 ++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/crates/ibc/src/client_state.rs b/crates/ibc/src/client_state.rs index abbd465..e5f016c 100644 --- a/crates/ibc/src/client_state.rs +++ b/crates/ibc/src/client_state.rs @@ -22,6 +22,7 @@ use ibc::core::ics02_client::client_state::{ClientState as Ics2ClientState, Upda use ibc::core::ics02_client::client_type::ClientType; use ibc::core::ics02_client::consensus_state::ConsensusState as Ics02ConsensusState; use ibc::core::ics02_client::error::ClientError; +use ibc::core::ics03_connection::connection::ConnectionEnd; use ibc::core::ics24_host::identifier::{ChainId, ClientId}; use ibc::core::ics24_host::path::ClientConsensusStatePath; use ibc::core::ics24_host::Path; @@ -578,7 +579,7 @@ impl Ics2ClientState for ClientState Ics2ClientState for ClientState Result<(), ClientError> { + verify_delay_passed(ctx, proof_height, connection_end)?; self.verify_membership( proof_height, connection_end.counterparty().prefix(), @@ -598,7 +600,7 @@ impl Ics2ClientState for ClientState Ics2ClientState for ClientState Result<(), ClientError> { + verify_delay_passed(ctx, proof_height, connection_end)?; self.verify_membership( proof_height, connection_end.counterparty().prefix(), @@ -618,7 +621,7 @@ impl Ics2ClientState for ClientState Ics2ClientState for ClientState Result<(), ClientError> { + verify_delay_passed(ctx, proof_height, connection_end)?; let mut seq_bytes = Vec::new(); u64::from(sequence) .encode(&mut seq_bytes) @@ -643,13 +647,14 @@ impl Ics2ClientState for ClientState Result<(), ClientError> { + verify_delay_passed(ctx, proof_height, connection_end)?; self.verify_non_membership( proof_height, connection_end.counterparty().prefix(), @@ -958,6 +963,58 @@ fn trim_left_zero(value: &[u8]) -> &[u8] { &value[pos..] } +// A copy from https://github.com/cosmos/ibc-rs/blob/eea4f0e7a1887f2f1cb18a550d08bb805a08240a/crates/ibc/src/clients/ics07_tendermint/client_state.rs#L1031 +fn verify_delay_passed( + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, +) -> Result<(), ClientError> { + let current_timestamp = ctx.host_timestamp().map_err(|e| ClientError::Other { + description: e.to_string(), + })?; + let current_height = ctx.host_height().map_err(|e| ClientError::Other { + description: e.to_string(), + })?; + + let client_id = connection_end.client_id(); + let processed_time = + ctx.client_update_time(client_id, &height) + .map_err(|_| Error::ProcessedTimeNotFound { + client_id: client_id.clone(), + height, + })?; + let processed_height = ctx.client_update_height(client_id, &height).map_err(|_| { + Error::ProcessedHeightNotFound { + client_id: client_id.clone(), + height, + } + })?; + + let delay_period_time = connection_end.delay_period(); + let delay_period_height = ctx.block_delay(&delay_period_time); + + let earliest_time = + (processed_time + delay_period_time).map_err(Error::TimestampOverflowError)?; + if !(current_timestamp == earliest_time || current_timestamp.after(&earliest_time)) { + return Err(Error::NotEnoughTimeElapsed { + current_timestamp, + earliest_time, + } + .into()); + } + + let earliest_height = processed_height.add(delay_period_height); + if current_height < earliest_height { + return Err(Error::NotEnoughBlocksElapsed { + current_height, + earliest_height, + } + .into()); + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/ibc/src/errors.rs b/crates/ibc/src/errors.rs index 70f08ce..4b34a89 100644 --- a/crates/ibc/src/errors.rs +++ b/crates/ibc/src/errors.rs @@ -117,6 +117,20 @@ pub enum Error { CannotInitializeFrozenClient, /// unexpected client ID in misbehaviour: expected={0} got={1} UnexpectedClientIdInMisbehaviour(ClientId, ClientId), + /// Processed time for the client `{client_id}` at height `{height}` not found + ProcessedTimeNotFound { client_id: ClientId, height: Height }, + /// Processed height for the client `{client_id}` at height `{height}` not found + ProcessedHeightNotFound { client_id: ClientId, height: Height }, + /// not enough time elapsed, current timestamp `{current_timestamp}` is still less than earliest acceptable timestamp `{earliest_time}` + NotEnoughTimeElapsed { + current_timestamp: Timestamp, + earliest_time: Timestamp, + }, + /// not enough blocks elapsed, current height `{current_height}` is still less than earliest acceptable height `{earliest_height}` + NotEnoughBlocksElapsed { + current_height: Height, + earliest_height: Height, + }, } impl Error {