Skip to content

checkInsufficientReserve ->checkXrpBalance #7542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
oleks-rip wants to merge 8 commits into
base: xrplf/sponsor
Choose a base branch
from
Open

checkInsufficientReserve ->checkXrpBalance #7542

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
26b8138
GetFeePayer some refactoring
oleks-rip Jun 6, 2026
8f3e5f9
checkXRPBalance
oleks-rip Jun 7, 2026
132cc3b
clang-tidy
oleks-rip Jun 15, 2026
43c02c3
Merge branch 'xrplf/sponsor' into check_xrp
oleks-rip Jun 15, 2026
8f8dd07
Delegating skip sponsorship check
oleks-rip Jun 16, 2026
5e83693
AI fixes
oleks-rip Jun 16, 2026
932f19d
Fix calculation fee for TxQ, now it is aware of Delegate and Sponsor
oleks-rip Jun 18, 2026
8e20597
Fix view.hook to works with sponsor
oleks-rip Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
239 changes: 219 additions & 20 deletions include/xrpl/ledger/helpers/AccountRootHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <expected>
#include <set>
#include <type_traits>
#include <vector>

namespace xrpl {
Expand All @@ -33,12 +34,26 @@ 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"
*/
[[nodiscard]] XRPAmount
xrpLiquid(
ReadView const& view,
SLE::const_ref accSle,
std::int32_t ownerCountAdj,
beast::Journal j);

[[nodiscard]] XRPAmount
xrpLiquid(
Comment on lines +37 to +45

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first helper is only used once in source code and the second helper isn't used at all. I would prefer just refactoring those areas so that we don't need to keep these.

@oleks-rip oleks-rip Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First helper used twice, second maybe left after it was used, need to remove

ApplyView const& view,
STTx const& tx,
SLE::const_ref accSle,
std::int32_t ownerCountAdj,
beast::Journal j);

// Returns the account reserve.
Comment thread
oleks-rip marked this conversation as resolved.
// Actual owner count and reserve count can be adjusted
// The reserve is calculated as
// (ownerCount + "sponsoring count" - "sponsored count" + "owner adjustment") * increment
// + ("1 if not sponsored account else 0" + "sponsoring account count") * "reserve base"
[[nodiscard]] XRPAmount
accountReserve(
ReadView const& view,
Expand All @@ -58,70 +73,254 @@ accountReserve(
return accountReserve(view, view.read(keylet::account(id)), j, ownerCountAdj, reserveCountAdj);
}

// Returns basic reserve for abstract account for given objects count
// Mostly used by tests
XRPAmount
baseAccountReserve(ReadView const& view, std::int32_t ownerCount);

[[nodiscard]] TER
checkInsufficientReserve(
enum class FeePayerType {
Account,
Delegate,
SponsorCoSigned,
SponsorPreFunded,
};
struct FeePayer
{
AccountID id;
Keylet keylet;
SF_AMOUNT const& balanceField;
FeePayerType type{FeePayerType::Account};
};
FeePayer
getFeePayer(ReadView const& view, STTx const& tx);

// checkXrpBalance - checks whether there are enough funds in the account for cover reserve and
// additional expenses (balanceAdj). Works through xrpLiquid()

TER
checkXrpBalanceHlp(

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO a function with this many parameters is a code smell

@oleks-rip oleks-rip Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ye, that why it is helper, not intended for user usage. If you like to refactor - you should start from amm / token functions, which are not helpers and have more parameters

ReadView const& view,
bool apply,
STTx const& tx,
std::optional<AccountID> const& accID,
std::optional<std::reference_wrapper<SLE::const_pointer const>> const& accOpt,
Comment on lines +105 to +106

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are both these fields needed?

@oleks-rip oleks-rip Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometime tx have sle, sometime doesn't, to decrease number of shamap searches.

XRPAmount balanceAcc, // if set disable automatic balance calculation

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed if you're passing in the account SLE?

@oleks-rip oleks-rip Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Special cases when balance passed after some tx manipulations

std::optional<std::reference_wrapper<SLE::const_pointer const>> const& sponsorOpt,
std::int32_t ownerCountAdj,
std::int32_t reserveCountAdj,
XRPAmount balanceAdj,
Comment on lines +109 to +111

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the differences between these 3 values?

@oleks-rip oleks-rip Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't they self explanatory?
Owner count adjustement adjust the number of the objectes owned by the account
Reserve count adjustement adjust the number of the accounts "owned" by the account
Balance adjustement adjust the balance of the account.
Adjustments need to make calculation BEFORE actually change the values

bool moreThan2, // special case, reserve doesn't check if current ownerCount < 2

@yinyiqian1 yinyiqian1 Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining a policy enum can be more descriptive and robust.

enum class ReserveCheckPolicy {
Enforce,
SkipIfOwnerCountBelow2,
};

beast::Journal j,
bool checkApplicability = true // SponsorTransfer can break relations tx[sfAccount] === accSle

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this comment means/what checkApplicability means

@oleks-rip oleks-rip Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is check tx[sfAccount] === accSle, if not - throw. SponsorTransfer can break it. Flag for special case added

);

// simple case, only ownerAdjustment
template <class V>
[[nodiscard]] TER
checkXrpBalance(
V const& view,
STTx const& tx,
AccountID const& accID,
std::int32_t ownerCountAdj,
beast::Journal j)
{
static XRPAmount const kA;
bool apply = false; // NOLINT
if constexpr (std::is_base_of_v<ApplyView, std::remove_cvref_t<decltype(view)>>)
apply = true;
return checkXrpBalanceHlp(view, apply, tx, accID, {}, kA, {}, ownerCountAdj, 0, kA, false, j);
}

// simple + balance adjustment
template <class V>
[[nodiscard]] TER
checkXrpBalance(
V const& view,
STTx const& tx,
AccountID const& accID,
std::int32_t ownerCountAdj,
XRPAmount balanceAdj,
beast::Journal j)
{
static XRPAmount const kA;
bool apply = false; // NOLINT
if constexpr (std::is_base_of_v<ApplyView, std::remove_cvref_t<decltype(view)>>)
apply = true;
return checkXrpBalanceHlp(
view, apply, tx, accID, {}, kA, {}, ownerCountAdj, 0, balanceAdj, false, j);
}

// simple/SLE + balance adjustment
template <class V>
[[nodiscard]] TER
checkXrpBalance(
V const& view,
STTx const& tx,
SLE::const_ref accSle,
std::int32_t ownerCountAdj,
XRPAmount balanceAdj,
beast::Journal j)
{
static XRPAmount const kA;
bool apply = false; // NOLINT
if constexpr (std::is_base_of_v<ApplyView, std::remove_cvref_t<decltype(view)>>)
apply = true;
return checkXrpBalanceHlp(
view, apply, tx, {}, accSle, kA, {}, ownerCountAdj, 0, balanceAdj, false, j);
}

// simple/Sle + sponsor(re-usage, checks on caller) + balance adjustment
template <class V>
[[nodiscard]] TER
checkXrpBalance(
V const& view,
STTx const& tx,
SLE::const_ref accSle,
STAmount const& accBalance,
SLE::const_ref sponsorSle,
std::int32_t ownerCountDelta,
std::int32_t reserveCountDelta = 0,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()});
std::int32_t ownerCountAdj,
XRPAmount balanceAdj,
beast::Journal j)
{
static XRPAmount const kA;
bool apply = false; // NOLINT
if constexpr (std::is_base_of_v<ApplyView, std::remove_cvref_t<decltype(view)>>)
apply = true;
return checkXrpBalanceHlp(
view, apply, tx, {}, accSle, kA, sponsorSle, ownerCountAdj, 0, balanceAdj, false, j);
}

// simple + sponsor(re-usage, checks on caller) + balance adjustment
template <class V>

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all these different versions of the function necessary?

@oleks-rip oleks-rip Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they more than 1, why not?

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's harder to read and audit, and a larger diff

@oleks-rip oleks-rip Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

larger in 1 place, lesser in all usage places (~ 30).

[[nodiscard]] TER
checkXrpBalance(
V const& view,
STTx const& tx,
AccountID const& accID,
SLE::const_ref sponsorSle,
std::int32_t ownerCountAdj,
XRPAmount balanceAdj,
beast::Journal j)
{
static XRPAmount const kA;
bool apply = false;
if constexpr (std::is_base_of_v<ApplyView, std::remove_cvref_t<decltype(view)>>)
apply = true;
return checkXrpBalanceHlp(
view, apply, tx, accID, {}, kA, sponsorSle, ownerCountAdj, 0, balanceAdj, false, j);
}

// simple/accountSle + balance(passed manually) + sponsor(re-usage, checks on caller) + moreThan2
// check
template <class V>
[[nodiscard]] TER
checkXrpBalance(
V const& view,
STTx const& tx,
SLE::const_ref accSle,
SLE::const_ref sponsorSle,
std::int32_t ownerCountAdj,
bool moreThan2,
beast::Journal j)
{
static XRPAmount const kA;

bool apply = false; // NOLINT
if constexpr (std::is_base_of_v<ApplyView, std::remove_cvref_t<decltype(view)>>)
apply = true;
return checkXrpBalanceHlp(
view, apply, tx, {}, accSle, kA, sponsorSle, ownerCountAdj, 0, kA, moreThan2, j);
}

// simple/accountSle + sponsor(re-usage, checks on caller)
template <class V>
[[nodiscard]] inline TER
checkXrpBalance(
V const& view,
STTx const& tx,
SLE::const_ref accSle,
SLE::const_ref sponsorSle,
std::int32_t ownerCountAdj,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()})
{
return checkXrpBalance(view, tx, accSle, sponsorSle, ownerCountAdj, false, j);
}

