feePayer

include/xrpl/protocol/STTx.h

@@ -106,9 +106,6 @@ public:
     std::uint32_t
     getSeqValue() const;
 
-    AccountID
-    getFeePayer() const;
-
     boost::container::flat_set<AccountID>
     getMentionedAccounts() const;
 

include/xrpl/protocol/TER.h

@@ -227,6 +227,7 @@ enum TERcodes : TERUnderlyingType {
     terNO_AMM,       // AMM doesn't exist for the asset pair
     terADDRESS_COLLISION,  // Failed to allocate AccountID when trying to
                            // create a pseudo-account
+    terNO_SPONSORSHIP,     // No sponsorship found
 };
 
 //------------------------------------------------------------------------------

src/libxrpl/protocol/STTx.cpp

@@ -245,25 +245,6 @@ STTx::getSeqValue() const
     return getSeqProxy().value();
 }
 
-AccountID
-STTx::getFeePayer() const
-{
-    if (isFieldPresent(sfSponsor))
-    {
-        if (getFieldObject(sfSponsor).isFlag(tfSponsorFee))
-        {
-            return getFieldObject(sfSponsor)[sfAccount];
-        }
-    }
-
-    if (isFieldPresent(sfDelegate))
-    {
-        return getAccountID(sfDelegate);
-    }
-
-    return getAccountID(sfAccount);
-}
-
 void
 STTx::sign(PublicKey const& publicKey, SecretKey const& secretKey)
 {

src/libxrpl/protocol/TER.cpp

@@ -236,6 +236,7 @@ transResults()
         MAKE_ERROR(terPRE_TICKET,             "Ticket is not yet in ledger."),
         MAKE_ERROR(terNO_AMM,                 "AMM doesn't exist for the asset pair."),
         MAKE_ERROR(terADDRESS_COLLISION,      "Failed to allocate an unique account address."),
+        MAKE_ERROR(terNO_SPONSORSHIP,         "No sponsorship found."),
 
         MAKE_ERROR(tesSUCCESS,                "The transaction was applied. Only final in a validated ledger."),
     };

src/test/app/Sponsor_test.cpp

@@ -471,58 +471,189 @@ public:
             // co-signing
             Env env{*this, testable_amendments()};
             Account const alice("alice");
+            Account const bob("bob");
             Account const sponsor("sponsor");
-            env.fund(XRP(10000), alice, sponsor);
+            env.fund(XRP(10000), alice, bob);
             env.close();
 
-            env(noop(alice),
-                fee(XRP(1)),
-                sponsor::as(sponsor, tfSponsorFee),
-                sponsor::sig(sponsor),
-                ter(tesSUCCESS));
+            {
+                // Fee should be checked before permission check,
+                // otherwise tecNO_SPONSOR_PERMISSION returned when permission
+                // check fails could cause context reset to pay fee because it
+                // is tec error
+                auto aliceBalance = env.balance(alice);
+                auto bobBalance = env.balance(bob);
+                auto sponsorBalance = env.balance(sponsor);
+
+                env(pay(alice, bob, XRP(100)),
+                    fee(XRP(2000)),
+                    sponsor::as(sponsor, tfSponsorFee),
+                    sponsor::sig(sponsor),
+                    ter(terNO_ACCOUNT));
+                env.close();
+                BEAST_EXPECT(env.balance(alice) == aliceBalance);
+                BEAST_EXPECT(env.balance(bob) == bobBalance);
+                BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
+            }
+
+            env.fund(XRP(1000), sponsor);
             env.close();
 
