more fixes from review (part 4) (#7775)

This commit is contained in:
Mayukha Vadari
2026-07-09 15:36:14 -04:00
committed by GitHub
parent 152130082d
commit 6a6b97b529
24 changed files with 127 additions and 120 deletions

View File

@@ -3,6 +3,7 @@
#include <xrpl/basics/base_uint.h>
#include <xrpl/basics/safe_cast.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/ledger/OwnerCounts.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Issue.h> // IWYU pragma: keep

View File

@@ -0,0 +1,79 @@
#pragma once
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STInteger.h> // IWYU pragma: keep
#include <xrpl/protocol/STLedgerEntry.h>
#include <cstdint>
#include <limits>
namespace xrpl {
struct OwnerCounts
{
std::uint32_t owner = 0;
std::uint32_t sponsored = 0;
std::uint32_t sponsoring = 0;
OwnerCounts() = default;
OwnerCounts(SLE::const_ref sle)
: owner(sle->getFieldU32(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");
}
[[nodiscard]] std::uint32_t
count() const
{
std::int64_t const x = static_cast<std::int64_t>(owner) - sponsored + sponsoring;
if (x < 0)
{
// LCOV_EXCL_START
UNREACHABLE("xrpl::OwnerCounts::count : count less than zero");
return 0;
// LCOV_EXCL_STOP
}
if (x > std::numeric_limits<std::uint32_t>::max())
return std::numeric_limits<std::uint32_t>::max();
return static_cast<std::uint32_t>(x);
}
auto
operator<=>(OwnerCounts const& o) const
{
if (auto cmp = count() <=> o.count(); cmp != 0)
return cmp;
if (auto cmp = owner <=> o.owner; cmp != 0)
return cmp;
if (auto cmp = sponsored <=> o.sponsored; cmp != 0)
return cmp;
return sponsoring <=> o.sponsoring;
}
bool
operator==(OwnerCounts const& o) const
{
return this == &o ||
(owner == o.owner && sponsored == o.sponsored && sponsoring == o.sponsoring);
}
[[nodiscard]] bool
valid() const
{
int64_t const x = static_cast<int64_t>(owner) - sponsored + sponsoring;
if (x < 0 || x > std::numeric_limits<std::uint32_t>::max())
return false;
return owner >= sponsored;
}
};
} // namespace xrpl

View File

@@ -1,6 +1,7 @@
#pragma once
#include <xrpl/ledger/ApplyView.h>
#include <xrpl/ledger/OwnerCounts.h>
#include <xrpl/ledger/RawView.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/ledger/detail/ApplyViewBase.h>

View File

@@ -4,12 +4,12 @@
#include <xrpl/basics/chrono.h>
#include <xrpl/beast/hash/uhash.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/ledger/OwnerCounts.h>
#include <xrpl/ledger/detail/ReadViewFwdRange.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Fees.h>
#include <xrpl/protocol/Issue.h> // IWYU pragma: keep
#include <xrpl/protocol/Keylet.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/LedgerHeader.h>
#include <xrpl/protocol/MPTIssue.h>
#include <xrpl/protocol/Protocol.h>
@@ -21,7 +21,6 @@
#include <xrpl/protocol/STTx.h>
#include <cstdint>
#include <limits>
#include <memory>
#include <optional>
#include <unordered_set>
@@ -29,71 +28,6 @@
namespace xrpl {
struct OwnerCounts
{
std::uint32_t owner = 0;
std::uint32_t sponsored = 0;
std::uint32_t sponsoring = 0;
OwnerCounts() = default;
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");
}
[[nodiscard]] std::uint32_t
count() const
{
std::int64_t const x = static_cast<std::int64_t>(owner) - sponsored + sponsoring;
if (x < 0)
{
// LCOV_EXCL_START
UNREACHABLE("xrpl::OwnerCounts::count : count less than zero");
return 0;
// LCOV_EXCL_STOP
}
if (x > std::numeric_limits<std::uint32_t>::max())
return std::numeric_limits<std::uint32_t>::max();
return static_cast<std::uint32_t>(x);
}
auto
operator<=>(OwnerCounts const& o) const
{
if (auto cmp = count() <=> o.count(); cmp != 0)
return cmp;
if (auto cmp = owner <=> o.owner; cmp != 0)
return cmp;
if (auto cmp = sponsored <=> o.sponsored; cmp != 0)
return cmp;
return sponsoring <=> o.sponsoring;
}
bool
operator==(OwnerCounts const& o) const
{
return this == &o ||
(owner == o.owner && sponsored == o.sponsored && sponsoring == o.sponsoring);
}
[[nodiscard]] bool
valid() const
{
int64_t const x = static_cast<int64_t>(owner) - sponsored + sponsoring;
if (x < 0 || x > std::numeric_limits<std::uint32_t>::max())
return false;
return owner >= sponsored;
}
};
//------------------------------------------------------------------------------
/** A view into a ledger.

View File

@@ -18,6 +18,9 @@ namespace xrpl {
* - The sum of all per-account deltas of `sfSponsoredOwnerCount` equals
* the sum of all per-account deltas of `sfSponsoringOwnerCount`.
* - Account OwnerCount must be greater than or equal to SponsoredOwnerCount.
* - The net delta of sponsored object owner counts (the owner-count
* magnitude of sponsored ledger entries) equals the net delta of
* `sfSponsoredOwnerCount`.
*/
class SponsorshipOwnerCountsMatch
{

View File

@@ -22,6 +22,9 @@ public:
{
}
static TxConsequences
makeTxConsequences(PreflightContext const& ctx);
static std::uint32_t
getFlagsMask(PreflightContext const& ctx);

View File

@@ -3,8 +3,8 @@
#include <xrpl/basics/base_uint.h>
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/ledger/OwnerCounts.h>
#include <xrpl/ledger/RawView.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Issue.h>
#include <xrpl/protocol/MPTIssue.h>

View File

@@ -7,6 +7,7 @@
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/ledger/ApplyView.h>
#include <xrpl/ledger/OwnerCounts.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/ledger/helpers/SponsorHelpers.h>
#include <xrpl/protocol/AccountID.h>

View File

@@ -485,7 +485,7 @@ issueIOU(
limit,
0,
0,
{},
sponsorSle,
j);
}

View File

@@ -636,14 +636,14 @@ STObject::getAccountID(SField const& field) const
AccountID
STObject::getInitiator() const
{
// If sfDelegate is present, the delegate account is the payer
// If sfDelegate is present, the delegate account is the initiator
// note: if a delegate is specified, its authorization to act on behalf of the account is
// enforced in `Transactor::invokeCheckPermission`
// cryptographic signature validity is checked separately (e.g., in `Transactor::checkSign`)
if (isFieldPresent(sfDelegate))
return getAccountID(sfDelegate);
// Default payer
// Default initiator
return getAccountID(sfAccount);
}

View File

@@ -12,7 +12,6 @@
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/Protocol.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>

View File

@@ -14,7 +14,6 @@
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/Protocol.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>

View File

@@ -9,7 +9,6 @@
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/Protocol.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>

View File

@@ -14,7 +14,6 @@
#include <xrpl/ledger/helpers/EscrowHelpers.h>
#include <xrpl/ledger/helpers/MPTokenHelpers.h>
#include <xrpl/ledger/helpers/RippleStateHelpers.h>
#include <xrpl/ledger/helpers/SponsorHelpers.h>
#include <xrpl/ledger/helpers/TokenHelpers.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Concepts.h>
@@ -224,10 +223,6 @@ EscrowFinish::preclaim(PreclaimContext const& ctx)
}
}
auto const sponsorSle = getTxReserveSponsor(ctx.view, ctx.tx);
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE
return tesSUCCESS;
}

View File

@@ -14,7 +14,6 @@
#include <xrpl/protocol/Keylet.h>
#include <xrpl/protocol/Protocol.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/STArray.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STTx.h>

View File

@@ -406,7 +406,7 @@ Payment::preclaim(PreclaimContext const& ctx)
// Since the reserve is covered by the sponsor, you don't need to hold the
// 1-increment reserve yourself.
JLOG(ctx.j.trace()) << "Delay transaction: Destination account does not exist. "
<< "Insufficent payment to create account.";
<< "Insufficient payment to create account.";
// TODO: de-dupe
// Another transaction could create the account and then this

View File

@@ -22,6 +22,13 @@
namespace xrpl {
TxConsequences
SponsorshipSet::makeTxConsequences(PreflightContext const& ctx)
{
auto const feeAmount = ctx.tx[~sfFeeAmount];
return TxConsequences{ctx.tx, feeAmount.has_value() ? feeAmount->xrp() : beast::kZero};
}
std::uint32_t
SponsorshipSet::getFlagsMask(PreflightContext const& ctx)
{

View File

@@ -316,18 +316,19 @@ TrustSet::doApply()
// well. A person with no intention of using the gateway
// could use the extra XRP for their own purposes.
auto const sponsorSle = getTxReserveSponsor(ctx_.getApplyViewContext());
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE
auto const sponsorExp = getTxReserveSponsor(ctx_.getApplyViewContext());
if (!sponsorExp)
return sponsorExp.error(); // LCOV_EXCL_LINE
auto const sponsorSle = *sponsorExp;
auto getSponsor = [&sponsorSle = *sponsorSle, this](AccountID const& account) {
auto getSponsor = [&sponsorSle, this](AccountID const& account) {
return (sponsorSle && account == accountID_) ? sponsorSle : SLE::pointer();
};
// The "free-tier" shortcut (ownerCount < 2) only applies when there is no sponsor.
// 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);
bool const freeTrustLine = !sponsorSle && (ownerCount(sle, j_) < 2);
std::uint32_t const uQualityIn(bQualityIn ? ctx_.tx.getFieldU32(sfQualityIn) : 0);
std::uint32_t uQualityOut(bQualityOut ? ctx_.tx.getFieldU32(sfQualityOut) : 0);
@@ -610,7 +611,7 @@ TrustSet::doApply()
// Reserve is not scaled by load.
else if (
auto const ret =
checkReserve(ctx_.getApplyViewContext(), sle, preFeeBalance_, *sponsorSle, {}, j_);
checkReserve(ctx_.getApplyViewContext(), sle, preFeeBalance_, sponsorSle, {}, j_);
!freeTrustLine && bReserveIncrease && !isTesSuccess(ret))
{
JLOG(j_.trace()) << "Delay transaction: Insufficent reserve to "
@@ -646,7 +647,7 @@ TrustSet::doApply()
ctx_.getApplyViewContext(),
sle,
preFeeBalance_,
*sponsorSle,
sponsorSle,
{.ownerCountDelta = 1},
j_);
!freeTrustLine && !isTesSuccess(ret)) // Reserve is not scaled by load.
@@ -685,7 +686,7 @@ TrustSet::doApply()
saLimitAllow, // Limit for who is being charged.
uQualityIn,
uQualityOut,
*sponsorSle,
sponsorSle,
viewJ);
}

