-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(rpc): greatly enhance simulate RPC
#5637
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
base: develop
Are you sure you want to change the base?
Changes from all commits
9d8b7d7
fb4b933
7e16bc5
e377a0d
f84950a
b92ca51
85e89ed
7110bf1
20cb0ce
10a6a44
c5a31da
3877263
5fc32a2
cc7585e
6506a60
3daee17
60fe9f4
0d3e30e
07de564
2c94917
c3a3ae7
f161ab7
550d7e4
49e5da8
d5f363e
aff497e
6c05d8e
1842df1
2088fcf
7a6c583
4de189e
2b91667
47b2aa7
52f6a3a
3b12a74
ae70a1f
d557be1
426fa70
3e88d14
493ff9f
73e29f8
4f6c2aa
0cbe4e6
be70f58
0d11a84
fee96cc
d9bb2be
2ee928c
dab5078
538f3ef
00bd84d
3e863fd
b05d921
e2a0e92
ced1253
410661b
39c88ad
e055aff
8c289f0
afa69bd
883792e
b011cdf
3f75ab0
ae947f3
5c6ebe7
44a947e
742c282
406e18f
b1f7dbb
876bed8
8f4a532
55b8469
e0316e6
c65c49a
e689976
2836a7d
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -71,6 +71,12 @@ This release contains bug fixes only and no API changes. | |||||||||
|
|
||||||||||
| This release contains bug fixes only and no API changes. | ||||||||||
|
|
||||||||||
| ## Unreleased | ||||||||||
|
|
||||||||||
| ### Additions and bugfixes | ||||||||||
|
|
||||||||||
| - `simulate`: There is now additional support for simulating transactions in past ledgers, by providing the `ledger_index` or `ledger_hash`. | ||||||||||
|
||||||||||
| - `simulate`: There is now additional support for simulating transactions in past ledgers, by providing the `ledger_index` or `ledger_hash`. | |
| - `simulate`: Added support for simulating transactions against historical ledgers by providing the `ledger_index` or `ledger_hash`. | |
| - `simulate`: You can now request simulation of an existing historical transaction by specifying its `tx_hash` or `ctid`, without needing to resubmit the full transaction. | |
| - `simulate`: Added batch transaction support via a new top-level `transactions` array parameter, allowing multiple transactions to be simulated in a single request. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| #pragma once | ||
|
|
||
| #include <xrpl/beast/utility/Journal.h> | ||
| #include <xrpl/core/ServiceRegistry.h> | ||
| #include <xrpl/ledger/View.h> | ||
| #include <xrpl/protocol/STTx.h> | ||
| #include <xrpl/tx/applySteps.h> | ||
|
|
@@ -115,6 +116,14 @@ enum class ApplyTransactionResult { | |
| Retry | ||
| }; | ||
|
|
||
| std::optional<std::vector<ApplyResult>> | ||
| applyBatchTransactions( | ||
| ServiceRegistry& registry, | ||
| OpenView& batchView, | ||
| STTx const& batchTxn, | ||
| ApplyFlags flags, | ||
| beast::Journal j); | ||
|
Comment on lines
+119
to
+125
|
||
|
|
||
| /** Transaction application helper | ||
|
|
||
| Provides more detailed logging and decodes the | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -246,11 +246,8 @@ ApplyStateTable::apply( | |||||||||||||||||||||||||||||
| metadata = meta; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| // Note: We intentionally insert the transaction and apply the pending | |
| // state changes to the provided OpenView even when isDryRun is true. | |
| // In simulate RPC/dry-run contexts, `to` is a temporary view that is | |
| // discarded after the call, so these modifications do not affect any | |
| // persisted ledger state. This allows simulation to generate accurate | |
| // metadata while keeping the base ledger unchanged. |
Copilot
AI
Dec 10, 2025
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.
The removal of the isDryRun check means that transactions will now be inserted into the view even during dry runs. This change appears intentional to support simulating batch transactions, but it fundamentally changes the behavior of dry runs. The rawTxInsert and apply calls will now execute for dry runs, which could have unintended consequences. Ensure this is the desired behavior and that it doesn't break existing functionality that depends on dry runs not modifying the view.
| to.rawTxInsert(tx.getTransactionID(), sTx, sMeta); | |
| apply(to); | |
| if (!isDryRun) | |
| { | |
| to.rawTxInsert(tx.getTransactionID(), sTx, sMeta); | |
| apply(to); | |
| } |
Copilot
AI
Feb 26, 2026
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.
Removing the isDryRun check means transactions are now inserted into the view even during dry runs. While this should be safe because simulate uses temporary OpenView copies, this changes the behavior where previously dry runs would not insert transactions into views at all. Consider verifying this behavior change is intentional and document it if necessary, as it may have implications for how views track transaction insertions during simulation.
| to.rawTxInsert(tx.getTransactionID(), sTx, sMeta); | |
| apply(to); | |
| if (!isDryRun) | |
| { | |
| to.rawTxInsert(tx.getTransactionID(), sTx, sMeta); | |
| apply(to); | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -135,11 +135,12 @@ | |||||||||
| }); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| static bool | ||||||||||
| std::optional<std::vector<ApplyResult>> | ||||||||||
| applyBatchTransactions( | ||||||||||
| ServiceRegistry& registry, | ||||||||||
| OpenView& batchView, | ||||||||||
| STTx const& batchTxn, | ||||||||||
| ApplyFlags flags, | ||||||||||
| beast::Journal j) | ||||||||||
| { | ||||||||||
| XRPL_ASSERT( | ||||||||||
|
|
@@ -149,41 +150,57 @@ | |||||||||
| auto const parentBatchId = batchTxn.getTransactionID(); | ||||||||||
| auto const mode = batchTxn.getFlags(); | ||||||||||
|
|
||||||||||
| auto applyOneTransaction = [®istry, &j, &parentBatchId, &batchView](STTx&& tx) { | ||||||||||
| auto applyOneTransaction = [®istry, &j, &parentBatchId, &batchView, &flags](STTx&& tx) { | ||||||||||
| OpenView perTxBatchView(batch_view, batchView); | ||||||||||
|
|
||||||||||
| auto const ret = apply(registry, perTxBatchView, parentBatchId, tx, tapBATCH, j); | ||||||||||
| XRPL_ASSERT( | ||||||||||
| ret.applied == (isTesSuccess(ret.ter) || isTecClaim(ret.ter)), | ||||||||||
| "Inner transaction should not be applied"); | ||||||||||
| auto const ret = | ||||||||||
| apply(registry, perTxBatchView, parentBatchId, tx, (flags & tapDRY_RUN) | tapBATCH, j); | ||||||||||
| if (flags & tapDRY_RUN) | ||||||||||
|
||||||||||
| { | ||||||||||
| XRPL_ASSERT(ret.applied == false, "Inner transaction should not be applied in dry run"); | ||||||||||
| } | ||||||||||
| else | ||||||||||
| { | ||||||||||
| XRPL_ASSERT( | ||||||||||
| ret.applied == (isTesSuccess(ret.ter) || isTecClaim(ret.ter)), | ||||||||||
| "Inner transaction should not be applied"); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| JLOG(j.debug()) << "BatchTrace[" << parentBatchId << "]: " << tx.getTransactionID() << " " | ||||||||||
| << (ret.applied ? "applied" : "failure") << ": " << transToken(ret.ter); | ||||||||||
|
|
||||||||||
| // If the transaction should be applied push its changes to the | ||||||||||
| // whole-batch view. | ||||||||||
| if (ret.applied && (isTesSuccess(ret.ter) || isTecClaim(ret.ter))) | ||||||||||
| if ((ret.applied || flags & tapDRY_RUN) && (isTesSuccess(ret.ter) || isTecClaim(ret.ter))) | ||||||||||
| perTxBatchView.apply(batchView); | ||||||||||
|
|
||||||||||
| return ret; | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| int applied = 0; | ||||||||||
| std::vector<ApplyResult> results; | ||||||||||
|
|
||||||||||
| for (STObject rb : batchTxn.getFieldArray(sfRawTransactions)) | ||||||||||
| { | ||||||||||
| auto const result = applyOneTransaction(STTx{std::move(rb)}); | ||||||||||
| XRPL_ASSERT( | ||||||||||
| result.applied == (isTesSuccess(result.ter) || isTecClaim(result.ter)), | ||||||||||
| "Outer Batch failure, inner transaction should not be applied"); | ||||||||||
| if (flags & tapDRY_RUN) | ||||||||||
|
||||||||||
| { | ||||||||||
| XRPL_ASSERT( | ||||||||||
| result.applied == false, "Inner transaction should not be applied in dry run"); | ||||||||||
| } | ||||||||||
| else | ||||||||||
| { | ||||||||||
| XRPL_ASSERT( | ||||||||||
| result.applied == (isTesSuccess(result.ter) || isTecClaim(result.ter)), | ||||||||||
| "Outer Batch failure, inner transaction should not be applied"); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (result.applied) | ||||||||||
| ++applied; | ||||||||||
| if (result.applied || flags & tapDRY_RUN) | ||||||||||
|
||||||||||
| if (result.applied || flags & tapDRY_RUN) | |
| if (result.applied || (flags & tapDRY_RUN) != 0) |
Copilot
AI
Feb 26, 2026
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.
For better readability, consider adding explicit parentheses around the bitwise AND operation: (result.applied || (flags & tapDRY_RUN)). While the operator precedence is correct, the explicit parentheses make the intent clearer and prevent potential confusion.
| if (result.applied || flags & tapDRY_RUN) | |
| if (result.applied || (flags & tapDRY_RUN)) |
Copilot
AI
Feb 10, 2026
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.
applyBatchTransactions now always allocates and fills a std::vector<ApplyResult> even when called from normal transaction processing (applyTransaction), where the return value is only used as a boolean. This introduces unnecessary allocations/copies on the hot path for batch execution. Consider only collecting/returning per-inner ApplyResults when flags & tapDRY_RUN (or when an output parameter is provided), and otherwise keep a lightweight bool/counter-based path.
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.
The API changelog entry mentions simulating transactions in past ledgers but doesn't mention the other major features added in this PR: support for tx_hash/ctid parameters, support for multiple transactions via the transactions array parameter, and batch transaction support. These are all significant API changes that should be documented in the changelog.