refactor: Move reserve checks from preclaim to doApply where relevant by mvadari · Pull Request #7607 · XRPLF/rippled

mvadari · 2026-06-23T21:17:43Z

High Level Overview of Change

This PR moves reserve checks in PaymentChannelCreate and OracleSet from preclaim to doApply, and incorporates preFeeBalance_ into the calculations instead of sfBalance.

Context of Change

Having reserve checks in preclaim is an anti-pattern - see #6599 for a full explanation

API Impact

N/A

xrplf-ai-reviewer

No issues.

Review by Claude Sonnet 4.6 · Prompt: V15

Copilot

Pull request overview

This PR aims to standardize reserve enforcement for PaymentChannelCreate and OracleSet by shifting reserve checks away from preclaim() (which uses SLE pre-fee balance reads) into doApply() using preFeeBalance_, aligning these transactors with the dominant reserve-check pattern described in issue #6599.

Changes:

Gate the existing preclaim() reserve checks so they only run when featureSponsor is not enabled.
Add doApply()-time reserve checks (using preFeeBalance_) when featureSponsor is enabled for PaymentChannelCreate and OracleSet.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp	Gated preclaim reserve checks and added doApply reserve/funding checks under `featureSponsor`.
src/libxrpl/tx/transactors/oracle/OracleSet.cpp	Gated preclaim reserve checks and added doApply reserve checks under `featureSponsor` for both create/update paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        if (auto const ret = checkInsufficientReserve(
+                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()},
+                *sponsorSle,
+                1,
+                0,
+                j_);
+            !isTesSuccess(ret))
+            return tecUNFUNDED;


codecov · 2026-06-23T22:02:19Z

Codecov Report

❌ Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.1%. Comparing base (494391b) to head (8bb462a).
⚠️ Report is 15 commits behind head on xrplf/sponsor.

Files with missing lines	Patch %	Lines
...ansactors/payment_channel/PaymentChannelCreate.cpp	71.4%	6 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           xrplf/sponsor   #7607     +/-   ##
===============================================
- Coverage           82.2%   82.1%   -0.1%     
===============================================
  Files               1016    1016             
  Lines              78375   78209    -166     
  Branches            8996    9006     +10     
===============================================
- Hits               64438   64243    -195     
- Misses             13928   13957     +29     
  Partials               9       9

Files with missing lines	Coverage Δ
...ansactors/payment_channel/PaymentChannelCreate.cpp	`86.5% <71.4%> (-8.3%)`	⬇️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

xrplf-ai-reviewer

No issues.

Review by Claude Sonnet 4.6 · Prompt: V15

github-actions · 2026-06-25T16:34:21Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

… mvadari/sponsor/preclaim

github-actions · 2026-06-26T03:28:37Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

xrplf-ai-reviewer

Two correctness bugs flagged inline: a compile error (undeclared sponsorSle) in the OracleSet create branch, and a mutation-before-check ordering issue in the update branch. An unsigned underflow risk is also flagged in PaymentChannelCreate.

Review by Claude Sonnet 4.6 · Prompt: V15

xrplf-ai-reviewer · 2026-06-26T03:31:33Z

+                ctx_.view(),
+                ctx_.tx,
+                sle,
+                STAmount{preFeeBalance_ - ctx_.tx[sfAmount].xrp()},


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

Reserve sponsor SLE is never retrieved in PaymentChannelCreate::doApply — both checkInsufficientReserve calls silently ignore any sponsor, which can cause a valid sponsored transaction to fail with tecINSUFFICIENT_RESERVE. See inline.

Review by Claude Sonnet 4.6 · Prompt: V15

xrplf-ai-reviewer · 2026-06-26T04:23:45Z

