Skip to content

runtime: resolve compute budget in check_transaction()#9155

Merged
2501babe merged 2 commits into
anza-xyz:masterfrom
2501babe:20251119_earlyerrcbudget
Nov 21, 2025
Merged

runtime: resolve compute budget in check_transaction()#9155
2501babe merged 2 commits into
anza-xyz:masterfrom
2501babe:20251119_earlyerrcbudget

Conversation

@2501babe
Copy link
Copy Markdown
Member

@2501babe 2501babe commented Nov 19, 2025

Problem

in implementing simd290 and simd297 it would be convenient if the only errors we had to handle in svm validate_transaction_nonce_and_fee_payer() were the nonce and feepayer errors because we will have to propagate particular representations for those cases upwards

the only error other than nonce/feepayer is unwrapping Result<SVMTransactionExecutionAndFeeBudgetLimits>, constructed in runtime check_transactions()

Summary of Changes

instead of delivering a malformed compute budget as its own error, transform it into a Result<CheckedTransactionDetails> error in runtime. the only possible errors are in sanitize_and_convert_to_compute_budget_limits(), checking that requested data and heap size values are sane

there are tests in several crates for a DuplicateInstruction error, which is not relevant to this pr. this type of error gets resolved much earlier in RuntimeTransaction<SanitizedVersionedTransaction>::try_from() and never makes it into a batch

@2501babe 2501babe self-assigned this Nov 19, 2025
@2501babe 2501babe force-pushed the 20251119_earlyerrcbudget branch 4 times, most recently from 1f9b2a7 to 429f0c7 Compare November 19, 2025 18:49
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.6%. Comparing base (144249c) to head (bed5ddd).

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #9155     +/-   ##
=========================================
- Coverage    82.6%    82.6%   -0.1%     
=========================================
  Files         890      890             
  Lines      320911   320873     -38     
=========================================
- Hits       265364   265282     -82     
- Misses      55547    55591     +44     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@2501babe 2501babe marked this pull request as ready for review November 19, 2025 19:45
@2501babe 2501babe requested a review from a team as a code owner November 19, 2025 19:45
@LucasSte LucasSte requested a review from buffalojoec November 20, 2025 14:49
Copy link
Copy Markdown

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes lgtm, nice and clean! Just one question about the metrics.

@@ -142,7 +142,7 @@ impl Bank {
raise_cpi_limit,
)
}
});
})?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth incrementing error_counters.invalid_compute_budget here instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesnt exist yet; we create TransactionErrorMetrics in svm load_and_execute_sanitized_transactions()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i never noticed this before, it actually accumulates the svm ones into a parent in runtime. ill make that change, ty!

@2501babe 2501babe force-pushed the 20251119_earlyerrcbudget branch from e78a393 to bed5ddd Compare November 21, 2025 05:44
Copy link
Copy Markdown

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@2501babe 2501babe added this pull request to the merge queue Nov 21, 2025
Merged via the queue into anza-xyz:master with commit abcf518 Nov 21, 2025
47 checks passed
@2501babe 2501babe deleted the 20251119_earlyerrcbudget branch November 21, 2025 06:48
@2501babe 2501babe added the v3.1 Backport to v3.1 branch label Jan 7, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Jan 7, 2026

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

mergify Bot pushed a commit that referenced this pull request Jan 7, 2026
* runtime: resolve compute budget in `check_transaction()`

(cherry picked from commit abcf518)
2501babe added a commit that referenced this pull request Jan 9, 2026
…ort of #9155) (#9848)

runtime: resolve compute budget in `check_transaction()` (#9155)

* runtime: resolve compute budget in `check_transaction()`

(cherry picked from commit abcf518)

Co-authored-by: hana <81144685+2501babe@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v3.1 Backport to v3.1 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants