[ALPHANET] Token PayChannel

[dangell7]

High Level Overview of Change

Context of Change

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)

[sentinel-ai-reviewer]

Every other hook that runs on C++ headers in this file explicitly excludes include/xrpl/protocol_autogen/ (clang-tidy, fix-include-style, and clang-format all do so), but the new fix-pragma-once hook has no exclude at all. When any autogenerated header under that directory is staged, the hook will attempt to prepend #pragma once to it — modifying a file that is supposed to be owned by the code generator, and causing the hook to return a failure exit code that blocks the commit. Add exclude: ^include/xrpl/protocol_autogen/ to match the pattern the sibling hooks already use.

Suggested fix

Add exclude: ^include/xrpl/protocol_autogen/ to the fix-pragma-once hook definition, consistent with the exclude patterns on fix-include-style, clang-tidy, and clang-format.

[sentinel-ai-reviewer]

mpt-crypto/0.4.0-rc2 — a release-candidate pre-release — is introduced as a hard required dependency and linked into xrpl_libs, making it the sole verification engine for all zero-knowledge proofs in ConfidentialMPTSend, ConfidentialMPTConvert, and ConfidentialMPTConvertBack transactions on the consensus-critical path. A proof-verification defect in a pre-release cryptographic library at this position lets an attacker craft a structurally valid but mathematically invalid proof that passes mpt_verify_send_proof / mpt_verify_convert_back_proof, enabling confidential token movements that are unsupported by the underlying balance commitments. The package is served from XRPLF's private Conan remote (conan.xrplf.org) rather than public Conan Center, meaning it has not received the independent third-party review expected of a production cryptographic dependency. Promote this to a stable, externally audited release before mainnet activation, and gate all ConfidentialMPT* transaction types behind an amendment that remains disabled until that review is complete.

Suggested fix

Replace mpt-crypto/0.4.0-rc2 with a stable, non-RC release that has undergone external cryptographic audit. Until then, ensure all ConfidentialMPT* transaction types are guarded by an amendment flag that is not enabled on mainnet, so a latent proof-verification bug in the RC library cannot be triggered on production ledger state. Additionally, consider publishing the mpt-crypto recipe to public Conan Center or providing a reproducible build from a public source repository to enable community security review.

[sentinel-ai-reviewer]

PatchNixBinary.cmake is now included unconditionally and, during the CMake configure phase, executes /tmp/loader-path.sh via execute_process and passes its stdout verbatim as the --set-interpreter argument to patchelf on every built binary. /tmp/ is world-writable on Linux, so any concurrent process on the CI host — another job, a dependency script, or an attacker who has compromised the build environment — can overwrite that script and plant an arbitrary ELF interpreter path in the shipped rippled binary before main() ever runs. Validate the script's integrity against a pinned hash before executing it, or move it to a root-owned path outside /tmp/.

Suggested fix

Hash /tmp/loader-path.sh against a known-good value at configure time and abort if the hash does not match, or relocate the loader-path mechanism to a path that cannot be written by non-root processes (e.g., an image-baked /etc/xrplf/loader-path.sh). Alternatively, store the expected default loader path as a compile-time constant in the Nix flake and skip the runtime script entirely.

[sentinel-ai-reviewer]

The IOU unlock path gates delivery on isDeepFrozen(), which only checks lsfHighDeepFreeze/lsfLowDeepFreeze on the trust line — it ignores both the issuer's global freeze (lsfGlobalFreeze on their AccountRoot) and regular per-line freezes (lsfHighFreeze/lsfLowFreeze). This is directly inconsistent with the corresponding lock path in escrowLockPreclaimHelper<Issue>, which correctly uses isFrozen() covering all three freeze forms, and with the MPT unlock counterpart (escrowUnlockPreclaimHelper<MPTIssue>) which also uses isFrozen(). Since neither EscrowFinish::doApply nor PaymentChannelClaim::doApply perform any additional freeze check, this preclaim helper is the sole enforcement point. An issuer who applies a compliance freeze after an IOU escrow or payment channel is created cannot prevent the frozen destination from receiving the tokens. Replace isDeepFrozen(view, account, amount.get<Issue>().currency, amount.getIssuer()) with isFrozen(view, account, amount.get<Issue>()) to match the lock-side semantics and the MPT specialization.

Suggested fix

Replace isDeepFrozen(view, account, amount.get<Issue>().currency, amount.getIssuer()) with isFrozen(view, account, amount.get<Issue>()). This matches the lock path (escrowLockPreclaimHelper<Issue>) and the MPT unlock path (escrowUnlockPreclaimHelper<MPTIssue>), both of which use isFrozen(). The isFrozen() overload for Issue internally checks global freeze on the issuer's AccountRoot and both per-line freeze flags on the trust line — all three mechanisms a compliant issuer would use.

