From 506ab2b59025b763f69a5bd974a80da09fb65ae2 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 24 Jun 2026 12:54:46 -0400 Subject: [PATCH] more cleanup --- .../xrpl/ledger/helpers/AccountRootHelpers.h | 90 +++++++----- .../ledger/helpers/AccountRootHelpers.cpp | 129 ++---------------- src/libxrpl/ledger/helpers/MPTokenHelpers.cpp | 3 +- src/libxrpl/tx/Transactor.cpp | 4 +- .../tx/transactors/bridge/XChainBridge.cpp | 2 +- .../tx/transactors/dex/AMMWithdraw.cpp | 3 +- .../tx/transactors/payment/Payment.cpp | 2 +- src/libxrpl/tx/transactors/token/TrustSet.cpp | 2 +- src/test/app/LoanBroker_test.cpp | 3 +- src/test/app/Loan_test.cpp | 5 +- src/test/jtx/TestHelpers.h | 18 +++ 11 files changed, 93 insertions(+), 168 deletions(-) diff --git a/include/xrpl/ledger/helpers/AccountRootHelpers.h b/include/xrpl/ledger/helpers/AccountRootHelpers.h index e19fe23393..e1096f149e 100644 --- a/include/xrpl/ledger/helpers/AccountRootHelpers.h +++ b/include/xrpl/ledger/helpers/AccountRootHelpers.h @@ -33,33 +33,56 @@ isGlobalFrozen(ReadView const& view, AccountID const& issuer); [[nodiscard]] XRPAmount xrpLiquid(ReadView const& view, AccountID const& id, std::int32_t ownerCountAdj, beast::Journal j); -/** Returns the account reserve, in drops. - Actual owner count can be adjusted by delta in ownerCountAdj - The reserve is calculated as - (ownerCount + "sponsoring object count" - "sponsored object count" + additionalOwnerCount) * - increment + (1 if not sponsored account + sponsoringAccountCount) * "reserve base" +/* Calculates the number of objects that the account is responsible for reserve-wise + sfOwnerCount - sfSponsoredOwnerCount + sfSponsoringOwnerCount */ -[[nodiscard]] XRPAmount -accountReserve( - ReadView const& view, - SLE::const_ref sle, - beast::Journal j, - std::int32_t ownerCountAdj = 0, - std::int32_t reserveCountAdj = 0); - -[[nodiscard]] inline XRPAmount -accountReserve( - ReadView const& view, - AccountID const& id, - beast::Journal j, - std::int32_t ownerCountAdj = 0, - std::int32_t reserveCountAdj = 0) +inline std::uint32_t +objectOwnerCount(SLE::const_pointer accountSle, std::int32_t delta = 0) { - return accountReserve(view, view.read(keylet::account(id)), j, ownerCountAdj, reserveCountAdj); + XRPL_ASSERT( + accountSle && accountSle->getType() == ltACCOUNT_ROOT, + "xrpl::objectOwnerCount : valid account sle"); + return accountSle->at(sfOwnerCount) - accountSle->at(sfSponsoredOwnerCount) + + accountSle->at(sfSponsoringOwnerCount) + delta; } -XRPAmount -baseAccountReserve(ReadView const& view, std::int32_t ownerCount); +/* Calculates the number of accounts that the account is responsible for reserve-wise + (accountIsSponsored ? 0 : 1) + sfSponsoringAccountCount +*/ +inline std::uint32_t +accountOwnerCount(SLE::const_pointer accountSle, std::int32_t delta = 0) +{ + XRPL_ASSERT( + accountSle && accountSle->getType() == ltACCOUNT_ROOT, + "xrpl::accountOwnerCount : valid account sle"); + return (accountSle->isFieldPresent(sfSponsor) ? 0 : 1) + + accountSle->getFieldU32(sfSponsoringAccountCount) + delta; +} + +/* Calculates the total reserve, in XRP, that the account is currently responsible for + */ +inline XRPAmount +totalAccountReserve( + ReadView const& view, + SLE::const_pointer accountSle, + std::int32_t objectDelta = 0, + std::int32_t accountDelta = 0) +{ + auto const& fees = view.fees(); + return (fees.reserve * accountOwnerCount(accountSle, accountDelta)) + + (fees.increment * objectOwnerCount(accountSle, objectDelta)); +} + +[[nodiscard]] inline XRPAmount +totalAccountReserve( + ReadView const& view, + AccountID const& id, + std::int32_t ownerCountDelta = 0, + std::int32_t accountCountDelta = 0) +{ + return totalAccountReserve( + view, view.read(keylet::account(id)), ownerCountDelta, accountCountDelta); +} [[nodiscard]] TER checkInsufficientReserve( @@ -72,19 +95,12 @@ checkInsufficientReserve( std::int32_t reserveCountDelta = 0, beast::Journal j = beast::Journal{beast::Journal::getNullSink()}); -std::uint32_t -ownerCount( - ReadView const& view, - SLE::const_ref sle, - beast::Journal j, - std::int32_t ownerCountAdj = 0); - /** Adjust the owner count up or down. */ void adjustOwnerCount( ApplyView& view, - SLE::ref accountSle, - SLE::ref sponsorSle, + SLE::pointer accountSle, + SLE::pointer sponsorSle, std::int32_t amount, beast::Journal j = beast::Journal{beast::Journal::getNullSink()}); @@ -107,8 +123,8 @@ adjustOwnerCount( void adjustOwnerCountObj( ApplyView& view, - SLE::ref accountSle, - SLE::ref objectSle, + SLE::pointer accountSle, + SLE::pointer objectSle, std::int32_t amount, beast::Journal j = beast::Journal{beast::Journal::getNullSink()}); @@ -116,11 +132,11 @@ inline void adjustOwnerCountObj( ApplyView& view, AccountID const& account, - SLE::ref objectSle, + SLE::pointer objectSle, std::int32_t amount, beast::Journal j = beast::Journal{beast::Journal::getNullSink()}) { - SLE::ref accountSle = view.peek(keylet::account(account)); + SLE::pointer accountSle = view.peek(keylet::account(account)); adjustOwnerCountObj(view, accountSle, objectSle, amount, j); } @@ -158,7 +174,7 @@ getPseudoAccountFields(); - null pointer */ [[nodiscard]] bool -isPseudoAccount(SLE::const_ref sleAcct, std::set const& pseudoFieldFilter = {}); +isPseudoAccount(SLE::const_pointer sleAcct, std::set const& pseudoFieldFilter = {}); /** Convenience overload that reads the account from the view. */ [[nodiscard]] inline bool diff --git a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp index e222896822..c33c3618b6 100644 --- a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp +++ b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp @@ -85,85 +85,6 @@ confineOwnerCount( return adjusted; } -static std::uint32_t -ownerCountHlp( - ReadView const& view, - SLE::const_ref sle, - std::int32_t adjustment, - std::optional reportId, - beast::Journal j) -{ - AccountID const id = sle->getAccountID(sfAccount); - std::uint32_t const savedCount = sle->at(sfOwnerCount); - std::uint32_t const hookedCount = view.ownerCountHook(id, savedCount); - - std::uint32_t const sponsoredCount = sle->at(sfSponsoredOwnerCount); - std::uint32_t const sponsoringCount = sle->at(sfSponsoringOwnerCount); - - if (hookedCount < sponsoredCount) - { - Throw( - "xrpl::ownerCountHlp : OwnerCount must be greater than or equal to " - "SponsoredOwnerCount"); - } - - std::int64_t deltaCount = - static_cast(adjustment) - sponsoredCount + sponsoringCount; - if (deltaCount > std::numeric_limits::max()) - { - deltaCount = std::numeric_limits::max(); - JLOG(j.fatal()) << "Account " << id << " delta count exceeds max, " - << "adjustment: " << adjustment << ", sponsoredCount: " << sponsoredCount - << ", sponsoringOwnerCount: " << sponsoringCount; - } - else if (deltaCount < std::numeric_limits::min()) - { - deltaCount = std::numeric_limits::min(); - JLOG(j.fatal()) << "Account " << id << " delta count exceeds min, " - << "adjustment: " << adjustment << ", sponsoredCount: " << sponsoredCount - << ", sponsoringCount: " << sponsoringCount; - } - - return confineOwnerCount(hookedCount, static_cast(deltaCount), reportId, j); -} - -static std::uint32_t -reserveCountHlp(SLE::const_ref sle, std::int32_t adjustment) -{ - bool const isSponsored = sle->isFieldPresent(sfSponsor); - std::uint32_t const sponsoringCount = sle->getFieldU32(sfSponsoringAccountCount); - std::uint32_t const reserveCount = (isSponsored ? 0 : 1) + sponsoringCount; - return confineOwnerCount(reserveCount, adjustment); -} - -static inline XRPAmount -baseReserveHlp(ReadView const& view, std::uint32_t ownerCount, std::uint32_t reserveCount) -{ - auto const& fees = view.fees(); - return (fees.reserve * reserveCount) + (fees.increment * ownerCount); -} - -static XRPAmount -reserveHlp( - ReadView const& view, - SLE::const_ref sle, - std::uint32_t ownerCount, - std::uint32_t reserveCount) -{ - // Pseudo-accounts have no reserve requirement - if (isPseudoAccount(sle)) - return XRPAmount(0); - - auto const reserve = baseReserveHlp(view, ownerCount, reserveCount); - return reserve; -} - -std::uint32_t -ownerCount(ReadView const& view, SLE::const_ref sle, beast::Journal j, std::int32_t adjustment) -{ - return ownerCountHlp(view, sle, adjustment, sle->getAccountID(sfAccount), j); -} - XRPAmount xrpLiquid(ReadView const& view, AccountID const& id, std::int32_t ownerCountAdj, beast::Journal j) { @@ -171,9 +92,10 @@ xrpLiquid(ReadView const& view, AccountID const& id, std::int32_t ownerCountAdj, if (sle == nullptr) return beast::kZero; - std::uint32_t const ownerCount = ownerCountHlp(view, sle, ownerCountAdj, std::nullopt, j); - std::uint32_t const reserveCount = reserveCountHlp(sle, 0); - auto const reserve = reserveHlp(view, sle, ownerCount, reserveCount); + std::uint32_t const ownerCount = view.ownerCountHook(id, sle->at(sfOwnerCount)) - + sle->at(sfSponsoredOwnerCount) + sle->at(sfSponsoringOwnerCount) + ownerCountAdj; + auto const& fees = view.fees(); + auto const reserve = (fees.reserve * accountOwnerCount(sle)) + (fees.increment * ownerCount); auto const fullBalance = sle->getFieldAmount(sfBalance); @@ -221,8 +143,8 @@ adjustOwnerCountHlp( void adjustOwnerCount( ApplyView& view, - SLE::ref accountSle, - SLE::ref sponsorSle, + SLE::pointer accountSle, + SLE::pointer sponsorSle, std::int32_t adjustment, beast::Journal j) { @@ -265,8 +187,8 @@ adjustOwnerCount( void adjustOwnerCountObj( ApplyView& view, - SLE::ref accountSle, - SLE::ref objectSle, + SLE::pointer accountSle, + SLE::pointer objectSle, std::int32_t amount, beast::Journal j) { @@ -275,37 +197,10 @@ adjustOwnerCountObj( if (objectSle->getType() == ltACCOUNT_ROOT) Throw("xrpl::adjustOwnerCount : valid object sle type"); - SLE::ref sponsorSle = getLedgerEntryReserveSponsor(view, objectSle); + SLE::pointer sponsorSle = getLedgerEntryReserveSponsor(view, objectSle); adjustOwnerCount(view, accountSle, sponsorSle, amount, j); } -XRPAmount -accountReserve( - ReadView const& view, - SLE::const_ref sle, - beast::Journal j, - std::int32_t ownerCountAdj, - std::int32_t reserveCountAdj) -{ - if (!sle) - Throw("xrpl::accountReserve : valid sle"); - if (sle->getType() != ltACCOUNT_ROOT) - Throw("xrpl::accountReserve : valid sle type"); - - std::uint32_t const ownerCount = - ownerCountHlp(view, sle, ownerCountAdj, sle->getAccountID(sfAccount), j); - std::uint32_t const reserveCount = reserveCountHlp(sle, reserveCountAdj); - - return reserveHlp(view, sle, ownerCount, reserveCount); -} - -XRPAmount -baseAccountReserve(ReadView const& view, std::int32_t ownerCount) -{ - auto const reserve = baseReserveHlp(view, ownerCount, 1); - return reserve; -} - TER checkInsufficientReserve( ReadView const& view, @@ -338,7 +233,7 @@ checkInsufficientReserve( auto const sponsorBalance = sponsorSle->getFieldAmount(sfBalance); STAmount const sponsorReserve = - accountReserve(view, sponsorSle, j, ownerCountDelta, reserveCountDelta); + totalAccountReserve(view, sponsorSle, ownerCountDelta, reserveCountDelta); if (sponsorBalance < sponsorReserve) return tecINSUFFICIENT_RESERVE; @@ -346,7 +241,7 @@ checkInsufficientReserve( else { STAmount const reserve = - accountReserve(view, accSle, j, ownerCountDelta, reserveCountDelta); + totalAccountReserve(view, accSle, ownerCountDelta, reserveCountDelta); if (accBalance < reserve) return tecINSUFFICIENT_RESERVE; } @@ -405,7 +300,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/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index 23269f710a..15bd4054d6 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -206,8 +206,7 @@ authorizeMPToken( // items. This is similar to the reserve requirements of trust lines. // If PreFunded Sponsor, it must be checked whether sufficient // ReserveCount exists. - if (ownerCount(view, *sponsorSle ? *sponsorSle : sleAcct, journal) >= 2 || - isSponsoredAndPreFunded) + if (objectOwnerCount(*sponsorSle ? *sponsorSle : sleAcct) >= 2 || isSponsoredAndPreFunded) { if (auto const ret = checkInsufficientReserve( view, tx, sleAcct, priorBalance, *sponsorSle, 1, 0, journal); diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index cd92bc4eae..e6b7ee581f 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -553,8 +553,8 @@ Transactor::checkFee(PreclaimContext const& ctx, XRPAmount baseFee) if (feePayer.type == FeePayerType::SponsorCoSigned) { - STAmount const sponsorReserve = accountReserve(ctx.view, payerSle, ctx.j); - maxSpendable = payerSle->getFieldAmount(sfBalance).xrp() - sponsorReserve.xrp(); + auto const sponsorReserve = totalAccountReserve(ctx.view, payerSle); + maxSpendable = payerSle->getFieldAmount(sfBalance).xrp() - sponsorReserve; } else { diff --git a/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp b/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp index 8229f9fd7b..06af667583 100644 --- a/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp +++ b/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp @@ -437,7 +437,7 @@ transferHelper( return tecINTERNAL; // LCOV_EXCL_LINE { - auto const reserve = accountReserve(psb, sleSrc, j, 0, 0); + auto const reserve = totalAccountReserve(psb, sleSrc); auto const availableBalance = [&]() -> STAmount { STAmount curBal = (*sleSrc)[sfBalance]; diff --git a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp index d0ac7fdbec..166305d3b8 100644 --- a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp @@ -657,8 +657,7 @@ AMMWithdraw::withdraw( return tecINTERNAL; // LCOV_EXCL_LINE auto const balance = (*sleAccount)[sfBalance]->xrp(); - std::uint32_t const count = - ownerCount(view, sponsorSle ? sponsorSle : sleAccount, journal); + std::uint32_t const count = objectOwnerCount(sponsorSle ? sponsorSle : sleAccount); // See also TrustSet::doApply() and MPTokenAuthorize::authorize() if (count >= 2) { diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index 6eab8b45c6..f7069b4373 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -636,7 +636,7 @@ Payment::doApply() // the number of reserves in this ledger for this account that require a // reserve. - auto const reserve = accountReserve(view(), sleSrc, j_); + auto const reserve = totalAccountReserve(view(), sleSrc); // In a delegated payment, the fee payer is the delegated account, // not the source account (accountID_). diff --git a/src/libxrpl/tx/transactors/token/TrustSet.cpp b/src/libxrpl/tx/transactors/token/TrustSet.cpp index 5dfd4effe9..2516dea5c4 100644 --- a/src/libxrpl/tx/transactors/token/TrustSet.cpp +++ b/src/libxrpl/tx/transactors/token/TrustSet.cpp @@ -332,7 +332,7 @@ TrustSet::doApply() if (!sponsorSle) return sponsorSle.error(); // LCOV_EXCL_LINE - std::uint32_t const uOwnerCount = ownerCount(view(), *sponsorSle ? *sponsorSle : sle, j_); + std::uint32_t const uOwnerCount = objectOwnerCount(*sponsorSle ? *sponsorSle : sle); bool const isSponsoredAndPreFunded = *sponsorSle && !isSponsorReserveCoSigning(ctx_.tx); // If PreFunded Sponsor, it must be checked whether sufficient diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index fe966d0258..046c4493cf 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -1094,8 +1094,7 @@ class LoanBroker_test : public beast::unit_test::Suite env.close(); } - auto const amt = - env.balance(alice) - accountReserve(*env.current(), alice.id(), env.journal); + auto const amt = env.balance(alice) - accountReserve(*env.current(), alice.id()); env(pay(alice, issuer, amt)); // preclaim:: tecINSUFFICIENT_RESERVE diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 47ef458518..bbabe5a433 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -861,7 +861,7 @@ protected: auto const totalNeeded = state.totalValue + (serviceFee * state.paymentRemaining) + (broker.asset.native() ? Number( baseFee * state.paymentRemaining + - accountReserve(*env.current(), borrower.id(), env.journal)) + accountReserve(*env.current(), borrower.id())) : broker.asset(15).number()); auto const shortage = totalNeeded - borrowerBalance.number(); @@ -4545,8 +4545,7 @@ protected: BrokerInfo const& brokerInfo, jtx::Fee const& loanSetFee, Number const& debtMaximumRequest) { - auto const amt = - env.balance(borrower) - accountReserve(*env.current(), borrower.id(), env.journal); + auto const amt = env.balance(borrower) - accountReserve(*env.current(), borrower.id()); env(pay(borrower, issuer, amt)); // tecINSUFFICIENT_RESERVE diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index a23bf80a52..bba0dacb02 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -6,7 +6,10 @@ #include #include +#include +#include #include +#include #include #include #include @@ -516,6 +519,21 @@ txFee(Env const& env, std::uint16_t n); PrettyAmount xrpMinusFee(Env const& env, std::int64_t xrpAmount); +/** Returns the base reserve for a non-sponsored account with `count` owned objects. */ +inline XRPAmount +baseAccountReserve(ReadView const& view, std::uint32_t count = 0) +{ + auto const& fees = view.fees(); + return fees.reserve + fees.increment * count; +} + +/** Returns the total reserve for an account, read from the ledger. */ +inline XRPAmount +accountReserve(ReadView const& view, AccountID const& id) +{ + return totalAccountReserve(view, view.read(keylet::account(id))); +} + bool expectHolding( Env& env,