feat: XLS-93d: Token-Enabled Payment Channels by dangell7 · Pull Request #5448 · XRPLF/rippled

dangell7 · 2025-05-24T16:56:44Z

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)

codecov · 2025-08-17T19:07:51Z

Codecov Report

❌ Patch coverage is 96.07843% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.8%. Comparing base (2b4d067) to head (df1458a).
⚠️ Report is 12 commits behind head on develop.

Files with missing lines	Patch %	Lines
include/xrpl/protocol/PayChan.h	87.9%	4 Missing ⚠️
...ansactors/payment_channel/PaymentChannelCreate.cpp	94.1%	4 Missing ⚠️
src/xrpld/rpc/handlers/account/GatewayBalances.cpp	66.7%	4 Missing ⚠️
...transactors/payment_channel/PaymentChannelFund.cpp	97.2%	2 Missing ⚠️
...c/libxrpl/ledger/helpers/PaymentChannelHelpers.cpp	96.3%	1 Missing ⚠️
...ransactors/payment_channel/PaymentChannelClaim.cpp	98.5%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5448     +/-   ##
=========================================
+ Coverage     82.6%   82.8%   +0.2%     
=========================================
  Files         1036    1036             
  Lines        79870   80244    +374     
  Branches      8949    8923     -26     
=========================================
+ Hits         65956   66415    +459     
+ Misses       13905   13820     -85     
  Partials         9       9

Files with missing lines	Coverage Δ
include/xrpl/ledger/helpers/EscrowHelpers.h	`99.5% <100.0%> (+0.6%)`	⬆️
include/xrpl/protocol/detail/ledger_entries.macro	`100.0% <ø> (ø)`
include/xrpl/protocol/detail/transactions.macro	`100.0% <ø> (ø)`
.../xrpl/protocol_autogen/ledger_entries/PayChannel.h	`100.0% <100.0%> (ø)`
...rotocol_autogen/transactions/PaymentChannelClaim.h	`100.0% <ø> (ø)`
...otocol_autogen/transactions/PaymentChannelCreate.h	`100.0% <ø> (ø)`
...protocol_autogen/transactions/PaymentChannelFund.h	`100.0% <ø> (ø)`
...transactors/payment_channel/PaymentChannelCreate.h	`100.0% <ø> (ø)`
...x/transactors/payment_channel/PaymentChannelFund.h	`100.0% <ø> (ø)`
src/libxrpl/tx/invariants/InvariantCheck.cpp	`98.4% <100.0%> (+2.3%)`	⬆️
... and 10 more

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:

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

github-actions · 2026-03-26T00:43:26Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

godexsoft · 2026-05-04T14:27:40Z

We recently merged a refactor to develop that enables clang-tidy's readability-identifier-naming. Your branch now has heavy conflicts that are largely mechanical. Below is a workflow that aligns your branch's naming with develop before merging, which should minimize the merge conflicts.

One-time setup

If you don't already have clang-tidy working in your env, on macOS:

brew install llvm@21
# Follow brew's hint to put $(brew --prefix llvm@21)/bin on PATH so run-clang-tidy is found.

Workflow on your branch (before merging develop)

1. Grab the new .clang-tidy from develop without pulling anything else. Sync your fork on GitHub first, then:

git remote -v   # should show 'upstream' among others; if not:
# git remote set-url upstream git@github.com:XRPLF/rippled.git
git fetch upstream
git checkout upstream/develop -- .clang-tidy

2. Reconfigure conan/cmake so compile_commands.json is fresh.

3. Apply renames for the files modified in your PR:

git diff --name-only $(git merge-base HEAD upstream/develop) HEAD \
  | grep -E '\.(cpp|h|hpp|ipp)$' \
  | xargs run-clang-tidy -p build -fix -allow-no-checks
# or -p .build, or whatever your build dir is called

4. Build + test, then commit as a single dedicated commit:

cmake --build build -j8
git commit -am "refactor: Align identifier naming with develop"

5. Now merge develop:

git merge upstream/develop

Extra

Run clang-tidy once more after the merge to catch any stragglers introduced from develop's side:

run-clang-tidy -p build -fix -allow-no-checks src tests
# or -p .build, or whatever your build dir is called

dangell7 · 2026-05-06T18:10:06Z

@copilot resolve the merge conflicts in this pull request

github-actions · 2026-05-06T18:52:52Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

Copilot

Pull request overview

This PR implements XLS-93d token-enabled Payment Channels, extending payment channels to support non-XRP assets (IOUs and MPTs) behind a new TokenPaychan amendment, and refactors/extends related ledger helpers and tests to cover the new behavior.

Changes:

