ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-27 09:00:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

more fixes from review part 3 (#7771)

Browse Source
This commit is contained in:
Mayukha Vadari
2026-07-09 13:31:59 -04:00
committed by GitHub
parent e0c9cf8b97
commit 28b7fb7cc7
15 changed files with 140 additions and 140 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
include/xrpl/ledger/ReadView.h
View File

@@ -36,16 +36,16 @@ struct OwnerCounts
std::uint32_t sponsoring = 0;
OwnerCounts() = default;
OwnerCounts(SLE const& sle)
: owner(sle[sfOwnerCount])
, sponsored(sle[sfSponsoredOwnerCount])
, sponsoring(sle[sfSponsoringOwnerCount])
OwnerCounts(SLE::const_ref sle)
: owner(sle->at(sfOwnerCount))
, sponsored(sle->at(~sfSponsoredOwnerCount).value_or(0))
, sponsoring(sle->at(~sfSponsoringOwnerCount).value_or(0))
{
XRPL_ASSERT(
owner >= sponsored,
"xrpl::OwnerCounts : OwnerCount must be greater than or equal to "
"SponsoredOwnerCount");
XRPL_ASSERT(sle.getType() == ltACCOUNT_ROOT, "xrpl::OwnerCounts : sle is AccountRoot");
XRPL_ASSERT(sle->getType() == ltACCOUNT_ROOT, "xrpl::OwnerCounts : sle is AccountRoot");
}
[[nodiscard]] std::uint32_t

18
include/xrpl/ledger/helpers/OracleHelpers.h
View File

@@ -1,5 +1,9 @@
#pragma once
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STArray.h> // IWYU pragma: keep
#include <xrpl/protocol/STLedgerEntry.h>
#include <cstddef>
#include <cstdint>
@@ -9,11 +13,19 @@ constexpr std::uint32_t kMinOracleReserveCount = 1;
constexpr std::uint32_t kMaxOracleReserveCount = 2;
constexpr std::size_t kOracleReserveCountThreshold = 5;
template <typename T>
requires requires(T const& t) { t.size(); }
inline std::uint32_t
calculateOracleReserve(std::size_t priceDataSeriesCount)
calculateOracleReserve(T const& priceDataSeries)
{
return priceDataSeriesCount > kOracleReserveCountThreshold ? kMaxOracleReserveCount
: kMinOracleReserveCount;
return priceDataSeries.size() > kOracleReserveCountThreshold ? kMaxOracleReserveCount
: kMinOracleReserveCount;
}
inline std::uint32_t
calculateOracleReserve(SLE::const_ref oracleSle)
{
return calculateOracleReserve(oracleSle->getFieldArray(sfPriceDataSeries));
}
} // namespace xrpl

6
src/libxrpl/ledger/helpers/AccountRootHelpers.cpp
View File

@@ -154,7 +154,7 @@ adjustOwnerCountSigned(
XRPL_ASSERT(adjustment, "xrpl::adjustOwnerCountSigned : nonzero adjustment input");
OwnerCounts const currentOwnerCount(
sleType == ltACCOUNT_ROOT ? OwnerCounts(*accountSle) : OwnerCounts());
sleType == ltACCOUNT_ROOT ? OwnerCounts(accountSle) : OwnerCounts());
OwnerCounts totalOwnerCount(currentOwnerCount);
if (sponsorSle)
@@ -169,7 +169,7 @@ adjustOwnerCountSigned(
adjustOwnerCountImpl(view, accountSle, sfSponsoredOwnerCount, accountID, adjustment, j);
{
OwnerCounts const sponsorCurrent(*sponsorSle);
OwnerCounts const sponsorCurrent(sponsorSle);
OwnerCounts sponsorAdjustment(sponsorCurrent);
sponsorAdjustment.sponsoring = adjustOwnerCountImpl(
view, sponsorSle, sfSponsoringOwnerCount, sponsorID, adjustment, j);
@@ -242,7 +242,7 @@ xrpLiquid(ReadView const& view, AccountID const& id, std::int32_t ownerCountAdj,
// Return balance minus reserve
std::uint32_t const currentOwnerCount =
confineOwnerCount(view.ownerCountHook(id, OwnerCounts(*sle)).count(), ownerCountAdj);
confineOwnerCount(view.ownerCountHook(id, OwnerCounts(sle)).count(), ownerCountAdj);
std::uint32_t const currentAccountCount = accountCountImpl(sle, 0, j);
// Pseudo-accounts have no reserve requirement

4
src/libxrpl/ledger/helpers/RippleStateHelpers.cpp
View File

@@ -675,9 +675,7 @@ addEmptyHolding(
!isTesSuccess(ret))
{
// checkReserve can return tecINSUFFICIENT_RESERVE or tecINTERNAL
if (ret == tecINSUFFICIENT_RESERVE)
return tecNO_LINE_INSUF_RESERVE;
return ret;
return (ret == tecINSUFFICIENT_RESERVE) ? tecNO_LINE_INSUF_RESERVE : ret;
}
return trustCreate(

6
src/libxrpl/ledger/helpers/SponsorHelpers.cpp
View File

@@ -66,6 +66,7 @@ std::expected<SLE::pointer, TER>
getEffectiveTxReserveSponsor(ApplyViewContext ctx, SLE::const_ref accountSle)
{
// A reserve sponsor only covers tx.Account's own objects.
XRPL_ASSERT(!!accountSle, "getEffectiveTxReserveSponsor : accountSle exists");
if (isPseudoAccount(accountSle) || accountSle->getAccountID(sfAccount) != ctx.tx[sfAccount])
return SLE::pointer();
return getTxReserveSponsor(ctx);
@@ -205,7 +206,7 @@ getLedgerEntryOwnerCount(SLE const& sle)
switch (sle.getType())
{
case ltORACLE: {
return calculateOracleReserve(sle.getFieldArray(sfPriceDataSeries).size());
return calculateOracleReserve(sle.getFieldArray(sfPriceDataSeries));
}
// Vaults require 2 owner counts (the vault and a pseudo-account)
case ltVAULT:
@@ -219,6 +220,9 @@ getLedgerEntryOwnerCount(SLE const& sle)
return 1;
return 2 + static_cast<std::uint32_t>(sle.getFieldArray(sfSignerEntries).size());
}
case ltACCOUNT_ROOT:
UNREACHABLE("AccountRoots are not supported by object sponsorship.");
return 0;
default:
return 1;
}

30
src/libxrpl/tx/Transactor.cpp
View File

@@ -640,16 +640,27 @@ Transactor::payFee()
return tefINTERNAL; // LCOV_EXCL_LINE
}
// A co-signed sponsor pays the fee out of its own account balance, but must
// never be charged into its account reserve. Mirror the spendable amount
// computed in checkFee() so the reserve is protected on the apply path too.
XRPAmount spendable = balance;
if (feePayer.type == FeePayerType::SponsorCoSigned)
{
auto const sponsorReserve = accountReserve(view(), sle, j_);
// max(balance - reserve, 0) with overflow handling
spendable = balance > sponsorReserve ? balance - sponsorReserve : beast::kZero;
}
// Only sponsor fee-payers reject here on insufficient funds. For an
// ordinary account, the fee falls through and is capped by reset(), which
// caps to the account's balance. That capping is wrong for sponsors: a
// co-signed sponsor would be charged into its own reserve, and a prefunded
// sponsorship's fee amount should be rejected rather than partially spent.
if (feePaid > balance &&
if (feePaid > spendable &&
(feePayer.type == FeePayerType::SponsorPreFunded ||
feePayer.type == FeePayerType::SponsorCoSigned))
{
if ((balance > beast::kZero) && !view().open())
if ((spendable > beast::kZero) && !view().open())
return tecINSUFF_FEE;
return terINSUF_FEE_B;
@@ -1301,6 +1312,17 @@ Transactor::reset(XRPAmount fee)
fee = std::min(fee, cap);
}
// A co-signed sponsor must never be charged into its own account reserve,
// so the fee is capped to the balance above the reserve rather than to the
// full balance.
XRPAmount spendable = balance;
if (feePayer.type == FeePayerType::SponsorCoSigned)
{
auto const sponsorReserve = accountReserve(view(), payerSle, j_);
// max(balance - reserve, 0) with overflow handling
spendable = balance > sponsorReserve ? balance - sponsorReserve : beast::kZero;
}
// balance should have already been checked in checkFee / preFlight.
XRPL_ASSERT(
(fee == beast::kZero || balance != beast::kZero) && (!view().open() || balance >= fee),
@@ -1309,8 +1331,8 @@ Transactor::reset(XRPAmount fee)
// We retry/reject the transaction if the account balance is zero or
// we're applying against an open ledger and the balance is less than
// the fee
if (fee > balance)
fee = balance;
if (fee > spendable)
fee = spendable;
// Since we reset the context, we need to charge the fee and update
// the account's sequence number (or consume the Ticket) again.

36
src/libxrpl/tx/invariants/SponsorshipInvariant.cpp
View File

@@ -4,10 +4,9 @@
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/core/ServiceRegistry.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/ledger/helpers/OracleHelpers.h>
#include <xrpl/ledger/helpers/SponsorHelpers.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STArray.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/XRPAmount.h>
@@ -42,10 +41,11 @@ SponsorshipOwnerCountsMatch::visitEntry(bool isDelete, SLE::const_ref before, SL
return 0;
switch (sle->getType())
{
case ltACCOUNT_ROOT: {
case ltACCOUNT_ROOT:
return 0;
}
case ltRIPPLE_STATE: {
// A trust line can be reserve-sponsored independently on each
// side, so it may contribute up to two sponsored owner counts.
uint32_t ownerCount = 0;
if (sle->isFieldPresent(sfHighSponsor))
ownerCount++;
@@ -53,31 +53,13 @@ SponsorshipOwnerCountsMatch::visitEntry(bool isDelete, SLE::const_ref before, SL
ownerCount++;
return ownerCount;
}
case ltORACLE: {
default:
// Every other supported type carries a single sfSponsor field
// and contributes its full owner-count magnitude only when it is
// sponsored.
if (!sle->isFieldPresent(sfSponsor))
return 0;
auto const priceDataSeries = sle->getFieldArray(sfPriceDataSeries);
return calculateOracleReserve(priceDataSeries.size());
}
case ltVAULT: {
if (!sle->isFieldPresent(sfSponsor))
return 0;
return 2;
}
case ltSIGNER_LIST: {
if (!sle->isFieldPresent(sfSponsor))
return 0;
// Modern lists carry lsfOneOwnerCount and cost 1.
// Legacy (pre-MultiSignReserve) lists cost 2 + signer_count.
if (sle->isFlag(lsfOneOwnerCount))
return 1;
return 2 + static_cast<std::uint32_t>(sle->getFieldArray(sfSignerEntries).size());
}
default: {
if (sle->isFieldPresent(sfSponsor))
return 1;
return 0;
}
return getLedgerEntryOwnerCount(*sle);
}
};

3
src/libxrpl/tx/transactors/oracle/OracleDelete.cpp
View File

@@ -71,8 +71,7 @@ OracleDelete::deleteOracle(
if (!sleOwner)
return tecINTERNAL; // LCOV_EXCL_LINE
std::uint32_t const count =
calculateOracleReserve(sle->getFieldArray(sfPriceDataSeries).size());
std::uint32_t const count = calculateOracleReserve(sle);
decreaseOwnerCountForObject(view, sleOwner, sle, count, j);
view.erase(sle);

12
src/libxrpl/tx/transactors/oracle/OracleSet.cpp
View File

@@ -151,8 +151,8 @@ OracleSet::preclaim(PreclaimContext const& ctx)
if (!pairsDel.empty())
return tecTOKEN_PAIR_NOT_FOUND;
auto const oldCount = calculateOracleReserve(sle->getFieldArray(sfPriceDataSeries).size());
auto const newCount = calculateOracleReserve(pairs.size());
auto const oldCount = calculateOracleReserve(sle);
auto const newCount = calculateOracleReserve(pairs);
adjustReserve = newCount - oldCount;
}
@@ -162,7 +162,7 @@ OracleSet::preclaim(PreclaimContext const& ctx)
if (!ctx.tx.isFieldPresent(sfProvider) || !ctx.tx.isFieldPresent(sfAssetClass))
return temMALFORMED;
adjustReserve = calculateOracleReserve(pairs.size());
adjustReserve = calculateOracleReserve(pairs);
}
if (pairs.empty())
@@ -239,7 +239,7 @@ OracleSet::doApply()
priceData.setFieldCurrency(sfQuoteAsset, entry.getFieldCurrency(sfQuoteAsset));
pairs.emplace(tokenPairKey(entry), std::move(priceData));
}
auto const oldCount = calculateOracleReserve(pairs.size());
auto const oldCount = calculateOracleReserve(pairs);
// update/add/delete pairs
for (auto const& entry : ctx_.tx.getFieldArray(sfPriceDataSeries))
{
@@ -277,7 +277,7 @@ OracleSet::doApply()
(*sle)[sfOracleDocumentID] = ctx_.tx[sfOracleDocumentID];
}
auto const newCount = calculateOracleReserve(pairs.size());
auto const newCount = calculateOracleReserve(pairs);
int32_t const adjust = newCount - oldCount;
if (adjust != 0 && !adjustOracleOwnerCount(ctx_, adjust))
@@ -329,7 +329,7 @@ OracleSet::doApply()
(*sle)[sfOwnerNode] = *page;
auto const count = calculateOracleReserve(series.size());
auto const count = calculateOracleReserve(series);
if (!adjustOracleOwnerCount(ctx_, count))
return tefINTERNAL; // LCOV_EXCL_LINE

8
src/libxrpl/tx/transactors/token/TrustSet.cpp
View File

@@ -618,7 +618,9 @@ TrustSet::doApply()
// Another transaction could provide XRP to the account and then
// this transaction would succeed.
terResult = tecINSUF_RESERVE_LINE;
// checkReserve can return tecINSUFFICIENT_RESERVE or tecINTERNAL;
// don't mask an internal error as a reserve shortfall.
terResult = ((ret == tecINSUFFICIENT_RESERVE) ? tecINSUF_RESERVE_LINE : ret);
}
else
{
@@ -654,7 +656,9 @@ TrustSet::doApply()
// Another transaction could create the account and then this
// transaction would succeed.
terResult = tecNO_LINE_INSUF_RESERVE;
// checkReserve can return tecINSUFFICIENT_RESERVE or tecINTERNAL;
// don't mask an internal error as a reserve shortfall.
terResult = ((ret == tecINSUFFICIENT_RESERVE) ? tecNO_LINE_INSUF_RESERVE : ret);
}
else
{

20
src/test/app/Sponsor_test.cpp
View File

@@ -1681,6 +1681,26 @@ public:
BEAST_EXPECT(env.balance(sponsor) == sponsorBalance - feeAmt);
}
{
// A co-signed sponsor pays the fee from its own balance, but
// must never be charged into its own account reserve. With a
// balance of exactly reserve + fee the fee is still payable,
// charging the sponsor down to precisely its reserve.
auto const feeAmt = XRP(10);
adjustAccountXRPBalance(env, sponsor, reserve(env, 0) + feeAmt);
auto const sponsorBalance = env.balance(sponsor);
env(noop(alice),
Fee(feeAmt),
sponsor::As(sponsor, spfSponsorFee),
Sig(sfSponsorSignature, sponsor),
Ter(tesSUCCESS));
env.close();
BEAST_EXPECT(env.balance(sponsor) == sponsorBalance - feeAmt);
BEAST_EXPECT(env.balance(sponsor) == reserve(env, 0));
}
{
// below reserve
adjustAccountXRPBalance(env, sponsor, env.current()->fees().reserve);

45
src/test/jtx/impl/sponsor.cpp
View File

@@ -21,9 +21,9 @@ namespace xrpl::test::jtx::sponsor {
json::Value
set(jtx::Account const& account,
uint32_t flags,
std::optional<uint32_t> reserveCount,
std::optional<STAmount> feeAmount,
std::optional<STAmount> maxFee)
std::optional<uint32_t> const reserveCount,
std::optional<STAmount> const feeAmount,
std::optional<STAmount> const maxFee)
{
json::Value jv;
jv[jss::TransactionType] = jss::SponsorshipSet;
@@ -38,45 +38,6 @@ set(jtx::Account const& account,
return jv;
}
json::Value
set_fee(
jtx::Account const& account,
uint32_t flags,
STAmount feeAmount,
std::optional<STAmount> maxFee)
{
json::Value jv;
jv[jss::TransactionType] = jss::SponsorshipSet;
jv[jss::Account] = account.human();
jv[sfFlags.jsonName] = flags;
jv[sfFeeAmount.jsonName] = feeAmount.getJson(JsonOptions::Values::None);
if (maxFee)
jv[sfMaxFee.jsonName] = maxFee->getJson(JsonOptions::Values::None);
return jv;
}
json::Value
set_reserve(jtx::Account const& account, uint32_t flags, uint32_t reserveCount)
{
json::Value jv;
jv[jss::TransactionType] = jss::SponsorshipSet;
jv[jss::Account] = account.human();
jv[sfFlags.jsonName] = flags;
jv[sfRemainingOwnerCount.jsonName] = reserveCount;
return jv;
}
json::Value
set_max_fee(jtx::Account const& account, uint32_t flags, STAmount maxFee)
{
json::Value jv;
jv[jss::TransactionType] = jss::SponsorshipSet;
jv[jss::Account] = account.human();
jv[sfFlags.jsonName] = flags;
jv[sfMaxFee.jsonName] = maxFee.getJson(JsonOptions::Values::None);
return jv;
}
json::Value
del(jtx::Account const& account)
{

27
src/test/jtx/sponsor.h
View File

@@ -17,22 +17,31 @@ namespace xrpl::test::jtx::sponsor {
json::Value
set(jtx::Account const& account,
std::uint32_t flags,
std::optional<std::uint32_t> reserveCount = std::nullopt,
std::optional<STAmount> feeAmount = std::nullopt,
std::optional<STAmount> maxFee = std::nullopt);
std::optional<std::uint32_t> const reserveCount = std::nullopt,
std::optional<STAmount> const feeAmount = std::nullopt,
std::optional<STAmount> const maxFee = std::nullopt);
json::Value
inline json::Value
set_fee(
jtx::Account const& account,
std::uint32_t flags,
STAmount feeAmount,
std::optional<STAmount> maxFee = std::nullopt);
std::optional<STAmount> maxFee = std::nullopt)
{
return set(account, flags, std::nullopt, std::move(feeAmount), std::move(maxFee));
}
json::Value
set_reserve(jtx::Account const& account, std::uint32_t flags, std::uint32_t reserveCount);
inline json::Value
set_reserve(jtx::Account const& account, std::uint32_t flags, std::uint32_t reserveCount)
{
return set(account, flags, reserveCount);
}
json::Value
set_max_fee(jtx::Account const& account, std::uint32_t flags, STAmount maxFee);
inline json::Value
set_max_fee(jtx::Account const& account, std::uint32_t flags, STAmount maxFee)
{
return set(account, flags, std::nullopt, std::nullopt, std::move(maxFee));
}
json::Value
del(jtx::Account const& account);

8
src/test/ledger/PaymentSandbox_test.cpp
View File

@@ -419,7 +419,7 @@ class PaymentSandbox_test : public beast::unit_test::Suite
auto const aliceSle = sb.peek(keylet::account(alice));
BEAST_EXPECT(aliceSle);
OwnerCounts const initial(*aliceSle);
OwnerCounts const initial(aliceSle);
OwnerCounts updated = initial;
updated.owner = initial.owner + 2;
@@ -438,7 +438,7 @@ class PaymentSandbox_test : public beast::unit_test::Suite
auto const sponsorSle = sb.peek(keylet::account(sponsor));
BEAST_EXPECT(sponsorSle);
OwnerCounts const sponsorInitial(*sponsorSle);
OwnerCounts const sponsorInitial(sponsorSle);
OwnerCounts sponsorUpdated = sponsorInitial;
sponsorUpdated.owner = sponsorInitial.owner + 1;
sponsorUpdated.sponsoring = sponsorInitial.sponsoring + 1;
@@ -455,7 +455,7 @@ class PaymentSandbox_test : public beast::unit_test::Suite
PaymentSandbox sb2(&sb);
auto const aliceSle = sb2.peek(keylet::account(alice));
OwnerCounts const current(*aliceSle);
OwnerCounts const current(aliceSle);
OwnerCounts further = current;
further.owner = current.owner + 3;
@@ -469,7 +469,7 @@ class PaymentSandbox_test : public beast::unit_test::Suite
// Test that max logic works correctly
{
auto const aliceSle = sb.peek(keylet::account(alice));
OwnerCounts const current(*aliceSle);
OwnerCounts const current(aliceSle);
OwnerCounts lower = current;
lower.owner = (current.owner > 0) ? current.owner - 1 : 0;

47
src/xrpld/rpc/handlers/account/AccountObjects.cpp
View File

@@ -7,6 +7,7 @@
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/json/json_value.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/ledger/helpers/SponsorHelpers.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/ErrorCodes.h>
#include <xrpl/protocol/Indexes.h>
@@ -101,12 +102,10 @@ getAccountObjects(
while (currentPage)
{
std::optional<AccountID> const nftSponsor = currentPage->isFieldPresent(sfSponsor)
? currentPage->getAccountID(sfSponsor)
: std::optional<AccountID>(std::nullopt);
std::optional<AccountID> const nftSponsor = currentPage->at(~sfSponsor);
bool const canAppendNFT = sponsoredMatchesFilter(nftSponsor);
if (canAppendNFT)
jvObjects.append(currentPage->getJson(JsonOptions::Values::None));
jvObjects.append(currentPage->getJson());
auto const npm = (*currentPage)[~sfNextPageMin];
if (npm)
{
@@ -198,31 +197,21 @@ getAccountObjects(
!typeMatchesFilter(typeFilter.value(), sleNode->getType()))
canAppend = false;
auto const getSponsor = [&account, &sleNode]() -> std::optional<AccountID> {
if (sleNode->getType() == ltRIPPLE_STATE)
{
if (sleNode->isFlag(lsfHighReserve) &&
sleNode->getFieldAmount(sfHighLimit).getIssuer() == account &&
sleNode->isFieldPresent(sfHighSponsor))
return sleNode->getAccountID(sfHighSponsor);
if (sleNode->isFlag(lsfLowReserve) &&
sleNode->getFieldAmount(sfLowLimit).getIssuer() == account &&
sleNode->isFieldPresent(sfLowSponsor))
return sleNode->getAccountID(sfLowSponsor);
return std::nullopt;
}
if (sleNode->getType() == ltSPONSORSHIP &&
sleNode->getAccountID(sfOwner) != account)
return std::nullopt;
if (sleNode->isFieldPresent(sfSponsor))
return sleNode->getAccountID(sfSponsor);
return std::nullopt;
};
std::optional<AccountID> const sponsor = getSponsor();
std::optional<AccountID> sponsor;
if (sleNode->getType() == ltSPONSORSHIP)
{
// ltSPONSORSHIP is in both parties' directories; only the
// owner's side carries a sponsor.
if (sleNode->getAccountID(sfOwner) == account)
sponsor = getLedgerEntryReserveSponsorID(sleNode);
}
else if (
isLedgerEntrySupportedBySponsorship(*sleNode) &&
getLedgerEntryOwner(ledger, *sleNode, account).has_value())
{
sponsor = getLedgerEntryReserveSponsorID(
sleNode, getLedgerEntrySponsorField(*sleNode, account));
}
if (!sponsoredMatchesFilter(sponsor))
canAppend = false;