-
Notifications
You must be signed in to change notification settings - Fork 151
feat(builder): resource metering for transaction time limits #534
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
Open
niran
wants to merge
7
commits into
main
Choose a base branch
from
feature/resource-metering-builder
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
🟡 Heimdall Review Status
|
Refactor is_tx_over_limits to use structured ResourceLimits and TxResources parameters, and add execution time tracking and enforcement: - Add ResourceLimits struct to encapsulate all limit parameters - Add TxResources struct for transaction resource consumption values - Add cumulative_execution_time_us field to ExecutionInfo - Add TransactionExecutionTimeExceeded and BlockExecutionTimeExceeded error variants to TxnExecutionResult - Use metering data from TxDataStore in execute_best_transactions - Track actual execution time post-simulation when metering unavailable - Add execution time per-batch tracking for flashblocks - Add CLI flags for execution time limits: - --builder.max-execution-time-per-tx-us - --builder.block-execution-time-budget-us The metering data (MeterBundleResponse.total_execution_time_us) is now used to predict transaction execution time before simulation. This enables rejection of expensive transactions that would exceed configured time budgets, helping ensure blocks complete within their allotted time.
Resource metering now tracks both execution time and state root time: - Execution time: "use it or lose it" per flashblock - budgets reset at the start of each flashblock and don't carry over - State root time: cumulative across the block - since state root is calculated once at the end, this resource accumulates like gas/DA New CLI flags: - --builder.max-state-root-time-per-tx-us: per-tx state root time limit - --builder.block-state-root-time-budget-us: block-level cumulative limit The metering data from MeterBundleResponse includes both total_execution_time_us and state_root_time_us fields which are now used for pre-simulation resource prediction.
Add 22 unit tests covering: - Basic limit checks (gas, DA limits) - Transaction and flashblock execution time limits - Transaction and block state root time limits - Reset behavior (execution time resets per flashblock, state root time persists across flashblocks) - Edge cases (zero limits, overflow prevention, missing metering data) - Combined resource limit scenarios
Combine enable_resource_metering and resource_metering_observe_only flags into a single ResourceMeteringMode enum with three modes: - off: disabled (default) - observe: collect metrics without enforcing time limits - enforce: collect metrics and reject transactions exceeding limits Add observation metrics for monitoring metering accuracy: - Rejection counters for each limit type - Execution time prediction error (predicted vs actual) - State root time / gas ratio for anomaly detection - Distribution histograms for predicted and actual times
Replace "observe" terminology with "dry-run" throughout resource metering to use more standard industry terminology for non-enforcement mode.
Separate resource metering limits (execution time, state root time) from protocol-enforced limits (gas, DA) at the type level. This makes the distinction explicit and simplifies the dry-run mode handling in the payload builder context. The new enum wraps all time-based limit violations, making pattern matching cleaner and eliminating the need for matches! with a long list of variants.
Remove redundant inline comments and improve field documentation: - Simplify "Total gas left" to "Cumulative gas target" - Remove "use it or lose it" descriptions (behavior is documented in ResourceLimits) - Add TODO for backrun bundle metering data consideration
cde0759 to
fbd169d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
ResourceMeteringModeenum with three modes:off,dry-run,enforcebase_meterBundleRPC viaTxDataStoreChanges
New CLI Flag
Resource Limits
Metrics
metering_known_transaction/metering_unknown_transactioncounterstx_execution_time_exceeded,flashblock_execution_time_exceeded,tx_state_root_time_exceeded,block_state_root_time_exceededTest Plan