Add IOU/MPT support to PaymentChannelCreate, PaymentChannelFund, and PaymentChannelClaim, including transfer-rate capture and escrow-style locking/unlocking.
Extend paychan claim signing to serialize non-XRP amounts, and add new ledger fields (sfTransferRate, sfIssuerNode) to paychan entries.
Add extensive unit test coverage and new jtx helpers for paychan operations and balance/escrow assertions.

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/xrpld/rpc/handlers/account/GatewayBalances.cpp	Adds paychan amounts into `gateway_balances` “locked” aggregation.
src/xrpld/app/misc/EscrowUtils.h	Introduces new app-level escrow/paychan helper templates (currently appears unused).
src/test/jtx/TestHelpers.h	Declares new test helpers for escrowed balances (IOU/MPT).
src/test/jtx/paychan.h	Adds jtx paychan transaction builders/utilities.
src/test/jtx/impl/TestHelpers.cpp	Implements new gateway balance / escrowed amount helpers for tests.
src/test/jtx/impl/paychan.cpp	Implements the new jtx paychan helpers.
src/test/jtx.h	Exposes the new jtx paychan helpers.
src/test/app/PayChanToken_test.cpp	Adds comprehensive IOU/MPT paychan test suite.
src/test/app/PayChan_test.cpp	Migrates existing paychan tests to new `jtx::paychan::*` helpers and adds coverage for feature interactions.
src/test/app/MPToken_test.cpp	Expands MPT JSON field-validation coverage to more txn types.
src/test/app/EscrowToken_test.cpp	Minor formatting cleanup.
src/test/app/AMM_test.cpp	Formatting adjustment for paychan create call.
src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp	Extends fund to support token amounts via escrow-style locking.
src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp	Extends create to support token amounts, transfer rate capture, and issuer owner-dir tracking.
src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp	Extends claim to support token payouts via escrow-style unlock/apply logic and new auth serialization.
src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp	Refactors to reuse shared escrow unlock preclaim helper.
src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp	Refactors to reuse shared escrow create preflight/preclaim helpers.
src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp	Refactors to reuse shared escrow unlock preclaim helper.
src/libxrpl/tx/invariants/MPTInvariant.cpp	Updates invariant handling to accommodate paychan claim flows.
src/libxrpl/tx/invariants/InvariantCheck.cpp	Adjusts XRP-not-created invariant to account for non-XRP paychans.
src/libxrpl/ledger/helpers/PaymentChannelHelpers.cpp	Extends close logic to return token funds and clean up issuer owner-dir link.
include/xrpl/protocol/PayChan.h	Adds non-XRP paychan authorization serialization.
include/xrpl/protocol/jss.h	Adds `jss::escrowed` JSON key.
include/xrpl/protocol/detail/transactions.macro	Marks paychan amount/balance fields as MPT-supported.
include/xrpl/protocol/detail/ledger_entries.macro	Adds optional fields to paychan ledger entry format.
include/xrpl/protocol/detail/features.macro	Introduces the `TokenPaychan` amendment.
include/xrpl/ledger/helpers/PaymentChannelHelpers.h	Updates `closeChannel` signature to include submitting account.
include/xrpl/ledger/helpers/EscrowHelpers.h	Adds shared escrow/paychan create/lock/unlock helpers for IOU/MPT flows.

Comments suppressed due to low confidence (2)

src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp:60

sfBalance/sfAmount are no longer restricted to XRP, but this preflight still compares *bal > *amt directly. STAmount comparisons throw std::runtime_error when the two amounts are not comparable (different asset types/currencies), which would make preflight throw instead of returning a TER. Add explicit validation that bal and amt (when both present) are comparable / same asset before comparing, and reject invalid combinations with temBAD_AMOUNT.

    auto const bal = ctx.tx[~sfBalance];
    if (bal && *bal <= beast::kZERO)
        return temBAD_AMOUNT;

    auto const amt = ctx.tx[~sfAmount];
    if (amt && *amt <= beast::kZERO)
        return temBAD_AMOUNT;

    if (bal && amt && *bal > *amt)
        return temBAD_AMOUNT;

src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp:176

