From e34a193b13c168e6568e5a7bfb5fdfb2e9b2603a Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 8 Nov 2024 13:38:12 -0800 Subject: [PATCH] chore: remove channel initializer --- .../src/transfer/components/transfer.cairo | 7 +-- .../packages/apps/src/transfer/errors.cairo | 1 + .../packages/contracts/src/core.cairo | 2 - .../contracts/src/tests/channel.cairo | 26 ++++++--- .../contracts/src/tests/transfer.cairo | 20 ++++++- .../core/src/channel/components/handler.cairo | 54 ++----------------- .../packages/core/src/tests/channel.cairo | 21 ++++---- .../packages/testkit/src/configs/core.cairo | 29 ++++++++-- .../testkit/src/configs/transfer.cairo | 6 +-- .../packages/testkit/src/handles/core.cairo | 4 +- .../packages/testkit/src/mocks/channel.cairo | 6 +-- 11 files changed, 86 insertions(+), 90 deletions(-) diff --git a/cairo-contracts/packages/apps/src/transfer/components/transfer.cairo b/cairo-contracts/packages/apps/src/transfer/components/transfer.cairo index 6117515b..ab552290 100644 --- a/cairo-contracts/packages/apps/src/transfer/components/transfer.cairo +++ b/cairo-contracts/packages/apps/src/transfer/components/transfer.cairo @@ -194,6 +194,7 @@ pub mod TokenTransferComponent { version_on_a: AppVersion, ordering: ChannelOrdering ) -> AppVersion { + assert(version_on_a == VERSION(), TransferErrors::INVALID_APP_VERSION); VERSION() } @@ -202,7 +203,9 @@ pub mod TokenTransferComponent { port_id_on_a: PortId, chan_id_on_a: ChannelId, version_on_b: AppVersion - ) {} + ) { + assert(version_on_b == VERSION(), TransferErrors::INVALID_APP_VERSION); + } fn on_chan_open_confirm( ref self: ComponentState, port_id_on_b: PortId, chan_id_on_b: ChannelId @@ -432,8 +435,6 @@ pub mod TokenTransferComponent { match @packet_data.denom.base { Denom::Native(erc20_token) => { - packet_data.denom.remove_prefix(@trace_prefix); - self .unescrow_execute( receiver, diff --git a/cairo-contracts/packages/apps/src/transfer/errors.cairo b/cairo-contracts/packages/apps/src/transfer/errors.cairo index 250c3c05..8bc738e7 100644 --- a/cairo-contracts/packages/apps/src/transfer/errors.cairo +++ b/cairo-contracts/packages/apps/src/transfer/errors.cairo @@ -6,6 +6,7 @@ pub mod TransferErrors { pub const ZERO_AMOUNT: felt252 = 'ICS20: transfer amount is 0'; 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 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/contracts/src/core.cairo b/cairo-contracts/packages/contracts/src/core.cairo index 38c88412..d4b92527 100644 --- a/cairo-contracts/packages/contracts/src/core.cairo +++ b/cairo-contracts/packages/contracts/src/core.cairo @@ -40,7 +40,6 @@ pub mod IBCCore { #[abi(embed_v0)] impl CoreChannelQueryImpl = ChannelHandlerComponent::CoreChannelQuery; - impl ChannelInitializerImpl = ChannelHandlerComponent::ChannelInitializerImpl; // ----------------------------------------------------------- // Setup Router Components @@ -86,7 +85,6 @@ pub mod IBCCore { #[constructor] fn constructor(ref self: ContractState) { self.client_handler.initializer(); - self.channel_handler.initializer(); self.router_handler.initializer(); } } diff --git a/cairo-contracts/packages/contracts/src/tests/channel.cairo b/cairo-contracts/packages/contracts/src/tests/channel.cairo index 2afe1dca..07569a0f 100644 --- a/cairo-contracts/packages/contracts/src/tests/channel.cairo +++ b/cairo-contracts/packages/contracts/src/tests/channel.cairo @@ -172,7 +172,7 @@ fn test_chan_open_confirm_ok() { spy .assert_chan_open_confirm_event( - core.address, PORT_ID(), CHANNEL_ID(1), CONNECTION_ID(0), msg.clone() + core.address, PORT_ID(), CHANNEL_ID(0), CONNECTION_ID(0), msg.clone() ); let chan_end_on_b = core.channel_end(msg.port_id_on_b, msg.chan_id_on_b); @@ -186,7 +186,9 @@ fn test_send_packet_ok() { // Setup Essentials // ----------------------------------------------------------- - let (core, _, erc20, _, _, transfer_cfg, mut spy) = setup(); + let (core, _, erc20, mut core_cfg, _, transfer_cfg, mut spy) = setup(); + + core_cfg.create_channel(@core); // ----------------------------------------------------------- // Send Packet (from Starknet to Cosmos) @@ -225,7 +227,9 @@ fn test_recv_packet_ok() { // Setup Essentials // ----------------------------------------------------------- - let (core, ics20, _, _, _, transfer_cfg, mut spy) = setup(); + let (core, ics20, _, mut core_cfg, _, transfer_cfg, mut spy) = setup(); + + core_cfg.create_channel(@core); // ----------------------------------------------------------- // Receive Packet (from Cosmos to Starknet) @@ -284,7 +288,9 @@ fn test_successful_ack_packet_ok() { // Setup Essentials // ----------------------------------------------------------- - let (core, ics20, mut erc20, _, _, transfer_cfg, mut spy) = setup(); + let (core, ics20, mut erc20, mut core_cfg, _, transfer_cfg, mut spy) = setup(); + + core_cfg.create_channel(@core); // ----------------------------------------------------------- // Send Packet (from Starknet to Cosmos) @@ -363,7 +369,9 @@ fn test_failure_ack_packet_ok() { // Setup Essentials // ----------------------------------------------------------- - let (core, ics20, mut erc20, _, _, transfer_cfg, mut spy) = setup(); + let (core, ics20, mut erc20, mut core_cfg, _, transfer_cfg, mut spy) = setup(); + + core_cfg.create_channel(@core); // ----------------------------------------------------------- // Send Packet (from Starknet to Cosmos) @@ -444,7 +452,9 @@ fn test_ack_packet_for_never_sent_packet() { // Setup Essentials // ----------------------------------------------------------- - let (core, _, _, _, _, transfer_cfg, _) = setup(); + let (core, _, _, mut core_cfg, _, transfer_cfg, _) = setup(); + + core_cfg.create_channel(@core); // ----------------------------------------------------------- // Acknowledge Packet (on Starknet) @@ -463,7 +473,7 @@ fn try_timeout_packet(timeout_height: Height, timeout_timestamp: Timestamp) { // Setup Essentials // ----------------------------------------------------------- - let (core, ics20, mut erc20, _, comet_cfg, mut transfer_cfg, mut spy) = setup(); + let (core, ics20, mut erc20, mut core_cfg, comet_cfg, mut transfer_cfg, mut spy) = setup(); let updating_height = HEIGHT(11); // Set to 11 as client is created at height 10. @@ -473,6 +483,8 @@ fn try_timeout_packet(timeout_height: Height, timeout_timestamp: Timestamp) { transfer_cfg.set_timeout_timestamp(timeout_timestamp); + core_cfg.create_channel(@core); + // ----------------------------------------------------------- // Send Packet (from Starknet to Cosmos) // ----------------------------------------------------------- diff --git a/cairo-contracts/packages/contracts/src/tests/transfer.cairo b/cairo-contracts/packages/contracts/src/tests/transfer.cairo index 43faec9f..b5403905 100644 --- a/cairo-contracts/packages/contracts/src/tests/transfer.cairo +++ b/cairo-contracts/packages/contracts/src/tests/transfer.cairo @@ -1,7 +1,9 @@ use openzeppelin_testing::events::EventSpyExt; use snforge_std::spy_events; use starknet_ibc_apps::transfer::ERC20Contract; -use starknet_ibc_testkit::configs::{TransferAppConfigTrait, CometClientConfigTrait}; +use starknet_ibc_testkit::configs::{ + CoreConfigTrait, TransferAppConfigTrait, CometClientConfigTrait +}; use starknet_ibc_testkit::dummies::{NAME, SYMBOL, SUPPLY, OWNER, COSMOS, STARKNET}; use starknet_ibc_testkit::event_spy::TransferEventSpyExt; use starknet_ibc_testkit::handles::{AppHandle, CoreHandle, ERC20Handle}; @@ -14,6 +16,8 @@ fn test_escrow_unescrow_roundtrip() { // Setup Essentials // ----------------------------------------------------------- + let mut core_cfg = CoreConfigTrait::default(); + let mut comet_cfg = CometClientConfigTrait::default(); let mut transfer_cfg = TransferAppConfigTrait::default(); @@ -34,6 +38,12 @@ fn test_escrow_unescrow_roundtrip() { // Submit the message and create a client. core.create_client(msg_create_client); + // ----------------------------------------------------------- + // Create Channel + // ----------------------------------------------------------- + + core_cfg.create_channel(@core); + // ----------------------------------------------------------- // Escrow // ----------------------------------------------------------- @@ -94,6 +104,8 @@ fn test_mint_burn_roundtrip() { // Setup Essentials // ----------------------------------------------------------- + let mut core_cfg = CoreConfigTrait::default(); + let mut comet_cfg = CometClientConfigTrait::default(); let mut transfer_cfg = TransferAppConfigTrait::default(); @@ -112,6 +124,12 @@ fn test_mint_burn_roundtrip() { // Submit the message and create a client. core.create_client(msg_create_client); + // ----------------------------------------------------------- + // Create Channel + // ----------------------------------------------------------- + + core_cfg.create_channel(@core); + // ----------------------------------------------------------- // Mint // ----------------------------------------------------------- diff --git a/cairo-contracts/packages/core/src/channel/components/handler.cairo b/cairo-contracts/packages/core/src/channel/components/handler.cairo index c5357cbe..058ff8f5 100644 --- a/cairo-contracts/packages/core/src/channel/components/handler.cairo +++ b/cairo-contracts/packages/core/src/channel/components/handler.cairo @@ -1,7 +1,4 @@ -use starknet_ibc_core::channel::{ - ChannelEnd, ChannelState, ChannelOrdering, Counterparty, AppVersion -}; -use starknet_ibc_core::host::{ClientId, PortId, ChannelId, SequencePartialOrd, SequenceZero}; +use starknet_ibc_core::host::{SequencePartialOrd, SequenceZero}; #[starknet::component] pub mod ChannelHandlerComponent { @@ -36,7 +33,6 @@ pub mod ChannelHandlerComponent { }; use starknet_ibc_core::router::{RouterHandlerComponent, AppContractTrait, AppContract}; use starknet_ibc_utils::ValidateBasic; - use super::{PORT_ID, CHANNEL_ID, CHANNEL_END}; #[storage] pub struct Storage { @@ -54,26 +50,6 @@ pub mod ChannelHandlerComponent { #[derive(Debug, Drop, starknet::Event)] pub enum Event {} - // ----------------------------------------------------------- - // Channel Initializer - // ----------------------------------------------------------- - - #[generate_trait] - pub impl ChannelInitializerImpl< - TContractState, +HasComponent, +Drop - > of ChannelInitializerTrait { - fn initializer(ref self: ComponentState) { - // TODO: Initialize a temporary dummy `ChannelEnd`s for testing the - // handlers. This should be removed once the channel handshake is - // implemented. - self.write_channel_end(@PORT_ID(), @CHANNEL_ID(0), CHANNEL_END(1)); - self.write_next_sequence_recv(@PORT_ID(), @CHANNEL_ID(0), SequenceZero::zero()); - - self.write_channel_end(@PORT_ID(), @CHANNEL_ID(1), CHANNEL_END(0)); - self.write_next_sequence_send(@PORT_ID(), @CHANNEL_ID(1), SequenceZero::zero()); - } - } - // ----------------------------------------------------------- // IChannelHandler // ----------------------------------------------------------- @@ -304,6 +280,8 @@ pub mod ChannelHandlerComponent { self.write_channel_end(@msg.port_id_on_b, @chan_id_on_b, chan_end_on_b); + self.write_next_channel_sequence(channel_sequence + 1); + self.write_next_sequence_send(@msg.port_id_on_b, @chan_id_on_b, SequenceImpl::one()); self.write_next_sequence_recv(@msg.port_id_on_b, @chan_id_on_b, SequenceImpl::one()); @@ -1034,7 +1012,7 @@ pub mod ChannelHandlerComponent { fn write_next_channel_sequence( ref self: ComponentState, channel_sequence: u64 ) { - self.next_channel_sequence.write(channel_sequence + 1); + self.next_channel_sequence.write(channel_sequence); } fn write_channel_end( @@ -1239,27 +1217,3 @@ pub mod ChannelHandlerComponent { } } -// ----------------- Temporary until handshakes are implemented --------------- -pub(crate) fn CLIENT_ID() -> ClientId { - ClientId { client_type: '07-cometbft', sequence: 0 } -} - -pub(crate) fn PORT_ID() -> PortId { - PortId { port_id: "transfer" } -} - -pub(crate) fn CHANNEL_ID(sequence: u64) -> ChannelId { - ChannelId { channel_id: format!("channel-{sequence}") } -} - -pub(crate) fn CHANNEL_END(counterparty_channel_sequence: u64) -> ChannelEnd { - ChannelEnd { - state: ChannelState::Open, - ordering: ChannelOrdering::Unordered, - remote: Counterparty { - port_id: PORT_ID(), channel_id: CHANNEL_ID(counterparty_channel_sequence), - }, - client_id: CLIENT_ID(), - version: AppVersion { version: "" }, - } -} diff --git a/cairo-contracts/packages/core/src/tests/channel.cairo b/cairo-contracts/packages/core/src/tests/channel.cairo index 48e1b463..ec22c192 100644 --- a/cairo-contracts/packages/core/src/tests/channel.cairo +++ b/cairo-contracts/packages/core/src/tests/channel.cairo @@ -1,11 +1,7 @@ use ChannelHandlerComponent::ChannelReaderTrait; use core::num::traits::Zero; -use starknet_ibc_core::channel::ChannelHandlerComponent::{ - ChannelInitializerImpl, ChannelWriterTrait -}; -use starknet_ibc_core::channel::{ - ChannelHandlerComponent, ChannelState, ChannelOrdering, Receipt, ReceiptTrait -}; +use starknet_ibc_core::channel::ChannelHandlerComponent::ChannelWriterTrait; +use starknet_ibc_core::channel::{ChannelHandlerComponent, Receipt, ReceiptTrait}; use starknet_ibc_testkit::dummies::{CHANNEL_END, CHANNEL_ID, PORT_ID, SEQUENCE}; use starknet_ibc_testkit::mocks::MockChannelHandler; @@ -17,22 +13,23 @@ fn COMPONENT_STATE() -> ComponentState { fn setup() -> ComponentState { let mut state = COMPONENT_STATE(); - state.initializer(); state } #[test] fn test_intial_state() { let state = setup(); - let channel_end = state.read_channel_end(@PORT_ID(), @CHANNEL_ID(0)); - assert_eq!(channel_end.state, ChannelState::Open); - assert_eq!(channel_end.ordering, ChannelOrdering::Unordered); + let next_channel_sequence = state.read_next_channel_sequence(); + assert!(next_channel_sequence.is_zero()); + + let next_sequence_send = state.read_next_sequence_send(@PORT_ID(), @CHANNEL_ID(0)); + assert!(next_sequence_send.is_zero()); let next_sequence_recv = state.read_next_sequence_recv(@PORT_ID(), @CHANNEL_ID(0)); assert!(next_sequence_recv.is_zero()); - let next_sequence_send = state.read_next_sequence_send(@PORT_ID(), @CHANNEL_ID(1)); - assert!(next_sequence_send.is_zero()); + let next_sequence_ack = state.read_next_sequence_ack(@PORT_ID(), @CHANNEL_ID(0)); + assert!(next_sequence_ack.is_zero()); } #[test] diff --git a/cairo-contracts/packages/testkit/src/configs/core.cairo b/cairo-contracts/packages/testkit/src/configs/core.cairo index 8a017947..92d9f66f 100644 --- a/cairo-contracts/packages/testkit/src/configs/core.cairo +++ b/cairo-contracts/packages/testkit/src/configs/core.cairo @@ -5,16 +5,27 @@ use starknet_ibc_core::channel::{ use starknet_ibc_testkit::dummies::{ HEIGHT, CONNECTION_ID, CHANNEL_ID, PORT_ID, VERSION_PROPOSAL, STATE_PROOF }; +use starknet_ibc_testkit::handles::{CoreContract, CoreHandle}; #[derive(Clone, Debug, Drop, Serde)] pub struct CoreConfig { + chan_sequence_on_a: u64, + chan_sequence_on_b: u64, channel_ordering: ChannelOrdering, } #[generate_trait] pub impl CoreConfigImpl of CoreConfigTrait { fn default() -> CoreConfig { - CoreConfig { channel_ordering: ChannelOrdering::Unordered, } + CoreConfig { + chan_sequence_on_a: 0, + chan_sequence_on_b: 0, + channel_ordering: ChannelOrdering::Unordered + } + } + + fn set_chan_sequence_on_b(ref self: CoreConfig, sequence: u64) { + self.chan_sequence_on_b = sequence; } fn dummy_msg_chan_open_init(self: @CoreConfig) -> MsgChanOpenInit { @@ -32,7 +43,7 @@ pub impl CoreConfigImpl of CoreConfigTrait { port_id_on_b: PORT_ID(), conn_id_on_b: CONNECTION_ID(0), port_id_on_a: PORT_ID(), - chan_id_on_a: CHANNEL_ID(1), + chan_id_on_a: CHANNEL_ID(*self.chan_sequence_on_b), version_on_a: VERSION(), proof_chan_end_on_a: STATE_PROOF(), proof_height_on_a: HEIGHT(10), @@ -43,8 +54,8 @@ pub impl CoreConfigImpl of CoreConfigTrait { fn dummy_msg_chan_open_ack(self: @CoreConfig) -> MsgChanOpenAck { MsgChanOpenAck { port_id_on_a: PORT_ID(), - chan_id_on_a: CHANNEL_ID(0), - chan_id_on_b: CHANNEL_ID(1), + chan_id_on_a: CHANNEL_ID(*self.chan_sequence_on_a), + chan_id_on_b: CHANNEL_ID(*self.chan_sequence_on_b), version_on_b: VERSION(), proof_chan_end_on_b: STATE_PROOF(), proof_height_on_b: HEIGHT(10), @@ -54,9 +65,17 @@ pub impl CoreConfigImpl of CoreConfigTrait { fn dummy_msg_chan_open_confirm(self: @CoreConfig) -> MsgChanOpenConfirm { MsgChanOpenConfirm { port_id_on_b: PORT_ID(), - chan_id_on_b: CHANNEL_ID(0), + chan_id_on_b: CHANNEL_ID(*self.chan_sequence_on_b), proof_chan_end_on_a: STATE_PROOF(), proof_height_on_a: HEIGHT(10), } } + + fn create_channel(ref self: CoreConfig, core: @CoreContract) { + core.chan_open_init(self.dummy_msg_chan_open_init()); + + core.chan_open_ack(self.dummy_msg_chan_open_ack()); + + self.chan_sequence_on_a += 1; + } } diff --git a/cairo-contracts/packages/testkit/src/configs/transfer.cairo b/cairo-contracts/packages/testkit/src/configs/transfer.cairo index 31d6dac6..de668f12 100644 --- a/cairo-contracts/packages/testkit/src/configs/transfer.cairo +++ b/cairo-contracts/packages/testkit/src/configs/transfer.cairo @@ -29,7 +29,7 @@ pub impl TransferAppConfigImpl of TransferAppConfigTrait { TransferAppConfig { native_denom: NATIVE_DENOM(), hosted_denom: HOSTED_DENOM(), - chan_id_on_a: CHANNEL_ID(1), + chan_id_on_a: CHANNEL_ID(0), chan_id_on_b: CHANNEL_ID(0), amount: AMOUNT, timeout_height: TIMEOUT_HEIGHT(1000), @@ -119,7 +119,7 @@ pub impl TransferAppConfigImpl of TransferAppConfigTrait { ) -> MsgTimeoutPacket { MsgTimeoutPacket { packet: self.dummy_packet(denom, sender, receiver), - next_seq_recv_on_b: Sequence { sequence: 1 }, + next_seq_recv_on_b: Sequence { sequence: 2 }, proof_unreceived_on_b: STATE_PROOF(), proof_height_on_b: proof_height, } @@ -132,7 +132,7 @@ pub impl TransferAppConfigImpl of TransferAppConfigTrait { Serde::serialize(@self.dummy_packet_data(denom, sender, receiver), ref serialized_data); Packet { - seq_on_a: Sequence { sequence: 0 }, + seq_on_a: Sequence { sequence: 1 }, port_id_on_a: PORT_ID(), chan_id_on_a: self.chan_id_on_a.clone(), port_id_on_b: PORT_ID(), diff --git a/cairo-contracts/packages/testkit/src/handles/core.cairo b/cairo-contracts/packages/testkit/src/handles/core.cairo index 8bc30686..657fa57a 100644 --- a/cairo-contracts/packages/testkit/src/handles/core.cairo +++ b/cairo-contracts/packages/testkit/src/handles/core.cairo @@ -2,8 +2,8 @@ use openzeppelin_testing::declare_and_deploy; use starknet::ContractAddress; use starknet_ibc_core::channel::{ IChannelHandlerDispatcher, IChannelHandlerDispatcherTrait, MsgChanOpenInit, MsgChanOpenTry, - MsgChanOpenAck, MsgChanOpenConfirm, MsgRecvPacket, MsgAckPacket, MsgTimeoutPacket, IChannelQueryDispatcher, - IChannelQueryDispatcherTrait, ChannelEnd, Packet, + MsgChanOpenAck, MsgChanOpenConfirm, MsgRecvPacket, MsgAckPacket, MsgTimeoutPacket, + IChannelQueryDispatcher, IChannelQueryDispatcherTrait, ChannelEnd, Packet, }; use starknet_ibc_core::client::{ IClientHandlerDispatcher, IClientHandlerDispatcherTrait, IRegisterClientDispatcher, diff --git a/cairo-contracts/packages/testkit/src/mocks/channel.cairo b/cairo-contracts/packages/testkit/src/mocks/channel.cairo index 1ec0527d..26b6a030 100644 --- a/cairo-contracts/packages/testkit/src/mocks/channel.cairo +++ b/cairo-contracts/packages/testkit/src/mocks/channel.cairo @@ -9,8 +9,6 @@ pub mod MockChannelHandler { ); component!(path: ChannelHandlerComponent, storage: channel_handler, event: ChannelHandlerEvent); - impl ChannelInitializerImpl = ChannelHandlerComponent::ChannelInitializerImpl; - #[storage] struct Storage { #[substorage(v0)] @@ -29,7 +27,5 @@ pub mod MockChannelHandler { } #[constructor] - fn constructor(ref self: ContractState) { - self.channel_handler.initializer(); - } + fn constructor(ref self: ContractState) {} }