Conversation
While the current logic intertwines reads and writes, in fact it can be cleanly separated into a read-only validation step, and a sequential commit step that simply bumps the sequence numbers and removes pre-authorized tx signers. This is possible because that while the writes change the entries that take part in validation, none of these changes are relevant during the validation. Specifically, sequence number bump is only observable by a single transaction (the one that has the respective account as a source), and the pre-authorized tx signer by definition belongs to a single transaction. There is also a subtle caveat to the latter operation: it increases the available balance of the signer owner (or its sponsor), but since at the pre-apply time the fees have already been charged, we're only checking that the account available balance is non-negative, which is an invariant that must always hold in the current protocol. The change is not protocol-gated because it's not a protocol change for the *current* protocol. It was technically a protocol change prior to p26 where we had a bug that actually did allow overcharging the fee bump source accounts and thus making their available balance to go negative. However, the bug has been fixed without the behavior ever triggering on-chain, and thus this replay-only behavior change should be non-observable. This change significantly speeds up the pre-apply step. On the local high TPL benchmarks I'm getting 30-60ms improvement locally compared to the main branch version. (cherry picked from commit 3e8d70b)
This affects both nominating a new transaction set, and validating the incoming transaction set. Thanks to the fact that most of the time we're either applying the ledger, or validating a transaction set, we can use the efficient CPU-pinned batch executor for this. This speeds up the invalid transaction trimming step by ~20ms on large benchmarks, and in general should increase the transaction validation step proportionally to the number of cores. (cherry picked from commit e154b21)
Contributor
There was a problem hiding this comment.
Pull request overview
Parallelizes transaction-set validation and Soroban pre-apply validation to reduce consensus latency.
Changes:
- Adds reusable range-based batch execution.
- Splits Soroban pre-apply into parallel read-only validation and serial writes.
- Adds concurrency and transaction metadata coverage.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
test-tx-meta-baseline-current/InvokeHostFunctionTests.json |
Adds metadata baselines. |
src/util/BatchExecutor.h |
Adds range execution and preferred task count APIs. |
src/util/BatchExecutor.cpp |
Implements range partitioning and task-count selection. |
src/transactions/TransactionFrameBase.h |
Splits the pre-apply interface. |
src/transactions/TransactionFrame.h |
Declares read-only and write pre-apply paths. |
src/transactions/TransactionFrame.cpp |
Implements split pre-apply processing. |
src/transactions/test/TxEnvelopeTests.cpp |
Supplies network configuration to test ledger views. |
src/transactions/test/TransactionTestFrame.h |
Updates the test-frame interface. |
src/transactions/test/TransactionTestFrame.cpp |
Delegates split pre-apply operations. |
src/transactions/test/SorobanTxTestUtils.cpp |
Fixes footprint deduplication and validation setup. |
src/transactions/test/ParallelApplyTest.cpp |
Tests worker-count-independent pre-apply results. |
src/transactions/test/InvokeHostFunctionTests.cpp |
Adds pre-apply regression coverage. |
src/transactions/ParallelApplyUtils.h |
Declares parallel pre-apply orchestration. |
src/transactions/ParallelApplyUtils.cpp |
Runs validation concurrently and commits writes serially. |
src/transactions/FeeBumpTransactionFrame.h |
Splits fee-bump pre-apply APIs. |
src/transactions/FeeBumpTransactionFrame.cpp |
Implements split fee-bump pre-apply behavior. |
src/ledger/ImmutableLedgerView.h |
Adds Soroban configuration and pre-apply view APIs. |
src/ledger/ImmutableLedgerView.cpp |
Implements immutable pre-apply ledger access. |
src/herder/TxSetUtils.cpp |
Parallelizes transaction validation. |
src/herder/test/TxSetTests.cpp |
Tests validation across worker counts. |
src/herder/test/HerderTests.cpp |
Updates fee-source validation expectations. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+283
to
287
| int64_t& accFee = accountFeeMap[tx->getFeeSourceID()]; | ||
| if (INT64_MAX - accFee < tx->getFullFee()) | ||
| { | ||
| int64_t& accFee = accountFeeMap[tx->getFeeSourceID()]; | ||
| if (INT64_MAX - accFee < tx->getFullFee()) | ||
| { | ||
| accFee = INT64_MAX; | ||
| } | ||
| else | ||
| { | ||
| accFee += tx->getFullFee(); | ||
| } | ||
| accFee = INT64_MAX; | ||
| } |
Comment on lines
+867
to
+871
| // SECTION("txs that fit into the balance are accepted") | ||
| // { | ||
| // auto tx1 = makePayment(feeSource, feeSourceBalance / 2); | ||
| // auto tx2 = makeFeeBump(account1, feeSourceBalance / 2); | ||
| // checkInvalidTxs({tx1, tx2}, {}, {}, TxSetValidationResult::VALID); |
Comment on lines
+223
to
+235
| auto rangeSize = (count + numTasks - 1) / numTasks; | ||
| std::vector<std::function<int()>> tasks; | ||
| tasks.reserve(numTasks); | ||
| for (size_t begin = 0; begin < count; begin += rangeSize) | ||
| { | ||
| auto end = std::min(begin + rangeSize, count); | ||
| auto rangeIndex = tasks.size(); | ||
| tasks.emplace_back([begin, end, rangeIndex, &work]() { | ||
| work(begin, end, rangeIndex); | ||
| return 0; | ||
| }); | ||
| } | ||
| executeBatch(std::move(tasks)); |
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
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.
Description
Merges changes from #5449. This improves out tx set validation speed by makiing it parallel, which is important for consensus latency.
Improvements are as follows. Measured with 10 tier 1 topology (no watchers) with 2 second block time target:
2000 TPS
This still fails at 2000 TPS, but is much, much closer. It appears that whole second rounding in close times can cause poor trigger times, leading to long p99 blocks. This should be addressed with #5423.
Checklist
clang-formatv8.0.0 (viamake formator the Visual Studio extension)