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

delayedack should validate ibc client state against latest rollapp committed state #874

Open
omritoptix opened this issue May 10, 2024 · 5 comments
Assignees

Comments

@omritoptix
Copy link
Contributor

omritoptix commented May 10, 2024

Besides given us the gurantee that we only accept packets after a sequencer committed to it on the hub (i.e non-trusted sequencer assumption), it also protects us from a potential failure (non-malicious) where ibc transferes from rollapp to hub are processed before state committed and the blocks could potentially get lost. Assume the following scenario:

  1. Sequencer writes block 100, 101
  2. Block 100 has ibc tx
  3. Sequencer doesn’t submit block 100,101 to DA or gossips them and blocks are deleted (due to bug)
  4. Relayer relies block 101 with proof to the hub
  5. Block 100,101 are accidentally deleted before sequencer managed to gossip or write to DA (due to bug)

the "hacky" solution for now is just to make sure relayers are only connected to full nodes (and not to the sequencer) so at least we know the block was already gossiped to the network.

Long term solution (besides the obvious maliciuos behavior a sequencer can perform) would be to validate against latest committed state of the sequencer.

This has the obvious downside of making ibc transfers wait until the sequencer committed the state which could take an order of dozens of seconds.

@omritoptix
Copy link
Contributor Author

omritoptix commented Jun 2, 2024

As discussed with @yishay-dym , we're gonna handle it optimistically to not hurt UX.
so basically the general idea is to accept the headers optimistically and in case of mismatch after the fact handle it.

Will probably require ADR.

@omritoptix omritoptix assigned danwt and unassigned danwt Jun 9, 2024
@mtsitrin
Copy link
Contributor

trying to reprhase the issue:

  • while in the "weak L2 consensus", before submitting the blocks to the hub
  • how should we treat ibc txs in this time, as we can't know for sure the sequencer will commit the correct state and not some forked state / unverifiable state

IMO this issue is solved by design.
if the state is syncable on the L2, the market maker will fulfill the transfer, even before state update.
once state update is posted, the full nodes consensus moves from "weak L2 consensus" to "L1 committed".
if the state update will be different than expected, a fraud proof can be submitted, and the rollapp state is expected to be restored

@danwt
Copy link
Contributor

danwt commented Jun 10, 2024

Not sure I got you @mtsitrin

the state update will be different than expected, a fraud proof can be submitted, and the rollapp state is expected to be restored
you mean if the state is fraudulent for a given set of TX's (wrong STF). But what if the TX's are different?

Are you trying to say it's already solved by eibc? In this way, the fulfillers absorb risk?

@mtsitrin
Copy link
Contributor

I suggested that we can detect the invalid ibc state even before the sequencer state update.
By the full nodes themselves.
When a malicious IBC state update posted on the hub, the full nodes can detect the fraud.
The L2 doesn't need to wait for the sequencer state update to be posted on the hub

@omritoptix
Copy link
Contributor Author

IMO this issue is solved by design.

for the record, as discussed over a call, this is not solved by design as it's not eibc related but the ability of the sequencer to submit invalid ibc headers for specific heights which is not verified against the state update.

I suggested that we can detect the invalid ibc state even before the sequencer state update.
By the full nodes themselves.

This is a possible solution but I think it has a few drawbacks in comparison to verify it on the hub:

  1. it adds another layer of optimism (vs hub validates and rejects)
  2. Requires gov proposal currently and in the future Hub logic anyway requires to validate it (so this logic should exist either way)
  3. has more complicated implementation imo
    i. the ibc headers can come after the message update state for a given height (so you may need to save in dymint now historical ibc headers/state updates, where in the hub you have the historical consensus states saved already)
    ii. maintenance of this will require maintenance across various rollapp versions (vs one version on the hub)

the only pro I see in this appraoch is that you save time in the case where state update comes after the ibc header. however as this time is very negligible (i.e usually 1 min in happy case + currently requires gov proposal, and in any other case if the sequencer doesn't send msgUpdate anyway no eibc will be approved or any related transfers)

In general I think in that case the "optimistic" solution doesn't provide significant value over the "pessimistic" but actually makes the system more complex. IMO we should be pessimistic as possible where we can, assuming it doesn't create a substantial overhead.

With that being said, a more through research should be done on the optimal solution and trade-offs.

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

No branches or pull requests

3 participants