Add support for Packet memo string & testing case

[adizere]

Summary

There is a new field for adding metadata bytes as part of ICS20 Packets.
This will be released with IBC-go versions 2.5, v3.4, v4.2 and v5.1. And then also in v6.0.0 (scheduled for late November).

The changes are relatively straightforward and I suspect do not require any modifications in Hermes. This is because the MsgTransfer structure seems to be opaque to the relayer, encapsulated (hidden) inside the Packet.data field, which Hermes does not typically deserialize, though I might be wrong.

Since this is an important feature for the ecosystem, we should test and ensure relaying works as expected.

Acceptance Criteria

We should

• re-generate the proto-files to ensure we support relaying of the new Metadata field of ICS20 Packets,
  • This proved to be unnecessary, see explanation in the follow-up comments of this issue
• stretch goal, in case the new field is not opaque to the relayer: add a test case in our integration test framework to assert that Hermes relays correctly across networks the Metadata bytes in the packet
  • The new memo field is opaque to the relayer, so no changes are necessary

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[adizere]

Context from Carlos:

We tagged some of the minor backports with the memo field:
https://github.com/cosmos/ibc-go/releases/tag/v2.5.0
https://github.com/cosmos/ibc-go/releases/tag/v3.4.0
https://github.com/cosmos/ibc-go/releases/tag/v4.2.0
We will tag v5.1.0 when SDK releases v0.46.5 with this fix.

[adizere]

There was a redesign (i.e., rename) from "metadata" to "memo string". Ref cosmos/ibc#877

[adizere]

Adding here a manual testing recipe with the steps I did to manually inspect if the memo field has any impact --or it's entirely opaque to the relayer. The conclusion was that the field is opaque and relayer needs no update/changes to support it. The impact this new field has is on message payload size and therefore likely on gas costs, but not on the relayer logic or dependencies.

Dependencies:

• hermes 1.1.0+f5263bf4
• simd binary compiled from v5.1.0 but reported version is: 5.0.0-beta1-61-gc0acd5bd
  • this will allow us to build testnets that have the new memo field
• gaiad version v7.1.0
  • this will allow us to build testnets that have no support for the new memo field

Case 2: Relay between two networks which both have support for the new memo field

gm config

[global]
add_to_hermes = false
auto_maintain_config = true
extra_wallets = 1
gaiad_binary = '/Users/adiseredinschi/Hammers/ibc-go/build/simd'
hdpath = ''
home_dir = '$HOME/.gm'
ports_start_at = 27040

[global.hermes]
binary = '/Users/adiseredinschi/Hammers/hermes/target/debug//hermes'
config = '$HOME/.hermes/config.toml'
log_level = 'info'
telemetry_enabled = true
telemetry_host = '127.0.0.1'
telemetry_port = 3001

[ibc-0]
ports_start_at = 27000

[ibc-1]
ports_start_at = 27010

[node1]
network = 'ibc-0'
ports_start_at = 27020
add_to_hermes = true

[node2]
network = 'ibc-1'
ports_start_at = 27030
add_to_hermes =true 

The important part of gm.toml is we want both network to spawn from ibc-go/build/simd, having support for the new memo field.

Steps:

1. gm start
2. gm hermes keys && gm hermes config
3. hermes start
4. hermes create channel --a-chain ibc-0 --b-chain ibc-1 --a-port transfer --b-port transfer --new-client-connection
5. trigger a new packet send with the memo field

   • % ./build/simd tx ibc-transfer transfer transfer channel-0 cosmos15kxm89ws2lcccwj36fjamh84h44zkhweys0xrf 10samoleans --home ~/.gm/ibc-0 --keyring-backend test --node http://localhost:27000 --from cosmos12kykcw0v0vnms8lqynu5sk050ep0lw69r5gqhh --memo MemoWithHermesV1.1.0

   • Note: we're initiate the packet send at network ibc-0
   • To obtain the value for the --from parameter, use any account associated with network ibc-0
6. wait for Hermes to relay the packet
7. grab the tx hash for the transaction submitted to ibc-1 from Hermes logs
   • should look something like:

   • response(s): 1; Ok:0CEBDB88B3F22A81C8053AD08C2862C5F9FBA1AE8137C5BA02742EC9FD8B2A78 target_chain=ibc-1

8. Now use the hash 0CEBDB88B3F22A81C8053AD08C2862C5F9FBA1AE8137C5BA02742EC9FD8B2A78 to query the packet data as it arrived on the destination chain ibc-1

   • % ./build/simd query tx 0CEBDB88B3F22A81C8053AD08C2862C5F9FBA1AE8137C5BA02742EC9FD8B2A78 --node http://localhost:27010

   • Output:

   • value: '{"amount":"10","denom":"samoleans","memo":"MemoWithHermesV1.1.0","receiver":"cosmos15kxm89ws2lcccwj36fjamh84h44zkhweys0xrf","sender":"cosmos12kykcw0v0vnms8lqynu5sk050ep0lw69r5gqhh"}'

I tried relaying longer memo fields with different content, nothing jumped at me as problematic.

Case 1: Relay between two networks where one network does not support the new memo field

We'll send a packet with a memo field from a network that enables that field to a network that has no support for it.

gm config

[global]
add_to_hermes = false
auto_maintain_config = true
extra_wallets = 1
gaiad_binary = '/Users/adiseredinschi/Hammers/ibc-go/build/simd'
#gaiad_binary = '/Users/adiseredinschi/go//bin/gaiad'
hdpath = ''
home_dir = '$HOME/.gm'
ports_start_at = 27040

[global.hermes]
binary = '/Users/adiseredinschi/Hammers/hermes/target/debug//hermes'
config = '$HOME/.hermes/config.toml'
log_level = 'info'
telemetry_enabled = true
telemetry_host = '127.0.0.1'
telemetry_port = 3001

[ibc-0]
ports_start_at = 27000

[ibc-1]
ports_start_at = 27010
gaiad_binary = '/Users/adiseredinschi/go//bin/gaiad'

[node1]
network = 'ibc-0'
ports_start_at = 27020
add_to_hermes = true

[node2]
network = 'ibc-1'
ports_start_at = 27030
add_to_hermes =true 
gaiad_binary = '/Users/adiseredinschi/go//bin/gaiad'

The important part of gm.toml is we want one network to spawn from gaiad (which has no support yet) while the other network from ibc-go/build/simd (support for the new memo field).

Redo the same steps as for case 1 above.
I noticed the following, which I'll need to double-check if it's the expected behaviour or not:

% ./build/simd query tx 713D692FA7051DA0E89372601BA8A2450C101E3E29223DFBDECDCD3375BE1257 --home ~/.gm/ibc-0 --node http://localhost:27010

- key: packet_ack

  value: '{"error":"cannot unmarshal ICS-20 transfer packet data"}'
- key: packet_ack_hex
  value: 7b226572726f72223a2263616e6e6f7420756e6d61727368616c204943532d3230207472616e73666572207061636b65742064617461227d

On the sending chain, the packet events look as follows:

- key: memo

  value: HighForever
- key: acknowledgement
  value: 'error:"cannot unmarshal ICS-20 transfer packet data" '
- key: error
  value: cannot unmarshal ICS-20 transfer packet data

Update: Confirmed with the IBC-go team that the behaviour highlighted in case 2 is expected.

https://github.com/cosmos/ibc-go/releases/tag/v4.2.0
Token transfers that include a non-empty memo field will fail if the receiver chain is not running a version of ibc-go that also supports the memo field. Please read this blog post for more information.