refactor(voyager): ensure_ibc_interface supports multiple interfaces #3578
+19
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@kayandra is attempting to deploy a commit to the unionbuild Team on Vercel. A member of the Team first needs to authorize it. |
kayandra
changed the title
benluelo
reviewed
Jan 18, 2025
| @@ -176,12 +176,18 @@ impl ClientModuleInfo { | |||
|
|
|||
| pub fn ensure_ibc_interface( | |||
| &self, | |||
| ibc_interface: impl AsRef<str>, | |||
| expected_interfaces: Vec<impl AsRef<str>>, | |||
There was a problem hiding this comment.
instead of Vec, you can take impl IntoIterator<Item = impl AsRef<str>>
| @@ -47,7 +47,7 @@ impl ClientModule for Module { | |||
| async fn new(config: Self::Config, info: ClientModuleInfo) -> Result<Self, BoxDynError> { | |||
| info.ensure_client_type(ClientType::ETHEREUM)?; | |||
| info.ensure_consensus_type(ConsensusType::ETHEREUM)?; | |||
| info.ensure_ibc_interface(IbcInterface::IBC_COSMWASM)?; | |||
| info.ensure_ibc_interface(vec![IbcInterface::IBC_COSMWASM])?; | |||
There was a problem hiding this comment.
if you change this to accept impl IntoIterator<Item = impl AsRef<str>>, you can simply pass [IbcInterface::IBC_COSMWASM] here
| @@ -46,7 +46,7 @@ impl ClientModule for Module { | |||
| async fn new(Config {}: Self::Config, info: ClientModuleInfo) -> Result<Self, BoxDynError> { | |||
| info.ensure_client_type(ClientType::MOVEMENT)?; | |||
| info.ensure_consensus_type(ConsensusType::MOVEMENT)?; | |||
| info.ensure_ibc_interface(IbcInterface::IBC_COSMWASM)?; | |||
| info.ensure_ibc_interface(vec![IbcInterface::IBC_COSMWASM])?; | |||
| @@ -46,7 +46,7 @@ impl ClientModule for Module { | |||
| async fn new(_: Self::Config, info: ClientModuleInfo) -> Result<Self, BoxDynError> { | |||
| info.ensure_client_type(ClientType::STATE_LENS_ICS23_ICS23)?; | |||
| info.ensure_consensus_type(ConsensusType::TENDERMINT)?; | |||
| info.ensure_ibc_interface(IbcInterface::IBC_SOLIDITY)?; | |||
| info.ensure_ibc_interface(vec![IbcInterface::IBC_COSMWASM])?; | |||
| @@ -81,8 +81,7 @@ impl ClientModule for Module { | |||
| async fn new(_: Self::Config, info: ClientModuleInfo) -> Result<Self, BoxDynError> { | |||
| info.ensure_client_type(ClientType::STATE_LENS_ICS23_MPT)?; | |||
| info.ensure_consensus_type(ConsensusType::ETHEREUM)?; | |||
| info.ensure_ibc_interface(IbcInterface::IBC_SOLIDITY) | |||
| .or(info.ensure_ibc_interface(IbcInterface::IBC_COSMWASM))?; | |||
| info.ensure_ibc_interface(vec![IbcInterface::IBC_SOLIDITY, IbcInterface::IBC_COSMWASM])?; | |||
Comment on lines
+1
to
+6
| #[cfg(not(feature = "library"))] | ||
| use cosmwasm_std::entry_point; | ||
| use cosmwasm_std::{ | ||
| attr, entry_point, DepsMut, Env, IbcBasicResponse, IbcChannel, IbcChannelCloseMsg, | ||
| IbcChannelConnectMsg, IbcChannelOpenMsg, IbcOrder, IbcPacket, IbcPacketAckMsg, | ||
| IbcPacketReceiveMsg, IbcPacketTimeoutMsg, IbcReceiveResponse, Reply, Response, StdError, | ||
| attr, DepsMut, Env, IbcBasicResponse, IbcChannel, IbcChannelCloseMsg, IbcChannelConnectMsg, | ||
| IbcChannelOpenMsg, IbcOrder, IbcPacket, IbcPacketAckMsg, IbcPacketReceiveMsg, | ||
| IbcPacketTimeoutMsg, IbcReceiveResponse, Reply, Response, StdError, |
There was a problem hiding this comment.
are these changes related to this PR?
There was a problem hiding this comment.
got a warning in my editor, so i cleaned it. should have made it a separate commit.
Comment on lines
+181
to
+190
| if !expected_interfaces | ||
| .iter() | ||
| .any(|expected| expected.as_ref() == self.ibc_interface.as_str()) | ||
| { | ||
| Err(UnexpectedIbcInterfaceError { | ||
| expected: self.ibc_interface.clone(), | ||
| found: ibc_interface.as_ref().to_owned(), | ||
| found: expected_interfaces | ||
| .iter() | ||
| .map(|s| s.as_ref().to_string()) | ||
| .collect(), |
There was a problem hiding this comment.
i think we actually have all of these backwards now that i look at it - all of the Unexpected*Error found/expected semantics should be switched such that the found contains what was in the config (i.e. self.ibc_interface in this context) and expected contains the list of supported values.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow checking an interface against multiple expected values by taking a Vec of interfaces.
Updates error type to show all valid interfaces in error message.
Ref #3438