-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[BlockSTM] Per-block Gas Limit #7488
Conversation
…os-core into daniel-PE-module-fallback
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.
Current semantics seems that we skip the rest of the block once accumulated gas exceeds the limit, but include the first transaction that exceeded (made it >= to be precise). Is this what we want, or should we include the prefix of txns such that the total consumed gas is <limit? (or <=, but I guess < is fine here).
@@ -390,6 +435,14 @@ where | |||
if must_skip { |
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.
could merge this check with must_skip || accumulated_gas >= PER_BLOCK_GAS_LIMIT?
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.
Actually need to separate them to distinguish reconfiguration from exceeding the per-block gas limit.
Do we have a (public) design doc on this topic? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
} else { | ||
unreachable!(); | ||
} | ||
drop(lock); |
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.
ugly
…re into daniel-per-block-gas
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|
Description
This PR supports early halting parallel execution (BlockSTM), for scenarios such as exceeding per-block gas limit, module publishing read / write conflicts, reconfiguration and VM internal error. For per-block gas limit, a new on-chain config is added for compatibility.
The PR builds upon rolling commit, which allows the BlockSTM to keep track of committed transaction prefix, and therefore compute the accumulated gas of committed transactions.
Once the parallel execution needs to be early halted, one thread will be in charge of waking up any pending thread (on dependency reads) and mark the execution status of each transaction properly (a new status named ExecutionHalted is introduced).
Fixed related tests on parallel executor and executor. Added on-chain config for compatibility. Passed forge test with block gas limits. Additional monitoring metrics are added in the Execution dashboard with row named "Execution Per Block Gas".
TODO:
Test Plan
Existing and new unit / prop tests on parallel executor and executor with block gas limit on and off.
Smoke test related to execution / storage.
Forge performance test and forge compatibility test.
Will add additional tests if necessary.