From c35f05631b6bf2b98946ec0085eaf9bc20f426e4 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Mon, 18 Nov 2024 09:35:17 -0500 Subject: [PATCH] Add lsf[High/Low]AMMNode to indicate which side of trustline is AMM. --- include/xrpl/protocol/LedgerFormats.h | 2 + src/xrpld/app/tx/detail/AMMCreate.cpp | 11 +- src/xrpld/app/tx/detail/InvariantCheck.cpp | 165 +++++++++------------ src/xrpld/ledger/detail/View.cpp | 17 +++ 4 files changed, 102 insertions(+), 93 deletions(-) diff --git a/include/xrpl/protocol/LedgerFormats.h b/include/xrpl/protocol/LedgerFormats.h index 4f3eef4919d..f8429fe78e4 100644 --- a/include/xrpl/protocol/LedgerFormats.h +++ b/include/xrpl/protocol/LedgerFormats.h @@ -164,6 +164,8 @@ enum LedgerSpecificFlags { lsfHighFreeze = 0x00800000, // True, high side has set freeze flag lsfAMMNode = 0x01000000, // True, trust line to AMM. Used by client // apps to identify payments via AMM. + lsfLowAMMNode = 0x02000000, // True, the low side is AMM + lsfHighAMMNode = 0x04000000, // True, the high side is AMM // ltSIGNER_LIST lsfOneOwnerCount = 0x00010000, // True, uses only one OwnerCount diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index 31773166d4a..957e8652af5 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -327,8 +327,15 @@ applyCreate( return tecINTERNAL; else { - auto const flags = sleRippleState->getFlags(); - sleRippleState->setFieldU32(sfFlags, flags | lsfAMMNode); + std::uint32_t const flags = [&] { + auto const flags = sleRippleState->getFlags() | lsfAMMNode; + if (!sb.rules().enabled(fixAMMv1_3)) + return flags; + if (*ammAccount > amount.get().account) + return flags | lsfHighAMMNode; + return flags | lsfLowAMMNode; + }(); + sleRippleState->setFieldU32(sfFlags, flags); sb.update(sleRippleState); } } diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index ebb2b30b077..adfb24f7cd9 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1132,31 +1132,29 @@ ValidAMM::visitEntry( return; using Balance = hash_map>; - auto set = [&](SLE const& sle, - Balance& balance, - STAmount const& a1, - STAmount const& a2) { - STAmount assetBalance = sle[sfBalance]; - // don't know account/issuer - if (assetBalance.negative()) - assetBalance.negate(); - // Don't know which side of the trustline is AMM so save both. - // finalize() does lookup to figure out the AMM side. - balance[a1.getIssuer()][a2.get()] = assetBalance; - balance[a2.getIssuer()][a1.get()] = assetBalance; - }; auto update = [&](SLE const& sle, Balance& balance) { auto const leType = sle.getType(); + auto const flags = sle.getFlags(); if (leType == ltACCOUNT_ROOT && sle.isFieldPresent(sfAMMID)) { balance[sle[sfAccount]][xrpIssue()] = sle[sfBalance]; } - else if (leType == ltRIPPLE_STATE && (sle.getFlags() & lsfAMMNode)) + else if ( + leType == ltRIPPLE_STATE && + (flags & (lsfLowAMMNode | lsfHighAMMNode))) { + STAmount assetBalance = sle[sfBalance]; auto const& highLimit = sle.getFieldAmount(sfHighLimit); auto const& lowLimit = sle.getFieldAmount(sfLowLimit); - set(sle, balance, highLimit, lowLimit); - set(sle, balance, lowLimit, highLimit); + if (flags & lsfHighAMMNode) + { + assetBalance.negate(); + balance[highLimit.getIssuer()][lowLimit.get()] = + assetBalance; + } + else + balance[lowLimit.getIssuer()][highLimit.get()] = + assetBalance; } }; @@ -1260,89 +1258,74 @@ ValidAMM::finalize( txType == ttPAYMENT || txType == ttOFFER_CREATE || txType == ttCHECK_CASH) { - if (balanceBefore_.size() != balanceAfter_.size()) - return false; - for (auto const& [k, v] : balanceBefore_) + for (auto const& [ammAccount, assets] : balanceBefore_) { - if (auto const root = _view.read(keylet::account(k)); - root && root->isFieldPresent(sfAMMID)) + static Issue iss{}; + auto it = assets.cbegin(); + auto const& asset = it->first; + auto const& asset2 = (++it) != assets.end() ? it->first : iss; + // AMM must have two balances - one for each pool side + if (assets.size() != 2 || !balanceAfter_.contains(ammAccount) || + !balanceAfter_[ammAccount].contains(asset) || + !balanceAfter_[ammAccount].contains(asset2)) { - std::uint16_t tfee = 0; - STAmount lptAMMBalance{}; - if (auto const amm = - _view.read(keylet::amm((*root)[sfAMMID])); - amm && (*amm)[sfLPTokenBalance] <= beast::zero) - { - // LCOV_EXCL_START - JLOG(j.error()) - << "AMM swap invariant failed: invalid token " - "balance " - << to_string(k) << " " << (*amm)[sfLPTokenBalance]; - return false; - // LCOV_EXCL_STOP - } - else - { - tfee = (*amm)[sfTradingFee]; - lptAMMBalance = (*amm)[sfLPTokenBalance]; - } - // AMM must have two balances - one for each pool side - if (!balanceAfter_.contains(k) || v.size() != 2) - { - JLOG(j.error()) - << "AMM swap invariant failed: inconsistent assets " - << to_string(k) << " " << v.size(); - return false; - } - auto it = v.begin(); - auto const asset = it->first; - auto const asset2 = (++it)->first; - if (!balanceAfter_[k].contains(asset) || - !balanceAfter_[k].contains(asset2)) - { - JLOG(j.error()) - << "AMM swap invariant failed: inconsistent assets " - << to_string(k) << " " << asset << " " << asset2; - return false; - } - auto const ammKeylet = keylet::amm(asset, asset2); - if (_view.exists(ammKeylet)) + JLOG(j.error()) + << "AMM swap invariant failed: inconsistent assets " + << to_string(ammAccount) << " " << assets.size() << " " + << asset << " " << asset2; + return false; + } + if (auto const amm = _view.read(keylet::amm(asset, asset2))) + { + auto const lptAMMBalance = (*amm)[sfLPTokenBalance]; + auto const tfee = (*amm)[sfTradingFee]; + Number const productBefore = + balanceBefore_[ammAccount][asset] * + balanceBefore_[ammAccount][asset2]; + auto const amount = balanceAfter_[ammAccount][asset]; + auto const amount2 = balanceAfter_[ammAccount][asset2]; + Number const productAfter = amount * amount2; + if (positiveBalances( + amount, amount2, lptAMMBalance, false) && + (productAfter >= productBefore || + withinRelativeDistance( + productBefore, productAfter, Number{1, -7}))) { - Number const productBefore = balanceBefore_[k][asset] * - balanceBefore_[k][asset2]; - auto const amount = balanceAfter_[k][asset]; - auto const amount2 = balanceAfter_[k][asset2]; - Number const productAfter = amount * amount2; - if (positiveBalances( - amount, amount2, lptAMMBalance, false) && - (productAfter >= productBefore || - withinRelativeDistance( - productBefore, productAfter, Number{1, -7}))) - return true; - - JLOG(j.error()) - << "AMM swap invariant failed: old balances " - << balanceBefore_[k][asset].getText() << " " - << balanceBefore_[k][asset2].getText() - << " new balances " - << balanceAfter_[k][asset].getText() << " " - << balanceAfter_[k][asset2].getText() - << " lptAMMBalance " << lptAMMBalance << " tfee " - << tfee << " old/new product " << productBefore - << " " << productAfter << " diff " - << (productBefore != Number{0} - ? to_string( - (productBefore - productAfter) / - productBefore) - : "undefined") - << std::endl; - return false; + balanceAfter_.erase(ammAccount); + continue; } + + JLOG(j.error()) + << "AMM swap invariant failed: old balances " + << balanceBefore_[ammAccount][asset].getText() << " " + << balanceBefore_[ammAccount][asset2].getText() + << " new balances " + << balanceAfter_[ammAccount][asset].getText() << " " + << balanceAfter_[ammAccount][asset2].getText() + << " lptAMMBalance " << lptAMMBalance << " tfee " + << tfee << " diff " + << (productBefore != Number{0} + ? to_string( + (productBefore - productAfter) / + productBefore) + : "undefined") + << std::endl; + return false; + } + else + { + JLOG(j.error()) + << "AMM swap invariant failed: amm is not found " + << asset << " " << asset2; + return false; } - balanceAfter_.erase(k); } if (!balanceAfter_.empty()) + { + JLOG(j.error()) + << "AMM swap invariant failed: inconsistent before/after"; return false; + } } } diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index ae4eb095017..b27a89e3baa 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -1144,6 +1144,23 @@ rippleCreditIOU( j); } + if (view.rules().enabled(fixAMMv1_3) && (uFlags & lsfAMMNode) && + !(uFlags & (lsfHighAMMNode | lsfLowAMMNode))) + { + auto const sleSender = view.read(keylet::account(uSenderID)); + if (!sleSender) + return tefINTERNAL; + if (sleSender->isFieldPresent(sfAMMID)) + { + auto const flags = sleRippleState->getFlags(); + if (uSenderID > uReceiverID) + sleRippleState->setFieldU32( + sfFlags, flags | lsfHighAMMNode); + else + sleRippleState->setFieldU32(sfFlags, flags | lsfLowAMMNode); + } + } + view.update(sleRippleState); return tesSUCCESS; }