diff --git a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp index a650aed310a..d70c78b3139 100644 --- a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp @@ -13,6 +13,7 @@ #include #include +#include namespace xrpl { NotTEC @@ -226,7 +227,11 @@ VaultClawback::assetsToClawback( auto const mptIssuanceID = *vault->at(sfShareMPTID); MPTIssue const share{mptIssuanceID}; - if (clawbackAmount == beast::zero) + // Pre-fixSecurity3_1_3: zero-amount clawback returned early without + // clamping to assetsAvailable, allowing more assets to be recovered + // than available when there was an outstanding loan. Retained for + // ledger replay compatibility. + if (!ctx_.view().rules().enabled(fixSecurity3_1_3) && clawbackAmount == beast::zero) { auto const sharesDestroyed = accountHolds( view(), @@ -243,22 +248,40 @@ VaultClawback::assetsToClawback( } STAmount sharesDestroyed; - STAmount assetsRecovered = clawbackAmount; + STAmount assetsRecovered; + try { + if (clawbackAmount == beast::zero) + { + sharesDestroyed = accountHolds( + view(), + holder, + share, + FreezeHandling::fhIGNORE_FREEZE, + AuthHandling::ahIGNORE_AUTH, + j_); + auto const maybeAssets = + sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed); + if (!maybeAssets) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + + assetsRecovered = *maybeAssets; + } + else { auto const maybeShares = - assetsToSharesWithdraw(vault, sleShareIssuance, assetsRecovered); + assetsToSharesWithdraw(vault, sleShareIssuance, clawbackAmount); if (!maybeShares) return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE sharesDestroyed = *maybeShares; - } - - auto const maybeAssets = sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed); - if (!maybeAssets) - return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE - assetsRecovered = *maybeAssets; + auto const maybeAssets = + sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed); + if (!maybeAssets) + return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE + assetsRecovered = *maybeAssets; + } // Clamp to maximum. if (assetsRecovered > *assetsAvailable) { diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index fa935eba6e7..92699032d51 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -4785,6 +4785,7 @@ class Vault_test : public beast::unit_test::suite using namespace loanBroker; using namespace loan; Env env(*this); + env.enableFeature(fixSecurity3_1_3); auto const setupVault = [&](PrettyAsset const& asset, Account const& owner, @@ -4798,6 +4799,7 @@ class Vault_test : public beast::unit_test::suite auto const& vaultSle = env.le(vaultKeylet); BEAST_EXPECT(vaultSle != nullptr); + env.memoize(Account("vault", vaultSle->at(sfAccount))); env(vault.deposit( {.depositor = depositor, .id = vaultKeylet.key, .amount = asset(100)}), ter(tesSUCCESS)); @@ -4954,6 +4956,124 @@ class Vault_test : public beast::unit_test::suite }), ter(tesSUCCESS)); } + + { + testcase( + "VaultClawback (asset) - " + prefix + + " zero-amount clawback clamped with outstanding loan"); + auto [vault, vaultKeylet] = setupVault(asset, owner, depositor, issuer); + + auto const vaultSle = env.le(vaultKeylet); + BEAST_EXPECT(vaultSle != nullptr); + if (!vaultSle) + return; + + PrettyAsset shares = MPTIssue(vaultSle->at(sfShareMPTID)); + + // Create a loan broker backed by this vault + auto const brokerKeylet = keylet::loanbroker(owner.id(), env.seq(owner)); + env(set(owner, vaultKeylet.key)); + env.close(); + + // Depositor borrows 40 units, reducing assetsAvailable to 60 + // while assetsTotal stays at 100 + env(set(depositor, brokerKeylet.key, asset(40).value()), + loan::interestRate(TenthBips32(0)), + gracePeriod(60), + paymentInterval(120), + paymentTotal(10), + sig(sfCounterpartySignature, owner), + fee(env.current()->fees().base * 2), + ter(tesSUCCESS)); + env.close(); + + { + auto const sle = env.le(vaultKeylet); + BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(60).value()); + BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(100).value()); + } + + // Zero-amount clawback (= "clawback all") should succeed, + // clamped to assetsAvailable (60) rather than the full + // share value (100). + env(vault.clawback({ + .issuer = issuer, + .id = vaultKeylet.key, + .holder = depositor, + }), + ter(tesSUCCESS)); + env.close(); + + // Only 60 assets clawed back; loan's 40 still outstanding + { + auto const sle = env.le(vaultKeylet); + BEAST_EXPECT(sle != nullptr); + BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(0).value()); + BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(40).value()); + + // 60 of 100 shares destroyed (1:1 ratio), 40 remain + auto const sharesAfter = env.balance(depositor, shares); + BEAST_EXPECT(sharesAfter == shares(Number{4, sle->at(sfScale) + 1})); + } + } + + { + testcase( + "VaultClawback (asset) - " + prefix + + " non-zero clawback clamped with outstanding loan"); + auto [vault, vaultKeylet] = setupVault(asset, owner, depositor, issuer); + + auto const vaultSle = env.le(vaultKeylet); + BEAST_EXPECT(vaultSle != nullptr); + if (!vaultSle) + return; + PrettyAsset shares = MPTIssue(vaultSle->at(sfShareMPTID)); + + // Create a loan broker backed by this vault + auto const brokerKeylet = keylet::loanbroker(owner.id(), env.seq(owner)); + env(set(owner, vaultKeylet.key)); + env.close(); + + // Depositor borrows 40 units + env(set(depositor, brokerKeylet.key, asset(40).value()), + loan::interestRate(TenthBips32(0)), + gracePeriod(60), + paymentInterval(120), + paymentTotal(10), + sig(sfCounterpartySignature, owner), + fee(env.current()->fees().base * 2), + ter(tesSUCCESS)); + env.close(); + + { + auto const sle = env.le(vaultKeylet); + BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(60).value()); + BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(100).value()); + } + + auto const sharesBefore = env.balance(depositor, shares); + + // Request 100 but only 60 available — clamped to 60 + env(vault.clawback({ + .issuer = issuer, + .id = vaultKeylet.key, + .holder = depositor, + .amount = asset(100).value(), + }), + ter(tesSUCCESS)); + env.close(); + + { + auto const sle = env.le(vaultKeylet); + BEAST_EXPECT(sle != nullptr); + BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(0).value()); + BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(40).value()); + + // 60 of 100 shares destroyed (1:1 ratio), 40 remain + auto const sharesAfter = env.balance(depositor, shares); + BEAST_EXPECT(sharesAfter == shares(Number{4, sle->at(sfScale) + 1})); + } + } }; Account owner{"alice"}; @@ -4987,6 +5107,74 @@ class Vault_test : public beast::unit_test::suite env(pay(issuer, depositor, MPT(1000))); env.close(); testCase(MPT, "MPT", owner, depositor, issuer); + + // Test pre-fixSecurity3_1_3 legacy path: zero-amount clawback + // returns early without clamping to assetsAvailable. + { + testcase( + "VaultClawback (asset) - IOU pre-fixSecurity3_1_3" + " zero-amount clawback unclamped with outstanding loan"); + + env.disableFeature(fixSecurity3_1_3); + + auto [vault, vaultKeylet] = setupVault(IOU, owner, depositor, issuer); + + auto const vaultSle = env.le(vaultKeylet); + BEAST_EXPECT(vaultSle != nullptr); + if (!vaultSle) + return; + + PrettyAsset shares = MPTIssue(vaultSle->at(sfShareMPTID)); + + // Create a loan broker backed by this vault + auto const brokerKeylet = keylet::loanbroker(owner.id(), env.seq(owner)); + env(set(owner, vaultKeylet.key)); + env.close(); + + // Depositor borrows 40 units, reducing assetsAvailable to 60 + // while assetsTotal stays at 100 + env(set(depositor, brokerKeylet.key, IOU(40).value()), + loan::interestRate(TenthBips32(0)), + gracePeriod(60), + paymentInterval(120), + paymentTotal(10), + sig(sfCounterpartySignature, owner), + fee(env.current()->fees().base * 2), + ter(tesSUCCESS)); + env.close(); + + { + auto const sle = env.le(vaultKeylet); + BEAST_EXPECT(sle->at(sfAssetsAvailable) == IOU(60).value()); + BEAST_EXPECT(sle->at(sfAssetsTotal) == IOU(100).value()); + } + + auto const sharesBefore = env.balance(depositor, shares); + + // Legacy: zero-amount clawback tries to recover the full + // share value (100) without clamping to assetsAvailable (60). + // This causes the vault balance to go negative, triggering + // the sanity check in doApply → tefINTERNAL. + env(vault.clawback({ + .issuer = issuer, + .id = vaultKeylet.key, + .holder = depositor, + }), + ter(tefINTERNAL)); + env.close(); + + { + // Transaction rolled back — vault and shares unchanged + auto const sle = env.le(vaultKeylet); + BEAST_EXPECT(sle != nullptr); + BEAST_EXPECT(sle->at(sfAssetsAvailable) == IOU(60).value()); + BEAST_EXPECT(sle->at(sfAssetsTotal) == IOU(100).value()); + auto const sharesAfter = env.balance(depositor, shares); + BEAST_EXPECT(sharesAfter == sharesBefore); + } + + env.enableFeature(fixSecurity3_1_3); + } } void