View File

@@ -5155,8 +5155,7 @@ class Invariants_test : public beast::unit_test::Suite
testcase("Sponsorship");
{
auto const expectMessage =
"SponsoredOwnerCount does not equal "
"SponsoringOwnerCount delta.";
"SponsoredOwnerCount does not equal SponsoringOwnerCount delta.";
doInvariantCheck(
{{expectMessage}}, [&](Account const& a1, Account const& a2, ApplyContext& ac) {

View File

@@ -1781,7 +1781,7 @@ public:
auto aliceBalance = env.balance(alice);
auto bobBalance = env.balance(bob);
auto sponsorBalance = env.balance(sponsor);
auto sponsorFee = sponsor::sponsorFeeBalance(env, sponsor, alice);
auto sponsorFee = sponsor::sponsorshipFeeBalance(env, sponsor, alice);
auto const sendAmt = XRP(100);
auto const feeAmt = XRP(10);
@@ -1792,7 +1792,7 @@ public:
BEAST_EXPECT(env.balance(bob) == bobBalance + sendAmt);
BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
BEAST_EXPECT(
sponsor::sponsorFeeBalance(env, sponsor, alice) == sponsorFee - feeAmt);
sponsor::sponsorshipFeeBalance(env, sponsor, alice) == sponsorFee - feeAmt);
}
{
@@ -1802,7 +1802,7 @@ public:
auto aliceBalance = env.balance(alice);
auto bobBalance = env.balance(bob);
auto sponsorBalance = env.balance(sponsor);
auto sponsorFee = sponsor::sponsorFeeBalance(env, sponsor, alice);
auto sponsorFee = sponsor::sponsorshipFeeBalance(env, sponsor, alice);
env(pay(alice, bob, XRP(100)),
Fee(XRP(90) + drops(1)),
@@ -1813,7 +1813,7 @@ public:
BEAST_EXPECT(env.balance(alice) == aliceBalance);
BEAST_EXPECT(env.balance(bob) == bobBalance);
BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
BEAST_EXPECT(sponsor::sponsorFeeBalance(env, sponsor, alice) == sponsorFee);
BEAST_EXPECT(sponsor::sponsorshipFeeBalance(env, sponsor, alice) == sponsorFee);
}
// use all FeeAmount
{
@@ -1846,7 +1846,7 @@ public:
auto aliceBalance = env.balance(alice);
auto bobBalance = env.balance(bob);
auto sponsorBalance = env.balance(sponsor);
auto sponsorFee = sponsor::sponsorFeeBalance(env, sponsor, alice);
auto sponsorFee = sponsor::sponsorshipFeeBalance(env, sponsor, alice);
env(pay(alice, bob, XRP(100)),
Fee(XRP(1) + drops(1)),
@@ -1857,7 +1857,7 @@ public:
BEAST_EXPECT(env.balance(alice) == aliceBalance);
BEAST_EXPECT(env.balance(bob) == bobBalance);
BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
BEAST_EXPECT(sponsor::sponsorFeeBalance(env, sponsor, alice) == sponsorFee);
BEAST_EXPECT(sponsor::sponsorshipFeeBalance(env, sponsor, alice) == sponsorFee);
}
}
@@ -1867,7 +1867,7 @@ public:
auto aliceBalance = env.balance(alice);
auto bobBalance = env.balance(bob);
auto sponsorBalance = env.balance(sponsor);
auto sponsorFee = sponsor::sponsorFeeBalance(env, sponsor, alice);
auto sponsorFee = sponsor::sponsorshipFeeBalance(env, sponsor, alice);
auto const feeAmt = XRP(1);
env(pay(alice, bob, XRP(20000)),
@@ -1880,7 +1880,7 @@ public:
BEAST_EXPECT(env.balance(bob) == bobBalance);
BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
BEAST_EXPECT(
sponsor::sponsorFeeBalance(env, sponsor, alice) == sponsorFee - feeAmt);
sponsor::sponsorshipFeeBalance(env, sponsor, alice) == sponsorFee - feeAmt);
}
// make sfFeeAmount absent if tec error and all Fee is paid
@@ -1892,7 +1892,7 @@ public:
BEAST_EXPECT(
env.le(keylet::sponsorship(sponsor, alice))->isFieldPresent(sfFeeAmount));
auto sponsorAvailableFee = sponsor::sponsorFeeBalance(env, sponsor, alice);
auto sponsorAvailableFee = sponsor::sponsorshipFeeBalance(env, sponsor, alice);
env(check::cancel(alice, uint256(1)),
Fee(sponsorAvailableFee),
sponsor::As(sponsor, spfSponsorFee),
@@ -4614,7 +4614,7 @@ public:
auto const bobBalance = env.balance(bob);
auto const carolBalance = env.balance(carol);
auto const sponsorBalance = env.balance(sponsor);
auto const sponsorFee = sponsor::sponsorFeeBalance(env, sponsor, bob);
auto const sponsorFee = sponsor::sponsorshipFeeBalance(env, sponsor, bob);
auto const sendAmt = XRP(100);
auto const feeAmt = XRP(10);
@@ -4633,7 +4633,7 @@ public:
BEAST_EXPECT(env.balance(carol) == carolBalance + sendAmt);
BEAST_EXPECT(env.balance(sponsor) == sponsorBalance);
// sponsorship(sponsor, bob) pays the fee, bob is sfDelegate
BEAST_EXPECT(sponsor::sponsorFeeBalance(env, sponsor, bob) == sponsorFee - feeAmt);
BEAST_EXPECT(sponsor::sponsorshipFeeBalance(env, sponsor, bob) == sponsorFee - feeAmt);
}
}

