feat: ensure light client uniqueness for each rollup

clients/sov-celestia-cw/src/tests/fixture.rs

@@ -89,7 +89,6 @@ impl Fixture {
             .build();
 
         let sov_client_state = ClientStateConfig::builder()
-            .rollup_id(self.chain_id.clone())
             .latest_height(self.trusted_height)
             .tendermint_params(tendermint_params)
             .build();

clients/sov-celestia/src/client_state/execution.rs

@@ -77,7 +77,7 @@ where
         substitute_client_state: Any,
     ) -> Result<(), ClientError> {
         update_on_recovery(
-            self.inner(),
+            self.inner().clone(),
             ctx,
             subject_client_id,
             substitute_client_state,
@@ -206,7 +206,7 @@ where
 }
 
 pub fn update_state_on_upgrade<E>(
-    _client_state: &SovTmClientState,
+    client_state: &SovTmClientState,
     ctx: &mut E,
     client_id: &ClientId,
     upgraded_client_state: Any,
@@ -222,16 +222,24 @@ where
 
     upgraded_client_state.zero_custom_fields();
 
-    // Construct new client state and consensus state relayer chosen client
-    // parameters are ignored. All chain-chosen parameters come from
-    // committed client, all client-chosen parameters come from current
-    // client.
+    // Constructs the new client state. All rollup-chosen parameters come from
+    // `upgraded_client_state` except the `genesis_state_root`. All
+    // relayer-chosen parameters come from the current client.
+    let new_da_params = TmClientParams::new(
+        upgraded_client_state.da_params.chain_id,
+        client_state.da_params.trust_level,
+        client_state.da_params.trusting_period,
+        upgraded_client_state.da_params.unbonding_period,
+        client_state.da_params.max_clock_drift,
+    );
+
     let new_client_state = SovTmClientState::new(
-        upgraded_client_state.rollup_id,
+        client_state.genesis_state_root.clone(),
+        upgraded_client_state.code_commitment,
         upgraded_client_state.latest_height,
         None,
         upgraded_client_state.upgrade_path,
-        upgraded_client_state.da_params,
+        new_da_params,
     );
 
     // The new consensus state is merely used as a trusted kernel against
@@ -287,7 +295,7 @@ where
 /// verified substitute client state in response to a successful client
 /// recovery.
 pub fn update_on_recovery<E>(
-    subject_client_state: &SovTmClientState,
+    subject_client_state: SovTmClientState,
     ctx: &mut E,
     subject_client_id: &ClientId,
     substitute_client_state: Any,
@@ -303,17 +311,18 @@ where
     let trusting_period = substitute_client_state.da_params.trusting_period;
     let latest_height = substitute_client_state.latest_height;
 
-    let new_client_state = SovTmClientState {
-        rollup_id: subject_client_state.rollup_id.clone(),
+    let new_client_state = SovTmClientState::new(
+        subject_client_state.genesis_state_root,
+        subject_client_state.code_commitment,
         latest_height,
-        frozen_height: None,
-        upgrade_path: subject_client_state.upgrade_path.clone(),
-        da_params: TmClientParams {
+        None,
+        subject_client_state.upgrade_path,
+        TmClientParams {
             chain_id,
             trusting_period,
             ..subject_client_state.da_params
         },
-    };
+    );
 
     let host_timestamp = E::host_timestamp(ctx)?;
     let host_height = E::host_height(ctx)?;

clients/sov-celestia/src/client_state/misbehaviour.rs

@@ -1,21 +1,20 @@
 use ibc_client_tendermint::client_state::verify_misbehaviour_header;
 use ibc_client_tendermint::context::TmVerifier;
 use ibc_core::client::types::error::ClientError;
-use ibc_core::host::types::identifiers::{ChainId, ClientId};
+use ibc_core::host::types::identifiers::ClientId;
 use ibc_core::host::types::path::ClientConsensusStatePath;
 use sov_celestia_client_types::client_message::SovTmMisbehaviour;
-use tendermint_light_client_verifier::options::Options;
+use sov_celestia_client_types::client_state::SovTmClientState;
 
 use crate::context::{ConsensusStateConverter, ValidationContext as SovValidationContext};
 
 /// Determines whether or not two conflicting headers at the same height would
 /// have convinced the light client.
 pub fn verify_misbehaviour<V>(
     ctx: &V,
+    client_state: &SovTmClientState,
     misbehaviour: &SovTmMisbehaviour,
     client_id: &ClientId,
-    chain_id: &ChainId,
-    options: &Options,
     verifier: &impl TmVerifier,
 ) -> Result<(), ClientError>
 where
@@ -52,8 +51,8 @@ where
 
     verify_misbehaviour_header(
         &header_1.da_header,
-        chain_id,
-        options,
+        client_state.chain_id(),
+        &client_state.as_light_client_options()?,
         trusted_consensus_state_1.timestamp(),
         trusted_consensus_state_1.da_params.next_validators_hash,
         current_timestamp,
@@ -62,8 +61,8 @@ where
 
     verify_misbehaviour_header(
         &header_2.da_header,
-        chain_id,
-        options,
+        client_state.chain_id(),
+        &client_state.as_light_client_options()?,
         trusted_consensus_state_2.timestamp(),
         trusted_consensus_state_2.da_params.next_validators_hash,
         current_timestamp,

clients/sov-celestia/src/client_state/update_client.rs

@@ -4,7 +4,10 @@ use ibc_core::client::types::error::ClientError;
 use ibc_core::client::types::Height;
 use ibc_core::host::types::identifiers::{ChainId, ClientId};
 use ibc_core::host::types::path::ClientConsensusStatePath;
-use sov_celestia_client_types::client_message::SovTmHeader;
+use sov_celestia_client_types::client_message::{
+    AggregatedProof, CodeCommitment, Root, SovTmHeader,
+};
+use sov_celestia_client_types::client_state::SovTmClientState;
 use sov_celestia_client_types::consensus_state::SovTmConsensusState;
 use sov_celestia_client_types::error::IntoResult;
 use tendermint_light_client_verifier::options::Options;
@@ -17,10 +20,9 @@ use crate::context::{ConsensusStateConverter, ValidationContext as SovValidation
 /// of the DA header and the aggregated proof date validation.
 pub fn verify_header<V>(
     ctx: &V,
+    client_state: &SovTmClientState,
     header: &SovTmHeader,
     client_id: &ClientId,
-    chain_id: &ChainId,
-    options: &Options,
     verifier: &impl TmVerifier,
 ) -> Result<(), ClientError>
 where
@@ -34,13 +36,17 @@ where
         ctx,
         &header.da_header,
         client_id,
-        chain_id,
-        options,
+        client_state.chain_id(),
+        &client_state.as_light_client_options()?,
         verifier,
     )?;
 
-    // TODO: Implement the verification of the `AggregatedProof`.
-    // aggregated_proof.verify()?;
+    verify_aggregated_proof(
+        ctx,
+        client_state.genesis_state_root(),
+        client_state.code_commitment(),
+        &header.aggregated_proof,
+    )?;
 
     Ok(())
 }
@@ -126,6 +132,34 @@ where
     Ok(())
 }
 
+pub fn verify_aggregated_proof<V>(
+    _ctx: &V,
+    genesis_state_root: &Root,
+    code_commitment: &CodeCommitment,
+    aggregated_proof: &AggregatedProof,
+) -> Result<(), ClientError>
+where
+    V: SovValidationContext,
+    V::ConsensusStateRef: ConsensusStateConverter,
+{
+    if !genesis_state_root.matches(aggregated_proof.genesis_state_root()) {
+        return Err(ClientError::Other {
+            description: "genesis state root does not match".to_string(),
+        });
+    }
+
+    if !code_commitment.matches(aggregated_proof.code_commitment()) {
+        return Err(ClientError::Other {
+            description: "code commitment does not match".to_string(),
+        });
+    }
+
+    // TODO: Implement the verification of the `AggregatedProof`.
+    // aggregated_proof.verify()?;
+
+    Ok(())
+}
+
 /// Checks for DA misbehaviour upon receiving a new consensus state as part of a
 /// client update.
 pub fn check_da_misbehaviour_on_update<V>(

clients/sov-celestia/src/client_state/validation.rs

@@ -71,25 +71,11 @@ where
     match client_message.type_url.as_str() {
         SOV_TENDERMINT_HEADER_TYPE_URL => {
             let header = SovTmHeader::try_from(client_message)?;
-            verify_header(
-                ctx,
-                &header,
-                client_id,
-                client_state.chain_id(),
-                &client_state.as_light_client_options()?,
-                verifier,
-            )
+            verify_header(ctx, client_state, &header, client_id, verifier)
         }
         SOV_TENDERMINT_MISBEHAVIOUR_TYPE_URL => {
             let misbehaviour = SovTmMisbehaviour::try_from(client_message)?;
-            verify_misbehaviour(
-                ctx,
-                &misbehaviour,
-                client_id,
-                client_state.chain_id(),
-                &client_state.as_light_client_options()?,
-                verifier,
-            )
+            verify_misbehaviour(ctx, client_state, &misbehaviour, client_id, verifier)
         }
         _ => Err(ClientError::InvalidUpdateClientMessage),
     }
@@ -186,7 +172,8 @@ where
     V::ConsensusStateRef: ConsensusStateConverter,
 {
     let SovTmClientState {
-        rollup_id: subject_rollup_id,
+        genesis_state_root: subject_genesis_state_root,
+        code_commitment: subject_code_commitment,
         latest_height: _,
         frozen_height: _,
         upgrade_path: subject_upgrade_path,
@@ -196,14 +183,16 @@ where
     let substitute_client_state = SovTmClientState::try_from(substitute_client_state)?;
 
     let SovTmClientState {
-        rollup_id: substitute_rollup_id,
+        genesis_state_root: substitute_genesis_state_root,
+        code_commitment: substitute_code_commitment,
         latest_height: _,
         frozen_height: _,
         upgrade_path: substitute_upgrade_path,
         da_params: substitute_da_params,
     } = substitute_client_state;
 
-    (subject_rollup_id == &substitute_rollup_id
+    (subject_genesis_state_root == &substitute_genesis_state_root
+        && subject_code_commitment == &substitute_code_commitment
         && subject_upgrade_path == &substitute_upgrade_path
         && subject_da_params.trust_level == substitute_da_params.trust_level
         && subject_da_params.max_clock_drift == substitute_da_params.max_clock_drift

clients/sov-celestia/types/src/client_message/aggregated_proof.rs

@@ -57,6 +57,26 @@ impl AggregatedProof {
         &self.serialized_proof
     }
 
+    pub fn initial_slot_number(&self) -> Height {
+        self.public_data.initial_slot_number
+    }
+
+    pub fn final_slot_number(&self) -> Height {
+        self.public_data.final_slot_number
+    }
+
+    pub fn genesis_state_root(&self) -> &Root {
+        &self.public_data.genesis_state_root
+    }
+
+    pub fn final_state_root(&self) -> &Root {
+        &self.public_data.final_state_root
+    }
+
+    pub fn code_commitment(&self) -> &CodeCommitment {
+        &self.public_data.code_commitment
+    }
+
     pub fn validate_basic(&self) -> Result<(), Error> {
         self.public_data.basic_validate()?;
 
@@ -129,26 +149,6 @@ pub struct AggregatedProofPublicData {
 }
 
 impl AggregatedProofPublicData {
-    pub fn initial_slot_number(&self) -> Height {
-        self.initial_slot_number
-    }
-
-    pub fn final_slot_number(&self) -> Height {
-        self.final_slot_number
-    }
-
-    pub fn genesis_state_root(&self) -> &Root {
-        &self.genesis_state_root
-    }
-
-    pub fn final_state_root(&self) -> &Root {
-        &self.final_state_root
-    }
-
-    pub fn code_commitment(&self) -> &CodeCommitment {
-        &self.code_commitment
-    }
-
     pub fn basic_validate(&self) -> Result<(), Error> {
         if self.validity_conditions.is_empty() {
             return Err(Error::empty("validity conditions"));
@@ -335,6 +335,10 @@ impl CodeCommitment {
     pub fn is_empty(&self) -> bool {
         self.0.is_empty()
     }
+
+    pub fn matches(&self, other: &CodeCommitment) -> bool {
+        self.0 == other.0
+    }
 }
 
 impl Display for CodeCommitment {
@@ -436,6 +440,12 @@ impl From<SerializedAggregatedProof> for SovSerializedAggregatedProof {
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Root([u8; 32]);
 
+impl Root {
+    pub fn matches(&self, other: &Root) -> bool {
+        self.0 == other.0
+    }
+}
+
 impl AsRef<[u8; 32]> for Root {
     fn as_ref(&self) -> &[u8; 32] {
         &self.0

clients/sov-celestia/types/src/client_message/header.rs

@@ -49,7 +49,7 @@ impl SovTmHeader {
 
     /// Returns the height of the Sovereign-Tendermint header.
     pub fn height(&self) -> Height {
-        self.aggregated_proof.public_data.final_slot_number()
+        self.aggregated_proof.final_slot_number()
     }
 
     /// Returns the trusted height of the Sovereign-Tendermint header, which
@@ -67,7 +67,7 @@ impl SovTmHeader {
 
         if self.height() != self.da_header.height() {
             return Err(Error::mismatch(format!(
-                "DA header height {} does not match aggregated proof height(rollup slot number) {}",
+                "DA header height {} does not match aggregated proof height(slot number) {}",
                 self.da_header.height(),
                 self.height()
             )))?;

clients/sov-celestia/types/src/client_message/mod.rs

@@ -116,7 +116,7 @@ pub mod test_util {
         pub initial_slot_hash: Vec<u8>,
         #[builder(default = vec![0; 32])]
         pub final_slot_hash: Vec<u8>,
-        #[builder(default = CodeCommitment::from(vec![0; 32]))]
+        #[builder(default = CodeCommitment::from(vec![1; 32]))]
         pub code_commitment: CodeCommitment,
     }