Make the sender set MASP transaction to IBC memo

[yito88]

Describe your changes

closes #3438

WARNING: don't merge this PR for now because tx_ibc.wasm failed to be compiled on my Apple silicon machine.

Transaction runner error: Wasm compilation error: Compilation error: Assembler failed finalization with: ImpossibleRelocation(Dynamic(DynamicLabel(0)))

We could workaround this error.

Indicate on which release or other PRs this topic is based on

#3422
Diff: 5c3b3ab...ca79554

Checklist before merging to draft

• I have added a changelog
• Git history is in acceptable state

[grarco]

This is used to generate both the refunding tx and the actual shielding transfer on the destination chain, correct? If that's the case maybe we can change the message here

[yito88]

Thanks, Hermes used the argument. Now build_ibc_transfer generates the masp tx for the refunding.
Users don't have to generate it with this command anymore.
I'll remove this argument.

[grarco]

Same here as in the client

[grarco]

Suggested change

	Error::Other("Missing expected masp transaction".to_string())
	Error::Other("Missing transaction data".to_string())

[grarco]

Suggested change

	"IBC meesage doesn't have masp transaction".to_string(),
	"IBC message doesn't have masp transaction".to_string(),

[grarco]

I was thinking that instead of IbcShielding perhaps we could just expand the MaspAction to be an enum like:

enum MaspAction {
    TxSection(Hash),
    InnerTxCmt(Hash)
}

The first case is the one we already have and its hash is the hash of the tx's Masp section. The second case is an alternative to IbcShielding and its hash is that of the TxCommitments (i.e. the inner tx hash). The ibc transaction could write the second variant when the tx is an incoming one. In the SDK then, we can look at the specific inner txs of the batch that contain a masp ibc tx and extract only their memos without looking at all of them.

This is a minor suggestion, feel free to ignore it

[yito88]

Thanks, it looks better. However, the change for setting Hash into the action caused wasm compilation error on arm mac. Let's leave it as it is.

[grarco]

Minor comments but looks good to me, thanks!

[grarco]

Actually, I believe there's an issue, my bad for not spotting it earlier. In the sdk we use extract_masp_tx_from_ibc_message when we have a valid masp tx which is missing the masp ref event. In this case we iterate over all the inner txs of the batch and try to extract the ibc message from the tx data and if we can we push the masp Transaction to the result.

The issue is that we don't know if that specific inner tx was successful: if it wasn't, the client (or indexer) extracts information from a wrong source and spoils the internal state of the ShieldedContext, which might prevent it from constructing valid transactions in the future.

We need a way to tell, in the events we emit, which transactions of the batch are actually valid and filter only these ones in extract_masp_tx_from_ibc_message

[grarco]

Actually, I believe there's an issue, my bad for not spotting it earlier. In the sdk we use extract_masp_tx_from_ibc_message when we have a valid masp tx which is missing the masp ref event. In this case we iterate over all the inner txs of the batch and try to extract the ibc message from the tx data and if we can we push the masp Transaction to the result.

The issue is that we don't know if that specific inner tx was successful: if it wasn't, the client (or indexer) extracts information from a wrong source and spoils the internal state of the ShieldedContext, which might prevent it from constructing valid transactions in the future.

We need a way to tell, in the events we emit, which transactions of the batch are actually valid and filter only these ones in extract_masp_tx_from_ibc_message

Opened #3488 to track this separately