fix: Better handling of txs that send XRP + charge reserve (#7671)

This commit is contained in:
Mayukha Vadari
2026-07-01 11:37:45 -04:00
committed by GitHub
parent 4e6f8f334f
commit 730e526080
3 changed files with 133 additions and 11 deletions

View File

@@ -33,6 +33,7 @@
#include <xrpl/tx/Transactor.h>
#include <xrpl/tx/applySteps.h>
#include <cstdint>
#include <memory>
#include <system_error>
#include <variant>
@@ -439,16 +440,37 @@ EscrowCreate::doApply()
auto const sponsorSle = getTxReserveSponsor(view(), ctx_.tx);
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE
// First check: whoever is on the hook for the new owner increment
// can cover it. When sponsored this hits the sponsor branch and
// validates the sponsor's reserve + remaining credit. When
// unsponsored this hits the source branch and validates the
// source's pre-lock balance against base + (currentOC+1)*increment.
if (auto const ret =
checkInsufficientReserve(ctx_.view(), ctx_.tx, sle, balance, *sponsorSle, 1, 0, j_);
!isTesSuccess(ret))
return ret;
// Check reserve and funds availability
if (isXRP(amount))
{
// Second check (XRP escrow only): after locking the escrowed
// amount, the source must still meet its own reserve floor.
// Always passes `{}` so the source branch runs (the sponsor's
// reserve was already validated above; here we're verifying the
// source can fund the lock without dipping below its own
// reserve). ownerCountAdj differs by case:
// - sponsored: adj=0 — sponsor covers the new owner increment,
// so the source only owes its base reserve.
// - unsponsored: adj=1 — source owes base + the new increment.
std::int32_t const ownerCountAdj = *sponsorSle ? 0 : 1;
if (auto const ret = checkInsufficientReserve(
ctx_.view(), ctx_.tx, sle, balance - STAmount(amount).xrp(), {}, 1, 0, j_);
ctx_.view(),
ctx_.tx,
sle,
balance - STAmount(amount).xrp(),
{},
ownerCountAdj,
0,
j_);
!isTesSuccess(ret))
return tecUNFUNDED;
}

View File

@@ -22,6 +22,7 @@
#include <xrpl/tx/Transactor.h>
#include <xrpl/tx/applySteps.h>
#include <cstdint>
#include <memory>
namespace xrpl {
@@ -134,22 +135,38 @@ PaymentChannelCreate::doApply()
return tecEXPIRED;
}
auto const sponsorSle = getTxReserveSponsor(view(), ctx_.tx);
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE
if (ctx_.view().rules().enabled(featureSponsor))
{
auto const sponsorSle = getTxReserveSponsor(ctx_.view(), ctx_.tx);
if (!sponsorSle)
return sponsorSle.error();
// First check: whoever is on the hook for the new owner increment
// can cover it. When sponsored this hits the sponsor branch and
// validates the sponsor's reserve + remaining credit. When
// unsponsored this hits the source branch and validates the
// source's pre-lock balance against base + (currentOC+1)*increment.
if (auto const ret = checkInsufficientReserve(
ctx_.view(), ctx_.tx, sle, STAmount{preFeeBalance_}, *sponsorSle, 1, 0, j_);
ctx_.view(), ctx_.tx, sle, preFeeBalance_, *sponsorSle, 1, 0, j_);
!isTesSuccess(ret))
return ret;
// Second check: after locking sfAmount in the channel, the source
// must still meet its own reserve floor. Always passes `{}` so the
// source branch runs (the sponsor's reserve was already validated
// above; here we're verifying the source can fund the lock without
// dipping below its own reserve). ownerCountAdj differs by case:
// - sponsored: adj=0 — sponsor covers the new owner increment,
// so the source only owes its base reserve.
// - unsponsored: adj=1 — source owes base + the new increment.
std::int32_t const ownerCountAdj = *sponsorSle ? 0 : 1;
if (auto const ret = checkInsufficientReserve(
ctx_.view(),
ctx_.tx,
sle,
STAmount{preFeeBalance_ - ctx_.tx[sfAmount].xrp()},
preFeeBalance_ - ctx_.tx[sfAmount].xrp(),
{},
1,
ownerCountAdj,
0,
j_);
!isTesSuccess(ret))
@@ -203,9 +220,6 @@ PaymentChannelCreate::doApply()
// Deduct owner's balance, increment owner count
(*sle)[sfBalance] = (*sle)[sfBalance] - ctx_.tx[sfAmount];
auto const sponsorSle = getTxReserveSponsor(view(), ctx_.tx);
if (!sponsorSle)
return sponsorSle.error(); // LCOV_EXCL_LINE
adjustOwnerCount(ctx_.view(), sle, *sponsorSle, 1, ctx_.journal);
addSponsorToLedgerEntry(slep, *sponsorSle);
ctx_.view().update(sle);