[sentinel-ai-reviewer]

The MPT overload calls msg.add64(amt.value()) where MPTAmount::value() returns std::int64_t, not uint64_t. Serializer::add64 accepts signed via its requires constraint and writes raw two's-complement bytes — so a negative MPTAmount silently serializes as a very large unsigned value with no sign flag, no error, and no indication of the sign inversion. The IOU overload directly above explicitly branches on signum() == -1 to negate the mantissa and omit the positive flag — this asymmetry is dangerous. While PaymentChannelClaim::preflight currently guards against negative amounts (amt <= kZero → temBAD_AMOUNT), this function is public inline API and has no internal validation; any caller outside the transactor path (future escrow helpers, custom code) can reach the silent-cast path. Add XRPL_ASSERT(amt.value() >= 0, "MPT PayChan amount must be non-negative") at the top of this overload — or, better, change the parameter to uint64_t directly if MPT amounts are always non-negative by design.

Suggested fix

Add an explicit sign guard matching the IOU overload's defensive pattern: if (amt.value() < 0) return; or XRPL_ASSERT(amt.value() >= 0, "serializePayChanAuthorization: MPT amount must be non-negative");. Alternatively, change the parameter type to accept the magnitude directly as std::uint64_t, forcing callers to extract and validate the sign before calling.

[sentinel-ai-reviewer]

Adding sfIssuerNode to the PayChannel schema enables inserting a PayChannel entry into a third-party IOU issuer's owner directory, but PaymentChannelCreate::doApply never calls adjustOwnerCount on the issuer — only on the channel's creating account. This means an attacker holding any positive balance of issuer X's IOU can create arbitrarily many PayChannels denominated in X's IOU, each growing X's owner directory without X paying any XRP reserve for those slots. Because ltPAYCHAN is not in nonObligationDeleter (AccountDelete.cpp lines 193-218), AccountDelete::preclaim returns tecHAS_OBLIGATIONS whenever it finds any of these entries during its owner-directory walk, permanently blocking the issuer from deleting their account until the attacker voluntarily closes the channels. The same gap exists in EscrowCreate.cpp (also skips adjustOwnerCount for the issuer), so this extends a pre-existing class-level defect to a new transaction type; fix both together by either charging the issuer reserve via adjustOwnerCount(view, sleIssuer, 1, j) at creation (and decrementing at close in closeChannel), or by adding ltPAYCHAN to nonObligationDeleter so these tracking entries are auto-deletable.

Suggested fix

In PaymentChannelCreate::doApply, after the dirInsert into the issuer's directory, load the issuer's AccountRoot SLE and call adjustOwnerCount(ctx_.view(), sleIssuer, 1, ctx_.journal). Mirror this in closeChannel by decrementing the issuer's owner count after dirRemove. Alternatively, add ltPAYCHAN to nonObligationDeleter in AccountDelete.cpp so channels in the issuer's directory are treated as auto-deletable obligations. Apply the identical fix to EscrowCreate.cpp / EscrowCancel.cpp / EscrowFinish.cpp for consistency.

[sentinel-ai-reviewer]

The newly introduced checkExtraFeatures override handles only the fixCleanup3_2_0 + MPTIssue sub-case, while the primary amendment gate for this feature — featureTokenPaychan — remains inside preflight() at line 116 of the .cpp file, returning temBAD_AMOUNT instead of temDISABLED. The Transactor base class explicitly documents that amendment gates must live in checkExtraFeatures (which converts false to temDISABLED); putting the gate in preflight returns the wrong TER code, making the rejection appear to clients as a malformed amount rather than a disabled feature. This can cause retry logic, error displays, and tooling to misinterpret the rejection reason. Move the featureTokenPaychan check into checkExtraFeatures — return false when the amount is non-XRP and featureTokenPaychan is not active — and remove the now-redundant temBAD_AMOUNT guard from preflight.

Suggested fix

In checkExtraFeatures, add: if (!isXRP(ctx.tx[sfAmount]) && !ctx.rules.enabled(featureTokenPaychan)) return false; before the existing MPT+fixCleanup3_2_0 check. Remove the corresponding if (!ctx.rules.enabled(featureTokenPaychan)) return temBAD_AMOUNT; block from preflight. This correctly returns temDISABLED for the amendment gate and consolidates all feature checks in the designated override.

[sentinel-ai-reviewer]

