🎯 Practical: refactor: Split combined transactor files into individual classes by Tapanito · Pull Request #6495 · XRPLF/rippled · GitHub

Tapanito · 2026-03-06T15:53:26Z

Each of the following files previously contained multiple unrelated transactor classes in a single header/implementation pair. This PR splits each into one class per file, following the same pattern established by the rest of the codebase.

Changes

File removed	Files added
`did/DID.h/.cpp`	`did/DIDSet.h/.cpp`, `did/DIDDelete.h/.cpp`
`escrow/Escrow.h/.cpp`	`escrow/EscrowCreate.h/.cpp`, `escrow/EscrowFinish.h/.cpp`, `escrow/EscrowCancel.h/.cpp`
`credentials/Credentials.h/.cpp`	`credentials/CredentialCreate.h/.cpp`, `credentials/CredentialDelete.h/.cpp`, `credentials/CredentialAccept.h/.cpp`
`payment_channel/PayChan.h/.cpp`	`payment_channel/PayChanCreate.h/.cpp`, `payment_channel/PayChanFund.h/.cpp`, `payment_channel/PayChanClaim.h/.cpp`

Where classes shared implementation helpers, these were extracted into internal headers:

escrow/EscrowHelpers.h — escrowUnlockApplyHelper shared by EscrowFinish and EscrowCancel
payment_channel/PayChanHelpers.h/.cpp — closeChannel shared by PayChanFund and PayChanClaim

transactions.macro updated to include each class header individually.

Next Steps

This PR purposefully omits refactoring XChainBridge, as it's a complex refactor that should be performed in a separate PR.

No behavioral changes

This is a pure refactor. No logic was modified.

High Level Overview of Change

Context of Change

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
Tests (you added tests for code that already exists, or your new feature included in this PR)
Documentation update
Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
Release

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)

- Rename subdirs to lowercase (AMM→amm, Check→check, NFT→nft, etc.) - Merge AMM and Offer into dex/, add PermissionedDEXHelpers - Rename mpt→token, add SetTrust and Clawback - Group: account, bridge, oracle, payment, system, escrow, did, credentials, payment_channel, permissioned_domain, vault, lending Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

…Accept Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

codecov · 2026-03-06T16:42:54Z

Codecov Report

❌ Patch coverage is 98.55395% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.8%. Comparing base (91a23cf) to head (e0e5a62).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
...pl/tx/transactors/payment_channel/PayChanClaim.cpp	94.9%	5 Missing ⚠️
...rpl/tx/transactors/payment_channel/PayChanFund.cpp	90.9%	4 Missing ⚠️
...l/tx/transactors/payment_channel/PayChanCreate.cpp	97.0%	2 Missing ⚠️
src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp	99.3%	1 Missing ⚠️
src/libxrpl/tx/transactors/escrow/EscrowHelpers.h	98.7%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6495     +/-   ##
=========================================
- Coverage     79.8%   79.8%   -0.0%     
=========================================
  Files          862     878     +16     
  Lines        67883   67889      +6     
  Branches      7551    7554      +3     
=========================================
+ Hits         54199   54201      +2     
- Misses       13684   13688      +4

Files with missing lines	Coverage Δ
include/xrpl/protocol/detail/transactions.macro	`100.0% <ø> (ø)`
...xrpl/tx/transactors/credentials/CredentialAccept.h	`100.0% <100.0%> (ø)`
...xrpl/tx/transactors/credentials/CredentialCreate.h	`100.0% <100.0%> (ø)`
...xrpl/tx/transactors/credentials/CredentialDelete.h	`100.0% <100.0%> (ø)`
include/xrpl/tx/transactors/did/DIDDelete.h	`100.0% <ø> (ø)`
include/xrpl/tx/transactors/did/DIDSet.h	`100.0% <100.0%> (ø)`
include/xrpl/tx/transactors/escrow/EscrowCancel.h	`100.0% <100.0%> (ø)`
include/xrpl/tx/transactors/escrow/EscrowCreate.h	`100.0% <100.0%> (ø)`
include/xrpl/tx/transactors/escrow/EscrowFinish.h	`100.0% <100.0%> (ø)`
...xrpl/tx/transactors/payment_channel/PayChanClaim.h	`100.0% <100.0%> (ø)`
... and 16 more

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

