From a5ac1d494403377cf1ecafff46409fd1bd277458 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 10 Jul 2026 14:11:52 -0400 Subject: [PATCH] more fixes (part 7) (#7787) --- .../xrpl/ledger/helpers/AccountRootHelpers.h | 32 ++--- include/xrpl/ledger/helpers/SponsorHelpers.h | 118 +++++++++++++----- .../ledger/helpers/AccountRootHelpers.cpp | 111 +++++++++------- src/libxrpl/ledger/helpers/SponsorHelpers.cpp | 67 +++++++--- src/libxrpl/tx/Transactor.cpp | 4 +- .../sponsor/SponsorshipTransfer.cpp | 27 ++-- src/libxrpl/tx/transactors/token/TrustSet.cpp | 93 ++++++++++---- src/test/app/Sponsor_test.cpp | 34 +++-- src/xrpld/app/misc/detail/TxQ.cpp | 4 +- 9 files changed, 318 insertions(+), 172 deletions(-) diff --git a/include/xrpl/ledger/helpers/AccountRootHelpers.h b/include/xrpl/ledger/helpers/AccountRootHelpers.h index f718cdab2f..ae8cf53f2f 100644 --- a/include/xrpl/ledger/helpers/AccountRootHelpers.h +++ b/include/xrpl/ledger/helpers/AccountRootHelpers.h @@ -333,25 +333,25 @@ pseudoAccountAddress(ReadView const& view, uint256 const& pseudoOwnerKey); /** Returns the list of fields that define an ACCOUNT_ROOT as a pseudo-account if set. - The list is constructed during initialization and is const after that. - Pseudo-account designator fields MUST be maintained by including the - SField::sMD_PseudoAccount flag in the SField definition. + The list is constructed during initialization and is const after that. + Pseudo-account designator fields MUST be maintained by including the + SField::sMD_PseudoAccount flag in the SField definition. */ [[nodiscard]] std::vector const& getPseudoAccountFields(); -/** Convenience overload that reads the account from the view. */ -[[nodiscard]] bool -isPseudoAccount(SLE::const_ref sleAcct, std::set const& pseudoFieldFilter = {}); +/** Returns true if and only if sleAcct is a pseudo-account or specific + pseudo-accounts in pseudoFieldFilter. -/** Convenience overload that reads the account from the view. - * - * @param view The ledger view to read from - * @param accountId The account ID to check - * @param pseudoFieldFilter Optional set of specific pseudo-account fields to filter (default: - * empty) - * @return true if the account is a pseudo-account (or matches the filter), false otherwise - */ + Returns false if sleAcct is: + - NOT a pseudo-account OR + - NOT a ltACCOUNT_ROOT OR + - null pointer +*/ +[[nodiscard]] bool +isPseudoAccount(SLE::const_pointer sleAcct, std::set const& pseudoFieldFilter = {}); + +/** Convenience overload that reads the account from the view. */ [[nodiscard]] inline bool isPseudoAccount( ReadView const& view, @@ -374,8 +374,8 @@ createPseudoAccount(ApplyView& view, uint256 const& pseudoOwnerKey, SField const /** Checks the destination and tag. -- Checks that the SLE is not null. -- If the SLE requires a destination tag, checks that there is a tag. + - Checks that the SLE is not null. + - If the SLE requires a destination tag, checks that there is a tag. */ [[nodiscard]] TER checkDestinationAndTag(SLE::const_ref toSle, bool hasDestinationTag); diff --git a/include/xrpl/ledger/helpers/SponsorHelpers.h b/include/xrpl/ledger/helpers/SponsorHelpers.h index 350f57805d..fd2315b024 100644 --- a/include/xrpl/ledger/helpers/SponsorHelpers.h +++ b/include/xrpl/ledger/helpers/SponsorHelpers.h @@ -13,45 +13,25 @@ #include #include #include -#include namespace xrpl { -inline std::unordered_set const kReserveSponsorAllowed = { - // Explicitly allow-listed for v1. - ttDELEGATE_SET, - ttDEPOSIT_PREAUTH, - ttPAYMENT, - ttSIGNER_LIST_SET, - ttCHECK_CANCEL, - ttCHECK_CASH, - ttCHECK_CREATE, - ttESCROW_CANCEL, - ttESCROW_CREATE, - ttESCROW_FINISH, - ttPAYCHAN_CLAIM, - ttPAYCHAN_CREATE, - ttPAYCHAN_FUND, - ttCLAWBACK, - ttMPTOKEN_AUTHORIZE, - ttMPTOKEN_ISSUANCE_CREATE, - ttMPTOKEN_ISSUANCE_DESTROY, - ttMPTOKEN_ISSUANCE_SET, - ttTRUST_SET, - ttCREDENTIAL_ACCEPT, - ttCREDENTIAL_CREATE, - ttCREDENTIAL_DELETE, - ttACCOUNT_SET, - ttREGULAR_KEY_SET, - ttSPONSORSHIP_TRANSFER, -}; +/** Whether the given transaction type may use reserve sponsorship (v1). + * + * Reserve sponsorship is restricted to an explicit allow-list of transaction + * types; all others reject spfSponsorReserve at preflight. + */ +bool +isReserveSponsorAllowed(TxType txType); +/** Whether the transaction's fee is sponsored (sfSponsor present + spfSponsorFee set). */ inline bool isFeeSponsored(STTx const& tx) { return tx.isFieldPresent(sfSponsor) && ((tx.getFieldU32(sfSponsorFlags) & spfSponsorFee) != 0u); } +/** Whether the transaction's reserve is sponsored (sfSponsor present + spfSponsorReserve set). */ inline bool isReserveSponsored(STTx const& tx) { @@ -59,12 +39,28 @@ isReserveSponsored(STTx const& tx) ((tx.getFieldU32(sfSponsorFlags) & spfSponsorReserve) != 0u); } +/** Return the AccountID of the transaction's reserve sponsor, or nullopt if unsponsored. */ std::optional getTxReserveSponsorID(STTx const& tx); +/** Return a mutable SLE for the transaction's reserve sponsor account. + * + * @param ctx The apply-view context (view + tx) + * @return The sponsor account SLE, a null pointer if the tx is not + * reserve-sponsored, or tecINTERNAL if the sponsor account cannot + * be loaded (an already-checked invariant). + */ std::expected getTxReserveSponsor(ApplyViewContext ctx); +/** Return a read-only SLE for the transaction's reserve sponsor account. + * + * @param view The ledger read view + * @param tx The transaction to inspect + * @return The sponsor account SLE, a null pointer if the tx is not + * reserve-sponsored, or tecINTERNAL if the sponsor account cannot + * be loaded (an already-checked invariant). + */ std::expected getTxReserveSponsor(ReadView const& view, STTx const& tx); @@ -84,15 +80,38 @@ getTxReserveSponsor(ReadView const& view, STTx const& tx); [[nodiscard]] std::expected getEffectiveTxReserveSponsor(ApplyViewContext ctx, SLE::const_ref accountSle); +/** Return the AccountID stored in the given sponsor field of a ledger entry, or nullopt if absent. + */ std::optional getLedgerEntryReserveSponsorID(SLE::const_ref sle, SF_ACCOUNT const& field = sfSponsor); +/** Return a mutable SLE for the reserve sponsor recorded on a ledger entry. + * + * Reads the sponsor AccountID from @p field on @p sle and peeks the + * corresponding account root in @p view. + * + * @param view The mutable apply view + * @param sle The ledger entry whose sponsor field is inspected + * @param field The field that holds the sponsor AccountID (defaults to sfSponsor) + * @return The sponsor account SLE, or a null pointer if the entry is unsponsored. + */ SLE::pointer getLedgerEntryReserveSponsor( ApplyView& view, SLE::const_ref sle, SF_ACCOUNT const& field = sfSponsor); +/** Stamp a reserve sponsor onto a ledger entry using an explicit sponsor SLE. + * + * Sets @p field on @p sle to the AccountID from @p sponsorSle. A no-op when + * @p sponsorSle is null (unsponsored). For RippleState entries the field must + * be sfHighSponsor or sfLowSponsor; for all other entry types it must be + * sfSponsor. + * + * @param sle The ledger entry to stamp + * @param sponsorSle The sponsor's account root SLE (null → no-op) + * @param field The sponsor field to set (defaults to sfSponsor) + */ void addSponsorToLedgerEntry( SLE::ref sle, @@ -110,18 +129,55 @@ addSponsorToLedgerEntry( void addSponsorToLedgerEntry(ApplyViewContext ctx, SLE::ref sle, SF_ACCOUNT const& field = sfSponsor); +/** Remove the reserve sponsor field from a ledger entry. + * + * A no-op when @p field is not present on @p sle. For RippleState entries + * the field must be sfHighSponsor or sfLowSponsor; for all other entry types + * it must be sfSponsor. + * + * @param sle The ledger entry to modify + * @param field The sponsor field to clear (defaults to sfSponsor) + */ void removeSponsorFromLedgerEntry(SLE::ref sle, SF_ACCOUNT const& field = sfSponsor); -std::optional -getLedgerEntryOwner(ReadView const& view, SLE const& sle, AccountID const& account); +/** Whether @p account is the owner of a ledger entry for sponsorship purposes. + * + * Ownership rules vary by entry type. For RippleState entries the owner is + * whichever side of the trust line holds the reserve. For credentials, the + * owner is the subject once accepted and the issuer before acceptance. + * + * @param view The ledger read view (used for SignerList lookup) + * @param sle The ledger entry whose owner is checked + * @param account The candidate account to match against + * @return true if @p account owns @p sle, false otherwise. + */ +bool +isLedgerEntryOwner(ReadView const& view, SLE const& sle, AccountID const& account); +/** Whether this ledger entry type can have a reserve sponsor attached to it. */ bool isLedgerEntrySupportedBySponsorship(SLE const& sle); +/** Return the number of owner-count units the ledger entry consumes. + * + * Most entries cost 1. Exceptions: Oracles scale with their price-data series + * size, Vaults cost 2 (vault + pseudo-account), and legacy SignerList entries + * (pre-MultiSignReserve) cost 2 + signer count. + */ std::uint32_t getLedgerEntryOwnerCount(SLE const& sle); +/** Return the SField used to store the reserve sponsor for @p owner on @p sle. + * + * For most entry types this is sfSponsor. RippleState entries use + * sfHighSponsor or sfLowSponsor depending on which side of the trust line + * @p owner holds. + * + * @param sle The ledger entry + * @param owner The account whose sponsor field is needed + * @return sfHighSponsor, sfLowSponsor, or sfSponsor as appropriate. + */ SF_ACCOUNT const& getLedgerEntrySponsorField(SLE const& sle, AccountID const& owner); diff --git a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp index 60f358a136..faca4ebfb6 100644 --- a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp +++ b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp @@ -146,58 +146,81 @@ adjustOwnerCountSigned( std::int32_t adjustment, beast::Journal j) { - XRPL_ASSERT(accountSle, "xrpl::adjustOwnerCountSigned : valid account sle"); - if (!accountSle) - return; // LCOV_EXCL_LINE - - auto const accountID = accountSle->getAccountID(sfAccount); - bool const validType = accountSle->getType() == ltACCOUNT_ROOT; - XRPL_ASSERT(validType, "xrpl::adjustOwnerCountSigned : valid account sle type"); - if (!validType) - return; // LCOV_EXCL_LINE - - XRPL_ASSERT(adjustment, "xrpl::adjustOwnerCountSigned : nonzero adjustment input"); - - OwnerCounts const currentOwnerCount(accountSle); - OwnerCounts totalOwnerCount(currentOwnerCount); - - if (sponsorSle) + if (view.rules().enabled(featureSponsor)) { - XRPL_ASSERT( - view.rules().enabled(featureSponsor), - "xrpl::getTxReserveSponsor : sponsor exists + Sponsor enabled"); - bool const validSponsorType = sponsorSle->getType() == ltACCOUNT_ROOT; - XRPL_ASSERT(validSponsorType, "xrpl::adjustOwnerCountSigned : valid sponsor sle type"); - if (!validSponsorType) + XRPL_ASSERT(accountSle, "xrpl::adjustOwnerCountSigned : valid account sle"); + if (!accountSle) return; // LCOV_EXCL_LINE - auto const sponsorID = sponsorSle->getAccountID(sfAccount); - totalOwnerCount.sponsored = - adjustOwnerCountImpl(view, accountSle, sfSponsoredOwnerCount, accountID, adjustment, j); + auto const accountID = accountSle->getAccountID(sfAccount); + bool const validType = accountSle->getType() == ltACCOUNT_ROOT; + XRPL_ASSERT(validType, "xrpl::adjustOwnerCountSigned : valid account sle type"); + if (!validType) + return; // LCOV_EXCL_LINE + XRPL_ASSERT(adjustment, "xrpl::adjustOwnerCountSigned : nonzero adjustment input"); + + OwnerCounts const currentOwnerCount(accountSle); + OwnerCounts totalOwnerCount(currentOwnerCount); + + if (sponsorSle) { - OwnerCounts const sponsorCurrent(sponsorSle); - OwnerCounts sponsorAdjustment(sponsorCurrent); - sponsorAdjustment.sponsoring = adjustOwnerCountImpl( - view, sponsorSle, sfSponsoringOwnerCount, sponsorID, adjustment, j); - view.adjustOwnerCountHook(sponsorID, sponsorCurrent, sponsorAdjustment); + bool const validSponsorType = sponsorSle->getType() == ltACCOUNT_ROOT; + XRPL_ASSERT(validSponsorType, "xrpl::adjustOwnerCountSigned : valid sponsor sle type"); + if (!validSponsorType) + return; // LCOV_EXCL_LINE + auto const sponsorID = sponsorSle->getAccountID(sfAccount); + + totalOwnerCount.sponsored = adjustOwnerCountImpl( + view, accountSle, sfSponsoredOwnerCount, accountID, adjustment, j); + + { + OwnerCounts const sponsorCurrent(sponsorSle); + OwnerCounts sponsorAdjustment(sponsorCurrent); + sponsorAdjustment.sponsoring = adjustOwnerCountImpl( + view, sponsorSle, sfSponsoringOwnerCount, sponsorID, adjustment, j); + view.adjustOwnerCountHook(sponsorID, sponsorCurrent, sponsorAdjustment); + } + + auto sponsorshipSle = view.peek(keylet::sponsorship(sponsorID, accountID)); + if (sponsorshipSle && adjustment > 0) + { + // Only decrease the pre-funded ReserveCount on Sponsorship if we assign new + // objects. Removing/reassigning ownership of the object doesn't increase + // RemainingOwnerCount back. Don't call hook because this counter is not something + // that requires reserve (like other sf...OwnerCounts do). + adjustOwnerCountImpl( + view, sponsorshipSle, sfRemainingOwnerCount, sponsorID, -adjustment, j); + } } - auto sponsorshipSle = view.peek(keylet::sponsorship(sponsorID, accountID)); - if (sponsorshipSle && adjustment > 0) - { - // Only decrease the pre-funded ReserveCount on Sponsorship if we assign new objects. - // Removing/reassigning ownership of the object doesn't increase RemainingOwnerCount - // back. Don't call hook because this counter is not something that requires reserve - // (like other sf...OwnerCounts do). - adjustOwnerCountImpl( - view, sponsorshipSle, sfRemainingOwnerCount, sponsorID, -adjustment, j); - } + totalOwnerCount.owner = + adjustOwnerCountImpl(view, accountSle, sfOwnerCount, accountID, adjustment, j); + view.adjustOwnerCountHook(accountID, currentOwnerCount, totalOwnerCount); } + else + { + XRPL_ASSERT(accountSle, "xrpl::adjustOwnerCountSigned : valid account sle"); + if (!accountSle) + return; + // the remaining are only asserts to preserve existing behavior + XRPL_ASSERT(sponsorSle == nullptr, "xrpl::adjustOwnerCountSigned : sponsor not enabled"); + XRPL_ASSERT( + accountSle->getType() == ltACCOUNT_ROOT, + "xrpl::adjustOwnerCountSigned : valid account sle type"); + XRPL_ASSERT(adjustment, "xrpl::adjustOwnerCount : nonzero adjustment input"); + std::uint32_t const current{accountSle->getFieldU32(sfOwnerCount)}; + AccountID const id = (*accountSle)[sfAccount]; + std::uint32_t const adjusted = confineOwnerCount(current, adjustment, id, j); - totalOwnerCount.owner = - adjustOwnerCountImpl(view, accountSle, sfOwnerCount, accountID, adjustment, j); - view.adjustOwnerCountHook(accountID, currentOwnerCount, totalOwnerCount); + OwnerCounts const currentOwnerCount(accountSle); + OwnerCounts finalOwnerCount(currentOwnerCount); + finalOwnerCount.owner = adjusted; + + view.adjustOwnerCountHook(id, currentOwnerCount, finalOwnerCount); + accountSle->at(sfOwnerCount) = adjusted; + view.update(accountSle); + } } } // namespace @@ -524,7 +547,7 @@ getPseudoAccountFields() } [[nodiscard]] bool -isPseudoAccount(SLE::const_ref sleAcct, std::set const& pseudoFieldFilter) +isPseudoAccount(SLE::const_pointer sleAcct, std::set const& pseudoFieldFilter) { auto const& fields = getPseudoAccountFields(); diff --git a/src/libxrpl/ledger/helpers/SponsorHelpers.cpp b/src/libxrpl/ledger/helpers/SponsorHelpers.cpp index d8e5023b3c..7e0c041854 100644 --- a/src/libxrpl/ledger/helpers/SponsorHelpers.cpp +++ b/src/libxrpl/ledger/helpers/SponsorHelpers.cpp @@ -15,13 +15,51 @@ #include #include #include +#include #include #include #include +#include namespace xrpl { +bool +isReserveSponsorAllowed(TxType txType) +{ + // Transaction types explicitly allow-listed for reserve sponsorship, for + // v1. Lazily-initialized function-local static: constructed once on first + // use, with no startup cost paid by clients that never call this. + static std::unordered_set const kReserveSponsorAllowed = { + ttDELEGATE_SET, + ttDEPOSIT_PREAUTH, + ttPAYMENT, + ttSIGNER_LIST_SET, + ttCHECK_CANCEL, + ttCHECK_CASH, + ttCHECK_CREATE, + ttESCROW_CANCEL, + ttESCROW_CREATE, + ttESCROW_FINISH, + ttPAYCHAN_CLAIM, + ttPAYCHAN_CREATE, + ttPAYCHAN_FUND, + ttCLAWBACK, + ttMPTOKEN_AUTHORIZE, + ttMPTOKEN_ISSUANCE_CREATE, + ttMPTOKEN_ISSUANCE_DESTROY, + ttMPTOKEN_ISSUANCE_SET, + ttTRUST_SET, + ttCREDENTIAL_ACCEPT, + ttCREDENTIAL_CREATE, + ttCREDENTIAL_DELETE, + ttACCOUNT_SET, + ttREGULAR_KEY_SET, + ttSPONSORSHIP_TRANSFER, + }; + return kReserveSponsorAllowed.contains(txType); +} + std::optional getTxReserveSponsorID(STTx const& tx) { @@ -175,8 +213,8 @@ removeSponsorFromLedgerEntry(SLE::ref sle, SF_ACCOUNT const& field) } } -std::optional -getLedgerEntryOwner(ReadView const& view, SLE const& sle, AccountID const& account) +bool +isLedgerEntryOwner(ReadView const& view, SLE const& sle, AccountID const& account) { switch (sle.getType()) { @@ -186,44 +224,41 @@ getLedgerEntryOwner(ReadView const& view, SLE const& sle, AccountID const& accou case ltMPTOKEN: case ltDELEGATE: case ltDEPOSIT_PREAUTH: - return sle.getAccountID(sfAccount); + return sle.getAccountID(sfAccount) == account; case ltMPTOKEN_ISSUANCE: - return sle.getAccountID(sfIssuer); + return sle.getAccountID(sfIssuer) == account; case ltSIGNER_LIST: { auto const signerList = view.read(keylet::signerList(account)); if (!signerList) - return std::nullopt; - if (signerList->key() == sle.key()) - return account; - return std::nullopt; + return false; + return signerList->key() == sle.key(); } case ltCREDENTIAL: { - if (sle.isFlag(lsfAccepted)) - return sle.getAccountID(sfSubject); - return sle.getAccountID(sfIssuer); + auto const& ownerField = sle.isFlag(lsfAccepted) ? sfSubject : sfIssuer; + return sle.getAccountID(ownerField) == account; } case ltRIPPLE_STATE: { if (sle.isFlag(lsfHighReserve)) { auto const highAccount = sle.getFieldAmount(sfHighLimit).getIssuer(); if (highAccount == account) - return highAccount; + return true; } if (sle.isFlag(lsfLowReserve)) { auto const lowAccount = sle.getFieldAmount(sfLowLimit).getIssuer(); if (lowAccount == account) - return lowAccount; + return true; } // Reachable: the sponsee may be a third party or the side of the // line that holds no reserve (e.g. the issuer). Callers map this // to tecNO_PERMISSION. - return std::nullopt; + return false; } default: // LCOV_EXCL_START - UNREACHABLE("xrpl::getLedgerEntryOwner : object is not supported by sponsorship."); - return std::nullopt; + UNREACHABLE("xrpl::isLedgerEntryOwner : object is not supported by sponsorship."); + return false; // LCOV_EXCL_STOP }; } diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index fb036a16b4..065dead1fd 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -203,7 +203,7 @@ preflight1Sponsor(PreflightContext const& ctx) // transaction types, for v1. All other tx types reject spfSponsorReserve here. if (isReserveSponsored(ctx.tx)) { - if (!kReserveSponsorAllowed.contains(ctx.tx.getTxnType())) + if (!isReserveSponsorAllowed(ctx.tx.getTxnType())) { JLOG(ctx.j.debug()) << "preflight1: spfSponsorReserve not allowed for this transaction type"; @@ -214,7 +214,7 @@ preflight1Sponsor(PreflightContext const& ctx) if (hasSponsor && ctx.tx.getAccountID(sfSponsor) == ctx.tx.getAccountID(sfAccount)) { - JLOG(ctx.j.debug()) << "preflight1: Sponsor account should be the same as the account"; + JLOG(ctx.j.debug()) << "preflight1: Sponsor account cannot be the same as the account"; return temMALFORMED; } diff --git a/src/libxrpl/tx/transactors/sponsor/SponsorshipTransfer.cpp b/src/libxrpl/tx/transactors/sponsor/SponsorshipTransfer.cpp index fb2c1104ef..0e036649fd 100644 --- a/src/libxrpl/tx/transactors/sponsor/SponsorshipTransfer.cpp +++ b/src/libxrpl/tx/transactors/sponsor/SponsorshipTransfer.cpp @@ -247,15 +247,14 @@ SponsorshipTransfer::preclaim(PreclaimContext const& ctx) if (!isLedgerEntrySupportedBySponsorship(*objectSle)) return tecNO_PERMISSION; - auto const owner = getLedgerEntryOwner(ctx.view, *objectSle, sponseeID); - if (!owner.has_value() || owner.value() != sponseeID) + if (!isLedgerEntryOwner(ctx.view, *objectSle, 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()); + sponsorField = &getLedgerEntrySponsorField(*objectSle, sponseeID); } bool const isSponsored = targetSle->isFieldPresent(*sponsorField); @@ -321,17 +320,17 @@ SponsorshipTransfer::doApply() if (!objectSle) return tefINTERNAL; // LCOV_EXCL_LINE - auto const ownerID = getLedgerEntryOwner(view(), *objectSle, sponseeID); - if (!ownerID) + // preclaim established that the sponsee owns the object. + if (!isLedgerEntryOwner(view(), *objectSle, sponseeID)) return tefINTERNAL; // LCOV_EXCL_LINE - auto const ownerSle = view().peek(keylet::account(*ownerID)); + auto const ownerSle = view().peek(keylet::account(sponseeID)); if (!ownerSle) return tefINTERNAL; // LCOV_EXCL_LINE auto const ownerCountDelta = static_cast(getLedgerEntryOwnerCount(*objectSle)); - auto const& sponsorField = getLedgerEntrySponsorField(*objectSle, *ownerID); + auto const& sponsorField = getLedgerEntrySponsorField(*objectSle, sponseeID); if (isCreate || isReassign) { @@ -413,17 +412,9 @@ SponsorshipTransfer::doApply() if (!oldSponsorSle) return tefINTERNAL; // LCOV_EXCL_LINE - // The owner takes the reserve burden back when the object is - // no longer sponsored. - if (auto const ter = checkReserve( - ctx_.getApplyViewContext(), - ownerSle, - balanceBeforeFee(ownerSle), - SLE::pointer(), - {.ownerCountDelta = ownerCountDelta}, - ctx_.journal); - !isTesSuccess(ter)) - return ter; + // The owner reclaims the reserve burden when the object is no longer sponsored. + // We do not check the sponsee's reserve here (via `checkReserve`) so that a sponsor can + // always end a sponsorship, even if the sponsee lacks sufficient reserve. // Decrement sponsored count if (auto const ter = decrementSponsorCount( diff --git a/src/libxrpl/tx/transactors/token/TrustSet.cpp b/src/libxrpl/tx/transactors/token/TrustSet.cpp index 9f79e5fb3a..3d078ce825 100644 --- a/src/libxrpl/tx/transactors/token/TrustSet.cpp +++ b/src/libxrpl/tx/transactors/token/TrustSet.cpp @@ -329,6 +329,10 @@ TrustSet::doApply() // 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 = !sponsorSle && (ownerCount(sle, j_) < 2); + std::uint32_t const uOwnerCount = ownerCount(sle, j_); + XRPAmount const reserveCreate( + (uOwnerCount < 2) ? XRPAmount(beast::kZero) + : accountReserve(view(), sle, j_, {.ownerCountDelta = 1})); std::uint32_t const uQualityIn(bQualityIn ? ctx_.tx.getFieldU32(sfQualityIn) : 0); std::uint32_t uQualityOut(bQualityOut ? ctx_.tx.getFieldU32(sfQualityOut) : 0); @@ -608,36 +612,66 @@ TrustSet::doApply() if (uFlagsIn != uFlagsOut) sleRippleState->setFieldU32(sfFlags, uFlagsOut); - if (bDefault || badCurrency() == currency) + if (view().rules().enabled(featureSponsor)) { - // Delete. + if (bDefault || badCurrency() == currency) + { + // Delete. - terResult = trustDelete(view(), sleRippleState, uLowAccountID, uHighAccountID, viewJ); - } - // Reserve is not scaled by load. - else if ( - auto const ret = checkReserve( - ctx_.getApplyViewContext(), - sle, - preFeeBalance_, - sponsorSle, - {}, - j_, - tecINSUF_RESERVE_LINE); - !freeTrustLine && bReserveIncrease && !isTesSuccess(ret)) - { - JLOG(j_.trace()) << "Delay transaction: Insufficent reserve to " - "add trust line."; + terResult = + trustDelete(view(), sleRippleState, uLowAccountID, uHighAccountID, viewJ); + } + // Reserve is not scaled by load + else if ( + auto const ret = checkReserve( + ctx_.getApplyViewContext(), + sle, + preFeeBalance_, + sponsorSle, + {}, + j_, + tecINSUF_RESERVE_LINE); + !freeTrustLine && bReserveIncrease && !isTesSuccess(ret)) + { + JLOG(j_.trace()) << "Delay transaction: Insufficent reserve to " + "add trust line."; - // Another transaction could provide XRP to the account and then - // this transaction would succeed. - terResult = ret; + // Another transaction could provide XRP to the account and then + // this transaction would succeed. + terResult = ret; + } + else + { + view().update(sleRippleState); + + JLOG(j_.trace()) << "Modify ripple line"; + } } else { - view().update(sleRippleState); + if (bDefault || badCurrency() == currency) + { + // Delete. - JLOG(j_.trace()) << "Modify ripple line"; + terResult = + trustDelete(view(), sleRippleState, uLowAccountID, uHighAccountID, viewJ); + } + // Reserve is not scaled by load. + else if (bReserveIncrease && preFeeBalance_ < reserveCreate) + { + JLOG(j_.trace()) << "Delay transaction: Insufficent reserve to " + "add trust line."; + + // Another transaction could provide XRP to the account and then + // this transaction would succeed. + terResult = tecINSUF_RESERVE_LINE; + } + else + { + view().update(sleRippleState); + + JLOG(j_.trace()) << "Modify ripple line"; + } } } // Line does not exist. @@ -652,6 +686,16 @@ TrustSet::doApply() JLOG(j_.trace()) << "Redundant: Setting non-existent ripple line to defaults."; return tecNO_LINE_REDUNDANT; } + // reserve is not scaled by load + else if (!view().rules().enabled(featureSponsor) && preFeeBalance_ < reserveCreate) + { + JLOG(j_.trace()) << "Delay transaction: Line does not exist. " + "Insufficent reserve to create line."; + + // Another transaction could create the account and then this + // transaction would succeed. + terResult = tecNO_LINE_INSUF_RESERVE; + } else if ( auto const ret = checkReserve( ctx_.getApplyViewContext(), @@ -661,7 +705,8 @@ TrustSet::doApply() {.ownerCountDelta = 1}, j_, tecNO_LINE_INSUF_RESERVE); - !freeTrustLine && !isTesSuccess(ret)) // Reserve is not scaled by load. + view().rules().enabled(featureSponsor) && !freeTrustLine && + !isTesSuccess(ret)) // Reserve is not scaled by load. { JLOG(j_.trace()) << "Delay transaction: Line does not exist. " "Insufficent reserve to create line."; diff --git a/src/test/app/Sponsor_test.cpp b/src/test/app/Sponsor_test.cpp index de7174e5e7..4ad935c460 100644 --- a/src/test/app/Sponsor_test.cpp +++ b/src/test/app/Sponsor_test.cpp @@ -1223,27 +1223,10 @@ public: BEAST_EXPECT(sle2->isFieldPresent(sfSponsor)); BEAST_EXPECT(sle2->getAccountID(sfSponsor) == sponsor2.id()); - // dissolve sponsor + // dissolve sponsor: ending an object sponsorship succeeds even + // when the sponsee lacks sufficient reserve to reclaim the object. adjustAccountXRPBalance(env, alice, reserve(env, 1) - drops(1)); - env(sponsor::transfer(alice, tfSponsorshipEnd, checkId), Ter(tecINSUFFICIENT_RESERVE)); - env.close(); - - adjustAccountXRPBalance(env, alice, reserve(env, 1)); - - // object doesn't sponsored - auto const ticketSeq = env.seq(alice); - env(ticket::create(alice, 1)); - env.close(); - auto ticketId = keylet::ticket(alice, ticketSeq + 1).key; - BEAST_EXPECT(env.le(keylet::unchecked(ticketId))); - env(sponsor::transfer(alice, tfSponsorshipEnd, ticketId), Ter(tecNO_PERMISSION)); - env.close(); - env(noop(alice), ticket::Use(ticketSeq + 1)); - env.close(); - - adjustAccountXRPBalance(env, alice, reserve(env, 1)); - env(sponsor::transfer(alice, tfSponsorshipEnd, checkId)); env.close(); @@ -1260,6 +1243,19 @@ public: !env.le(keylet::account(sponsor2))->isFieldPresent(sfSponsoringOwnerCount)); auto const sle3 = env.le(keylet::unchecked(checkId)); BEAST_EXPECT(!sle3->isFieldPresent(sfSponsor)); + + // Ending sponsorship on an object that is not sponsored (a ticket, + // which cannot be sponsored) is rejected. + adjustAccountXRPBalance(env, alice, reserve(env, 2)); + auto const ticketSeq = env.seq(alice); + env(ticket::create(alice, 1)); + env.close(); + auto ticketId = keylet::ticket(alice, ticketSeq + 1).key; + BEAST_EXPECT(env.le(keylet::unchecked(ticketId))); + env(sponsor::transfer(alice, tfSponsorshipEnd, ticketId), Ter(tecNO_PERMISSION)); + env.close(); + env(noop(alice), ticket::Use(ticketSeq + 1)); + env.close(); } { // sponsor object (pre-funded + no ltSponsorship entry) diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index 2092b4ec27..041d2ade1e 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -409,8 +409,8 @@ TxQ::canBeHeld( // Disallow delegated transactions from being queued. if (tx.isFieldPresent(sfDelegate)) return telCAN_NOT_QUEUE; - // Disallow sponsored transactions from being queued. - if (tx.isFieldPresent(sfSponsor) && isFeeSponsored(tx)) + // Disallow fee-sponsored transactions from being queued. + if (isFeeSponsored(tx)) return telCAN_NOT_QUEUE; {