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

feat: delay buffer #160

Closed
wants to merge 52 commits into from
Closed

feat: delay buffer #160

wants to merge 52 commits into from

Conversation

shotaronowhere
Copy link
Contributor

@shotaronowhere shotaronowhere commented Mar 18, 2024

Introduces a delay buffer to limit repeated sequencer censorship enabling optimistic mechanisms (eg interactive L3 fraud proofs) on L2s without unreasonable delay. The delay buffer is represented by a struct stored in the SequencerInbox, and managed with the internal DelayBuffer library.

There are 2 ways to post batches.

  1. No Proof

No changes to existing batch posting function selectors. This method of batch posting is permissible when the sequencer inbox is in a synced state OR if no new delayed messages are read. In the former case, when the sequencer is including delay messages without unexpected delay, there is a minimum amount of time during which no unexpected delays are possible, and hence the sequencer can post batches without additional delay buffer related logic. Eg. if the delay threshold is 2 hours, and the sequencer proves the most recent sequenced delayed message is delayed 30 min, then the margin of 1 hr 30 min defines a sync period during which it is not possible to have unexpectedly delayed messages. In the later case, when no new delayed messages are read, no buffer state updates can be calculated since the elapsed time in the delayed message queue is zero.

  1. Delay Proof

When the sequencer reads atleast one new delayed message, it can include a pre-image of the first delayed message read in the batch of the following form

        bytes32 beforeDelayedAcc,
        Messages.Message calldata delayedMessage

in order to calculate any delays and apply buffer updates as well as establish a sync state.


The batch posting strategy should include delayProofs if new delayed messages are read, otherwise, the original batch posting method without delay proof is sufficient.

@cla-bot cla-bot bot added the s label Mar 18, 2024
src/bridge/DelayBuffer.sol Fixed Show resolved Hide resolved
Copy link
Contributor

@godzillaba godzillaba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so far it looks good, i need to explore the non happy sync path further though.

i just have some very small suggestions for now, feel free to ignore them if you don't agree with namings and things :)

src/bridge/SequencerInbox.sol Outdated Show resolved Hide resolved
src/bridge/SequencerInbox.sol Outdated Show resolved Hide resolved
src/bridge/DelayBuffer.sol Outdated Show resolved Hide resolved
src/bridge/DelayBuffer.sol Outdated Show resolved Hide resolved
src/bridge/DelayBuffer.sol Outdated Show resolved Hide resolved
godzillaba
godzillaba previously approved these changes Mar 28, 2024
src/bridge/DelayBuffer.sol Outdated Show resolved Hide resolved
@godzillaba
Copy link
Contributor

looks good!

src/bridge/SequencerInbox.sol Outdated Show resolved Hide resolved
src/rollup/BridgeCreator.sol Outdated Show resolved Hide resolved
scripts/deploymentUtils.ts Outdated Show resolved Hide resolved
src/bridge/DelayBufferTypes.sol Outdated Show resolved Hide resolved
@gzeoneth gzeoneth self-requested a review April 16, 2024 14:30
Copy link
Member

@gzeoneth gzeoneth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some nit about naming and documentation
Great work!

src/bridge/DelayBuffer.sol Outdated Show resolved Hide resolved
src/bridge/DelayBuffer.sol Show resolved Hide resolved
src/bridge/DelayBuffer.sol Outdated Show resolved Hide resolved
godzillaba
godzillaba previously approved these changes May 2, 2024
@godzillaba godzillaba self-requested a review May 2, 2024 20:00
@gzeoneth
Copy link
Member

gzeoneth commented May 7, 2024

this will be merged as part of bold
OffchainLabs/bold#617

@gzeoneth
Copy link
Member

gzeoneth commented Jul 1, 2024

Shipped with #193

@gzeoneth gzeoneth closed this Jul 1, 2024
@gzeoneth gzeoneth deleted the delay-buffer branch August 9, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants