Use a tag/discriminant to distinguish between IBC NFT transfers and fungible token transfers

[murisi]

Parsing IBC transfers is a task that was completed in the context of #3494. Right now it seems that the best way for a hardware wallet to parse a TX_IBC_WASM transaction is to attempt to parse it as a MsgTransfer, and if that fails then to attempt to parse it as a MsgNftTransfer. Is this the case? If so, then I guess the assumption here is that there is no byte sequence that can be successfully interpreted as both a MsgTransfer and a MsgNftTransfer? If so, then it might be simpler/more robust to just use two separate WASMs for these two transaction types, or use the same WASM and embed in the Data section a tagged enumeration instance that can either be inhabited by a MsgTransfer or MsgNftTransfer. In this way, the errors reported by a parser could also be more precise.

[murisi]

Another separate issue on the topic of IBC (N)FT Transfer (de)serialization: While most of the Tx data structure is serialized using Borsh, MsgTransfers and MsgNftTransfers are encoded by first Protobuf encoding these structures into a byte vector, and then Borsh serializing that vector. While this is perfectly fine, it might be more consistent (for the transaction schema and parser implementers) to just directly Borsh serialize MsgTransfers and MsgNftTransfers since both these structures implement Borsh (de)serialization. The drawback of doing this is that the borsh feature of the IBC crate would need to be enabled, and this would need to use its own older copy of the Borsh crate.

[Fraccaman]

why is this breaking the tx format? I think this is only breaking the wasm payload deserialization for the IBC txs no?

[murisi]

why is this breaking the tx format? I think this is only breaking the wasm payload deserialization for the IBC txs no?

My mistake here. You are correct. I've removed the breaking: tx format label.

[yito88]

As you mentioned, currently, an IBC transaction can have a message for all IBC operations not only for IBC transfers.
We could separate the WASM to at least 3 wasms for two transfer messages and one envelope and serialize these messages with Borsh (They are still serialized with protobuf because old ibc-rs couldn't serialize them with borsh).

[cwgoes]

@murisi Is this still required, or did ZondaX find a workaround?

[murisi]

@murisi Is this still required, or did ZondaX find a workaround?

Zondax found a workaround. There's a certain byte they check in the Protobuf encoding. See: https://github.com/Zondax/ledger-namada/blob/6e0ac0f674ee3f9429244c7089fa31335e5f80af/app/src/parser_impl_txn.c#L853 .

[cwgoes]

Thanks - closing for now then.