View File

@@ -90,7 +90,7 @@ ledgerEntry(jtx::Env& env, jtx::Account const& sponsor, jtx::Account const& spon
}
STAmount
sponsorFeeBalance(jtx::Env& env, jtx::Account const& sponsor, jtx::Account const& sponsee)
sponsorshipFeeBalance(jtx::Env& env, jtx::Account const& sponsor, jtx::Account const& sponsee)
{
return env.le(keylet::sponsorship(sponsor, sponsee))->getFieldAmount(sfFeeAmount).xrp();
}

View File

@@ -6,6 +6,7 @@
#include <xrpl/basics/base_uint.h>
#include <xrpl/json/json_value.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <cstdint>
@@ -99,6 +100,6 @@ json::Value
ledgerEntry(jtx::Env& env, jtx::Account const& sponsor, jtx::Account const& sponsee);
STAmount
sponsorFeeBalance(jtx::Env& env, jtx::Account const& sponsor, jtx::Account const& sponsee);
sponsorshipFeeBalance(jtx::Env& env, jtx::Account const& sponsor, jtx::Account const& sponsee);
} // namespace xrpl::test::jtx::sponsor

View File

@@ -17,6 +17,7 @@
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/ledger/ApplyView.h>
#include <xrpl/ledger/ApplyViewImpl.h>
#include <xrpl/ledger/OwnerCounts.h>
#include <xrpl/ledger/PaymentSandbox.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/ledger/helpers/RippleStateHelpers.h>

