imp: add checks for on_chan_open_init + use PortId type in IRouter

informalsystems · Nov 9, 2024 · b9ad97f · b9ad97f
1 parent e34a193
commit b9ad97f
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 23 deletions.
diff --git a/cairo-contracts/packages/apps/src/lib.cairo b/cairo-contracts/packages/apps/src/lib.cairo
@@ -27,8 +27,8 @@ pub mod transfer {
     pub const TRANSFER_PORT_ID_HASH: felt252 =
         506076466176013583354797631368330115868609515147080483618120063858966368900;
 
-    pub fn TRANSFER_PORT_ID() -> ByteArray {
-        "transfer"
+    pub fn TRANSFER_PORT_ID() -> starknet_ibc_core::host::PortId {
+        starknet_ibc_core::host::PortId { port_id: "transfer" }
     }
 
     pub fn VERSION() -> starknet_ibc_core::channel::AppVersion {

diff --git a/cairo-contracts/packages/apps/src/transfer/components/transfer.cairo b/cairo-contracts/packages/apps/src/transfer/components/transfer.cairo
@@ -20,7 +20,7 @@ pub mod TokenTransferComponent {
     };
     use starknet_ibc_apps::transfer::{
         ITransferrable, ISendTransfer, ITransferQuery, ERC20Contract, ERC20ContractTrait,
-        TransferErrors, VERSION, SUCCESS_ACK
+        TransferErrors, TRANSFER_PORT_ID, VERSION, SUCCESS_ACK
     };
     use starknet_ibc_core::channel::{
         Packet, Acknowledgement, AckStatus, AckStatusImpl, IAppCallback, ChannelContract,
@@ -182,6 +182,14 @@ pub mod TokenTransferComponent {
             version_proposal: AppVersion,
             ordering: ChannelOrdering
         ) -> AppVersion {
+            assert(port_id_on_a == TRANSFER_PORT_ID(), TransferErrors::INVALID_PORT_ID);
+
+            if version_proposal.is_non_zero() {
+                assert(version_proposal == VERSION(), TransferErrors::INVALID_APP_VERSION);
+            }
+
+            assert(ordering == ChannelOrdering::Unordered, TransferErrors::UNSUPPORTED_ORDERING);
+
             VERSION()
         }
 

diff --git a/cairo-contracts/packages/apps/src/transfer/errors.cairo b/cairo-contracts/packages/apps/src/transfer/errors.cairo
@@ -7,6 +7,8 @@ pub mod TransferErrors {
     pub const ZERO_SALT: felt252 = 'ICS20: salt is 0';
     pub const ZERO_TOKEN_ADDRESS: felt252 = 'ICS20: missing token address';
     pub const INVALID_APP_VERSION: felt252 = 'ICS20: invalid app version';
+    pub const UNSUPPORTED_ORDERING: felt252 = 'ICS20: unsupported ordering';
+    pub const INVALID_PORT_ID: felt252 = 'ICS20: invalid port ID';
     pub const INVALID_DENOM: felt252 = 'ICS20: invalid denom';
     pub const INVALID_PACKET_DATA: felt252 = 'ICS20: invalid packet data';
     pub const INVALID_OWNER: felt252 = 'ICS20: invalid owner';

diff --git a/cairo-contracts/packages/core/src/router/component.cairo b/cairo-contracts/packages/core/src/router/component.cairo
@@ -35,22 +35,18 @@ pub mod RouterHandlerComponent {
     impl CoreRouterHandlerImpl<
         TContractState, +HasComponent<TContractState>, +Drop<TContractState>
     > of IRouter<ComponentState<TContractState>> {
-        fn app_address(
-            self: @ComponentState<TContractState>, port_id: ByteArray
-        ) -> ContractAddress {
-            self.read_app_address(@PortIdImpl::new(port_id))
+        fn app_address(self: @ComponentState<TContractState>, port_id: PortId) -> ContractAddress {
+            self.read_app_address(@port_id)
         }
 
         fn bind_port_id(
-            ref self: ComponentState<TContractState>,
-            port_id: ByteArray,
-            app_address: ContractAddress
+            ref self: ComponentState<TContractState>, port_id: PortId, app_address: ContractAddress
         ) {
-            self.write_app_address(PortIdImpl::new(port_id), app_address)
+            self.write_app_address(port_id, app_address)
         }
 
-        fn release_port_id(ref self: ComponentState<TContractState>, port_id: ByteArray) {
-            self.remove_app_address(PortIdImpl::new(port_id))
+        fn release_port_id(ref self: ComponentState<TContractState>, port_id: PortId) {
+            self.remove_app_address(port_id)
         }
     }
 

diff --git a/cairo-contracts/packages/core/src/router/interface.cairo b/cairo-contracts/packages/core/src/router/interface.cairo
@@ -1,10 +1,11 @@
 use starknet::ContractAddress;
+use starknet_ibc_core::host::PortId;
 
 #[starknet::interface]
 pub trait IRouter<TContractState> {
-    fn app_address(self: @TContractState, port_id: ByteArray) -> ContractAddress;
+    fn app_address(self: @TContractState, port_id: PortId) -> ContractAddress;
 
-    fn bind_port_id(ref self: TContractState, port_id: ByteArray, app_address: ContractAddress);
+    fn bind_port_id(ref self: TContractState, port_id: PortId, app_address: ContractAddress);
 
-    fn release_port_id(ref self: TContractState, port_id: ByteArray);
+    fn release_port_id(ref self: TContractState, port_id: PortId);
 }
diff --git a/cairo-contracts/packages/core/src/tests/router.cairo b/cairo-contracts/packages/core/src/tests/router.cairo
@@ -1,6 +1,7 @@
 use starknet::contract_address_const;
 use starknet_ibc_core::router::RouterHandlerComponent::{RouterInitializerImpl, CoreRouterHandler};
 use starknet_ibc_core::router::RouterHandlerComponent;
+use starknet_ibc_testkit::dummies::PORT_ID;
 use starknet_ibc_testkit::mocks::MockRouterHandler;
 
 type ComponentState = RouterHandlerComponent::ComponentState<MockRouterHandler::ContractState>;
@@ -19,20 +20,19 @@ fn setup() -> ComponentState {
 #[should_panic(expected: 'ICS26: unsupported port id')]
 fn test_missing_app_address() {
     let mut state = setup();
-    state.app_address("transfer");
+    state.app_address(PORT_ID());
 }
 
 #[test]
 #[should_panic(expected: 'ICS26: unsupported port id')]
 fn test_bind_release_port_id_ok() {
     let mut state = setup();
-    let port_id = "transfer";
     let app_address = contract_address_const::<'transfer'>();
 
-    state.bind_port_id(port_id.clone(), app_address);
-    let stored_app_address = state.app_address(port_id.clone());
+    state.bind_port_id(PORT_ID(), app_address);
+    let stored_app_address = state.app_address(PORT_ID());
     assert_eq!(stored_app_address, app_address);
 
-    state.release_port_id(port_id.clone());
-    state.app_address(port_id);
+    state.release_port_id(PORT_ID());
+    state.app_address(PORT_ID());
 }
diff --git a/cairo-contracts/packages/testkit/src/handles/core.cairo b/cairo-contracts/packages/testkit/src/handles/core.cairo
@@ -61,7 +61,7 @@ pub impl CoreHandleImpl of CoreHandle {
         self.register_client_dispatcher().register_client(client_type, client_address)
     }
 
-    fn register_app(self: @CoreContract, port_id: ByteArray, app_address: ContractAddress) {
+    fn register_app(self: @CoreContract, port_id: PortId , app_address: ContractAddress) {
         self.router_dispatcher().bind_port_id(port_id, app_address)
     }