In doApply, reqBalance > chanFunds and reqBalance - chanBalance will throw if the claim supplies an amount/balance with an asset that doesn't match the channel's sfAmount/sfBalance (comparisons/arithmetic on non-comparable STAmount throw). Please validate in preclaim (after loading the channel) that any provided sfBalance/sfAmount are the same asset type as the channel (XRP vs IOU currency/issuer vs MPT issuance) before doing comparisons/arithmetic, and return temBAD_AMOUNT (or similar) on mismatch.

    if (ctx_.tx[~sfBalance])
    {
        auto const chanBalance = slep->getFieldAmount(sfBalance);
        auto const chanFunds = slep->getFieldAmount(sfAmount);
        auto const reqBalance = ctx_.tx[sfBalance];

        if (account_ == dst && !ctx_.tx[~sfSignature])
            return temBAD_SIGNATURE;

        if (ctx_.tx[~sfSignature])
        {
            PublicKey const pk((*slep)[sfPublicKey]);
            if (ctx_.tx[sfPublicKey] != pk)
                return temBAD_SIGNER;
        }

        if (reqBalance > chanFunds)
            return tecUNFUNDED_PAYMENT;

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

Copilot

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 3 comments.

github-actions · 2026-05-11T16:34:47Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

github-actions · 2026-07-11T04:27:21Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

github-actions · 2026-07-13T11:12:50Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

github-actions · 2026-07-13T18:06:09Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

github-actions · 2026-07-14T19:05:17Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

Includes post-merge compile fixes (build-verified).

github-actions · 2026-07-17T15:02:16Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

TimothyBanks · 2026-07-13T23:33:22Z

+    beast::Journal j)
+{
+    auto const& issue = amount.get<Issue>();
+    AccountID const& issuer = amount.getIssuer();


nit: use auto to remain consistent with the rest of this function.

Done. resolved: 3c85a01

TimothyBanks · 2026-07-21T01:09:59Z

+    STAmount const balance = (*sleRippleState)[sfBalance];
+
+    // If balance is positive, issuer must have higher address than account
+    if (balance > beast::kZero && issuer < account)


I didn't see this failure condition within the spec

This is a guard on the internal RippleState representation, the balance sign must match the low/high account ordering. Unreachable for a well formed line (hence the LCOV_EXCL), not protocol behavior, so its not in the spec.

TimothyBanks · 2026-07-21T01:10:03Z

+        return tecNO_PERMISSION;  // LCOV_EXCL_LINE
+
+    // If balance is negative, issuer must have lower address than account
+    if (balance < beast::kZero && issuer > account)


I didn't see this failure condition within the spec

Same as above, internal representation guard, not spec material.

TimothyBanks · 2026-07-21T01:14:23Z

+        return tecLOCKED;
+
+    // If the mpt cannot be transferred, return tecNO_AUTH
+    if (auto const ter = canTransfer(view, mptIssue, account, dest); !isTesSuccess(ter))


I am not quite sure if this failure case is mentioned in the spec.

This one is in the spec, PaymentChannelCreate failure conditions: lacking lsfMPTCanTransfer fails with tecNO_AUTH unless the destination is the issuer.

TimothyBanks · 2026-07-21T01:16:45Z

        return tesSUCCESS;

-    if (!ctx.view.exists(trustLineKey) && createAsset)
+    if (!lineExisted && createAsset)


Why the need for this change?

Leftover hoist from an earlier revision, functionally identical. Reverted to the develop form. resolved: 3c85a01

TimothyBanks · 2026-07-21T01:58:24Z

+    }
+    else
+    {
+        if (amount <= beast::kZero)


Can this conditional be rolled up into the outer conditional?

Done. resolved: 3c85a01

TimothyBanks · 2026-07-21T01:59:19Z

    if (!sle)
        return tefINTERNAL;  // LCOV_EXCL_LINE

+    STAmount const amount{ctx_.tx[sfAmount]};


Reference type could be used here.

auto const, tx[sfAmount] is by value. resolved: 3c85a01

TimothyBanks · 2026-07-21T01:59:24Z

        return tefINTERNAL;  // LCOV_EXCL_LINE

+    STAmount const amount{ctx_.tx[sfAmount]};
+    STAmount const chanAmt{(*slep)[sfAmount]};


Reference type could be used here.

Done, getFieldAmount reference. resolved: 3c85a01

TimothyBanks · 2026-07-21T01:59:47Z

+{
+    if (ctx.view.rules().enabled(featureTokenPaychan))
+    {
+        Keylet const k(ltPAYCHAN, ctx.tx[sfChannel]);


Nit: Can we use uniform initialization instead?

Done. resolved: 3c85a01

TimothyBanks · 2026-07-21T02:02:20Z

+            for (json::Value::ArrayIndex i = 0; i < jv.size(); ++i)
+                if (!isValidJson2(jv[i]))
+                    return false;
+            return true;


These four lines could be collapsed into

return std::all_of(std::begin(jv), std::end(jv), isValidJson2);

That was a bad merge resolution on my side, restored develop's std::all_of form verbatim. resolved: 096fa96

github-actions · 2026-07-23T19:23:33Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

dangell7 · 2026-08-03T15:37:01Z

Superseded by #7935 (moved to an XRPLF branch for native CI and review). PaymentChannelClawback is split out into #7936, stacked on #7935.

dangell7 force-pushed the token-paychan branch 2 times, most recently from dabe68d to 5c1949a Compare May 24, 2025 16:58

mvadari changed the title ~~XLS-103d: Token-Enabled Payment Channels~~ XLS-93d: Token-Enabled Payment Channels May 27, 2025

dangell7 force-pushed the token-paychan branch 3 times, most recently from 5afa17d to db65118 Compare August 16, 2025 12:20

dangell7 marked this pull request as ready for review August 16, 2025 12:21

dangell7 requested a review from a team August 16, 2025 12:21

dangell7 added the Amendment label Aug 16, 2025

dangell7 force-pushed the token-paychan branch from 482f7ff to 4da5bf5 Compare August 17, 2025 18:48

vlntb added the Triaged Issue/PR has been triaged for viability, liveliness, etc. label Oct 17, 2025

mvadari added the Blocked: Needs Draft XLS The corresponding draft XLS must have been merged before this PR is merged. label Jan 28, 2026

github-actions Bot added the PR: has conflicts label Mar 26, 2026

Copilot AI review requested due to automatic review settings May 6, 2026 18:52

github-actions Bot removed the PR: has conflicts label May 6, 2026

Copilot started reviewing on behalf of dangell7 May 6, 2026 18:53 View session

dangell7 changed the title ~~XLS-93d: Token-Enabled Payment Channels~~ feat: XLS-93d: Token-Enabled Payment Channels May 6, 2026

Copilot AI reviewed May 6, 2026

View reviewed changes

dangell7 force-pushed the token-paychan branch from ecc9473 to 69add35 Compare May 7, 2026 10:33

Copilot AI review requested due to automatic review settings May 10, 2026 16:10

Copilot started reviewing on behalf of dangell7 May 10, 2026 16:10 View session

Copilot AI reviewed May 10, 2026

View reviewed changes

Comment thread src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp Outdated

Comment thread src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp

Comment thread src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp

github-actions Bot added the PR: has conflicts label May 11, 2026

dangell7 force-pushed the token-paychan branch from 2c6012c to fd466c7 Compare May 20, 2026 09:53

Merge develop into token-paychan

939f8b1

github-actions Bot removed the PR: has conflicts label Jul 11, 2026

github-actions Bot added the PR: has conflicts label Jul 13, 2026

Merge branch 'develop' into token-paychan

6c74fc1

github-actions Bot removed the PR: has conflicts label Jul 13, 2026

depthfirst-app Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp Outdated

dangell7 added 3 commits July 13, 2026 14:59

fix: adapt token paychan to develop merge helpers and sponsor reserve

fed5093

style: conform doxygen comments to check-doxygen-style hook

a9430d4

style: remove includes flagged by misc-include-cleaner

1a9a11f

depthfirst-app Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp Outdated

dangell7 added 2 commits July 13, 2026 16:33

fix: validate paychan asset match in preclaim with tecWRONG_ASSET

cdd5ebf

test: expand token paychan transactor phase coverage

df1458a

github-actions Bot added the PR: has conflicts label Jul 14, 2026

Merge branch 'develop' into token-paychan

1ced7dc

Includes post-merge compile fixes (build-verified).

github-actions Bot removed the PR: has conflicts label Jul 17, 2026

TimothyBanks reviewed Jul 21, 2026

View reviewed changes

github-actions Bot added the PR: has conflicts label Jul 23, 2026

dangell7 mentioned this pull request Jul 28, 2026

refactor: Extract escrow lock helpers and paychan test helpers #7882

Open

1 task

dangell7 added 3 commits August 2, 2026 16:05

fix: repair bad develop merge resolutions

096fa96

refactor: address review nits

3c85a01

feat: add PaymentChannelClawback for issuer clawback of channeled tokens

96d1764

dangell7 mentioned this pull request Aug 3, 2026

feat: XLS-93d: Token-Enabled Payment Channels #7935

Open

2 tasks

dangell7 closed this Aug 3, 2026

Uh oh!

Conversation

dangell7 commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

codecov Bot commented Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Mar 26, 2026

Uh oh!

godexsoft commented May 4, 2026

One-time setup

Workflow on your branch (before merging develop)

Extra

Uh oh!

dangell7 commented May 6, 2026

Uh oh!

github-actions Bot commented May 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

github-actions Bot commented Jul 11, 2026

Uh oh!

github-actions Bot commented Jul 13, 2026

Uh oh!

github-actions Bot commented Jul 13, 2026

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 14, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dangell7 commented May 24, 2025 •

edited

Loading

codecov Bot commented Aug 17, 2025 •

edited

Loading