UX and Security Updates for Stargate release

[ebuchman]

The initial Stargate release in Gov proposal 35 refers to a codehash that contains a security vulnerability in Tendermint, as well as some issues in the Gaia migration script related to Proposal 29. It is also missing a few minor things which would have a significant impact on UX. We should fix the following before pushing a new Stargate proposal:

• Fix Tendermint security issues - Bump to sdk@v0.40.1 and tendermint@v0.34.3 #552
• Fix Proposal 29 verification in migration script - Additional review of prop 29 and migration testing #559
• ATOM Denom Metadata - to be added to migration script - Set the denom metadata for the cosmoshub in the stargate migration #564
• Add support for Amino codec to IBC message types in Cosmos-SDK to enable IBC messages to be signed by existing Ledger Nano apps - @fedekunze - original in Add amino JSON support for IBC messages cosmos-sdk#8266, supersceded by IBC amino for MsgTransfer only, credit: @fedekunze cosmos-sdk#8437
• Reorder IBC channel callbacks to ensure channel ID is available in OnChanOpenAck callback - fixed in Reorder IBC channel callbacks cosmos-sdk#8404 - please confirm @colin-axner @AdityaSripal @cwgoes @ethanfrey

Note that since the changes to the SDK are technically breaking, these should be released in v0.41 of the SDK. Gaia will then need to be updated per #566

[zmanian]

Tendermint issues are fixed in #552
Prop 29 is fixed in #559
Atom denom is fixed in #564

[ethanfrey]

I can confirm that cosmos/cosmos-sdk#8404 is working. I tested this downstream and it allowed sending ibc packets in the OnChanOpenAck callback.

The only question is that this merged on master and not to be backported to the 0.40.x branch. And it is unclear where the next sdk release will be cut (or where Fede's PR will get merged into). I think this is the key piece of missing info

[ebuchman]

I understood these changes to be breaking and thus to require v0.41

[cwgoes]

I can confirm that cosmos/cosmos-sdk#8404 is working. I tested this downstream and it allowed sending ibc packets in the OnChanOpenAck callback.

The only question is that this merged on master and not to be backported to the 0.40.x branch. And it is unclear where the next sdk release will be cut (or where Fede's PR will get merged into). I think this is the key piece of missing info

As this release is breaking anyways, it is the preference of the IBC team to have 8404 included as well as the Amino codec changes.

[ebuchman]

Re Amino support for IBC messages to allow Ledger signing. This was not as easy as it seemed when messages include public keys due to subtleties around protobuf types between tendermint and the sdk: https://github.com/cosmos/cosmos-sdk/pull/8422/files#r564061094

Since supporting all messages isn't straight forward, and there is limited use case for them beyond MsgTransfer, Stargate will only add Amino support for ICS20 MsgTransfer, and not for any of the other IBC messages.

This means only MsgTransfer will be able to be signed by Ledgers. In general this should be fine, as MsgTransfer is the only real application-level / user-facing message. All other message types ought to be handled by relayers, which are online processes signing many txs and typically not using a ledger anyways.

The other consequence here is that solo-machines, which are responsible for relaying their own client updates and packets, will not be able to use Ledgers either.

[ebuchman]

I believe this is all complete, yes? Should we close ? I know there's still some testing going on but can be tracked separately.