Refactor accumulator flow#692
Conversation
…ncer acc is updated
|
|
||
| acc = keccak256(abi.encodePacked(beforeAcc, dataHash, delayedAcc)); | ||
| inboxAccs.push(acc); | ||
| if (afterDelayedMessagesRead > delayedBridge.delayedMessageCount()) revert DelayedTooFar(); |
There was a problem hiding this comment.
this check could be in Bridge.enqueueSequencerMessage(...)
| function inboxAccs(uint256 index) external view returns (bytes32) { | ||
| return delayedBridge.sequencerInboxAccs(index); | ||
| } | ||
|
|
||
| function batchCount() external view override returns (uint256) { | ||
| return inboxAccs.length; | ||
| return delayedBridge.sequencerMessageCount(); | ||
| } |
There was a problem hiding this comment.
kept these for backwards compatibility, but we can remove it
| ); | ||
|
|
||
| return | ||
| addMessageToDelayedAccumulator( |
There was a problem hiding this comment.
We'll need to emit the message data in an inbox-like event somehow. Maybe it'd be better for the sequencer inbox to still submit the batch spending report?
There was a problem hiding this comment.
pushed a change for the sequencer inbox to do so
There was a problem hiding this comment.
chose to add a different entrypoint instead of allowing the sequencer inbox to call enqueueDelayedMessage to avoid gas overhead of an extra sload in either every delayed inbox or every sequencer inbox call
| frame.rollupEventInbox.initialize(address(frame.bridge), rollup); | ||
| frame.outbox.initialize(rollup, IBridge(frame.bridge)); | ||
|
|
||
| frame.delayedBridge.setInbox(address(frame.inbox), true); |
There was a problem hiding this comment.
this is now done inside the rollup admin's init
connectedContracts.bridge.setDelayedInbox(address(connectedContracts.inbox), true);
Codecov Report
@@ Coverage Diff @@
## redo-devnet-reinit-1 #692 +/- ##
========================================================
- Coverage 51.98% 51.97% -0.02%
========================================================
Files 224 224
Lines 26433 26467 +34
Branches 489 489
========================================================
+ Hits 13741 13755 +14
- Misses 11244 11264 +20
Partials 1448 1448 |
PlasmaPower
left a comment
PlasmaPower
left a comment
There was a problem hiding this comment.
Mostly LGTM, just a couple recommended changes
3 participants
This refactors the flow of the contracts and where message accumulators are stored.
This makes it so the Sequencer Inbox behaves more similarly to the Delayed Inbox.
Accumulators for both are stored in the Bridge contract.
This also include a couple other PRs merged into it: