From 60291c3ed613a749f6aeced06d485b1d478d9843 Mon Sep 17 00:00:00 2001 From: Kassaking7 <96991820+Kassaking7@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:34:28 +0000 Subject: [PATCH] fix: Allow OverrideFreeze to bypass individual/deep freeze on AMM trust lines (#6959) --- include/xrpl/tx/invariants/FreezeInvariant.h | 6 +- src/libxrpl/tx/invariants/FreezeInvariant.cpp | 23 +- src/test/app/AMMClawback_test.cpp | 204 ++++++++++++++++++ 3 files changed, 222 insertions(+), 11 deletions(-) diff --git a/include/xrpl/tx/invariants/FreezeInvariant.h b/include/xrpl/tx/invariants/FreezeInvariant.h index 4b3e9beec4..c66e002872 100644 --- a/include/xrpl/tx/invariants/FreezeInvariant.h +++ b/include/xrpl/tx/invariants/FreezeInvariant.h @@ -69,7 +69,8 @@ private: IssuerChanges const& changes, STTx const& tx, beast::Journal const& j, - bool enforce); + bool enforce, + bool fixOverrideFreeze); static bool validateFrozenState( @@ -78,7 +79,8 @@ private: STTx const& tx, beast::Journal const& j, bool enforce, - bool globalFreeze); + bool globalFreeze, + bool fixOverrideFreeze); }; } // namespace xrpl diff --git a/src/libxrpl/tx/invariants/FreezeInvariant.cpp b/src/libxrpl/tx/invariants/FreezeInvariant.cpp index 0a604d4c39..c4340b9aec 100644 --- a/src/libxrpl/tx/invariants/FreezeInvariant.cpp +++ b/src/libxrpl/tx/invariants/FreezeInvariant.cpp @@ -73,6 +73,7 @@ TransfersNotFrozen::finalize( * view.rules().enabled(fixFreezeExploit); */ [[maybe_unused]] bool const enforce = view.rules().enabled(featureDeepFreeze); + bool const fixOverrideFreeze = view.rules().enabled(fixCleanup3_4_0); return std::ranges::all_of(balanceChanges_, [&](auto const& entry) { auto const& [issue, changes] = entry; @@ -90,7 +91,7 @@ TransfersNotFrozen::finalize( return !enforce; } - return validateIssuerChanges(issuerSle, changes, tx, j, enforce); + return validateIssuerChanges(issuerSle, changes, tx, j, enforce, fixOverrideFreeze); }); } @@ -199,7 +200,8 @@ TransfersNotFrozen::validateIssuerChanges( IssuerChanges const& changes, STTx const& tx, beast::Journal const& j, - bool enforce) + bool enforce, + bool fixOverrideFreeze) { if (!issuer) { @@ -225,7 +227,7 @@ TransfersNotFrozen::validateIssuerChanges( { bool const high = change.line->at(sfLowLimit).getIssuer() == issuer->at(sfAccount); - if (!validateFrozenState(change, high, tx, j, enforce, globalFreeze)) + if (!validateFrozenState(change, high, tx, j, enforce, globalFreeze, fixOverrideFreeze)) { return false; } @@ -241,26 +243,29 @@ TransfersNotFrozen::validateFrozenState( STTx const& tx, beast::Journal const& j, bool enforce, - bool globalFreeze) + bool globalFreeze, + bool fixOverrideFreeze) { bool const freeze = change.balanceChangeSign < 0 && change.line->isFlag(high ? lsfLowFreeze : lsfHighFreeze); bool const deepFreeze = change.line->isFlag(high ? lsfLowDeepFreeze : lsfHighDeepFreeze); bool const frozen = globalFreeze || deepFreeze || freeze; - bool const isAMMLine = change.line->isFlag(lsfAMMNode); - if (!frozen) { return true; } - // AMMClawbacks are allowed to override some freeze rules - if ((!isAMMLine || globalFreeze) && hasPrivilege(tx, OverrideFreeze)) + // Pre-fixCleanup3_4_0: the isAMMLine check incorrectly blocked clawback on + // individually-frozen or deep-frozen AMM trust lines. + // Post-fixCleanup3_4_0: AMMClawbacks are allowed to override all freeze types. + bool const isAMMLine = change.line->isFlag(lsfAMMNode); + if ((fixOverrideFreeze || !isAMMLine || globalFreeze) && hasPrivilege(tx, OverrideFreeze)) { JLOG(j.debug()) << "Invariant check allowing funds to be moved " << (change.balanceChangeSign > 0 ? "to" : "from") - << " a frozen trustline for AMMClawback " << tx.getTransactionID(); + << " a frozen trustline for a freeze privileged transaction " + << tx.getTransactionID(); return true; } diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index ba416d8192..90bface1fb 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -2155,6 +2155,209 @@ class AMMClawback_test : public beast::unit_test::Suite } BEAST_EXPECT(env.balance(carol, eur) == eur(7750)); } + + // gw (USD issuer) individually freezes the AMM-USD trust line. + // AMMClawback must still succeed because the freeze invariant + // short-circuits before reaching the AMM line check (no receivers in + // the USD issuer's change set). Behavior is identical with or without + // fixCleanup3_4_0. + { + Env env(*this, features); + Account const gw{"gateway"}; + Account const gw2{"gateway2"}; + Account const alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(Flags(gw, asfAllowTrustLineClawback)); + + auto const usd = gw["USD"]; + env.trust(usd(100000), alice); + env(pay(gw, alice, usd(3000))); + env.close(); + + auto const eur = gw2["EUR"]; + env.trust(eur(100000), alice); + env(pay(gw2, alice, eur(3000))); + env.close(); + + AMM const amm(env, alice, eur(1000), usd(2000), Ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT( + amm.expectBalances(usd(2000), eur(1000), IOUAmount{1414213562373095, -12})); + + // gw individually freezes the AMM-USD trust line (AMM pseudo-account + // <-> gw), not alice's trust line. + env(trust(gw, STAmount{Issue{usd.currency, amm.ammAccount()}, 0}, tfSetFreeze)); + env.close(); + + env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tesSUCCESS)); + env.close(); + + env.require(Balance(alice, usd(1000))); + env.require(Balance(alice, eur(2500))); + BEAST_EXPECT(amm.expectBalances(usd(1000), eur(500), IOUAmount{7071067811865475, -13})); + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13})); + } + + // gw2 (EUR issuer) individually freezes the AMM-EUR trust line. + // The EUR flow (AMM → alice) is a genuine P2P transfer checked by the + // freeze invariant. Pre-fixCleanup3_4_0 the isAMMNode guard incorrectly + // blocked AMMClawback's overrideFreeze privilege on that trust line. + { + Env env(*this, features); + Account const gw{"gateway"}; + Account const gw2{"gateway2"}; + Account const alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(Flags(gw, asfAllowTrustLineClawback)); + + auto const usd = gw["USD"]; + env.trust(usd(100000), alice); + env(pay(gw, alice, usd(3000))); + env.close(); + + auto const eur = gw2["EUR"]; + env.trust(eur(100000), alice); + env(pay(gw2, alice, eur(3000))); + env.close(); + + AMM const amm(env, alice, eur(1000), usd(2000), Ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT( + amm.expectBalances(usd(2000), eur(1000), IOUAmount{1414213562373095, -12})); + + // gw2 individually freezes the AMM-EUR trust line. + env(trust(gw2, STAmount{Issue{eur.currency, amm.ammAccount()}, 0}, tfSetFreeze)); + env.close(); + + if (features[fixCleanup3_4_0]) + { + // Post-fixCleanup3_4_0: overrideFreeze privilege applies to + // all freeze types on AMM trust lines. + env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tesSUCCESS)); + env.close(); + + env.require(Balance(alice, usd(1000))); + env.require(Balance(alice, eur(2500))); + BEAST_EXPECT( + amm.expectBalances(usd(1000), eur(500), IOUAmount{7071067811865475, -13})); + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13})); + } + else + { + // Pre-fixCleanup3_4_0: the isAMMNode guard prevents the + // overrideFreeze privilege from applying to individually-frozen + // AMM trust lines, so the invariant blocks the clawback. + env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tecINVARIANT_FAILED)); + } + } + + // gw2 (EUR issuer) globally freezes its issued assets. AMMClawback + // must still be able to return EUR from the AMM to alice. + { + Env env(*this, features); + Account const gw{"gateway"}; + Account const gw2{"gateway2"}; + Account const alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(Flags(gw, asfAllowTrustLineClawback)); + + auto const usd = gw["USD"]; + env.trust(usd(100000), alice); + env(pay(gw, alice, usd(3000))); + env.close(); + + auto const eur = gw2["EUR"]; + env.trust(eur(100000), alice); + env(pay(gw2, alice, eur(3000))); + env.close(); + + AMM const amm(env, alice, eur(1000), usd(2000), Ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT( + amm.expectBalances(usd(2000), eur(1000), IOUAmount{1414213562373095, -12})); + + env(fset(gw2, asfGlobalFreeze)); + env.close(); + + env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tesSUCCESS)); + env.close(); + + env.require(Balance(alice, usd(1000))); + env.require(Balance(alice, eur(2500))); + BEAST_EXPECT(amm.expectBalances(usd(1000), eur(500), IOUAmount{7071067811865475, -13})); + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13})); + } + + // Same as above but gw2 deep-freezes the AMM-EUR trust line. + if (features[featureDeepFreeze]) + { + Env env(*this, features); + Account const gw{"gateway"}; + Account const gw2{"gateway2"}; + Account const alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(Flags(gw, asfAllowTrustLineClawback)); + + auto const usd = gw["USD"]; + env.trust(usd(100000), alice); + env(pay(gw, alice, usd(3000))); + env.close(); + + auto const eur = gw2["EUR"]; + env.trust(eur(100000), alice); + env(pay(gw2, alice, eur(3000))); + env.close(); + + AMM const amm(env, alice, eur(1000), usd(2000), Ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT( + amm.expectBalances(usd(2000), eur(1000), IOUAmount{1414213562373095, -12})); + + // gw2 deep-freezes the AMM-EUR trust line. + env(trust( + gw2, + STAmount{Issue{eur.currency, amm.ammAccount()}, 0}, + tfSetFreeze | tfSetDeepFreeze)); + env.close(); + + if (features[fixCleanup3_4_0]) + { + env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tesSUCCESS)); + env.close(); + + env.require(Balance(alice, usd(1000))); + env.require(Balance(alice, eur(2500))); + BEAST_EXPECT( + amm.expectBalances(usd(1000), eur(500), IOUAmount{7071067811865475, -13})); + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13})); + } + else + { + // Pre-fixCleanup3_4_0: same isAMMNode guard issue blocks the + // clawback on deep-frozen AMM trust lines. + env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tecINVARIANT_FAILED)); + } + } } void @@ -2530,6 +2733,7 @@ class AMMClawback_test : public beast::unit_test::Suite // precision loss caught in transaction layer -> tecPRECISION_LOSS all - fixAMMClawbackRounding - featureMPTokensV2, all - featureMPTokensV2, + all - fixCleanup3_4_0, all}) { testAMMClawbackSpecificAmount(features);