Skip to content

refactor: Move reserve checks from preclaim to doApply where relevant #7607

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

Merged
mvadari merged 11 commits into from
Jun 29, 2026
Merged

refactor: Move reserve checks from preclaim to doApply where relevant #7607

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
db0371a
refactor: Move reserve checks from preclaim to doApply where relevant
mvadari Jun 23, 2026
49735d1
remove sponsor from pre-sponsor block
mvadari Jun 24, 2026
0c7aa43
fix it better
mvadari Jun 26, 2026
f53b810
Merge branch 'xrplf/sponsor' of https://github.com/XRPLF/rippled into…
mvadari Jun 26, 2026
2ed9a69
fix build
mvadari Jun 26, 2026
2185a1e
fix build
mvadari Jun 26, 2026
3c64c3c
fix tests, whoops
mvadari Jun 26, 2026
f4cfe0b
fix clang-tidy issues
mvadari Jun 26, 2026
a504034
roll back OracleSet changes
mvadari Jun 29, 2026
eed58b7
Merge branch 'xrplf/sponsor' of https://github.com/XRPLF/rippled into…
mvadari Jun 29, 2026
8bb462a
whoops
mvadari Jun 29, 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
39 changes: 29 additions & 10 deletions src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,16 @@ PaymentChannelCreate::preclaim(PreclaimContext const& ctx)
return terNO_ACCOUNT;

// Check reserve and funds availability
if (!ctx.view.rules().enabled(featureSponsor))
{
auto const balance = (*sle)[sfBalance];
Comment thread
mvadari marked this conversation as resolved.
auto const sponsorSle = getTxReserveSponsor(ctx.view, ctx.tx);
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE
if (auto const ret =
checkInsufficientReserve(ctx.view, ctx.tx, sle, balance, *sponsorSle, 1, 0, ctx.j);
!isTesSuccess(ret))
return ret;
auto const fees = ctx.view.fees();
auto const reserve = fees.reserve + fees.increment * ((*sle)[sfOwnerCount] + 1);

if (auto const ret = checkInsufficientReserve(
ctx.view, ctx.tx, sle, balance - ctx.tx[sfAmount], *sponsorSle, 1, 0, ctx.j);
!isTesSuccess(ret))
if (balance < reserve)
return tecINSUFFICIENT_RESERVE;

if (balance < reserve + ctx.tx[sfAmount])
return tecUNFUNDED;
}

Expand Down Expand Up @@ -137,6 +134,28 @@ PaymentChannelCreate::doApply()
return tecEXPIRED;
}

if (ctx_.view().rules().enabled(featureSponsor))
{
auto const sponsorSle = getTxReserveSponsor(ctx_.view(), ctx_.tx);
if (!sponsorSle)
return sponsorSle.error();
if (auto const ret = checkInsufficientReserve(
Comment thread
depthfirst-app[bot] marked this conversation as resolved.

@Tapanito Tapanito Jun 26, 2026 •
edited
Loading

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 mehod name checkInsufficientReserve is a little bit confusing. The reserve can either be sufficient or insufficient, and what the method is doing is determining which one it is. What do you think about calling it checkAccountReserve? Doesn't have to be part of this PR though.

@mvadari mvadari Jun 26, 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.

I would consider that out of scope of this PR but I'll take note of it

ctx_.view(), ctx_.tx, sle, STAmount{preFeeBalance_}, *sponsorSle, 1, 0, j_);
!isTesSuccess(ret))
return ret;
if (auto const ret = checkInsufficientReserve(
ctx_.view(),
ctx_.tx,
sle,
STAmount{preFeeBalance_ - ctx_.tx[sfAmount].xrp()},

@xrplf-ai-reviewer xrplf-ai-reviewer Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unsigned underflow: subtraction can wrap if sfAmount > preFeeBalance_. Guard before subtracting:

        if (ctx_.tx[sfAmount].xrp() > preFeeBalance_)
            return tecUNFUNDED;
        if (auto const ret = checkInsufficientReserve(
                ctx_.view(),
                ctx_.tx,
                sle,
                STAmount{preFeeBalance_ - ctx_.tx[sfAmount].xrp()},
                {},
                1,
                0,
                j_);

{},

@xrplf-ai-reviewer xrplf-ai-reviewer Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Second checkInsufficientReserve passes {} (no sponsor) but the first passes *sponsorSle — inconsistent reserve baseline for sponsored transactions may produce wrong tecUNFUNDED results.

        if (auto const ret = checkInsufficientReserve(
                ctx_.view(),
                ctx_.tx,
                sle,
                STAmount{preFeeBalance_ - ctx_.tx[sfAmount].xrp()},
                *sponsorSle,
                1,
                0,
                j_);
            !isTesSuccess(ret))
            return tecUNFUNDED;

@xrplf-ai-reviewer xrplf-ai-reviewer Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Second checkInsufficientReserve passes {} for sponsor, but the first call (line 143) uses *sponsorSle. If intentional, add a comment explaining why the sponsor is excluded from the funding check; if accidental, fix to match the original semantics:

                *sponsorSle,

1,
0,
j_);
!isTesSuccess(ret))
return tecUNFUNDED;
}

auto const dst = ctx_.tx[sfDestination];

// Create PayChan in ledger.
Expand Down
Loading