Sponsored Content
Skip to content

refactor: Move more helper files into libxrpl/ledger/helpers - #6731

Merged
bthomee merged 24 commits into
developfrom
mvadari/move-helpers
Apr 6, 2026
Merged

refactor: Move more helper files into libxrpl/ledger/helpers#6731
bthomee merged 24 commits into
developfrom
mvadari/move-helpers

Conversation

@mvadari

@mvadari mvadari commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

This PR moves all remaining helper files into the libxrpl/ledger/helpers folder, after #6453.

This is a pure refactor. There is no functionality change.

Context of Change

This PR is a simplified version of #6638, to get the easy parts done quickly - and we can wait on the pieces that will take longer to review.

API Impact

N/A

@mvadari
mvadari requested a review from bthomee April 6, 2026 21:40
@mvadari mvadari added the Trivial Simple change with minimal effect, or already tested. Only needs one approval. label Apr 6, 2026
Copilot AI review requested due to automatic review settings April 6, 2026 21:40

Copilot AI 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.

Pull request overview

This PR continues the helper-file refactor by relocating remaining helper APIs into include/xrpl/ledger/helpers and updating call sites to include the new header locations (no intended behavior changes).

Changes:

  • Repoint includes across rpc/tx/path/invariant code to xrpl/ledger/helpers/* headers (AMM, NFT, delegate, escrow, permissioned DEX, payment channels).
  • Introduce/relocate public helper headers under include/xrpl/ledger/helpers and update corresponding helper .cpp files to include them.
  • Adjust a few includes/comments to match the new header boundaries (e.g., add st.h where a removed include previously provided it).

Reviewed changes

Copilot reviewed 53 out of 59 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp Switch AMM helper include to ledger/helpers/AMMUtils.h.
src/xrpld/rpc/handlers/account/AccountObjects.cpp Add ledger/helpers/NFTokenHelpers.h include.
src/xrpld/rpc/handlers/account/AccountNFTs.cpp Replace NFTokenUtils include with ledger/helpers/NFTokenHelpers.h.
src/xrpld/rpc/detail/RPCHelpers.cpp Replace NFTokenUtils include with ledger/helpers/NFTokenHelpers.h.
src/xrpld/app/ledger/OrderBookDBImpl.cpp Switch AMM helper include to ledger/helpers/AMMUtils.h.
src/test/jtx/impl/AMM.cpp Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h.
src/test/app/NFTokenDir_test.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/test/app/NFTokenBurn_test.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h and adjust other includes.
src/test/app/NFTokenAuth_test.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/test/app/NFToken_test.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/test/app/FixNFTokenPageLinks_test.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/test/app/AMMExtended_test.cpp Switch AMM helper include to ledger/helpers/AMMUtils.h.
src/test/app/AMMClawback_test.cpp Switch AMM helper include to ledger/helpers/AMMUtils.h.
src/test/app/AMMCalc_test.cpp Switch AMM helper include to ledger/helpers/AMMHelpers.h.
src/test/app/AMM_test.cpp Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h.
src/libxrpl/tx/transactors/token/TrustSet.cpp Switch delegate helper include to ledger/helpers/DelegateHelpers.h.
src/libxrpl/tx/transactors/token/MPTokenIssuanceSet.cpp Switch delegate helper include to ledger/helpers/DelegateHelpers.h.
src/libxrpl/tx/transactors/system/LedgerStateFix.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/libxrpl/tx/transactors/payment/Payment.cpp Switch includes to ledger/helpers/{DelegateHelpers,PermissionedDEXHelpers}.h.
src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp Switch payment channel helper include to ledger/helpers/PaymentChannelHelpers.h.
src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp Switch payment channel helper include to ledger/helpers/PaymentChannelHelpers.h.
src/libxrpl/tx/transactors/nft/NFTokenModify.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/libxrpl/tx/transactors/nft/NFTokenCreateOffer.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/libxrpl/tx/transactors/nft/NFTokenCancelOffer.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/libxrpl/tx/transactors/nft/NFTokenBurn.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp Switch escrow helper include to ledger/helpers/EscrowHelpers.h.
src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp Switch escrow helper include to ledger/helpers/EscrowHelpers.h.
src/libxrpl/tx/transactors/dex/OfferCreate.cpp Switch permissioned DEX helper include to ledger/helpers/PermissionedDEXHelpers.h.
src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h.
src/libxrpl/tx/transactors/dex/AMMVote.cpp Switch AMM helper include to ledger/helpers/AMMUtils.h.
src/libxrpl/tx/transactors/dex/AMMDeposit.cpp Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h.
src/libxrpl/tx/transactors/dex/AMMDelete.cpp Switch AMM helper include to ledger/helpers/AMMUtils.h.
src/libxrpl/tx/transactors/dex/AMMCreate.cpp Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h.
src/libxrpl/tx/transactors/dex/AMMClawback.cpp Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h and reflow comments.
src/libxrpl/tx/transactors/dex/AMMBid.cpp Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h.
src/libxrpl/tx/transactors/delegate/DelegateUtils.cpp Update to include ledger/helpers/DelegateHelpers.h.
src/libxrpl/tx/transactors/account/AccountSet.cpp Switch delegate helper include to ledger/helpers/DelegateHelpers.h.
src/libxrpl/tx/transactors/account/AccountDelete.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
src/libxrpl/tx/Transactor.cpp Switch includes to ledger/helpers/{DelegateHelpers,NFTokenHelpers}.h.
src/libxrpl/tx/paths/OfferStream.cpp Switch permissioned DEX helper include to ledger/helpers/PermissionedDEXHelpers.h.
src/libxrpl/tx/paths/BookStep.cpp Switch AMM helper include to ledger/helpers/AMMUtils.h.
src/libxrpl/tx/invariants/NFTInvariant.cpp Switch NFT helper include to ledger/helpers/NFTokenHelpers.h and add st.h.
src/libxrpl/tx/invariants/AMMInvariant.cpp Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h.
src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp Update to include new ledger/helpers/PermissionedDEXHelpers.h.
src/libxrpl/ledger/helpers/PaymentChannelHelpers.cpp Update to include new ledger/helpers/PaymentChannelHelpers.h.
src/libxrpl/ledger/helpers/NFTokenHelpers.cpp Update to include new ledger/helpers/NFTokenHelpers.h and remove old include.
src/libxrpl/ledger/helpers/AMMUtils.cpp Update to include new ledger/helpers/{AMMHelpers,AMMUtils}.h and remove old includes.
src/libxrpl/ledger/helpers/AMMHelpers.cpp Update to include new ledger/helpers/AMMHelpers.h.
include/xrpl/tx/transactors/nft/NFTokenMint.h Switch NFT helper include to ledger/helpers/NFTokenHelpers.h.
include/xrpl/tx/paths/detail/StrandFlow.h Switch AMM helper include to ledger/helpers/AMMHelpers.h.
include/xrpl/tx/paths/AMMLiquidity.h Switch AMM helper includes to ledger/helpers/{AMMHelpers,AMMUtils}.h.
include/xrpl/ledger/helpers/PermissionedDEXHelpers.h New public header for permissioned DEX helper APIs.
include/xrpl/ledger/helpers/PaymentChannelHelpers.h New public header for payment channel helper APIs.
include/xrpl/ledger/helpers/NFTokenHelpers.h Update public NFT helper header includes.
include/xrpl/ledger/helpers/EscrowHelpers.h New public header for escrow unlock helper templates.
include/xrpl/ledger/helpers/DelegateHelpers.h New public header for delegate permission helpers.
include/xrpl/ledger/helpers/AMMUtils.h New public header for AMM utility APIs.
include/xrpl/ledger/helpers/AMMHelpers.h New public header for AMM math/offer helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov

codecov Bot commented Apr 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.4%. Comparing base (077e03f) to head (d4502b0).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #6731     +/-   ##
=========================================
- Coverage     81.5%   81.4%   -0.0%     
=========================================
  Files         1006    1006             
  Lines        74461   74460      -1     
  Branches      7582    7558     -24     
=========================================
- Hits         60651   60636     -15     
- Misses       13810   13824     +14     
Files with missing lines Coverage Δ
include/xrpl/ledger/helpers/AMMHelpers.h 96.0% <ø> (ø)
include/xrpl/ledger/helpers/EscrowHelpers.h 98.6% <100.0%> (ø)
include/xrpl/ledger/helpers/NFTokenHelpers.h 100.0% <ø> (ø)
include/xrpl/tx/paths/AMMLiquidity.h 100.0% <ø> (ø)
include/xrpl/tx/paths/detail/StrandFlow.h 92.4% <ø> (ø)
include/xrpl/tx/transactors/nft/NFTokenMint.h 100.0% <ø> (ø)
...clude/xrpl/tx/transactors/token/MPTokenAuthorize.h 100.0% <ø> (ø)
src/libxrpl/ledger/helpers/AMMHelpers.cpp 96.7% <ø> (ø)
src/libxrpl/ledger/helpers/AMMUtils.cpp 95.5% <ø> (ø)
src/libxrpl/ledger/helpers/MPTokenHelpers.cpp 96.4% <100.0%> (+0.1%) ⬆️
... and 38 more

... and 5 files 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.

@mvadari mvadari 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 Apr 6, 2026
@bthomee
bthomee enabled auto-merge April 6, 2026 22:27
@bthomee
bthomee added this pull request to the merge queue Apr 6, 2026
Merged via the queue into develop with commit f239256 Apr 6, 2026
3 checks passed
@bthomee
bthomee deleted the mvadari/move-helpers branch April 6, 2026 22:58
mvadari added a commit that referenced this pull request Apr 7, 2026
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
marek-foss-neti pushed a commit to marek-foss-neti/rippled that referenced this pull request May 5, 2026
…#6731)

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
@mvadari mvadari added this to the 3.2.0 milestone May 20, 2026
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
…#6731)

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
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. Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants