diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index a06bf7fb45..a1e529fe18 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -106,9 +106,6 @@ public: std::uint32_t getSeqValue() const; - AccountID - getFeePayer() const; - boost::container::flat_set getMentionedAccounts() const; diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index 95e9c27816..39ba667ead 100644 --- a/include/xrpl/protocol/TER.h +++ b/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 }; //------------------------------------------------------------------------------ diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 3129c8635e..134319c7d3 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/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) { diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index 1093d8fc6d..3ab4fe3f20 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/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."), }; diff --git a/src/test/app/Sponsor_test.cpp b/src/test/app/Sponsor_test.cpp index ff96b236e5..dd3607cc17 100644 --- a/src/test/app/Sponsor_test.cpp +++ b/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(); } }; diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index d98209686d..dd0f6ea9f8 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -371,53 +371,44 @@ Transactor::checkFee(PreclaimContext const& ctx, XRPAmount baseFee) std::optional 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 diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index d5beac5805..0877d16352 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/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 const& field; + FeePayerType type; +}; + class Transactor { protected: @@ -254,6 +268,9 @@ private: std::pair reset(XRPAmount fee); + static FeePayer + getFeePayer(STTx const& tx); + TER consumeSeqProxy(SLE::pointer const& sleAccount); TER diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index 9b315462b2..79dd713451 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/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;