View File

@@ -2985,6 +2985,50 @@ public:
BEAST_EXPECT(sponsoredOwnerCount(env, bob) == 1);
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor) == 1);
}
// A sponsored EscrowCreate must still verify that the source
// can fund the escrow amount and stay above its own base
// reserve. The sponsor covers the new object's owner
// increment, but cannot cover the source's base reserve.
{
Env env{*this, testableAmendments()};
env.fund(XRP(10000), alice, bob, sponsor);
env.close();
// alice's balance is just above the base reserve. After
// locking escrowAmount she would dip below it.
adjustAccountXRPBalance(env, alice, accountReserve(env, 1) + XRP(1));
auto const escrowAmount = XRP(2);
auto const seq = env.seq(alice);
if (cosigning)
{
env(escrow::create(alice, bob, escrowAmount),
escrow::kCondition(escrow::kCb1),
escrow::kCancelTime(env.now() + 100s),
sponsor::As(sponsor, spfSponsorReserve),
Sig(sfSponsorSignature, sponsor),
Ter(tecUNFUNDED));
}
else
{
env(sponsor::set(sponsor, 0, 1, XRP(1)), sponsor::SponseeAcc(alice));
env.close();
env(escrow::create(alice, bob, escrowAmount),
escrow::kCondition(escrow::kCb1),
escrow::kCancelTime(env.now() + 100s),
sponsor::As(sponsor, spfSponsorReserve),
Ter(tecUNFUNDED));
}
env.close();
BEAST_EXPECT(!env.le(keylet::escrow(alice, seq)));
BEAST_EXPECT(ownerCount(env, alice) == 0);
BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 0);
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor) == 0);
}
}
void
@@ -3251,6 +3295,48 @@ public:
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor) == 0);
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor2) == 0);
}
// A sponsored PaymentChannelCreate must still verify that the
// source can fund the channel amount and stay above its own
// base reserve. The sponsor covers the new object's owner
// increment, but cannot cover the source's base reserve.
{
Env env{*this, testableAmendments()};
env.fund(XRP(10000), alice, bob, sponsor);
env.close();
// alice's balance is just above the base reserve. After
// locking channelAmount she would dip below it.
adjustAccountXRPBalance(env, alice, accountReserve(env, 1) + XRP(1));
auto const pk = alice.pk();
auto const settleDelay = 10s;
auto const channelAmount = XRP(2);
auto const chan = paychan::channel(alice, bob, env.seq(alice));
if (cosigning)
{
env(paychan::create(alice, bob, channelAmount, settleDelay, pk),
sponsor::As(sponsor, spfSponsorReserve),
Sig(sfSponsorSignature, sponsor),
Ter(tecUNFUNDED));
}
else
{
env(sponsor::set(sponsor, 0, 1, XRP(1)), sponsor::SponseeAcc(alice));
env.close();
env(paychan::create(alice, bob, channelAmount, settleDelay, pk),
sponsor::As(sponsor, spfSponsorReserve),
Ter(tecUNFUNDED));
}
env.close();
BEAST_EXPECT(!paychan::channelExists(*env.current(), chan));
BEAST_EXPECT(ownerCount(env, alice) == 0);
BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 0);
BEAST_EXPECT(sponsoringOwnerCount(env, sponsor) == 0);
}
}
void