From 7150af52102d7faa5b7324de92fd2aef21355a85 Mon Sep 17 00:00:00 2001 From: srdtrk <59252793+srdtrk@users.noreply.github.com> Date: Mon, 11 Dec 2023 14:13:10 +0300 Subject: [PATCH] imp: remove stargate query (#31) * imp: removed stargate query * imp(e2e): fixed tests * deps(e2e): ran 'go mod tidy' * style: ran 'cargo fmt' * test: fixed tests --- e2e/interchaintest/contract_test.go | 5 +- e2e/interchaintest/go.mod | 2 +- e2e/interchaintest/types/ica_contract.go | 8 ++ e2e/interchaintest/types/ica_msg.go | 14 +++- src/contract.rs | 31 ++++++-- src/ibc/handshake.rs | 2 +- src/ibc/types/metadata.rs | 72 +++++++++++++----- src/ibc/types/stargate.rs | 96 ------------------------ src/types/error.rs | 3 + src/types/msg.rs | 14 +++- src/types/state.rs | 6 +- 11 files changed, 117 insertions(+), 136 deletions(-) diff --git a/e2e/interchaintest/contract_test.go b/e2e/interchaintest/contract_test.go index 30ae46cb..19c41aeb 100644 --- a/e2e/interchaintest/contract_test.go +++ b/e2e/interchaintest/contract_test.go @@ -255,7 +255,7 @@ func (s *ContractTestSuite) TestRecoveredIcaContractInstantiatedChannelHandshake }) s.Run("TestChannelHandshakeSuccessAfterFail", func() { - err = s.Contract.ExecCreateChannel(ctx, wasmdUser.KeyName(), s.ChainAConnID, s.ChainBConnID, nil, nil, "--gas", "500000") + err = s.Contract.ExecCreateChannelWithOptions(ctx, wasmdUser.KeyName(), s.ChainAConnID, s.ChainBConnID, nil, nil, "--gas", "500000") s.Require().NoError(err) // Wait for the channel to get set up @@ -555,8 +555,7 @@ func (s *ContractTestSuite) TestIcaContractTimeoutPacket() { s.Run("TestChannelReopening", func() { // Reopen the channel: - txEncoding := icatypes.EncodingProto3JSON - err := s.Contract.ExecCreateChannel(ctx, wasmdUser.KeyName(), s.ChainAConnID, s.ChainBConnID, nil, &txEncoding, "--gas", "500000") + err := s.Contract.ExecCreateChannel(ctx, wasmdUser.KeyName(), "--gas", "500000") s.Require().NoError(err) // Wait for the channel to get set up diff --git a/e2e/interchaintest/go.mod b/e2e/interchaintest/go.mod index 200c8d69..665c6ee0 100644 --- a/e2e/interchaintest/go.mod +++ b/e2e/interchaintest/go.mod @@ -11,6 +11,7 @@ require ( github.com/strangelove-ventures/interchaintest/v7 v7.0.0-20231025163603-99ec15f2674d github.com/stretchr/testify v1.8.4 go.uber.org/zap v1.26.0 + google.golang.org/grpc v1.57.0 ) require ( @@ -212,7 +213,6 @@ require ( google.golang.org/genproto v0.0.0-20230706204954-ccb25ca9f130 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20230629202037-9506855d4529 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98 // indirect - google.golang.org/grpc v1.57.0 // indirect google.golang.org/protobuf v1.31.0 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce // indirect diff --git a/e2e/interchaintest/types/ica_contract.go b/e2e/interchaintest/types/ica_contract.go index bb5989e5..1687ea19 100644 --- a/e2e/interchaintest/types/ica_contract.go +++ b/e2e/interchaintest/types/ica_contract.go @@ -50,6 +50,14 @@ func StoreAndInstantiateNewIcaContract( } func (c *IcaContract) ExecCreateChannel( + ctx context.Context, callerKeyName string, extraExecTxArgs ...string, +) error { + msg := newEmptyCreateChannelMsg() + err := c.Execute(ctx, callerKeyName, msg, extraExecTxArgs...) + return err +} + +func (c *IcaContract) ExecCreateChannelWithOptions( ctx context.Context, callerKeyName string, connectionId string, counterpartyConnectionId string, counterpartyPortId *string, txEncoding *string, extraExecTxArgs ...string, diff --git a/e2e/interchaintest/types/ica_msg.go b/e2e/interchaintest/types/ica_msg.go index 2fd2ad60..26c36b13 100644 --- a/e2e/interchaintest/types/ica_msg.go +++ b/e2e/interchaintest/types/ica_msg.go @@ -67,12 +67,20 @@ func NewInstantiateMsgWithChannelInitOptions( return string(jsonBytes) } +func newEmptyCreateChannelMsg() string { + return `{ "create_channel": {} }` +} + func newCreateChannelMsg( connectionId string, counterpartyConnectionId string, counterpartyPortId *string, txEncoding *string, ) string { + type ChannelCreateMsg struct { + ChannelOpenInitOptions *ChannelOpenInitOptions `json:"channel_open_init_options,omitempty"` + } + type ChannelCreateMsgWrapper struct { - CreateChannelMsg ChannelOpenInitOptions `json:"create_channel"` + CreateChannelMsg ChannelCreateMsg `json:"create_channel"` } channelOpenInitOptions := ChannelOpenInitOptions{ @@ -83,7 +91,9 @@ func newCreateChannelMsg( } msg := ChannelCreateMsgWrapper{ - CreateChannelMsg: channelOpenInitOptions, + CreateChannelMsg: ChannelCreateMsg{ + ChannelOpenInitOptions: &channelOpenInitOptions, + }, } jsonBytes, err := json.Marshal(msg) diff --git a/src/contract.rs b/src/contract.rs index 2beccf99..6caf0406 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -7,7 +7,8 @@ use crate::ibc::types::stargate::channel::new_ica_channel_open_init_cosmos_msg; use crate::types::keys::{CONTRACT_NAME, CONTRACT_VERSION}; use crate::types::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; use crate::types::state::{ - CallbackCounter, ChannelState, ContractState, CALLBACK_COUNTER, CHANNEL_STATE, STATE, + CallbackCounter, ChannelState, ContractState, CALLBACK_COUNTER, CHANNEL_OPEN_INIT_OPTIONS, + CHANNEL_STATE, STATE, }; use crate::types::ContractError; @@ -37,6 +38,8 @@ pub fn instantiate( // If channel open init options are provided, open the channel. if let Some(channel_open_init_options) = msg.channel_open_init_options { + CHANNEL_OPEN_INIT_OPTIONS.save(deps.storage, &channel_open_init_options)?; + let ica_channel_open_init_msg = new_ica_channel_open_init_cosmos_msg( env.contract.address.to_string(), channel_open_init_options.connection_id, @@ -61,7 +64,9 @@ pub fn execute( msg: ExecuteMsg, ) -> Result { match msg { - ExecuteMsg::CreateChannel(options) => execute::create_channel(deps, env, info, options), + ExecuteMsg::CreateChannel { + channel_open_init_options, + } => execute::create_channel(deps, env, info, channel_open_init_options), ExecuteMsg::SendCustomIcaMessages { messages, packet_memo, @@ -112,13 +117,13 @@ pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result, ) -> Result { cw_ownable::assert_owner(deps.storage, &info.sender)?; - let mut contract_state = STATE.load(deps.storage)?; - contract_state.enable_channel_open_init(); - STATE.save(deps.storage, &contract_state)?; + STATE.update(deps.storage, |mut state| -> StdResult<_> { + state.enable_channel_open_init(); + Ok(state) + })?; + + let options = if let Some(new_options) = options { + CHANNEL_OPEN_INIT_OPTIONS.save(deps.storage, &new_options)?; + new_options + } else { + CHANNEL_OPEN_INIT_OPTIONS + .may_load(deps.storage)? + .ok_or(ContractError::NoChannelInitOptions)? + }; let ica_channel_open_init_msg = new_ica_channel_open_init_cosmos_msg( env.contract.address.to_string(), diff --git a/src/ibc/handshake.rs b/src/ibc/handshake.rs index 4dc86638..8f7c1021 100644 --- a/src/ibc/handshake.rs +++ b/src/ibc/handshake.rs @@ -93,7 +93,7 @@ mod ibc_channel_open { // serde::Deserialize the metadata let metadata: IcaMetadata = if channel.version.is_empty() { // if empty, use create new metadata. - IcaMetadata::from_channel(deps.as_ref(), &channel) + IcaMetadata::from_channel(deps.as_ref(), &channel)? } else { serde_json_wasm::from_str(&channel.version).map_err(|_| { ContractError::UnknownDataType( diff --git a/src/ibc/types/metadata.rs b/src/ibc/types/metadata.rs index 0b1cfea5..2d88b1e8 100644 --- a/src/ibc/types/metadata.rs +++ b/src/ibc/types/metadata.rs @@ -9,7 +9,10 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::{Deps, IbcChannel}; -use crate::types::{state::CHANNEL_STATE, ContractError}; +use crate::types::{ + state::{CHANNEL_OPEN_INIT_OPTIONS, CHANNEL_STATE}, + ContractError, +}; use super::keys::ICA_VERSION; @@ -72,10 +75,13 @@ impl IcaMetadata { /// This is a fallback option if the ICA controller is not provided with the /// handshake version metadata by the relayer. It first tries to load the /// previous version of the [`IcaMetadata`] from the store, and if it fails, - /// it uses a stargate query to get the counterparty connection id. - /// Stargate queries are not universally supported, so this is a fallback option. - #[must_use] - pub fn from_channel(deps: Deps, channel: &IbcChannel) -> Self { + /// it uses the [`CHANNEL_OPEN_INIT_OPTIONS`] to create a new [`IcaMetadata`]. + /// + /// # Errors + /// + /// Returns an error if the previous version of the [`IcaMetadata`] cannot be loaded + /// from the store, and no [`CHANNEL_OPEN_INIT_OPTIONS`] are set in the store. + pub fn from_channel(deps: Deps, channel: &IbcChannel) -> Result { // If the the counterparty chain is using the fee middleware, and the this chain is not, // and the previous handshake was initiated with an empty version string, then the // previous version in the contract's channel state will be wrapped by the fee middleware, @@ -83,26 +89,26 @@ impl IcaMetadata { if let Ok(channel_state) = CHANNEL_STATE.load(deps.storage) { if let Ok(previous_metadata) = serde_json_wasm::from_str(&channel_state.channel.version) { - return previous_metadata; + return Ok(previous_metadata); } } - let counterparty_connection_id = super::stargate::query::counterparty_connection_id( - &deps.querier, - channel.connection_id.clone(), - ) - .unwrap_or_default(); - Self { + let options = CHANNEL_OPEN_INIT_OPTIONS.load(deps.storage)?; + if options.connection_id != channel.connection_id { + return Err(ContractError::InvalidConnection); + } + + Ok(Self { version: ICA_VERSION.to_string(), - controller_connection_id: channel.connection_id.clone(), + encoding: options.tx_encoding(), + controller_connection_id: options.connection_id, // counterparty connection_id is not exposed to the contract, so we // use a stargate query to get it. Stargate queries are not universally // supported, so this is a fallback option. - host_connection_id: counterparty_connection_id, + host_connection_id: options.counterparty_connection_id, address: String::new(), - encoding: TxEncoding::Proto3Json, tx_type: "sdk_multi_msg".to_string(), - } + }) } /// Validates the [`IcaMetadata`] @@ -179,6 +185,8 @@ fn validate_ica_address(address: &str) -> Result<(), ContractError> { mod tests { use cosmwasm_std::{testing::mock_dependencies, IbcEndpoint, IbcOrder}; + use crate::types::msg::options::ChannelOpenInitOptions; + use super::*; fn mock_channel( @@ -220,7 +228,7 @@ mod tests { #[test] fn test_validate_success() { - let deps = mock_dependencies(); + let mut deps = mock_dependencies(); let channel = mock_channel( "ics27-1", @@ -230,13 +238,25 @@ mod tests { "channel-1", "port-1", ); - let metadata = IcaMetadata::from_channel(deps.as_ref(), &channel); + let stored_init_options = ChannelOpenInitOptions { + connection_id: "connection-0".to_string(), + counterparty_connection_id: "connection-1".to_string(), + tx_encoding: None, + counterparty_port_id: Some(super::super::keys::HOST_PORT_ID.to_string()), + }; + + CHANNEL_OPEN_INIT_OPTIONS + .save(deps.as_mut().storage, &stored_init_options) + .unwrap(); + + let metadata = IcaMetadata::from_channel(deps.as_ref(), &channel).unwrap(); + assert!(metadata.validate(&channel).is_ok()); } #[test] fn test_validate_fail() { - let deps = mock_dependencies(); + let mut deps = mock_dependencies(); let channel_1 = mock_channel( "ics27-1", @@ -246,6 +266,18 @@ mod tests { "channel-1", "port-1", ); + + let stored_init_options = ChannelOpenInitOptions { + connection_id: "connection-0".to_string(), + counterparty_connection_id: "connection-1".to_string(), + tx_encoding: None, + counterparty_port_id: Some(super::super::keys::HOST_PORT_ID.to_string()), + }; + + CHANNEL_OPEN_INIT_OPTIONS + .save(deps.as_mut().storage, &stored_init_options) + .unwrap(); + let channel_2 = mock_channel( "ics27-1", "connection-1", @@ -254,7 +286,7 @@ mod tests { "channel-1", "port-1", ); - let metadata = IcaMetadata::from_channel(deps.as_ref(), &channel_1); + let metadata = IcaMetadata::from_channel(deps.as_ref(), &channel_1).unwrap(); assert!(metadata.validate(&channel_2).is_err()); } diff --git a/src/ibc/types/stargate.rs b/src/ibc/types/stargate.rs index d53faaea..3333fae9 100644 --- a/src/ibc/types/stargate.rs +++ b/src/ibc/types/stargate.rs @@ -13,7 +13,6 @@ use cosmos_sdk_proto::traits::Message; -use crate::types::ContractError; use cosmwasm_std::Binary; /// Contains the stargate channel lifecycle helper methods. @@ -90,98 +89,3 @@ pub mod channel { } } } -/// Contains the stargate query methods. -pub mod query { - use super::{Binary, ContractError, Message}; - - use cosmos_sdk_proto::ibc::core::connection::v1::QueryConnectionRequest; - use cosmwasm_std::{Empty, QuerierWrapper, QueryRequest}; - - /// Queries the counterparty connection id using stargate queries. - /// - /// # Errors - /// - /// Returns an error if the query fails. - pub fn counterparty_connection_id( - querier: &QuerierWrapper, - connection_id: impl Into, - ) -> Result { - let request = QueryConnectionRequest { - connection_id: connection_id.into(), - }; - let query: QueryRequest = QueryRequest::Stargate { - path: "/ibc.core.connection.v1.Query/Connection".into(), - data: Binary(request.encode_to_vec()), - }; - - let response: response::QueryConnectionResponse = querier.query(&query)?; - Ok(response.connection.counterparty.connection_id) - } - - /// Contains the types used in query responses. - mod response { - /// `QueryConnectionResponse` is the response type for the Query/Connection RPC - /// method. Besides the connection end, it includes a proof and the height from - /// which the proof was retrieved. - #[allow(clippy::module_name_repetitions)] - #[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize)] - pub struct QueryConnectionResponse { - /// connection associated with the request identifier - pub connection: ConnectionEnd, - } - - /// `ConnectionEnd` defines a stateful object on a chain connected to another - /// separate one. - /// NOTE: there must only be 2 defined `ConnectionEnds` to establish - /// a connection between two chains. - #[derive(Clone, PartialEq, Debug, serde::Deserialize, serde::Serialize)] - pub struct ConnectionEnd { - /// client associated with this connection. - pub client_id: String, - /// IBC version which can be utilised to determine encodings or protocols for - /// channels or packets utilizing this connection. - pub versions: Vec, - /// current state of the connection end. - pub state: String, - /// counterparty chain associated with this connection. - pub counterparty: Counterparty, - /// delay period that must pass before a consensus state can be used for - /// packet-verification NOTE: delay period logic is only implemented by some - /// clients. - pub delay_period: String, - } - - /// `Counterparty` defines the counterparty chain associated with a connection end. - #[derive(Clone, PartialEq, Debug, serde::Deserialize, serde::Serialize)] - pub struct Counterparty { - /// identifies the client on the counterparty chain associated with a given - /// connection. - pub client_id: String, - /// identifies the connection end on the counterparty chain associated with a - /// given connection. - pub connection_id: String, - /// commitment merkle prefix of the counterparty chain. - pub prefix: MerklePrefix, - } - - /// `MerklePrefix` is merkle path prefixed to the key. - #[allow(clippy::derive_partial_eq_without_eq)] - #[derive(Clone, PartialEq, Debug, serde::Deserialize, serde::Serialize)] - pub struct MerklePrefix { - /// The constructed key from the Path and the key will be append(Path.KeyPath, - /// append(Path.KeyPrefix, key...)) - pub key_prefix: String, - } - - /// `Version` defines the versioning scheme used to negotiate the IBC verison in - /// the connection handshake. - #[allow(clippy::derive_partial_eq_without_eq)] - #[derive(Clone, PartialEq, Debug, serde::Deserialize, serde::Serialize)] - pub struct Version { - /// unique version identifier - pub identifier: String, - /// list of features compatible with the specified identifier - pub features: Vec, - } - } -} diff --git a/src/types/error.rs b/src/types/error.rs index e5105fc1..1006dad2 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -78,4 +78,7 @@ pub enum ContractError { #[error("interchain account information is not set")] IcaInfoNotSet, + + #[error("no channel init options are provided to the contract")] + NoChannelInitOptions, } diff --git a/src/types/msg.rs b/src/types/msg.rs index fbe2df5c..93ab41cd 100644 --- a/src/types/msg.rs +++ b/src/types/msg.rs @@ -26,11 +26,17 @@ pub struct InstantiateMsg { #[cw_ownable::cw_ownable_execute] #[cw_serde] pub enum ExecuteMsg { - /// CreateChannel makes the contract submit a stargate MsgChannelOpenInit to the chain. + /// `CreateChannel` makes the contract submit a stargate MsgChannelOpenInit to the chain. /// This is a wrapper around [`options::ChannelOpenInitOptions`] and thus requires the - /// same fields. - CreateChannel(options::ChannelOpenInitOptions), - /// SendCustomIcaMessages sends custom messages from the ICA controller to the ICA host. + /// same fields. If not specified, then the options specified in the contract instantiation + /// are used. + CreateChannel { + /// The options to initialize the IBC channel. + /// If not specified, the options specified in the contract instantiation are used. + #[serde(skip_serializing_if = "Option::is_none")] + channel_open_init_options: Option, + }, + /// `SendCustomIcaMessages` sends custom messages from the ICA controller to the ICA host. SendCustomIcaMessages { /// Base64-encoded json or proto messages to send to the ICA host. /// diff --git a/src/types/state.rs b/src/types/state.rs index 0d903766..96a27718 100644 --- a/src/types/state.rs +++ b/src/types/state.rs @@ -4,7 +4,7 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::{Addr, IbcChannel}; use cw_storage_plus::Item; -use super::ContractError; +use super::{msg::options::ChannelOpenInitOptions, ContractError}; #[allow(clippy::module_name_repetitions)] pub use channel::{State as ChannelState, Status as ChannelStatus}; @@ -20,6 +20,10 @@ pub const CHANNEL_STATE: Item = Item::new("ica_channel"); /// The item used to store the successful and erroneous callbacks in store. pub const CALLBACK_COUNTER: Item = Item::new("callback_counter"); +/// The item used to store the channel open init options. +pub const CHANNEL_OPEN_INIT_OPTIONS: Item = + Item::new("channel_open_init_options"); + mod contract { use crate::ibc::types::metadata::TxEncoding;