-            BEAST_EXPECT(env.balance(alice) == XRP(10000));
-            BEAST_EXPECT(env.balance(sponsor) == XRP(9999));
+            {
+                // Sponsor pays the fee
+                auto aliceBalance = env.balance(alice);
+                auto bobBalance = env.balance(bob);
+                auto sponsorBalance = env.balance(sponsor);
+
+                auto const sendAmt = XRP(100);
+                auto const feeAmt = XRP(10);
+                env(pay(alice, bob, sendAmt),
+                    fee(feeAmt),
+                    sponsor::as(sponsor, tfSponsorFee),
+                    sponsor::sig(sponsor));
+                env.close();
+                BEAST_EXPECT(env.balance(alice) == aliceBalance - sendAmt);
+                BEAST_EXPECT(env.balance(bob) == bobBalance + sendAmt);
+                BEAST_EXPECT(env.balance(sponsor) == sponsorBalance - feeAmt);
+            }
+
+            {
+                // insufficient balance to pay fee
+                auto aliceBalance = env.balance(alice);
+                auto bobBalance = env.balance(bob);
+                auto sponsorBalance = env.balance(sponsor);
+
+                env(pay(alice, bob, XRP(100)),
+                    fee(XRP(2000)),
+                    sponsor::as(sponsor, tfSponsorFee),
+                    sponsor::sig(sponsor),
+                    ter(terINSUF_FEE_B));
+                env.close();
+                BEAST_EXPECT(env.balance(alice) == aliceBalance);
+                BEAST_EXPECT(env.balance(bob) == bobBalance);
+                BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
+            }
+
+            {
+                // fee is paid by Sponsor
+                // on context reset (tec error)
+                auto aliceBalance = env.balance(alice);
+                auto bobBalance = env.balance(bob);
+                auto sponsorBalance = env.balance(sponsor);
+                auto const feeAmt = XRP(10);
+
+                env(pay(alice, bob, XRP(20000)),
+                    fee(feeAmt),
+                    sponsor::as(sponsor, tfSponsorFee),
+                    sponsor::sig(sponsor),
+                    ter(tecUNFUNDED_PAYMENT));
+                env.close();
+
+                BEAST_EXPECT(env.balance(alice) == aliceBalance);
+                BEAST_EXPECT(env.balance(bob) == bobBalance);
+                BEAST_EXPECT(env.balance(sponsor) == sponsorBalance - feeAmt);
+            }
         }
+
         {
             // pre funded
             Env env{*this, testable_amendments()};
             Account const alice("alice");
+            Account const bob("bob");
             Account const sponsor("sponsor");
-            env.fund(XRP(10000), alice, sponsor);
+            env.fund(XRP(10000), alice, bob, sponsor);
             env.close();
 
-            // not yet funded
-            env(noop(alice),
-                fee(drops(500)),
-                sponsor::as(sponsor, tfSponsorFee),
-                ter(tecNO_SPONSOR_PERMISSION));
+            auto const sponsorFeeBalance = [&](Account const& sponsor,
+                                               Account const& alice) {
+                return env.le(keylet::sponsor(sponsor, alice))
+                    ->getFieldAmount(sfFeeAmount)
+                    .xrp();
+            };
 
-            // set sponsorship
-            env(sponsor::set(sponsor, alice, 0, std::nullopt, XRP(1)),
-                ter(tesSUCCESS));
+            {
+                // Fee should be checked before permission check,
+                // otherwise tecNO_SPONSOR_PERMISSION returned when permission
+                // check fails could cause context reset to pay fee because it
+                // is tec error
+                auto aliceBalance = env.balance(alice);
+                auto bobBalance = env.balance(bob);
+                auto sponsorBalance = env.balance(sponsor);
+
+                env(pay(alice, bob, XRP(100)),
+                    fee(XRP(2000)),
+                    sponsor::as(sponsor, tfSponsorFee),
+                    ter(terNO_SPONSORSHIP));
+                env.close();
+                BEAST_EXPECT(env.balance(alice) == aliceBalance);
+                BEAST_EXPECT(env.balance(bob) == bobBalance);
+                BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
+            }
+
+            env(sponsor::set(sponsor, alice, 0, std::nullopt, XRP(100)));
             env.close();
 
-            auto const sle = env.le(keylet::sponsor(sponsor, alice));
-            BEAST_EXPECT(sle->getFieldAmount(sfFeeAmount) == XRP(1));
-            BEAST_EXPECT(!sle->isFieldPresent(sfReserveCount));
+            {
+                // Sponsor pays the fee
+                auto aliceBalance = env.balance(alice);
+                auto bobBalance = env.balance(bob);
+                auto sponsorBalance = env.balance(sponsor);
+                auto sponsorFee = sponsorFeeBalance(sponsor, alice);
 
-            auto const sponsorBalanceBefore = env.balance(sponsor);
-            auto const aliceBalanceBefore = env.balance(alice);
+                auto const sendAmt = XRP(100);
+                auto const feeAmt = XRP(10);
+                env(pay(alice, bob, sendAmt),
+                    fee(feeAmt),
+                    sponsor::as(sponsor, tfSponsorFee));
+                env.close();
 
-            env(noop(alice),
-                fee(drops(500)),
-                sponsor::as(sponsor, tfSponsorFee),
-                ter(tesSUCCESS));
-            env.close();
+                BEAST_EXPECT(env.balance(alice) == aliceBalance - sendAmt);
+                BEAST_EXPECT(env.balance(bob) == bobBalance + sendAmt);
+                BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
+                BEAST_EXPECT(
+                    sponsorFeeBalance(sponsor, alice) == sponsorFee - feeAmt);
+            }
 
-            BEAST_EXPECT(env.balance(alice) == aliceBalanceBefore);
-            BEAST_EXPECT(env.balance(sponsor) == sponsorBalanceBefore);
+            {
+                // insufficient balance to pay fee
+                auto aliceBalance = env.balance(alice);
+                auto bobBalance = env.balance(bob);
+                auto sponsorBalance = env.balance(sponsor);
+                auto sponsorFee = sponsorFeeBalance(sponsor, alice);
 
-            auto const sle2 = env.le(keylet::sponsor(sponsor, alice));
-            BEAST_EXPECT(
-                sle2->getFieldAmount(sfFeeAmount) == XRP(1) - drops(500));
+                env(pay(alice, bob, XRP(100)),
+                    fee(XRP(2000)),
+                    sponsor::as(sponsor, tfSponsorFee),
+                    ter(terINSUF_FEE_B));
+                env.close();
+
+                BEAST_EXPECT(env.balance(alice) == aliceBalance);
+                BEAST_EXPECT(env.balance(bob) == bobBalance);
+                BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
+                BEAST_EXPECT(sponsorFeeBalance(sponsor, alice) == sponsorFee);
+            }
+
+            {
+                // fee is paid by Sponsor
+                // on context reset (tec error)
+                auto aliceBalance = env.balance(alice);
+                auto bobBalance = env.balance(bob);
+                auto sponsorBalance = env.balance(sponsor);
+                auto sponsorFee = sponsorFeeBalance(sponsor, alice);
+                auto const feeAmt = XRP(10);
+
+                env(pay(alice, bob, XRP(20000)),
+                    fee(feeAmt),
+                    sponsor::as(sponsor, tfSponsorFee),
+                    ter(tecUNFUNDED_PAYMENT));
+                env.close();
+
+                BEAST_EXPECT(env.balance(alice) == aliceBalance);
+                BEAST_EXPECT(env.balance(bob) == bobBalance);
+                BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
+                BEAST_EXPECT(
+                    sponsorFeeBalance(sponsor, alice) == sponsorFee - feeAmt);
+            }
         }
     }
 
