From e5e809844f978acdae7902c4c1062c1f89a79feb Mon Sep 17 00:00:00 2001 From: srdtrk Date: Sat, 3 Feb 2024 19:38:15 +0100 Subject: [PATCH] imp: added CloseChannel --- e2e/interchaintest/contract_test.go | 59 ++++--------------- e2e/interchaintest/testsuite/constants.go | 11 ++++ e2e/interchaintest/testsuite/suite.go | 4 +- e2e/interchaintest/types/icacontroller/msg.go | 1 + src/contract.rs | 29 ++++++++- src/ibc/handshake.rs | 43 +++++++++++++- src/types/error.rs | 6 ++ src/types/msg.rs | 2 + src/types/state.rs | 18 ++++++ 9 files changed, 119 insertions(+), 54 deletions(-) create mode 100644 e2e/interchaintest/testsuite/constants.go diff --git a/e2e/interchaintest/contract_test.go b/e2e/interchaintest/contract_test.go index f1450892..232d8597 100644 --- a/e2e/interchaintest/contract_test.go +++ b/e2e/interchaintest/contract_test.go @@ -127,7 +127,7 @@ func (s *ContractTestSuite) IcaContractChannelHandshakeTest_WithEncodingAndOrder // This starts the chains, relayer, creates the user accounts, creates the ibc clients and connections, // sets up the contract and does the channel handshake for the contract test suite. - s.SetupContractTestSuite(ctx, icatypes.EncodingProto3JSON, ordering) + s.SetupContractTestSuite(ctx, encoding, ordering) wasmd, simd := s.ChainA, s.ChainB s.Run("TestChannelHandshakeSuccess", func() { @@ -1056,51 +1056,18 @@ func (s *ContractTestSuite) TestMigrateOrderedToUnordered() { // Fund the ICA address: s.FundAddressChainB(ctx, s.Contract.IcaAddress) - contractState, err := types.QueryAnyMsg[icacontroller.ContractState]( - ctx, &s.Contract.Contract, - icacontroller.GetContractStateRequest, - ) - s.Require().NoError(err) - var simdChannelsLen int - s.Run("TestTimeout", func() { - // We will send a message to the host that will timeout after 3 seconds. - // You cannot use 0 seconds because block timestamp will be greater than the timeout timestamp which is not allowed. - // Host will not be able to respond to this message in time. - - // Stop the relayer so that the host cannot respond to the message: - err := s.Relayer.StopRelayer(ctx, s.ExecRep) - s.Require().NoError(err) - - time.Sleep(5 * time.Second) - - timeout := uint64(3) - // Execute the contract: - sendCustomIcaMsg := icacontroller.NewExecuteMsg_SendCustomIcaMessages_FromProto( - simd.Config().EncodingConfig.Codec, []proto.Message{}, - icatypes.EncodingProto3JSON, nil, &timeout, - ) - err = s.Contract.Execute(ctx, wasmdUser.KeyName(), sendCustomIcaMsg) + s.Run("TestCloseChannel", func() { + // Close the channel: + closeChannelMsg := icacontroller.ExecuteMsg{ + CloseChannel: &struct{}{}, + } + err := s.Contract.Execute(ctx, wasmdUser.KeyName(), closeChannelMsg) s.Require().NoError(err) - // Wait until timeout: err = testutil.WaitForBlocks(ctx, 5, wasmd, simd) s.Require().NoError(err) - err = s.Relayer.StartRelayer(ctx, s.ExecRep) - s.Require().NoError(err) - - // Wait until timeout packet is received: - err = testutil.WaitForBlocks(ctx, 2, wasmd, simd) - s.Require().NoError(err) - - // Flush to make sure the channel is closed in simd: - err = s.Relayer.Flush(ctx, s.ExecRep, s.PathName, contractState.IcaInfo.ChannelID) - s.Require().NoError(err) - - err = testutil.WaitForBlocks(ctx, 2, wasmd, simd) - s.Require().NoError(err) - // Check if channel was closed: wasmdChannels, err := s.Relayer.GetChannels(ctx, s.ExecRep, wasmd.Config().ChainID) s.Require().NoError(err) @@ -1119,7 +1086,7 @@ func (s *ContractTestSuite) TestMigrateOrderedToUnordered() { s.Require().NoError(err) s.Require().Equal(uint64(0), callbackCounter.Success) s.Require().Equal(uint64(0), callbackCounter.Error) - s.Require().Equal(uint64(1), callbackCounter.Timeout) + s.Require().Equal(uint64(0), callbackCounter.Timeout) // Check if contract channel state was updated: contractChannelState, err := types.QueryAnyMsg[icacontroller.ContractChannelState](ctx, &s.Contract.Contract, icacontroller.GetChannelRequest) @@ -1150,7 +1117,7 @@ func (s *ContractTestSuite) TestMigrateOrderedToUnordered() { s.Require().NoError(err) // Wait for the channel to get set up - err = testutil.WaitForBlocks(ctx, 10, s.ChainA, s.ChainB) + err = testutil.WaitForBlocks(ctx, 5, s.ChainA, s.ChainB) s.Require().NoError(err) // Check if a new channel was opened in simd @@ -1194,7 +1161,7 @@ func (s *ContractTestSuite) TestMigrateOrderedToUnordered() { s.Require().Equal(uint64(0), callbackCounter.Success) s.Require().Equal(uint64(0), callbackCounter.Error) - s.Require().Equal(uint64(1), callbackCounter.Timeout) + s.Require().Equal(uint64(0), callbackCounter.Timeout) }) s.Run("TestSendCustomIcaMessagesAfterReopen", func() { @@ -1211,10 +1178,10 @@ func (s *ContractTestSuite) TestMigrateOrderedToUnordered() { []proto.Message{sendMsg}, icatypes.EncodingProtobuf, nil, nil, ) - err = s.Contract.Execute(ctx, wasmdUser.KeyName(), sendCustomIcaMsg) + err := s.Contract.Execute(ctx, wasmdUser.KeyName(), sendCustomIcaMsg) s.Require().NoError(err) - err = testutil.WaitForBlocks(ctx, 10, wasmd, simd) + err = testutil.WaitForBlocks(ctx, 5, wasmd, simd) s.Require().NoError(err) icaBalance, err := simd.GetBalance(ctx, s.Contract.IcaAddress, simd.Config().Denom) @@ -1227,7 +1194,7 @@ func (s *ContractTestSuite) TestMigrateOrderedToUnordered() { s.Require().Equal(uint64(1), callbackCounter.Success) s.Require().Equal(uint64(0), callbackCounter.Error) - s.Require().Equal(uint64(1), callbackCounter.Timeout) + s.Require().Equal(uint64(0), callbackCounter.Timeout) }) } diff --git a/e2e/interchaintest/testsuite/constants.go b/e2e/interchaintest/testsuite/constants.go new file mode 100644 index 00000000..6418c1da --- /dev/null +++ b/e2e/interchaintest/testsuite/constants.go @@ -0,0 +1,11 @@ +package testsuite + +const ( + // hermesRelayerImage = "ghcr.io/informalsystems/hermes" + // hermesRelayerTag = "v1.8.0" + // hermesRelayerUidGid = "1000:1000" + + goRelayerImage = "ghcr.io/cosmos/relayer" + goRelayerTag = "" + goRelayerUidGid = "100:1000" +) diff --git a/e2e/interchaintest/testsuite/suite.go b/e2e/interchaintest/testsuite/suite.go index 8ded2d42..cfbde16a 100644 --- a/e2e/interchaintest/testsuite/suite.go +++ b/e2e/interchaintest/testsuite/suite.go @@ -52,12 +52,12 @@ func (s *TestSuite) SetupSuite(ctx context.Context, chainSpecs []*interchaintest s.ChainB = chains[1].(*cosmos.CosmosChain) // docker run -it --rm --entrypoint echo ghcr.io/cosmos/relayer "$(id -u):$(id -g)" - customRelayerImage := relayer.CustomDockerImage("ghcr.io/cosmos/relayer", "", "100:1000") + goRelayerImage := relayer.CustomDockerImage(goRelayerImage, goRelayerTag, goRelayerUidGid) s.Relayer = interchaintest.NewBuiltinRelayerFactory( ibc.CosmosRly, zaptest.NewLogger(t), - customRelayerImage, + goRelayerImage, ).Build(t, s.dockerClient, s.network) s.ExecRep = testreporter.NewNopReporter().RelayerExecReporter(t) diff --git a/e2e/interchaintest/types/icacontroller/msg.go b/e2e/interchaintest/types/icacontroller/msg.go index 073b4a58..f5af747c 100644 --- a/e2e/interchaintest/types/icacontroller/msg.go +++ b/e2e/interchaintest/types/icacontroller/msg.go @@ -17,6 +17,7 @@ type InstantiateMsg struct { // ExecuteMsg is the message to execute cw-ica-controller type ExecuteMsg struct { CreateChannel *ExecuteMsg_CreateChannel `json:"create_channel,omitempty"` + CloseChannel *struct{} `json:"close_channel,omitempty"` SendCosmosMsgs *ExecuteMsg_SendCosmosMsgs `json:"send_cosmos_msgs,omitempty"` SendCustomIcaMessages *ExecuteMsg_SendCustomIcaMessages `json:"send_custom_ica_messages,omitempty"` UpdateCallbackAddress *ExecuteMsg_UpdateCallbackAddress `json:"update_callback_address,omitempty"` diff --git a/src/contract.rs b/src/contract.rs index a93bdc7f..47cb86d3 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -60,6 +60,7 @@ pub fn execute( ExecuteMsg::CreateChannel { channel_open_init_options, } => execute::create_channel(deps, env, info, channel_open_init_options), + ExecuteMsg::CloseChannel {} => execute::close_channel(deps, info), ExecuteMsg::SendCustomIcaMessages { messages, packet_memo, @@ -109,7 +110,7 @@ pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result Result { cw_ownable::assert_owner(deps.storage, &info.sender)?; - state::ALLOW_CHANNEL_OPEN_INIT.save(deps.storage, &true)?; - let options = if let Some(new_options) = options { state::CHANNEL_OPEN_INIT_OPTIONS.save(deps.storage, &new_options)?; new_options @@ -141,6 +140,8 @@ mod execute { .ok_or(ContractError::NoChannelInitOptions)? }; + state::ALLOW_CHANNEL_OPEN_INIT.save(deps.storage, &true)?; + let ica_channel_open_init_msg = new_ica_channel_open_init_cosmos_msg( env.contract.address.to_string(), options.connection_id, @@ -153,6 +154,28 @@ mod execute { Ok(Response::new().add_message(ica_channel_open_init_msg)) } + /// Submits a [`IbcMsg::CloseChannel`]. + #[allow(clippy::needless_pass_by_value)] + pub fn close_channel(deps: DepsMut, info: MessageInfo) -> Result { + cw_ownable::assert_owner(deps.storage, &info.sender)?; + + let channel_state = state::CHANNEL_STATE.load(deps.storage)?; + if !channel_state.is_open() { + return Err(ContractError::InvalidChannelStatus { + expected: state::ChannelStatus::Open.to_string(), + actual: channel_state.channel_status.to_string(), + }); + } + + state::ALLOW_CHANNEL_CLOSE_INIT.save(deps.storage, &true)?; + + let channel_close_msg = CosmosMsg::Ibc(IbcMsg::CloseChannel { + channel_id: channel_state.channel.endpoint.channel_id, + }); + + Ok(Response::new().add_message(channel_close_msg)) + } + // Sends custom messages to the ICA host. #[allow(clippy::needless_pass_by_value)] pub fn send_custom_ica_messages( diff --git a/src/ibc/handshake.rs b/src/ibc/handshake.rs index 9f87d9a6..8d731a35 100644 --- a/src/ibc/handshake.rs +++ b/src/ibc/handshake.rs @@ -63,14 +63,13 @@ pub fn ibc_channel_connect( } /// Handles the `ChanCloseInit` and `ChanCloseConfirm` for the IBC module. -/// `ChanCloseInit` is not implemented yet. /// /// # Errors /// /// This function returns an error if: /// /// - The channel is not stored in the contract state. -/// - The channel is already closed. +/// - The channel is not open. /// - The channel is not the same as the stored channel. #[entry_point] #[allow(clippy::needless_pass_by_value)] // entry point needs this signature @@ -80,7 +79,7 @@ pub fn ibc_channel_close( msg: IbcChannelCloseMsg, ) -> Result { match msg { - IbcChannelCloseMsg::CloseInit { .. } => unimplemented!(), + IbcChannelCloseMsg::CloseInit { channel } => ibc_channel_close::init(deps, channel), IbcChannelCloseMsg::CloseConfirm { channel } => ibc_channel_close::confirm(deps, channel), } } @@ -206,6 +205,38 @@ mod ibc_channel_open { mod ibc_channel_close { use super::{state, ContractError, DepsMut, IbcBasicResponse, IbcChannel}; + /// Handles the `ChanClosedInit` for the IBC module. + #[allow(clippy::needless_pass_by_value)] + pub fn init(deps: DepsMut, channel: IbcChannel) -> Result { + if !state::ALLOW_CHANNEL_CLOSE_INIT + .load(deps.storage) + .unwrap_or_default() + { + return Err(ContractError::ChannelCloseInitNotAllowed); + }; + + state::ALLOW_CHANNEL_CLOSE_INIT.save(deps.storage, &false)?; + + // Validate that this is the stored channel + let mut channel_state = state::CHANNEL_STATE.load(deps.storage)?; + if channel_state.channel != channel { + return Err(ContractError::InvalidChannelInContractState); + } + if !channel_state.is_open() { + return Err(ContractError::InvalidChannelStatus { + expected: state::ChannelStatus::Open.to_string(), + actual: channel_state.channel_status.to_string(), + }); + } + + // Update the channel state + channel_state.close(); + state::CHANNEL_STATE.save(deps.storage, &channel_state)?; + + // Return the response, emit events if needed + Ok(IbcBasicResponse::default()) + } + /// Handles the `ChanCloseConfirm` for the IBC module. #[allow(clippy::needless_pass_by_value)] pub fn confirm(deps: DepsMut, channel: IbcChannel) -> Result { @@ -214,6 +245,12 @@ mod ibc_channel_close { if channel_state.channel != channel { return Err(ContractError::InvalidChannelInContractState); } + if !channel_state.is_open() { + return Err(ContractError::InvalidChannelStatus { + expected: state::ChannelStatus::Open.to_string(), + actual: channel_state.channel_status.to_string(), + }); + } // Update the channel state channel_state.close(); diff --git a/src/types/error.rs b/src/types/error.rs index 1006dad2..0bb7d04a 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -55,6 +55,9 @@ pub enum ContractError { #[error("MsgChannelOpenInit is not allowed")] ChannelOpenInitNotAllowed, + #[error("MsgChannelCloseInit is not allowed")] + ChannelCloseInitNotAllowed, + #[error("codec is not supported: unsupported codec format {0}")] UnsupportedCodec(String), @@ -81,4 +84,7 @@ pub enum ContractError { #[error("no channel init options are provided to the contract")] NoChannelInitOptions, + + #[error("invalid channel status: expected {expected}, got {actual}")] + InvalidChannelStatus { expected: String, actual: String }, } diff --git a/src/types/msg.rs b/src/types/msg.rs index 5b2cbf8a..0444f4fe 100644 --- a/src/types/msg.rs +++ b/src/types/msg.rs @@ -35,6 +35,8 @@ pub enum ExecuteMsg { #[serde(skip_serializing_if = "Option::is_none")] channel_open_init_options: Option, }, + /// `CloseChannel` closes the IBC channel. + CloseChannel {}, /// `SendCosmosMsgs` converts the provided array of [`CosmosMsg`] to an ICA tx and sends them to the ICA host. /// [`CosmosMsg::Stargate`] and [`CosmosMsg::Wasm`] are only supported if the [`TxEncoding`](crate::ibc::types::metadata::TxEncoding) is [`TxEncoding::Protobuf`](crate::ibc::types::metadata::TxEncoding). /// diff --git a/src/types/state.rs b/src/types/state.rs index a8942c61..c83c9ee6 100644 --- a/src/types/state.rs +++ b/src/types/state.rs @@ -25,6 +25,10 @@ pub const CHANNEL_OPEN_INIT_OPTIONS: Item = /// Used to prevent relayers from opening channels. This right is reserved to the contract. pub const ALLOW_CHANNEL_OPEN_INIT: Item = Item::new("allow_channel_open_init"); +/// The item used to store whether or not channel close init is allowed. +/// Used to prevent relayers from closing channels. This right is reserved to the contract. +pub const ALLOW_CHANNEL_CLOSE_INIT: Item = Item::new("allow_channel_close_init"); + mod contract { use crate::ibc::types::metadata::TxEncoding; @@ -177,6 +181,20 @@ mod channel { matches!(self.channel.order, IbcOrder::Ordered) } } + + impl ToString for Status { + fn to_string(&self) -> String { + match self { + Self::Uninitialized => "STATE_UNINITIALIZED_UNSPECIFIED".to_string(), + Self::Init => "STATE_INIT".to_string(), + Self::TryOpen => "STATE_TRYOPEN".to_string(), + Self::Open => "STATE_OPEN".to_string(), + Self::Closed => "STATE_CLOSED".to_string(), + Self::Flushing => "STATE_FLUSHING".to_string(), + Self::FlushComplete => "STATE_FLUSHCOMPLETE".to_string(), + } + } + } } #[cfg(test)]