Refactoring and add verify_delay_passed checks

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;
@@ -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;
@@ -43,29 +44,42 @@ pub const ETHEREUM_ACCOUNT_STORAGE_ROOT_INDEX: usize = 2;
 
 #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
 pub struct ClientState<const SYNC_COMMITTEE_SIZE: usize> {
-    /// 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<Height>,
 
-    /// Verifier
+    // Verifiers
     #[serde(skip)]
     pub consensus_verifier:
         SyncProtocolVerifier<SYNC_COMMITTEE_SIZE, TrustedConsensusState<SYNC_COMMITTEE_SIZE>>,
@@ -188,7 +202,8 @@ impl<const SYNC_COMMITTEE_SIZE: usize> ClientState<SYNC_COMMITTEE_SIZE> {
                 ),
             });
         }
-        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,
@@ -226,7 +241,8 @@ impl<const SYNC_COMMITTEE_SIZE: usize> ClientState<SYNC_COMMITTEE_SIZE> {
                 ),
             });
         }
-        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 {
@@ -424,6 +440,12 @@ impl<const SYNC_COMMITTEE_SIZE: usize> Ics2ClientState for ClientState<SYNC_COMM
         }
         let misbehaviour = Misbehaviour::<SYNC_COMMITTEE_SIZE>::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),
@@ -557,14 +579,15 @@ impl<const SYNC_COMMITTEE_SIZE: usize> Ics2ClientState for ClientState<SYNC_COMM
 
     fn verify_packet_data(
         &self,
-        _ctx: &dyn ibc::core::ValidationContext,
+        ctx: &dyn ibc::core::ValidationContext,
         proof_height: ibc::Height,
         connection_end: &ibc::core::ics03_connection::connection::ConnectionEnd,
         proof: &ibc::core::ics23_commitment::commitment::CommitmentProofBytes,
         root: &ibc::core::ics23_commitment::commitment::CommitmentRoot,
         commitment_path: &ibc::core::ics24_host::path::CommitmentPath,
         commitment: ibc::core::ics04_channel::commitment::PacketCommitment,
     ) -> Result<(), ClientError> {
+        verify_delay_passed(ctx, proof_height, connection_end)?;
         self.verify_membership(
             proof_height,
             connection_end.counterparty().prefix(),
@@ -577,14 +600,15 @@ impl<const SYNC_COMMITTEE_SIZE: usize> Ics2ClientState for ClientState<SYNC_COMM
 
     fn verify_packet_acknowledgement(
         &self,
-        _ctx: &dyn ibc::core::ValidationContext,
+        ctx: &dyn ibc::core::ValidationContext,
         proof_height: ibc::Height,
         connection_end: &ibc::core::ics03_connection::connection::ConnectionEnd,
         proof: &ibc::core::ics23_commitment::commitment::CommitmentProofBytes,
         root: &ibc::core::ics23_commitment::commitment::CommitmentRoot,
         ack_path: &ibc::core::ics24_host::path::AckPath,
         ack: ibc::core::ics04_channel::commitment::AcknowledgementCommitment,
     ) -> Result<(), ClientError> {
+        verify_delay_passed(ctx, proof_height, connection_end)?;
         self.verify_membership(
             proof_height,
             connection_end.counterparty().prefix(),
@@ -597,14 +621,15 @@ impl<const SYNC_COMMITTEE_SIZE: usize> Ics2ClientState for ClientState<SYNC_COMM
 
     fn verify_next_sequence_recv(
         &self,
-        _ctx: &dyn ibc::core::ValidationContext,
+        ctx: &dyn ibc::core::ValidationContext,
         proof_height: ibc::Height,
         connection_end: &ibc::core::ics03_connection::connection::ConnectionEnd,
         proof: &ibc::core::ics23_commitment::commitment::CommitmentProofBytes,
         root: &ibc::core::ics23_commitment::commitment::CommitmentRoot,
         seq_recv_path: &ibc::core::ics24_host::path::SeqRecvPath,
         sequence: ibc::core::ics04_channel::packet::Sequence,
     ) -> Result<(), ClientError> {
+        verify_delay_passed(ctx, proof_height, connection_end)?;
         let mut seq_bytes = Vec::new();
         u64::from(sequence)
             .encode(&mut seq_bytes)
@@ -622,13 +647,14 @@ impl<const SYNC_COMMITTEE_SIZE: usize> Ics2ClientState for ClientState<SYNC_COMM
 
     fn verify_packet_receipt_absence(
         &self,
-        _ctx: &dyn ibc::core::ValidationContext,
+        ctx: &dyn ibc::core::ValidationContext,
         proof_height: ibc::Height,
         connection_end: &ibc::core::ics03_connection::connection::ConnectionEnd,
         proof: &ibc::core::ics23_commitment::commitment::CommitmentProofBytes,
         root: &ibc::core::ics23_commitment::commitment::CommitmentRoot,
         receipt_path: &ibc::core::ics24_host::path::ReceiptPath,
     ) -> Result<(), ClientError> {
+        verify_delay_passed(ctx, proof_height, connection_end)?;
         self.verify_non_membership(
             proof_height,
             connection_end.counterparty().prefix(),
@@ -937,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::*;

crates/ibc/src/commitment.rs

@@ -1,15 +1,14 @@
 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 {
+/// 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()),
@@ -37,24 +36,6 @@ pub fn decode_eip1184_rlp_proof(proof: Vec<u8>) -> Result<Vec<Vec<u8>>, Error> {
     }
 }
 
-pub fn extract_storage_root_from_account(account_rlp: &[u8]) -> Result<H256, Error> {
-    let r = Rlp::new(account_rlp);
-    if !r.is_list() {
-        let items: Vec<Vec<u8>> = 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();

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,
 };
@@ -34,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<String>),
-    /// invalid account storage root: {0:?}
-    InvalidAccountStorageRoot(Vec<u8>),
     /// 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}
@@ -115,6 +115,22 @@ pub enum Error {
     UnknownMessageType(String),
     /// cannot initialize frozen client
     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 {
@@ -137,12 +153,6 @@ impl From<Error> for ContextError {
     }
 }
 
-impl From<rlp::DecoderError> for Error {
-    fn from(value: rlp::DecoderError) -> Self {
-        Error::RLPDecodeError(value)
-    }
-}
-
 impl From<ethereum_consensus::errors::Error> for Error {
     fn from(value: ethereum_consensus::errors::Error) -> Self {
         Error::EthereumConsensusError(value)

crates/ibc/src/header.rs

@@ -62,10 +62,15 @@ impl<const SYNC_COMMITTEE_SIZE: usize> From<ClientMessage<SYNC_COMMITTEE_SIZE>>
 
 #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 pub struct Header<const SYNC_COMMITTEE_SIZE: usize> {
+    /// trusted sync committee corresponding to the period of the signature slot of the `consensus_update`
     pub trusted_sync_committee: TrustedSyncCommittee<SYNC_COMMITTEE_SIZE>,
+    /// consensus update attested by the `trusted_sync_committee`
     pub consensus_update: ConsensusUpdateInfo<SYNC_COMMITTEE_SIZE>,
+    /// 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,
 }
 

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<const SYNC_COMMITTEE_SIZE: usize> {
+    /// The client identifier
     pub client_id: ClientId,
+    /// The sync committee related to the misbehaviour
     pub trusted_sync_committee: TrustedSyncCommittee<SYNC_COMMITTEE_SIZE>,
+    /// The misbehaviour data
     pub data: MisbehaviourData<SYNC_COMMITTEE_SIZE, ConsensusUpdateInfo<SYNC_COMMITTEE_SIZE>>,
 }