Add ibc v3 support to message types #1302
Conversation
| #[cfg(feature = "ibcv3")] | ||
| pub fn new(packet: IbcPacket, relayer: Addr) -> Self { | ||
| Self { packet, relayer } | ||
| } |
There was a problem hiding this comment.
This is a bit of a problematic use of features. Rust assumes adding features does not break compilation. The original dependency resolver just enabled all features required by some caller and compiled the dependency once. Code like this then suddenly breaks in a hard to understand way.
But yeah, we'll probably remove all non-v3 code soon anyways.
There was a problem hiding this comment.
This is a good point. My thought was a contract is written either with one or the other. I guess for some internal tests this may cause issues, or unit tests where ibc1 contracts import ibc3 contracts.
Happy to change this if you have a better idea
Happy to change this and some of the smaller feedback. |
|
@webmaster128 I addressed the majority of the points. I am unsure if there is some way I can improve contract compilation with feature flags. |
|
I made a demo on handling this less-breaking in #1304 feel free to merge that into this PR if you like the approach, leave it as is, or give me a concrete suggestion of another approach to take here. |
packages/vm/src/calls.rs
Outdated
| @@ -227,7 +227,7 @@ where | |||
| let env = to_vec(env)?; | |||
| let msg = to_vec(msg)?; | |||
| let data = call_ibc_channel_open_raw(instance, &env, &msg)?; | |||
| let result: ContractResult<()> = | |||
| let result: ContractResult<Option<IbcV3ChannelOpenResponse>> = | |||
There was a problem hiding this comment.
Ah nice, here we unify the two versions now into one type.
There was a problem hiding this comment.
Yup, the VM uses the backwards compatible ibc3 versions. Well noted we don't need a flag for this repo
CHANGELOG.md
Outdated
| like version negotiation and exposing relayer address to the contract. | ||
| Requires a compatible wasmd runtime (v0.27.0+) ([#1297]) | ||
|
|
||
| [#1302]: https://github.com/CosmWasm/cosmwasm/pull/1302 |
There was a problem hiding this comment.
Can you rebase this to main and move the block to the Unreleased section?
CHANGELOG.md
Outdated
| @@ -23,6 +23,11 @@ and this project adheres to | |||
| - cosmwasm-std: Implement `Div`/`DivAssign` for `Decimal`/`Decimal256`. | |||
| - cosmwasm-vm: Add feature `allow_interface_version_7` to run CosmWasm 0.16 | |||
| contracts in modern hosts. Be careful if you consider using this! | |||
| - cosmwasm-std: Add new `ibc3` feature that allows to use IBC-Go V3 features, | |||
| like version negotiation and exposing relayer address to the contract. | |||
| Requires a compatible wasmd runtime (v0.27.0+) ([#1297]) | |||
There was a problem hiding this comment.
The PR number needs to be updated
ethanfrey
force-pushed
the
add-ibc-v3-support
branch
from
392da4c
to
ada176f
Compare
|
🐎🐎🐎🐎🐎 |
Part of #1200 that is very close to me as I just worked on the ibcv3 integration in
wasmd.Added a new feature flag "ibcv3".
I made all effort to ensure the types are backwards compatible with ibcv1. With ibcv3 enabled:
The
ibc_connection_opencall may return a struct with a version field to set a custom version in handshake. (Previously it returned()which serialised tonull, so they both can be safely parsed into*MyStruct/Option<MyStruct>)IbcPacketReceiveMsg,IbcPacketAckMsg, andIbcPacketTimeoutMsgall require an additional relayer field from the runtime. Ibc v1 contracts will silently ignore this info if passed from the runtime.Also updated the VM to handle this, and updated
ibc-reflectcontract to use ibcv3 (whileibc-reflect-sendremains with "stargate" to ensure they both work)@webmaster128 I would really like to get this into the 1.0 release if possible (along with your #1299 enhancement). It is just a type signature issue, so no major code/logic changes, but it will allow wasmd to to a lot more. (And implement ICA as a contract)