ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-09-27 07:26:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Recycle the escrow reserve in EscrowCancel and EscrowFinish (#8142)

Browse Source
This commit is contained in:
Denis Angell
2026-09-03 16:27:45 +00:00
committed by GitHub
parent 2ddf6ee148
commit 37cb4cdbe3
4 changed files with 119 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/libxrpl/tx/transactors/escrow/EscrowCancel.cpp
View File

@@ -169,6 +169,12 @@ EscrowCancel::doApply()
auto const sle = ctx_.view().peek(keylet::account(account));
STAmount const amount = slep->getFieldAmount(sfAmount);
// The return can re-create a holding the owner deleted while the escrow
// was pending; the removed escrow must not be counted against its reserve.
bool const recycleReserve = ctx_.view().rules().enabled(fixCleanup3_4_0);
if (recycleReserve)
decreaseOwnerCountForObject(ctx_.view(), sle, slep, 1, ctx_.journal);
// Transfer amount back to the owner
if (isXRP(amount))
{
@@ -212,7 +218,8 @@ EscrowCancel::doApply()
}
}
decreaseOwnerCountForObject(ctx_.view(), sle, slep, 1, ctx_.journal);
if (!recycleReserve)
decreaseOwnerCountForObject(ctx_.view(), sle, slep, 1, ctx_.journal);
// Remove escrow from ledger
ctx_.view().erase(slep);

17
src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp
View File

@@ -343,14 +343,12 @@ EscrowFinish::doApply()
}
}
// With the Sponsor amendment, release the escrow reserve before delivery.
// Token delivery can auto-create a destination holding, and the same
// sponsor (or the same account, for a self-escrow) may cover both the
// escrow being removed and the holding being created. Without the
// amendment, keep the legacy order: releasing early changes the reserve
// arithmetic for self-escrows and would break consensus if not gated.
bool const sponsorEnabled = ctx_.view().rules().enabled(featureSponsor);
if (sponsorEnabled)
// Delivery can auto-create the destination's holding; the removed escrow
// must not be counted against its reserve. The two share a reserve payer
// for a self-escrow, or when one sponsor covers both.
bool const recycleReserve =
ctx_.view().rules().enabled(featureSponsor) || ctx_.view().rules().enabled(fixCleanup3_4_0);
if (recycleReserve)
decreaseOwnerCountForObject(ctx_.view(), account, slep, 1, ctx_.journal);
STAmount const amount = slep->getFieldAmount(sfAmount);
@@ -402,8 +400,7 @@ EscrowFinish::doApply()
ctx_.view().update(sled);
// Adjust source owner count (legacy position, pre-Sponsor)
if (!sponsorEnabled)
if (!recycleReserve)
decreaseOwnerCountForObject(ctx_.view(), account, slep, 1, ctx_.journal);
// Remove escrow from ledger

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

