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

assorted fixes from review part 2 (#7757)

Browse Source
This commit is contained in:
Mayukha Vadari
2026-07-08 12:29:22 -04:00
committed by GitHub
parent 78e4bb17c8
commit d8616d486c
10 changed files with 57 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
include/xrpl/ledger/helpers/AccountRootHelpers.h
View File

@@ -62,8 +62,8 @@ struct Adjustment
* @param view The ledger view to read from
* @param sle The ledger entry for the account
* @param j Journal for logging
* @param ownerCountAdj Adjustment to the owner count (default: 0)
* @param accountCountAdj Adjustment to the account count (default: 0)
* @param adj Adjustment to the owner/account count (default: 0/0). Positive to add, negative to
* subtract.
* @return The account reserve amount in drops
*/
[[nodiscard]] XRPAmount
@@ -74,8 +74,8 @@ accountReserve(ReadView const& view, SLE::const_ref sle, beast::Journal j, Adjus
* @param view The ledger view to read from
* @param id The account ID
* @param j Journal for logging
* @param ownerCountAdj Adjustment to the owner count (default: 0)
* @param accountCountAdj Adjustment to the account count (default: 0)
* @param adj Adjustment to the owner/account count (default: 0/0). Positive to add, negative to
* subtract.
* @return The account reserve amount in drops
*/
[[nodiscard]] inline XRPAmount
@@ -91,8 +91,8 @@ accountReserve(ReadView const& view, AccountID const& id, beast::Journal j, Adju
* @param accSle The account's ledger entry
* @param accBalance The account's balance
* @param sponsorSle The sponsor's ledger entry (if applicable)
* @param ownerCountAdj Adjustment to the owner count
* @param accountCountAdj Adjustment to the account count (default: 0)
* @param adj Adjustment to the owner/account count (default: 0/0). Positive to add, negative to
* subtract.
* @param j Journal for logging (default: null sink)
* @return Transaction result code
*/
@@ -103,7 +103,7 @@ checkReserve(
STAmount const& accBalance,
SLE::const_ref sponsorSle,
Adjustment adj,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()});
beast::Journal j);
/** Return number of the objects which reserve is covered by the account(sle) (so called "owner
* count"). Actual owner count can be adjusted by delta in ownerCountAdj.

8
include/xrpl/ledger/helpers/EscrowHelpers.h
View File

@@ -78,8 +78,10 @@ escrowUnlockApplyHelper<Issue>(
{
JLOG(journal.trace()) << "Trust line does not exist. "
"Insufficient reserve to create line.";
return tecNO_LINE_INSUF_RESERVE;
// checkReserve can return tecINSUFFICIENT_RESERVE or tecINTERNAL
if (ret == tecINSUFFICIENT_RESERVE)
return tecNO_LINE_INSUF_RESERVE;
return ret;
}
Currency const currency = issue.currency;
@@ -214,8 +216,6 @@ escrowUnlockApplyHelper<MPTIssue>(
// update owner count.
increaseOwnerCount(ctx.view, sleDest, *sponsorSle, 1, journal);
auto mptSle = ctx.view.peek(mptKeylet);
addSponsorToLedgerEntry(mptSle, *sponsorSle);
}
if (!ctx.view.exists(mptKeylet) && !receiverIssuer)

10
include/xrpl/protocol/TxFlags.h
View File

@@ -142,7 +142,7 @@ inline constexpr FlagValue tfUniversalMask = ~tfUniversal;
TF_FLAG(tfMPTCanTrade, lsfMPTCanTrade) \
TF_FLAG(tfMPTCanTransfer, lsfMPTCanTransfer) \
TF_FLAG(tfMPTCanClawback, lsfMPTCanClawback) \
TF_FLAG(tfMPTCanHoldConfidentialBalance, lsfMPTCanHoldConfidentialBalance), \
TF_FLAG(tfMPTCanHoldConfidentialBalance, lsfMPTCanHoldConfidentialBalance), \
MASK_ADJ(0)) \
\
TRANSACTION(MPTokenAuthorize, \
@@ -222,14 +222,14 @@ inline constexpr FlagValue tfUniversalMask = ~tfUniversal;
TF_FLAG(tfSponsorshipSetRequireSignForFee, 0x00010000) \
TF_FLAG(tfSponsorshipClearRequireSignForFee, 0x00020000) \
TF_FLAG(tfSponsorshipSetRequireSignForReserve, 0x00040000) \
TF_FLAG(tfSponsorshipClearRequireSignForReserve, 0x00080000) \
TF_FLAG(tfSponsorshipClearRequireSignForReserve, 0x00080000) \
TF_FLAG(tfDeleteObject, 0x00100000), \
MASK_ADJ(0)) \
\
TRANSACTION(SponsorshipTransfer, \
TF_FLAG(tfSponsorshipEnd, 0x00000001) \
TF_FLAG(tfSponsorshipCreate, 0x00000002) \
TF_FLAG(tfSponsorshipReassign, 0x00000004), \
TF_FLAG(tfSponsorshipEnd, 0x00010000) \
TF_FLAG(tfSponsorshipCreate, 0x00020000) \
TF_FLAG(tfSponsorshipReassign, 0x00040000), \
MASK_ADJ(0))
// clang-format on

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

@@ -97,7 +97,7 @@ accountCountImpl(SLE::const_ref sle, std::int32_t accountCountAdj, beast::Journa
std::int64_t totalAccountCount{currentAccountCount + accountCountAdj};
if (totalAccountCount > std::numeric_limits<std::uint32_t>::max())
{
JLOG(j.error()) << "Reserve count exceeds max!";
JLOG(j.fatal()) << "Reserve count exceeds max!";
totalAccountCount = std::numeric_limits<std::uint32_t>::max();
}
else if (totalAccountCount < 0)
@@ -124,8 +124,7 @@ ownerCount(SLE::const_ref sle, beast::Journal j, std::int32_t ownerCountAdj)
XRPL_ASSERT(
currentOwnerCount >= sponsoredOwnerCount,
"xrpl::ownerCount : OwnerCount must be greater than or equal to "
"SponsoredOwnerCount");
"xrpl::ownerCount : OwnerCount must be greater than or equal to SponsoredOwnerCount");
std::int64_t deltaCount =
static_cast<std::int64_t>(ownerCountAdj) - sponsoredOwnerCount + sponsoringOwnerCount;
@@ -331,15 +330,7 @@ decreaseOwnerCountForObject(
XRPAmount
accountReserve(ReadView const& view, SLE::const_ref sle, beast::Journal j, Adjustment adj)
{
if (!sle)
{
Throw<std::runtime_error>("xrpl::accountReserve : valid sle"); // LCOV_EXCL_LINE
}
if (sle->getType() != ltACCOUNT_ROOT)
{
Throw<std::logic_error>("xrpl::accountReserve : valid sle type"); // LCOV_EXCL_LINE
}
XRPL_ASSERT(sle && sle->getType() == ltACCOUNT_ROOT, "xrpl::accountReserve : valid sle");
std::uint32_t const currentOwnerCount = ownerCount(sle, j, adj.ownerCountDelta);
std::uint32_t const currentAccountCount = accountCountImpl(sle, adj.accountCountDelta, j);
@@ -356,6 +347,9 @@ checkReserve(
Adjustment adj,
beast::Journal j)
{
// TODO: swap to assert after fixCleanup3_2_0 is retired
if (!accSle || accSle->getType() != ltACCOUNT_ROOT)
return tefINTERNAL; // LCOV_EXCL_LINE
if (sponsorSle)
{
auto const sle = ctx.view.read(

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

@@ -679,7 +679,12 @@ addEmptyHolding(
if (auto const ret =
checkReserve(ctx, sleDst, priorBalance, sponsorSle, {.ownerCountDelta = 1}, journal);
!isTesSuccess(ret))
return tecNO_LINE_INSUF_RESERVE;
{
// checkReserve can return tecINSUFFICIENT_RESERVE or tecINTERNAL
if (ret == tecINSUFFICIENT_RESERVE)
return tecNO_LINE_INSUF_RESERVE;
return ret;
}
return trustCreate(
ctx.view,

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

@@ -416,6 +416,10 @@ Transactor::checkSponsor(ReadView const& view, STTx const& tx)
auto const hasSponsorSignature = tx.isFieldPresent(sfSponsorSignature);
// Skip Sponsorship existence checks if the sponsor has signed the transaction - this
// transaction is valid regardless of the Sponsorship object.
// The use of the Sponsorship object is properly handled in
// getFeePayer/checkReserve/increaseOwnerCount/decreaseOwnerCount.
if (hasSponsorSignature)
return tesSUCCESS;
@@ -636,7 +640,14 @@ Transactor::payFee()
return tefINTERNAL; // LCOV_EXCL_LINE
}
if (feePaid > balance)
// 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 &&
(feePayer.type == FeePayerType::SponsorPreFunded ||
feePayer.type == FeePayerType::SponsorCoSigned))
{
if ((balance > beast::kZero) && !view().open())
return tecINSUFF_FEE;

11
src/libxrpl/tx/invariants/InvariantCheck.cpp
View File

@@ -138,9 +138,13 @@ XRPNotCreated::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref a
{
switch (before->getType())
{
case ltACCOUNT_ROOT:
case ltACCOUNT_ROOT: {
XRPL_ASSERT(
isXRP((*before)[sfBalance]),
"XRPNotCreated::visitEntry : AccountRoot.Balance is XRP");
drops_ -= (*before)[sfBalance].xrp().drops();
break;
}
case ltPAYCHAN:
drops_ -= ((*before)[sfAmount] - (*before)[sfBalance]).xrp().drops();
break;
@@ -150,7 +154,12 @@ XRPNotCreated::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref a
break;
case ltSPONSORSHIP:
if (before->isFieldPresent(sfFeeAmount))
{
XRPL_ASSERT(
isXRP((*before)[sfFeeAmount]),
"XRPNotCreated::visitEntry : Sponsorship.FeeAmount is XRP");
drops_ -= (*before)[sfFeeAmount].xrp().drops();
}
break;
default:
break;

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

@@ -527,9 +527,6 @@ TrustSet::doApply()
{
SLE::pointer const lowSponsor = getSponsor(uLowAccountID);
// should be checked PreFunded Sponsor before increaseOwnerCount()
// For PreFunded sponsors, we need to check if there are sufficient reserves before
// calling increaseOwnerCount().
if (auto const ret = checkReserve(
ctx_.getApplyViewContext(),
sleLowAccount,
@@ -538,7 +535,12 @@ TrustSet::doApply()
{.ownerCountDelta = 1},
j_);
lowSponsor && !isTesSuccess(ret))
return tecINSUF_RESERVE_LINE;
{
// checkReserve can return tecINSUFFICIENT_RESERVE or tecINTERNAL
if (ret == tecINSUFFICIENT_RESERVE)
return tecINSUF_RESERVE_LINE;
return ret;
}
// Set reserve for low account.
increaseOwnerCount(view(), sleLowAccount, lowSponsor, 1, viewJ);

2
src/test/app/EscrowToken_test.cpp
View File

@@ -937,7 +937,7 @@ struct EscrowToken_test : public beast::unit_test::Suite
auto const expectedResult = env.current()->rules().enabled(fixCleanup3_2_0)
? Ter(tesSUCCESS)
: Ter(tefEXCEPTION);
: Ter(tefINTERNAL);
env(escrow::cancel(alice, alice, seq), Fee(baseFee), expectedResult);
env.close();

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

@@ -3027,76 +3027,6 @@ public:
}
}
void
testDID(bool cosigning)
{
testcase("DID");
using namespace test::jtx;
Account const alice("alice");
Account const sponsor("sponsor");
Account const sponsor2("sponsor2");
{
Env env{*this, testableAmendments()};
env.fund(XRP(1000000), alice, sponsor, sponsor2);
env.close();
// DIDSet
testEachSponsorship(
env,
cosigning,
sponsor,
alice,
1,
1,
tecINSUFFICIENT_RESERVE,
[&](Env& env, auto const& submit) { submit(did::set(alice), did::Uri("uri")); });
// transfer sponsor
auto const keylet = keylet::did(alice);
if (cosigning)
{
env(sponsor::transfer(alice, tfSponsorshipReassign, keylet.key),
sponsor::As(sponsor2, spfSponsorReserve),
Sig(sfSponsorSignature, sponsor2));
env.close();
}
else
{
env(sponsor::set_reserve(sponsor2, 0, 1), sponsor::SponseeAcc(alice));
env.close();
env(sponsor::transfer(alice, tfSponsorshipReassign, keylet.key),
sponsor::As(sponsor2, spfSponsorReserve));
env.close();
}
BEAST_EXPECT(ownerCount(env, alice) == 1);
BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 1);
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor) == 0);
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor2) == 1);
if (!cosigning)
{
auto const sponsorshipSle = env.le(keylet::sponsorship(sponsor2, alice));
BEAST_EXPECT(sponsorshipSle);
if (sponsorshipSle)
{
BEAST_EXPECT(
!sponsorshipSle->isFieldPresent(sfRemainingOwnerCount) ||
sponsorshipSle->getFieldU32(sfRemainingOwnerCount) == 0);
}
}
// DIDDelete
env(did::del(alice));
env.close();
BEAST_EXPECT(ownerCount(env, alice) == 0);
BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 0);
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor) == 0);
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor2) == 0);
}
}
void
testEscrow(bool cosigning)
{