fix: Allow OverrideFreeze to bypass individual/deep freeze on AMM trust lines (#6959)

This commit is contained in:
Kassaking7
2026-08-10 21:34:28 +00:00
committed by GitHub
parent 4173f7e499
commit 60291c3ed6
3 changed files with 222 additions and 11 deletions

View File

@@ -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

View File

@@ -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;
}

View File

@@ -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);