The new checkExtraFeatures override handles only the fixCleanup3_2_0 + MPTIssue sub-case, but the primary featureTokenPaychan gate for all non-XRP funding amounts remains inside preflight, where it returns temBAD_AMOUNT instead of the semantically correct temDISABLED. Unlike PaymentChannelCreate, which may carry a macro-level amendment gate, ttPAYCHAN_FUND registers with uint256{} — no feature gate at all at the transaction-type level — making checkExtraFeatures the sole correct early-rejection hook. Clients and validators that receive temBAD_AMOUNT will interpret the failure as a permanently malformed amount (and never retry), when the real reason is that featureTokenPaychan has not yet activated; temDISABLED is the canonical signal for that condition. Move the !ctx.rules.enabled(featureTokenPaychan) && !isXRP(ctx.tx[sfAmount]) check into checkExtraFeatures returning false, and remove the corresponding guard from preflight.

Suggested fix

In checkExtraFeatures, add an early guard: if (!ctx.rules.enabled(featureTokenPaychan) && !isXRP(ctx.tx[sfAmount])) return false; before the existing fixCleanup3_2_0 block. Remove the if (!ctx.rules.enabled(featureTokenPaychan)) return temBAD_AMOUNT; line from preflight. This consolidates all amendment gates in the correct pipeline phase and ensures the correct temDISABLED code is surfaced for feature-gated rejections.

[sentinel-ai-reviewer]

The MPT unlock preclaim has no canTransfer check, while the symmetric lock preclaim at line 175 explicitly gates on canTransfer(view, mptIssue, account, dest). If an issuer clears lsfMPTCanTransfer after the escrow is created, the unlock path silently succeeds for non-issuer destinations — the invariant check (ValidMPTTransfer) doesn't catch this either because unlockEscrowMPT decrements sfLockedAmount rather than sfMPTAmount, so the invariant sees zero senders. Other MPT recovery paths that intentionally bypass transferability (vault/lending) do so with an explicit WaiveMPTCanTransfer::Yes argument and are whitelisted in the invariant; escrow/paychan does neither. If the design intent is a deliberate "don't trap funds" recovery policy, it needs a comment, a WaiveMPTCanTransfer parameter, and a corresponding entry in the ValidMPTTransfer invariant whitelist; if it's an oversight, add the canTransfer check (or pass the original escrow owner's AccountID into the helper so the check can be formed correctly).

Suggested fix

Either: (1) document the intentional omission, add an explicit WaiveMPTCanTransfer concept analogous to vault/lending, and whitelist ttESCROW_FINISH/ttPAYCHAN_CLAIM in ValidMPTTransfer; or (2) add the original escrow sender as a parameter to escrowUnlockPreclaimHelper and call canTransfer(view, mptIssue, sender, account) before the final return.

[sentinel-ai-reviewer]

In the non-XRP (IOU/MPT) branch of closeChannel, createAsset is computed as src == txAccount, meaning it is false whenever the transaction submitter is not the channel source. Two call sites reach closeChannel on expired channels with no prior authorization gate — PaymentChannelFund::doApply and the expired-channel path in PaymentChannelClaim::doApply (which fires before the src/dst authorization check). A third call site reaches it when the channel destination closes a non-dry channel via tfClose. In all three cases txAccount != src, so createAsset = false. With createAsset = false, escrowUnlockApplyHelper<Issue> enforces the source's trust-line limit when crediting the returned residual balance. If the source has lowered their trust-line limit below the residual amount after the channel was created, any third-party or destination-initiated close is rejected and the funds are inaccessible until the source self-closes — because only the source's own close produces createAsset = true, which bypasses the limit check. The correct semantic for returning locked funds to the channel owner in a close is that createAsset should be true unconditionally in closeChannel (the source is always the recipient of their own returned funds), matching the invariant that a valid close must always be able to drain the residual balance.

Suggested fix

Remove the dependency on txAccount for computing createAsset inside closeChannel. Since the channel source is always the recipient of the residual balance in a close, hardcode createAsset = true in the std::visit call, and drop the txAccount parameter entirely. If there is a deliberate design intent to distinguish self-close from third-party close for some other purpose, that intent must be documented and the trust-line limit asymmetry addressed separately (e.g., do the limit check only when the source chose to reduce their limit below the locked amount post-creation).

[sentinel-ai-reviewer]