@@ -951,6 +951,99 @@ struct EscrowToken_test : public beast::unit_test::Suite
}
}
void
testIOUCancelReserveRecycle(FeatureBitset features)
{
testcase("IOU Cancel Reserve Recycle");
using namespace jtx;
using namespace std::literals;
// Escrowing the whole IOU balance lets the owner delete the now-zero
// trust line, so cancelling has to re-create it: one object destroyed,
// one created, and the reserve requirement unchanged.
Env env{*this, features};
bool const fixEnabled = env.current()->rules().enabled(fixCleanup3_4_0);
auto const baseFee = env.current()->fees().base;
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const gw = Account("gw");
auto const usd = gw["USD"];
env.fund(XRP(10'000), alice, bob, gw);
env.close();
env(fset(gw, asfAllowTrustLineLocking));
env.close();
env.trust(usd(10'000), alice);
env.close();
env(pay(gw, alice, usd(10'000)));
env.close();
BEAST_EXPECT(env.ownerCount(alice) == 1);
auto const cancelAfter = env.now() + 100s;
auto const seq = env.seq(alice);
env(escrow::create(alice, bob, usd(10'000)),
escrow::kFinishTime(env.now() + 1s),
escrow::kCancelTime(cancelAfter),
Fee(baseFee));
env.close();
BEAST_EXPECT(env.ownerCount(alice) == 2);
auto const trustLineKey = keylet::trustLine(alice.id(), gw.id(), usd.currency);
env(trust(alice, usd(0)));
env.close();
BEAST_EXPECT(!env.current()->exists(trustLineKey));
BEAST_EXPECT(env.ownerCount(alice) == 1);
// Leave alice holding the reserve for exactly one owned object. That
// is the escrow now and the re-created trust line after the cancel.
auto const oneObject = env.current()->fees().accountReserve(1, 1);
auto const twoObjects = env.current()->fees().accountReserve(2, 1);
auto const balance = env.balance(alice).value().xrp();
auto const feeCushion = baseFee.drops() * 20;
env(pay(alice, bob, drops(balance.drops() - oneObject.drops() - feeCushion)));
env.close();
BEAST_EXPECT(env.balance(alice).value().xrp() >= oneObject);
BEAST_EXPECT(env.balance(alice).value().xrp() < twoObjects);
for (; env.now() < cancelAfter; env.close())
{
}
env.close();
env.close();
auto const expectedResult = fixEnabled ? Ter(tesSUCCESS) : Ter(tecNO_LINE_INSUF_RESERVE);
env(escrow::cancel(alice, alice, seq), Fee(baseFee), expectedResult);
env.close();
auto const escrowKey = keylet::escrow(alice.id(), SeqProxy::rawSequence(seq));
if (fixEnabled)
{
BEAST_EXPECT(!env.le(escrowKey));
BEAST_EXPECT(env.current()->exists(trustLineKey));
BEAST_EXPECT(env.balance(alice, usd) == usd(10'000));
BEAST_EXPECT(env.ownerCount(alice) == 1);
}
else
{
// The tec keeps the escrow, so one more owner reserve lets the
// retry through.
BEAST_EXPECT(env.le(escrowKey) != nullptr);
BEAST_EXPECT(!env.current()->exists(trustLineKey));
BEAST_EXPECT(env.ownerCount(alice) == 1);
env(pay(bob, alice, drops(twoObjects.drops() - oneObject.drops())));
env.close();
env(escrow::cancel(alice, alice, seq), Fee(baseFee), Ter(tesSUCCESS));
env.close();
BEAST_EXPECT(!env.le(escrowKey));
BEAST_EXPECT(env.balance(alice, usd) == usd(10'000));
}
}
void
testIOUBalances(FeatureBitset features)
{
@@ -4250,6 +4343,8 @@ public:
}
testMPTSplitEscrowTransferFee(all - fixCleanup3_4_0);
testMPTSplitEscrowTransferFee(all);
testIOUCancelReserveRecycle(all - fixCleanup3_4_0);
testIOUCancelReserveRecycle(all);
}
};

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

@@ -5467,13 +5467,12 @@ public:
using namespace test::jtx;
using namespace std::chrono_literals;
// Finishing a self-escrow (source == destination) whose trust line
// was deleted while the escrow was outstanding auto-creates the line,
// and the outcome of that reserve check depends on whether the escrow
// reserve is released before delivery (Sponsor) or after (legacy).
// With the source's balance in the one-increment window
// [reserve(1), reserve(2)), the legacy order requires reserve(2) and
// fails, while the Sponsor order requires reserve(1) and succeeds.
// Finishing a self-escrow (source == destination) whose trust line was
// deleted while the escrow was outstanding auto-creates the line. With
// the source's balance in the one-increment window
// [reserve(1), reserve(2)), the finish succeeds only when the escrow
// reserve is released before delivery, which either featureSponsor or
// fixCleanup3_4_0 does.
auto runTest = [&](FeatureBitset features, TER expected) {
Account const alice("alice");
Account const gw("gw");
@@ -5538,11 +5537,9 @@ public:
}
};
// Pre-amendment: legacy order — the escrow still counts against the
// reserve while the auto-created line is checked.
runTest(testableAmendments() - featureSponsor, tecNO_LINE_INSUF_RESERVE);
// Post-amendment: the escrow reserve is recycled into the new line.
runTest(testableAmendments() - featureSponsor - fixCleanup3_4_0, tecNO_LINE_INSUF_RESERVE);
runTest(testableAmendments() - featureSponsor, tesSUCCESS);
runTest(testableAmendments() - fixCleanup3_4_0, tesSUCCESS);
runTest(testableAmendments(), tesSUCCESS);
}