@@ -1067,7 +1198,7 @@ public:
         testSponsorReserve();
         testDisallowIncoming();
 
-        testAccountDelete();
+        // testAccountDelete();
     }
 };
 

src/xrpld/app/tx/detail/Transactor.cpp

@@ -371,53 +371,44 @@ Transactor::checkFee(PreclaimContext const& ctx, XRPAmount baseFee)
 
     std::optional<XRPAmount> availableBalance;
 
-    if (ctx.tx.isFieldPresent(sfSponsor))
+    auto const result = getFeePayer(ctx.tx);
+
+    if (result.type == FeePayerType::SponsorPreFunded)
     {
-        auto const txSponsor = ctx.tx.getFieldObject(sfSponsor);
-        if (txSponsor.isFlag(tfSponsorFee) &&
-            (!txSponsor.isFieldPresent(sfTxnSignature) &&
-             !txSponsor.isFieldPresent(sfSigners)))
-        {
-            // use prefunded fee sponsor
-            auto const keylet = keylet::sponsor(
-                txSponsor.getAccountID(sfAccount), ctx.tx[sfAccount]);
-            auto const sponsorSle = ctx.view.read(keylet);
-            if (!sponsorSle)
-                return tecNO_SPONSOR_PERMISSION;
-
-            XRPAmount const maxFee = sponsorSle->isFieldPresent(sfMaxFee)
-                ? sponsorSle->getFieldAmount(sfMaxFee).xrp()
-                : INITIAL_XRP;
-
-            XRPAmount const feeAmount = sponsorSle->isFieldPresent(sfFeeAmount)
-                ? sponsorSle->getFieldAmount(sfFeeAmount).xrp()
-                : XRPAmount(0);
-
-            // feePaid should <= maxFee
-            if (feePaid > maxFee)
-                return tecNO_SPONSOR_PERMISSION;
-
-            // feePaid should <= feeAmount
-            if (feePaid > feeAmount)
-                return tecNO_SPONSOR_PERMISSION;
-
-            availableBalance = feeAmount;
-        }
+        // use prefunded fee sponsor
+        auto const sponsorSle = ctx.view.read(result.keylet);
+        if (!sponsorSle)
+            return terNO_SPONSORSHIP;
         else
         {
-            // proceed to use fee payer
+            XRPL_ASSERT(
+                sponsorSle->getType() == ltSPONSORSHIP,
+                "ripple::Transactor::checkFee : could not get sponsorship");
+
+            XRPAmount feeAmount = sponsorSle->isFieldPresent(result.field)
+                ? sponsorSle->getFieldAmount(result.field).xrp()
+                : XRPAmount(0);
+
+            if (sponsorSle->isFieldPresent(sfMaxFee))
+                feeAmount = std::min(
+                    feeAmount, sponsorSle->getFieldAmount(sfMaxFee).xrp());
+            availableBalance = feeAmount;
         }
     }