The channel_authorize and channel_verify RPC handlers were not updated as part of this PR — both still parse amount as raw XRP drops and call the XRP-only overload, producing a signature over HashPrefix | channelId | uint64(drops) with no currency or issuer bytes. PaymentChannelClaim::preflight now verifies IOU/MPT claims against the IOU-shaped preimage (HashPrefix | channelId | packedIOU64 | currency | issuer) via the new STAmount dispatcher. A malicious channel source can exploit this gap by creating an IOU payment channel, generating 'authorizations' via channel_authorize in exchange for off-chain value, and recovering all locked funds after expiry — because every on-chain claim attempt by the destination returns temBAD_SIGNATURE. Destinations also cannot use channel_verify to detect the problem before providing value. Add IOU/MPT parameter handling to both ChannelAuthorize.cpp and ChannelVerify.cpp, reading currency/issuer (or MPT issuance ID) from the RPC parameters and routing through the new STAmount-based overload.

Suggested fix

Update ChannelAuthorize.cpp (line 70-79) and ChannelVerify.cpp (line 60-73) to accept optional currency/issuer or mpt_issuance_id parameters. When present, construct an STAmount with the correct asset and call serializePayChanAuthorization(msg, channelId, stAmount) instead of serializePayChanAuthorization(msg, channelId, XRPAmount(drops)).

[sentinel-ai-reviewer]

The non-XRP branch calls escrowUnlockApplyHelper<T> passing the raw ApplyView& where an ApplyViewContext (which also requires an STTx const&) is expected — there is no implicit conversion, so this is an unambiguous compile error that kills the entire IOU/MPT close path. The structural problem is that closeChannel was never given access to the current STTx, so constructing a valid ApplyViewContext requires threading the transaction through as a new parameter. When this is eventually fixed, the fix must be correct: getEffectiveTxReserveSponsor uses ctx.tx[sfAccount] to gate which account bears the reserve cost for newly-created trust lines during channel close — a dummy or wrong STTx would misattribute or bypass that check. The fix is to add STTx const& tx to closeChannel's signature and pass ApplyViewContext{view, tx}, exactly as EscrowFinish::doApply and EscrowCancel::doApply already do via ctx_.getApplyViewContext().

Suggested fix

Add STTx const& tx as a parameter to closeChannel (updating all call sites in PaymentChannelClaim.cpp and PaymentChannelFund.cpp to supply ctx_.tx) and replace the bare view argument with ApplyViewContext{view, tx} in both escrowUnlockApplyHelper<T> calls (PaymentChannelHelpers.cpp and PaymentChannelClaim.cpp). This matches the established pattern in EscrowFinish::doApply and EscrowCancel::doApply.

[sentinel-ai-reviewer]

adjustOwnerCount(view, sle, -1, j) calls a free function that does not exist anywhere in this codebase — no declaration, no definition; only the call site itself appears. The function was refactored away into increaseOwnerCount/decreaseOwnerCount/decreaseOwnerCountForObject in this fork, and the linker will reject the translation unit. Beyond the build failure, the replaced call decreaseOwnerCountForObject(view, sle, slep, 1, j) does additional work that matters: it reads slep's sfSponsor field to find any reserve sponsor and decrements both sfSponsoredOwnerCount on the channel owner AND sfSponsoringOwnerCount on the sponsor — if a token PayChannel was created with a reserve sponsor, closing it will permanently leave both counts inflated, corrupting the sponsorship state machine. Restore the original decreaseOwnerCountForObject(view, sle, slep, 1, j) call.

Suggested fix

Replace adjustOwnerCount(view, sle, -1, j) with decreaseOwnerCountForObject(view, sle, slep, 1, j). This is the canonical deletion-path helper used by every other ledger-object-removal site in the codebase (EscrowCancel, EscrowFinish, OfferHelpers, MPTokenHelpers, etc.) and correctly handles reserve sponsor bookkeeping via the object's sfSponsor field.

[sentinel-ai-reviewer]

closeChannel returns temDISABLED — a tem* 'permanently malformed' code — from inside the doApply execution context of PaymentChannelClaim and PaymentChannelFund. tem* codes must only originate from preflight; when one is returned from doApply, applied stays false, ctx_.apply() is never called, and all view mutations including the fee deduction are silently discarded, meaning no fee is charged and the channel stays open. An expired IOU/MPT channel that hits this path is permanently uncloseable while the submitter retries at zero cost. The path is unreachable in valid protocol state since IOU channels cannot exist without featureTokenPaychan being active, but tefINTERNAL is the semantically correct code for a defensive guard reached inside doApply.

Suggested fix

Replace return temDISABLED; with return tefINTERNAL;, which correctly signals an unexpected internal state from within the apply phase. Alternatively, remove the check entirely since the invariant it guards (IOU channels cannot exist without featureTokenPaychan) is enforced at channel creation time and cannot be violated through normal ledger operation.