From cd5ce8dc50cf2297afe7d644851568295fd292eb Mon Sep 17 00:00:00 2001 From: Olek <115580134+oleks-rip@users.noreply.github.com> Date: Thu, 2 Jul 2026 22:08:37 -0400 Subject: [PATCH 1/5] Payment tx fee payer ID (#7682) --- include/xrpl/protocol/STTx.h | 3 + include/xrpl/tx/Transactor.h | 4 +- src/libxrpl/protocol/STTx.cpp | 10 ++++ src/libxrpl/tx/Transactor.cpp | 40 +++++++------ .../tx/transactors/payment/Payment.cpp | 5 +- src/test/app/Sponsor_test.cpp | 56 +++++++++++++++++++ 6 files changed, 98 insertions(+), 20 deletions(-) diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index b36207bf61..5d5424e623 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -141,6 +141,9 @@ public: [[nodiscard]] std::vector const& getBatchTransactionIDs() const; + [[nodiscard]] AccountID + getFeePayerID() const; + private: /** Check the signature. @param rules The current ledger rules. diff --git a/include/xrpl/tx/Transactor.h b/include/xrpl/tx/Transactor.h index d8f97d5a5b..bdb51b06ec 100644 --- a/include/xrpl/tx/Transactor.h +++ b/include/xrpl/tx/Transactor.h @@ -137,7 +137,8 @@ enum class FeePayerType { struct FeePayer { - Keylet entry; + AccountID id; + Keylet keylet; SF_AMOUNT const& balanceField; FeePayerType type{FeePayerType::Account}; }; @@ -317,6 +318,7 @@ public: static NotTEC checkSponsor(ReadView const& view, STTx const& tx); + ///////////////////////////////////////////////////// // Interface used by AccountDelete diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 93c90c9d95..dd2ec92c90 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -606,6 +607,15 @@ STTx::getBatchTransactionIDs() const return *batchTxnIds_; } +AccountID +STTx::getFeePayerID() const +{ + if (isFieldPresent(sfSponsor) && ((getFieldU32(sfSponsorFlags) & spfSponsorFee) != 0u)) + return at(sfSponsor); + + return getInitiator(); +} + //------------------------------------------------------------------------------ static bool diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index af1491acb0..1447554812 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -578,7 +578,7 @@ Transactor::checkFee(PreclaimContext const& ctx, XRPAmount baseFee) return tesSUCCESS; auto const feePayer = getFeePayer(ctx.view, ctx.tx); - auto const payerSle = ctx.view.read(feePayer.entry); + auto const payerSle = ctx.view.read(feePayer.keylet); if (!payerSle) { @@ -653,9 +653,9 @@ Transactor::payFee() auto const feePaid = ctx_.tx[sfFee].xrp(); auto const feePayer = getFeePayer(view(), ctx_.tx); - auto const sle = view().peek(feePayer.entry); + auto const sle = view().peek(feePayer.keylet); - JLOG(j_.trace()) << "Fee payer: " + to_string(feePayer.entry.key); + JLOG(j_.trace()) << "Fee payer: " + to_string(feePayer.id); if (!sle) return tefINTERNAL; // LCOV_EXCL_LINE @@ -1306,7 +1306,7 @@ Transactor::reset(XRPAmount fee) return {tefINTERNAL, beast::kZero}; auto const feePayer = getFeePayer(view(), ctx_.tx); - auto const payerSle = view().peek(feePayer.entry); + auto const payerSle = view().peek(feePayer.keylet); if (!payerSle) return {tefINTERNAL, beast::kZero}; // LCOV_EXCL_LINE @@ -1375,31 +1375,39 @@ Transactor::getFeePayer(ReadView const& view, STTx const& tx) { auto const sponsorID = tx.getAccountID(sfSponsor); auto const sponseeID = tx.getInitiator(); - auto const hasSponsorSignature = tx.isFieldPresent(sfSponsorSignature); auto const sponsorshipKeylet = keylet::sponsorship(sponsorID, sponseeID); // if pre-funded sponsorship exists, prefer it - if (hasSponsorSignature && !view.exists(sponsorshipKeylet)) + if (view.exists(sponsorshipKeylet)) { - // co-signed + // pre funded return FeePayer{ - .entry = keylet::account(sponsorID), - .balanceField = sfBalance, - .type = FeePayerType::SponsorCoSigned}; + .id = sponsorID, + .keylet = sponsorshipKeylet, + .balanceField = sfFeeAmount, + .type = FeePayerType::SponsorPreFunded}; } - // pre funded + // Checked in Transactor::checkSponsor + XRPL_ASSERT( + tx.isFieldPresent(sfSponsorSignature), + "xrpl::getFeePayer has sponsor signature without a sponsorship object"); + + // co-signed return FeePayer{ - .entry = sponsorshipKeylet, - .balanceField = sfFeeAmount, - .type = FeePayerType::SponsorPreFunded}; + .id = sponsorID, + .keylet = keylet::account(sponsorID), + .balanceField = sfBalance, + .type = FeePayerType::SponsorCoSigned}; } - auto const payerAccountKeylet = keylet::account(tx.getInitiator()); + AccountID const payerID = tx.getInitiator(); + auto const payerAccountKeylet = keylet::account(payerID); auto const payerType = tx.isFieldPresent(sfDelegate) ? FeePayerType::Delegate : FeePayerType::Account; - return FeePayer{.entry = payerAccountKeylet, .balanceField = sfBalance, .type = payerType}; + return FeePayer{ + .id = payerID, .keylet = payerAccountKeylet, .balanceField = sfBalance, .type = payerType}; } // The sole purpose of this function is to provide a convenient, named diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index f445e26178..41614b1943 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -691,9 +691,8 @@ Payment::doApply() // reserve. auto const reserve = accountReserve(view(), sleSrc, j_); - // In a delegated payment, the fee payer is the delegated account, - // not the source account (accountID_). - bool const accountIsPayer = (ctx_.tx.getInitiator() == accountID_); + // In a delegated / fee sponsored payment, the fee payer is not the source account (accountID_). + bool const accountIsPayer = ctx_.tx.getFeePayerID() == accountID_; // preFeeBalance_ is the balance on the source account (accountID_) BEFORE the fees // were charged. If source account is the fee payer, it must also cover the fee. diff --git a/src/test/app/Sponsor_test.cpp b/src/test/app/Sponsor_test.cpp index 2e7d5ab6f2..9c5e28d0db 100644 --- a/src/test/app/Sponsor_test.cpp +++ b/src/test/app/Sponsor_test.cpp @@ -4398,6 +4398,60 @@ public: testTrustSet(cosigning); } + void + testZeroBalanceSponsoredPaymentFeePayerCheck() + { + // Zero-balance sponsored Payment: getFeePayer() consistency check + testcase("Sponsored Payment: minimal-balance account with sponsor-pays-fee"); + + using namespace jtx; + Env env{*this, testableAmendments()}; + Account const alice("alice"); + Account const sponsor("sponsor"); + Account const dest("dest"); + + auto const baseFee = env.current()->fees().base; + auto const baseReserve = env.current()->fees().reserve; + + // Fund sponsor and dest generously, alice with base reserve + 1 XRP for payment + env.fund(XRP(10000), sponsor, dest); + env.fund(baseReserve + XRP(1), alice); + env.close(); + + // Precondition: alice has base reserve + 1 XRP (enough for payment but not fee) + BEAST_EXPECT(env.balance(alice) == baseReserve + XRP(1)); + + // BUG SCENARIO (if it existed): Alice tries to send a Payment to dest + // where sponsor pays the fee via spfSponsorFee. + // If Payment.cpp used ctx_.tx.getFeePayer() (STTx version), it would + // incorrectly identify alice as the fee payer and check if alice has + // balance >= amount + fee + reserve, which would fail. + // FIX: Payment.cpp uses getFeePayer(view(), ctx_.tx) (Transactor version) + // which correctly identifies sponsor as the fee payer, so only checks + // if alice has balance >= amount + reserve (not including fee). + + auto const preDest = env.balance(dest); + auto const preSponsor = env.balance(sponsor); + + // Alice sends 1 XRP to dest, sponsor pays the fee + env(pay(alice, dest, XRP(1)), + sponsor::As(sponsor, spfSponsorFee), + Sig(sfSponsorSignature, sponsor), + Fee(baseFee), + Ter(tesSUCCESS)); + env.close(); + + // FIX VERIFIED: Payment succeeded + // Alice's balance decreased by 1 XRP (the payment amount, NOT the fee) + BEAST_EXPECT(env.balance(alice) == baseReserve); + + // Dest received 1 XRP + BEAST_EXPECT(env.balance(dest) == preDest + XRP(1)); + + // Sponsor paid the fee (NOT alice) + BEAST_EXPECT(env.balance(sponsor) == preSponsor - baseFee); + } + protected: void testSponsor() @@ -4432,6 +4486,8 @@ protected: testSponsoredTrustLineNoFreeReserve(); testCoSignReserveBoundedBySponsorshipBudget(); testReserveSponsorGate(); + + testZeroBalanceSponsoredPaymentFeePayerCheck(); } void From c7b10a3153b4b6235d92d6ea10061dc66f23989f Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 3 Jul 2026 21:00:16 -0400 Subject: [PATCH 2/5] refactor: Rename `checkInsufficientReserve` to `checkReserve` (#7712) --- include/xrpl/ledger/helpers/AccountRootHelpers.h | 4 ++-- include/xrpl/ledger/helpers/EscrowHelpers.h | 4 ++-- src/libxrpl/ledger/helpers/AccountRootHelpers.cpp | 2 +- src/libxrpl/ledger/helpers/MPTokenHelpers.cpp | 2 +- src/libxrpl/ledger/helpers/RippleStateHelpers.cpp | 4 ++-- .../tx/transactors/Sponsor/SponsorshipSet.cpp | 4 ++-- .../tx/transactors/Sponsor/SponsorshipTransfer.cpp | 12 ++++++------ src/libxrpl/tx/transactors/account/SignerListSet.cpp | 2 +- src/libxrpl/tx/transactors/check/CheckCash.cpp | 8 ++++---- src/libxrpl/tx/transactors/check/CheckCreate.cpp | 2 +- src/libxrpl/tx/transactors/delegate/DelegateSet.cpp | 2 +- src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp | 4 ++-- .../tx/transactors/payment/DepositPreauth.cpp | 4 ++-- .../payment_channel/PaymentChannelCreate.cpp | 4 ++-- .../payment_channel/PaymentChannelFund.cpp | 6 +++--- .../tx/transactors/token/MPTokenIssuanceCreate.cpp | 2 +- src/libxrpl/tx/transactors/token/TrustSet.cpp | 10 +++++----- 17 files changed, 38 insertions(+), 38 deletions(-) diff --git a/include/xrpl/ledger/helpers/AccountRootHelpers.h b/include/xrpl/ledger/helpers/AccountRootHelpers.h index 41361a7e39..13b22becc2 100644 --- a/include/xrpl/ledger/helpers/AccountRootHelpers.h +++ b/include/xrpl/ledger/helpers/AccountRootHelpers.h @@ -84,7 +84,7 @@ accountReserve(ReadView const& view, AccountID const& id, beast::Journal j, Adju return accountReserve(view, view.read(keylet::account(id)), j, adj); } -/** Check if an account has insufficient reserve. +/** Check if an account has sufficient reserve. * * @param view The ledger view to read from * @param tx The transaction being processed @@ -97,7 +97,7 @@ accountReserve(ReadView const& view, AccountID const& id, beast::Journal j, Adju * @return Transaction result code */ [[nodiscard]] TER -checkInsufficientReserve( +checkReserve( ApplyViewContext ctx, SLE::const_ref accSle, STAmount const& accBalance, diff --git a/include/xrpl/ledger/helpers/EscrowHelpers.h b/include/xrpl/ledger/helpers/EscrowHelpers.h index 6a7d182dce..65f19dc3d7 100644 --- a/include/xrpl/ledger/helpers/EscrowHelpers.h +++ b/include/xrpl/ledger/helpers/EscrowHelpers.h @@ -72,7 +72,7 @@ escrowUnlockApplyHelper( if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx, sleDest, xrpBalance, *sponsorSle, {.ownerCountDelta = 1}, journal); !isTesSuccess(ret)) { @@ -201,7 +201,7 @@ escrowUnlockApplyHelper( if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx, sleDest, xrpBalance, *sponsorSle, {.ownerCountDelta = 1}, journal); !isTesSuccess(ret)) return ret; diff --git a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp index d8e34622db..249e843683 100644 --- a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp +++ b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp @@ -329,7 +329,7 @@ accountReserve(ReadView const& view, SLE::const_ref sle, beast::Journal j, Adjus } TER -checkInsufficientReserve( +checkReserve( ApplyViewContext ctx, SLE::const_ref accSle, STAmount const& accBalance, diff --git a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index f87171a3b7..bb7822a268 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -210,7 +210,7 @@ authorizeMPToken( // budget), so this check always runs for sponsored transactions. if (sponsorSle || ownerCount(sleAcct, journal) >= 2) { - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx, sleAcct, priorBalance, sponsorSle, {.ownerCountDelta = 1}, journal); !isTesSuccess(ret)) return ret; diff --git a/src/libxrpl/ledger/helpers/RippleStateHelpers.cpp b/src/libxrpl/ledger/helpers/RippleStateHelpers.cpp index d6e1d499a4..5f86642360 100644 --- a/src/libxrpl/ledger/helpers/RippleStateHelpers.cpp +++ b/src/libxrpl/ledger/helpers/RippleStateHelpers.cpp @@ -674,8 +674,8 @@ addEmptyHolding( } // Can the account cover the trust line reserve ? - if (auto const ret = checkInsufficientReserve( - ctx, sleDst, priorBalance, sponsorSle, {.ownerCountDelta = 1}, journal); + if (auto const ret = + checkReserve(ctx, sleDst, priorBalance, sponsorSle, {.ownerCountDelta = 1}, journal); !isTesSuccess(ret)) return tecNO_LINE_INSUF_RESERVE; diff --git a/src/libxrpl/tx/transactors/Sponsor/SponsorshipSet.cpp b/src/libxrpl/tx/transactors/Sponsor/SponsorshipSet.cpp index fa29b45902..bbc57c2e10 100644 --- a/src/libxrpl/tx/transactors/Sponsor/SponsorshipSet.cpp +++ b/src/libxrpl/tx/transactors/Sponsor/SponsorshipSet.cpp @@ -225,7 +225,7 @@ SponsorshipSet::doApply() if (hasPositiveFeeAmount) sponsorBalanceAfterFee -= *feeAmount; - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sponsorAccSle, sponsorBalanceAfterFee.xrp(), @@ -292,7 +292,7 @@ SponsorshipSet::doApply() STAmount sponsorBalanceAfterFee = (*sponsorAccSle)[sfBalance]; sponsorBalanceAfterFee -= feeAmountDelta; - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sponsorAccSle, sponsorBalanceAfterFee.xrp(), diff --git a/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp b/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp index e1acaa9949..e6193569f0 100644 --- a/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp +++ b/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp @@ -344,7 +344,7 @@ SponsorshipTransfer::doApply() // check new sponsor have sufficient balance // NOLINTNEXTLINE(readability-suspicious-call-argument) - if (auto const ter = checkInsufficientReserve( + if (auto const ter = checkReserve( ctx_.getApplyViewContext(), sponseeSle, sponseeSle->getFieldAmount(sfBalance), @@ -397,7 +397,7 @@ SponsorshipTransfer::doApply() // check new sponsor have sufficient balance // NOLINTNEXTLINE(readability-suspicious-call-argument) - if (auto const ter = checkInsufficientReserve( + if (auto const ter = checkReserve( ctx_.getApplyViewContext(), sponseeSle, sponseeSle->getFieldAmount(sfBalance), @@ -445,7 +445,7 @@ SponsorshipTransfer::doApply() // The owner takes the reserve burden back when the object is // no longer sponsored. - if (auto const ter = checkInsufficientReserve( + if (auto const ter = checkReserve( ctx_.getApplyViewContext(), ownerSle, balanceBeforeFee(ownerSle), @@ -485,7 +485,7 @@ SponsorshipTransfer::doApply() if (!newSponsorSle) return tefINTERNAL; // LCOV_EXCL_LINE - if (auto const ter = checkInsufficientReserve( + if (auto const ter = checkReserve( ctx_.getApplyViewContext(), sponseeSle, sponseeSle->getFieldAmount(sfBalance), @@ -513,7 +513,7 @@ SponsorshipTransfer::doApply() if (!newSponsorSle) return tefINTERNAL; // LCOV_EXCL_LINE - if (auto const ter = checkInsufficientReserve( + if (auto const ter = checkReserve( ctx_.getApplyViewContext(), sponseeSle, sponseeSle->getFieldAmount(sfBalance), @@ -549,7 +549,7 @@ SponsorshipTransfer::doApply() // The sponsee must be able to hold its own account reserve after // the sponsorship is removed. - if (auto const ter = checkInsufficientReserve( + if (auto const ter = checkReserve( ctx_.getApplyViewContext(), sponseeSle, balanceBeforeFee(sponseeSle), diff --git a/src/libxrpl/tx/transactors/account/SignerListSet.cpp b/src/libxrpl/tx/transactors/account/SignerListSet.cpp index 457639195a..6238e30fd4 100644 --- a/src/libxrpl/tx/transactors/account/SignerListSet.cpp +++ b/src/libxrpl/tx/transactors/account/SignerListSet.cpp @@ -322,7 +322,7 @@ SignerListSet::replaceSignerList() auto const sponsorSle = getTxReserveSponsor(ctx_.getApplyViewContext()); if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sle, preFeeBalance_, diff --git a/src/libxrpl/tx/transactors/check/CheckCash.cpp b/src/libxrpl/tx/transactors/check/CheckCash.cpp index 3597dd8358..4e810d94cf 100644 --- a/src/libxrpl/tx/transactors/check/CheckCash.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCash.cpp @@ -396,11 +396,11 @@ CheckCash::doApply() // Check reserve. Return destination account SLE if enough reserve, // otherwise return nullptr. - auto checkReserve = [&]() -> SLE::pointer { + auto checkDstReserve = [&]() -> SLE::pointer { auto sleDst = psb.peek(keylet::account(accountID_)); // Can the account cover the trust line's or MPT reserve? - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( applyViewContext, sleDst, preFeeBalance_, @@ -441,7 +441,7 @@ CheckCash::doApply() // a. this (destination) account and // b. issuing account (not sending account). - auto const sleDst = checkReserve(); + auto const sleDst = checkDstReserve(); if (sleDst == nullptr) return tecNO_LINE_INSUF_RESERVE; @@ -507,7 +507,7 @@ CheckCash::doApply() auto const mptokenKey = keylet::mptoken(mptID, accountID_); if (!psb.exists(mptokenKey)) { - auto sleDst = checkReserve(); + auto sleDst = checkDstReserve(); if (sleDst == nullptr) return tecINSUFFICIENT_RESERVE; diff --git a/src/libxrpl/tx/transactors/check/CheckCreate.cpp b/src/libxrpl/tx/transactors/check/CheckCreate.cpp index 10d5b778fd..e6c8e6dd00 100644 --- a/src/libxrpl/tx/transactors/check/CheckCreate.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCreate.cpp @@ -198,7 +198,7 @@ CheckCreate::doApply() auto const sponsorSle = getTxReserveSponsor(ctx_.getApplyViewContext()); if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sle, preFeeBalance_, diff --git a/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp b/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp index 9fc4a81029..8a1c617708 100644 --- a/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp +++ b/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp @@ -102,7 +102,7 @@ DelegateSet::doApply() auto const sponsorSle = getTxReserveSponsor(ctx_.getApplyViewContext()); if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sleOwner, preFeeBalance_, diff --git a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp index 4b2f853b1b..b1de636cbf 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp @@ -445,7 +445,7 @@ EscrowCreate::doApply() // validates the sponsor's reserve + remaining credit. When // unsponsored this hits the source branch and validates the // source's pre-lock balance against base + (currentOC+1)*increment. - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sle, balance, *sponsorSle, {.ownerCountDelta = 1}, j_); !isTesSuccess(ret)) return ret; @@ -462,7 +462,7 @@ EscrowCreate::doApply() // so the source only owes its base reserve. // - unsponsored: adj=1 — source owes base + the new increment. std::int32_t const ownerCountAdj = *sponsorSle ? 0 : 1; - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sle, balance - STAmount(amount).xrp(), diff --git a/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp b/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp index 195e4d2df7..bdaa90af57 100644 --- a/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp +++ b/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp @@ -165,7 +165,7 @@ DepositPreauth::doApply() auto const sponsorSle = getTxReserveSponsor(applyViewContext); if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( applyViewContext, sleOwner, preFeeBalance_, @@ -218,7 +218,7 @@ DepositPreauth::doApply() auto const sponsorSle = getTxReserveSponsor(applyViewContext); if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( applyViewContext, sleOwner, preFeeBalance_, diff --git a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp index 1b82fcf88f..3425877026 100644 --- a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp +++ b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp @@ -146,7 +146,7 @@ PaymentChannelCreate::doApply() // validates the sponsor's reserve + remaining credit. When // unsponsored this hits the source branch and validates the // source's pre-lock balance against base + (currentOC+1)*increment. - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sle, preFeeBalance_, @@ -165,7 +165,7 @@ PaymentChannelCreate::doApply() // so the source only owes its base reserve. // - unsponsored: adj=1 — source owes base + the new increment. std::int32_t const ownerCountAdj = *sponsorSle ? 0 : 1; - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sle, preFeeBalance_ - ctx_.tx[sfAmount].xrp(), diff --git a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp index ab0635068b..3fcddcad16 100644 --- a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp +++ b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp @@ -93,12 +93,12 @@ PaymentChannelFund::doApply() auto const sponsorSle = getTxReserveSponsor(ctx_.getApplyViewContext()); if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - if (auto const ret = checkInsufficientReserve( - ctx_.getApplyViewContext(), sle, balance, *sponsorSle, {}, j_); + if (auto const ret = + checkReserve(ctx_.getApplyViewContext(), sle, balance, *sponsorSle, {}, j_); !isTesSuccess(ret)) return ret; - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sle, balance - ctx_.tx[sfAmount], {}, {}, j_); !isTesSuccess(ret)) return tecUNFUNDED; diff --git a/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp b/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp index a1a75ac531..d24dcbb3f0 100644 --- a/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp +++ b/src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp @@ -135,7 +135,7 @@ MPTokenIssuanceCreate::create( if (args.priorBalance) { - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx, acct, *(args.priorBalance), sponsorSle, {.ownerCountDelta = 1}, journal); !isTesSuccess(ret)) return std::unexpected(ret); // tecINSUFFICIENT_RESERVE diff --git a/src/libxrpl/tx/transactors/token/TrustSet.cpp b/src/libxrpl/tx/transactors/token/TrustSet.cpp index cf3f05152d..ec6e600f6e 100644 --- a/src/libxrpl/tx/transactors/token/TrustSet.cpp +++ b/src/libxrpl/tx/transactors/token/TrustSet.cpp @@ -538,7 +538,7 @@ TrustSet::doApply() // should be checked PreFunded Sponsor before increaseOwnerCount() // For PreFunded sponsors, we need to check if there are sufficient reserves before // calling increaseOwnerCount(). - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sleLowAccount, preFeeBalance_, @@ -572,7 +572,7 @@ TrustSet::doApply() // should be checked PreFunded Sponsor before increaseOwnerCount() // For PreFunded sponsors, we need to check if there are sufficient reserves before // calling increaseOwnerCount(). - if (auto const ret = checkInsufficientReserve( + if (auto const ret = checkReserve( ctx_.getApplyViewContext(), sleHighAccount, preFeeBalance_, @@ -612,8 +612,8 @@ TrustSet::doApply() } // Reserve is not scaled by load. else if ( - auto const ret = checkInsufficientReserve( - ctx_.getApplyViewContext(), sle, preFeeBalance_, *sponsorSle, {}, j_); + auto const ret = + checkReserve(ctx_.getApplyViewContext(), sle, preFeeBalance_, *sponsorSle, {}, j_); !freeTrustLine && bReserveIncrease && !isTesSuccess(ret)) { JLOG(j_.trace()) << "Delay transaction: Insufficent reserve to " @@ -643,7 +643,7 @@ TrustSet::doApply() return tecNO_LINE_REDUNDANT; } else if ( - auto const ret = checkInsufficientReserve( + auto const ret = checkReserve( ctx_.getApplyViewContext(), sle, preFeeBalance_, From 60702858a12e00f4e96918d8ee59dc8354c1e1d2 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 3 Jul 2026 21:00:42 -0400 Subject: [PATCH 3/5] refactor: Move SponsorHelpers code to SponsorHelpers.cpp (#7713) --- include/xrpl/ledger/helpers/SponsorHelpers.h | 98 +++------------- src/libxrpl/ledger/helpers/SponsorHelpers.cpp | 109 ++++++++++++++++++ 2 files changed, 125 insertions(+), 82 deletions(-) create mode 100644 src/libxrpl/ledger/helpers/SponsorHelpers.cpp diff --git a/include/xrpl/ledger/helpers/SponsorHelpers.h b/include/xrpl/ledger/helpers/SponsorHelpers.h index 17b287c5b2..3c4615b325 100644 --- a/include/xrpl/ledger/helpers/SponsorHelpers.h +++ b/include/xrpl/ledger/helpers/SponsorHelpers.h @@ -31,104 +31,38 @@ isReserveSponsored(STTx const& tx) return (tx.getFieldU32(sfSponsorFlags) & spfSponsorReserve) != 0u; } -inline std::optional -getTxReserveSponsorAccountID(STTx const& tx) -{ - if (tx.isFieldPresent(sfSponsor) && isReserveSponsored(tx)) - { - return tx.getAccountID(sfSponsor); - } - return {}; -} +std::optional +getTxReserveSponsorAccountID(STTx const& tx); -inline std::expected -getTxReserveSponsor(ApplyViewContext ctx) -{ - auto const sponsorID = getTxReserveSponsorAccountID(ctx.tx); - if (sponsorID) - { - auto sle = ctx.view.peek(keylet::account(*sponsorID)); +std::expected +getTxReserveSponsor(ApplyViewContext ctx); - // already checked in Transactor::checkSponsor - if (!sle) - return std::unexpected(tecINTERNAL); - return sle; - } - return SLE::pointer(); -} +std::expected +getTxReserveSponsor(ReadView const& view, STTx const& tx); -inline std::expected -getTxReserveSponsor(ReadView const& view, STTx const& tx) -{ - auto const sponsorID = getTxReserveSponsorAccountID(tx); - if (sponsorID) - { - auto sle = view.read(keylet::account(*sponsorID)); +std::optional +getLedgerEntryReserveSponsorAccountID(SLE::const_ref sle, SF_ACCOUNT const& field = sfSponsor); - // already checked in Transactor::checkSponsor - if (!sle) - return std::unexpected(tecINTERNAL); - return sle; - } - return SLE::pointer(); -} - -inline std::optional -getLedgerEntryReserveSponsorAccountID(SLE::const_ref sle, SF_ACCOUNT const& field = sfSponsor) -{ - if (sle->isFieldPresent(field)) - return sle->getAccountID(field); - return {}; -} - -inline SLE::pointer +SLE::pointer getLedgerEntryReserveSponsor( ApplyView& view, SLE::const_ref sle, - SF_ACCOUNT const& field = sfSponsor) -{ - auto const sponsorID = getLedgerEntryReserveSponsorAccountID(sle, field); - if (sponsorID) - return view.peek(keylet::account(*sponsorID)); - return {}; -} + SF_ACCOUNT const& field = sfSponsor); -inline SLE::const_pointer +SLE::const_pointer getLedgerEntryReserveSponsor( ReadView const& view, SLE::const_ref sle, - SF_ACCOUNT const& field = sfSponsor) -{ - auto const sponsorID = getLedgerEntryReserveSponsorAccountID(sle, field); - if (sponsorID) - return view.read(keylet::account(*sponsorID)); - return {}; -} + SF_ACCOUNT const& field = sfSponsor); -inline void +void addSponsorToLedgerEntry( SLE::ref sle, SLE::const_ref sponsorSle, - SF_ACCOUNT const& field = sfSponsor) -{ - XRPL_ASSERT( - (sle->getType() == ltRIPPLE_STATE && (field == sfHighSponsor || field == sfLowSponsor)) || - (sle->getType() != ltRIPPLE_STATE && field == sfSponsor), - "addSponsorToLedgerEntry : Invalid field to the LedgerEntry"); - if (sponsorSle) - sle->setAccountID(field, sponsorSle->getAccountID(sfAccount)); -} + SF_ACCOUNT const& field = sfSponsor); -inline void -removeSponsorFromLedgerEntry(SLE::ref sle, SF_ACCOUNT const& field = sfSponsor) -{ - XRPL_ASSERT( - (sle->getType() == ltRIPPLE_STATE && (field == sfHighSponsor || field == sfLowSponsor)) || - (sle->getType() != ltRIPPLE_STATE && field == sfSponsor), - "removeSponsorFromLedgerEntry : Invalid field to the LedgerEntry"); - if (sle->isFieldPresent(field)) - sle->makeFieldAbsent(field); -} +void +removeSponsorFromLedgerEntry(SLE::ref sle, SF_ACCOUNT const& field = sfSponsor); template inline std::optional diff --git a/src/libxrpl/ledger/helpers/SponsorHelpers.cpp b/src/libxrpl/ledger/helpers/SponsorHelpers.cpp new file mode 100644 index 0000000000..cb7f0fedc2 --- /dev/null +++ b/src/libxrpl/ledger/helpers/SponsorHelpers.cpp @@ -0,0 +1,109 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace xrpl { + +std::optional +getTxReserveSponsorAccountID(STTx const& tx) +{ + if (tx.isFieldPresent(sfSponsor) && isReserveSponsored(tx)) + { + return tx.getAccountID(sfSponsor); + } + return {}; +} + +std::expected +getTxReserveSponsor(ApplyViewContext ctx) +{ + auto const sponsorID = getTxReserveSponsorAccountID(ctx.tx); + if (sponsorID) + { + auto sle = ctx.view.peek(keylet::account(*sponsorID)); + + // already checked in Transactor::checkSponsor + if (!sle) + return std::unexpected(tecINTERNAL); + return sle; + } + return SLE::pointer(); +} + +std::expected +getTxReserveSponsor(ReadView const& view, STTx const& tx) +{ + auto const sponsorID = getTxReserveSponsorAccountID(tx); + if (sponsorID) + { + auto sle = view.read(keylet::account(*sponsorID)); + + // already checked in Transactor::checkSponsor + if (!sle) + return std::unexpected(tecINTERNAL); + return sle; + } + return SLE::pointer(); +} + +std::optional +getLedgerEntryReserveSponsorAccountID(SLE::const_ref sle, SF_ACCOUNT const& field) +{ + if (sle->isFieldPresent(field)) + return sle->getAccountID(field); + return {}; +} + +SLE::pointer +getLedgerEntryReserveSponsor(ApplyView& view, SLE::const_ref sle, SF_ACCOUNT const& field) +{ + auto const sponsorID = getLedgerEntryReserveSponsorAccountID(sle, field); + if (sponsorID) + return view.peek(keylet::account(*sponsorID)); + return {}; +} + +SLE::const_pointer +getLedgerEntryReserveSponsor(ReadView const& view, SLE::const_ref sle, SF_ACCOUNT const& field) +{ + auto const sponsorID = getLedgerEntryReserveSponsorAccountID(sle, field); + if (sponsorID) + return view.read(keylet::account(*sponsorID)); + return {}; +} + +void +addSponsorToLedgerEntry(SLE::ref sle, SLE::const_ref sponsorSle, SF_ACCOUNT const& field) +{ + XRPL_ASSERT( + (sle->getType() == ltRIPPLE_STATE && (field == sfHighSponsor || field == sfLowSponsor)) || + (sle->getType() != ltRIPPLE_STATE && field == sfSponsor), + "addSponsorToLedgerEntry : Invalid field to the LedgerEntry"); + if (sponsorSle) + sle->setAccountID(field, sponsorSle->getAccountID(sfAccount)); +} + +void +removeSponsorFromLedgerEntry(SLE::ref sle, SF_ACCOUNT const& field) +{ + XRPL_ASSERT( + (sle->getType() == ltRIPPLE_STATE && (field == sfHighSponsor || field == sfLowSponsor)) || + (sle->getType() != ltRIPPLE_STATE && field == sfSponsor), + "removeSponsorFromLedgerEntry : Invalid field to the LedgerEntry"); + if (sle->isFieldPresent(field)) + sle->makeFieldAbsent(field); +} + +} // namespace xrpl From c738ad14ed88b07698530539ca906483a0f16e8d Mon Sep 17 00:00:00 2001 From: Olek <115580134+oleks-rip@users.noreply.github.com> Date: Mon, 6 Jul 2026 11:16:35 -0400 Subject: [PATCH 4/5] Fix trust <-> sponsor interaction (#7710) --- src/libxrpl/tx/transactors/token/TrustSet.cpp | 26 ++-- src/test/app/Sponsor_test.cpp | 116 ++++++++++++++++-- 2 files changed, 120 insertions(+), 22 deletions(-) diff --git a/src/libxrpl/tx/transactors/token/TrustSet.cpp b/src/libxrpl/tx/transactors/token/TrustSet.cpp index ec6e600f6e..5a7a62b00a 100644 --- a/src/libxrpl/tx/transactors/token/TrustSet.cpp +++ b/src/libxrpl/tx/transactors/token/TrustSet.cpp @@ -332,12 +332,14 @@ TrustSet::doApply() if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - std::uint32_t const uOwnerCount = ownerCount(*sponsorSle ? *sponsorSle : sle, j_); + auto getSponsor = [&sponsorSle = *sponsorSle, this](AccountID const& account) { + return (sponsorSle && account == accountID_) ? sponsorSle : SLE::pointer(); + }; // The "free-tier" shortcut (ownerCount < 2) only applies when there is no sponsor. // With any sponsor on the tx, the sponsor must cover the reserve (via balance or // prefunded budget), so the reserve check always runs. - bool const freeTrustLine = uOwnerCount < 2 && !*sponsorSle; + bool const freeTrustLine = !*sponsorSle && (ownerCount(sle, j_) < 2); std::uint32_t const uQualityIn(bQualityIn ? ctx_.tx.getFieldU32(sfQualityIn) : 0); std::uint32_t uQualityOut(bQualityOut ? ctx_.tx.getFieldU32(sfQualityOut) : 0); @@ -535,6 +537,8 @@ TrustSet::doApply() if (bLowReserveSet && !bLowReserved) { + SLE::pointer const lowSponsor = getSponsor(uLowAccountID); + // should be checked PreFunded Sponsor before increaseOwnerCount() // For PreFunded sponsors, we need to check if there are sufficient reserves before // calling increaseOwnerCount(). @@ -542,17 +546,17 @@ TrustSet::doApply() ctx_.getApplyViewContext(), sleLowAccount, preFeeBalance_, - *sponsorSle, + lowSponsor, {.ownerCountDelta = 1}, j_); - *sponsorSle && !isTesSuccess(ret)) + lowSponsor && !isTesSuccess(ret)) return tecINSUF_RESERVE_LINE; // Set reserve for low account. - increaseOwnerCount(view(), sleLowAccount, *sponsorSle, 1, viewJ); + increaseOwnerCount(view(), sleLowAccount, lowSponsor, 1, viewJ); uFlagsOut |= lsfLowReserve; - addSponsorToLedgerEntry(sleRippleState, *sponsorSle, sfLowSponsor); + addSponsorToLedgerEntry(sleRippleState, lowSponsor, sfLowSponsor); if (!bHigh) bReserveIncrease = true; @@ -569,6 +573,8 @@ TrustSet::doApply() if (bHighReserveSet && !bHighReserved) { + SLE::pointer const highSponsor = getSponsor(uHighAccountID); + // should be checked PreFunded Sponsor before increaseOwnerCount() // For PreFunded sponsors, we need to check if there are sufficient reserves before // calling increaseOwnerCount(). @@ -576,17 +582,17 @@ TrustSet::doApply() ctx_.getApplyViewContext(), sleHighAccount, preFeeBalance_, - *sponsorSle, + highSponsor, {.ownerCountDelta = 1}, j_); - *sponsorSle && !isTesSuccess(ret)) + highSponsor && !isTesSuccess(ret)) return tecINSUF_RESERVE_LINE; // Set reserve for high account. - increaseOwnerCount(view(), sleHighAccount, *sponsorSle, 1, viewJ); + increaseOwnerCount(view(), sleHighAccount, highSponsor, 1, viewJ); uFlagsOut |= lsfHighReserve; - addSponsorToLedgerEntry(sleRippleState, *sponsorSle, sfHighSponsor); + addSponsorToLedgerEntry(sleRippleState, highSponsor, sfHighSponsor); if (bHigh) bReserveIncrease = true; diff --git a/src/test/app/Sponsor_test.cpp b/src/test/app/Sponsor_test.cpp index 9c5e28d0db..813783dafa 100644 --- a/src/test/app/Sponsor_test.cpp +++ b/src/test/app/Sponsor_test.cpp @@ -4421,15 +4421,8 @@ public: // Precondition: alice has base reserve + 1 XRP (enough for payment but not fee) BEAST_EXPECT(env.balance(alice) == baseReserve + XRP(1)); - // BUG SCENARIO (if it existed): Alice tries to send a Payment to dest - // where sponsor pays the fee via spfSponsorFee. - // If Payment.cpp used ctx_.tx.getFeePayer() (STTx version), it would - // incorrectly identify alice as the fee payer and check if alice has - // balance >= amount + fee + reserve, which would fail. - // FIX: Payment.cpp uses getFeePayer(view(), ctx_.tx) (Transactor version) - // which correctly identifies sponsor as the fee payer, so only checks - // if alice has balance >= amount + reserve (not including fee). - + // Alice tries to send a Payment to dest where sponsor pays the fee via spfSponsorFee. + // Passed even alice balance doesn't have enough to pay fee. auto const preDest = env.balance(dest); auto const preSponsor = env.balance(sponsor); @@ -4437,11 +4430,10 @@ public: env(pay(alice, dest, XRP(1)), sponsor::As(sponsor, spfSponsorFee), Sig(sfSponsorSignature, sponsor), - Fee(baseFee), - Ter(tesSUCCESS)); + Fee(baseFee)); env.close(); - // FIX VERIFIED: Payment succeeded + // Payment succeeded // Alice's balance decreased by 1 XRP (the payment amount, NOT the fee) BEAST_EXPECT(env.balance(alice) == baseReserve); @@ -4452,6 +4444,105 @@ public: BEAST_EXPECT(env.balance(sponsor) == preSponsor - baseFee); } + void + testTrustSetCounterpartySponsorMisroute() + { + // TrustSet's modify path applies the tx-level reserve sponsor to whichever + // side has its reserve gate trip on this update, regardless of whether that + // side belongs to the tx submitter. trustCreate only sets the submitter's + // reserve flag and snapshots the counterparty's asfDefaultRipple state into + // the line's NoRipple bit; if the counterparty later toggles asfDefaultRipple + // (the canonical issuer flow), the line and account flags disagree and on + // the submitter's next TrustSet the counterparty-side gate fires. Sponsor still will be + // checked if it can be applied to that end of the trustLine. + + testcase("TrustSet modify with sponsor does not misroute onto counterparty side"); + + using namespace test::jtx; + + Env env(*this); + Account const alice{"alice_t2178"}; + Account const bob{"bob_t2178"}; + Account const carol{"carol_t2178"}; + + // Fund without auto-setting asfDefaultRipple + env.fund(XRP(100'000), alice, bob, carol); + env.close(); + + // Determine account ordering + bool const aliceIsHigh = alice.id() > bob.id(); + + // To trigger the bug, we need the COUNTERPARTY's reserve gate to trip + // We use issuer/holder terminology where: + // - holder creates the trust line (their reserve is set first) + // - issuer enables DefaultRipple after (creates flag mismatch) + // - holder's second TrustSet triggers issuer's reserve gate + + auto const issuer = aliceIsHigh ? bob : alice; + auto const holder = aliceIsHigh ? alice : bob; + auto const usd = issuer["USD"]; + + // Issuer must NOT have DefaultRipple set initially + // Clear it explicitly (env.fund may have set it) + env(fclear(issuer, asfDefaultRipple)); + env.close(); + + // Holder creates the trust line first (holder's reserve flag is set) + // At this point, issuer does NOT have DefaultRipple set, so + // the NoRipple bit on issuer's side is set according to issuer's current flag + env(trust(holder, usd(1'000))); + env.close(); + + // Issuer now enables asfDefaultRipple (canonical issuer flow) + // This creates a mismatch: issuer's account flag says DefaultRipple=true + // but the trust line's NoRipple bit on issuer's side is still set + env(fset(issuer, asfDefaultRipple)); + env.close(); + + SF_ACCOUNT const& issuerSponsorField = aliceIsHigh ? sfLowSponsor : sfHighSponsor; + SF_ACCOUNT const& holderSponsorField = aliceIsHigh ? sfHighSponsor : sfLowSponsor; + + auto const lineKey = keylet::trustLine(alice, bob, usd.currency); + auto const sleLineBefore = env.le(lineKey); + if (!BEAST_EXPECT(sleLineBefore)) + return; + BEAST_EXPECT(!sleLineBefore->isFieldPresent(sfLowSponsor)); + BEAST_EXPECT(!sleLineBefore->isFieldPresent(sfHighSponsor)); + + auto const carolBefore = sponsoringOwnerCount(env, carol); + BEAST_EXPECT(carolBefore == 0); + auto const issuerSponsoredBefore = sponsoredOwnerCount(env, issuer); + BEAST_EXPECT(issuerSponsoredBefore == 0); + + // Holder modifies the trust line with Carol as sponsor + // This should trigger the issuer's reserve gate because of the DefaultRipple mismatch + // Carol (sponsor) should NOT be applied to issuer's side (issuer != tx submitter) + env(trust(holder, usd(2'000)), + sponsor::As(carol, spfSponsorReserve), + Sig(sfSponsorSignature, carol), + Ter(tesSUCCESS)); + env.close(); + + auto const sleLineAfter = env.le(lineKey); + if (!BEAST_EXPECT(sleLineAfter)) + return; + + // Carol only agreed to back the holder, not the issuer + BEAST_EXPECT(!sleLineAfter->isFieldPresent(issuerSponsorField)); + + // Holder's side also has no sponsor because holder's reserve flag was + // already set on the FIRST TrustSet (no sponsor in scope then) + BEAST_EXPECT(!sleLineAfter->isFieldPresent(holderSponsorField)); + + // Carol's sponsoring count should remain unchanged (no misroute) + auto const carolAfter = sponsoringOwnerCount(env, carol); + BEAST_EXPECT(carolAfter == carolBefore); + + // Issuer's sponsored count should remain unchanged (no misroute) + auto const issuerSponsoredAfter = sponsoredOwnerCount(env, issuer); + BEAST_EXPECT(issuerSponsoredAfter == issuerSponsoredBefore); + } + protected: void testSponsor() @@ -4488,6 +4579,7 @@ protected: testReserveSponsorGate(); testZeroBalanceSponsoredPaymentFeePayerCheck(); + testTrustSetCounterpartySponsorMisroute(); } void From 8a71f96f783f62a91a497af35f84918b255df2c5 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Mon, 6 Jul 2026 12:37:49 -0400 Subject: [PATCH 5/5] fix and refactor SponsorshipTransfer preclaim (#7720) --- .../Sponsor/SponsorshipTransfer.cpp | 141 ++++++++---------- src/test/app/Sponsor_test.cpp | 22 +++ 2 files changed, 84 insertions(+), 79 deletions(-) diff --git a/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp b/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp index e6193569f0..9ea159e8a8 100644 --- a/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp +++ b/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp @@ -112,12 +112,18 @@ SponsorshipTransfer::preflight(PreflightContext const& ctx) return temMALFORMED; } - // sfSponsorFlags should not be present if it is ending sponsorship + // sfSponsorFlags should not be present if it is ending sponsorship. if (ctx.tx.isFieldPresent(sfSponsorFlags)) { - JLOG(ctx.j.debug()) - << "preflight: sfSponsorFlags should not be present when ending sponsorship"; + // Unreachable: reaching here means sfSponsor is absent, which is already checked above, + // and preflight1Sponsor already rejects sfSponsorFlags present without sfSponsor with + // temINVALID_FLAG. Keep this as a defensive check. + // LCOV_EXCL_START + UNREACHABLE( + "xrpl::SponsorshipTransfer::preflight : sfSponsorFlags present without sfSponsor " + "when ending sponsorship"); return temINVALID_FLAG; + // LCOV_EXCL_STOP } if (ctx.tx.isFieldPresent(sfSponsee) && @@ -149,102 +155,79 @@ SponsorshipTransfer::preflight(PreflightContext const& ctx) TER SponsorshipTransfer::preclaim(PreclaimContext const& ctx) { - auto const index = ctx.tx[~sfObjectID]; + auto const objectID = ctx.tx[~sfObjectID]; auto const newSponsorSleExpected = getTxReserveSponsor(ctx.view, ctx.tx); if (!newSponsorSleExpected) return newSponsorSleExpected.error(); // LCOV_EXCL_LINE auto const newSponsorSle = *newSponsorSleExpected; - bool const isObjectSponsor = !!index; - auto const account = ctx.tx[sfAccount]; auto const sponseeID = ctx.tx[~sfSponsee].value_or(account); auto const sponseeSle = ctx.view.read(keylet::account(sponseeID)); if (!sponseeSle) - return tecINTERNAL; // LCOV_EXCL_LINE - - if (isObjectSponsor) { - auto const sle = ctx.view.read(keylet::unchecked(*index)); - if (!sle) + // If it is ending sponsorship, sfSponsee is user input, return terNO_ACCOUNT if it does not + // exist. + if (ctx.tx.isFieldPresent(sfSponsee)) + return terNO_ACCOUNT; + + // If it is creating or reassigning sponsorship, sfSponsee is the account itself, which is + // always present by the time preclaim runs. Return tecINTERNAL if it does not exist. + return tecINTERNAL; // LCOV_EXCL_LINE + } + + // Default setup with an account sponsorship transfer. If it is an object transfer, they will be + // overridden to the object SLE and its type-specific sponsor field: + // sfHighSponsor/sfLowSponsor for a RippleState, sfSponsor for other object types. + SLE::const_pointer targetSle = sponseeSle; + auto const* sponsorField = &sfSponsor; + + if (objectID.has_value()) + { + auto const objectSle = ctx.view.read(keylet::unchecked(*objectID)); + if (!objectSle) return tecNO_ENTRY; - if (!isLedgerEntrySupportedBySponsorship(sle)) + if (!isLedgerEntrySupportedBySponsorship(objectSle)) return tecNO_PERMISSION; - auto const owner = getLedgerEntryOwner(ctx.view, sle, sponseeID); + auto const owner = getLedgerEntryOwner(ctx.view, objectSle, sponseeID); if (!owner.has_value() || owner.value() != sponseeID) return tecNO_PERMISSION; - auto const& sponsorField = getLedgerEntrySponsorField(sle, owner.value()); - - if (ctx.tx.isFlag(tfSponsorshipCreate)) - { - if (!newSponsorSle) - return tecNO_PERMISSION; - - // check that the object is not sponsored yet - if (sle->isFieldPresent(sponsorField)) - return tecNO_PERMISSION; - } - else if (ctx.tx.isFlag(tfSponsorshipReassign)) - { - if (!newSponsorSle) - return tecNO_PERMISSION; - - // check object is already ctx.sponsored - if (!sle->isFieldPresent(sponsorField)) - return tecNO_PERMISSION; - } - else if (ctx.tx.isFlag(tfSponsorshipEnd)) - { - if (newSponsorSle) - return tecNO_PERMISSION; - - // check object is sponsored - if (!sle->isFieldPresent(sponsorField)) - return tecNO_PERMISSION; - - // only the sponsor or sponsee can end sponsorship - auto const sponsor = sle->getAccountID(sponsorField); - if (account != sponsor && account != sponseeID) - return tecNO_PERMISSION; - } + // Object transfer: the target is the object, and its sponsor field + // depends on the object type, a RippleState stores the sponsor in + // sfHighSponsor/sfLowSponsor, while other object type uses sfSponsor. + targetSle = objectSle; + sponsorField = &getLedgerEntrySponsorField(objectSle, owner.value()); } - else + + bool const isSponsored = targetSle->isFieldPresent(*sponsorField); + + if (ctx.tx.isFlag(tfSponsorshipCreate)) { - if (ctx.tx.isFlag(tfSponsorshipCreate)) - { - if (!newSponsorSle) - return tecNO_PERMISSION; + // Creating a new sponsorship: needs a new reserve sponsor, and the + // target must not already be sponsored + if (!newSponsorSle || isSponsored) + return tecNO_PERMISSION; + } + else if (ctx.tx.isFlag(tfSponsorshipReassign)) + { + // Reassigning sponsorship: needs a new reserve sponsor, and the target must already + // be sponsored + if (!newSponsorSle || !isSponsored) + return tecNO_PERMISSION; + } + else if (ctx.tx.isFlag(tfSponsorshipEnd)) + { + // Ending sponsorship: no new reserve sponsor, the target must be sponsored. + if (newSponsorSle || !isSponsored) + return tecNO_PERMISSION; - // check account is not sponsored yet - if (sponseeSle->isFieldPresent(sfSponsor)) - return tecNO_PERMISSION; - } - else if (ctx.tx.isFlag(tfSponsorshipReassign)) - { - if (!newSponsorSle) - return tecNO_PERMISSION; - - // check account is already sponsored - if (!sponseeSle->isFieldPresent(sfSponsor)) - return tecNO_PERMISSION; - } - else if (ctx.tx.isFlag(tfSponsorshipEnd)) - { - if (newSponsorSle) - return tecNO_PERMISSION; - - // check account is sponsored - if (!sponseeSle->isFieldPresent(sfSponsor)) - return tecNO_PERMISSION; - - // only the sponsor or sponsee can end sponsorship - auto const sponsor = sponseeSle->getAccountID(sfSponsor); - if (account != sponsor && account != sponseeID) - return tecNO_PERMISSION; - } + // Only the sponsor or sponsee can end sponsorship. + auto const sponsor = targetSle->getAccountID(*sponsorField); + if (account != sponsor && account != sponseeID) + return tecNO_PERMISSION; } return tesSUCCESS; diff --git a/src/test/app/Sponsor_test.cpp b/src/test/app/Sponsor_test.cpp index 813783dafa..21320c034a 100644 --- a/src/test/app/Sponsor_test.cpp +++ b/src/test/app/Sponsor_test.cpp @@ -921,6 +921,14 @@ public: sponsor::SponseeAcc(alice), Ter(tecNO_PERMISSION)); } + { + // The provided sfSponsee account does not exist + // when ending sponsorship. + Account const ghost("ghost"); // never funded, absent from ledger + env(sponsor::transfer(sponsor, tfSponsorshipEnd), + sponsor::SponseeAcc(ghost), + Ter(terNO_ACCOUNT)); + } } { @@ -1112,6 +1120,13 @@ public: Ter(tecNO_PERMISSION)); env.close(); + // Reassign an object that is not sponsored yet + env(sponsor::transfer(alice, tfSponsorshipReassign, checkId), + sponsor::As(sponsor1, spfSponsorReserve), + Sig(sfSponsorSignature, sponsor1), + Ter(tecNO_PERMISSION)); + env.close(); + // Valid Owner env(sponsor::transfer(alice, tfSponsorshipCreate, checkId), sponsor::As(sponsor1, spfSponsorReserve), @@ -1129,6 +1144,13 @@ public: BEAST_EXPECT(sle1->isFieldPresent(sfSponsor)); BEAST_EXPECT(sle1->getAccountID(sfSponsor) == sponsor1.id()); + // Create on an object that is already sponsored + env(sponsor::transfer(alice, tfSponsorshipCreate, checkId), + sponsor::As(sponsor2, spfSponsorReserve), + Sig(sfSponsorSignature, sponsor2), + Ter(tecNO_PERMISSION)); + env.close(); + // transfer sponsor env(sponsor::transfer(alice, tfSponsorshipReassign, checkId), sponsor::As(sponsor2, spfSponsorReserve),