assorted fixes from review (#7752)

This commit is contained in:
Mayukha Vadari
2026-07-08 12:12:38 -04:00
committed by GitHub
parent 5be01f06ce
commit ec77e15ac7
18 changed files with 104 additions and 111 deletions

View File

@@ -179,6 +179,7 @@ issueIOU(
AccountID const& account,
STAmount const& amount,
Issue const& issue,
SLE::ref sponsorSle,
beast::Journal j);
[[nodiscard]] TER

View File

@@ -12,13 +12,44 @@
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/TxFormats.h>
#include <cstdint>
#include <expected>
#include <optional>
#include <unordered_set>
namespace xrpl {
inline std::unordered_set<TxType> 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,
};
inline bool
isFeeSponsored(STTx const& tx)
{

View File

@@ -222,6 +222,7 @@ adjustOwnerCountSigned(
if (!accountSle)
return; // LCOV_EXCL_LINE
auto const accountID = accountSle->getAccountID(sfAccount);
auto const sleType = accountSle->getType();
bool const validType = sponsorSle ? sleType == ltACCOUNT_ROOT
: sleType == ltLOAN_BROKER || sleType == ltACCOUNT_ROOT;
@@ -235,7 +236,6 @@ adjustOwnerCountSigned(
sleType == ltACCOUNT_ROOT ? OwnerCounts(*accountSle) : OwnerCounts());
OwnerCounts totalOwnerCount(currentOwnerCount);
auto const accountID = accountSle->getAccountID(sfAccount);
if (sponsorSle)
{
bool const validSponsorType = sponsorSle->getType() == ltACCOUNT_ROOT;

View File

@@ -378,7 +378,7 @@ updateTrustLine(
// VFALCO Where is the line being deleted?
// Clear the reserve of the sender, possibly delete the line!
auto const currentSponsor =
getLedgerEntryReserveSponsor(view, state, !bSenderHigh ? sfLowSponsor : sfHighSponsor);
getLedgerEntryReserveSponsor(view, state, bSenderHigh ? sfHighSponsor : sfLowSponsor);
decreaseOwnerCount(view, sle, currentSponsor, 1, j);
// Clear reserve flag.
@@ -393,12 +393,14 @@ updateTrustLine(
return false;
}
// Only used in tests
TER
issueIOU(
ApplyView& view,
AccountID const& account,
STAmount const& amount,
Issue const& issue,
SLE::ref sponsorSle,
beast::Journal j)
{
XRPL_ASSERT(

View File

@@ -34,7 +34,6 @@
#include <xrpl/protocol/SystemParameters.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/TxFormats.h>
#include <xrpl/protocol/TxMeta.h>
#include <xrpl/protocol/XRPAmount.h>
#include <xrpl/server/LoadFeeTrack.h>
@@ -180,21 +179,18 @@ preflight1Sponsor(PreflightContext const& ctx, AccountID const& id)
if ((hasSponsor || hasSponsorFlags || hasSponsorSig) && !ctx.rules.enabled(featureSponsor))
return temDISABLED;
if (!hasSponsor)
if (hasSponsor != hasSponsorFlags)
{
if (hasSponsorFlags)
{
JLOG(ctx.j.debug()) << "preflight1: sponsor flags without sponsor definition";
return temINVALID_FLAG;
}
if (hasSponsorSig)
{
JLOG(ctx.j.debug()) << "preflight1: sponsor signature without sponsor definition";
return temMALFORMED;
}
JLOG(ctx.j.debug()) << "preflight1: sponsor and sponsor flags mismatch";
return temINVALID_FLAG;
}
else if (hasSponsorFlags)
if (hasSponsorSig && (!hasSponsor || !hasSponsorFlags))
{
JLOG(ctx.j.debug()) << "preflight1: sponsor signature without sponsor definition";
return temMALFORMED;
}
if (hasSponsorFlags)
{
auto const sponsorFlags = ctx.tx.getFieldU32(sfSponsorFlags);
if (((sponsorFlags & spfSponsorFlagMask) != 0u) || sponsorFlags == 0)
@@ -205,36 +201,8 @@ preflight1Sponsor(PreflightContext const& ctx, AccountID const& id)
// Reserve sponsorship is only permitted for an explicit allow-list of
// transaction types, for v1. All other tx types reject spfSponsorReserve here.
if ((sponsorFlags & spfSponsorReserve) != 0u)
if (isReserveSponsored(ctx.tx))
{
static std::unordered_set<TxType> 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,
};
if (!kReserveSponsorAllowed.contains(ctx.tx.getTxnType()))
{
JLOG(ctx.j.debug())
@@ -243,11 +211,6 @@ preflight1Sponsor(PreflightContext const& ctx, AccountID const& id)
}
}
}
else
{
JLOG(ctx.j.debug()) << "preflight1: no sponsor flags";
return temINVALID_FLAG;
}
if (hasSponsor && ctx.tx.getAccountID(sfSponsor) == id)
{

View File

@@ -1026,7 +1026,7 @@ applyCreateAccountAttestations(
// Check reserve
auto const balance = (*sleDoor)[sfBalance];
auto const reserve = accountReserve(view, sleDoor, j, {.ownerCountDelta = 1});
auto const reserve = accountReserve(psb, sleDoor, j, {.ownerCountDelta = 1});
if (balance < reserve)
return std::unexpected(tecINSUFFICIENT_RESERVE);

View File

@@ -94,6 +94,20 @@ CredentialAccept::doApply()
if (!sleSubject || !sleIssuer)
return tefINTERNAL; // LCOV_EXCL_LINE
auto txSponsorSle = getTxReserveSponsor(ctx_.getApplyViewContext());
if (!txSponsorSle)
return txSponsorSle.error(); // LCOV_EXCL_LINE
if (auto const ret = checkReserve(
ctx_.getApplyViewContext(),
sleSubject,
preFeeBalance_,
*txSponsorSle,
{.ownerCountDelta = 1},
j_);
!isTesSuccess(ret))
return ret;
auto const credType(ctx_.tx[sfCredentialType]);
Keylet const credentialKey = keylet::credential(accountID_, issuer, credType);
auto const sleCred = view().peek(credentialKey); // Checked in preclaim()
@@ -110,29 +124,12 @@ CredentialAccept::doApply()
sleCred->setFieldU32(sfFlags, lsfAccepted);
auto txSponsorSle = getTxReserveSponsor(ctx_.getApplyViewContext());
if (!txSponsorSle)
return txSponsorSle.error(); // LCOV_EXCL_LINE
// Release the original creation sponsor from the credential (it covered
// the issuer's reserve), then assign the accept tx's sponsor (if any) so
// the credential reflects whoever is now covering the subject's reserve.
decreaseOwnerCountForObject(view(), sleIssuer, sleCred, 1, j_);
removeSponsorFromLedgerEntry(sleCred);
if (view().rules().enabled(featureSponsor) || view().rules().enabled(fixCleanup3_3_0))
{
if (auto const ret = checkReserve(
ctx_.getApplyViewContext(),
sleSubject,
preFeeBalance_,
*txSponsorSle,
{.ownerCountDelta = 1},
j_);
!isTesSuccess(ret))
return ret;
}
addSponsorToLedgerEntry(sleCred, *txSponsorSle);
increaseOwnerCount(view(), sleSubject, *txSponsorSle, 1, j_);
view().update(sleCred);

View File

@@ -661,10 +661,9 @@ AMMWithdraw::withdraw(
return tecINTERNAL; // LCOV_EXCL_LINE
auto const balance = (*sleAccount)[sfBalance]->xrp();
std::uint32_t const currentOwnerCount = ownerCount(sleAccount, journal);
// See also TrustSet::doApply() and MPTokenAuthorize::authorize()
XRPAmount const reserve(
(currentOwnerCount < 2)
(ownerCount(sleAccount, journal) < 2)
? XRPAmount(beast::kZero)
: accountReserve(view, sleAccount, journal, {.ownerCountDelta = 1}));

View File

@@ -333,8 +333,8 @@ NFTokenMint::doApply()
// requiring the reserve to be met each time. The reserve is
// only managed when a new NFT page or sell offer is added.
auto const sleAccount = view().read(keylet::account(accountID_));
if (!sleAccount)
return tecINTERNAL; // LCOV_EXCL_LINE
if (!sleAccount && view().rules().enabled(featureSponsor))
return tefINTERNAL; // LCOV_EXCL_LINE
if (auto const ownerCountAfter = sleAccount->getFieldU32(sfOwnerCount);
ownerCountAfter > ownerCountBefore)

View File

@@ -4,6 +4,7 @@
#include <xrpl/core/ServiceRegistry.h>
#include <xrpl/ledger/ApplyView.h>
#include <xrpl/ledger/helpers/AccountRootHelpers.h>
#include <xrpl/ledger/helpers/OracleHelpers.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/SField.h>
@@ -70,7 +71,8 @@ OracleDelete::deleteOracle(
if (!sleOwner)
return tecINTERNAL; // LCOV_EXCL_LINE
std::uint32_t const count = sle->getFieldArray(sfPriceDataSeries).size() > 5 ? 2 : 1;
std::uint32_t const count =
calculateOracleReserve(sle->getFieldArray(sfPriceDataSeries).size());
decreaseOwnerCountForObject(view, sleOwner, sle, count, j);
view.erase(sle);

View File

@@ -396,44 +396,42 @@ Payment::preclaim(PreclaimContext const& ctx)
if (ctx.parentBatchId && ctx.view.rules().enabled(featureBatchV1_1))
return tefNO_DST_PARTIAL;
}
if (ctx.tx.isFlag(tfSponsorCreatedAccount))
{
// The minimum amount when creating a Sponsored Account is 1 drop.
// Since the reserve is covered by the sponsor, you don't need to hold the 1-increment
// reserve yourself.
}
else if (dstAmount < STAmount(ctx.view.fees().reserve))
if (dstAmount < STAmount(ctx.view.fees().reserve))
{
// accountReserve is the minimum amount that an account can have.
// Reserve is not scaled by load.
JLOG(ctx.j.trace()) << "Delay transaction: Destination account does not exist. "
<< "Insufficent payment to create account.";
if (!ctx.tx.isFlag(tfSponsorCreatedAccount))
{
// The minimum amount when creating a Sponsored Account is 1 drop.
// 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.";
// TODO: de-dupe
// Another transaction could create the account and then this
// transaction would succeed.
return tecNO_DST_INSUF_XRP;
// TODO: de-dupe
// Another transaction could create the account and then this
// transaction would succeed.
return tecNO_DST_INSUF_XRP;
}
}
}
else
else if (ctx.tx.isFlag(tfSponsorCreatedAccount))
{
// The tfSponsorCreatedAccount flag is specific to account creation via
// sponsorship. If the destination account already exists, applying this
// flag is invalid.
if (ctx.tx.isFlag(tfSponsorCreatedAccount))
return tecNO_SPONSOR_PERMISSION;
return tecNO_SPONSOR_PERMISSION;
}
else if (sleDst->isFlag(lsfRequireDestTag) && !ctx.tx.isFieldPresent(sfDestinationTag))
{
// The tag is basically account-specific information we don't
// understand, but we can require someone to fill it in.
if (sleDst->isFlag(lsfRequireDestTag) && !ctx.tx.isFieldPresent(sfDestinationTag))
{
// The tag is basically account-specific information we don't
// understand, but we can require someone to fill it in.
// We didn't make this test for a newly-formed account because there's
// no way for this field to be set.
JLOG(ctx.j.trace()) << "Malformed transaction: DestinationTag required.";
// We didn't make this test for a newly-formed account because
// there's no way for this field to be set.
JLOG(ctx.j.trace()) << "Malformed transaction: DestinationTag required.";
return tecDST_TAG_NEEDED;
}
return tecDST_TAG_NEEDED;
}
// Payment with at least one intermediate step and uses transitive balances.

View File

@@ -82,8 +82,7 @@ PaymentChannelCreate::preclaim(PreclaimContext const& ctx)
if (!ctx.view.rules().enabled(featureSponsor))
{
auto const balance = (*sle)[sfBalance];
auto const fees = ctx.view.fees();
auto const reserve = fees.reserve + fees.increment * ((*sle)[sfOwnerCount] + 1);
auto const reserve = ctx.view.fees().accountReserve((*sle)[sfOwnerCount] + 1, 1);
if (balance < reserve)
return tecINSUFFICIENT_RESERVE;

View File

@@ -93,14 +93,15 @@ PaymentChannelFund::doApply()
auto const sponsorSle = getTxReserveSponsor(ctx_.getApplyViewContext());
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE
if (auto const ret =
checkReserve(ctx_.getApplyViewContext(), sle, balance, *sponsorSle, {}, j_);
!isTesSuccess(ret))
return ret;
if (auto const ret = checkReserve(
ctx_.getApplyViewContext(), sle, balance - ctx_.tx[sfAmount], {}, {}, j_);
!isTesSuccess(ret))
auto const reserve = accountReserve(view(), sle, j_);
// There are no objects being created here, so sponsorship does not need to be taken into
// account
if (balance < reserve)
return tecINSUFFICIENT_RESERVE;
if (balance < reserve + ctx_.tx[sfAmount])
return tecUNFUNDED;
}

View File

@@ -125,7 +125,7 @@ TicketCreate::doApply()
sleAccountRoot->setFieldU32(sfTicketCount, oldTicketCount + ticketCount);
// Every added Ticket counts against the creator's reserve.
increaseOwnerCount(view(), accountID_, {}, ticketCount, viewJ);
increaseOwnerCount(view(), sleAccountRoot, {}, ticketCount, viewJ);
// TicketCreate is the only transaction that can cause an account root's
// Sequence field to increase by more than one. October 2018.

View File

@@ -5152,7 +5152,7 @@ class Invariants_test : public beast::unit_test::Suite
{
using namespace test::jtx;
using namespace std::string_literals;
testcase << "Sponsorship";
testcase("Sponsorship");
{
auto const expectMessage =
"SponsoredOwnerCount does not equal "

View File

@@ -243,7 +243,7 @@ class PaymentSandbox_test : public beast::unit_test::Suite
auto const startingAmount =
accountHolds(pv, alice, iss.currency, iss.account, FreezeHandling::IgnoreFreeze, j);
BEAST_EXPECT(issueIOU(pv, alice, toCredit, iss, j) == tesSUCCESS);
BEAST_EXPECT(issueIOU(pv, alice, toCredit, iss, {}, j) == tesSUCCESS);
BEAST_EXPECT(
accountHolds(
pv, alice, iss.currency, iss.account, FreezeHandling::IgnoreFreeze, j) ==