// simple/accountSle + balance(passed manually) + sponsor(re-usage, checks on caller)
template <class V>
[[nodiscard]] inline TER
checkXrpBalance(
V const& view,
STTx const& tx,
SLE::const_ref accSle,
XRPAmount balanceAcc,
SLE::const_ref sponsorSle,
std::int32_t ownerCountAdj,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()})
{
static XRPAmount const kA;

bool apply = false;
if constexpr (std::is_base_of_v<ApplyView, std::remove_cvref_t<decltype(view)>>)
apply = true;
return checkXrpBalanceHlp(
view, apply, tx, {}, accSle, balanceAcc, sponsorSle, ownerCountAdj, 0, kA, false, j);
}

// Returns the number of objects owned by the account
std::uint32_t
ownerCount(
ReadView const& view,
SLE::const_ref sle,
SLE::const_ref accSle,
Comment thread
oleks-rip marked this conversation as resolved.
beast::Journal j,
std::int32_t ownerCountAdj = 0);

/** Adjust the owner count up or down. */
// Adjust the owner count up or down for Account, Sponsor Account(if set) and Sponsorship object(if
// exists)
void
adjustOwnerCount(
ApplyView& view,
SLE::ref accountSle,
SLE::ref sponsorSle,
std::int32_t amount,
std::int32_t ownerCountAdj,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()});

inline void
adjustOwnerCount(
ApplyView& view,
AccountID const& account,
std::optional<AccountID> const& sponsor,
std::int32_t amount,
std::int32_t ownerCountAdj,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()})
{
adjustOwnerCount(
view,
view.peek(keylet::account(account)),
sponsor ? view.peek(keylet::account(*sponsor)) : SLE::pointer(),
amount,
ownerCountAdj,
j);
}

// Wrappers for adjustOwnerCount, retrive Sponsor(if exists) from the object.
// If |adjustment| > 0 then object is complex (like SignerList).
// ownerCountAdj < 0
void
adjustOwnerCountObj(
ApplyView& view,
SLE::ref accountSle,
SLE::ref objectSle,
std::int32_t amount,
std::int32_t ownerCountAdj,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()});

inline void
adjustOwnerCountObj(
ApplyView& view,
AccountID const& account,
SLE::ref objectSle,
std::int32_t amount,
std::int32_t ownerCountAdj,
beast::Journal j = beast::Journal{beast::Journal::getNullSink()})
{
SLE::ref accountSle = view.peek(keylet::account(account));
adjustOwnerCountObj(view, accountSle, objectSle, amount, j);
adjustOwnerCountObj(view, accountSle, objectSle, ownerCountAdj, j);
}