View File

@@ -4,7 +4,6 @@
#include <test/jtx/amount.h>
#include <test/jtx/deposit.h>
#include <test/jtx/envconfig.h>
#include <test/jtx/fee.h>
#include <test/jtx/multisign.h>
#include <test/jtx/offer.h>
#include <test/jtx/owners.h> // IWYU pragma: keep
@@ -1420,10 +1419,6 @@ public:
return testEnv.rpc("json", "account_objects", to_string(params));
};
// Create a sponsorship (alice sponsors bob)
env(sponsor::set(alice, 0, 100, XRP(100)), sponsor::SponseeAcc(bob), Fee(XRP(1)));
env.close();
// Create a trust line for bob (not sponsored)
env(trust(bob, usd(1000)));
env.close();
@@ -1469,17 +1464,7 @@ public:
// sponsored=false on bob should NOT include the sponsored trust line
{
auto const resp = acctObjsSponsored(env, bob.id(), false);
auto const& objs = resp[jss::result][jss::account_objects];
bool foundSponsoredTrustLine = false;
for (auto const& obj : objs)
{
if (obj[sfLedgerEntryType.jsonName] == jss::RippleState)
{
if (obj.isMember(sfHighSponsor.jsonName) || obj.isMember(sfLowSponsor.jsonName))
foundSponsoredTrustLine = true;
}
}
BEAST_EXPECT(!foundSponsoredTrustLine);
BEAST_EXPECT(resp[jss::result][jss::account_objects].size() == 0);
}
// Only the queried side of a shared trust line should determine