feat: allow authorized relayers to update light client contract (#204)

* feat: allow only IBC core to perfrom client execution

* fix: typos

* feat: reject unathorized relayers

* test: add unit test allowed relayers

* fix: authorize contract deployer as an allowed relayer

* fix: set RELAYER as caller address of core in unit tests

cairo-contracts/packages/apps/src/transfer/components/transfer.cairo

@@ -45,7 +45,7 @@ pub mod TokenTransferComponent {
         RecvEvent: RecvEvent,
         AckEvent: AckEvent,
         AckStatusEvent: AckStatusEvent,
-        TimoutEvent: TimeoutEvent,
+        TimeoutEvent: TimeoutEvent,
         CreateTokenEvent: CreateTokenEvent,
     }
 

cairo-contracts/packages/apps/src/transfer/types.cairo

@@ -54,9 +54,9 @@ pub impl PacketDataJsonSerialize of Serialize<PacketData> {
 
 pub impl ArrayFelt252IntoPacketData of Into<Array<felt252>, PacketData> {
     fn into(self: Array<felt252>) -> PacketData {
-        let mut pakcet_data_span = self.span();
+        let mut packet_data_span = self.span();
 
-        let maybe_packet_data: Option<PacketData> = Serde::deserialize(ref pakcet_data_span);
+        let maybe_packet_data: Option<PacketData> = Serde::deserialize(ref packet_data_span);
 
         match maybe_packet_data {
             Option::Some(packet_data) => packet_data,

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

@@ -3,11 +3,13 @@ pub mod CometClientComponent {
     use alexandria_data_structures::array_ext::ArrayTraitExt;
     use alexandria_sorting::MergeSort;
     use core::num::traits::Zero;
+    use openzeppelin_access::ownable::OwnableComponent;
+    use openzeppelin_access::ownable::interface::IOwnable;
     use starknet::storage::{
         Map, StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess,
         StoragePointerWriteAccess,
     };
-    use starknet::{get_block_timestamp, get_block_number};
+    use starknet::{ContractAddress, get_block_timestamp, get_block_number, get_caller_address};
     use starknet_ibc_clients::cometbft::{
         CometClientState, CometClientStateImpl, CometConsensusState, CometConsensusStateImpl,
         CometHeader, CometHeaderImpl, CometErrors
@@ -42,11 +44,15 @@ pub mod CometClientComponent {
 
     #[embeddable_as(CometClientHandler)]
     impl ClientHandlerImpl<
-        TContractState, +HasComponent<TContractState>, +Drop<TContractState>
+        TContractState,
+        +HasComponent<TContractState>,
+        +Drop<TContractState>,
+        impl Ownable: OwnableComponent::HasComponent<TContractState>,
     > of IClientHandler<ComponentState<TContractState>> {
         fn create_client(
             ref self: ComponentState<TContractState>, msg: MsgCreateClient
         ) -> CreateResponse {
+            self.assert_owner();
             let client_sequence = self.read_next_client_sequence();
             self.create_validate(client_sequence, msg.clone());
             self.create_execute(client_sequence, msg)
@@ -55,13 +61,18 @@ pub mod CometClientComponent {
         fn update_client(
             ref self: ComponentState<TContractState>, msg: MsgUpdateClient
         ) -> UpdateResponse {
+            self.assert_owner();
             self.update_validate(msg.clone());
             self.update_execute(msg)
         }
 
-        fn recover_client(ref self: ComponentState<TContractState>, msg: MsgRecoverClient) {}
+        fn recover_client(ref self: ComponentState<TContractState>, msg: MsgRecoverClient) {
+            self.assert_owner();
+        }
 
-        fn upgrade_client(ref self: ComponentState<TContractState>, msg: MsgUpgradeClient) {}
+        fn upgrade_client(ref self: ComponentState<TContractState>, msg: MsgUpgradeClient) {
+            self.assert_owner();
+        }
     }
 
     // -----------------------------------------------------------
@@ -375,6 +386,26 @@ pub mod CometClientComponent {
         ) {}
     }
 
+    // -----------------------------------------------------------
+    // Client Owner
+    // -----------------------------------------------------------
+
+    #[generate_trait]
+    pub(crate) impl ClientOwnerImpl<
+        TContractState,
+        +HasComponent<TContractState>,
+        +Drop<TContractState>,
+        impl Ownable: OwnableComponent::HasComponent<TContractState>,
+    > of ClientOwnerTrait<TContractState> {
+        fn owner(self: @ComponentState<TContractState>) -> ContractAddress {
+            get_dep_component!(self, Ownable).owner()
+        }
+
+        fn assert_owner(self: @ComponentState<TContractState>) {
+            assert(self.owner() == get_caller_address(), CometErrors::INVALID_OWNER);
+        }
+    }
+
     // -----------------------------------------------------------
     // Client Internal
     // -----------------------------------------------------------

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

@@ -5,9 +5,11 @@ pub mod CometErrors {
     pub const INVALID_CONSENSUS_STATE: felt252 = 'ICS07: invalid consensus state';
     pub const INVALID_HEADER: felt252 = 'ICS07: invalid header';
     pub const INVALID_HEADER_TIMESTAMP: felt252 = 'ICS07: invalid header timestamp';
+    pub const INVALID_OWNER: felt252 = 'ICS07: invalid owner';
     pub const MISSING_CLIENT_STATE: felt252 = 'ICS07: missing client state';
     pub const MISSING_CONSENSUS_STATE: felt252 = 'ICS07: missing consensus state';
     pub const MISSING_CLIENT_PROCESSED_TIME: felt252 = 'ICS07: missing processed time';
     pub const MISSING_CLIENT_PROCESSED_HEIGHT: felt252 = 'ICS07: missing processed height';
     pub const ZERO_UPDATE_HEIGHTS: felt252 = 'ICS07: zero update heights';
+    pub const ZERO_OWNER: felt252 = 'ICS07: zero owner';
 }

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

@@ -52,10 +52,10 @@ fn test_update_client_ok() {
     let msg = cfg.dummy_msg_create_client();
     let create_resp = state.create_client(msg);
     let updating_height = cfg.latest_height.clone() + HEIGHT(1);
-    let updating_timestmap = cfg.latest_timestamp + 1;
+    let updating_timestamp = cfg.latest_timestamp + 1;
     let msg = cfg
         .dummy_msg_update_client(
-            create_resp.client_id, create_resp.height, updating_height.clone(), updating_timestmap
+            create_resp.client_id, create_resp.height, updating_height.clone(), updating_timestamp
         );
     state.update_client(msg);
     assert_eq!(state.client_type(), cfg.client_type);

cairo-contracts/packages/contracts/src/apps/transfer.cairo

@@ -26,9 +26,9 @@ pub mod TransferApp {
 
     // Transferrable
     #[abi(embed_v0)]
-    impl TokenTransferreableImpl =
+    impl TokenTransferrableImpl =
         TransferrableComponent::Transferrable<ContractState>;
-    impl TokenTransferreableInternal =
+    impl TokenTransferrableInternal =
         TransferrableComponent::TransferrableInternalImpl<ContractState>;
 
     // Token Transfer

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

@@ -1,11 +1,19 @@
 #[starknet::contract]
 pub mod CometClient {
-    use starknet_ibc_clients::cometbft::CometClientComponent;
+    use core::num::traits::Zero;
+    use openzeppelin_access::ownable::OwnableComponent;
+    use starknet::ContractAddress;
+    use starknet_ibc_clients::cometbft::{CometClientComponent, CometErrors};
     use starknet_ibc_utils::governance::IBCGovernanceComponent;
 
+    component!(path: OwnableComponent, storage: ownable, event: OwnableEvent);
     component!(path: IBCGovernanceComponent, storage: governance, event: IBCGovernanceEvent);
     component!(path: CometClientComponent, storage: client, event: CometClientEvent);
 
+    #[abi(embed_v0)]
+    impl OwnableMixinImpl = OwnableComponent::OwnableMixinImpl<ContractState>;
+    impl OwnableInternalImpl = OwnableComponent::InternalImpl<ContractState>;
+
     #[abi(embed_v0)]
     impl IBCGovernanceImpl = IBCGovernanceComponent::Governance<ContractState>;
     impl IBCGovernanceInternalImpl = IBCGovernanceComponent::GovernanceInternalImpl<ContractState>;
@@ -18,7 +26,7 @@ pub mod CometClient {
         CometClientComponent::CometClientQuery<ContractState>;
 
     // NOTE: The client state validation interface is exposed for public use.
-    // However, only the handler contract can invoke the execution methods.
+    // However, only the IBC core contract (owner) can invoke the execution methods.
 
     #[abi(embed_v0)]
     impl CometClientValidationImpl =
@@ -27,23 +35,29 @@ pub mod CometClient {
 
     #[storage]
     struct Storage {
+        #[substorage(v0)]
+        ownable: OwnableComponent::Storage,
         #[substorage(v0)]
         governance: IBCGovernanceComponent::Storage,
         #[substorage(v0)]
         client: CometClientComponent::Storage,
     }
 
     #[event]
-    #[derive(Debug, Drop, starknet::Event)]
+    #[derive(Drop, starknet::Event)]
     pub enum Event {
+        #[flat]
+        OwnableEvent: OwnableComponent::Event,
         #[flat]
         IBCGovernanceEvent: IBCGovernanceComponent::Event,
         #[flat]
         CometClientEvent: CometClientComponent::Event,
     }
 
     #[constructor]
-    fn constructor(ref self: ContractState) {
+    fn constructor(ref self: ContractState, owner: ContractAddress) {
+        assert(owner.is_non_zero(), CometErrors::ZERO_OWNER);
+        self.ownable.initializer(owner);
         self.governance.initializer();
     }
 }

cairo-contracts/packages/contracts/src/core.cairo

@@ -7,6 +7,17 @@ pub mod IBCCore {
     use starknet_ibc_core::connection::ConnectionEventEmitterComponent;
     use starknet_ibc_core::connection::ConnectionHandlerComponent;
     use starknet_ibc_core::router::RouterHandlerComponent;
+    use starknet_ibc_utils::governance::IBCGovernanceComponent;
+
+    // -----------------------------------------------------------
+    // Setup Governance Component
+    // -----------------------------------------------------------
+
+    component!(path: IBCGovernanceComponent, storage: governance, event: IBCGovernanceEvent);
+
+    #[abi(embed_v0)]
+    impl IBCGovernanceImpl = IBCGovernanceComponent::Governance<ContractState>;
+    impl IBCGovernanceInternalImpl = IBCGovernanceComponent::GovernanceInternalImpl<ContractState>;
 
     // -----------------------------------------------------------
     // Setup Client Components
@@ -23,6 +34,9 @@ pub mod IBCCore {
     #[abi(embed_v0)]
     impl CoreRegisterClientImpl =
         ClientHandlerComponent::CoreRegisterClient<ContractState>;
+    #[abi(embed_v0)]
+    impl CoreRegisterRelayerImpl =
+        ClientHandlerComponent::CoreRegisterRelayer<ContractState>;
     impl ClientInitializerImpl = ClientHandlerComponent::ClientInitializerImpl<ContractState>;
 
     // -----------------------------------------------------------
@@ -57,7 +71,7 @@ pub mod IBCCore {
     component!(path: ChannelHandlerComponent, storage: channel_handler, event: ChannelHandlerEvent);
 
     #[abi(embed_v0)]
-    impl CoreChannelHanderImpl =
+    impl CoreChannelHandlerImpl =
         ChannelHandlerComponent::CoreChannelHandler<ContractState>;
     #[abi(embed_v0)]
     impl CoreChannelQueryImpl =
@@ -70,13 +84,15 @@ pub mod IBCCore {
     component!(path: RouterHandlerComponent, storage: router_handler, event: RouterHandlerEvent);
 
     #[abi(embed_v0)]
-    impl CoreRouterHanderImpl =
+    impl CoreRouterHandlerImpl =
         RouterHandlerComponent::CoreRouterHandler<ContractState>;
     impl RouterInitializerImpl = RouterHandlerComponent::RouterInitializerImpl<ContractState>;
 
 
     #[storage]
     struct Storage {
+        #[substorage(v0)]
+        governance: IBCGovernanceComponent::Storage,
         #[substorage(v0)]
         client_emitter: ClientEventEmitterComponent::Storage,
         #[substorage(v0)]
@@ -96,6 +112,8 @@ pub mod IBCCore {
     #[event]
     #[derive(Debug, Drop, starknet::Event)]
     pub enum Event {
+        #[flat]
+        IBCGovernanceEvent: IBCGovernanceComponent::Event,
         #[flat]
         ClientEventEmitterEvent: ClientEventEmitterComponent::Event,
         #[flat]
@@ -114,6 +132,7 @@ pub mod IBCCore {
 
     #[constructor]
     fn constructor(ref self: ContractState) {
+        self.governance.initializer();
         self.client_handler.initializer();
         self.router_handler.initializer();
     }

cairo-contracts/packages/contracts/src/tests/channel.cairo

@@ -7,7 +7,7 @@ use starknet_ibc_testkit::configs::{
 };
 use starknet_ibc_testkit::dummies::{
     OWNER, HEIGHT, TIMESTAMP, COSMOS, STARKNET, CLIENT_ID, CONNECTION_ID, CHANNEL_ID, PORT_ID,
-    SUPPLY, PACKET_COMMITMENT_ON_SN,
+    SUPPLY, PACKET_COMMITMENT_ON_SN, RELAYER
 };
 use starknet_ibc_testkit::event_spy::{TransferEventSpyExt, ChannelEventSpyExt};
 use starknet_ibc_testkit::handles::{CoreHandle, AppHandle, ERC20Handle};
@@ -453,6 +453,8 @@ fn try_timeout_packet(timeout_height: Height, timeout_timestamp: Timestamp) {
     // Update Client
     // -----------------------------------------------------------
 
+    core.register_relayer(RELAYER());
+
     let msg = comet_cfg
         .dummy_msg_update_client(
             CLIENT_ID(), comet_cfg.latest_height, updating_height.clone(), updating_timestamp,

cairo-contracts/packages/contracts/src/tests/client.cairo

@@ -1,7 +1,7 @@
 use snforge_std::spy_events;
 use starknet_ibc_core::client::{UpdateResponse, StatusTrait, ClientContractTrait};
 use starknet_ibc_testkit::configs::CometClientConfigTrait;
-use starknet_ibc_testkit::dummies::HEIGHT;
+use starknet_ibc_testkit::dummies::{HEIGHT, RELAYER};
 use starknet_ibc_testkit::event_spy::ClientEventSpyExt;
 use starknet_ibc_testkit::handles::CoreHandle;
 use starknet_ibc_testkit::setup::SetupImpl;
@@ -58,6 +58,8 @@ fn test_update_comet_client_ok() {
     // Update Client
     // -----------------------------------------------------------
 
+    core.register_relayer(RELAYER());
+
     // Update the client to a new height and time.
     let updating_height = cfg.latest_height.clone() + HEIGHT(1);
     let updating_time = cfg.latest_timestamp.clone() + 1;

cairo-contracts/packages/contracts/src/tests/transfer.cairo

@@ -127,6 +127,6 @@ fn test_mint_burn_roundtrip() {
     // Check the balance of the `TransferApp` contract.
     erc20.assert_balance(ics20.address, 0);
 
-    // Chekck the total supply of the ERC20 contract.
+    // Check the total supply of the ERC20 contract.
     erc20.assert_total_supply(0);
 }

cairo-contracts/packages/core/src/channel/components/handler.cairo

@@ -646,7 +646,7 @@ pub mod ChannelHandlerComponent {
 
                     // If the packet sequence matches the expected next
                     // sequence, we check if the ack not exists. As the
-                    // existance means the packet was already relayed.
+                    // existence means the packet was already relayed.
                     if next_sequence_recv == msg.packet.seq_on_a {
                         let ack_exists = self
                             .packet_ack_exists(