-
Notifications
You must be signed in to change notification settings - Fork 174
perf(l1): disable balance check for prewarming to avoid early reverts #6259
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -41,6 +41,9 @@ pub struct Environment { | |||||||||||||||||||||||
| pub block_gas_limit: u64, | ||||||||||||||||||||||||
| pub is_privileged: bool, | ||||||||||||||||||||||||
| pub fee_token: Option<Address>, | ||||||||||||||||||||||||
| /// When true, skip balance deduction in `deduct_caller`. Used by the prewarmer | ||||||||||||||||||||||||
| /// to avoid early reverts on insufficient balance so that warming touches more storage. | ||||||||||||||||||||||||
|
Comment on lines
+44
to
+45
|
||||||||||||||||||||||||
| /// When true, skip balance deduction in `deduct_caller`. Used by the prewarmer | |
| /// to avoid early reverts on insufficient balance so that warming touches more storage. | |
| /// When true, skip balance deduction in `deduct_caller`. | |
| /// | |
| /// This flag is intended **only** for the prewarmer to avoid early reverts on | |
| /// insufficient balance so that cache warming can touch more storage/state. | |
| /// | |
| /// Any execution performed with this flag set to `true` must have its results | |
| /// discarded and MUST NOT be used for actual transaction execution, validation, | |
| /// or consensus-critical state transitions. This flag MUST NEVER be set to | |
| /// `true` in normal transaction processing; it is for cache warming only. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -488,6 +488,10 @@ pub fn validate_gas_allowance(vm: &mut VM<'_>) -> Result<(), TxValidationError> | |
| } | ||
|
|
||
| pub fn validate_sender_balance(vm: &mut VM<'_>, sender_balance: U256) -> Result<(), VMError> { | ||
| if vm.env.disable_balance_check { | ||
| return Ok(()); | ||
| } | ||
|
Comment on lines
490
to
+493
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The L2 hook's Prompt To Fix With AIThis is a comment left during a code review.
Path: crates/vm/levm/src/hooks/default_hook.rs
Line: 490-493
Comment:
The L2 hook's `deduct_caller_fee_token` function (called from L2 transaction validation) doesn't check `disable_balance_check`, which means L2 transactions with fee tokens will still revert early during prewarming. Consider applying the same pattern to `l2_hook::deduct_caller_fee_token` at crates/vm/levm/src/hooks/l2_hook.rs:557
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| // Up front cost is the maximum amount of wei that a user is willing to pay for. Gaslimit * gasprice + value + blob_gas_cost | ||
| let value = vm.current_call_frame.msg_value; | ||
|
|
||
|
|
@@ -523,6 +527,10 @@ pub fn deduct_caller( | |
| gas_limit_price_product: U256, | ||
| sender_address: Address, | ||
| ) -> Result<(), VMError> { | ||
| if vm.env.disable_balance_check { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| // Up front cost is the maximum amount of wei that a user is willing to pay for. Gaslimit * gasprice + value + blob_gas_cost | ||
| let value = vm.current_call_frame.msg_value; | ||
|
|
||
|
|
||
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.
nit: bare boolean at the call site (
true/false) doesn't convey intent. Reads a bit better with a/* disable_balance_check */comment or a constant, e.g.Same applies to the
falseat the normal execution call site (line 238).