Skip building invariant delta when no invariants are enabled.#5289
Skip building invariant delta when no invariants are enabled.#5289dmkozh wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes parallel Soroban transaction application by skipping construction of the ledger-entry delta used for invariant checking when no invariants are configured/enabled, and renames related APIs to make the delta’s purpose explicit.
Changes:
- Guard delta construction in
TransactionFrame::parallelApplybehind a newConfig::invariantsEnabled()helper. - Rename
TxEffects/parallel-apply delta APIs to clarify they exist specifically for invariants. - Skip
checkAllTxBundleInvariantsentirely when invariants are disabled, and relocate refundable-fee meta setting accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/transactions/TransactionFrame.cpp | Avoids building invariant-only deltas when invariants are disabled. |
| src/transactions/ParallelApplyUtils.h | Renames thread-state helper to reflect invariant-specific delta. |
| src/transactions/ParallelApplyUtils.cpp | Updates delta construction to write into invariant-specific delta storage. |
| src/transactions/ParallelApplyStage.h | Renames TxEffects delta storage/accessors to invariant-specific names. |
| src/main/Config.h | Adds Config::invariantsEnabled() API. |
| src/main/Config.cpp | Implements Config::invariantsEnabled() based on INVARIANT_CHECKS. |
| src/ledger/LedgerManagerImpl.cpp | Skips invariant checks when disabled; updates delta accessor usage; adjusts refundable-fee meta placement. |
The delta is only used for invariants, so also updated the naming to reflect that.
graydon
left a comment
graydon
left a comment
There was a problem hiding this comment.
I'm not clear on one of the changes here, otherwise looks ok?
|
|
||
| // We don't call processPostApply for post v23 transactions at the | ||
| // moment because processPostApply is currently a no-op for those | ||
| // transactions. | ||
|
|
||
| txBundle.getEffects().getMeta().maybeSetRefundableFeeMeta( | ||
| txBundle.getResPayload().getRefundableFeeTracker()); |
There was a problem hiding this comment.
I don't understand why this call moved to the other loop
There was a problem hiding this comment.
maybeSetRefundableFeeMeta was incorrectly placed into checkAllTxBundleInvariants which makes no sense logically. I just cleaned up the code and separated invariant checks and refundable fee meta population. The logic remains the same otherwise (it's the same loop over all txs in stage).
Description
Skip building invariant delta when no invariants are enabled.
The delta is only used for invariants, so also updated the naming to reflect that.
Checklist
clang-formatv8.0.0 (viamake formator the Visual Studio extension)