ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-10-06 11:48:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/tapanito/empty-holding' into ripple/lending-protocol-fv

Browse Source
This commit is contained in:
Vito
2026-09-02 11:07:29 +02:00
parent 6db6e7379a 6428435882
commit 5b832250c8
12 changed files with 937 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
include/xrpl/ledger/helpers/RippleStateHelpers.h
View File

@@ -239,8 +239,14 @@ canTransfer(ReadView const& view, Issue const& issue, AccountID const& from, Acc
//------------------------------------------------------------------------------
/**
* Any transactors that call addEmptyHolding() in doApply must call
* canAddHolding() in preflight with the same View and Asset
* After fixCleanup3_4_0, if the destination already holds this IOU, returns
* tecDUPLICATE and does not consult issuer freeze or DefaultRipple. Freeze
* and DefaultRipple still apply on the create path (DefaultRipple off is
* terNO_RIPPLE). Transactors that may create a new holding in doApply must
* call canAddHolding() in preclaim only when that destination does not
* already hold the asset. Do not call canAddHolding() merely because
* addEmptyHolding() is invoked: that helper does not check whether a
* holding already exists.
*/
[[nodiscard]] TER
addEmptyHolding(

6
include/xrpl/ledger/helpers/TokenHelpers.h
View File

@@ -319,6 +319,12 @@ transferRate(ReadView const& view, STAmount const& amount);
[[nodiscard]] TER
canAddHolding(ReadView const& view, Asset const& asset);
/**
* True if the account already holds this asset (or is the issuer / XRP).
*/
[[nodiscard]] bool
holdingExists(ReadView const& view, AccountID const& account, Asset const& asset);
[[nodiscard]] TER
addEmptyHolding(
ApplyViewContext ctx,

14
merged-prs.md
View File

@@ -2,9 +2,11 @@
PRs merged into the `ripple/lending-protocol-fv` branch.
| PR | Title | Author | Branch | Merged |
| --------------------------------------------------- | ----------------------------------------------- | ---------- | --------------------------------------------------------- | ---------- |
| [#6383](https://github.com/XRPLF/rippled/pull/6383) | feat: Add tfVaultDonate feature | @Tapanito | `tapanito/vault-donation` | 2026-09-02 |
| [#7820](https://github.com/XRPLF/rippled/pull/7820) | feat: Split LoanSet and LoanAccept | @a1q123456 | `a1q123456/split-loan-set-and-loan-accept-implementation` | 2026-09-02 |
| [#6528](https://github.com/XRPLF/rippled/pull/6528) | feat: Make VaultID conditional on LoanBrokerSet | @Tapanito | `tapanito/loan-broker-set` | 2026-09-02 |
| [#6361](https://github.com/XRPLF/rippled/pull/6361) | Adds functionality to block vault deposits | @Tapanito | `tapanito/vault-block-deposit` | 2026-09-02 |
| PR | Title | Author | Branch | Merged |
| --------------------------------------------------- | -------------------------------------------------------------------------- | ---------- | --------------------------------------------------------- | ---------- |
| [#6383](https://github.com/XRPLF/rippled/pull/6383) | feat: Add tfVaultDonate feature | @Tapanito | `tapanito/vault-donation` | 2026-09-02 |
| [#7820](https://github.com/XRPLF/rippled/pull/7820) | feat: Split LoanSet and LoanAccept | @a1q123456 | `a1q123456/split-loan-set-and-loan-accept-implementation` | 2026-09-02 |
| [#6528](https://github.com/XRPLF/rippled/pull/6528) | feat: Make VaultID conditional on LoanBrokerSet | @Tapanito | `tapanito/loan-broker-set` | 2026-09-02 |
| [#6361](https://github.com/XRPLF/rippled/pull/6361) | Adds functionality to block vault deposits | @Tapanito | `tapanito/vault-block-deposit` | 2026-09-02 |
| [#8153](https://github.com/XRPLF/rippled/pull/8153) | fix: Allow zero-value MPT vault withdraw when the asset holding is missing | @Tapanito | `tapanito/vault-zero-delta` | 2026-09-02 |
| [#8154](https://github.com/XRPLF/rippled/pull/8154) | fix: Treat an existing IOU line as a no-op in addEmptyHolding | @Tapanito | `tapanito/empty-holding` | 2026-09-02 |

2
src/libxrpl/ledger/helpers/MPTokenHelpers.cpp
View File

@@ -184,6 +184,8 @@ addEmptyHolding(
auto const mpt = ctx.view.peek(keylet::mptokenIssuance(mptID));
if (!mpt)
return tefINTERNAL; // LCOV_EXCL_LINE
// Unlike IOU addEmptyHolding (post-fixCleanup3_4_0), a locked issuance is
// still rejected before the "MPToken already exists" short circuit.
if (mpt->isFlag(lsfMPTLocked))
return tefINTERNAL; // LCOV_EXCL_LINE
if (ctx.view.peek(keylet::mptoken(mptID, accountID)))

21
src/libxrpl/ledger/helpers/RippleStateHelpers.cpp
View File

@@ -652,21 +652,32 @@ addEmptyHolding(
auto const& issuerId = issue.getIssuer();
auto const& currency = issue.currency;
if (isGlobalFrozen(ctx.view, issuerId))
return tecFROZEN; // LCOV_EXCL_LINE
auto const& srcId = issuerId;
auto const& dstId = accountID;
auto const high = srcId > dstId;
auto const index = keylet::trustLine(srcId, dstId, currency);
// Post-fixCleanup3_4_0: an existing line is a no-op. Issuer freeze and
// DefaultRipple only matter when this function has to create a line.
bool const fix340Enabled = ctx.view.rules().enabled(fixCleanup3_4_0);
if (fix340Enabled && ctx.view.exists(index))
return tecDUPLICATE;
if (isGlobalFrozen(ctx.view, issuerId))
return tecFROZEN; // LCOV_EXCL_LINE
auto const sleSrc = ctx.view.peek(keylet::account(srcId));
auto const sleDst = ctx.view.peek(keylet::account(dstId));
if (!sleDst || !sleSrc)
return tefINTERNAL; // LCOV_EXCL_LINE
// Create path: DefaultRipple is still required. terNO_RIPPLE is
// intentional so VaultWithdraw / CoverWithdraw fail in preclaim via
// canAddHolding (retryable, no fee) rather than claiming a tec* fee
// in doApply. Transactor::operator() will not apply and will not
// convert it to tefINTERNAL.
if (!sleSrc->isFlag(lsfDefaultRipple))
return tecINTERNAL; // LCOV_EXCL_LINE
return fix340Enabled ? TER{terNO_RIPPLE} : tecINTERNAL;
// If the line already exists, don't create it again.
if (ctx.view.read(index))
if (!fix340Enabled && ctx.view.exists(index))
return tecDUPLICATE;
// A reserve sponsor only covers tx.Account's own objects.

26
src/libxrpl/ledger/helpers/TokenHelpers.cpp
View File

@@ -583,6 +583,32 @@ canAddHolding(ReadView const& view, Asset const& asset)
asset.value());
}
[[nodiscard]] bool
holdingExists(ReadView const& view, AccountID const& account, Issue const& issue)
{
if (issue.native() || account == issue.getIssuer())
return true;
return view.exists(keylet::trustLine(account, issue));
}
[[nodiscard]] bool
holdingExists(ReadView const& view, AccountID const& account, MPTIssue const& mptIssue)
{
if (account == mptIssue.getIssuer())
return true;
return view.exists(keylet::mptoken(mptIssue.getMptID(), account));
}
[[nodiscard]] bool
holdingExists(ReadView const& view, AccountID const& account, Asset const& asset)
{
return std::visit(
[&]<ValidIssueType TIss>(TIss const& issue) -> bool {
return holdingExists(view, account, issue);
},
asset.value());
}
TER
addEmptyHolding(
ApplyViewContext ctx,

7
src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp
View File

@@ -65,6 +65,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx)
{
auto const fix320Enabled = ctx.view.rules().enabled(fixCleanup3_2_0);
auto const fix330Enabled = ctx.view.rules().enabled(fixCleanup3_3_0);
auto const fix340Enabled = ctx.view.rules().enabled(fixCleanup3_4_0);
auto const& tx = ctx.tx;
auto const account = tx[sfAccount];
@@ -140,6 +141,12 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx)
if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct, authType))
return ter;
if (fix340Enabled && account == dstAcct && !holdingExists(ctx.view, dstAcct, vaultAsset))
{
if (auto const ter = canAddHolding(ctx.view, vaultAsset); !isTesSuccess(ter))
return ter;
}
if (fix330Enabled)
{
if (auto const ret =

13
src/libxrpl/tx/transactors/lending/LoanSet.cpp
View File

@@ -965,6 +965,19 @@ LoanSet::preclaim(PreclaimContext const& ctx)
ctx.view, asset, vaultPseudo, brokerPseudo, borrower, brokerOwner, ctx.j))
return ter;
// canAddHolding does not look at existing lines. After
// fixCleanup3_4_0, addEmptyHolding is a no-op when the destination
// already holds the asset, so skip this gate unless a holding would
// actually be created (borrower always; broker owner if there is an
// origination fee).
auto const originationFee = tx[~sfLoanOriginationFee].value_or(Number{});
if (!ctx.view.rules().enabled(fixCleanup3_4_0) || !holdingExists(ctx.view, borrower, asset) ||
(originationFee != beast::kZero && !holdingExists(ctx.view, brokerOwner, asset)))
{
if (auto const ter = canAddHolding(ctx.view, asset))
return ter;
}
if (twoStepFlow)
{
// Reject a pending loan up front if the borrower or broker owner (the

9
src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp
View File

@@ -205,6 +205,15 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct, authType); !isTesSuccess(ter))
return ter;
// Fail early when self-destination would have to create a holding.
// Skip when a holding already exists: canAddHolding does not look at that,
// and would block a no-op create (the DefaultRipple-cleared self-withdraw).
if (fix340Enabled && account == dstAcct && !holdingExists(ctx.view, dstAcct, vaultAsset))
{
if (auto const ter = canAddHolding(ctx.view, vaultAsset); !isTesSuccess(ter))
return ter;
}
// The checks above only establish that an account may hold the asset. A
// private vault additionally restricts who may take part in it, so paying
// its asset out to a third party requires both ends of that payout to be

65
src/test/app/lending/LoanPay_test.cpp
View File

@@ -37,6 +37,7 @@
#include <cstdint>
#include <limits>
#include <memory>
#include <string>
#include <type_traits>
namespace xrpl::test {
@@ -1419,6 +1420,69 @@ private:
BEAST_EXPECT(stateAfter.nextPaymentDate == exactDueDate);
}
// LoanPay does not call canAddHolding. addEmptyHolding recreates the
// broker-owner holding when the borrower is also the broker owner. After
// fixCleanup3_4_0 an existing line is a no-op even if DefaultRipple is
// off; pre-fix that path dies with tecINTERNAL.
void
testLoanPaySelfBrokerExistingLineDefaultRipple()
{
using namespace jtx;
using namespace loan;
auto run = [this](FeatureBitset features, TER expected) {
testcase(
std::string(
"LoanPay broker-owner borrower existing line after "
"issuer clears asfDefaultRipple (") +
(features[fixCleanup3_4_0] ? "post" : "pre") + "-fixCleanup3_4_0)");
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
env.fund(XRP(10'000), issuer, alice);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
env(trust(alice, usd(100'000)));
env.close();
env(pay(issuer, alice, usd(50'000)));
env.close();
auto const broker = createVaultAndBroker(env, usd, alice);
auto const brokerSle = env.le(keylet::loanBroker(broker.brokerID));
if (!BEAST_EXPECT(brokerSle))
return;
auto const loanKeylet =
keylet::loan(broker.brokerID, SeqProxy::rawSequence(brokerSle->at(sfLoanSequence)));
Number const serviceFee = usd(2).value();
env(set(alice, broker.brokerID, usd(1'000).value()),
Sig(sfCounterpartySignature, alice),
kLoanServiceFee(serviceFee),
Fee(env.current()->fees().base * 2));
env.close();
env(fclear(issuer, asfDefaultRipple));
env.close();
BEAST_EXPECT(env.le(keylet::trustLine(alice.id(), usd.raw().get<Issue>())));
auto const state = getCurrentState(env, broker, loanKeylet);
STAmount const payment{
usd,
roundPeriodicPayment(usd, state.periodicPayment + serviceFee, state.loanScale)};
env(pay(alice, loanKeylet.key, payment), Ter(expected));
env.close();
};
run(all_ - fixCleanup3_4_0, tecINTERNAL);
run(all_, tesSUCCESS);
}
void
runAmendmentIndependent()
{
@@ -1429,6 +1493,7 @@ private:
testLoanPayCatchUpFeeAtExactDueDatePostAmendment();
testLoanPayCatchUpFeeAtExactDueDatePreAmendment();
testRepayIntoUnauthorizedVault();
testLoanPaySelfBrokerExistingLineDefaultRipple();
}
// Tests run under each entry in amendmentCombinations().

61
src/test/app/lending/LoanSet_test.cpp
View File

@@ -33,6 +33,7 @@
#include <array>
#include <cstdint>
#include <functional>
#include <string>
#include <utility>
#include <vector>
@@ -1101,6 +1102,65 @@ private:
});
}
// LoanSet used to call canAddHolding unconditionally, so an existing
// borrower line still failed with terNO_RIPPLE after the issuer cleared
// DefaultRipple. After fixCleanup3_4_0, skip that gate when the holding
// already exists.
void
testLoanSetExistingLineAfterIssuerClearsDefaultRipple()
{
using namespace jtx;
using namespace loan;
auto run = [this](FeatureBitset features, TER expected) {
testcase(
std::string(
"LoanSet existing borrower line after issuer "
"clears asfDefaultRipple (") +
(features[fixCleanup3_4_0] ? "post" : "pre") + "-fixCleanup3_4_0)");
Env env(*this, features);
Account const issuer{"issuer"};
Account const lender{"lender"};
Account const borrower{"borrower"};
env.fund(XRP(10'000), issuer, lender, borrower);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
env(trust(lender, usd(100'000)));
env(trust(borrower, usd(100'000)));
env.close();
env(pay(issuer, lender, usd(50'000)));
env(pay(issuer, borrower, usd(1'000)));
env.close();
BEAST_EXPECT(env.le(keylet::trustLine(borrower.id(), usd.raw().get<Issue>())));
auto const broker = createVaultAndBroker(env, usd, lender);
env(fclear(issuer, asfDefaultRipple));
env.close();
Number const destBefore = env.balance(borrower, usd.raw()).number();
env(set(borrower, broker.brokerID, usd(100).value()),
Sig(sfCounterpartySignature, lender),
Fee(env.current()->fees().base * 2),
Ter(expected));
env.close();
Number const destAfter = env.balance(borrower, usd.raw()).number();
if (isTesSuccess(expected))
BEAST_EXPECT(destAfter == destBefore + Number{100});
else
BEAST_EXPECT(destAfter == destBefore);
};
run(all_ - fixCleanup3_4_0, terNO_RIPPLE);
run(all_, tesSUCCESS);
}
public:
void
run() override
@@ -1111,6 +1171,7 @@ public:
testTwoStepLoanSet();
testLoanSetClosedEnded();
testLoanSetExistingLineAfterIssuerClearsDefaultRipple();
}
};

744
src/test/app/vault/VaultBugs_test.cpp
View File

@@ -9,6 +9,7 @@
#include <test/jtx/flags.h>
#include <test/jtx/mpt.h>
#include <test/jtx/pay.h>
#include <test/jtx/permissioned_domains.h>
#include <test/jtx/sig.h>
#include <test/jtx/sponsor.h>
#include <test/jtx/ter.h>
@@ -16,6 +17,7 @@
#include <test/jtx/vault.h>
#include <xrpl/basics/Number.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/basics/chrono.h>
#include <xrpl/beast/unit_test/suite.h>
#include <xrpl/json/json_forwards.h>
@@ -2133,6 +2135,357 @@ private:
runScenario(all_ - fixCleanup3_4_0, tecINVARIANT_FAILED);
}
// addEmptyHolding() used to check isGlobalFrozen(issuer) and
// !lsfDefaultRipple before the "line already exists" tecDUPLICATE
// short circuit. doWithdraw() calls addEmptyHolding() for a
// self-destination payout and only tolerates tecDUPLICATE, so
// tecINTERNAL from a missing DefaultRipple flag aborted the
// withdrawal. fixCleanup3_4_0 checks existence first and maps the
// create-path DefaultRipple miss to terNO_RIPPLE. Global freeze on an
// existing line is still rejected later by checkWithdrawFreeze.
void
testBugSelfWithdrawAfterIssuerClearsDefaultRipple()
{
using namespace test::jtx;
auto runExistingLine = [this](
FeatureBitset features,
TER selfExpected,
bool issuerGlobalFreeze = false) {
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(10'000), issuer, alice, bob);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
Issue const usdIssue = usd.raw().get<Issue>();
env(trust(alice, usd(10'000)));
env(trust(bob, usd(10'000)));
env.close();
env(pay(issuer, alice, usd(1'000)));
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = alice, .asset = usd});
env(vaultTx);
env.close();
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = usd(500)}));
env.close();
env(vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = usd(50)}));
env.close();
env(fclear(issuer, asfDefaultRipple));
env.close();
if (issuerGlobalFreeze)
{
env(fset(issuer, asfGlobalFreeze));
env.close();
}
BEAST_EXPECT(env.le(keylet::trustLine(alice.id(), usdIssue)));
// Alice's USD line is unchanged; a later deposit still succeeds
// unless the issuer is globally frozen.
if (!issuerGlobalFreeze)
{
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = usd(10)}));
env.close();
}
Number const destBefore = env.balance(alice, usd.raw()).number();
Number const vaultBefore = env.le(vaultKeylet)->at(sfAssetsTotal);
Number const withdrawAmt{50};
env(vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = usd(50)}),
Ter(selfExpected));
env.close();
Number const destAfter = env.balance(alice, usd.raw()).number();
Number const vaultAfter = env.le(vaultKeylet)->at(sfAssetsTotal);
if (isTesSuccess(selfExpected))
{
BEAST_EXPECT(destAfter == destBefore + withdrawAmt);
BEAST_EXPECT(vaultAfter == vaultBefore - withdrawAmt);
}
else
{
BEAST_EXPECT(destAfter == destBefore);
BEAST_EXPECT(vaultAfter == vaultBefore);
}
if (!issuerGlobalFreeze)
{
auto destTx =
vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = usd(50)});
destTx[sfDestination] = bob.human();
env(destTx);
env.close();
}
};
auto runDeletedLine = [this](FeatureBitset features, TER selfExpected) {
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
env.fund(XRP(10'000), issuer, alice);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
Issue const usdIssue = usd.raw().get<Issue>();
env(trust(alice, usd(10'000)));
env.close();
env(pay(issuer, alice, usd(500)));
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = alice, .asset = usd});
env(vaultTx);
env.close();
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = usd(500)}));
env.close();
env(trust(alice, usd(0)));
env.close();
BEAST_EXPECT(!env.le(keylet::trustLine(alice.id(), usdIssue)));
env(fclear(issuer, asfDefaultRipple));
env.close();
env(vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = usd(50)}),
Ter(selfExpected));
env.close();
};
auto runCoverWithdraw = [this](FeatureBitset features, TER selfExpected) {
using namespace loan_broker;
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
env.fund(XRP(10'000), issuer, alice);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
Issue const usdIssue = usd.raw().get<Issue>();
env(trust(alice, usd(10'000)));
env.close();
env(pay(issuer, alice, usd(1'000)));
env.close();
Vault const vault{env};
auto const [createTx, vaultKeylet, subscriptionDate] = vault.createClosedEnded(
{.owner = alice, .asset = usd, .subscriptionOffset = std::chrono::seconds{60}});
(void)subscriptionDate;
env(createTx);
env.close();
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = usd(500)}));
env.close();
auto const brokerKeylet =
keylet::loanBroker(alice.id(), SeqProxy::rawSequence(env.seq(alice)));
env(set(alice, vaultKeylet.key));
env.close();
env(coverDeposit(alice, brokerKeylet.key, usd(100).value()));
env.close();
env(fclear(issuer, asfDefaultRipple));
env.close();
BEAST_EXPECT(env.le(keylet::trustLine(alice.id(), usdIssue)));
Number const destBefore = env.balance(alice, usd.raw()).number();
Number const coverBefore = env.le(brokerKeylet)->at(sfCoverAvailable);
Number const withdrawAmt{50};
env(coverWithdraw(alice, brokerKeylet.key, usd(50).value()), Ter(selfExpected));
env.close();
Number const destAfter = env.balance(alice, usd.raw()).number();
Number const coverAfter = env.le(brokerKeylet)->at(sfCoverAvailable);
if (isTesSuccess(selfExpected))
{
BEAST_EXPECT(destAfter == destBefore + withdrawAmt);
BEAST_EXPECT(coverAfter == coverBefore - withdrawAmt);
}
else
{
BEAST_EXPECT(destAfter == destBefore);
BEAST_EXPECT(coverAfter == coverBefore);
}
};
auto runDeletedCoverWithdraw = [this](FeatureBitset features, TER selfExpected) {
using namespace loan_broker;
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
env.fund(XRP(10'000), issuer, alice);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
Issue const usdIssue = usd.raw().get<Issue>();
env(trust(alice, usd(10'000)));
env.close();
env(pay(issuer, alice, usd(600)));
env.close();
Vault const vault{env};
auto const [createTx, vaultKeylet, subscriptionDate] = vault.createClosedEnded(
{.owner = alice, .asset = usd, .subscriptionOffset = std::chrono::seconds{60}});
(void)subscriptionDate;
env(createTx);
env.close();
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = usd(500)}));
env.close();
auto const brokerKeylet =
keylet::loanBroker(alice.id(), SeqProxy::rawSequence(env.seq(alice)));
env(set(alice, vaultKeylet.key));
env.close();
env(coverDeposit(alice, brokerKeylet.key, usd(100).value()));
env.close();
env(trust(alice, usd(0)));
env.close();
BEAST_EXPECT(!env.le(keylet::trustLine(alice.id(), usdIssue)));
env(fclear(issuer, asfDefaultRipple));
env.close();
env(coverWithdraw(alice, brokerKeylet.key, usd(50).value()), Ter(selfExpected));
env.close();
};
auto runPrivateVault = [this](FeatureBitset features, TER selfExpected) {
Env env(*this, features);
Account const issuer{"issuer"};
Account const alice{"alice"};
Account const pdOwner{"pdOwner"};
Account const credIssuer{"credIssuer"};
std::string const credType = "credential";
env.fund(XRP(10'000), issuer, alice, pdOwner, credIssuer);
env.close();
env(fset(issuer, asfDefaultRipple));
env.close();
PrettyAsset const usd{issuer["USD"]};
env(trust(alice, usd(10'000)));
env.close();
env(pay(issuer, alice, usd(1'000)));
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] =
vault.create({.owner = alice, .asset = usd, .flags = tfVaultPrivate});
env(vaultTx);
env.close();
pdomain::Credentials const credentials{{.issuer = credIssuer, .credType = credType}};
env(pdomain::setTx(pdOwner, credentials));
auto const domainId = pdomain::getNewDomain(env.meta());
{
auto domainTx = vault.set({.owner = alice, .id = vaultKeylet.key});
domainTx[sfDomainID] = to_string(domainId);
env(domainTx);
env.close();
}
env(credentials::create(alice, credIssuer, credType));
env(credentials::accept(alice, credIssuer, credType));
env.close();
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = usd(500)}));
env.close();
env(fclear(issuer, asfDefaultRipple));
env.close();
env(vault.withdraw({.depositor = alice, .id = vaultKeylet.key, .amount = usd(50)}),
Ter(selfExpected));
env.close();
};
testcase(
"bug: VaultWithdraw to self fails with tecINTERNAL after issuer "
"clears asfDefaultRipple even though the trust line exists "
"(pre-fixCleanup3_4_0)");
runExistingLine(all_ - fixCleanup3_4_0, tecINTERNAL);
testcase(
"bug: VaultWithdraw to self succeeds after issuer clears "
"asfDefaultRipple when the trust line exists (post-fixCleanup3_4_0)");
runExistingLine(all_, tesSUCCESS);
testcase(
"bug: VaultWithdraw to self with an existing line still gets "
"tecFROZEN under asfGlobalFreeze (post-fixCleanup3_4_0)");
runExistingLine(all_, tecFROZEN, true);
testcase(
"bug: VaultWithdraw to self fails with tecINTERNAL after issuer "
"clears asfDefaultRipple and the trust line was deleted "
"(pre-fixCleanup3_4_0)");
runDeletedLine(all_ - fixCleanup3_4_0, tecINTERNAL);
testcase(
"bug: VaultWithdraw to self fails with terNO_RIPPLE after issuer "
"clears asfDefaultRipple and the trust line was deleted "
"(post-fixCleanup3_4_0)");
runDeletedLine(all_, terNO_RIPPLE);
testcase(
"bug: LoanBrokerCoverWithdraw to self fails with tecINTERNAL after "
"issuer clears asfDefaultRipple even though the trust line exists "
"(pre-fixCleanup3_4_0)");
runCoverWithdraw(all_ - fixCleanup3_4_0, tecINTERNAL);
testcase(
"bug: LoanBrokerCoverWithdraw to self succeeds after issuer clears "
"asfDefaultRipple when the trust line exists (post-fixCleanup3_4_0)");
runCoverWithdraw(all_, tesSUCCESS);
testcase(
"bug: LoanBrokerCoverWithdraw to self fails with tecINTERNAL after "
"issuer clears asfDefaultRipple and the trust line was deleted "
"(pre-fixCleanup3_4_0)");
runDeletedCoverWithdraw(all_ - fixCleanup3_4_0, tecINTERNAL);
testcase(
"bug: LoanBrokerCoverWithdraw to self fails with terNO_RIPPLE after "
"issuer clears asfDefaultRipple and the trust line was deleted "
"(post-fixCleanup3_4_0)");
runDeletedCoverWithdraw(all_, terNO_RIPPLE);
testcase(
"bug: private VaultWithdraw to self fails with tecINTERNAL after "
"issuer clears asfDefaultRipple even though the trust line exists "
"(pre-fixCleanup3_4_0)");
runPrivateVault(all_ - fixCleanup3_4_0, tecINTERNAL);
testcase(
"bug: private VaultWithdraw to self succeeds after issuer clears "
"asfDefaultRipple when the trust line exists (post-fixCleanup3_4_0)");
runPrivateVault(all_, tesSUCCESS);
}
// Bug 1: a sponsored XRP VaultWithdraw to a distinct destination is
// rejected because the vault invariant treats the holder's touched
// but economically unchanged AccountRoot as a second payout
@@ -2468,34 +2821,369 @@ private:
}
public:
void
run() override
{
testVaultWithdrawEqualityEnforced();
testBugIssuerVaultDepositAtEdge();
testBugMakeDeltaPosteriorScale();
testBugMakeDeltaAnteriorScale();
testVaultDepositCanonicalizeToZero();
testBugDepositShareTruncationSubUlp();
testVaultWithdrawCanonicalizeToZero();
testBugVaultDustDebitCanonicalizesToNoOp();
testBugVaultDepositOvercreditsAcrossScaleBoundary();
testBugVaultLockedByPartialWithdraw();
testVaultDepositNegativeBalanceFromOppositeLimit();
testCredentialPinsPseudoAccount();
testCredentialPinOverflow();
testBug6LimitBypassWithShares();
testBugClawbackRoundTripOvershoot();
testBugWithdrawRoundTripOvershoot();
testBugClawbackAfterLoanImpair();
testBugMptZeroWithdrawMissingHolding();
testBugIouZeroWithdrawMissingTrustLine();
testBugXrpZeroWithdrawSponsoredFee();
testBugSponsoredWithdrawZeroDeltaMisclassifiedAsSecondRecipient();
testBugSponsorAsDestinationFeeMisappliedToPayout();
testPrefundedFeeWithdraw();
testUnsponsoredWithdrawToDistinctDestinationPreAmendment();
}
}
// Bug 1: a sponsored XRP VaultWithdraw to a distinct destination is
// rejected because the vault invariant treats the holder's touched
// but economically unchanged AccountRoot as a second payout
// recipient. Sequence/ticket processing still touches the holder
// while the sponsor pays the fee, so the holder's XRP delta is
// present-zero and is not normalized away. If this happens on the
// last Subscription ledger of a closed-ended vault, the holder
// cannot retry until Redemption (tecTOO_SOON during Investment).
//
// Fixed by ValidVault::deltaAssetsForParty always collapsing an
// economically-zero XRP delta to absence, regardless of who paid the
// fee.
void
testBugSponsoredWithdrawZeroDeltaMisclassifiedAsSecondRecipient()
{
using namespace test::jtx;
auto runScenario = [this](FeatureBitset features, TER expected) {
Env env{*this, features};
Account const owner{"owner"};
Account const holder{"holder"};
Account const destination{"destination"};
Account const sponsor{"sponsor"};
env.fund(XRP(10'000), owner, holder, destination, sponsor);
env.close();
constexpr std::uint32_t investmentPeriod = 14u * 24u * 60u * 60u;
auto const [vault, vaultKeylet, subscriptionDate, redemptionDate] =
makeClosedEndedVault(env, owner, xrpIssue(), 120u, investmentPeriod);
BEAST_EXPECT(redemptionDate - subscriptionDate == investmentPeriod);
env(vault.deposit(
{.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()}));
env.close();
// Inclusive SubscriptionDate boundary: still Subscription, so an
// ordinary withdrawal is allowed.
closeToTime(env, tp{d{subscriptionDate}});
auto const vaultBefore = env.le(vaultKeylet);
if (!BEAST_EXPECT(vaultBefore))
return;
auto const assetsTotalBefore = vaultBefore->at(sfAssetsTotal);
auto const holderBalanceBefore = env.balance(holder);
auto const destinationBalanceBefore = env.balance(destination);
auto const sponsorBalanceBefore = env.balance(sponsor);
auto const fee = env.current()->fees().base;
auto withdraw = vault.withdraw(
{.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()});
withdraw[sfDestination] = destination.human();
env(withdraw,
Fee(fee),
sponsor::As(sponsor, spfSponsorFee),
Sig(sfSponsorSignature, sponsor),
Ter(expected));
env.close();
auto const vaultAfter = env.le(vaultKeylet);
if (!BEAST_EXPECT(vaultAfter))
return;
BEAST_EXPECT(env.balance(sponsor) == sponsorBalanceBefore - fee);
if (expected == tesSUCCESS)
{
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == assetsTotalBefore - XRP(100).value());
BEAST_EXPECT(env.balance(holder) == holderBalanceBefore);
BEAST_EXPECT(env.balance(destination) == destinationBalanceBefore + XRP(100));
return;
}
// Invariant rollback: the payout and share burn are undone, but
// sequence processing and the sponsored fee charge remain.
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == assetsTotalBefore);
BEAST_EXPECT(env.balance(holder) == holderBalanceBefore);
BEAST_EXPECT(env.balance(destination) == destinationBalanceBefore);
// Once the ledger advances into Investment, the same holder
// cannot retry until Redemption.
auto retry = vault.withdraw(
{.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()});
retry[sfDestination] = destination.human();
env(retry, Ter(tecTOO_SOON));
};
testcase(
"bug: sponsored XRP withdrawal to a distinct destination misreads a "
"touched-but-zero sender delta as a second recipient "
"(pre-fixCleanup3_4_0)");
runScenario(all_ - fixCleanup3_4_0, tecINVARIANT_FAILED);
testcase(
"bug: sponsored XRP withdrawal to a distinct destination succeeds "
"(post-fixCleanup3_4_0)");
runScenario(all_, tesSUCCESS);
}
// Bug 2: a co-signed fee sponsor named as the withdrawal's own
// destination pays its fee from the same AccountRoot it is paid into,
// so its net XRP delta is (payout - fee). The invariant never fee-
// corrected the destination side at all, so this always failed the
// equal-amount check against the vault's outflow (payout).
//
// Fixed by ValidVault::deltaAssetsForParty adding the fee back onto
// whichever inspected party's AccountRoot actually paid it -- the
// sender, or a distinct destination -- not just the sender.
void
testBugSponsorAsDestinationFeeMisappliedToPayout()
{
using namespace test::jtx;
auto runScenario = [this](FeatureBitset features, TER expected) {
Env env{*this, features};
Account const owner{"owner"};
Account const holder{"holder"};
Account const sponsor{"sponsor"};
env.fund(XRP(10'000), owner, holder, sponsor);
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = owner, .asset = xrpIssue()});
env(vaultTx);
env.close();
env(vault.deposit(
{.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()}));
env.close();
auto const vaultBefore = env.le(vaultKeylet);
if (!BEAST_EXPECT(vaultBefore))
return;
auto const assetsTotalBefore = vaultBefore->at(sfAssetsTotal);
auto const sponsorBalanceBefore = env.balance(sponsor);
auto const fee = env.current()->fees().base;
// The sponsor both receives the withdrawal (as sfDestination)
// and pays its own fee (co-signed) from the same AccountRoot.
auto withdraw = vault.withdraw(
{.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()});
withdraw[sfDestination] = sponsor.human();
env(withdraw,
Fee(fee),
sponsor::As(sponsor, spfSponsorFee),
Sig(sfSponsorSignature, sponsor),
Ter(expected));
env.close();
auto const vaultAfter = env.le(vaultKeylet);
if (!BEAST_EXPECT(vaultAfter))
return;
if (expected == tesSUCCESS)
{
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == assetsTotalBefore - XRP(100).value());
// Paid the withdrawal, then separately debited for the fee
// it chose to cover; net effect is payout minus fee.
BEAST_EXPECT(env.balance(sponsor) == sponsorBalanceBefore + XRP(100) - fee);
return;
}
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == assetsTotalBefore);
BEAST_EXPECT(env.balance(sponsor) == sponsorBalanceBefore - fee);
};
testcase(
"bug: co-signed sponsor named as withdrawal destination has its "
"own fee debit misread as breaking the payout equality "
"(pre-fixCleanup3_4_0)");
runScenario(all_ - fixCleanup3_4_0, tecINVARIANT_FAILED);
testcase(
"bug: co-signed sponsor named as withdrawal destination succeeds "
"(post-fixCleanup3_4_0)");
runScenario(all_, tesSUCCESS);
}
// Pre-funded fee sponsorship draws the fee from ltSponsorship.sfFeeAmount,
// so feePayerAccountRoot must return nullopt rather than the sponsor's
// AccountRoot. A bystander sponsor leaves that branch unexercised: the
// result is only consulted by deltaAssetsForParty via `payer && *payer ==
// id`. Naming the sponsor as sfDestination makes the early return
// load-bearing -- returning the sponsor's id instead of nullopt would add
// the fee back onto a balance that never paid it, and the equal-amount
// check against the vault outflow would fail.
//
// Contrast testBugSponsorAsDestinationFeeMisappliedToPayout, where the
// sponsor co-signs and so really does pay from its own AccountRoot.
void
testPrefundedFeeWithdraw()
{
using namespace test::jtx;
auto runScenario = [this](
FeatureBitset features, TER expected, bool const sponsorIsDestination) {
Env env{*this, features};
Account const owner{"owner"};
Account const holder{"holder"};
Account const destination{"destination"};
Account const sponsor{"sponsor"};
env.fund(XRP(10'000), owner, holder, destination, sponsor);
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = owner, .asset = xrpIssue()});
env(vaultTx);
env.close();
env(vault.deposit(
{.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()}));
env.close();
auto const fee = env.current()->fees().base;
env(sponsor::set_fee(sponsor, 0, fee), sponsor::SponseeAcc(holder));
env.close();
auto const vaultBefore = env.le(vaultKeylet);
if (!BEAST_EXPECT(vaultBefore))
return;
auto const assetsTotalBefore = vaultBefore->at(sfAssetsTotal);
auto const holderBalanceBefore = env.balance(holder);
auto const destinationBalanceBefore = env.balance(destination);
auto const sponsorBalanceBefore = env.balance(sponsor);
Account const& recipient = sponsorIsDestination ? sponsor : destination;
auto withdraw = vault.withdraw(
{.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()});
withdraw[sfDestination] = recipient.human();
env(withdraw, Fee(fee), sponsor::As(sponsor, spfSponsorFee), Ter(expected));
env.close();
auto const vaultAfter = env.le(vaultKeylet);
if (!BEAST_EXPECT(vaultAfter))
return;
// Holder is economically unchanged (sequence only); the fee is
// taken from the sponsorship object, not any AccountRoot.
BEAST_EXPECT(env.balance(holder) == holderBalanceBefore);
if (expected == tesSUCCESS)
{
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == assetsTotalBefore - XRP(100).value());
if (sponsorIsDestination)
{
// The sponsor receives the payout and is not debited for
// the fee. The sponsor has to BE the destination for
// FeePayerType::SponsorPreFunded to matter.
BEAST_EXPECT(env.balance(sponsor) == sponsorBalanceBefore + XRP(100));
}
else
{
BEAST_EXPECT(env.balance(destination) == destinationBalanceBefore + XRP(100));
BEAST_EXPECT(env.balance(sponsor) == sponsorBalanceBefore);
}
auto const sponsorship = env.le(keylet::sponsorship(sponsor, holder));
if (!BEAST_EXPECT(sponsorship))
return;
BEAST_EXPECT(!sponsorship->isFieldPresent(sfFeeAmount));
return;
}
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == assetsTotalBefore);
BEAST_EXPECT(env.balance(sponsor) == sponsorBalanceBefore);
if (!sponsorIsDestination)
BEAST_EXPECT(env.balance(destination) == destinationBalanceBefore);
};
testcase(
"pre-funded fee XRP withdrawal to a distinct destination succeeds "
"(post-fixCleanup3_4_0)");
runScenario(all_, tesSUCCESS, false);
testcase(
"bug: pre-funded sponsor named as withdrawal destination misreads "
"the sender's touched-but-zero delta as a second recipient "
"(pre-fixCleanup3_4_0)");
runScenario(all_ - fixCleanup3_4_0, tecINVARIANT_FAILED, true);
testcase(
"bug: pre-funded sponsor named as withdrawal destination receives "
"the full payout (post-fixCleanup3_4_0)");
runScenario(all_, tesSUCCESS, true);
}
// Unsponsored third-party XRP withdrawal: the sender's AccountRoot moves
// by exactly -fee. Pre-amendment, the sender-only fee correction then
// collapses that to absence so the dual-recipient guard does not fire.
void
testUnsponsoredWithdrawToDistinctDestinationPreAmendment()
{
using namespace test::jtx;
testcase(
"unsponsored XRP withdrawal to a distinct destination succeeds "
"(pre-fixCleanup3_4_0)");
Env env{*this, all_ - fixCleanup3_4_0};
Account const owner{"owner"};
Account const holder{"holder"};
Account const destination{"destination"};
env.fund(XRP(10'000), owner, holder, destination);
env.close();
Vault const vault{env};
auto [vaultTx, vaultKeylet] = vault.create({.owner = owner, .asset = xrpIssue()});
env(vaultTx);
env.close();
env(vault.deposit({.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()}));
env.close();
auto const vaultBefore = env.le(vaultKeylet);
if (!BEAST_EXPECT(vaultBefore))
return;
auto const assetsTotalBefore = vaultBefore->at(sfAssetsTotal);
auto const holderBalanceBefore = env.balance(holder);
auto const destinationBalanceBefore = env.balance(destination);
auto const fee = env.current()->fees().base;
auto withdraw =
vault.withdraw({.depositor = holder, .id = vaultKeylet.key, .amount = XRP(100).value()});
withdraw[sfDestination] = destination.human();
env(withdraw, Fee(fee), Ter(tesSUCCESS));
env.close();
auto const vaultAfter = env.le(vaultKeylet);
if (!BEAST_EXPECT(vaultAfter))
return;
BEAST_EXPECT(vaultAfter->at(sfAssetsTotal) == assetsTotalBefore - XRP(100).value());
BEAST_EXPECT(env.balance(holder) == holderBalanceBefore - fee);
BEAST_EXPECT(env.balance(destination) == destinationBalanceBefore + XRP(100));
}
public:
void
run() override
{
testVaultWithdrawEqualityEnforced();
testBugIssuerVaultDepositAtEdge();
testBugMakeDeltaPosteriorScale();
testBugMakeDeltaAnteriorScale();
testVaultDepositCanonicalizeToZero();
testBugDepositShareTruncationSubUlp();
testVaultWithdrawCanonicalizeToZero();
testBugVaultDustDebitCanonicalizesToNoOp();
testBugVaultDepositOvercreditsAcrossScaleBoundary();
testBugVaultLockedByPartialWithdraw();
testVaultDepositNegativeBalanceFromOppositeLimit();
testCredentialPinsPseudoAccount();
testCredentialPinOverflow();
testBug6LimitBypassWithShares();
testBugClawbackRoundTripOvershoot();
testBugWithdrawRoundTripOvershoot();
testBugClawbackAfterLoanImpair();
testBugMptZeroWithdrawMissingHolding();
testBugIouZeroWithdrawMissingTrustLine();
testBugXrpZeroWithdrawSponsoredFee();
testBugSelfWithdrawAfterIssuerClearsDefaultRipple();
testBugSponsoredWithdrawZeroDeltaMisclassifiedAsSecondRecipient();
testBugSponsorAsDestinationFeeMisappliedToPayout();
testPrefundedFeeWithdraw();
testUnsponsoredWithdrawToDistinctDestinationPreAmendment();
}
};
BEAST_DEFINE_TESTSUITE(VaultBugs, app, xrpl);