/** Returns IOU issuer transfer fee as Rate. Rate specifies
Expand Down
27 changes: 10 additions & 17 deletions include/xrpl/ledger/helpers/EscrowHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,14 @@ escrowUnlockApplyHelper<Issue>(
if (!view.exists(trustLineKey) && createAsset)
{
// Can the account cover the trust line's reserve?
auto const sponsorSle = getTxReserveSponsor(view, tx);
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE

if (auto const ret =
checkInsufficientReserve(view, tx, sleDest, xrpBalance, *sponsorSle, 1, 0, journal);
auto const sponsorSle = getTxReserveSponsor(view, tx, sleDest->at(sfAccount));
if (auto const ret = checkXrpBalance(view, tx, sleDest, sponsorSle, 1, journal);
!isTesSuccess(ret))
{
JLOG(journal.trace()) << "Trust line does not exist. "
"Insufficient reserve to create line.";

if (ret == tecINTERNAL && !view.rules().enabled(fixCleanup3_2_0))
return tefEXCEPTION;
Comment on lines +68 to +69

@mvadari mvadari Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did this come from? Looks like a potential consensus breaking change

@oleks-rip oleks-rip Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From that bug that you simulate to keep consensus. It is hard to simulate it now, so we simulate exception

return tecNO_LINE_INSUF_RESERVE;
}

Expand All @@ -92,7 +89,7 @@ escrowUnlockApplyHelper<Issue>(
Issue(currency, receiver), // limit of zero
0, // quality in
0, // quality out
*sponsorSle, // sponsor
sponsorSle, // sponsor
journal); // journal
!isTesSuccess(ter))
{
Expand Down Expand Up @@ -189,25 +186,21 @@ escrowUnlockApplyHelper<MPTIssue>(
auto const mptKeylet = keylet::mptoken(issuanceKey.key, receiver);
if (!view.exists(mptKeylet) && createAsset && !receiverIssuer)
{
auto const sponsorSle = getTxReserveSponsor(view, tx);
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE

if (auto const ret =
checkInsufficientReserve(view, tx, sleDest, xrpBalance, *sponsorSle, 1, 0, journal);
auto const sponsorSle = getTxReserveSponsor(view, tx, sleDest->at(sfAccount));
if (auto const ret = checkXrpBalance(view, tx, sleDest, sponsorSle, 1, journal);
!isTesSuccess(ret))
return ret;

if (auto const ter = createMPToken(view, mptID, receiver, *sponsorSle, 0);
if (auto const ter = createMPToken(view, mptID, receiver, sponsorSle, 0);
!isTesSuccess(ter))
{
return ter; // LCOV_EXCL_LINE
}

// update owner count.
adjustOwnerCount(view, sleDest, *sponsorSle, 1, journal);
adjustOwnerCount(view, sleDest, sponsorSle, 1, journal);
auto mptSle = view.peek(mptKeylet);
addSponsorToLedgerEntry(mptSle, *sponsorSle);
addSponsorToLedgerEntry(mptSle, sponsorSle);
}

if (!view.exists(mptKeylet) && !receiverIssuer)
Expand Down
11 changes: 9 additions & 2 deletions include/xrpl/ledger/helpers/NFTokenHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,14 @@ findTokenAndPage(ApplyView& view, AccountID const& owner, uint256 const& nftoken

/** Insert the token in the owner's token directory. */
TER
insertToken(ApplyView& view, STTx const& tx, AccountID owner, SLE::ref sponsorSle, STObject&& nft);
insertToken(
ApplyView& view,
STTx const& tx,
SLE::ref ownerSle,
XRPAmount priorBalance,
SLE::ref sponsorSle,
STObject&& nft,
bool checkBalance = true);

/** Remove the token from the owner's token directory. */
TER
Expand Down Expand Up @@ -108,7 +115,7 @@ TER
tokenOfferCreateApply(
ApplyView& view,
STTx const& tx,
AccountID const& acctID,
SLE::ref accSle,
STAmount const& amount,
std::optional<AccountID> const& dest,
std::optional<std::uint32_t> const& expiration,
Expand Down
Loading
Loading