Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
zmanian opened this issue Apr 15, 2024 · 4 comments

Comments

@zmanian
Copy link
Member

zmanian commented Apr 15, 2024

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.

@bpiv400
Copy link

bpiv400 commented Apr 15, 2024

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

@hdevalence
Copy link

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.

@crodriguezvega
Copy link
Contributor

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.

@srdtrk
Copy link
Member

srdtrk commented Apr 16, 2024

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..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants