From 7817245314a1a6bf8854e1bfcd3add16050b08d6 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 1 Jul 2026 13:18:20 -0400 Subject: [PATCH] refactor: Switch `checkInsufficientReserve` from `ReadView` to `ApplyView` (#7667) --- .../xrpl/ledger/helpers/AccountRootHelpers.h | 2 +- include/xrpl/ledger/helpers/SponsorHelpers.h | 22 +++ .../ledger/helpers/AccountRootHelpers.cpp | 2 +- .../Sponsor/SponsorshipTransfer.cpp | 163 +++++++++++------- 4 files changed, 123 insertions(+), 66 deletions(-) diff --git a/include/xrpl/ledger/helpers/AccountRootHelpers.h b/include/xrpl/ledger/helpers/AccountRootHelpers.h index 403626d06f..6c8233fa61 100644 --- a/include/xrpl/ledger/helpers/AccountRootHelpers.h +++ b/include/xrpl/ledger/helpers/AccountRootHelpers.h @@ -109,7 +109,7 @@ baseAccountReserve(ReadView const& view, std::int32_t ownerCount, std::int32_t a */ [[nodiscard]] TER checkInsufficientReserve( - ReadView const& view, + ApplyView const& view, STTx const& tx, SLE::const_ref accSle, STAmount const& accBalance, diff --git a/include/xrpl/ledger/helpers/SponsorHelpers.h b/include/xrpl/ledger/helpers/SponsorHelpers.h index 93614e5e17..4b35e155b4 100644 --- a/include/xrpl/ledger/helpers/SponsorHelpers.h +++ b/include/xrpl/ledger/helpers/SponsorHelpers.h @@ -173,6 +173,28 @@ getLedgerEntryOwner(ReadView const& view, T const& sle, AccountID const& account }; } +template +inline bool +isLedgerEntrySupportedBySponsorship(T const& sle) +{ + switch (sle->getType()) + { + case ltCHECK: + case ltESCROW: + case ltPAYCHAN: + case ltMPTOKEN: + case ltDELEGATE: + case ltDEPOSIT_PREAUTH: + case ltMPTOKEN_ISSUANCE: + case ltSIGNER_LIST: + case ltCREDENTIAL: + case ltRIPPLE_STATE: + return true; + default: + return false; + }; +} + template inline std::uint32_t getLedgerEntryOwnerCount(T const& sle) diff --git a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp index 3264fa1a45..26a3a0fabf 100644 --- a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp +++ b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp @@ -302,7 +302,7 @@ baseAccountReserve(ReadView const& view, std::int32_t ownerCount, std::int32_t a TER checkInsufficientReserve( - ReadView const& view, + ApplyView const& view, STTx const& tx, SLE::const_ref accSle, STAmount const& accBalance, diff --git a/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp b/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp index deea6edc84..3840c390b1 100644 --- a/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp +++ b/src/libxrpl/tx/transactors/Sponsor/SponsorshipTransfer.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -147,30 +146,8 @@ SponsorshipTransfer::preclaim(PreclaimContext const& ctx) if (!sle) return tecNO_ENTRY; - // v1 scope: an object is only sponsorable via SponsorshipTransfer if - // its creating transaction type is itself permitted to set - // spfSponsorReserve (the allow-list in preflight1Sponsor). Otherwise - // an Oracle / Ticket / DID / etc. could be retroactively sponsored - // even though its creating tx cannot be, leaving downstream - // transactors with no path to maintain the sponsorship invariants. - switch (sle->getType()) - { - case ltDELEGATE: - case ltDEPOSIT_PREAUTH: - case ltMPTOKEN: - case ltMPTOKEN_ISSUANCE: - case ltCREDENTIAL: - case ltRIPPLE_STATE: - case ltSIGNER_LIST: - case ltCHECK: - case ltESCROW: - case ltPAYCHAN: - break; - default: - return tecNO_PERMISSION; - } - - std::uint32_t const ownerCountDelta = 1; + if (!isLedgerEntrySupportedBySponsorship(sle)) + return tecNO_PERMISSION; auto const owner = getLedgerEntryOwner(ctx.view, sle, sponseeID); if (!owner.has_value() || owner.value() != sponseeID) @@ -210,20 +187,6 @@ SponsorshipTransfer::preclaim(PreclaimContext const& ctx) if (account != sponsor && account != sponseeID) return tecNO_PERMISSION; } - - // check new sponsor have sufficient balance - // NOLINTNEXTLINE(readability-suspicious-call-argument) - if (auto const ter = checkInsufficientReserve( - ctx.view, - ctx.tx, - sponseeSle, - sponseeSle->getFieldAmount(sfBalance), - newSponsorSle, - ownerCountDelta, - 0, - ctx.j); - !isTesSuccess(ter)) - return ter; } else { @@ -259,23 +222,6 @@ SponsorshipTransfer::preclaim(PreclaimContext const& ctx) if (account != sponsor && account != sponseeID) return tecNO_PERMISSION; } - - // check account have sufficient balance - // In the case of removing an account sponsor, accSle should have no sfSponsor set - // (AccountReserve = 0). However, by setting accountCountDelta = 1 here, we are able to - // calculate the actual required Account Reserve. - // NOLINTNEXTLINE(readability-suspicious-call-argument) - if (auto const ter = checkInsufficientReserve( - ctx.view, - ctx.tx, - sponseeSle, - sponseeSle->getFieldAmount(sfBalance), - newSponsorSle, - 0, - 1, - ctx.j); - !isTesSuccess(ter)) - return ter; } return tesSUCCESS; @@ -338,6 +284,12 @@ SponsorshipTransfer::doApply() return tesSUCCESS; }; + auto const balanceBeforeFee = [&](SLE::const_ref sle) -> STAmount { + if (sle->getAccountID(sfAccount) == accountID_) + return STAmount{preFeeBalance_}; + return sle->getFieldAmount(sfBalance); + }; + if (isObjectSponsor) { auto const hasSignature = tx.isFieldPresent(sfSponsorSignature); @@ -363,6 +315,23 @@ SponsorshipTransfer::doApply() { auto const newSponsorID = tx.getAccountID(sfSponsor); XRPL_ASSERT(!!newSponsorID, "New sponsor is required when creating sponsorship"); + auto const newSponsorSle = view().peek(keylet::account(newSponsorID)); + if (!newSponsorSle) + return tefINTERNAL; // LCOV_EXCL_LINE + + // check new sponsor have sufficient balance + // NOLINTNEXTLINE(readability-suspicious-call-argument) + if (auto const ter = checkInsufficientReserve( + ctx_.view(), + ctx_.tx, + sponseeSle, + sponseeSle->getFieldAmount(sfBalance), + newSponsorSle, + ownerCountDelta, + 0, + ctx_.journal); + !isTesSuccess(ter)) + return ter; // update owner's sponsored count if (auto const ter = @@ -372,9 +341,6 @@ SponsorshipTransfer::doApply() view().update(ownerSle); // increment new sponsor's sponsoring count - auto const newSponsorSle = view().peek(keylet::account(newSponsorID)); - if (!newSponsorSle) - return tefINTERNAL; // LCOV_EXCL_LINE if (auto const ter = setSponsorFieldU32(newSponsorSle, sfSponsoringOwnerCount, ownerCountDelta); !isTesSuccess(ter)) @@ -398,14 +364,31 @@ SponsorshipTransfer::doApply() { auto const newSponsorID = tx.getAccountID(sfSponsor); XRPL_ASSERT(!!newSponsorID, "New sponsor is required when reassigning sponsorship"); + auto const newSponsorSle = view().peek(keylet::account(newSponsorID)); + if (!newSponsorSle) + return tefINTERNAL; // LCOV_EXCL_LINE auto const oldSponsorID = objSle->getAccountID(sponsorField); XRPL_ASSERT(!!oldSponsorID, "Old sponsor is required when reassigning sponsorship"); - - // decrement old sponsor's sponsoring count auto const oldSponsorSle = view().peek(keylet::account(oldSponsorID)); if (!oldSponsorSle) return tefINTERNAL; // LCOV_EXCL_LINE + + // check new sponsor have sufficient balance + // NOLINTNEXTLINE(readability-suspicious-call-argument) + if (auto const ter = checkInsufficientReserve( + ctx_.view(), + ctx_.tx, + sponseeSle, + sponseeSle->getFieldAmount(sfBalance), + newSponsorSle, + ownerCountDelta, + 0, + ctx_.journal); + !isTesSuccess(ter)) + return ter; + + // decrement old sponsor's sponsoring count if (auto const ter = setSponsorFieldU32(oldSponsorSle, sfSponsoringOwnerCount, -ownerCountDelta); !isTesSuccess(ter)) @@ -413,9 +396,6 @@ SponsorshipTransfer::doApply() view().update(oldSponsorSle); // increment new sponsor's sponsoring count - auto const newSponsorSle = view().peek(keylet::account(newSponsorID)); - if (!newSponsorSle) - return tefINTERNAL; // LCOV_EXCL_LINE if (auto const ter = setSponsorFieldU32(newSponsorSle, sfSponsoringOwnerCount, ownerCountDelta); !isTesSuccess(ter)) @@ -444,6 +424,20 @@ 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 = checkInsufficientReserve( + ctx_.view(), + ctx_.tx, + ownerSle, + balanceBeforeFee(ownerSle), + SLE::pointer(), + ownerCountDelta, + 0, + ctx_.journal); + !isTesSuccess(ter)) + return ter; + // decrement sponsored count if (auto const ter = setSponsorFieldU32(sponseeSle, sfSponsoredOwnerCount, -ownerCountDelta); @@ -473,6 +467,19 @@ SponsorshipTransfer::doApply() auto const newSponsorSle = view().peek(keylet::account(newSponsorID)); if (!newSponsorSle) return tefINTERNAL; // LCOV_EXCL_LINE + + if (auto const ter = checkInsufficientReserve( + ctx_.view(), + ctx_.tx, + sponseeSle, + sponseeSle->getFieldAmount(sfBalance), + newSponsorSle, + 0, + 1, + ctx_.journal); + !isTesSuccess(ter)) + return ter; + if (auto const ter = setSponsorFieldU32(newSponsorSle, sfSponsoringAccountCount, 1); !isTesSuccess(ter)) return ter; @@ -490,6 +497,19 @@ SponsorshipTransfer::doApply() auto const newSponsorSle = view().peek(keylet::account(newSponsorID)); if (!newSponsorSle) return tefINTERNAL; // LCOV_EXCL_LINE + + if (auto const ter = checkInsufficientReserve( + ctx_.view(), + ctx_.tx, + sponseeSle, + sponseeSle->getFieldAmount(sfBalance), + newSponsorSle, + 0, + 1, + ctx_.journal); + !isTesSuccess(ter)) + return ter; + if (auto const ter = setSponsorFieldU32(newSponsorSle, sfSponsoringAccountCount, 1); !isTesSuccess(ter)) return ter; @@ -513,6 +533,21 @@ SponsorshipTransfer::doApply() { // dissolve account sponsor auto const oldSponsorID = sponseeSle->getAccountID(sfSponsor); + + // The sponsee must be able to hold its own account reserve after + // the sponsorship is removed. + if (auto const ter = checkInsufficientReserve( + ctx_.view(), + ctx_.tx, + sponseeSle, + balanceBeforeFee(sponseeSle), + SLE::pointer(), + 0, + 1, + ctx_.journal); + !isTesSuccess(ter)) + return ter; + sponseeSle->makeFieldAbsent(sfSponsor); view().update(sponseeSle);