Speedup RedundantRelayDecorator for Check and RecheckTx

[ValarDragon]

Summary

The RedundantRelayDecorator is a very significant slowdown to CheckTx and RecheckTx, recheckTx being more important due to how consensus-blocking this operation is:

This is a serious chain slowdown vector for all cosmos chains.

Problem Definition

This is a serious slowdown to Consensus due to the Recheck behavior of Comet, but is also a notable extra strain on mempool ingest.

Proposal

Remember that RedundantRelay is something thats there purely to help benefit altruistic relayers. So we do not need to ensure every packet being relayed will pass, just that if the relay will contribute nothing of help, skip it.

Here is the logic: https://github.com/cosmos/ibc-go/blob/main/modules/core/ante/ante.go#L24

We need to make direct logic to ensure:

• Only check ClientUpdate on CheckTx, not RecheckTx
• Every other operation should just do a state read to see if the respective PacketId is already handled. And if its not handled, do a write. It should not do the full logic, as that has notable expenses.

This is important for reducing mempool overhead, and the critical consensus delay that IBC txs in the mempool induce.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned

[ValarDragon]

Here is the pprof for Osmosis mainnet, restricted to calls that block consensus due to the mempool behavior right now:

[damiannolan]

do all validator nodes run this ante handler? should be unnecessary, right?

I suppose all node operators take whatever binary is distributed as is.

[ValarDragon]

All validators do run this ante handler. There isn't a way to express not running this on a validator atm.

But the redundant relay guarantee is something you'd want on the block proposer

[colin-axner]

Thanks @ValarDragon! I agree with the proposed solution

@damiannolan and I discussed the issue and came up with some extra details and notes

#3636 needs to be fixed, otherwise the current proposed pr would block submission of misbehaviour

We will split up ante handler logic for check tx and recheck tx to minimize necessary logic.

CheckTx should:

• check proofs
• check redundancy
• update state

RecheckTx should:

• check redundancy
• update state

That means, we can exclude (from checktx and rechecktx):

• pruning
• misbehaviour
• app callbacks

In rechecktx we can exclude proof checks. We will do this by only executing necessary logic, whereas checktx will make explicit exclusions in the code. MsgUpdateClient should only call UpdateState and MsgRecvPacket should only execute the following code

Action Items:

• fix: noop on UpdateState for invalid misbehaviour #6276 (requires minor release)
• Add exclusion logic for app callbacks, pruning (perf: exclude pruning from tendermint update client in ante handler execution #6278) (patch release okay)
• Update CheckTx UpdateClient handler to only run VerifyClientMessage + UpdateState (imp: add updateClientCheckTx to redunant relayer ante decorator #6279) (requires fix which is state breaking)
• Add rechecktx handler to remove proof checking on recvpacket and update client (perf: minimize logic on rechecktx for recvpacket #6280) (updateclient component requires fix which is state breaking)

note: we can do a non breaking adjustment to the patch releases for the update client fix if necessary

We will leave the other msg types as is for now, to focus on getting the update client and recv packet dealt with first. We can update the others in the future as necessary

[colin-axner]

@ValarDragon is there a specific patch release you would want these changes in?