-
-    if (!availableBalance)
+    else
     {
-        auto const id = ctx.tx.getFeePayer();
-        JLOG(ctx.j.trace()) << "Fee payer: " + to_string(id);
-        auto const sle = ctx.view.read(keylet::account(id));
+        auto const keylet = result.keylet;
+        JLOG(ctx.j.trace()) << "Fee payer: " + to_string(keylet.key);
+        auto const sle = ctx.view.read(keylet);
+
         if (!sle)
             return terNO_ACCOUNT;
 
-        availableBalance = (*sle)[sfBalance].xrp();
+        XRPL_ASSERT(
+            sle->getType() == ltACCOUNT_ROOT,
+            "ripple::Transactor::checkFee : could not get account");
+
+        availableBalance = (*sle)[result.field].xrp();
     }
 
     XRPL_ASSERT(
@@ -447,65 +438,26 @@ Transactor::payFee()
 {
     auto const feePaid = ctx_.tx[sfFee].xrp();
 
-    auto const isFeeSponsorObj = [&]() -> bool {
-        if (ctx_.tx.isFieldPresent(sfSponsor))
-        {
-            auto const sponsor = ctx_.tx.getFieldObject(sfSponsor);
-            if (sponsor.getFieldVL(sfSigningPubKey).empty() &&
-                !sponsor.isFieldPresent(sfSigners))
-                return sponsor.getFlags() & tfSponsorFee;
-        }
-        return false;
-    };
+    auto const result = getFeePayer(ctx_.tx);
 
-    if (ctx_.tx.isFieldPresent(sfDelegate))
-    {
-        // Delegated transactions are paid by the delegated account.
-        auto const delegate = ctx_.tx.getAccountID(sfDelegate);
-        auto const delegatedSle = view().peek(keylet::account(delegate));
-        if (!delegatedSle)
-            return tefINTERNAL;  // LCOV_EXCL_LINE
+    auto const sle = view().peek(result.keylet);
 
-        delegatedSle->setFieldAmount(
-            sfBalance, delegatedSle->getFieldAmount(sfBalance) - feePaid);
-        view().update(delegatedSle);
-    }
-    else if (isFeeSponsorObj())
-    {
-        auto const sponsor = ctx_.tx.getFieldObject(sfSponsor);
-        auto const sponsorAcc = sponsor.getAccountID(sfAccount);
-        auto const sponsorSle =
-            view().peek(keylet::sponsor(sponsorAcc, account_));
-        if (!sponsorSle)
-            return tefINTERNAL;  // LCOV_EXCL_LINE
+    JLOG(j_.trace()) << "Fee payer: " + to_string(result.keylet.key);
 
-        sponsorSle->setFieldAmount(
-            sfFeeAmount, sponsorSle->getFieldAmount(sfFeeAmount) - feePaid);
-        view().update(sponsorSle);
-    }
-    else
-    {
-        auto const id = ctx_.tx.getFeePayer();
-        auto const sle = view().peek(keylet::account(id));
-        JLOG(j_.trace()) << "Fee payer: " + to_string(id);
-        if (!sle)
-            return tefINTERNAL;  // LCOV_EXCL_LINE
+    if (!sle)
+        return tefINTERNAL;  // LCOV_EXCL_LINE
 
-        if (id != account_)  // sponsor
-        {
-            sle->setFieldAmount(
-                sfBalance, sle->getFieldAmount(sfBalance) - feePaid);
-            view().update(sle);
-            return tesSUCCESS;
-        }
+    sle->setFieldAmount(
+        result.field, sle->getFieldAmount(result.field) - feePaid);
 
+    view().update(sle);
+
+    if (result.type == FeePayerType::Account)
         // Deduct the fee, so it's not available during the transaction.
         // Will only write the account back if the transaction succeeds.
         mSourceBalance -= feePaid;
-        sle->setFieldAmount(sfBalance, mSourceBalance);
 
-        // VFALCO Should we call view().rawDestroyXRP() here as well?
-    }
+    // VFALCO Should we call view().rawDestroyXRP() here as well?
 
     return tesSUCCESS;
 }
@@ -1223,13 +1175,14 @@ Transactor::reset(XRPAmount fee)
     if (!txnAcct)
         return {tefINTERNAL, beast::zero};
 
-    auto const payerSle = ctx_.tx.isFieldPresent(sfDelegate)
-        ? view().peek(keylet::account(ctx_.tx.getAccountID(sfDelegate)))
-        : txnAcct;
+    auto const result = getFeePayer(ctx_.tx);
+
+    auto const payerSle = view().peek(result.keylet);
+
     if (!payerSle)
         return {tefINTERNAL, beast::zero};  // LCOV_EXCL_LINE
 
-    auto const balance = payerSle->getFieldAmount(sfBalance).xrp();
+    auto const balance = payerSle->getFieldAmount(result.field).xrp();
 
     // balance should have already been checked in checkFee / preFlight.
     XRPL_ASSERT(
@@ -1248,7 +1201,8 @@ Transactor::reset(XRPAmount fee)
     // If for some reason we are unable to consume the ticket or sequence
     // then the ledger is corrupted.  Rather than make things worse we
     // reject the transaction.
-    payerSle->setFieldAmount(sfBalance, balance - fee);
+    payerSle->setFieldAmount(result.field, balance - fee);
+
     TER const ter{consumeSeqProxy(txnAcct)};
     XRPL_ASSERT(
         isTesSuccess(ter), "ripple::Transactor::reset : result is tesSUCCESS");
@@ -1263,6 +1217,40 @@ Transactor::reset(XRPAmount fee)
     return {ter, fee};
 }
 
+FeePayer
+Transactor::getFeePayer(STTx const& tx)
+{
+    if (tx.isFieldPresent(sfSponsor) &&
+        tx.getFieldObject(sfSponsor).isFlag(tfSponsorFee))
+    {
+        auto const sponsor = tx.getFieldObject(sfSponsor);
+        auto const hasSignature = sponsor.isFieldPresent(sfTxnSignature) ||
+            !sponsor.getFieldVL(sfSigningPubKey).empty() ||
+            sponsor.isFieldPresent(sfSigners);
+
+        if (!hasSignature)
+        {
+            // pre funded
+            auto const keylet = keylet::sponsor(
+                sponsor.getAccountID(sfAccount), tx.getAccountID(sfAccount));
+            return FeePayer{
+                keylet, sfFeeAmount, FeePayerType::SponsorPreFunded};
+        }
+        // co-signed
+        auto const keylet = keylet::account(sponsor.getAccountID(sfAccount));
+        return FeePayer{keylet, sfBalance, FeePayerType::SponsorCoSigned};
+    }
+
+    if (tx.isFieldPresent(sfDelegate))
+    {
+        auto const keylet = keylet::account(tx.getAccountID(sfDelegate));
+        return FeePayer{keylet, sfBalance, FeePayerType::Delegate};
+    }
+
+    auto const keylet = keylet::account(tx.getAccountID(sfAccount));
+    return FeePayer{keylet, sfBalance, FeePayerType::Account};
+}
+
 // The sole purpose of this function is to provide a convenient, named
 // location to set a breakpoint, to be used when replaying transactions.
 void

src/xrpld/app/tx/detail/Transactor.h

@@ -134,6 +134,20 @@ public:
 class TxConsequences;
 struct PreflightResult;
 
+enum class FeePayerType {
+    Account,
+    Delegate,
+    SponsorCoSigned,
+    SponsorPreFunded,
+};
+
+struct FeePayer
+{
+    Keylet keylet;
+    TypedField<STAmount> const& field;
+    FeePayerType type;
+};
+
 class Transactor
 {
 protected:
@@ -254,6 +268,9 @@ private:
     std::pair<TER, XRPAmount>
     reset(XRPAmount fee);
 
+    static FeePayer
+    getFeePayer(STTx const& tx);
+
     TER
     consumeSeqProxy(SLE::pointer const& sleAccount);
     TER

src/xrpld/app/tx/detail/applySteps.cpp

@@ -200,12 +200,12 @@ invoke_preclaim(PreclaimContext const& ctx)
                 if (result != tesSUCCESS)
                     return result;
 
-                result = T::checkSponsor(ctx.view, ctx.tx);
+                result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx));
 
                 if (result != tesSUCCESS)
                     return result;
 
-                result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx));
+                result = T::checkSponsor(ctx.view, ctx.tx);
 
                 if (result != tesSUCCESS)
                     return result;