Sponsored Content
Skip to content

refactor: Prevent dry-run transactions from being queued (#92) - #7131

Merged
bthomee merged 3 commits into
developfrom
vlntb/dry-run-fix
May 13, 2026
Merged

refactor: Prevent dry-run transactions from being queued (#92)#7131
bthomee merged 3 commits into
developfrom
vlntb/dry-run-fix

Conversation

@vlntb

@vlntb vlntb commented May 12, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Transactions marked for simulation-only processing were not being filtered at queue entry point.

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

## High Level Overview of Change

Transactions marked for simulation-only processing were not being filtered at queue entry point.

### API Impact

<!--
Please check [x] relevant options, delete irrelevant ones.

* If there is any impact to the public API methods (HTTP / WebSocket), please update https://github.com/xrplf/rippled/blob/develop/API-CHANGELOG.md
  * Update API-CHANGELOG.md and add the change directly in this PR by pushing to your PR branch.
* libxrpl: See https://github.com/XRPLF/rippled/blob/develop/docs/build/depend.md
* Peer Protocol: See https://xrpl.org/peer-protocol.html
-->

- [ ] Public API: New feature (new methods and/or new fields)
- [ ] Public API: Breaking change (in general, breaking changes should only impact the next api_version)
- [ ] `libxrpl` change (any change that may affect `libxrpl` or dependents of `libxrpl`)
- [ ] Peer protocol change (must be backward compatible or bump the peer protocol version)

<!--
## Before / After
If relevant, use this section for an English description of the change at a technical level.
If this change affects an API, examples should be included here.

For performance-impacting changes, please provide these details:
1. Is this a new feature, bug fix, or improvement to existing functionality?
2. What behavior/functionality does the change impact?
3. In what processing can the impact be measured? Be as specific as possible - e.g. RPC client call, payment transaction that involves LOB, AMM, caching, DB operations, etc.
4. Does this change affect concurrent processing - e.g. does it involve acquiring locks, multi-threaded processing, or async processing?
-->

<!--
## Test Plan
If helpful, please describe the tests that you ran to verify your changes and provide instructions so that others can reproduce.
This section may not be needed if your change includes thoroughly commented unit tests.
-->

<!--
## Future Tasks
For future tasks related to PR.
-->
@vlntb vlntb added this to the 3.1.3 (develop) milestone May 12, 2026

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@vlntb vlntb self-assigned this May 12, 2026
@vlntb
vlntb requested review from a1q123456 and mvadari May 12, 2026 19:34

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@codecov

codecov Bot commented May 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.1%. Comparing base (45b1f4d) to head (b1897bd).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
src/xrpld/app/misc/detail/TxQ.cpp 50.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #7131   +/-   ##
=======================================
  Coverage     82.1%   82.1%           
=======================================
  Files         1010    1010           
  Lines        76145   76147    +2     
  Branches      7376    7376           
=======================================
+ Hits         62548   62550    +2     
  Misses       13597   13597           
Files with missing lines Coverage Δ
src/xrpld/app/misc/detail/TxQ.cpp 97.1% <50.0%> (-0.1%) ⬇️

... and 1 file with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@a1q123456 a1q123456 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@bthomee

bthomee commented May 13, 2026

Copy link
Copy Markdown
Collaborator

@vlntb is this ready to merge?

@vlntb vlntb added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 13, 2026
@bthomee
bthomee added this pull request to the merge queue May 13, 2026
Merged via the queue into develop with commit 411286c May 13, 2026
2 of 3 checks passed
@bthomee
bthomee deleted the vlntb/dry-run-fix branch May 13, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants