-
Notifications
You must be signed in to change notification settings - Fork 651
[MEL] - Implement delayed message accumulation in native mode #3389
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
base: master
Are you sure you want to change the base?
Conversation
| ) | ||
|
|
||
| // InitializeDelayedMessageBacklog is to be only called by the Start fsm step of MEL. This function fills the backlog based on the seen and read count from the given mel state | ||
| func InitializeDelayedMessageBacklog(ctx context.Context, d *mel.DelayedMessageBacklog, db *Database, state *mel.State, finalizedAndReadIndexFetcher func(context.Context) (uint64, error)) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If only called by the FSM, why is it a public method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is used in system_tests later on, we can track this in linear to maybe clean it up later on?
| var err error | ||
| delayedMsgIndexToParentChainBlockNum := make(map[uint64]uint64) | ||
| curr := state | ||
| for i := state.ParentChainBlockNumber - 1; i > 0; i-- { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In prod, this will go all the way back to block num 0. Is that intended behavior? What if this is arb1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this loop rewinds and finds corresponding mel states for parent chain block numbers, so even assuming l2 chain genesis was seen in block=1 of parent chain, this loop should be able to find the melstate corresponding to that.
For arb1, the upgrade process is a bit different- I would prefer our first mel state to have DelayedMessagedSeen = DelayedMessagesRead and do the upgrade after the parent chain block containing last batch is finalized to prevent complexities with reorg.
| func (d *DelayedMessageBacklog) setInitMsg(msg *DelayedInboxMessage) { d.initMessage = msg } | ||
|
|
||
| // clear removes from backlog (if exceeds capacity) the entries that correspond to the delayed messages that are both READ and belong to finalized parent chain blocks | ||
| func (d *DelayedMessageBacklog) clear() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is more idiomatic for clear to take in a context rather than storing a context in the struct, IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had thought a lot about this, the ctx would have to trickle down from AccumulateDelayedMessage function of the state itself! I didnt want that method to take in context solely to trim entries from the backlog- this should be the responsibility of backlog alone. We could make the backlog a stopwaiter, but it seemed overkill
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3389 +/- ##
==========================================
- Coverage 22.67% 22.43% -0.25%
==========================================
Files 383 390 +7
Lines 58109 59058 +949
==========================================
+ Hits 13177 13250 +73
- Misses 42903 43767 +864
- Partials 2029 2041 +12 🚀 New features to boost your workflow:
|
Message extraction function works with logs instead of receipts
This PR implements delayed message accumulation logic for native mode of MEL. We introduce a new data structure
DelayedMessageBacklogto enable validation of seen but not-yet-read delayed messages against the current merkle root formed by accumulating all the delayed messages seen by MEL up until now.For optimization, future messages that are prefetched for generating the merkle root are also pre-validated (or pre-read) i.e we don't re-validate them again when we read them in the future, instead we check that the pre-validated message's hash matches the hash of delayed message we fetch from the DB using the same delayed message index.
This PR also brings in necessary changes needed to other parts of MEL code to successfully test it.