+    if (ctx_.view().rules().enabled(featureSponsor))
+    {
+        if (auto const ret = checkInsufficientReserve(
+                ctx_.view(), ctx_.tx, sle, STAmount{preFeeBalance_}, {}, 1, 0, j_);


Missing sponsor SLE — both checkInsufficientReserve calls pass {}, silently dropping any reserve sponsor. Mirror OracleSet::doApply: retrieve the sponsor SLE via getTxReserveSponsor and forward it to both calls.

auto const sponsorSle = getTxReserveSponsor(ctx_.view(), ctx_.tx); if (!sponsorSle) return sponsorSle.error(); if (auto const ret = checkInsufficientReserve( 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()}, *sponsorSle, 1, 0, j_); !isTesSuccess(ret)) return tecUNFUNDED;

xrplf-ai-reviewer

Looks good.

Review by Claude Sonnet 4.6 · Prompt: V15

xrplf-ai-reviewer

Clean refactor. No issues.

Review by Claude Sonnet 4.6 · Prompt: V15

Tapanito · 2026-06-26T11:45:36Z

+        auto const sponsorSle = getTxReserveSponsor(ctx_.view(), ctx_.tx);
+        if (!sponsorSle)
+            return sponsorSle.error();
+        if (auto const ret = checkInsufficientReserve(


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.

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

xrplf-ai-reviewer

Took a pass through this

Sponsored-transaction reserve check uses inconsistent sponsor argument — flagged inline.

Review by ReviewBot 🤖

Review by Claude Sonnet 4.6 · Prompt: V15

xrplf-ai-reviewer · 2026-06-26T16:21:06Z

+                ctx_.tx,
+                sle,
+                STAmount{preFeeBalance_ - ctx_.tx[sfAmount].xrp()},
+                {},


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;

PeterChen13579 · 2026-06-29T19:39:52Z

        auto const count = calculateOracleReserve(series.size());
+        if (ctx_.view().rules().enabled(featureSponsor))
+        {
+            if (auto const ret = checkInsufficientReserve(


I thought oracleSet is out of scope for reserve sponsor?

… mvadari/sponsor/preclaim

xrplf-ai-reviewer

Possible sponsor argument mismatch in the second reserve check — see inline.

Review by Claude Sonnet 4.6 · Prompt: V15

xrplf-ai-reviewer · 2026-06-29T19:51:13Z

+                ctx_.tx,
+                sle,
+                STAmount{preFeeBalance_ - ctx_.tx[sfAmount].xrp()},
+                {},


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,

PeterChen13579

LGTM

refactor: Move reserve checks from preclaim to doApply where relevant

db0371a

mvadari requested review from PeterChen13579, Tapanito and Copilot June 23, 2026 21:17

Copilot started reviewing on behalf of mvadari June 23, 2026 21:18 View session

xrplf-ai-reviewer Bot approved these changes Jun 23, 2026

View reviewed changes

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Tapanito reviewed Jun 24, 2026

View reviewed changes

Comment thread src/libxrpl/tx/transactors/oracle/OracleSet.cpp Outdated

remove sponsor from pre-sponsor block

49735d1

xrplf-ai-reviewer Bot approved these changes Jun 24, 2026

View reviewed changes

depthfirst-app Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp

github-actions Bot added the PR: has conflicts label Jun 25, 2026

mvadari added 2 commits June 25, 2026 22:26

fix it better

0c7aa43

Merge branch 'xrplf/sponsor' of https://github.com/XRPLF/rippled into…

f53b810

… mvadari/sponsor/preclaim

github-actions Bot removed the PR: has conflicts label Jun 26, 2026

xrplf-ai-reviewer Bot reviewed Jun 26, 2026

View reviewed changes

depthfirst-app Bot reviewed Jun 26, 2026

View reviewed changes

Comment thread src/libxrpl/tx/transactors/oracle/OracleSet.cpp Outdated

Comment thread src/libxrpl/tx/transactors/oracle/OracleSet.cpp Outdated

fix build

2ed9a69

xrplf-ai-reviewer Bot reviewed Jun 26, 2026

View reviewed changes

fix build

2185a1e

xrplf-ai-reviewer Bot approved these changes Jun 26, 2026

View reviewed changes

fix tests, whoops

3c64c3c

xrplf-ai-reviewer Bot approved these changes Jun 26, 2026

View reviewed changes

Tapanito reviewed Jun 26, 2026

View reviewed changes

fix clang-tidy issues

f4cfe0b

xrplf-ai-reviewer Bot reviewed Jun 26, 2026

View reviewed changes

Tapanito approved these changes Jun 26, 2026

View reviewed changes

PeterChen13579 reviewed Jun 29, 2026

View reviewed changes

mvadari added 3 commits June 29, 2026 15:47

roll back OracleSet changes

a504034

Merge branch 'xrplf/sponsor' of https://github.com/XRPLF/rippled into…

eed58b7

… mvadari/sponsor/preclaim

whoops

8bb462a

mvadari requested a review from PeterChen13579 June 29, 2026 19:49

xrplf-ai-reviewer Bot reviewed Jun 29, 2026

View reviewed changes

PeterChen13579 approved these changes Jun 29, 2026

View reviewed changes

mvadari merged commit 35e1a6e into xrplf/sponsor Jun 29, 2026
39 checks passed

mvadari deleted the mvadari/sponsor/preclaim branch June 29, 2026 20:04

Uh oh!

Conversation

mvadari commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

API Impact

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jun 25, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Tapanito Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvadari Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

PeterChen13579 Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

PeterChen13579 left a comment

Choose a reason for hiding this comment

Uh oh!

mvadari commented Jun 23, 2026 •

edited

Loading

codecov Bot commented Jun 23, 2026 •

edited

Loading

Tapanito Jun 26, 2026 •

edited

Loading