Create AnteDecorator to reject rendundant relayer messages

[AdityaSripal]

By creating an antedecorator that rejects redundant relayer messages, we can prevent relayers from wasting fees on trying to relay packets that have been relayed by another relayer.

This antedecorator can be chained to the app's Antehandler.

This is a non-breaking change.

cc: @dtribble

[alexanderbez]

Great idea! How would you know if a tx is redundant?

[dtribble]

At a high level, the goal is that it checks whether the incoming commit is for a packet that has already been processed by IBC (e.g., channel/sequencenumber). The details may be more challenging. This should avoid anyone spoofing that they already addressed a packet. It should be about the actual IBC connection state.

[fedekunze]

This would be a great addition

[AdityaSripal]

One thing to note is that this would reject the entire tx.

Imagine if a relayer is trying to relay packets with sequence: 3, 4 with a MultiMsg Tx.

Packet Sequence 3 has already been relayed, but sequence 4 hasn't.

Here the relayer trying to submit the tx, would have the entire tx rejected. They wouldn't pay fees, but they would lose out on being able to submit packet 4. In the time it takes for them to resubmit packet 4, another relayer may have already succeeded.

[adizere]

This sounds like a valuable addition.

I don't understand all the details of what a rejected tx looks like, so could we go into some details there? Specifically, it would be good to have a well-defined Err::Code to represent the notion of rejected. Also, is it possible to return (e.g., as part of the response data or log) the seq nr. or identifiers of the rejected transactions?

[ancazamfir]

Here the relayer trying to submit the tx, would have the entire tx rejected. They wouldn't pay fees, but they would lose out on being able to submit packet 4. In the time it takes for them to resubmit packet 4, another relayer may have already succeeded.

I think we saw this at the osmosis launch

[dtribble]

Here the relayer trying to submit the tx, would have the entire tx rejected. They wouldn't pay fees, but they would lose out on being able to submit packet 4. In the time it takes for them to resubmit packet 4, another relayer may have already succeeded.

I think we saw this at the osmosis launch

Oh that's interesting. Specifically, if a transaction submits 2 packets and one is rejected, is the txn failed? That would make competitive relayers push for one packet per txn regardless of this fix. That's compatible with this proposal. It also sounds like we should further consider quick, low-gas skip of packets that are redundant in txns that are actually processed, without failing the txn.

[AdityaSripal]

It also sounds like we should further consider quick, low-gas skip of packets that are redundant in txns that are actually processed, without failing the txn.

Yup I considered that. Though that would be a breaking change, since a transaction that used to fail will now pass.
We actually do this approach already for redundant UpdateClient messages. Although, unfortunately it won't mean so much in terms of relayer fees, since fees are not refunded if you don't use all your gas. Hence why I wrote up this issue yesterday:

cosmos/cosmos-sdk#9569

If the above can get solved soon, just having a successful low-gas skip is the best option.
If not, we'll have to decide whether rejecting txs outright is worth it. As @dtribble said, the optimal strategy then is to push one packet/tx.

[AdityaSripal]

Specifically, it would be good to have a well-defined Err::Code to represent the notion of rejected.

This has already been implemented by @colin-axner . However, this currently gets returned on DeliverTx instead of CheckTx so relayers still pay fees on the failed tx.

https://github.com/cosmos/ibc-go/blob/main/modules/core/04-channel/keeper/packet.go#L253

https://github.com/cosmos/ibc-go/blob/main/modules/core/04-channel/keeper/packet.go#L483

#184

[AdityaSripal]

I believe given that currently there isn't a way to refund unused fee on successful txs, we should just reject redundant packets.

This will cause the entire tx to fail, and relayers are advised to not have multiple RecvPacket txs for the time being.

Once fees are refunded, we can remove this antedecorator (both adding and removing are non-breaking changes), and turn the behaviour into a successful no-op (this is a breaking change).

[AdityaSripal]

After discussing with relayer teams, supporting multiple receive packets in a single transaction is a priority. To support this, we will only reject a transaction if ALL receive packets are redundant.

If a single transaction is not redundant, then the entire transaction will go through and the relayer pays the entire fee.

A profit-optimizing relayer will still want to send a single packet per tx. However, relayers that send multiple packets per tx can still have their new packets committed on the first attempt.

NOTE: This will be a breaking change.

[colin-axner]

closed via #235