Interval Sealing support for engine_createBlock
#1745
Conversation
node/service/src/block_sealing.rs
Outdated
| if sealing_mode != SealingMode::Interval || | ||
| sealing_create_empty_blocks || | ||
| pool.ready().count() > 0 | ||
| // To keep the logs quieter, don't even attempt to create empty blocks unless there are transactions in the pool | ||
| if create_empty || pool.ready().count() > 0 |
There was a problem hiding this comment.
This is the important logic change!
There was a problem hiding this comment.
How is this different from specifying to not create empty blocks?
There was a problem hiding this comment.
The if statement will make it not call the request to make a non-empty block which otherwise would throw an error log.
If I recall that's the original reason for the original if statement, but it only applied in interval sealing mode.
| --dev \ | ||
| -lruntime=debug \ | ||
| --sealing=interval \ | ||
| --sealing-interval=1 \ |
There was a problem hiding this comment.
Just a duplicate of the other one with the one second time set.
wilwade
force-pushed
the
chore/enable-engine-create-block-interval-sealing
branch
from
7b15710 to
aacdcc3
Compare
Codecov Report
@@ Coverage Diff @@
## main #1745 +/- ##
==========================================
+ Coverage 87.95% 88.07% +0.11%
==========================================
Files 50 50
Lines 4243 4243
==========================================
+ Hits 3732 3737 +5
+ Misses 511 506 -5
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Can you please add clarification to the commit message? I am a little unclear on what problem we are trying to solve. |
I'm not sure how to make it more clear than supporting the engine_createBlock rpc in the interval sealing mode. Ideas? |
We did not have it before why do we need it now? What triggers this to be needed? Is this a bug? Are we adding a new test that requires this for some test we plan to add in the future? |
Ah:
Or to put another way, I expected to be able to run the e2e tests against interval sealing and was surprised when it didn't work. |
That was helpful. Thanks! |
There was a problem hiding this comment.
Looks good!
I think our current interval-mode does not capture all the scenarios that can happen when submitting an extrinsic since it starts an interval with submission. Ideally we would like an independent block interval that would start independently and only create a block (if any extrinsic inside). this would allow us to be able to get more realistic result let's say for extrinsic that got submitted close to the end of current interval and they might get included in the next one and ...
Goal
The goal of this PR is enable
engine_createBlockfor Interval Sealing mode.Effectively it uses the should make empty block coming down the command instead of making a special exception for interval sealing.
Note: This does reduce the logs in other modes when
engine_createBlockis called with not making an empty block and no transactions are in the pool.Test
make start-interval(or the new 1 second make commandmake start-interval-1)make local-blocknow works (it will would before this PR)