a1q123456 · 2026-03-06T18:26:16Z

Is that all? I've got a feeling that we have more to split.

Tapanito · 2026-03-09T13:41:25Z

Is that all? I've got a feeling that we have more to split.

What comes to mind? The only one that is not split as part of this PR is the cross-chain bridge. That'll be done in a separate PR. It's more complex than the files in this PR.

…vate src/libxrpl/ These headers are internal implementation details consumed only by .cpp files within the tx module. Moving them out of include/xrpl/ keeps them off the public API surface installed by the build system. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

godexsoft

Leaving one suggestion but given this is a refactoring effort it isn't a blocker.

godexsoft · 2026-03-12T11:13:56Z

+std::uint32_t
+CredentialAccept::getFlagsMask(PreflightContext const& ctx)
+{
+    // 0 means "Allow any flags"


Maybe a named constant would be better? i imagine this can be useful in many places. The comment then becomes unnecessary

It's a good change, though definitely it should be part of a separate PR.

…PLF#6495) DID, Escrows, PaymentChannels, and Credentials previously contained multiple unrelated transactor classes in a single header/implementation pair. This change splits each into one class per file, following the same pattern established by the rest of the codebase.

Tapanito and others added 5 commits March 6, 2026 12:19

Split DID into DIDSet and DIDDelete

dfd61d9

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Split Escrow into EscrowCreate, EscrowFinish, EscrowCancel

7e73817

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Split Credentials into CredentialCreate, CredentialDelete, Credential…

ea426dc

…Accept Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Split PayChan into PayChanCreate, PayChanFund, PayChanClaim

d7505a9

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Tapanito requested review from a1q123456, godexsoft and mvadari March 6, 2026 15:53

Merge branch 'develop' into tapanito/tx-splits

482fd08

Tapanito and others added 2 commits March 9, 2026 16:00

Merge branch 'develop' into tapanito/tx-splits

41b4b40

a1q123456 approved these changes Mar 11, 2026

View reviewed changes

godexsoft approved these changes Mar 12, 2026

View reviewed changes

Merge branch 'develop' into tapanito/tx-splits

e0e5a62

Tapanito changed the title ~~Split combined transactor files into individual classes~~ refactor: Split combined transactor files into individual classes Mar 12, 2026

Tapanito 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 Mar 12, 2026

bthomee enabled auto-merge (squash) March 12, 2026 17:06

bthomee merged commit 2b14ee3 into develop Mar 12, 2026
3 checks passed

bthomee deleted the tapanito/tx-splits branch March 12, 2026 17:19

mvadari mentioned this pull request Mar 18, 2026

refactor: Rename transactor files/classes to match the tx name #6580

Merged

mvadari added this to the 3.2.0 milestone May 20, 2026

Navigation Menu

Uh oh!

refactor: Split combined transactor files into individual classes - #6495

refactor: Split combined transactor files into individual classes#6495
bthomee merged 9 commits into
developfrom
tapanito/tx-splits

Tapanito commented Mar 6, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Mar 6, 2026 •

edited

Loading

Uh oh!

a1q123456 commented Mar 6, 2026

Uh oh!

Tapanito commented Mar 9, 2026

Uh oh!

godexsoft left a comment

Uh oh!

godexsoft Mar 12, 2026

Uh oh!

Tapanito Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

Tapanito commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

No behavioral changes

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

codecov Bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

a1q123456 commented Mar 6, 2026

Uh oh!

Tapanito commented Mar 9, 2026

Uh oh!

godexsoft left a comment

Choose a reason for hiding this comment

Uh oh!

godexsoft Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Tapanito Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Tapanito commented Mar 6, 2026 •

edited

Loading

codecov Bot commented Mar 6, 2026 •

edited

Loading