-
Notifications
You must be signed in to change notification settings - Fork 208
feature: implement consensus Proposer interface #2854
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2854 +/- ##
==========================================
+ Coverage 74.79% 74.88% +0.09%
==========================================
Files 207 208 +1
Lines 22502 22716 +214
==========================================
+ Hits 16831 17012 +181
- Misses 4610 4636 +26
- Partials 1061 1068 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
--------- Co-authored-by: wojo <[email protected]>
4988ef5
to
c140309
Compare
ticker := time.NewTicker(100 * time.Millisecond) //nolint:mnd | ||
defer ticker.Stop() | ||
for { | ||
if !p.builder.MempoolIsEmpty() { | ||
break | ||
} | ||
|
||
select { | ||
case <-ctx.Done(): | ||
return types.BlockInfo{}, false | ||
case <-ticker.C: | ||
} | ||
} |
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.
Why do we need this? I think it's better if we can preemptively build the block info even before receiving transactions.
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.
Yup, that would be ideal. However, imagine the case where block times are 10 minutes, and when we start building the block at minute 0 the mempool is empty. There are two scenarios:
- Over the next ten minutes we don't receive any txns. We timeout, and then propose an empty block.
- At minute 9 we receive a txn. It's only at this point that we can build and propose a non-empty block.
The problem is, between minutes 0 and 9 we don't know if the block will be empty or not, so can't send the blockInfo to our peers at this point.
My proposed solution to this problem, is to periodically check if there are any txns in the mempool to see if we can propose a non-empty block or not. If there are txns in the mempool, then we can propose a non-empty block. If there aren't, then we should check periodically until either 1) we get a txn, and then continue with the non-empty block flow, or 2) we hit our timeout, and propose an empty block.
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.
We could in principle just check at time 0 whether we can txns in the mempool or not, and then decide whether to build an empty block or not. But I think that's bad for UX, since it increases the amount of time on average that users need to wait to have their txn included in a block.
I opened a fresh branch to solve an annoying rebase. #2873 |
This PR implements the
Proposer
interface for the consensus layer. The proposer is responsible for generating block proposals, handling both empty and non-empty blocks depending on whether the mempool receives transactions within a given time window.Notes:
Proposal timing: The caller is responsible for deciding:
* When to stop waiting for transactions and proceed with an empty block.
* When to stop executing transactions (in the non-empty block flow) and continue with proposal finalisation.
Gas strategy: This implementation reuses gas values from the previous block. Dynamic gas fee computation is out of scope of this PR, and probably requires SNs input.
Code reuse: Some logic is shared with the Validator PR. Either PR can be merged first without conflict.