ibc-go icon indicating copy to clipboard operation
ibc-go copied to clipboard

Request for the applications specific tx ordering behavior from the comet BFT mempool working group

Open zmanian opened this issue 1 year ago • 4 comments

In all current released versions of Comet BFT, the default proposer is semi-fifo. The proposer will propose txs in approximately the order broadcast but "actually the order received" until one of the system bottle necks like the ABCI mutex load, P2P bandwidth capacity or mempool size is hit and then basically nothing is guaranteed.

The semi FIFO property is going to be difficult to maintain as we try to get to a more robust design.

Right now IBC relayers depend on the semi FIFO property to reliably operate.

Newer features of the Cosmos SDK in 0.50+ would change things from relying on non specified to relaying on specific features of IBC-go. Specifically, IBC-go can hook into prepare block and improve relayer reliability under load.

The issues we are aware of are

  • duplicate client updates
  • Duplicate packets
  • in order channels like ICA.

A lot of this could be handled with prepare proposal.

The proposer could mark duplicates unexecutable and refund gas.

A collection of @ValarDragon ideas below.

"The sequence numbers are today solved via complex Authz hacks.

We can also solve them via unordered txs."

"Duplicate client updates and packet relays themselves have problems in gas charged to the relayer.

We multi-message these right now. I see two options:

  • Make it not a multi-message and make a system for queing packet relays for a client height not yet seen.
  • Can be pushed to a thin smart contract/new go module
  • Make a system in the state machine for putting gas bounds per tx, and if its "duplicate detected", not charge that gas to the client or count it towards block gas limits."

Prepare proposal can also check that all ordered channel packets are being deliver in order.

zmanian avatar Apr 15 '24 12:04 zmanian

These problems feel like they could be solved (along with other related problems) by an IBC lane in the BlockSDK

Basic behavior could be something like:

  1. Reserve a small % of block space for IBC client updates
  2. Throw out client updates that have already been committed
  3. Potentially refund gas for client updates that haven't been seen before (for a configurable set of clients)

(This is off the top of my head very quickly. We could come up with more intelligent versions of this)

There are a lot of variations of this we've discussed internally

bpiv400 avatar Apr 15 '24 13:04 bpiv400

It seems like there are two possible approaches here:

  1. Make IBC relaying more consensus-aware: provide mechanisms to tie IBC relaying and block construction together.
  2. Make IBC relaying less consensus-aware: change the IBC implementation to have less dependence on tx ordering, so that tying into block construction is not necessary.

An IBC lane in the BlockSDK is an example of (1).

I think it would be worthwhile to pursue (2) rather than (1) if possible, because it preserves the ability to use the IBC host implementation with a different consensus stack. For instance, what if someone wants to use IBC in a context where PrepareProposal doesn't exist? There's a danger in tying IBC more and more deeply to the current state of Comet.

For example, why can't IBC-go correctly handle duplicate client updates without any consensus integration at all? Penumbra just does the right thing, no PrepareProposal necessary.

hdevalence avatar Apr 15 '24 16:04 hdevalence

Thanks for opening the issue and the comments.

There's a danger in tying IBC more and more deeply to the current state of Comet.

Agree. We are planning to explore how to decouple ibc-go from SDK/CometBFT and make it a generic library that can be plugged in with various frameworks/ecosystems via adapter layers. Thus solutions that don't tidy ibc-go even more with CometBFT would be preferred.

why can't IBC-go correctly handle duplicate client updates without any consensus integration at all?

We actually do! In 07-tendermint we check for a duplicate update and no-op if that's the case.

crodriguezvega avatar Apr 15 '24 18:04 crodriguezvega

Yes, like @crodriguezvega mentioned and @hdevalence suggested, we currently do a no-op for duplicate updates. The reason why we don't return so early (like penumbra) is because we do a misbehavior check first.

I think there might be some issues with using PrepareProposal to ignore duplicate client updates:

  1. How can we do the misbehavior check? As this action should consume some gas imo.
  2. Relayers don't often include MsgUpdateClient in a standalone tx. Instead, they combine it with MsgRecvPacket (Timeout/Ack) (see example). And since PrepareProposal cannot breakdown transactions into individual messages, it is not clear how to handle a scenario where MsgUpdateClient is redundant but MsgRecvPacket is not.

I'd also like to understand why Right now IBC relayers depend on the semi FIFO property to reliably operate..

srdtrk avatar Apr 16 '24 07:04 srdtrk