feat: upgradable cometbft client (#405)

* build upgraded client and consensus state paths

* add tests for upgraded client and cons state path

* refactor MsgUpgradeClient

* impl update_on_upgrade

* upgrade client is permissionless

* copy impl to mock comet client component

* fill upgrade_validate client component

* refactor

* impl verify_upgrade

* rm unused import

* add upgrade_path in client state

* support upgrade_path in relayer

* rm old code

* fix tests

* cairo Array fields at the end

* use consistent field order

* revert to original field order

* add tests for Store<Array<T>>::size

* refactor new tests

* impl StorePacking via ByteArray for Array<u8> and Array<u32>

* nits

* use Array<u8> store impl for CometConsensusState

* rm redundant into-s

* fix lint

* impl Store for Array<felt252>

* impl StorePacking via Serde

* export impls

* rm redundant store

* use Serde StorePacking for Array<Height> and Array<ProofSpec>

* use Array<ByteArray> for Store

* fix after upgrade_path type change

* add missing changes

* fix cairo compilation

* scaffold protobytes of client and consensus state

* fix Array<felt252> unpack

* add cometbft dep in ibc-core

* serialize comet client and consensus state to protobuf bytes

* update comment for mock client

Author: rnbguy

cairo-contracts/Scarb.lock

@@ -183,6 +183,7 @@ version = "0.1.0"
 dependencies = [
  "alexandria_data_structures",
  "alexandria_numeric",
+ "cometbft",
  "ics23",
  "openzeppelin_testing",
  "protobuf",

cairo-contracts/packages/clients/src/cometbft/client_state.cairo

@@ -1,11 +1,14 @@
+use cometbft::light_client::ClientState as ProtoCometClientState;
 use cometbft::utils::Fraction;
 use core::num::traits::Zero;
-use ics23::ProofSpec;
-use starknet::SyscallResult;
-use starknet::storage_access::{StorageBaseAddress, Store};
+use ics23::{ArrayFelt252Store, ProofSpec};
+use protobuf::types::message::ProtoCodecImpl;
 use starknet_ibc_clients::cometbft::CometErrors;
 use starknet_ibc_core::client::{Duration, Height, HeightPartialOrd, Status, StatusTrait};
 
+pub impl ArrayProofSpecStore = ics23::StorePackingViaSerde<Array<ProofSpec>>;
+pub impl ArrayByteArrayStore = ics23::StorePackingViaSerde<Array<ByteArray>>;
+
 #[derive(Clone, Debug, Drop, PartialEq, Serde, starknet::Store)]
 pub struct CometClientState {
     pub latest_height: Height,
@@ -16,57 +19,7 @@ pub struct CometClientState {
     pub status: Status,
     pub chain_id: ByteArray,
     pub proof_spec: Array<ProofSpec>,
-}
-
-pub impl StoreProofSpecArray of Store<Array<ProofSpec>> {
-    fn read(address_domain: u32, base: StorageBaseAddress) -> SyscallResult<Array<ProofSpec>> {
-        Self::read_at_offset(address_domain, base, 0)
-    }
-
-    fn write(
-        address_domain: u32, base: StorageBaseAddress, value: Array<ProofSpec>,
-    ) -> SyscallResult<()> {
-        Self::write_at_offset(address_domain, base, 0, value)
-    }
-
-    fn read_at_offset(
-        address_domain: u32, base: StorageBaseAddress, mut offset: u8,
-    ) -> SyscallResult<Array<ProofSpec>> {
-        let mut arr: Array<ProofSpec> = array![];
-
-        let len: u8 = Store::<u8>::read_at_offset(address_domain, base, offset)
-            .expect('Storage Span too large');
-        offset += 1;
-
-        let exit = Store::<ProofSpec>::size() * len + offset;
-        while offset < exit {
-            let value = Store::<ProofSpec>::read_at_offset(address_domain, base, offset).unwrap();
-            arr.append(value);
-            offset += Store::<ProofSpec>::size();
-        }
-
-        Result::Ok(arr)
-    }
-
-    fn write_at_offset(
-        address_domain: u32, base: StorageBaseAddress, mut offset: u8, mut value: Array<ProofSpec>,
-    ) -> SyscallResult<()> {
-        let len: u8 = value.len().try_into().expect('Storage - Span too large');
-        Store::<u8>::write_at_offset(address_domain, base, offset, len).unwrap();
-        offset += 1;
-
-        while let Option::Some(element) = value.pop_front() {
-            Store::<ProofSpec>::write_at_offset(address_domain, base, offset, element).unwrap();
-            offset += Store::<ProofSpec>::size();
-        }
-
-        Result::Ok(())
-    }
-
-    // FIXME: Use correct size
-    fn size() -> u8 {
-        10 * Store::<ProofSpec>::size()
-    }
+    pub upgrade_path: Array<ByteArray>,
 }
 
 #[generate_trait]
@@ -109,4 +62,31 @@ pub impl CometClientStateImpl of CometClientStateTrait {
 
         @substitute_client_state == self
     }
+
+    fn protobuf_bytes(self: CometClientState) -> ByteArray {
+        let proto_client_state: ProtoCometClientState = self.try_into().unwrap();
+        ProtoCodecImpl::encode(@proto_client_state)
+    }
+}
+
+pub impl CometClientStateToProto of TryInto<CometClientState, ProtoCometClientState> {
+    fn try_into(self: CometClientState) -> Option<ProtoCometClientState> {
+        let frozen_height: Height = self.status.into();
+
+        Some(
+            ProtoCometClientState {
+                chain_id: self.chain_id,
+                trust_level: self.trust_level,
+                trusting_period: self.trusting_period.try_into()?,
+                unbonding_period: self.unbonding_period.try_into()?,
+                max_clock_drift: self.max_clock_drift.try_into()?,
+                frozen_height: frozen_height.into(),
+                latest_height: self.latest_height.into(),
+                proof_specs: self.proof_spec,
+                upgrade_path: self.upgrade_path,
+                allow_update_after_expiry: false,
+                allow_update_after_misbehaviour: false,
+            },
+        )
+    }
 }

cairo-contracts/packages/clients/src/cometbft/component.cairo

@@ -6,7 +6,7 @@ pub mod CometClientComponent {
     use cometbft::verifier::{verify_misbehaviour_header, verify_update_header};
     use core::num::traits::Zero;
     use ics23::{
-        MerkleProof, Proof, array_u8_to_byte_array, byte_array_to_array_u8,
+        ArrayFelt252Store, MerkleProof, Proof, array_u8_to_byte_array, byte_array_to_array_u8,
         verify_membership as ics23_verify_membership,
         verify_non_membership as ics23_verify_non_membership,
     };
@@ -20,9 +20,8 @@ pub mod CometClientComponent {
     use starknet::{ContractAddress, get_block_number, get_block_timestamp, get_caller_address};
     use starknet_ibc_clients::cometbft::{
         ClientMessage, CometClientState, CometClientStateImpl, CometConsensusState,
-        CometConsensusStateImpl, CometConsensusStateStore, CometConsensusStateToStore,
-        CometConsensusStateZero, CometErrors, CometHeader, CometHeaderImpl,
-        CometHeaderIntoConsensusState, Misbehaviour, MisbehaviourImpl, StoreToCometConsensusState,
+        CometConsensusStateImpl, CometConsensusStateZero, CometErrors, CometHeader, CometHeaderImpl,
+        CometHeaderIntoConsensusState, Misbehaviour, MisbehaviourImpl,
     };
     use starknet_ibc_core::client::{
         CreateResponse, CreateResponseImpl, Height, HeightImpl, HeightPartialOrd, HeightZero,
@@ -32,15 +31,17 @@ pub mod CometClientComponent {
         UpdateResponse,
     };
     use starknet_ibc_core::commitment::{StateProof, StateRoot, StateValue};
-    use starknet_ibc_core::host::ClientIdImpl;
+    use starknet_ibc_core::host::{
+        BasePrefix, ClientIdImpl, client_upgrade_path, consensus_upgrade_path,
+    };
     use starknet_ibc_utils::ValidateBasic;
 
     #[storage]
     pub struct Storage {
         next_client_sequence: u64,
         update_heights: Map<u64, Array<Height>>,
         client_states: Map<u64, CometClientState>,
-        consensus_states: Map<(u64, Height), CometConsensusStateStore>,
+        consensus_states: Map<(u64, Height), CometConsensusState>,
         client_processed_times: Map<(u64, Height), u64>,
         client_processed_heights: Map<(u64, Height), u64>,
     }
@@ -83,7 +84,8 @@ pub mod CometClientComponent {
         }
 
         fn upgrade_client(ref self: ComponentState<TContractState>, msg: MsgUpgradeClient) {
-            self.assert_owner();
+            self.upgrade_validate(msg.clone());
+            self.upgrade_execute(msg);
         }
     }
 
@@ -190,8 +192,7 @@ pub mod CometClientComponent {
             let mut serialized: Array<felt252> = ArrayTrait::new();
 
             let consensus_state: CometConsensusState = self
-                .read_consensus_state(client_sequence, height)
-                .into();
+                .read_consensus_state(client_sequence, height);
 
             consensus_state.serialize(ref serialized);
 
@@ -363,9 +364,23 @@ pub mod CometClientComponent {
     > of UpgradeClientTrait<TContractState> {
         fn upgrade_validate(self: @ComponentState<TContractState>, msg: MsgUpgradeClient) {
             msg.validate_basic();
+
+            self
+                .verify_upgrade(
+                    msg.client_id.sequence,
+                    msg.upgraded_client_state,
+                    msg.upgraded_consensus_state,
+                    msg.proof_upgrade_client,
+                    msg.proof_upgrade_consensus,
+                );
         }
 
-        fn upgrade_execute(ref self: ComponentState<TContractState>, msg: MsgUpgradeClient) {}
+        fn upgrade_execute(ref self: ComponentState<TContractState>, msg: MsgUpgradeClient) {
+            self
+                .update_on_upgrade(
+                    msg.client_id.sequence, msg.upgraded_client_state, msg.upgraded_consensus_state,
+                );
+        }
     }
 
     // -----------------------------------------------------------
@@ -468,12 +483,82 @@ pub mod CometClientComponent {
 
         fn verify_upgrade(
             self: @ComponentState<TContractState>,
+            client_sequence: u64,
             upgrade_client_state: Array<felt252>,
             upgrade_consensus_state: Array<felt252>,
-            proof_upgrade_client: Array<felt252>,
-            proof_upgrade_consensus: Array<felt252>,
-            root: ByteArray,
-        ) {}
+            proof_upgrade_client: StateProof,
+            proof_upgrade_consensus: StateProof,
+        ) {
+            let comet_client_state: CometClientState = self.read_client_state(client_sequence);
+            let latest_height = comet_client_state.latest_height.clone();
+
+            let latest_consensus_state = self
+                .read_consensus_state(client_sequence, comet_client_state.latest_height.clone());
+
+            let root = latest_consensus_state.root.clone();
+
+            let upgrade_path = comet_client_state.upgrade_path.clone();
+
+            let status = self._status(comet_client_state, latest_consensus_state, client_sequence);
+
+            assert(status.is_active(), CometErrors::INACTIVE_CLIENT);
+
+            assert(
+                upgrade_path.len() == 1 || upgrade_path.len() == 2,
+                CometErrors::INVALID_UPGRADE_PATH_LENGTH,
+            );
+
+            let (prefix, upgrade_path) = if upgrade_path.len() == 1 {
+                ("", upgrade_path[0].clone())
+            } else {
+                (upgrade_path[0].clone(), upgrade_path[1].clone())
+            };
+
+            let base_prefix = BasePrefix { prefix };
+
+            let upgraded_client_path = client_upgrade_path(
+                base_prefix.clone(), latest_height.revision_height, upgrade_path.clone(),
+            );
+
+            let upgraded_consensus_path = consensus_upgrade_path(
+                base_prefix, latest_height.revision_height, upgrade_path,
+            );
+
+            let upgraded_client_state = CometClientStateImpl::deserialize(upgrade_client_state);
+
+            let upgraded_consensus_state = CometConsensusStateImpl::deserialize(
+                upgrade_consensus_state,
+            );
+
+            let upgraded_height = upgraded_client_state.latest_height.clone();
+
+            assert(upgraded_height > latest_height, CometErrors::INVALID_UPGRADE_HEIGHT);
+
+            let upgraded_client_protobuf = StateValue {
+                value: ics23::byte_array_to_array_u8(@upgraded_client_state.protobuf_bytes()),
+            };
+            let upgraded_consensus_protobuf = StateValue {
+                value: ics23::byte_array_to_array_u8(@upgraded_consensus_state.protobuf_bytes()),
+            };
+
+            self
+                .verify_membership(
+                    client_sequence,
+                    upgraded_client_path,
+                    upgraded_client_protobuf,
+                    proof_upgrade_client,
+                    root.clone(),
+                );
+
+            self
+                .verify_membership(
+                    client_sequence,
+                    upgraded_consensus_path,
+                    upgraded_consensus_protobuf,
+                    proof_upgrade_consensus,
+                    root,
+                );
+        }
     }
 
     #[embeddable_as(CometClientExecution)]
@@ -652,7 +737,32 @@ pub mod CometClientComponent {
             client_sequence: u64,
             new_client_state: Array<felt252>,
             new_consensus_state: Array<felt252>,
-        ) {}
+        ) {
+            let update_heights = self.read_update_heights(client_sequence);
+
+            if update_heights.len() > 0 {
+                let mut update_heights_span = update_heights.span();
+
+                while let Option::Some(height) = update_heights_span.pop_front() {
+                    self.remove_consensus_state(client_sequence, height.clone());
+                }
+
+                self.update_heights.write(client_sequence, array![]);
+            }
+
+            let new_client_state = CometClientStateImpl::deserialize(new_client_state);
+            let new_consensus_state = CometConsensusStateImpl::deserialize(new_consensus_state);
+
+            self
+                ._update_state(
+                    client_sequence,
+                    new_client_state.latest_height,
+                    new_client_state,
+                    new_consensus_state,
+                    get_block_number(),
+                    get_block_timestamp(),
+                );
+        }
     }
 
     // -----------------------------------------------------------
@@ -880,8 +990,7 @@ pub mod CometClientComponent {
         ) -> CometConsensusState {
             let consensus_state: CometConsensusState = self
                 .consensus_states
-                .read((client_sequence, height))
-                .into();
+                .read((client_sequence, height));
 
             assert(consensus_state.is_non_zero(), CometErrors::MISSING_CONSENSUS_STATE);
 
@@ -893,8 +1002,7 @@ pub mod CometClientComponent {
         ) -> bool {
             let consensus_state: CometConsensusState = self
                 .consensus_states
-                .read((client_sequence, height))
-                .into();
+                .read((client_sequence, height));
 
             consensus_state.is_non_zero()
         }
@@ -1000,7 +1108,7 @@ pub mod CometClientComponent {
             processed_height: u64,
             processed_time: u64,
         ) {
-            self.consensus_states.write((client_sequence, height.clone()), consensus_state.into());
+            self.consensus_states.write((client_sequence, height.clone()), consensus_state);
             self
                 .client_processed_heights
                 .write((client_sequence, height.clone()), processed_height);
@@ -1011,7 +1119,7 @@ pub mod CometClientComponent {
             ref self: ComponentState<TContractState>, client_sequence: u64, height: Height,
         ) {
             let consensus_zero = CometConsensusStateZero::zero();
-            self.consensus_states.write((client_sequence, height), consensus_zero.into());
+            self.consensus_states.write((client_sequence, height), consensus_zero);
             self.client_processed_times.write((client_sequence, height), 0);
             self.client_processed_heights.write((client_sequence, height), 0);
         }