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