search

cosmos / ibc-apps

IBC applications and middleware for Cosmos SDK chains.
Apache License 2.0
82 stars 65 forks source link

polytone: migrate to CosmWasm `v2` #220

Open CyberHoward opened 2 months ago

CyberHoward commented 2 months ago

Description

CosmWasm v2 has been slowly gaining adoption throughout the ecosystem. As critical infrastructure, Polytone should be up-to-date with this latest version, allowing devs who are using CW v2 to make use of Polytone.

Issue Scope

While working to upgrade our own contracts we've identified two major changes that should be made to Polytone.

  1. SubMsgResponse's data field is deprecated. This field is currently used to retrieve and cache execution results before returning them in the Ack.
  2. Use Reply's Payload instead of state-caching. A new Payload field was added to the SubMsg which allows developers to pass data to the reply endpoint without needing to store it to state. Applying this to Polytone would reduce the gas usage and simplify the contract's implementation. It can also be used to cache the execution responses, further reducing gas-costs.

Any other issues encountered over the software's life should preferably also be addressed in this update.

Update Rollout

Polytone's canonical deployments are non-migratable. As a result rolling out this update will require a coordinated governance proposal on each deployed chain to migrate the contracts.

Open questions

JakeHartnell commented 2 months ago

Will migrating one side of the polytone channel cause the channel to break until the other side is migrated?

Only one way to find out. Maybe @Reecepbcups could grace us with the magic of interchain-test. 🙏

Are there any other issues that should be linked to this upgrade?

Would be open to other suggestions for how to improve Polytone @CyberHoward.

Reecepbcups commented 2 months ago

We have a WIP integration for polytone to ICT natively: https://github.com/strangelove-ventures/interchaintest/pull/1215

We have planed to re-allocate some engineering time to this again after we complete some more wormhole work

Integration test will be super easy to build off that base once we have it configured (plus Spawn integration)

CyberHoward commented 2 months ago

Will migrating one side of the polytone channel cause the channel to break until the other side is migrated?

Only one way to find out. Maybe @Reecepbcups could grace us with the magic of interchain-test. 🙏

Are there any other issues that should be linked to this upgrade?

Would be open to other suggestions for how to improve Polytone @CyberHoward.

Personally my main concern with Polytone is its vulnerability to a DOS attack by simply creating packet execution loops, as outlined by the Oak Audit (see screenshot)

Image

The way I'd like to see this mitigated is by adding the ability to use an optional whitelist for note senders (address and/or code-id based). That way protocols can deploy their own version of Polytone and configure it to ensure that only its own contracts can use that polytone channel.

Thoughts?