ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-10-01 17:28:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Move AMMInvariant weakInvariantCheck logic into the transaction (#7032)

Browse Source
This commit is contained in:
Zhiyuan Wang
2026-06-22 14:42:57 -04:00
committed by GitHub
parent 93eab33dc2
commit 19a9ed7767
8 changed files with 127 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
include/xrpl/ledger/helpers/AMMHelpers.h
View File

@@ -37,6 +37,8 @@ reduceOffer(auto const& amount)
enum class IsDeposit : bool { No = false, Yes = true };
inline Number const kAMMInvariantRelativeTolerance{1, -11};
/** Calculate LP Tokens given AMM pool reserves.
* @param asset1 AMM one side of the pool reserve
* @param asset2 AMM another side of the pool reserve
@@ -738,6 +740,30 @@ ammPoolHolds(
AuthHandling authHandling,
beast::Journal const j);
/** Check AMM pool product invariant after an AMM operation that changes LP tokens
* (deposit/withdraw/clawback) from an already calculated pool product mean.
* Returns tecPRECISION_LOSS if poolProductMean < newLPTokenBalance beyond the
* invariant tolerance,
* tesSUCCESS otherwise. Skips check when newLPTokenBalance is zero (last withdrawal).
*/
TER
checkAMMPrecisionLoss(Number const& poolProductMean, STAmount const& newLPTokenBalance);
/** Check AMM pool product invariant after an AMM operation that changes LP tokens
* (deposit/withdraw/clawback).
* Returns tecPRECISION_LOSS if sqrt(asset1 * asset2) < newLPTokenBalance beyond
* the invariant tolerance,
* tesSUCCESS otherwise. Skips check when newLPTokenBalance is zero (last withdrawal).
*/
TER
checkAMMPrecisionLoss(
ReadView const& view,
AccountID const& ammAccountID,
Asset const& asset1,
Asset const& asset2,
STAmount const& newLPTokenBalance,
beast::Journal const j);
/** Get AMM pool and LP token balances. If both optIssue are
* provided then they are used as the AMM token pair issues.
* Otherwise the missing issues are fetched from ammSle.

37
src/libxrpl/ledger/helpers/AMMHelpers.cpp
View File

@@ -433,6 +433,43 @@ ammPoolHolds(
return std::make_pair(assetInBalance, assetOutBalance);
}
TER
checkAMMPrecisionLoss(Number const& poolProductMean, STAmount const& newLPTokenBalance)
{
if (newLPTokenBalance <= beast::kZero)
return tesSUCCESS;
if (poolProductMean >= newLPTokenBalance)
return tesSUCCESS;
// Strong check failed. Allow the same relative tolerance as the invariant
// checker's weak check. Only return tecPRECISION_LOSS when both fail.
if (withinRelativeDistance(
poolProductMean, Number{newLPTokenBalance}, kAMMInvariantRelativeTolerance))
return tesSUCCESS;
return tecPRECISION_LOSS;
}
TER
checkAMMPrecisionLoss(
ReadView const& view,
AccountID const& ammAccountID,
Asset const& asset1,
Asset const& asset2,
STAmount const& newLPTokenBalance,
beast::Journal const j)
{
if (newLPTokenBalance <= beast::kZero)
return tesSUCCESS;
auto const [amount, amount2] = ammPoolHolds(
view,
ammAccountID,
asset1,
asset2,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j);
return checkAMMPrecisionLoss(root2(amount * amount2), newLPTokenBalance);
}
std::expected<std::tuple<STAmount, STAmount, STAmount>, TER>
ammHolds(
ReadView const& view,

9
src/libxrpl/tx/invariants/AMMInvariant.cpp
View File

@@ -270,13 +270,8 @@ ValidAMM::generalInvariant(
auto const poolProductMean = root2(amount * amount2);
bool const nonNegativeBalances =
validBalances(amount, amount2, *lptAMMBalanceAfter_, zeroAllowed);
bool const strongInvariantCheck = poolProductMean >= *lptAMMBalanceAfter_;
// Allow for a small relative error if strongInvariantCheck fails
auto weakInvariantCheck = [&]() {
return *lptAMMBalanceAfter_ != beast::kZero &&
withinRelativeDistance(poolProductMean, Number{*lptAMMBalanceAfter_}, Number{1, -11});
};
if (!nonNegativeBalances || (!strongInvariantCheck && !weakInvariantCheck()))
auto const precisionLoss = checkAMMPrecisionLoss(poolProductMean, *lptAMMBalanceAfter_);
if (!nonNegativeBalances || !isTesSuccess(precisionLoss))
{
JLOG(j.error()) << "Invariant failed: AMM " << tx.getTxnType() << " "
<< tx.getHash(HashPrefix::TransactionId) << " " << ammPoolChanged_ << " "

10
src/libxrpl/tx/transactors/dex/AMMClawback.cpp
View File

@@ -258,6 +258,16 @@ AMMClawback::applyGuts(Sandbox& sb)
if (!isTesSuccess(result))
return result; // LCOV_EXCL_LINE
if (sb.rules().enabled(fixCleanup3_3_0) && sb.rules().enabled(fixAMMv1_3))
{
if (auto const ter =
checkAMMPrecisionLoss(sb, ammAccount, asset, asset2, newLPTokenBalance, j_);
!isTesSuccess(ter))
{
return ter;
}
}
auto const res =
AMMWithdraw::deleteAMMAccountIfEmpty(sb, ammSle, newLPTokenBalance, asset, asset2, j_);
if (!res.second)

13
src/libxrpl/tx/transactors/dex/AMMDeposit.cpp
View File

@@ -470,6 +470,19 @@ AMMDeposit::applyGuts(Sandbox& sb)
XRPL_ASSERT(
newLPTokenBalance > beast::kZero,
"xrpl::AMMDeposit::applyGuts : valid new LP token balance");
// Defensive check: deposit formulas with fixAMMv1_3 round LP tokens
// down and asset amounts up, so sqrt(pool1*pool2) >= newLPTokenBalance
// is guaranteed to hold. A precision loss failure is not expected.
if (sb.rules().enabled(fixCleanup3_3_0) && sb.rules().enabled(fixAMMv1_3))
{
if (auto const ter = checkAMMPrecisionLoss(
sb, ammAccountID, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], newLPTokenBalance, j_);
!isTesSuccess(ter))
{
UNREACHABLE("xrpl::AMMDeposit::applyGuts : AMM precision loss");
return {ter, false}; // LCOV_EXCL_LINE
}
}
ammSle->setFieldAmount(sfLPTokenBalance, newLPTokenBalance);
// LP depositing into AMM empty state gets the auction slot
// and the voting

10
src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp
View File

@@ -406,6 +406,16 @@ AMMWithdraw::applyGuts(Sandbox& sb)
if (!isTesSuccess(result))
return {result, false};
if (sb.rules().enabled(fixCleanup3_3_0) && sb.rules().enabled(fixAMMv1_3))
{
if (auto const ter = checkAMMPrecisionLoss(
sb, ammAccountID, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], newLPTokenBalance, j_);
!isTesSuccess(ter))
{
return {ter, false};
}
}
auto const res = deleteAMMAccountIfEmpty(
sb, ammSle, newLPTokenBalance, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_);
// LCOV_EXCL_START

18
src/test/app/AMMClawback_test.cpp
View File

@@ -2486,8 +2486,17 @@ class AMMClawback_test : public beast::unit_test::Suite
else if (!features[fixAMMClawbackRounding])
{
// sqrt(amount * amount2) >= LPTokens and exceeds the allowed
// tolerance
env(amm::ammClawback(gw, alice, usd, eur, usd(1)), Ter(tecINVARIANT_FAILED));
// tolerance.
// With fixCleanup3_3_0 this is caught in the transaction layer;
// without it the invariant checker fires instead.
if (features[fixCleanup3_3_0])
{
env(amm::ammClawback(gw, alice, usd, eur, usd(1)), Ter(tecPRECISION_LOSS));
}
else
{
env(amm::ammClawback(gw, alice, usd, eur, usd(1)), Ter(tecINVARIANT_FAILED));
}
BEAST_EXPECT(amm.ammExists());
}
else if (features[fixAMMv1_3] && features[fixAMMClawbackRounding])
@@ -2514,6 +2523,11 @@ class AMMClawback_test : public beast::unit_test::Suite
testFeatureDisabled(all - featureAMMClawback);
for (auto const& features :
{all - fixAMMv1_3 - fixAMMClawbackRounding - featureMPTokensV2,
// fixAMMv1_3 on, fixAMMClawbackRounding off, fixCleanup3_3_0 off:
// precision loss caught by invariant checker -> tecINVARIANT_FAILED
all - fixAMMClawbackRounding - fixCleanup3_3_0 - featureMPTokensV2,
// fixAMMv1_3 on, fixAMMClawbackRounding off, fixCleanup3_3_0 on:
// precision loss caught in transaction layer -> tecPRECISION_LOSS
all - fixAMMClawbackRounding - featureMPTokensV2,
all - featureMPTokensV2,
all})

16
src/test/app/AMM_test.cpp
View File

@@ -1842,8 +1842,18 @@ private:
// are rounded to all LP tokens.
testAMM(
[&](AMM& ammAlice, Env& env) {
auto const err =
env.enabled(fixAMMv1_3) ? Ter(tecINVARIANT_FAILED) : Ter(tecAMM_BALANCE);
// Without fixAMMv1_3: sub-method returns tecAMM_BALANCE early.
// With fixAMMv1_3 but without fixCleanup3_3_0: sub-method succeeds
// but invariant check catches the precision violation.
// With fixCleanup3_3_0: caught in the transaction layer before
// the invariant checker runs.
auto const err = [&] {
if (!env.enabled(fixAMMv1_3))
return Ter(tecAMM_BALANCE);
if (env.enabled(fixCleanup3_3_0))
return Ter(tecPRECISION_LOSS);
return Ter(tecINVARIANT_FAILED);
}();
ammAlice.withdraw(
alice_,
STAmount{USD, UINT64_C(9'999'999999999999), -12},
@@ -1851,7 +1861,7 @@ private:
std::nullopt,
err);
},
{.features = {all, all - fixAMMv1_3}, .noLog = true});
{.features = {all, all - fixAMMv1_3, all - fixCleanup3_3_0}, .noLog = true});
// Tiny withdraw
testAMM([&](AMM& ammAlice, Env&) {