From cfd3d51338d391038454cb5dd8dbba4f2359d856 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:43:39 +0200 Subject: [PATCH] adds unified freeze checks for CoverDeposit --- src/libxrpl/ledger/helpers/TokenHelpers.cpp | 5 +- .../lending/LoanBrokerCoverDeposit.cpp | 17 +- src/test/app/LoanBroker_test.cpp | 440 ++++++++++++++---- 3 files changed, 375 insertions(+), 87 deletions(-) diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index 131e7a3e18..b3bfc8bc5a 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -211,8 +211,9 @@ checkDepositFreeze( if (auto const ret = checkFrozen(view, srcAcct, asset)) return ret; - // Pseudo-account cannot receive if asset is deep frozen - return checkDeepFrozen(view, dstAcct, asset); + // Unlike regular accounts, pseudo-accounts cannot receive deposits under a regular freeze + // because those funds cannot be later withdrawn + return checkFrozen(view, dstAcct, asset); } //------------------------------------------------------------------------------ diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp index 802fc5e9b9..b0b241c804 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp @@ -78,8 +78,21 @@ LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx) if (auto const ret = canTransfer(ctx.view, vaultAsset, account, pseudoAccountID)) return ret; - if (auto const ret = checkDepositFreeze(ctx.view, account, pseudoAccountID, vaultAsset)) - return ret; + if (ctx.view.rules().enabled(fixCleanup3_3_0)) + { + if (auto const ret = checkDepositFreeze(ctx.view, account, pseudoAccountID, vaultAsset)) + return ret; + } + else + { + if (auto const ret = checkFrozen(ctx.view, account, vaultAsset)) + return ret; + + // Unlike regular accounts, pseudo-accounts cannot receive assets + // Under a regular freeze because those funds cannot be later withdrawn + if (auto const ret = checkDeepFrozen(ctx.view, pseudoAccountID, vaultAsset)) + return ret; + } // Cannot transfer unauthorized asset if (auto const ret = requireAuth(ctx.view, vaultAsset, account, AuthType::StrongAuth)) diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 23c0fa34cf..954dee246a 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -936,11 +936,7 @@ class LoanBroker_test : public beast::unit_test::Suite env.close(); env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)), Ter(tecINSUFFICIENT_FUNDS)); - - // preclaim: tecFROZEN - env(fset(issuer, asfGlobalFreeze)); - env.close(); - env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)), Ter(tecFROZEN)); + // Freeze/lock tests are in testCoverDepositFreezes/testCoverWithdrawFreezes } else { @@ -966,35 +962,20 @@ class LoanBroker_test : public beast::unit_test::Suite // preclaim: tecDST_TAG_NEEDED Account const dest{"dest"}; env.fund(XRP(1'000), dest); + env(fset(dest, asfRequireDest)); - env.close(); env(coverWithdraw(alice, brokerKeylet.key, asset(10)), kDestination(dest), Ter(tecDST_TAG_NEEDED)); + env(fclear(dest, asfRequireDest)); // preclaim: tecNO_PERMISSION - env(fclear(dest, asfRequireDest)); env(fset(dest, asfDepositAuth)); - env.close(); env(coverWithdraw(alice, brokerKeylet.key, asset(10)), kDestination(dest), Ter(tecNO_PERMISSION)); - - // preclaim: tecFROZEN - env(trust(dest, asset(1'000))); env(fclear(dest, asfDepositAuth)); - env(fset(issuer, asfGlobalFreeze)); - env.close(); - env(coverWithdraw(alice, brokerKeylet.key, asset(10)), - kDestination(dest), - Ter(tecFROZEN)); - - // preclaim:: tecFROZEN (deep frozen) - env(fclear(issuer, asfGlobalFreeze)); - env(trust(issuer, asset(1'000), dest, tfSetFreeze | tfSetDeepFreeze)); - env(coverWithdraw(alice, brokerKeylet.key, asset(10)), - kDestination(dest), - Ter(tecFROZEN)); + // Freeze/lock tests are in testCoverDepositFreezes/testCoverWithdrawFreezes // preclaim: tecPSEUDO_ACCOUNT env(coverWithdraw(alice, brokerKeylet.key, asset(10)), @@ -1785,75 +1766,369 @@ class LoanBroker_test : public beast::unit_test::Suite } void - testCoverWithdrawFreezes(FeatureBitset features) + testCoverDepositFreezes() { - testcase << "LoanBrokerCoverWithdraw - freeze checks (fixCleanup3_3_0)"; using namespace jtx; using namespace loanBroker; - Account const issuer("issuer"); - Account const alice("alice"); + Account const issuer{"issuer"}; + Account const alice{"alice"}; - auto const withFix = features[fixCleanup3_3_0]; - Env env(*this, features); - env.fund(XRP(100'000), issuer, alice); - env.close(); + // === IOU === + { + testcase("LoanBrokerCoverDeposit IOU freeze checks"); + Env env(*this); + Vault const vault{env}; - auto const iou = issuer["IOU"]; + env.fund(XRP(100'000), issuer, alice); + env(trust(alice, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset(issuer["IOU"]); + env(pay(issuer, alice, asset(100'000))); + env.close(); - env(trust(alice, iou(1'000'000))); - env.close(); - env(pay(issuer, alice, iou(100'000))); - env.close(); + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50)})); + env.close(); - Vault const vault{env}; - auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = iou.asset()}); - env(tx); - env.close(); + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); - env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = iou(10'000)})); - env.close(); + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + Account const brokerPseudo("pseudo", broker->at(sfAccount)); - auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); - env(set(alice, vaultKeylet.key)); - env.close(); + env(coverDeposit(alice, brokerKeylet.key, asset(10))); + env.close(); - env(coverDeposit(alice, brokerKeylet.key, iou(5'000))); - env.close(); + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); - auto const broker = env.le(brokerKeylet); - if (!BEAST_EXPECT(broker)) - return; - auto const brokerPseudoID = broker->at(sfAccount); - auto const brokerPseudo = Account("BrokerPseudo", brokerPseudoID); + // Global freeze + env(fset(issuer, asfGlobalFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(fclear(issuer, asfGlobalFreeze)); - // Source (broker pseudo-account) frozen: blocked in both pre- and - // post-fixCleanup3_3_0. - env(trust(issuer, iou(0), brokerPseudo, tfSetFreeze)); - env.close(); - env(coverWithdraw(alice, brokerKeylet.key, iou(1)), Ter(tecFROZEN)); - env.close(); - env(trust(issuer, iou(0), brokerPseudo, tfClearFreeze)); - env.close(); + // Source regular freeze + env(trust(issuer, asset(0), alice, tfSetFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(trust(issuer, asset(0), alice, tfClearFreeze)); - // Pre-fixCleanup3_3_0: only the source and the destination's deep-freeze - // are checked. The submitter's (alice's) individual trust-line freeze is - // not checked — the withdraw succeeds. - // Post-fixCleanup3_3_0 (checkWithdrawFreezes): the submitter's individual - // freeze is also checked, so the withdraw is blocked with tecFROZEN. - env(trust(issuer, iou(0), alice, tfSetFreeze)); - env.close(); - env(coverWithdraw(alice, brokerKeylet.key, iou(1)), - Ter(withFix ? TER{tecFROZEN} : TER{tesSUCCESS})); - env.close(); + // Source deep freeze + env(trust(issuer, asset(0), alice, tfSetFreeze | tfSetDeepFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(trust(issuer, asset(0), alice, tfClearFreeze | tfClearDeepFreeze)); - // Sending to the issuer bypasses all freeze checks in both pre- and - // post-fixCleanup3_3_0. - env(coverWithdraw(alice, brokerKeylet.key, iou(1)), kDestination(issuer), Ter(tesSUCCESS)); - env.close(); + // Pseudo regular freeze — post-fix blocks, pre-fix allows (BUG) + TER const pseudoTer = fix330Enabled ? TER(tecFROZEN) : TER(tesSUCCESS); + env(trust(issuer, asset(0), brokerPseudo, tfSetFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(pseudoTer)); + env(trust(issuer, asset(0), brokerPseudo, tfClearFreeze)); - env(trust(issuer, iou(0), alice, tfClearFreeze)); - env.close(); + // Pseudo deep freeze + env(trust(issuer, asset(0), brokerPseudo, tfSetFreeze | tfSetDeepFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(trust(issuer, asset(0), brokerPseudo, tfClearFreeze | tfClearDeepFreeze)); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + + // === MPT === + { + testcase("LoanBrokerCoverDeposit MPT lock checks"); + Env env(*this); + Vault const vault{env}; + + env.fund(XRP(100'000), issuer, alice); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const mpt{mptt.issuanceID()}; + + mptt.authorize({.account = alice}); + env(pay(issuer, alice, mpt(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = mpt}); + env(tx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = mpt(50)})); + env.close(); + + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + Account const brokerPseudo("pseudo", broker->at(sfAccount)); + + env(coverDeposit(alice, brokerKeylet.key, mpt(10))); + env.close(); + + // For MPT isDeepFrozen == isFrozen, so all locks block in + // both pre- and post-fix. No behavioral difference. + auto runTests = [&]() { + // Global lock + mptt.set({.flags = tfMPTLock}); + env.close(); + env(coverDeposit(alice, brokerKeylet.key, mpt(1)), Ter(tecLOCKED)); + mptt.set({.flags = tfMPTUnlock}); + env.close(); + + // Source (alice) individual lock + mptt.set({.holder = alice, .flags = tfMPTLock}); + env.close(); + env(coverDeposit(alice, brokerKeylet.key, mpt(1)), Ter(tecLOCKED)); + mptt.set({.holder = alice, .flags = tfMPTUnlock}); + env.close(); + + // Pseudo individual lock + mptt.set({.holder = brokerPseudo, .flags = tfMPTLock}); + env.close(); + env(coverDeposit(alice, brokerKeylet.key, mpt(1)), Ter(tecLOCKED)); + mptt.set({.holder = brokerPseudo, .flags = tfMPTUnlock}); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + } + + void + testCoverWithdrawFreezes() + { + using namespace jtx; + using namespace loanBroker; + + Account const issuer{"issuer"}; + Account const alice{"alice"}; + + // === IOU === + { + testcase("LoanBrokerCoverWithdraw IOU freeze checks"); + Env env(*this); + Vault const vault{env}; + + env.fund(XRP(100'000), issuer, alice); + env(trust(alice, issuer["IOU"](1'000'000))); + env.close(); + PrettyAsset const asset(issuer["IOU"]); + env(pay(issuer, alice, asset(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset}); + env(tx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50)})); + env.close(); + + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + Account const brokerPseudo("pseudo", broker->at(sfAccount)); + + env(coverDeposit(alice, brokerKeylet.key, asset(10))); + env.close(); + + Account const dest{"dest"}; + env.fund(XRP(1'000), dest); + env(trust(dest, asset(1'000))); + + auto runTests = [&]() { + auto const fix330Enabled = env.current()->rules().enabled(fixCleanup3_3_0); + TER const expectedTec = fix330Enabled ? TER(tecFROZEN) : TER(tesSUCCESS); + + // Global freeze + env(fset(issuer, asfGlobalFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tecFROZEN)); + env(fclear(issuer, asfGlobalFreeze)); + + // Source (pseudo) regular freeze + env(trust(issuer, asset(0), brokerPseudo, tfSetFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tecFROZEN)); + env(trust(issuer, asset(0), brokerPseudo, tfClearFreeze)); + + // Source (pseudo) deep freeze + env(trust(issuer, asset(0), brokerPseudo, tfSetFreeze | tfSetDeepFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tecFROZEN)); + env(trust(issuer, asset(0), brokerPseudo, tfClearFreeze | tfClearDeepFreeze)); + + // Submitter regular freeze → dest + env(trust(issuer, asset(0), alice, tfSetFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(expectedTec)); + // Submitter regular freeze → self: always allowed + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), Ter(tesSUCCESS)); + env(trust(issuer, asset(0), alice, tfClearFreeze)); + env(coverDeposit( + alice, brokerKeylet.key, asset(isTesSuccess(expectedTec) ? 2 : 1))); + + // Submitter deep freeze → dest + env(trust(issuer, asset(0), alice, tfSetFreeze | tfSetDeepFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(expectedTec)); + // Submitter deep freeze → self: blocked (checkDeepFrozen) + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), Ter(tecFROZEN)); + env(trust(issuer, asset(0), alice, tfClearFreeze | tfClearDeepFreeze)); + if (isTesSuccess(expectedTec)) + env(coverDeposit(alice, brokerKeylet.key, asset(1))); + + // Destination regular freeze: only deep freeze blocks + env(trust(issuer, asset(0), dest, tfSetFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tesSUCCESS)); + env(trust(issuer, asset(0), dest, tfClearFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1))); + + // Destination deep freeze + env(trust(issuer, asset(0), dest, tfSetFreeze | tfSetDeepFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(dest), + Ter(tecFROZEN)); + env(trust(issuer, asset(0), dest, tfClearFreeze | tfClearDeepFreeze)); + + // Submitter frozen → issuer: bypasses all freeze checks + env(trust(issuer, asset(0), alice, tfSetFreeze)); + env(coverWithdraw(alice, brokerKeylet.key, asset(1)), + kDestination(issuer), + Ter(tesSUCCESS)); + env(trust(issuer, asset(0), alice, tfClearFreeze)); + env(coverDeposit(alice, brokerKeylet.key, asset(1))); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } + + // === MPT === + { + testcase("LoanBrokerCoverWithdraw MPT lock checks"); + Env env(*this); + Vault const vault{env}; + + env.fund(XRP(100'000), issuer, alice); + env.close(); + + MPTTester mptt{env, issuer, kMptInitNoFund}; + mptt.create({.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const mpt{mptt.issuanceID()}; + + mptt.authorize({.account = alice}); + env(pay(issuer, alice, mpt(100'000))); + env.close(); + + auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = mpt}); + env(tx); + env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = mpt(50)})); + env.close(); + + auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice)); + env(set(alice, vaultKeylet.key)); + env.close(); + + auto const broker = env.le(brokerKeylet); + if (!BEAST_EXPECT(broker)) + return; + Account const brokerPseudo("pseudo", broker->at(sfAccount)); + + env(coverDeposit(alice, brokerKeylet.key, mpt(10))); + env.close(); + + Account const dest{"dest"}; + env.fund(XRP(1'000), dest); + mptt.authorize({.account = dest}); + env.close(); + + auto runTests = [&]() { + auto const withFix = env.current()->rules().enabled(fixCleanup3_3_0); + // Only submitter-to-dest differs: post-fix blocks, pre-fix + // doesn't (BUG). All other locks block in both because for + // MPT isDeepFrozen == isFrozen. + TER const submitterToDest = withFix ? TER(tecLOCKED) : TER(tesSUCCESS); + + // Global lock + mptt.set({.flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(dest), + Ter(tecLOCKED)); + mptt.set({.flags = tfMPTUnlock}); + env.close(); + + // Source (pseudo) individual lock + mptt.set({.holder = brokerPseudo, .flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(dest), + Ter(tecLOCKED)); + mptt.set({.holder = brokerPseudo, .flags = tfMPTUnlock}); + env.close(); + + // Submitter individual lock → dest + mptt.set({.holder = alice, .flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(dest), + Ter(submitterToDest)); + // Submitter individual lock → self: blocked + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), Ter(tecLOCKED)); + mptt.set({.holder = alice, .flags = tfMPTUnlock}); + env.close(); + if (isTesSuccess(submitterToDest)) + env(coverDeposit(alice, brokerKeylet.key, mpt(1))); + env.close(); + + // Dest individual lock: blocked + mptt.set({.holder = dest, .flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(dest), + Ter(tecLOCKED)); + mptt.set({.holder = dest, .flags = tfMPTUnlock}); + env.close(); + + // Submitter locked → issuer: bypasses all freeze checks + mptt.set({.holder = alice, .flags = tfMPTLock}); + env.close(); + env(coverWithdraw(alice, brokerKeylet.key, mpt(1)), + kDestination(issuer), + Ter(tesSUCCESS)); + mptt.set({.holder = alice, .flags = tfMPTUnlock}); + env(coverDeposit(alice, brokerKeylet.key, mpt(1))); + env.close(); + }; + + runTests(); + env.disableFeature(fixCleanup3_3_0); + runTests(); + env.enableFeature(fixCleanup3_3_0); + } } void @@ -2316,6 +2591,12 @@ public: void run() override { + testInvalidLoanBrokerCoverClawback(); + testInvalidLoanBrokerCoverDeposit(); + testInvalidLoanBrokerCoverWithdraw(); + testCoverDepositFreezes(); + testCoverWithdrawFreezes(); + testCoverPrecisionGuard(); testLoanBrokerSetDebtMaximum(); @@ -2323,9 +2604,6 @@ public: testDisabled(); testLifecycle(); - testInvalidLoanBrokerCoverClawback(); - testInvalidLoanBrokerCoverDeposit(); - testInvalidLoanBrokerCoverWithdraw(); testInvalidLoanBrokerDelete(); testInvalidLoanBrokerSet(); testRequireAuth(); @@ -2340,10 +2618,6 @@ public: testLoanBrokerDeleteFrozenIOU(all_); testLoanBrokerDeleteFrozenIOU(all_ - fixCleanup3_2_0); - - testCoverWithdrawFreezes(all_); - testCoverWithdrawFreezes(all_ - fixCleanup3_3_0); - // TODO: Write clawback failure tests with an issuer / MPT that doesn't // have the right flags set. }