From 9b30183e943e34ffcaf606b12461cf17945a273f Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Mon, 23 Sep 2024 17:14:42 -0400 Subject: [PATCH 01/13] Add AMMClawback Transaction --- include/xrpl/protocol/Feature.h | 3 +- include/xrpl/protocol/SField.h | 2 + include/xrpl/protocol/STObject.h | 2 + include/xrpl/protocol/TER.h | 2 + include/xrpl/protocol/TxFlags.h | 4 + include/xrpl/protocol/TxFormats.h | 3 + include/xrpl/protocol/jss.h | 3 + src/libxrpl/protocol/Feature.cpp | 1 + src/libxrpl/protocol/SField.cpp | 2 + src/libxrpl/protocol/STObject.cpp | 7 + src/libxrpl/protocol/TER.cpp | 2 + src/libxrpl/protocol/TxFormats.cpp | 10 + src/test/app/AMMClawback_test.cpp | 1587 ++++++++++++++++++++ src/test/app/AMM_test.cpp | 310 +++- src/test/jtx/impl/trust.cpp | 22 + src/test/jtx/trust.h | 9 + src/xrpld/app/misc/AMMUtils.h | 25 + src/xrpld/app/misc/detail/AMMUtils.cpp | 202 +++ src/xrpld/app/tx/detail/AMMClawback.cpp | 323 ++++ src/xrpld/app/tx/detail/AMMClawback.h | 75 + src/xrpld/app/tx/detail/AMMCreate.cpp | 8 +- src/xrpld/app/tx/detail/AMMDeposit.cpp | 30 + src/xrpld/app/tx/detail/AMMWithdraw.cpp | 374 ++--- src/xrpld/app/tx/detail/AMMWithdraw.h | 44 +- src/xrpld/app/tx/detail/InvariantCheck.cpp | 7 +- src/xrpld/app/tx/detail/applySteps.cpp | 3 + 26 files changed, 2736 insertions(+), 324 deletions(-) create mode 100644 src/test/app/AMMClawback_test.cpp create mode 100644 src/xrpld/app/tx/detail/AMMClawback.cpp create mode 100644 src/xrpld/app/tx/detail/AMMClawback.h diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index a00d6b85c1b..f2c5950829d 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -80,7 +80,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 79; +static constexpr std::size_t numFeatures = 80; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -372,6 +372,7 @@ extern uint256 const fixEnforceNFTokenTrustline; extern uint256 const fixInnerObjTemplate2; extern uint256 const featureInvariantsV1_1; extern uint256 const fixNFTokenPageLinks; +extern uint256 const featureAMMClawback; } // namespace ripple diff --git a/include/xrpl/protocol/SField.h b/include/xrpl/protocol/SField.h index 7f54201a4b8..5b505fc3399 100644 --- a/include/xrpl/protocol/SField.h +++ b/include/xrpl/protocol/SField.h @@ -587,6 +587,8 @@ extern SF_ACCOUNT const sfUnauthorize; extern SF_ACCOUNT const sfRegularKey; extern SF_ACCOUNT const sfNFTokenMinter; extern SF_ACCOUNT const sfEmitCallback; +extern SF_ACCOUNT const sfHolder; +extern SF_ACCOUNT const sfAMMAccount; // account (uncommon) extern SF_ACCOUNT const sfHookAccount; diff --git a/include/xrpl/protocol/STObject.h b/include/xrpl/protocol/STObject.h index b3cef83de5f..f66ef8e6aed 100644 --- a/include/xrpl/protocol/STObject.h +++ b/include/xrpl/protocol/STObject.h @@ -243,6 +243,8 @@ class STObject : public STBase, public CountedObject getFieldArray(SField const& field) const; const STCurrency& getFieldCurrency(SField const& field) const; + const STIssue& + getFieldIssue(SField const& field) const; /** Get the value of a field. @param A TypedField built from an SField value representing the desired diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index aae3c7107bd..afc12ab9a0d 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -139,6 +139,8 @@ enum TEMcodes : TERUnderlyingType { temARRAY_EMPTY, temARRAY_TOO_LARGE, + temBAD_ASSET_AMOUNT, + temBAD_ASSET_ISSUER }; //------------------------------------------------------------------------------ diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index ba2b97562db..4cf04f4f42a 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/include/xrpl/protocol/TxFlags.h @@ -181,6 +181,10 @@ constexpr std::uint32_t tfDepositSubTx = constexpr std::uint32_t tfWithdrawMask = ~(tfUniversal | tfWithdrawSubTx); constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfDepositSubTx); +// AMMClawback flags: +constexpr std::uint32_t tfClawTwoAssets = 0x00000001; +constexpr std::uint32_t tfAMMClawbackMask = ~(tfUniversal | tfClawTwoAssets); + // BridgeModify flags: constexpr std::uint32_t tfClearAccountCreateAmount = 0x00010000; constexpr std::uint32_t tfBridgeModifyMask = ~(tfUniversal | tfClearAccountCreateAmount); diff --git a/include/xrpl/protocol/TxFormats.h b/include/xrpl/protocol/TxFormats.h index a3f5cca108c..e113e30b7f9 100644 --- a/include/xrpl/protocol/TxFormats.h +++ b/include/xrpl/protocol/TxFormats.h @@ -142,6 +142,9 @@ enum TxType : std::uint16_t /** This transaction claws back issued tokens. */ ttCLAWBACK = 30, + /** This transaction claws back tokens from an AMM pool. */ + ttAMM_CLAWBACK = 31, + /** This transaction type creates an AMM instance */ ttAMM_CREATE = 35, diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index e3eda80b44f..f56858a161a 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -48,7 +48,9 @@ JSS(AccountDelete); // transaction type. JSS(AccountRoot); // ledger type. JSS(AccountSet); // transaction type. JSS(AMM); // ledger type +JSS(AMMAccount); // field JSS(AMMBid); // transaction type +JSS(AMMClawback); // transaction type. JSS(AMMID); // field JSS(AMMCreate); // transaction type JSS(AMMDeposit); // transaction type @@ -89,6 +91,7 @@ JSS(EscrowFinish); // transaction type. JSS(Fee); // in/out: TransactionSign; field. JSS(FeeSettings); // ledger type. JSS(Flags); // in/out: TransactionSign; field. +JSS(Holder); // field. JSS(Invalid); // JSS(LastLedgerSequence); // in: TransactionSign; field JSS(LastUpdateTime); // field. diff --git a/src/libxrpl/protocol/Feature.cpp b/src/libxrpl/protocol/Feature.cpp index 078369bf20c..b350c1269ec 100644 --- a/src/libxrpl/protocol/Feature.cpp +++ b/src/libxrpl/protocol/Feature.cpp @@ -501,6 +501,7 @@ REGISTER_FIX (fixNFTokenPageLinks, Supported::yes, VoteBehavior::De // InvariantsV1_1 will be changes to Supported::yes when all the // invariants expected to be included under it are complete. REGISTER_FEATURE(InvariantsV1_1, Supported::no, VoteBehavior::DefaultNo); +REGISTER_FEATURE(AMMClawback, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/libxrpl/protocol/SField.cpp b/src/libxrpl/protocol/SField.cpp index f8eb2d6f877..1846414039f 100644 --- a/src/libxrpl/protocol/SField.cpp +++ b/src/libxrpl/protocol/SField.cpp @@ -319,6 +319,8 @@ CONSTRUCT_TYPED_SFIELD(sfUnauthorize, "Unauthorize", ACCOUNT, CONSTRUCT_TYPED_SFIELD(sfRegularKey, "RegularKey", ACCOUNT, 8); CONSTRUCT_TYPED_SFIELD(sfNFTokenMinter, "NFTokenMinter", ACCOUNT, 9); CONSTRUCT_TYPED_SFIELD(sfEmitCallback, "EmitCallback", ACCOUNT, 10); +CONSTRUCT_TYPED_SFIELD(sfHolder, "Holder", ACCOUNT, 11); +CONSTRUCT_TYPED_SFIELD(sfAMMAccount, "AMMAccount", ACCOUNT, 12); // account (uncommon) CONSTRUCT_TYPED_SFIELD(sfHookAccount, "HookAccount", ACCOUNT, 16); diff --git a/src/libxrpl/protocol/STObject.cpp b/src/libxrpl/protocol/STObject.cpp index bde83ec31a1..9b24d9866bc 100644 --- a/src/libxrpl/protocol/STObject.cpp +++ b/src/libxrpl/protocol/STObject.cpp @@ -659,6 +659,13 @@ STObject::getFieldCurrency(SField const& field) const return getFieldByConstRef(field, empty); } +STIssue const& +STObject::getFieldIssue(SField const& field) const +{ + static STIssue const empty{}; + return getFieldByConstRef(field, empty); +} + void STObject::set(std::unique_ptr v) { diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index 917bbf26a9f..5faf2479d83 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/src/libxrpl/protocol/TER.cpp @@ -205,6 +205,8 @@ transResults() MAKE_ERROR(temXCHAIN_BRIDGE_BAD_REWARD_AMOUNT, "Malformed: Bad reward amount."), MAKE_ERROR(temARRAY_EMPTY, "Malformed: Array is empty."), MAKE_ERROR(temARRAY_TOO_LARGE, "Malformed: Array is too large."), + MAKE_ERROR(temBAD_ASSET_AMOUNT, "Malformed: Amount does not match Asset."), + MAKE_ERROR(temBAD_ASSET_ISSUER, "Malformed: Issuer does not match Asset."), MAKE_ERROR(terRETRY, "Retry transaction."), MAKE_ERROR(terFUNDS_SPENT, "DEPRECATED."), diff --git a/src/libxrpl/protocol/TxFormats.cpp b/src/libxrpl/protocol/TxFormats.cpp index 8a93232604e..2d3732c265a 100644 --- a/src/libxrpl/protocol/TxFormats.cpp +++ b/src/libxrpl/protocol/TxFormats.cpp @@ -381,6 +381,16 @@ TxFormats::TxFormats() }, commonFields); + add(jss::AMMClawback, + ttAMM_CLAWBACK, + { + {sfHolder, soeREQUIRED}, + {sfAMMAccount, soeREQUIRED}, + {sfAsset, soeREQUIRED}, + {sfAmount, soeOPTIONAL}, + }, + commonFields); + add(jss::XChainCreateBridge, ttXCHAIN_CREATE_BRIDGE, { diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp new file mode 100644 index 00000000000..67ca3f6a577 --- /dev/null +++ b/src/test/app/AMMClawback_test.cpp @@ -0,0 +1,1587 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +namespace ripple { +namespace test { +class AMMClawback_test : public jtx::AMMTest +{ + void + testInvalidRequest(FeatureBitset features) + { + testcase("test invalid request"); + using namespace jtx; + + // Test if the AMMAccount provided does not exist at all. This should + // return terNO_AMM error. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(100000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(10000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + // Withdraw all the tokens from the AMMAccount. + // The AMMAccount will be auto deleted. + AMM amm(env, gw, XRP(100), USD(100)); + amm.withdrawAll(gw); + BEAST_EXPECT(!amm.ammExists()); + env.close(); + + // The AMM account does not exist at all now. + // It should return terNO_AMM error. + env(ammClawback( + gw, + alice, + USD, + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(terNO_AMM)); + } + + // Test if the AMMAccount provided exists, but it is not an AMM account. + // This should return terNO_AMM error. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // gw sends invalid request + // by passing alice account as AMMAccount. This should return + // terNO_AMM. + env(ammClawback( + gw, alice, USD, std::nullopt, alice.id(), std::nullopt), + ter(terNO_AMM)); + } + + // Test if the issuer field and holder field is the same. This should + // return temMALFORMED error. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // Issuer can not clawback from himself. + env(ammClawback( + gw, gw, USD, std::nullopt, amm.ammAccount(), std::nullopt), + ter(temMALFORMED)); + } + + // Test if the Asset field matches the Account field. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // The Asset's issuer field is alice, while the Account field is gw. + // This should return temBAD_ASSET_ISSUER because they do not match. + env(ammClawback( + gw, + alice, + Issue{gw["USD"].currency, alice.id()}, + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(temBAD_ASSET_ISSUER)); + } + + // Test if the Amount field matches the Asset field. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // The Asset's issuer subfield is gw account and Amount's issuer + // subfield is alice account. Return temBAD_ASSET_AMOUNT because + // they do not match. + env(ammClawback( + gw, + alice, + USD, + STAmount{Issue{gw["USD"].currency, alice.id()}, 1}, + amm.ammAccount(), + std::nullopt), + ter(temBAD_ASSET_AMOUNT)); + } + + // Test if the Amount is invalid, which is less than zero. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // Return temBAD_AMOUNT if the Amount value is less than 0. + env(ammClawback( + gw, + alice, + USD, + STAmount{Issue{gw["USD"].currency, gw.id()}, -1}, + amm.ammAccount(), + std::nullopt), + ter(temBAD_AMOUNT)); + + // Return temBAD_AMOUNT if the Amount value is 0. + env(ammClawback( + gw, + alice, + USD, + STAmount{Issue{gw["USD"].currency, gw.id()}, 0}, + amm.ammAccount(), + std::nullopt), + ter(temBAD_AMOUNT)); + } + + // Test if the issuer did not set asfAllowTrustLineClawback, AMMClawback + // transaction is prohibited. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + env.require(balance(alice, gw["USD"](100))); + env.require(balance(gw, alice["USD"](-100))); + + // gw creates AMM pool of XRP/USD. + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // If asfAllowTrustLineClawback is not set, the issuer is not + // allowed to send the AMMClawback transaction. + env(ammClawback( + gw, + alice, + USD, + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(tecNO_PERMISSION)); + } + + // Test if the Asset being clawed back does not exist in the AMM pool. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + // create AMM pool of XRP/USD + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // Return tecNO_PERMISSION because the Asset EUR does not + // match AMM pool assets XRP/USD. + env(ammClawback( + gw, + alice, + gw["EUR"], + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(tecNO_PERMISSION)); + } + + // Test invalid flag. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // Return temINVALID_FLAG when providing invalid flag. + env(ammClawback( + gw, + alice, + USD, + std::nullopt, + amm.ammAccount(), + tfTwoAssetIfEmpty), + ter(temINVALID_FLAG)); + } + + // Test if tfClawTwoAssets is set when the two assets in the AMM pool + // are not issued by the same issuer. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(10000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 100 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(100))); + env.close(); + + // gw creates AMM pool of XRP/USD. + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + + // Return tecNO_PERMISSION because the issuer set tfClawTwoAssets, + // but the issuer only issues USD in the pool. The issuer is not + // allowed to set tfClawTwoAssets flag if he did not issue both + // assts in the pool. + env(ammClawback( + gw, + alice, + USD, + std::nullopt, + amm.ammAccount(), + tfClawTwoAssets), + ter(tecNO_PERMISSION)); + } + + // Test clawing back XRP is being prohibited. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 3000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(3000))); + env.close(); + + // Alice creates AMM pool of XRP/USD. + AMM amm(env, alice, XRP(1000), USD(2000), ter(tesSUCCESS)); + env.close(); + + // Clawback XRP is prohibited. + env(ammClawback( + gw, + alice, + XRP, + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(temMALFORMED)); + } + } + + void + testFeatureDisabled(FeatureBitset features) + { + testcase("test featureAMMClawback is not enabled."); + using namespace jtx; + if (!features[featureAMMClawback]) + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 3000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(3000))); + env.close(); + + // When featureAMMClawback is not enabled, AMMClawback is disabled. + // Because when featureAMMClawback is disabled, we can not create + // amm account, use gw account for now for testing purpose. + env(ammClawback( + gw, alice, XRP, std::nullopt, gw.id(), std::nullopt), + ter(temDISABLED)); + } + } + + void + testAMMClawbackSpecificAmount(FeatureBitset features) + { + testcase("test AMMClawback specific amount"); + using namespace jtx; + + // Test AMMClawback for USD/EUR pool. The assets are issued by different + // issuer. Claw back USD, and EUR goes back to the holder. + { + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 3000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(3000))); + env.close(); + env.require(balance(gw, alice["USD"](-3000))); + env.require(balance(alice, gw["USD"](3000))); + + // gw2 issues 3000 EUR to Alice. + auto const EUR = gw2["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw2, alice, EUR(3000))); + env.close(); + env.require(balance(gw2, alice["EUR"](-3000))); + env.require(balance(alice, gw2["EUR"](3000))); + + // Alice creates AMM pool of EUR/USD. + AMM amm(env, alice, EUR(1000), USD(2000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + USD(2000), EUR(1000), IOUAmount{1414213562373095, -12})); + + // gw clawback 1000 USD from the AMM pool. + env(ammClawback( + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Alice's initial balance for USD is 3000 USD. Alice deposited 2000 + // USD into the pool, then she has 1000 USD. And 1000 USD was clawed + // back from the AMM pool, so she still has 1000 USD. + env.require(balance(gw, alice["USD"](-1000))); + env.require(balance(alice, gw["USD"](1000))); + + // Alice's initial balance for EUR is 3000 EUR. Alice deposited 1000 + // EUR into the pool, 500 EUR was withdrawn proportionally. So she + // has 2500 EUR now. + env.require(balance(gw2, alice["EUR"](-2500))); + env.require(balance(alice, gw2["EUR"](2500))); + + // 1000 USD and 500 EUR was withdrawn from the AMM pool, so the + // current balance is 1000 USD and 500 EUR. + BEAST_EXPECT(amm.expectBalances( + USD(1000), EUR(500), IOUAmount{7071067811865475, -13})); + + // Alice has half of its initial lptokens Left. + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13})); + + // gw clawback another 1000 USD from the AMM pool. The AMM pool will + // be empty and get deleted. + env(ammClawback( + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Alice should still has 1000 USD because gw clawed back from the + // AMM pool. + env.require(balance(gw, alice["USD"](-1000))); + env.require(balance(alice, gw["USD"](1000))); + + // Alice should has 3000 EUR now because another 500 EUR was + // withdrawn. + env.require(balance(gw2, alice["EUR"](-3000))); + env.require(balance(alice, gw2["EUR"](3000))); + + // amm is automatically deleted. + BEAST_EXPECT(!amm.ammExists()); + } + + // Test AMMClawback for USD/XRP pool. Claw back USD, and XRP goes back + // to the holder. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 3000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(3000))); + env.close(); + env.require(balance(gw, alice["USD"](-3000))); + env.require(balance(alice, gw["USD"](3000))); + + // Alice creates AMM pool of XRP/USD. + AMM amm(env, alice, XRP(1000), USD(2000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + USD(2000), XRP(1000), IOUAmount{1414213562373095, -9})); + + auto aliceXrpBalance = env.balance(alice, XRP); + + // gw clawback 1000 USD from the AMM pool. + env(ammClawback( + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Alice's initial balance for USD is 3000 USD. Alice deposited 2000 + // USD into the pool, then she has 1000 USD. And 1000 USD was clawed + // back from the AMM pool, so she still has 1000 USD. + env.require(balance(gw, alice["USD"](-1000))); + env.require(balance(alice, gw["USD"](1000))); + + // Alice will get 500 XRP back. + BEAST_EXPECT( + expectLedgerEntryRoot(env, alice, aliceXrpBalance + XRP(500))); + + // 1000 USD and 500 XRP was withdrawn from the AMM pool, so the + // current balance is 1000 USD and 500 XRP. + BEAST_EXPECT(amm.expectBalances( + USD(1000), XRP(500), IOUAmount{7071067811865475, -10})); + + // Alice has half of its initial lptokens Left. + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{7071067811865475, -10})); + + // gw clawback another 1000 USD from the AMM pool. The AMM pool will + // be empty and get deleted. + env(ammClawback( + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Alice should still has 1000 USD because gw clawed back from the + // AMM pool. + env.require(balance(gw, alice["USD"](-1000))); + env.require(balance(alice, gw["USD"](1000))); + + // Alice will get another 1000 XRP back. + BEAST_EXPECT( + expectLedgerEntryRoot(env, alice, aliceXrpBalance + XRP(1000))); + + // amm is automatically deleted. + BEAST_EXPECT(!amm.ammExists()); + } + } + + void + testAMMClawbackExceedBalance(FeatureBitset features) + { + testcase( + "test AMMClawback specific amount which exceeds the current " + "balance"); + using namespace jtx; + + // Test AMMClawback for USD/EUR pool. The assets are issued by different + // issuer. Claw back USD for multiple times, and EUR goes back to the + // holder. The last AMMClawback transaction exceeds the holder's USD + // balance in AMM pool. + { + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 6000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(6000))); + env.close(); + env.require(balance(alice, gw["USD"](6000))); + + // gw2 issues 6000 EUR to Alice. + auto const EUR = gw2["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw2, alice, EUR(6000))); + env.close(); + env.require(balance(alice, gw2["EUR"](6000))); + + // Alice creates AMM pool of EUR/USD + AMM amm(env, alice, EUR(5000), USD(4000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + USD(4000), EUR(5000), IOUAmount{4472135954999580, -12})); + + // gw clawback 1000 USD from the AMM pool + env(ammClawback( + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Alice's initial balance for USD is 6000 USD. Alice deposited 4000 + // USD into the pool, then she has 2000 USD. And 1000 USD was clawed + // back from the AMM pool, so she still has 2000 USD. + env.require(balance(alice, gw["USD"](2000))); + + // Alice's initial balance for EUR is 6000 EUR. Alice deposited 5000 + // EUR into the pool, 1250 EUR was withdrawn proportionally. So she + // has 2500 EUR now. + env.require(balance(alice, gw2["EUR"](2250))); + + // 1000 USD and 1250 EUR was withdrawn from the AMM pool, so the + // current balance is 3000 USD and 3750 EUR. + BEAST_EXPECT(amm.expectBalances( + USD(3000), EUR(3750), IOUAmount{3354101966249685, -12})); + + // Alice has 3/4 of its initial lptokens Left. + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{3354101966249685, -12})); + + // gw clawback another 500 USD from the AMM pool. + env(ammClawback( + gw, alice, USD, USD(500), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Alice should still has 2000 USD because gw clawed back from the + // AMM pool. + env.require(balance(alice, gw["USD"](2000))); + + BEAST_EXPECT(amm.expectBalances( + STAmount{USD, UINT64_C(2500000000000001), -12}, + STAmount{EUR, UINT64_C(3125000000000001), -12}, + IOUAmount{2795084971874738, -12})); + + BEAST_EXPECT( + env.balance(alice, EUR) == + STAmount(EUR, UINT64_C(2874999999999999), -12)); + + // gw clawback small amount, 1 USD. + env(ammClawback( + gw, alice, USD, USD(1), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Another 1 USD / 1.25 EUR was withdrawn. + env.require(balance(alice, gw["USD"](2000))); + + BEAST_EXPECT(amm.expectBalances( + STAmount{USD, UINT64_C(2499000000000002), -12}, + STAmount{EUR, UINT64_C(3123750000000002), -12}, + IOUAmount{2793966937885989, -12})); + + BEAST_EXPECT( + env.balance(alice, EUR) == + STAmount(EUR, UINT64_C(2876249999999998), -12)); + + // gw clawback 4000 USD, exceeding the current balance. We + // will clawback all. + env(ammClawback( + gw, alice, USD, USD(4000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw["USD"](2000))); + + // All alice's EUR in the pool goes back to alice. + BEAST_EXPECT( + env.balance(alice, EUR) == + STAmount(EUR, UINT64_C(6000000000000000), -12)); + + // amm is automatically deleted. + BEAST_EXPECT(!amm.ammExists()); + } + + // Test AMMClawback for USD/XRP pool. The assets are issued by different + // issuer. Claw back USD for multiple times, and XRP goes back to the + // holder. The last AMMClawback transaction exceeds the holder's USD + // balance in AMM pool. In this case, gw creates the AMM pool USD/XRP, + // both alice and bob deposit into it. gw2 creates the AMM pool EUR/XRP. + { + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + Account bob{"bob"}; + env.fund(XRP(1000000), gw, gw2, alice, bob); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw2 sets asfAllowTrustLineClawback. + env(fset(gw2, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw2, asfAllowTrustLineClawback)); + + // gw issues 6000 USD to Alice and 5000 USD to Bob. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(6000))); + env.trust(USD(100000), bob); + env(pay(gw, bob, USD(5000))); + env.close(); + + // gw2 issues 5000 EUR to Alice and 4000 EUR to Bob. + auto const EUR = gw2["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw2, alice, EUR(5000))); + env.trust(EUR(100000), bob); + env(pay(gw2, bob, EUR(4000))); + env.close(); + + // gw creates AMM pool of XRP/USD, alice and bob deposit XRP/USD. + AMM amm(env, gw, XRP(2000), USD(1000), ter(tesSUCCESS)); + BEAST_EXPECT(amm.expectBalances( + USD(1000), XRP(2000), IOUAmount{1414213562373095, -9})); + amm.deposit(alice, USD(1000), XRP(2000)); + BEAST_EXPECT(amm.expectBalances( + USD(2000), XRP(4000), IOUAmount{2828427124746190, -9})); + amm.deposit(bob, USD(1000), XRP(2000)); + BEAST_EXPECT(amm.expectBalances( + USD(3000), XRP(6000), IOUAmount{4242640687119285, -9})); + env.close(); + + // gw2 creates AMM pool of XRP/EUR, alice and bob deposit XRP/EUR. + AMM amm2(env, gw2, XRP(3000), EUR(1000), ter(tesSUCCESS)); + BEAST_EXPECT(amm2.expectBalances( + EUR(1000), XRP(3000), IOUAmount{1732050807568878, -9})); + amm2.deposit(alice, EUR(1000), XRP(3000)); + BEAST_EXPECT(amm2.expectBalances( + EUR(2000), XRP(6000), IOUAmount{3464101615137756, -9})); + amm2.deposit(bob, EUR(1000), XRP(3000)); + BEAST_EXPECT(amm2.expectBalances( + EUR(3000), XRP(9000), IOUAmount{5196152422706634, -9})); + env.close(); + + auto aliceXrpBalance = env.balance(alice, XRP); + auto bobXrpBalance = env.balance(bob, XRP); + + // gw clawback 500 USD from alice in amm + env(ammClawback( + gw, alice, USD, USD(500), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Alice's initial balance for USD is 6000 USD. Alice deposited 1000 + // USD into the pool, then she has 5000 USD. And 500 USD was clawed + // back from the AMM pool, so she still has 5000 USD. + env.require(balance(alice, gw["USD"](5000))); + + // Bob's balance is not changed. + env.require(balance(bob, gw["USD"](4000))); + + // Alice gets 1000 XRP back. + BEAST_EXPECT( + expectLedgerEntryRoot(env, alice, aliceXrpBalance + XRP(1000))); + + BEAST_EXPECT(amm.expectBalances( + USD(2500), XRP(5000), IOUAmount{3535533905932738, -9})); + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{7071067811865480, -10})); + BEAST_EXPECT( + amm.expectLPTokens(bob, IOUAmount{1414213562373095, -9})); + + // gw clawback 10 USD from bob in amm. + env(ammClawback( + gw, bob, USD, USD(10), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw["USD"](5000))); + env.require(balance(bob, gw["USD"](4000))); + + // Bob gets 20 XRP back. + BEAST_EXPECT( + expectLedgerEntryRoot(env, bob, bobXrpBalance + XRP(20))); + BEAST_EXPECT(amm.expectBalances( + STAmount{USD, UINT64_C(2490000000000001), -12}, + XRP(4980), + IOUAmount{3521391770309008, -9})); + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{7071067811865480, -10})); + BEAST_EXPECT( + amm.expectLPTokens(bob, IOUAmount{1400071426749365, -9})); + + // gw2 clawback 200 EUR from amm2. + env(ammClawback( + gw2, alice, EUR, EUR(200), amm2.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw2["EUR"](4000))); + env.require(balance(bob, gw2["EUR"](3000))); + + // Alice gets 600 XRP back. + BEAST_EXPECT(expectLedgerEntryRoot( + env, alice, aliceXrpBalance + XRP(1000) + XRP(600))); + BEAST_EXPECT(amm2.expectBalances( + EUR(2800), XRP(8400), IOUAmount{4849742261192859, -9})); + BEAST_EXPECT( + amm2.expectLPTokens(alice, IOUAmount{1385640646055103, -9})); + BEAST_EXPECT( + amm2.expectLPTokens(bob, IOUAmount{1732050807568878, -9})); + + // gw claw back 1000 USD from alice in amm, which exceeds alice's + // balance. This will clawback all the remaining LP tokens of alice + // (corresponding 500 USD / 1000 XRP). + env(ammClawback( + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw["USD"](5000))); + env.require(balance(bob, gw["USD"](4000))); + + // Alice gets 1000 XRP back. + BEAST_EXPECT(expectLedgerEntryRoot( + env, + alice, + aliceXrpBalance + XRP(1000) + XRP(600) + XRP(1000))); + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); + BEAST_EXPECT( + amm.expectLPTokens(bob, IOUAmount{1400071426749365, -9})); + BEAST_EXPECT(amm.expectBalances( + STAmount{USD, UINT64_C(1990000000000001), -12}, + XRP(3980), + IOUAmount{2814284989122460, -9})); + + // gw clawback 1000 USD from bob in amm, which also exceeds bob's + // balance in amm. All bob's lptoken in amm will be consumed, which + // corresponds to 990 USD / 1980 XRP + env(ammClawback( + gw, bob, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw["USD"](5000))); + env.require(balance(bob, gw["USD"](4000))); + + BEAST_EXPECT(expectLedgerEntryRoot( + env, + alice, + aliceXrpBalance + XRP(1000) + XRP(600) + XRP(1000))); + BEAST_EXPECT(expectLedgerEntryRoot( + env, bob, bobXrpBalance + XRP(20) + XRP(1980))); + + // Now neither alice nor bob has any lptoken in amm. + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(0))); + + // gw2 claw back 1000 EUR from alice in amm2, which exceeds alice's + // balance. All alice's lptokens will be consumed, which corresponds + // to 800EUR / 2400 XRP. + env(ammClawback( + gw2, + alice, + EUR, + EUR(1000), + amm2.ammAccount(), + std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw2["EUR"](4000))); + env.require(balance(bob, gw2["EUR"](3000))); + + // Alice gets another 2400 XRP back, bob's XRP balance remains the + // same. + BEAST_EXPECT(expectLedgerEntryRoot( + env, + alice, + aliceXrpBalance + XRP(1000) + XRP(600) + XRP(1000) + + XRP(2400))); + BEAST_EXPECT(expectLedgerEntryRoot( + env, bob, bobXrpBalance + XRP(20) + XRP(1980))); + + // Alice now does not have any lptoken in amm2 + BEAST_EXPECT(amm2.expectLPTokens(alice, IOUAmount(0))); + + BEAST_EXPECT(amm2.expectBalances( + EUR(2000), XRP(6000), IOUAmount{3464101615137756, -9})); + + // gw2 claw back 2000 EUR from bib in amm2, which exceeds bob's + // balance. All bob's lptokens will be consumed, which corresponds + // to 1000EUR / 3000 XRP. + env(ammClawback( + gw2, bob, EUR, EUR(2000), amm2.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw2["EUR"](4000))); + env.require(balance(bob, gw2["EUR"](3000))); + + // Bob gets another 3000 XRP back. Alice's XRP balance remains the + // same. + BEAST_EXPECT(expectLedgerEntryRoot( + env, + alice, + aliceXrpBalance + XRP(1000) + XRP(600) + XRP(1000) + + XRP(2400))); + BEAST_EXPECT(expectLedgerEntryRoot( + env, bob, bobXrpBalance + XRP(20) + XRP(1980) + XRP(3000))); + + // Neither alice nor bob has any lptoken in amm2 + BEAST_EXPECT(amm2.expectLPTokens(alice, IOUAmount(0))); + BEAST_EXPECT(amm2.expectLPTokens(bob, IOUAmount(0))); + + BEAST_EXPECT(amm2.expectBalances( + EUR(1000), XRP(3000), IOUAmount{1732050807568878, -9})); + } + } + + void + testAMMClawbackAll(FeatureBitset features) + { + testcase("test AMMClawback all the tokens in the AMM pool"); + using namespace jtx; + + // Test AMMClawback for USD/EUR pool. The assets are issued by different + // issuer. Claw back all the USD for different users. + { + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + Account bob{"bob"}; + Account carol{"carol"}; + env.fund(XRP(1000000), gw, gw2, alice, bob, carol); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw2 sets asfAllowTrustLineClawback. + env(fset(gw2, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw2, asfAllowTrustLineClawback)); + + // gw issues 6000 USD to Alice, 5000 USD to Bob, and 4000 USD + // to Carol. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(6000))); + env.trust(USD(100000), bob); + env(pay(gw, bob, USD(5000))); + env.trust(USD(100000), carol); + env(pay(gw, carol, USD(4000))); + env.close(); + + // gw2 issues 6000 EUR to Alice and 5000 EUR to Bob and 4000 + // EUR to Carol. + auto const EUR = gw2["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw2, alice, EUR(6000))); + env.trust(EUR(100000), bob); + env(pay(gw2, bob, EUR(5000))); + env.trust(EUR(100000), carol); + env(pay(gw2, carol, EUR(4000))); + env.close(); + + // Alice creates AMM pool of EUR/USD + AMM amm(env, alice, EUR(5000), USD(4000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + USD(4000), EUR(5000), IOUAmount{4472135954999580, -12})); + amm.deposit(bob, USD(2000), EUR(2500)); + BEAST_EXPECT(amm.expectBalances( + USD(6000), EUR(7500), IOUAmount{6708203932499370, -12})); + amm.deposit(carol, USD(1000), EUR(1250)); + BEAST_EXPECT(amm.expectBalances( + USD(7000), EUR(8750), IOUAmount{7826237921249265, -12})); + + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{4472135954999580, -12})); + BEAST_EXPECT( + amm.expectLPTokens(bob, IOUAmount{2236067977499790, -12})); + BEAST_EXPECT( + amm.expectLPTokens(carol, IOUAmount{1118033988749895, -12})); + + env.require(balance(alice, gw["USD"](2000))); + env.require(balance(alice, gw2["EUR"](1000))); + env.require(balance(bob, gw["USD"](3000))); + env.require(balance(bob, gw2["EUR"](2500))); + env.require(balance(carol, gw["USD"](3000))); + env.require(balance(carol, gw2["EUR"](2750))); + + // gw clawback all the bob's USD in amm. (2000 USD / 2500 EUR) + env(ammClawback( + gw, bob, USD, std::nullopt, amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + STAmount{USD, UINT64_C(4999999999999999), -12}, + STAmount{EUR, UINT64_C(6249999999999999), -12}, + IOUAmount{5590169943749475, -12})); + + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{4472135954999580, -12})); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(0))); + BEAST_EXPECT( + amm.expectLPTokens(carol, IOUAmount{1118033988749895, -12})); + + // Bob will get 2500 EUR back. + env.require(balance(alice, gw["USD"](2000))); + env.require(balance(alice, gw2["EUR"](1000))); + BEAST_EXPECT( + env.balance(bob, USD) == + STAmount(USD, UINT64_C(3000000000000000), -12)); + + BEAST_EXPECT( + env.balance(bob, EUR) == + STAmount(EUR, UINT64_C(5000000000000001), -12)); + env.require(balance(carol, gw["USD"](3000))); + env.require(balance(carol, gw2["EUR"](2750))); + + // gw2 clawback all carol's EUR in amm. (1000 USD / 1250 EUR) + env(ammClawback( + gw2, + carol, + EUR, + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + STAmount{USD, UINT64_C(3999999999999999), -12}, + STAmount{EUR, UINT64_C(4999999999999999), -12}, + IOUAmount{4472135954999580, -12})); + + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{4472135954999580, -12})); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(0))); + BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(0))); + + // gw2 clawback all alice's EUR in amm. (4000 USD / 5000 EUR) + env(ammClawback( + gw2, + alice, + EUR, + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(carol, gw2["EUR"](2750))); + env.require(balance(carol, gw["USD"](4000))); + BEAST_EXPECT(!amm.ammExists()); + } + + // Test AMMClawback for USD/XRP pool. The assets are issued by different + // issuer. Claw back all the USD for different users. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + Account bob{"bob"}; + env.fund(XRP(1000000), gw, alice, bob); + env.close(); + + // gw sets asfAllowTrustLineClawback + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 600000 USD to Alice and 500000 USD to Bob. + auto const USD = gw["USD"]; + env.trust(USD(1000000), alice); + env(pay(gw, alice, USD(600000))); + env.trust(USD(1000000), bob); + env(pay(gw, bob, USD(500000))); + env.close(); + + // gw creates AMM pool of XRP/USD, alice and bob deposit XRP/USD. + AMM amm(env, gw, XRP(2000), USD(10000), ter(tesSUCCESS)); + BEAST_EXPECT(amm.expectBalances( + USD(10000), XRP(2000), IOUAmount{4472135954999580, -9})); + amm.deposit(alice, USD(1000), XRP(200)); + BEAST_EXPECT(amm.expectBalances( + USD(11000), XRP(2200), IOUAmount{4919349550499538, -9})); + amm.deposit(bob, USD(2000), XRP(400)); + BEAST_EXPECT(amm.expectBalances( + USD(13000), XRP(2600), IOUAmount{5813776741499453, -9})); + env.close(); + + auto aliceXrpBalance = env.balance(alice, XRP); + auto bobXrpBalance = env.balance(bob, XRP); + + // gw clawback all alice's USD in amm. (1000 USD / 200 XRP) + env(ammClawback( + gw, + alice, + USD, + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + USD(12000), XRP(2400), IOUAmount{5366563145999495, -9})); + BEAST_EXPECT( + expectLedgerEntryRoot(env, alice, aliceXrpBalance + XRP(200))); + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); + + // gw clawback all bob's USD in amm. (2000 USD / 400 XRP) + env(ammClawback( + gw, bob, USD, std::nullopt, amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + USD(10000), XRP(2000), IOUAmount{4472135954999580, -9})); + BEAST_EXPECT( + expectLedgerEntryRoot(env, bob, bobXrpBalance + XRP(400))); + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(0))); + } + } + + void + testAMMClawbackSameIssuerAssets(FeatureBitset features) + { + testcase( + "test AMMClawback from AMM pool with assets having the same " + "issuer"); + using namespace jtx; + + // Test AMMClawback for USD/EUR pool. The assets are issued by different + // issuer. Claw back all the USD for different users. + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + Account bob{"bob"}; + Account carol{"carol"}; + env.fund(XRP(1000000), gw, alice, bob, carol); + env.close(); + + // gw sets asfAllowTrustLineClawback. + 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(10000))); + env.trust(USD(100000), bob); + env(pay(gw, bob, USD(9000))); + env.trust(USD(100000), carol); + env(pay(gw, carol, USD(8000))); + env.close(); + + auto const EUR = gw["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw, alice, EUR(10000))); + env.trust(EUR(100000), bob); + env(pay(gw, bob, EUR(9000))); + env.trust(EUR(100000), carol); + env(pay(gw, carol, EUR(8000))); + env.close(); + + AMM amm(env, alice, EUR(2000), USD(8000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances(USD(8000), EUR(2000), IOUAmount(4000))); + amm.deposit(bob, USD(4000), EUR(1000)); + BEAST_EXPECT( + amm.expectBalances(USD(12000), EUR(3000), IOUAmount(6000))); + amm.deposit(carol, USD(2000), EUR(500)); + BEAST_EXPECT( + amm.expectBalances(USD(14000), EUR(3500), IOUAmount(7000))); + + // gw clawback 1000 USD from carol. + env(ammClawback( + gw, carol, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT( + amm.expectBalances(USD(13000), EUR(3250), IOUAmount(6500))); + + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(4000))); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(2000))); + BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(500))); + BEAST_EXPECT(env.balance(alice, USD) == USD(2000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(bob, USD) == USD(5000)); + BEAST_EXPECT(env.balance(bob, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(carol, USD) == USD(6000)); + // 250 EUR goes back to carol. + BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); + + // gw clawback 1000 USD from bob with tfClawTwoAssets flag. + // then the corresponding EUR will also be clawed back + // by gw. + env(ammClawback( + gw, bob, USD, USD(1000), amm.ammAccount(), tfClawTwoAssets), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT( + amm.expectBalances(USD(12000), EUR(3000), IOUAmount(6000))); + + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(4000))); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(1500))); + BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(500))); + BEAST_EXPECT(env.balance(alice, USD) == USD(2000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(bob, USD) == USD(5000)); + // 250 EUR did not go back to bob because tfClawTwoAssets is set. + BEAST_EXPECT(env.balance(bob, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(carol, USD) == USD(6000)); + BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); + + // gw clawback all USD from alice and set tfClawTwoAssets. + env(ammClawback( + gw, + alice, + USD, + std::nullopt, + amm.ammAccount(), + tfClawTwoAssets), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances(USD(4000), EUR(1000), IOUAmount(2000))); + + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(1500))); + BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(500))); + BEAST_EXPECT(env.balance(alice, USD) == USD(2000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(bob, USD) == USD(5000)); + BEAST_EXPECT(env.balance(bob, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(carol, USD) == USD(6000)); + BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); + } + + void + testAMMClawbackSameCurrency(FeatureBitset features) + { + testcase( + "test AMMClawback from AMM pool with assets having the same " + "currency, but from different issuer"); + using namespace jtx; + + // Test AMMClawback for USD/EUR pool. The assets are issued by different + // issuer. Claw back all the USD for different users. + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + Account bob{"bob"}; + env.fund(XRP(1000000), gw, gw2, alice, bob); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw2 sets asfAllowTrustLineClawback. + env(fset(gw2, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw2, asfAllowTrustLineClawback)); + + env.trust(gw["USD"](100000), alice); + env(pay(gw, alice, gw["USD"](8000))); + env.trust(gw["USD"](100000), bob); + env(pay(gw, bob, gw["USD"](7000))); + + env.trust(gw2["USD"](100000), alice); + env(pay(gw2, alice, gw2["USD"](6000))); + env.trust(gw2["USD"](100000), bob); + env(pay(gw2, bob, gw2["USD"](5000))); + env.close(); + + AMM amm(env, alice, gw["USD"](1000), gw2["USD"](1500), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + gw["USD"](1000), + gw2["USD"](1500), + IOUAmount{1224744871391589, -12})); + amm.deposit(bob, gw["USD"](2000), gw2["USD"](3000)); + BEAST_EXPECT(amm.expectBalances( + gw["USD"](3000), + gw2["USD"](4500), + IOUAmount{3674234614174767, -12})); + + // Issuer does not match with asset. + env(ammClawback( + gw, + alice, + gw2["USD"], + STAmount{Issue{gw2["USD"].currency, gw2.id()}, 500}, + amm.ammAccount(), + std::nullopt), + ter(temBAD_ASSET_ISSUER)); + + // gw2 clawback 500 gw2[USD] from alice. + env(ammClawback( + gw2, + alice, + gw2["USD"], + STAmount{Issue{gw2["USD"].currency, gw2.id()}, 500}, + amm.ammAccount(), + std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + STAmount{gw["USD"], UINT64_C(2666666666666667), -12}, + gw2["USD"](4000), + IOUAmount{3265986323710904, -12})); + + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{8164965809277260, -13})); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount{2449489742783178, -12})); + BEAST_EXPECT( + env.balance(alice, gw["USD"]) == + STAmount(gw["USD"], UINT64_C(7333333333333333), -12)); + BEAST_EXPECT(env.balance(alice, gw2["USD"]) == gw2["USD"](4500)); + BEAST_EXPECT(env.balance(bob, gw["USD"]) == gw["USD"](5000)); + BEAST_EXPECT(env.balance(bob, gw2["USD"]) == gw2["USD"](2000)); + + // gw clawback all gw["USD"] from bob. + env(ammClawback( + gw, + bob, + gw["USD"], + std::nullopt, + amm.ammAccount(), + std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + STAmount{gw["USD"], UINT64_C(6666666666666670), -13}, + gw2["USD"](1000), + IOUAmount{8164965809277260, -13})); + + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{8164965809277260, -13})); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(0))); + BEAST_EXPECT( + env.balance(alice, gw["USD"]) == + STAmount(gw["USD"], UINT64_C(7333333333333333), -12)); + BEAST_EXPECT(env.balance(alice, gw2["USD"]) == gw2["USD"](4500)); + BEAST_EXPECT(env.balance(bob, gw["USD"]) == gw["USD"](5000)); + // Bob gets 3000 gw2["USD"] back and now his balance is 5000. + BEAST_EXPECT(env.balance(bob, gw2["USD"]) == gw2["USD"](5000)); + } + + void + testAMMClawbackIssuesEachOther(FeatureBitset features) + { + testcase("test AMMClawback when issuing token for each other"); + using namespace jtx; + + // gw and gw2 issues token for each other. Test AMMClawback from + // each other. + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw2 sets asfAllowTrustLineClawback. + env(fset(gw2, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw2, asfAllowTrustLineClawback)); + + auto const USD = gw["USD"]; + env.trust(USD(100000), gw2); + env(pay(gw, gw2, USD(5000))); + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(5000))); + + auto const EUR = gw2["EUR"]; + env.trust(EUR(100000), gw); + env(pay(gw2, gw, EUR(6000))); + env.trust(EUR(100000), alice); + env(pay(gw2, alice, EUR(6000))); + env.close(); + + AMM amm(env, gw, USD(1000), EUR(2000), ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + USD(1000), EUR(2000), IOUAmount{1414213562373095, -12})); + + amm.deposit(gw2, USD(2000), EUR(4000)); + BEAST_EXPECT(amm.expectBalances( + USD(3000), EUR(6000), IOUAmount{4242640687119285, -12})); + + amm.deposit(alice, USD(3000), EUR(6000)); + BEAST_EXPECT(amm.expectBalances( + USD(6000), EUR(12000), IOUAmount{8485281374238570, -12})); + + BEAST_EXPECT(amm.expectLPTokens(gw, IOUAmount{1414213562373095, -12})); + BEAST_EXPECT(amm.expectLPTokens(gw2, IOUAmount{2828427124746190, -12})); + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{4242640687119285, -12})); + + // gw claws back 1000 USD from gw2. + env(ammClawback( + gw, gw2, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + USD(5000), EUR(10000), IOUAmount{7071067811865475, -12})); + + BEAST_EXPECT(amm.expectLPTokens(gw, IOUAmount{1414213562373095, -12})); + BEAST_EXPECT(amm.expectLPTokens(gw2, IOUAmount{1414213562373095, -12})); + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{4242640687119285, -12})); + + BEAST_EXPECT(env.balance(alice, USD) == USD(2000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(0)); + BEAST_EXPECT(env.balance(gw, EUR) == EUR(4000)); + BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); + + // gw2 claws back 1000 EUR from gw. + env(ammClawback( + gw2, gw, EUR, EUR(1000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + USD(4500), + STAmount(EUR, UINT64_C(9000000000000001), -12), + IOUAmount{6363961030678928, -12})); + + BEAST_EXPECT(amm.expectLPTokens(gw, IOUAmount{7071067811865480, -13})); + BEAST_EXPECT(amm.expectLPTokens(gw2, IOUAmount{1414213562373095, -12})); + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{4242640687119285, -12})); + + BEAST_EXPECT(env.balance(alice, USD) == USD(2000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(0)); + BEAST_EXPECT(env.balance(gw, EUR) == EUR(4000)); + BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); + + // gw2 claws back 4000 EUR from alice. + env(ammClawback( + gw2, alice, EUR, EUR(4000), amm.ammAccount(), std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT(amm.expectBalances( + USD(2500), + STAmount(EUR, UINT64_C(5000000000000001), -12), + IOUAmount{3535533905932738, -12})); + + BEAST_EXPECT(amm.expectLPTokens(gw, IOUAmount{7071067811865480, -13})); + BEAST_EXPECT(amm.expectLPTokens(gw2, IOUAmount{1414213562373095, -12})); + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{1414213562373095, -12})); + + BEAST_EXPECT(env.balance(alice, USD) == USD(4000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(0)); + BEAST_EXPECT(env.balance(gw, EUR) == EUR(4000)); + BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); + } + + void + testNotHoldingLptoken(FeatureBitset features) + { + testcase( + "test AMMClawback from account which does not own any lptoken in " + "the pool"); + using namespace jtx; + + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + 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(5000))); + + AMM amm(env, gw, USD(1000), XRP(2000), ter(tesSUCCESS)); + env.close(); + + // Alice did not deposit in the amm pool. So AMMClawback from Alice + // will fail. + env(ammClawback( + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + ter(tecINTERNAL)); + } + +public: + void + run() override + { + FeatureBitset const all{jtx::supported_amendments()}; + testInvalidRequest(all); + testFeatureDisabled(all - featureAMMClawback); + testAMMClawbackSpecificAmount(all); + testAMMClawbackExceedBalance(all); + testAMMClawbackAll(all); + testAMMClawbackSameIssuerAssets(all); + testAMMClawbackSameCurrency(all); + testAMMClawbackIssuesEachOther(all); + testNotHoldingLptoken(all); + } +}; +BEAST_DEFINE_TESTSUITE(AMMClawback, app, ripple); +} // namespace test +} // namespace ripple diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index ceddc019504..26c18d3f4b4 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -416,25 +416,10 @@ struct AMM_test : public jtx::AMMTest AMM ammAlice1( env, alice, USD(10'000), USD1(10'000), ter(terNO_RIPPLE)); } - - // Issuer has clawback enabled - { - Env env(*this); - env.fund(XRP(1'000), gw); - env(fset(gw, asfAllowTrustLineClawback)); - fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}, Fund::Acct); - env.close(); - AMM amm(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION)); - AMM amm1(env, alice, USD(100), XRP(100), ter(tecNO_PERMISSION)); - env(fclear(gw, asfAllowTrustLineClawback)); - env.close(); - // Can't be cleared - AMM amm2(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION)); - } } void - testInvalidDeposit() + testInvalidDeposit(FeatureBitset features) { testcase("Invalid Deposit"); @@ -869,62 +854,112 @@ struct AMM_test : public jtx::AMMTest }); // Globally frozen asset - testAMM([&](AMM& ammAlice, Env& env) { - env(fset(gw, asfGlobalFreeze)); - // Can deposit non-frozen token - ammAlice.deposit(carol, XRP(100)); - ammAlice.deposit( - carol, - USD(100), - std::nullopt, - std::nullopt, - std::nullopt, - ter(tecFROZEN)); - ammAlice.deposit( - carol, 1'000'000, std::nullopt, std::nullopt, ter(tecFROZEN)); - ammAlice.deposit( - carol, - XRP(100), - USD(100), - std::nullopt, - std::nullopt, - ter(tecFROZEN)); - }); + testAMM( + [&](AMM& ammAlice, Env& env) { + env(fset(gw, asfGlobalFreeze)); + if (!features[featureAMMClawback]) + // If the issuer set global freeze, the holder still can + // deposit the other non-frozen token when AMMClawback is + // not enabled. + ammAlice.deposit(carol, XRP(100)); + else + // If the issuer set global freeze, the holder cannot + // deposit the other non-frozen token when AMMClawback is + // enabled. + ammAlice.deposit( + carol, + XRP(100), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + ammAlice.deposit( + carol, + USD(100), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + ammAlice.deposit( + carol, + 1'000'000, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + ammAlice.deposit( + carol, + XRP(100), + USD(100), + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + }, + std::nullopt, + 0, + std::nullopt, + {features}); // Individually frozen (AMM) account - testAMM([&](AMM& ammAlice, Env& env) { - env(trust(gw, carol["USD"](0), tfSetFreeze)); - env.close(); - // Can deposit non-frozen token - ammAlice.deposit(carol, XRP(100)); - ammAlice.deposit( - carol, 1'000'000, std::nullopt, std::nullopt, ter(tecFROZEN)); - ammAlice.deposit( - carol, - USD(100), - std::nullopt, - std::nullopt, - std::nullopt, - ter(tecFROZEN)); - env(trust(gw, carol["USD"](0), tfClearFreeze)); - // Individually frozen AMM - env(trust( - gw, - STAmount{Issue{gw["USD"].currency, ammAlice.ammAccount()}, 0}, - tfSetFreeze)); - env.close(); - // Can deposit non-frozen token - ammAlice.deposit(carol, XRP(100)); - ammAlice.deposit( - carol, 1'000'000, std::nullopt, std::nullopt, ter(tecFROZEN)); - ammAlice.deposit( - carol, - USD(100), - std::nullopt, - std::nullopt, - std::nullopt, - ter(tecFROZEN)); - }); + testAMM( + [&](AMM& ammAlice, Env& env) { + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + if (!features[featureAMMClawback]) + // Can deposit non-frozen token if AMMClawback is not + // enabled + ammAlice.deposit(carol, XRP(100)); + else + // Cannot deposit non-frozen token if the other token is + // frozen when AMMClawback is enabled + ammAlice.deposit( + carol, + XRP(100), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + + ammAlice.deposit( + carol, + 1'000'000, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + ammAlice.deposit( + carol, + USD(100), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + env(trust(gw, carol["USD"](0), tfClearFreeze)); + // Individually frozen AMM + env(trust( + gw, + STAmount{ + Issue{gw["USD"].currency, ammAlice.ammAccount()}, 0}, + tfSetFreeze)); + env.close(); + // Can deposit non-frozen token + ammAlice.deposit(carol, XRP(100)); + ammAlice.deposit( + carol, + 1'000'000, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + ammAlice.deposit( + carol, + USD(100), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + }, + std::nullopt, + 0, + std::nullopt, + {features}); // Individually frozen (AMM) account with IOU/IOU AMM testAMM( @@ -6862,13 +6897,140 @@ struct AMM_test : public jtx::AMMTest } } + void + testAMMClawback(FeatureBitset features) + { + testcase("test clawback from AMM account"); + using namespace jtx; + + // Issuer has clawback enabled + Env env(*this, features); + env.fund(XRP(1'000), gw); + env(fset(gw, asfAllowTrustLineClawback)); + fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}, Fund::Acct); + env.close(); + + // If featureAMMClawback is not enabled, AMMCreate is not allowed for + // clawback-enabled issuer + if (!features[featureAMMClawback]) + { + AMM amm(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION)); + AMM amm1(env, alice, USD(100), XRP(100), ter(tecNO_PERMISSION)); + env(fclear(gw, asfAllowTrustLineClawback)); + env.close(); + // Can't be cleared + AMM amm2(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION)); + } + // If featureAMMClawback is enabled, AMMCreate is allowed for + // clawback-enabled issuer. Clawback from the AMM Account is not + // allowed, which will return tecAMM_ACCOUNT. We can only use + // AMMClawback transaction to claw back from AMM Account. + else + { + AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); + AMM amm1(env, alice, USD(100), XRP(200), ter(tecDUPLICATE)); + + // Construct the amount being clawed back using AMM account. + // By doing this, we make the clawback transaction's Amount field's + // subfield `issuer` to be the AMM account, which means + // we are clawing back from an AMM account. This should return an + // tecAMM_ACCOUNT error because regular Clawback transaction is not + // allowed for clawing back from an AMM account. Please notice the + // `issuer` subfield represents the account being clawed back, which + // is confusing. + Issue usd(USD.issue().currency, amm.ammAccount()); + auto amount = amountFromString(usd, "10"); + env(claw(gw, amount), ter(tecAMM_ACCOUNT)); + } + } + + void + testAMMDepositWithFrozenAssets(FeatureBitset features) + { + testcase("test AMMDeposit with frozen assets"); + using namespace jtx; + + // This lambda function is used to create trustlines + // between gw and alice, and create and return an AMM account. + auto setupAMM = [&](Env& env) -> AMM* { + env.fund(XRP(1'000), gw); + fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}, Fund::Acct); + env.close(); + AMM* amm = new AMM(env, alice, XRP(100), USD(100), ter(tesSUCCESS)); + env(trust(gw, alice["USD"](0), tfSetFreeze)); + return amm; + }; + + { + // Deposit two assets, one of which is frozen, + // then we should get tecFROZEN error. + Env env(*this, features); + auto amm = setupAMM(env); + amm->deposit( + alice, + USD(100), + XRP(100), + std::nullopt, + tfTwoAsset, + ter(tecFROZEN)); + delete amm; + } + + { + // Deposit one asset, which is the frozen token, + // then we should get tecFROZEN error. + Env env(*this, features); + auto amm = setupAMM(env); + amm->deposit( + alice, + USD(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tecFROZEN)); + delete amm; + } + + if (features[featureAMMClawback]) + { + // Deposit one asset which is not the frozen token, + // but the other asset is frozen. We should get tecFROZEN error + // when feature AMMClawback is enabled. + Env env(*this, features); + auto amm = setupAMM(env); + amm->deposit( + alice, + XRP(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tecFROZEN)); + } + else + { + // Deposit one asset which is not the frozen token, + // but the other asset is frozen. We will get tecSUCCESS + // when feature AMMClawback is not enabled. + Env env(*this, features); + auto amm = setupAMM(env); + amm->deposit( + alice, + XRP(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tesSUCCESS)); + } + } + void run() override { FeatureBitset const all{jtx::supported_amendments()}; testInvalidInstance(); testInstanceCreate(); - testInvalidDeposit(); + testInvalidDeposit(all); + testInvalidDeposit(all - featureAMMClawback); testDeposit(); testInvalidWithdraw(); testWithdraw(); @@ -6908,6 +7070,12 @@ struct AMM_test : public jtx::AMMTest testFixAMMOfferBlockedByLOB(all - fixAMMv1_1); testLPTokenBalance(all); testLPTokenBalance(all - fixAMMv1_1); + testAMMClawback(all); + testAMMClawback(all - featureAMMClawback); + testAMMClawback(all - fixAMMv1_1 - featureAMMClawback); + testAMMDepositWithFrozenAssets(all); + testAMMDepositWithFrozenAssets(all - featureAMMClawback); + testAMMDepositWithFrozenAssets(all - fixAMMv1_1 - featureAMMClawback); } }; diff --git a/src/test/jtx/impl/trust.cpp b/src/test/jtx/impl/trust.cpp index 641a0f79f28..74886f01aa3 100644 --- a/src/test/jtx/impl/trust.cpp +++ b/src/test/jtx/impl/trust.cpp @@ -74,6 +74,28 @@ claw(Account const& account, STAmount const& amount) return jv; } +Json::Value +ammClawback( + Account const& issuer, + Account const& holder, + Issue const& asset, + std::optional const& amount, + AccountID const& ammAccount, + std::optional flags) +{ + Json::Value jv; + jv[jss::TransactionType] = jss::AMMClawback; + jv[jss::Account] = issuer.human(); + jv[jss::Holder] = holder.human(); + jv[jss::AMMAccount] = to_string(ammAccount); + jv[jss::Asset] = to_json(asset); + if (amount) + jv[jss::Amount] = amount->getJson(JsonOptions::none); + + if (flags) + jv[jss::Flags] = *flags; + return jv; +} } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/test/jtx/trust.h b/src/test/jtx/trust.h index f9fddf4871a..db31f50c8bd 100644 --- a/src/test/jtx/trust.h +++ b/src/test/jtx/trust.h @@ -43,6 +43,15 @@ trust( Json::Value claw(Account const& account, STAmount const& amount); +Json::Value +ammClawback( + Account const& issuer, + Account const& holder, + Issue const& asset, + std::optional const& amount, + AccountID const& ammAccount, + std::optional flags); + } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/xrpld/app/misc/AMMUtils.h b/src/xrpld/app/misc/AMMUtils.h index 52fe819a28e..e02ef6158ad 100644 --- a/src/xrpld/app/misc/AMMUtils.h +++ b/src/xrpld/app/misc/AMMUtils.h @@ -123,6 +123,31 @@ isOnlyLiquidityProvider( Issue const& ammIssue, AccountID const& lpAccount); +std::tuple> +withdraw( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee, + beast::Journal const& journal, + STTx const& tx, + bool withdrawAll); + +std::pair +deleteAMMAccountIfEmpty( + Sandbox& sb, + std::shared_ptr const ammSle, + STAmount const& lpTokenBalance, + Issue const& issue1, + Issue const& issue2, + beast::Journal const& journal); + } // namespace ripple #endif // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED diff --git a/src/xrpld/app/misc/detail/AMMUtils.cpp b/src/xrpld/app/misc/detail/AMMUtils.cpp index efc80cf17b6..b5aeb10326f 100644 --- a/src/xrpld/app/misc/detail/AMMUtils.cpp +++ b/src/xrpld/app/misc/detail/AMMUtils.cpp @@ -16,12 +16,14 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ //============================================================================== +#include #include #include #include #include #include #include +#include namespace ripple { @@ -430,4 +432,204 @@ isOnlyLiquidityProvider( return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE } +std::tuple> +withdraw( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee, + beast::Journal const& journal, + STTx const& tx, + bool withdrawAll) +{ + auto const lpTokens = ammLPHolds(view, ammSle, account, journal); + auto const expected = ammHolds( + view, + ammSle, + amountWithdraw.issue(), + std::nullopt, + FreezeHandling::fhZERO_IF_FROZEN, + journal); + // LCOV_EXCL_START + if (!expected) + return {expected.error(), STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + auto const [curBalance, curBalance2, _] = *expected; + (void)_; + + auto const + [amountWithdrawActual, amount2WithdrawActual, lpTokensWithdrawActual] = + [&]() -> std::tuple, STAmount> { + if (!withdrawAll) + return adjustAmountsByLPTokens( + amountBalance, + amountWithdraw, + amount2Withdraw, + lpTokensAMMBalance, + lpTokensWithdraw, + tfee, + false); + return std::make_tuple( + amountWithdraw, amount2Withdraw, lpTokensWithdraw); + }(); + + if (lpTokensWithdrawActual <= beast::zero || + lpTokensWithdrawActual > lpTokens) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw, invalid LP tokens: " + << lpTokensWithdrawActual << " " << lpTokens << " " + << lpTokensAMMBalance; + return {tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, STAmount{}}; + } + + // Should not happen since the only LP on last withdraw + // has the balance set to the lp token trustline balance. + if (view.rules().enabled(fixAMMv1_1) && + lpTokensWithdrawActual > lpTokensAMMBalance) + { + // LCOV_EXCL_START + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw, unexpected LP tokens: " + << lpTokensWithdrawActual << " " << lpTokens << " " + << lpTokensAMMBalance; + return {tecINTERNAL, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + // Withdrawing one side of the pool + if ((amountWithdrawActual == curBalance && + amount2WithdrawActual != curBalance2) || + (amount2WithdrawActual == curBalance2 && + amountWithdrawActual != curBalance)) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw one side of the pool " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // May happen if withdrawing an amount close to one side of the pool + if (lpTokensWithdrawActual == lpTokensAMMBalance && + (amountWithdrawActual != curBalance || + amount2WithdrawActual != curBalance2)) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw all tokens " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " curBalance2: " << amount2WithdrawActual.value_or(STAmount{0}) + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // Withdrawing more than the pool's balance + if (amountWithdrawActual > curBalance || + amount2WithdrawActual > curBalance2) + { + JLOG(journal.debug()) + << "AMM Withdraw: withdrawing more than the pool's balance " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " curBalance2: " << curBalance2 << " " + << (amount2WithdrawActual ? *amount2WithdrawActual : STAmount{}) + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // Withdraw amountWithdraw + auto res = accountSend( + view, + ammAccount, + account, + amountWithdrawActual, + journal, + WaiveTransferFee::Yes); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw " << amountWithdrawActual; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + // Withdraw amount2Withdraw + if (amount2WithdrawActual) + { + res = accountSend( + view, + ammAccount, + account, + *amount2WithdrawActual, + journal, + WaiveTransferFee::Yes); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw " + << *amount2WithdrawActual; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + } + + // Withdraw LP tokens + res = redeemIOU( + view, + account, + lpTokensWithdrawActual, + lpTokensWithdrawActual.issue(), + journal); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw LPTokens"; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + return std::make_tuple( + tesSUCCESS, + lpTokensAMMBalance - lpTokensWithdrawActual, + amountWithdrawActual, + amount2WithdrawActual); +} + +std::pair +deleteAMMAccountIfEmpty( + Sandbox& sb, + std::shared_ptr const ammSle, + STAmount const& lpTokenBalance, + Issue const& issue1, + Issue const& issue2, + beast::Journal const& journal) +{ + TER ter; + bool updateBalance = true; + if (lpTokenBalance == beast::zero) + { + ter = deleteAMMAccount(sb, issue1, issue2, journal); + if (ter != tesSUCCESS && ter != tecINCOMPLETE) + return {ter, false}; // LCOV_EXCL_LINE + else + updateBalance = (ter == tecINCOMPLETE); + } + + if (updateBalance) + { + ammSle->setFieldAmount(sfLPTokenBalance, lpTokenBalance); + sb.update(ammSle); + } + + return {ter, true}; +} } // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp new file mode 100644 index 00000000000..4512e860d30 --- /dev/null +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -0,0 +1,323 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace ripple { + +NotTEC +AMMClawback::preflight(PreflightContext const& ctx) +{ + if (!ctx.rules.enabled(featureAMMClawback)) + return temDISABLED; + + if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + return ret; // LCOV_EXCL_LINE + + if (ctx.tx.getFlags() & tfAMMClawbackMask) + return temINVALID_FLAG; + + AccountID const issuer = ctx.tx[sfAccount]; + AccountID const holder = ctx.tx[sfHolder]; + + if (issuer == holder) + { + JLOG(ctx.j.trace()) + << "AMMClawback: holder cannot be the same as issuer."; + return temMALFORMED; + } + + std::optional const clawAmount = ctx.tx[~sfAmount]; + auto const asset = ctx.tx[sfAsset]; + + if (isXRP(asset)) + return temMALFORMED; + + if (asset.account != issuer) + { + JLOG(ctx.j.trace()) << "AMMClawback: Asset's account does not " + "match Account field."; + return temBAD_ASSET_ISSUER; + } + + if (clawAmount && clawAmount->issue() != asset) + { + JLOG(ctx.j.trace()) << "AMMClawback: Amount's issuer/currency subfield " + "does not match Asset field"; + return temBAD_ASSET_AMOUNT; + } + + if (clawAmount && *clawAmount <= beast::zero) + return temBAD_AMOUNT; + + return preflight2(ctx); +} + +TER +AMMClawback::preclaim(PreclaimContext const& ctx) +{ + AccountID const issuer = ctx.tx[sfAccount]; + AccountID const holder = ctx.tx[sfHolder]; + AccountID const ammAccount = ctx.tx[sfAMMAccount]; + + auto const sleIssuer = ctx.view.read(keylet::account(issuer)); + auto const sleHolder = ctx.view.read(keylet::account(holder)); + auto const sleAMMAccount = ctx.view.read(keylet::account(ammAccount)); + + if (!sleAMMAccount) + { + JLOG(ctx.j.debug()) + << "AMMClawback: AMMAccount provided does not exist."; + return terNO_AMM; + } + + std::uint32_t const issuerFlagsIn = sleIssuer->getFieldU32(sfFlags); + + // If AllowTrustLineClawback is not set or NoFreeze is set, return no + // permission + if (!(issuerFlagsIn & lsfAllowTrustLineClawback) || + (issuerFlagsIn & lsfNoFreeze)) + return tecNO_PERMISSION; + + auto const ammID = sleAMMAccount->getFieldH256(sfAMMID); + if (!ammID) + { + JLOG(ctx.j.trace()) + << "AMMClawback: AMMAccount field is not an AMM account."; + return terNO_AMM; + } + + auto const sleAMM = ctx.view.read(keylet::amm(ammID)); + if (!sleAMM) + return tecINTERNAL; // LCOV_EXCL_LINE + + STIssue const& asset = sleAMM->getFieldIssue(sfAsset); + STIssue const& asset2 = sleAMM->getFieldIssue(sfAsset2); + + if (ctx.tx[sfAsset] != asset && ctx.tx[sfAsset] != asset2) + { + JLOG(ctx.j.trace()) << "AMMClawback: Asset being clawed back does not " + "match either asset in the AMM pool."; + return tecNO_PERMISSION; + } + + auto const flags = ctx.tx.getFlags(); + if (flags & tfClawTwoAssets) + { + if (asset.issue().account != asset2.issue().account) + { + JLOG(ctx.j.trace()) + << "AMMClawback: tfClawTwoAssets can only be enabled when two " + "assets in the AMM pool are both issued by the issuer"; + return tecNO_PERMISSION; + } + } + + return tesSUCCESS; +} + +TER +AMMClawback::doApply() +{ + Sandbox sb(&ctx_.view()); + + auto const ter = applyGuts(sb); + if (ter == tesSUCCESS) + sb.apply(ctx_.rawView()); + + return ter; +} + +TER +AMMClawback::applyGuts(Sandbox& sb) +{ + std::optional const clawAmount = ctx_.tx[~sfAmount]; + AccountID const ammAccount = ctx_.tx[sfAMMAccount]; + AccountID const issuer = ctx_.tx[sfAccount]; + AccountID const holder = ctx_.tx[sfHolder]; + Issue const asset = ctx_.tx[sfAsset]; + + auto const sleAMMAccount = ctx_.view().read(keylet::account(ammAccount)); + + // should not happen. checked in preclaim. + if (!sleAMMAccount) + return terNO_AMM; // LCOV_EXCL_LINE + + auto const ammID = sleAMMAccount->getFieldH256(sfAMMID); + if (!ammID) + return tecINTERNAL; // LCOV_EXCL_LINE + + auto ammSle = sb.peek(keylet::amm(ammID)); + if (!ammSle) + return tecINTERNAL; // LCOV_EXCL_LINE + + auto const tfee = getTradingFee(ctx_.view(), *ammSle, ammAccount); + Issue const& issue1 = ammSle->getFieldIssue(sfAsset).issue(); + Issue const& issue2 = ammSle->getFieldIssue(sfAsset2).issue(); + + Issue otherIssue = issue1; + if (asset == issue1) + otherIssue = issue2; + + auto const expected = ammHolds( + sb, + *ammSle, + asset, + otherIssue, + FreezeHandling::fhZERO_IF_FROZEN, + ctx_.journal); + + if (!expected) + return expected.error(); // LCOV_EXCL_LINE + auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; + + TER result; + STAmount newLPTokenBalance; + STAmount amountWithdraw; + std::optional amount2Withdraw; + + auto const holdLPtokens = ammLPHolds(sb, *ammSle, holder, j_); + if (holdLPtokens == beast::zero) + return tecINTERNAL; + + if (!clawAmount) + { + std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = + AMMWithdraw::equalWithdrawTokens( + sb, + *ammSle, + holder, + ammAccount, + amountBalance, + amount2Balance, + lptAMMBalance, + holdLPtokens, + holdLPtokens, + tfee, + ctx_.journal, + ctx_.tx, + true); + } + else + std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = + equalWithdrawMatchingOneAmount( + sb, + *ammSle, + holder, + ammAccount, + amountBalance, + amount2Balance, + lptAMMBalance, + *clawAmount, + tfee); + + if (result != tesSUCCESS) + return result; // LCOV_EXCL_LINE + + auto const res = deleteAMMAccountIfEmpty( + sb, ammSle, newLPTokenBalance, issue1, issue2, j_); + if (!res.second) + return res.first; // LCOV_EXCL_LINE + + JLOG(ctx_.journal.trace()) + << "AMM Withdraw during AMMClawback: lptoken new balance: " + << to_string(newLPTokenBalance.iou()) + << " old balance: " << to_string(lptAMMBalance.iou()); + + auto const ter = rippleCredit(sb, holder, issuer, amountWithdraw, true, j_); + if (ter != tesSUCCESS) + return ter; // LCOV_EXCL_LINE + + // if the issuer issues both assets and sets flag tfClawTwoAssets, we + // will claw the paired asset as well. We already checked if + // tfClawTwoAssets is enabled, the two assets have to be issued by the + // same issuer. + auto const flags = ctx_.tx.getFlags(); + if (flags & tfClawTwoAssets) + + return rippleCredit(sb, holder, issuer, *amount2Withdraw, true, j_); + + return tesSUCCESS; +} + +std::tuple> +AMMClawback::equalWithdrawMatchingOneAmount( + Sandbox& sb, + SLE const& ammSle, + AccountID const& holder, + AccountID const& ammAccount, + STAmount const& amountBalance, + STAmount const& amount2Balance, + STAmount const& lptAMMBalance, + STAmount const& amount, + std::uint16_t tfee) +{ + auto frac = Number{amount} / amountBalance; + auto const amount2Withdraw = amount2Balance * frac; + + auto const lpTokensWithdraw = + toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac); + auto const holdLPtokens = ammLPHolds(sb, ammSle, holder, j_); + if (lpTokensWithdraw > holdLPtokens) + // if lptoken balance less than what the issuer intended to clawback, + // clawback all the tokens + return AMMWithdraw::equalWithdrawTokens( + sb, + ammSle, + holder, + ammAccount, + amountBalance, + amount2Balance, + lptAMMBalance, + holdLPtokens, + holdLPtokens, + tfee, + ctx_.journal, + ctx_.tx, + true); + + return withdraw( + sb, + ammAccount, + holder, + ammSle, + amountBalance, + amount, + toSTAmount(amount2Balance.issue(), amount2Withdraw), + lptAMMBalance, + toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), + tfee, + ctx_.journal, + ctx_.tx, + false); +} + +} // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMClawback.h b/src/xrpld/app/tx/detail/AMMClawback.h new file mode 100644 index 00000000000..fcb56af1dc0 --- /dev/null +++ b/src/xrpld/app/tx/detail/AMMClawback.h @@ -0,0 +1,75 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_TX_AMMCLAWBACK_H_INCLUDED +#define RIPPLE_TX_AMMCLAWBACK_H_INCLUDED + +#include + +namespace ripple { +class Sandbox; +class AMMClawback : public Transactor +{ +public: + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; + + explicit AMMClawback(ApplyContext& ctx) : Transactor(ctx) + { + } + + static NotTEC + preflight(PreflightContext const& ctx); + + static TER + preclaim(PreclaimContext const& ctx); + + TER + doApply() override; + +private: + TER + applyGuts(Sandbox& view); + + /** Withdraw both assets by providing maximum amount of asset1, + * asset2's amount will be calculated according to the current proportion. + * @param view + * @param ammAccount current AMM account + * @param amountBalance current AMM asset1 balance + * @param amount2Balance current AMM asset2 balance + * @param lptAMMBalance current AMM LPT balance + * @param amount asset1 withdraw amount + * @param tfee trading fee in basis points + * @return + */ + std::tuple> + equalWithdrawMatchingOneAmount( + Sandbox& view, + SLE const& ammSle, + AccountID const& holder, + AccountID const& ammAccount, + STAmount const& amountBalance, + STAmount const& amount2Balance, + STAmount const& lptAMMBalance, + STAmount const& amount, + std::uint16_t tfee); +}; + +} // namespace ripple + +#endif diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index 237e1afa240..31773166d4a 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -184,7 +184,13 @@ AMMCreate::preclaim(PreclaimContext const& ctx) return tecAMM_INVALID_TOKENS; } - // Disallow AMM if the issuer has clawback enabled + // If featureAMMClawback is enabled, allow AMMCreate without checking + // if the issuer has clawback enabled + if (ctx.view.rules().enabled(featureAMMClawback)) + return tesSUCCESS; + + // Disallow AMM if the issuer has clawback enabled when featureAMMClawback + // is not enabled auto clawbackDisabled = [&](Issue const& issue) -> TER { if (isXRP(issue)) return tesSUCCESS; diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index 9bbf5b4a60a..d9bd22a0097 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -244,6 +244,36 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) : tecUNFUNDED_AMM; }; + if (ctx.view.rules().enabled(featureAMMClawback)) + { + // Check if either of the assets is frozen, AMMDeposit is not allowed + // if either asset is frozen + auto checkAsset = [&](std::optional const& asset) -> TER { + if (!asset.has_value()) + return temMALFORMED; // LCOV_EXCL_LINE + + if (isFrozen(ctx.view, accountID, asset.value())) + { + JLOG(ctx.j.debug()) << "AMM Deposit: account is frozen, " + << to_string(accountID) << " " + << to_string(asset->currency); + + return tecFROZEN; + } + + return tesSUCCESS; + }; + + auto const asset = ctx.tx[~sfAsset]; + auto const asset2 = ctx.tx[~sfAsset2]; + + if (auto const ter = checkAsset(asset)) + return ter; + + if (auto const ter = checkAsset(asset2)) + return ter; + } + auto const amount = ctx.tx[~sfAmount]; auto const amount2 = ctx.tx[~sfAmount2]; auto const ammAccountID = ammSle->getAccountID(sfAccount); diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 51b512aba0a..9dd71d1f6f8 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -358,6 +358,7 @@ AMMWithdraw::applyGuts(Sandbox& sb) if (subTxType & tfTwoAsset) return equalWithdrawLimit( sb, + *ammSle, ammAccountID, amountBalance, amount2Balance, @@ -368,6 +369,7 @@ AMMWithdraw::applyGuts(Sandbox& sb) if (subTxType & tfOneAssetLPToken || subTxType & tfOneAssetWithdrawAll) return singleWithdrawTokens( sb, + *ammSle, ammAccountID, amountBalance, lptAMMBalance, @@ -377,6 +379,7 @@ AMMWithdraw::applyGuts(Sandbox& sb) if (subTxType & tfLimitLPToken) return singleWithdrawEPrice( sb, + *ammSle, ammAccountID, amountBalance, lptAMMBalance, @@ -385,17 +388,36 @@ AMMWithdraw::applyGuts(Sandbox& sb) tfee); if (subTxType & tfSingleAsset) return singleWithdraw( - sb, ammAccountID, amountBalance, lptAMMBalance, *amount, tfee); - if (subTxType & tfLPToken || subTxType & tfWithdrawAll) - return equalWithdrawTokens( sb, + *ammSle, ammAccountID, amountBalance, - amount2Balance, lptAMMBalance, - lpTokens, - *lpTokensWithdraw, + *amount, tfee); + if (subTxType & tfLPToken || subTxType & tfWithdrawAll) + { + TER result; + STAmount newLPTokenBalance; + bool withdrawAll = + ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); + std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = + equalWithdrawTokens( + sb, + *ammSle, + account_, + ammAccountID, + amountBalance, + amount2Balance, + lptAMMBalance, + lpTokens, + *lpTokensWithdraw, + tfee, + ctx_.journal, + ctx_.tx, + withdrawAll); + return {result, newLPTokenBalance}; + } // should not happen. // LCOV_EXCL_START JLOG(j_.error()) << "AMM Withdraw: invalid options."; @@ -406,22 +428,12 @@ AMMWithdraw::applyGuts(Sandbox& sb) if (result != tesSUCCESS) return {result, false}; - bool updateBalance = true; - if (newLPTokenBalance == beast::zero) - { - if (auto const ter = - deleteAMMAccount(sb, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); - ter != tesSUCCESS && ter != tecINCOMPLETE) - return {ter, false}; - else - updateBalance = (ter == tecINCOMPLETE); - } - - if (updateBalance) - { - ammSle->setFieldAmount(sfLPTokenBalance, newLPTokenBalance); - sb.update(ammSle); - } + auto const res = deleteAMMAccountIfEmpty( + sb, ammSle, newLPTokenBalance, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); + // LCOV_EXCL_START + if (!res.second) + return {res.first, false}; + // LCOV_EXCL_STOP JLOG(ctx_.journal.trace()) << "AMM Withdraw: tokens " << to_string(newLPTokenBalance.iou()) << " " @@ -444,190 +456,44 @@ AMMWithdraw::doApply() return result.first; } -std::pair -AMMWithdraw::withdraw( - Sandbox& view, - AccountID const& ammAccount, - STAmount const& amountBalance, - STAmount const& amountWithdraw, - std::optional const& amount2Withdraw, - STAmount const& lpTokensAMMBalance, - STAmount const& lpTokensWithdraw, - std::uint16_t tfee) -{ - auto const ammSle = - ctx_.view().read(keylet::amm(ctx_.tx[sfAsset], ctx_.tx[sfAsset2])); - if (!ammSle) - return {tecINTERNAL, STAmount{}}; // LCOV_EXCL_LINE - auto const lpTokens = ammLPHolds(view, *ammSle, account_, ctx_.journal); - auto const expected = ammHolds( - view, - *ammSle, - amountWithdraw.issue(), - std::nullopt, - FreezeHandling::fhZERO_IF_FROZEN, - j_); - if (!expected) - return {expected.error(), STAmount{}}; - auto const [curBalance, curBalance2, _] = *expected; - (void)_; - - auto const - [amountWithdrawActual, amount2WithdrawActual, lpTokensWithdrawActual] = - [&]() -> std::tuple, STAmount> { - if (!(ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll))) - return adjustAmountsByLPTokens( - amountBalance, - amountWithdraw, - amount2Withdraw, - lpTokensAMMBalance, - lpTokensWithdraw, - tfee, - false); - return std::make_tuple( - amountWithdraw, amount2Withdraw, lpTokensWithdraw); - }(); - - if (lpTokensWithdrawActual <= beast::zero || - lpTokensWithdrawActual > lpTokens) - { - JLOG(ctx_.journal.debug()) - << "AMM Withdraw: failed to withdraw, invalid LP tokens: " - << lpTokensWithdrawActual << " " << lpTokens << " " - << lpTokensAMMBalance; - return {tecAMM_INVALID_TOKENS, STAmount{}}; - } - - // Should not happen since the only LP on last withdraw - // has the balance set to the lp token trustline balance. - if (view.rules().enabled(fixAMMv1_1) && - lpTokensWithdrawActual > lpTokensAMMBalance) - { - JLOG(ctx_.journal.debug()) - << "AMM Withdraw: failed to withdraw, unexpected LP tokens: " - << lpTokensWithdrawActual << " " << lpTokens << " " - << lpTokensAMMBalance; - return {tecINTERNAL, STAmount{}}; - } - - // Withdrawing one side of the pool - if ((amountWithdrawActual == curBalance && - amount2WithdrawActual != curBalance2) || - (amount2WithdrawActual == curBalance2 && - amountWithdrawActual != curBalance)) - { - JLOG(ctx_.journal.debug()) - << "AMM Withdraw: failed to withdraw one side of the pool " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}}; - } - - // May happen if withdrawing an amount close to one side of the pool - if (lpTokensWithdrawActual == lpTokensAMMBalance && - (amountWithdrawActual != curBalance || - amount2WithdrawActual != curBalance2)) - { - JLOG(ctx_.journal.debug()) - << "AMM Withdraw: failed to withdraw all tokens " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " curBalance2: " << amount2WithdrawActual.value_or(STAmount{0}) - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}}; - } - - // Withdrawing more than the pool's balance - if (amountWithdrawActual > curBalance || - amount2WithdrawActual > curBalance2) - { - JLOG(ctx_.journal.debug()) - << "AMM Withdraw: withdrawing more than the pool's balance " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " curBalance2: " << curBalance2 << " " - << (amount2WithdrawActual ? *amount2WithdrawActual : STAmount{}) - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}}; - } - - // Withdraw amountWithdraw - auto res = accountSend( - view, - ammAccount, - account_, - amountWithdrawActual, - ctx_.journal, - WaiveTransferFee::Yes); - if (res != tesSUCCESS) - { - JLOG(ctx_.journal.debug()) - << "AMM Withdraw: failed to withdraw " << amountWithdrawActual; - return {res, STAmount{}}; - } - - // Withdraw amount2Withdraw - if (amount2WithdrawActual) - { - res = accountSend( - view, - ammAccount, - account_, - *amount2WithdrawActual, - ctx_.journal, - WaiveTransferFee::Yes); - if (res != tesSUCCESS) - { - JLOG(ctx_.journal.debug()) << "AMM Withdraw: failed to withdraw " - << *amount2WithdrawActual; - return {res, STAmount{}}; - } - } - - // Withdraw LP tokens - res = redeemIOU( - view, - account_, - lpTokensWithdrawActual, - lpTokensWithdrawActual.issue(), - ctx_.journal); - if (res != tesSUCCESS) - { - JLOG(ctx_.journal.debug()) - << "AMM Withdraw: failed to withdraw LPTokens"; - return {res, STAmount{}}; - } - - return {tesSUCCESS, lpTokensAMMBalance - lpTokensWithdrawActual}; -} - /** Proportional withdrawal of pool assets for the amount of LPTokens. */ -std::pair +std::tuple> AMMWithdraw::equalWithdrawTokens( Sandbox& view, + SLE const& ammSle, + AccountID const account, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& amount2Balance, STAmount const& lptAMMBalance, STAmount const& lpTokens, STAmount const& lpTokensWithdraw, - std::uint16_t tfee) + std::uint16_t tfee, + beast::Journal const& journal, + STTx const& tx, + bool withdrawAll) { try { // Withdrawing all tokens in the pool if (lpTokensWithdraw == lptAMMBalance) + { return withdraw( view, ammAccount, + account, + ammSle, amountBalance, amountBalance, amount2Balance, lptAMMBalance, lpTokensWithdraw, - tfee); + tfee, + journal, + tx, + true); + } auto const frac = divide(lpTokensWithdraw, lptAMMBalance, noIssue()); auto const withdrawAmount = @@ -639,25 +505,30 @@ AMMWithdraw::equalWithdrawTokens( // withdrawal due to round off. Fail so the user withdraws // more tokens. if (withdrawAmount == beast::zero || withdraw2Amount == beast::zero) - return {tecAMM_FAILED, STAmount{}}; + return {tecAMM_FAILED, STAmount{}, STAmount{}, STAmount{}}; return withdraw( view, ammAccount, + account, + ammSle, amountBalance, withdrawAmount, withdraw2Amount, lptAMMBalance, lpTokensWithdraw, - tfee); + tfee, + journal, + tx, + withdrawAll); } // LCOV_EXCL_START catch (std::exception const& e) { - JLOG(j_.error()) << "AMMWithdraw::equalWithdrawTokens exception " - << e.what(); + JLOG(journal.error()) + << "AMMWithdraw::equalWithdrawTokens exception " << e.what(); } - return {tecINTERNAL, STAmount{}}; + return {tecINTERNAL, STAmount{}, STAmount{}, STAmount{}}; // LCOV_EXCL_STOP } @@ -689,6 +560,7 @@ AMMWithdraw::equalWithdrawTokens( std::pair AMMWithdraw::equalWithdrawLimit( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& amount2Balance, @@ -697,30 +569,50 @@ AMMWithdraw::equalWithdrawLimit( STAmount const& amount2, std::uint16_t tfee) { + TER result; + STAmount newLPTokenBalance; auto frac = Number{amount} / amountBalance; auto const amount2Withdraw = amount2Balance * frac; + bool withdrawAll = + ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); if (amount2Withdraw <= amount2) - return withdraw( - view, - ammAccount, - amountBalance, - amount, - toSTAmount(amount2.issue(), amount2Withdraw), - lptAMMBalance, - toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), - tfee); + { + std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = + withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amount, + toSTAmount(amount2.issue(), amount2Withdraw), + lptAMMBalance, + toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), + tfee, + ctx_.journal, + ctx_.tx, + withdrawAll); + return {result, newLPTokenBalance}; + } + frac = Number{amount2} / amount2Balance; auto const amountWithdraw = amountBalance * frac; assert(amountWithdraw <= amount); - return withdraw( + std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = withdraw( view, ammAccount, + account_, + ammSle, amountBalance, toSTAmount(amount.issue(), amountWithdraw), amount2, lptAMMBalance, toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), - tfee); + tfee, + ctx_.journal, + ctx_.tx, + withdrawAll); + return {result, newLPTokenBalance}; } /** Withdraw single asset equivalent to the amount specified in Asset1Out. @@ -731,6 +623,7 @@ AMMWithdraw::equalWithdrawLimit( std::pair AMMWithdraw::singleWithdraw( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& lptAMMBalance, @@ -740,15 +633,27 @@ AMMWithdraw::singleWithdraw( auto const tokens = lpTokensOut(amountBalance, amount, lptAMMBalance, tfee); if (tokens == beast::zero) return {tecAMM_FAILED, STAmount{}}; - return withdraw( + + TER result; + STAmount newLPTokenBalance; + bool withdrawAll = + ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); + std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = withdraw( view, ammAccount, + account_, + ammSle, amountBalance, amount, std::nullopt, lptAMMBalance, tokens, - tfee); + tfee, + ctx_.journal, + ctx_.tx, + withdrawAll); + + return {result, newLPTokenBalance}; } /** withdrawal of single asset specified in Asset1Out proportional @@ -764,6 +669,7 @@ AMMWithdraw::singleWithdraw( std::pair AMMWithdraw::singleWithdrawTokens( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& lptAMMBalance, @@ -774,15 +680,30 @@ AMMWithdraw::singleWithdrawTokens( auto const amountWithdraw = withdrawByTokens(amountBalance, lptAMMBalance, lpTokensWithdraw, tfee); if (amount == beast::zero || amountWithdraw >= amount) - return withdraw( - view, - ammAccount, - amountBalance, - amountWithdraw, - std::nullopt, - lptAMMBalance, - lpTokensWithdraw, - tfee); + { + TER result; + STAmount newLPTokenBalance; + bool withdrawAll = + ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); + std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = + withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amountWithdraw, + std::nullopt, + lptAMMBalance, + lpTokensWithdraw, + tfee, + ctx_.journal, + ctx_.tx, + withdrawAll); + + return {result, newLPTokenBalance}; + } + return {tecAMM_FAILED, STAmount{}}; } @@ -808,6 +729,7 @@ AMMWithdraw::singleWithdrawTokens( std::pair AMMWithdraw::singleWithdrawEPrice( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& lptAMMBalance, @@ -833,15 +755,29 @@ AMMWithdraw::singleWithdrawEPrice( return {tecAMM_FAILED, STAmount{}}; auto const amountWithdraw = toSTAmount(amount.issue(), tokens / ePrice); if (amount == beast::zero || amountWithdraw >= amount) - return withdraw( - view, - ammAccount, - amountBalance, - amountWithdraw, - std::nullopt, - lptAMMBalance, - toSTAmount(lptAMMBalance.issue(), tokens), - tfee); + { + TER result; + STAmount newLPTokenBalance; + bool withdrawAll = + ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); + std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = + withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amountWithdraw, + std::nullopt, + lptAMMBalance, + toSTAmount(lptAMMBalance.issue(), tokens), + tfee, + ctx_.journal, + ctx_.tx, + withdrawAll); + + return {result, newLPTokenBalance}; + } return {tecAMM_FAILED, STAmount{}}; } diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.h b/src/xrpld/app/tx/detail/AMMWithdraw.h index 9e9920aa5f6..e8147ec65d8 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.h +++ b/src/xrpld/app/tx/detail/AMMWithdraw.h @@ -80,33 +80,6 @@ class AMMWithdraw : public Transactor TER doApply() override; -private: - std::pair - applyGuts(Sandbox& view); - - /** Withdraw requested assets and token from AMM into LP account. - * Return new total LPToken balance. - * @param view - * @param ammAccount - * @param amountBalance - * @param amountWithdraw - * @param amount2Withdraw - * @param lpTokensAMMBalance current AMM LPT balance - * @param lpTokensWithdraw - * @param tfee - * @return - */ - std::pair - withdraw( - Sandbox& view, - AccountID const& ammAccount, - STAmount const& amountWithdraw, - STAmount const& amountBalance, - std::optional const& amount2Withdraw, - STAmount const& lpTokensAMMBalance, - STAmount const& lpTokensWithdraw, - std::uint16_t tfee); - /** Equal-asset withdrawal (LPTokens) of some AMM instance pools * shares represented by the number of LPTokens . * The trading fee is not charged. @@ -120,16 +93,25 @@ class AMMWithdraw : public Transactor * @param tfee trading fee in basis points * @return */ - std::pair + static std::tuple> equalWithdrawTokens( Sandbox& view, + SLE const& ammSle, + AccountID const account, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& amount2Balance, STAmount const& lptAMMBalance, STAmount const& lpTokens, STAmount const& lpTokensWithdraw, - std::uint16_t tfee); + std::uint16_t tfee, + beast::Journal const& journal, + STTx const& tx, + bool withdrawAll); + +private: + std::pair + applyGuts(Sandbox& view); /** Withdraw both assets (Asset1Out, Asset2Out) with the constraints * on the maximum amount of each asset that the trader is willing @@ -147,6 +129,7 @@ class AMMWithdraw : public Transactor std::pair equalWithdrawLimit( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& amount2Balance, @@ -168,6 +151,7 @@ class AMMWithdraw : public Transactor std::pair singleWithdraw( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& lptAMMBalance, @@ -188,6 +172,7 @@ class AMMWithdraw : public Transactor std::pair singleWithdrawTokens( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& lptAMMBalance, @@ -209,6 +194,7 @@ class AMMWithdraw : public Transactor std::pair singleWithdrawEPrice( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& lptAMMBalance, diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index f855ad8578c..4ec611b3762 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -342,11 +342,12 @@ AccountRootsNotDeleted::finalize( return false; } - // A successful AMMWithdraw MAY delete one account root + // A successful AMMWithdraw/AMMClawback MAY delete one account root // when the total AMM LP Tokens balance goes to 0. Not every AMM withdraw // deletes the AMM account, accountsDeleted_ is set if it is deleted. - if (tx.getTxnType() == ttAMM_WITHDRAW && result == tesSUCCESS && - accountsDeleted_ == 1) + if ((tx.getTxnType() == ttAMM_WITHDRAW || + tx.getTxnType() == ttAMM_CLAWBACK) && + result == tesSUCCESS && accountsDeleted_ == 1) return true; if (accountsDeleted_ == 0) diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index cbeabb6fc9c..c02355c97e2 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -132,6 +133,8 @@ with_txn_type(TxType txnType, F&& f) return f.template operator()(); case ttCLAWBACK: return f.template operator()(); + case ttAMM_CLAWBACK: + return f.template operator()(); case ttAMM_CREATE: return f.template operator()(); case ttAMM_DEPOSIT: From 3be6878973bcc6dc555f6d7fbbb287f097a56cd6 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Tue, 24 Sep 2024 15:33:35 -0400 Subject: [PATCH 02/13] Use asset and asset2 to navigate AMM --- src/libxrpl/protocol/SField.cpp | 1 - src/libxrpl/protocol/TxFormats.cpp | 2 +- src/test/app/AMMClawback_test.cpp | 263 ++++++------------------ src/test/jtx/AMM.h | 9 + src/test/jtx/impl/AMM.cpp | 23 +++ src/test/jtx/impl/trust.cpp | 22 -- src/test/jtx/trust.h | 9 - src/xrpld/app/tx/detail/AMMClawback.cpp | 69 ++----- 8 files changed, 110 insertions(+), 288 deletions(-) diff --git a/src/libxrpl/protocol/SField.cpp b/src/libxrpl/protocol/SField.cpp index 1846414039f..4f1f9c91216 100644 --- a/src/libxrpl/protocol/SField.cpp +++ b/src/libxrpl/protocol/SField.cpp @@ -320,7 +320,6 @@ CONSTRUCT_TYPED_SFIELD(sfRegularKey, "RegularKey", ACCOUNT, CONSTRUCT_TYPED_SFIELD(sfNFTokenMinter, "NFTokenMinter", ACCOUNT, 9); CONSTRUCT_TYPED_SFIELD(sfEmitCallback, "EmitCallback", ACCOUNT, 10); CONSTRUCT_TYPED_SFIELD(sfHolder, "Holder", ACCOUNT, 11); -CONSTRUCT_TYPED_SFIELD(sfAMMAccount, "AMMAccount", ACCOUNT, 12); // account (uncommon) CONSTRUCT_TYPED_SFIELD(sfHookAccount, "HookAccount", ACCOUNT, 16); diff --git a/src/libxrpl/protocol/TxFormats.cpp b/src/libxrpl/protocol/TxFormats.cpp index 2d3732c265a..7868560144a 100644 --- a/src/libxrpl/protocol/TxFormats.cpp +++ b/src/libxrpl/protocol/TxFormats.cpp @@ -385,8 +385,8 @@ TxFormats::TxFormats() ttAMM_CLAWBACK, { {sfHolder, soeREQUIRED}, - {sfAMMAccount, soeREQUIRED}, {sfAsset, soeREQUIRED}, + {sfAsset2, soeREQUIRED}, {sfAmount, soeOPTIONAL}, }, commonFields); diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index 67ca3f6a577..f1a44d347a2 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -34,7 +34,7 @@ class AMMClawback_test : public jtx::AMMTest testcase("test invalid request"); using namespace jtx; - // Test if the AMMAccount provided does not exist at all. This should + // Test if asset pair provided does not exist. This should // return terNO_AMM error. { Env env(*this, features); @@ -63,43 +63,8 @@ class AMMClawback_test : public jtx::AMMTest // The AMM account does not exist at all now. // It should return terNO_AMM error. - env(ammClawback( - gw, - alice, - USD, - std::nullopt, - amm.ammAccount(), - std::nullopt), - ter(terNO_AMM)); - } - - // Test if the AMMAccount provided exists, but it is not an AMM account. - // This should return terNO_AMM error. - { - Env env(*this, features); - Account gw{"gateway"}; - Account alice{"alice"}; - env.fund(XRP(10000), gw, alice); - env.close(); - - // gw sets asfAllowTrustLineClawback. - env(fset(gw, asfAllowTrustLineClawback)); - env.close(); - env.require(flags(gw, asfAllowTrustLineClawback)); - - // gw issues 100 USD to Alice. - auto const USD = gw["USD"]; - env.trust(USD(1000), alice); - env(pay(gw, alice, USD(100))); - env.close(); - - AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); - - // gw sends invalid request - // by passing alice account as AMMAccount. This should return - // terNO_AMM. - env(ammClawback( - gw, alice, USD, std::nullopt, alice.id(), std::nullopt), + env(amm::ammClawback( + gw, alice, USD, EUR, std::nullopt, std::nullopt), ter(terNO_AMM)); } @@ -126,8 +91,7 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); // Issuer can not clawback from himself. - env(ammClawback( - gw, gw, USD, std::nullopt, amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, gw, USD, XRP, std::nullopt, std::nullopt), ter(temMALFORMED)); } @@ -154,12 +118,12 @@ class AMMClawback_test : public jtx::AMMTest // The Asset's issuer field is alice, while the Account field is gw. // This should return temBAD_ASSET_ISSUER because they do not match. - env(ammClawback( + env(amm::ammClawback( gw, alice, Issue{gw["USD"].currency, alice.id()}, + XRP, std::nullopt, - amm.ammAccount(), std::nullopt), ter(temBAD_ASSET_ISSUER)); } @@ -188,12 +152,12 @@ class AMMClawback_test : public jtx::AMMTest // The Asset's issuer subfield is gw account and Amount's issuer // subfield is alice account. Return temBAD_ASSET_AMOUNT because // they do not match. - env(ammClawback( + env(amm::ammClawback( gw, alice, USD, + XRP, STAmount{Issue{gw["USD"].currency, alice.id()}, 1}, - amm.ammAccount(), std::nullopt), ter(temBAD_ASSET_AMOUNT)); } @@ -220,22 +184,22 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); // Return temBAD_AMOUNT if the Amount value is less than 0. - env(ammClawback( + env(amm::ammClawback( gw, alice, USD, + XRP, STAmount{Issue{gw["USD"].currency, gw.id()}, -1}, - amm.ammAccount(), std::nullopt), ter(temBAD_AMOUNT)); // Return temBAD_AMOUNT if the Amount value is 0. - env(ammClawback( + env(amm::ammClawback( gw, alice, USD, + XRP, STAmount{Issue{gw["USD"].currency, gw.id()}, 0}, - amm.ammAccount(), std::nullopt), ter(temBAD_AMOUNT)); } @@ -262,47 +226,8 @@ class AMMClawback_test : public jtx::AMMTest // If asfAllowTrustLineClawback is not set, the issuer is not // allowed to send the AMMClawback transaction. - env(ammClawback( - gw, - alice, - USD, - std::nullopt, - amm.ammAccount(), - std::nullopt), - ter(tecNO_PERMISSION)); - } - - // Test if the Asset being clawed back does not exist in the AMM pool. - { - Env env(*this, features); - Account gw{"gateway"}; - Account alice{"alice"}; - env.fund(XRP(10000), gw, alice); - env.close(); - - // gw sets asfAllowTrustLineClawback. - env(fset(gw, asfAllowTrustLineClawback)); - env.close(); - env.require(flags(gw, asfAllowTrustLineClawback)); - - // gw issues 100 USD to Alice. - auto const USD = gw["USD"]; - env.trust(USD(1000), alice); - env(pay(gw, alice, USD(100))); - env.close(); - - // create AMM pool of XRP/USD - AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); - - // Return tecNO_PERMISSION because the Asset EUR does not - // match AMM pool assets XRP/USD. - env(ammClawback( - gw, - alice, - gw["EUR"], - std::nullopt, - amm.ammAccount(), - std::nullopt), + env(amm::ammClawback( + gw, alice, USD, XRP, std::nullopt, std::nullopt), ter(tecNO_PERMISSION)); } @@ -328,13 +253,8 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); // Return temINVALID_FLAG when providing invalid flag. - env(ammClawback( - gw, - alice, - USD, - std::nullopt, - amm.ammAccount(), - tfTwoAssetIfEmpty), + env(amm::ammClawback( + gw, alice, USD, XRP, std::nullopt, tfTwoAssetIfEmpty), ter(temINVALID_FLAG)); } @@ -365,13 +285,8 @@ class AMMClawback_test : public jtx::AMMTest // but the issuer only issues USD in the pool. The issuer is not // allowed to set tfClawTwoAssets flag if he did not issue both // assts in the pool. - env(ammClawback( - gw, - alice, - USD, - std::nullopt, - amm.ammAccount(), - tfClawTwoAssets), + env(amm::ammClawback( + gw, alice, USD, XRP, std::nullopt, tfClawTwoAssets), ter(tecNO_PERMISSION)); } @@ -399,13 +314,8 @@ class AMMClawback_test : public jtx::AMMTest env.close(); // Clawback XRP is prohibited. - env(ammClawback( - gw, - alice, - XRP, - std::nullopt, - amm.ammAccount(), - std::nullopt), + env(amm::ammClawback( + gw, alice, XRP, USD, std::nullopt, std::nullopt), ter(temMALFORMED)); } } @@ -436,9 +346,9 @@ class AMMClawback_test : public jtx::AMMTest // When featureAMMClawback is not enabled, AMMClawback is disabled. // Because when featureAMMClawback is disabled, we can not create - // amm account, use gw account for now for testing purpose. - env(ammClawback( - gw, alice, XRP, std::nullopt, gw.id(), std::nullopt), + // amm account, call amm::ammClawback directly for testing purpose. + env(amm::ammClawback( + gw, alice, USD, XRP, std::nullopt, std::nullopt), ter(temDISABLED)); } } @@ -488,8 +398,7 @@ class AMMClawback_test : public jtx::AMMTest USD(2000), EUR(1000), IOUAmount{1414213562373095, -12})); // gw clawback 1000 USD from the AMM pool. - env(ammClawback( - gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -516,8 +425,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback another 1000 USD from the AMM pool. The AMM pool will // be empty and get deleted. - env(ammClawback( - gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -567,8 +475,7 @@ class AMMClawback_test : public jtx::AMMTest auto aliceXrpBalance = env.balance(alice, XRP); // gw clawback 1000 USD from the AMM pool. - env(ammClawback( - gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -593,8 +500,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback another 1000 USD from the AMM pool. The AMM pool will // be empty and get deleted. - env(ammClawback( - gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -659,8 +565,7 @@ class AMMClawback_test : public jtx::AMMTest USD(4000), EUR(5000), IOUAmount{4472135954999580, -12})); // gw clawback 1000 USD from the AMM pool - env(ammClawback( - gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -684,8 +589,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectLPTokens(alice, IOUAmount{3354101966249685, -12})); // gw clawback another 500 USD from the AMM pool. - env(ammClawback( - gw, alice, USD, USD(500), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(500), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -703,8 +607,7 @@ class AMMClawback_test : public jtx::AMMTest STAmount(EUR, UINT64_C(2874999999999999), -12)); // gw clawback small amount, 1 USD. - env(ammClawback( - gw, alice, USD, USD(1), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -722,8 +625,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 4000 USD, exceeding the current balance. We // will clawback all. - env(ammClawback( - gw, alice, USD, USD(4000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(4000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -806,8 +708,7 @@ class AMMClawback_test : public jtx::AMMTest auto bobXrpBalance = env.balance(bob, XRP); // gw clawback 500 USD from alice in amm - env(ammClawback( - gw, alice, USD, USD(500), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(500), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -831,8 +732,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectLPTokens(bob, IOUAmount{1414213562373095, -9})); // gw clawback 10 USD from bob in amm. - env(ammClawback( - gw, bob, USD, USD(10), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, bob, USD, XRP, USD(10), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -852,8 +752,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectLPTokens(bob, IOUAmount{1400071426749365, -9})); // gw2 clawback 200 EUR from amm2. - env(ammClawback( - gw2, alice, EUR, EUR(200), amm2.ammAccount(), std::nullopt), + env(amm::ammClawback(gw2, alice, EUR, XRP, EUR(200), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -873,8 +772,7 @@ class AMMClawback_test : public jtx::AMMTest // gw claw back 1000 USD from alice in amm, which exceeds alice's // balance. This will clawback all the remaining LP tokens of alice // (corresponding 500 USD / 1000 XRP). - env(ammClawback( - gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -897,8 +795,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 1000 USD from bob in amm, which also exceeds bob's // balance in amm. All bob's lptoken in amm will be consumed, which // corresponds to 990 USD / 1980 XRP - env(ammClawback( - gw, bob, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, bob, USD, XRP, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -919,13 +816,7 @@ class AMMClawback_test : public jtx::AMMTest // gw2 claw back 1000 EUR from alice in amm2, which exceeds alice's // balance. All alice's lptokens will be consumed, which corresponds // to 800EUR / 2400 XRP. - env(ammClawback( - gw2, - alice, - EUR, - EUR(1000), - amm2.ammAccount(), - std::nullopt), + env(amm::ammClawback(gw2, alice, EUR, XRP, EUR(1000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -951,8 +842,7 @@ class AMMClawback_test : public jtx::AMMTest // gw2 claw back 2000 EUR from bib in amm2, which exceeds bob's // balance. All bob's lptokens will be consumed, which corresponds // to 1000EUR / 3000 XRP. - env(ammClawback( - gw2, bob, EUR, EUR(2000), amm2.ammAccount(), std::nullopt), + env(amm::ammClawback(gw2, bob, EUR, XRP, EUR(2000), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1056,8 +946,7 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(carol, gw2["EUR"](2750))); // gw clawback all the bob's USD in amm. (2000 USD / 2500 EUR) - env(ammClawback( - gw, bob, USD, std::nullopt, amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, bob, USD, EUR, std::nullopt, std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1086,13 +975,8 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(carol, gw2["EUR"](2750))); // gw2 clawback all carol's EUR in amm. (1000 USD / 1250 EUR) - env(ammClawback( - gw2, - carol, - EUR, - std::nullopt, - amm.ammAccount(), - std::nullopt), + env(amm::ammClawback( + gw2, carol, EUR, USD, std::nullopt, std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1106,13 +990,8 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(0))); // gw2 clawback all alice's EUR in amm. (4000 USD / 5000 EUR) - env(ammClawback( - gw2, - alice, - EUR, - std::nullopt, - amm.ammAccount(), - std::nullopt), + env(amm::ammClawback( + gw2, alice, EUR, USD, std::nullopt, std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1160,13 +1039,8 @@ class AMMClawback_test : public jtx::AMMTest auto bobXrpBalance = env.balance(bob, XRP); // gw clawback all alice's USD in amm. (1000 USD / 200 XRP) - env(ammClawback( - gw, - alice, - USD, - std::nullopt, - amm.ammAccount(), - std::nullopt), + env(amm::ammClawback( + gw, alice, USD, XRP, std::nullopt, std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1176,8 +1050,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); // gw clawback all bob's USD in amm. (2000 USD / 400 XRP) - env(ammClawback( - gw, bob, USD, std::nullopt, amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, bob, USD, XRP, std::nullopt, std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1242,8 +1115,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectBalances(USD(14000), EUR(3500), IOUAmount(7000))); // gw clawback 1000 USD from carol. - env(ammClawback( - gw, carol, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, carol, USD, EUR, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( @@ -1263,8 +1135,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 1000 USD from bob with tfClawTwoAssets flag. // then the corresponding EUR will also be clawed back // by gw. - env(ammClawback( - gw, bob, USD, USD(1000), amm.ammAccount(), tfClawTwoAssets), + env(amm::ammClawback(gw, bob, USD, EUR, USD(1000), tfClawTwoAssets), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( @@ -1282,13 +1153,8 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); // gw clawback all USD from alice and set tfClawTwoAssets. - env(ammClawback( - gw, - alice, - USD, - std::nullopt, - amm.ammAccount(), - tfClawTwoAssets), + env(amm::ammClawback( + gw, alice, USD, EUR, std::nullopt, tfClawTwoAssets), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances(USD(4000), EUR(1000), IOUAmount(2000))); @@ -1357,22 +1223,22 @@ class AMMClawback_test : public jtx::AMMTest IOUAmount{3674234614174767, -12})); // Issuer does not match with asset. - env(ammClawback( + env(amm::ammClawback( gw, alice, gw2["USD"], + gw["USD"], STAmount{Issue{gw2["USD"].currency, gw2.id()}, 500}, - amm.ammAccount(), std::nullopt), ter(temBAD_ASSET_ISSUER)); // gw2 clawback 500 gw2[USD] from alice. - env(ammClawback( + env(amm::ammClawback( gw2, alice, gw2["USD"], + gw["USD"], STAmount{Issue{gw2["USD"].currency, gw2.id()}, 500}, - amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1392,13 +1258,8 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(bob, gw2["USD"]) == gw2["USD"](2000)); // gw clawback all gw["USD"] from bob. - env(ammClawback( - gw, - bob, - gw["USD"], - std::nullopt, - amm.ammAccount(), - std::nullopt), + env(amm::ammClawback( + gw, bob, gw["USD"], gw2["USD"], std::nullopt, std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1475,8 +1336,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectLPTokens(alice, IOUAmount{4242640687119285, -12})); // gw claws back 1000 USD from gw2. - env(ammClawback( - gw, gw2, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, gw2, USD, EUR, USD(1000), std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1493,8 +1353,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); // gw2 claws back 1000 EUR from gw. - env(ammClawback( - gw2, gw, EUR, EUR(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw2, gw, EUR, USD, EUR(1000), std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1513,8 +1372,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); // gw2 claws back 4000 EUR from alice. - env(ammClawback( - gw2, alice, EUR, EUR(4000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw2, alice, EUR, USD, EUR(4000), std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1561,8 +1419,7 @@ class AMMClawback_test : public jtx::AMMTest // Alice did not deposit in the amm pool. So AMMClawback from Alice // will fail. - env(ammClawback( - gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(1000), std::nullopt), ter(tecINTERNAL)); } diff --git a/src/test/jtx/AMM.h b/src/test/jtx/AMM.h index 77b9c8c9ec6..e3c33a402f5 100644 --- a/src/test/jtx/AMM.h +++ b/src/test/jtx/AMM.h @@ -438,6 +438,15 @@ trust( std::uint32_t flags = 0); Json::Value pay(Account const& account, AccountID const& to, STAmount const& amount); + +Json::Value +ammClawback( + Account const& issuer, + Account const& holder, + Issue const& asset, + Issue const& asset2, + std::optional const& amount, + std::optional flags); } // namespace amm } // namespace jtx diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index c083b6df35c..8431c1e7f21 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -823,6 +823,29 @@ pay(Account const& account, AccountID const& to, STAmount const& amount) jv[jss::Flags] = tfUniversal; return jv; } + +Json::Value +ammClawback( + Account const& issuer, + Account const& holder, + Issue const& asset, + Issue const& asset2, + std::optional const& amount, + std::optional flags) +{ + Json::Value jv; + jv[jss::TransactionType] = jss::AMMClawback; + jv[jss::Account] = issuer.human(); + jv[jss::Holder] = holder.human(); + jv[jss::Asset] = to_json(asset); + jv[jss::Asset2] = to_json(asset2); + if (amount) + jv[jss::Amount] = amount->getJson(JsonOptions::none); + + if (flags) + jv[jss::Flags] = *flags; + return jv; +} } // namespace amm } // namespace jtx } // namespace test diff --git a/src/test/jtx/impl/trust.cpp b/src/test/jtx/impl/trust.cpp index 74886f01aa3..641a0f79f28 100644 --- a/src/test/jtx/impl/trust.cpp +++ b/src/test/jtx/impl/trust.cpp @@ -74,28 +74,6 @@ claw(Account const& account, STAmount const& amount) return jv; } -Json::Value -ammClawback( - Account const& issuer, - Account const& holder, - Issue const& asset, - std::optional const& amount, - AccountID const& ammAccount, - std::optional flags) -{ - Json::Value jv; - jv[jss::TransactionType] = jss::AMMClawback; - jv[jss::Account] = issuer.human(); - jv[jss::Holder] = holder.human(); - jv[jss::AMMAccount] = to_string(ammAccount); - jv[jss::Asset] = to_json(asset); - if (amount) - jv[jss::Amount] = amount->getJson(JsonOptions::none); - - if (flags) - jv[jss::Flags] = *flags; - return jv; -} } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/test/jtx/trust.h b/src/test/jtx/trust.h index db31f50c8bd..f9fddf4871a 100644 --- a/src/test/jtx/trust.h +++ b/src/test/jtx/trust.h @@ -43,15 +43,6 @@ trust( Json::Value claw(Account const& account, STAmount const& amount); -Json::Value -ammClawback( - Account const& issuer, - Account const& holder, - Issue const& asset, - std::optional const& amount, - AccountID const& ammAccount, - std::optional flags); - } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 4512e860d30..36a2a46163f 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -84,18 +84,17 @@ AMMClawback::preflight(PreflightContext const& ctx) TER AMMClawback::preclaim(PreclaimContext const& ctx) { - AccountID const issuer = ctx.tx[sfAccount]; - AccountID const holder = ctx.tx[sfHolder]; - AccountID const ammAccount = ctx.tx[sfAMMAccount]; - + auto const issuer = ctx.tx[sfAccount]; + auto const holder = ctx.tx[sfHolder]; + auto const asset = ctx.tx[sfAsset]; + auto const asset2 = ctx.tx[sfAsset2]; auto const sleIssuer = ctx.view.read(keylet::account(issuer)); auto const sleHolder = ctx.view.read(keylet::account(holder)); - auto const sleAMMAccount = ctx.view.read(keylet::account(ammAccount)); - if (!sleAMMAccount) + auto const ammSle = ctx.view.read(keylet::amm(asset, asset2)); + if (!ammSle) { - JLOG(ctx.j.debug()) - << "AMMClawback: AMMAccount provided does not exist."; + JLOG(ctx.j.debug()) << "AMM Clawback: Invalid asset pair."; return terNO_AMM; } @@ -107,32 +106,10 @@ AMMClawback::preclaim(PreclaimContext const& ctx) (issuerFlagsIn & lsfNoFreeze)) return tecNO_PERMISSION; - auto const ammID = sleAMMAccount->getFieldH256(sfAMMID); - if (!ammID) - { - JLOG(ctx.j.trace()) - << "AMMClawback: AMMAccount field is not an AMM account."; - return terNO_AMM; - } - - auto const sleAMM = ctx.view.read(keylet::amm(ammID)); - if (!sleAMM) - return tecINTERNAL; // LCOV_EXCL_LINE - - STIssue const& asset = sleAMM->getFieldIssue(sfAsset); - STIssue const& asset2 = sleAMM->getFieldIssue(sfAsset2); - - if (ctx.tx[sfAsset] != asset && ctx.tx[sfAsset] != asset2) - { - JLOG(ctx.j.trace()) << "AMMClawback: Asset being clawed back does not " - "match either asset in the AMM pool."; - return tecNO_PERMISSION; - } - auto const flags = ctx.tx.getFlags(); if (flags & tfClawTwoAssets) { - if (asset.issue().account != asset2.issue().account) + if (asset.account != asset2.account) { JLOG(ctx.j.trace()) << "AMMClawback: tfClawTwoAssets can only be enabled when two " @@ -160,38 +137,25 @@ TER AMMClawback::applyGuts(Sandbox& sb) { std::optional const clawAmount = ctx_.tx[~sfAmount]; - AccountID const ammAccount = ctx_.tx[sfAMMAccount]; AccountID const issuer = ctx_.tx[sfAccount]; AccountID const holder = ctx_.tx[sfHolder]; Issue const asset = ctx_.tx[sfAsset]; + Issue const asset2 = ctx_.tx[sfAsset2]; - auto const sleAMMAccount = ctx_.view().read(keylet::account(ammAccount)); - - // should not happen. checked in preclaim. - if (!sleAMMAccount) - return terNO_AMM; // LCOV_EXCL_LINE - - auto const ammID = sleAMMAccount->getFieldH256(sfAMMID); - if (!ammID) - return tecINTERNAL; // LCOV_EXCL_LINE - - auto ammSle = sb.peek(keylet::amm(ammID)); + auto ammSle = sb.peek(keylet::amm(asset, asset2)); if (!ammSle) return tecINTERNAL; // LCOV_EXCL_LINE - auto const tfee = getTradingFee(ctx_.view(), *ammSle, ammAccount); - Issue const& issue1 = ammSle->getFieldIssue(sfAsset).issue(); - Issue const& issue2 = ammSle->getFieldIssue(sfAsset2).issue(); - - Issue otherIssue = issue1; - if (asset == issue1) - otherIssue = issue2; + auto const ammAccount = (*ammSle)[sfAccount]; + auto const accountSle = sb.read(keylet::account(ammAccount)); + if (!accountSle) + return tecINTERNAL; // LCOV_EXCL_LINE auto const expected = ammHolds( sb, *ammSle, asset, - otherIssue, + asset2, FreezeHandling::fhZERO_IF_FROZEN, ctx_.journal); @@ -208,6 +172,7 @@ AMMClawback::applyGuts(Sandbox& sb) if (holdLPtokens == beast::zero) return tecINTERNAL; + auto const tfee = getTradingFee(ctx_.view(), *ammSle, holder); if (!clawAmount) { std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = @@ -243,7 +208,7 @@ AMMClawback::applyGuts(Sandbox& sb) return result; // LCOV_EXCL_LINE auto const res = deleteAMMAccountIfEmpty( - sb, ammSle, newLPTokenBalance, issue1, issue2, j_); + sb, ammSle, newLPTokenBalance, asset, asset2, j_); if (!res.second) return res.first; // LCOV_EXCL_LINE From 01cbe025ae14b5bbaf6d4bfbe48ed21da33a4872 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Tue, 24 Sep 2024 16:08:25 -0400 Subject: [PATCH 03/13] Use tfee as 0 since it is a two asset withdrawal --- src/libxrpl/protocol/STObject.cpp | 7 ------- src/xrpld/app/tx/detail/AMMClawback.cpp | 20 +++++++++++--------- src/xrpld/app/tx/detail/AMMClawback.h | 5 ++--- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/libxrpl/protocol/STObject.cpp b/src/libxrpl/protocol/STObject.cpp index 9b24d9866bc..bde83ec31a1 100644 --- a/src/libxrpl/protocol/STObject.cpp +++ b/src/libxrpl/protocol/STObject.cpp @@ -659,13 +659,6 @@ STObject::getFieldCurrency(SField const& field) const return getFieldByConstRef(field, empty); } -STIssue const& -STObject::getFieldIssue(SField const& field) const -{ - static STIssue const empty{}; - return getFieldByConstRef(field, empty); -} - void STObject::set(std::unique_ptr v) { diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 36a2a46163f..82b7c78de3c 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -172,9 +172,10 @@ AMMClawback::applyGuts(Sandbox& sb) if (holdLPtokens == beast::zero) return tecINTERNAL; - auto const tfee = getTradingFee(ctx_.view(), *ammSle, holder); if (!clawAmount) { + // Because we are doing a two-asset withdrawal, + // tfee is actually not used, so pass tfee as 0. std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = AMMWithdraw::equalWithdrawTokens( sb, @@ -186,7 +187,7 @@ AMMClawback::applyGuts(Sandbox& sb) lptAMMBalance, holdLPtokens, holdLPtokens, - tfee, + 0, ctx_.journal, ctx_.tx, true); @@ -201,8 +202,7 @@ AMMClawback::applyGuts(Sandbox& sb) amountBalance, amount2Balance, lptAMMBalance, - *clawAmount, - tfee); + *clawAmount); if (result != tesSUCCESS) return result; // LCOV_EXCL_LINE @@ -242,8 +242,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( STAmount const& amountBalance, STAmount const& amount2Balance, STAmount const& lptAMMBalance, - STAmount const& amount, - std::uint16_t tfee) + STAmount const& amount) { auto frac = Number{amount} / amountBalance; auto const amount2Withdraw = amount2Balance * frac; @@ -253,7 +252,8 @@ AMMClawback::equalWithdrawMatchingOneAmount( auto const holdLPtokens = ammLPHolds(sb, ammSle, holder, j_); if (lpTokensWithdraw > holdLPtokens) // if lptoken balance less than what the issuer intended to clawback, - // clawback all the tokens + // clawback all the tokens. Because we are doing a two-asset withdrawal, + // tfee is actually not used, so pass tfee as 0. return AMMWithdraw::equalWithdrawTokens( sb, ammSle, @@ -264,11 +264,13 @@ AMMClawback::equalWithdrawMatchingOneAmount( lptAMMBalance, holdLPtokens, holdLPtokens, - tfee, + 0, ctx_.journal, ctx_.tx, true); + // Because we are doing a two-asset withdrawal, + // tfee is actually not used, so pass tfee as 0. return withdraw( sb, ammAccount, @@ -279,7 +281,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( toSTAmount(amount2Balance.issue(), amount2Withdraw), lptAMMBalance, toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), - tfee, + 0, ctx_.journal, ctx_.tx, false); diff --git a/src/xrpld/app/tx/detail/AMMClawback.h b/src/xrpld/app/tx/detail/AMMClawback.h index fcb56af1dc0..0978019e55c 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.h +++ b/src/xrpld/app/tx/detail/AMMClawback.h @@ -48,13 +48,13 @@ class AMMClawback : public Transactor /** Withdraw both assets by providing maximum amount of asset1, * asset2's amount will be calculated according to the current proportion. + * Since it is two-asset withdrawal, tfee is omitted. * @param view * @param ammAccount current AMM account * @param amountBalance current AMM asset1 balance * @param amount2Balance current AMM asset2 balance * @param lptAMMBalance current AMM LPT balance * @param amount asset1 withdraw amount - * @param tfee trading fee in basis points * @return */ std::tuple> @@ -66,8 +66,7 @@ class AMMClawback : public Transactor STAmount const& amountBalance, STAmount const& amount2Balance, STAmount const& lptAMMBalance, - STAmount const& amount, - std::uint16_t tfee); + STAmount const& amount); }; } // namespace ripple From 4bdb0192eaf93148a720ac80ac1167977f843933 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 27 Sep 2024 10:38:08 -0400 Subject: [PATCH 04/13] resolve review comments --- include/xrpl/protocol/SField.h | 1 - include/xrpl/protocol/STObject.h | 2 - include/xrpl/protocol/jss.h | 1 - src/test/app/AMMClawback_test.cpp | 19 +- src/test/app/AMM_test.cpp | 85 +++--- src/xrpld/app/misc/AMMUtils.h | 25 -- src/xrpld/app/misc/detail/AMMUtils.cpp | 200 ------------- src/xrpld/app/tx/detail/AMMClawback.cpp | 39 +-- src/xrpld/app/tx/detail/AMMDeposit.cpp | 27 +- src/xrpld/app/tx/detail/AMMWithdraw.cpp | 376 ++++++++++++++++++------ src/xrpld/app/tx/detail/AMMWithdraw.h | 75 ++++- 11 files changed, 436 insertions(+), 414 deletions(-) diff --git a/include/xrpl/protocol/SField.h b/include/xrpl/protocol/SField.h index 5b505fc3399..a0810f3c7b5 100644 --- a/include/xrpl/protocol/SField.h +++ b/include/xrpl/protocol/SField.h @@ -588,7 +588,6 @@ extern SF_ACCOUNT const sfRegularKey; extern SF_ACCOUNT const sfNFTokenMinter; extern SF_ACCOUNT const sfEmitCallback; extern SF_ACCOUNT const sfHolder; -extern SF_ACCOUNT const sfAMMAccount; // account (uncommon) extern SF_ACCOUNT const sfHookAccount; diff --git a/include/xrpl/protocol/STObject.h b/include/xrpl/protocol/STObject.h index f66ef8e6aed..b3cef83de5f 100644 --- a/include/xrpl/protocol/STObject.h +++ b/include/xrpl/protocol/STObject.h @@ -243,8 +243,6 @@ class STObject : public STBase, public CountedObject getFieldArray(SField const& field) const; const STCurrency& getFieldCurrency(SField const& field) const; - const STIssue& - getFieldIssue(SField const& field) const; /** Get the value of a field. @param A TypedField built from an SField value representing the desired diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index f56858a161a..0702d692f9f 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -48,7 +48,6 @@ JSS(AccountDelete); // transaction type. JSS(AccountRoot); // ledger type. JSS(AccountSet); // transaction type. JSS(AMM); // ledger type -JSS(AMMAccount); // field JSS(AMMBid); // transaction type JSS(AMMClawback); // transaction type. JSS(AMMID); // field diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index f1a44d347a2..812193932a5 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -93,6 +93,11 @@ class AMMClawback_test : public jtx::AMMTest // Issuer can not clawback from himself. env(amm::ammClawback(gw, gw, USD, XRP, std::nullopt, std::nullopt), ter(temMALFORMED)); + + // Holder can not clawback from himself. + env(amm::ammClawback( + alice, alice, USD, XRP, std::nullopt, std::nullopt), + ter(temMALFORMED)); } // Test if the Asset field matches the Account field. @@ -640,11 +645,11 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(!amm.ammExists()); } - // Test AMMClawback for USD/XRP pool. The assets are issued by different - // issuer. Claw back USD for multiple times, and XRP goes back to the - // holder. The last AMMClawback transaction exceeds the holder's USD - // balance in AMM pool. In this case, gw creates the AMM pool USD/XRP, - // both alice and bob deposit into it. gw2 creates the AMM pool EUR/XRP. + // Test AMMClawback for USD/XRP pool. Claw back USD for multiple times, + // and XRP goes back to the holder. The last AMMClawback transaction + // exceeds the holder's USD balance in AMM pool. In this case, gw + // creates the AMM pool USD/XRP, both alice and bob deposit into it. gw2 + // creates the AMM pool EUR/XRP. { Env env(*this, features); Account gw{"gateway"}; @@ -1000,8 +1005,8 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(!amm.ammExists()); } - // Test AMMClawback for USD/XRP pool. The assets are issued by different - // issuer. Claw back all the USD for different users. + // Test AMMClawback for USD/XRP pool. Claw back all the USD for + // different users. { Env env(*this, features); Account gw{"gateway"}; diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 26c18d3f4b4..feee7eae603 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -6951,44 +6951,45 @@ struct AMM_test : public jtx::AMMTest using namespace jtx; // This lambda function is used to create trustlines - // between gw and alice, and create and return an AMM account. - auto setupAMM = [&](Env& env) -> AMM* { + // between gw and alice, and create an AMM account. + // And also test the callback function. + auto testAMMDeposit = [&](Env& env, std::function cb) { env.fund(XRP(1'000), gw); fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}, Fund::Acct); env.close(); AMM* amm = new AMM(env, alice, XRP(100), USD(100), ter(tesSUCCESS)); env(trust(gw, alice["USD"](0), tfSetFreeze)); - return amm; + cb(*amm); }; + // Deposit two assets, one of which is frozen, + // then we should get tecFROZEN error. { - // Deposit two assets, one of which is frozen, - // then we should get tecFROZEN error. Env env(*this, features); - auto amm = setupAMM(env); - amm->deposit( - alice, - USD(100), - XRP(100), - std::nullopt, - tfTwoAsset, - ter(tecFROZEN)); - delete amm; + testAMMDeposit(env, [&](AMM& amm) { + amm.deposit( + alice, + USD(100), + XRP(100), + std::nullopt, + tfTwoAsset, + ter(tecFROZEN)); + }); } + // Deposit one asset, which is the frozen token, + // then we should get tecFROZEN error. { - // Deposit one asset, which is the frozen token, - // then we should get tecFROZEN error. Env env(*this, features); - auto amm = setupAMM(env); - amm->deposit( - alice, - USD(100), - std::nullopt, - std::nullopt, - tfSingleAsset, - ter(tecFROZEN)); - delete amm; + testAMMDeposit(env, [&](AMM& amm) { + amm.deposit( + alice, + USD(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tecFROZEN)); + }); } if (features[featureAMMClawback]) @@ -6997,14 +6998,15 @@ struct AMM_test : public jtx::AMMTest // but the other asset is frozen. We should get tecFROZEN error // when feature AMMClawback is enabled. Env env(*this, features); - auto amm = setupAMM(env); - amm->deposit( - alice, - XRP(100), - std::nullopt, - std::nullopt, - tfSingleAsset, - ter(tecFROZEN)); + testAMMDeposit(env, [&](AMM& amm) { + amm.deposit( + alice, + XRP(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tecFROZEN)); + }); } else { @@ -7012,14 +7014,15 @@ struct AMM_test : public jtx::AMMTest // but the other asset is frozen. We will get tecSUCCESS // when feature AMMClawback is not enabled. Env env(*this, features); - auto amm = setupAMM(env); - amm->deposit( - alice, - XRP(100), - std::nullopt, - std::nullopt, - tfSingleAsset, - ter(tesSUCCESS)); + testAMMDeposit(env, [&](AMM& amm) { + amm.deposit( + alice, + XRP(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tesSUCCESS)); + }); } } diff --git a/src/xrpld/app/misc/AMMUtils.h b/src/xrpld/app/misc/AMMUtils.h index e02ef6158ad..52fe819a28e 100644 --- a/src/xrpld/app/misc/AMMUtils.h +++ b/src/xrpld/app/misc/AMMUtils.h @@ -123,31 +123,6 @@ isOnlyLiquidityProvider( Issue const& ammIssue, AccountID const& lpAccount); -std::tuple> -withdraw( - Sandbox& view, - AccountID const& ammAccount, - AccountID const& account, - SLE const& ammSle, - STAmount const& amountBalance, - STAmount const& amountWithdraw, - std::optional const& amount2Withdraw, - STAmount const& lpTokensAMMBalance, - STAmount const& lpTokensWithdraw, - std::uint16_t tfee, - beast::Journal const& journal, - STTx const& tx, - bool withdrawAll); - -std::pair -deleteAMMAccountIfEmpty( - Sandbox& sb, - std::shared_ptr const ammSle, - STAmount const& lpTokenBalance, - Issue const& issue1, - Issue const& issue2, - beast::Journal const& journal); - } // namespace ripple #endif // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED diff --git a/src/xrpld/app/misc/detail/AMMUtils.cpp b/src/xrpld/app/misc/detail/AMMUtils.cpp index b5aeb10326f..d5af0ee02ef 100644 --- a/src/xrpld/app/misc/detail/AMMUtils.cpp +++ b/src/xrpld/app/misc/detail/AMMUtils.cpp @@ -432,204 +432,4 @@ isOnlyLiquidityProvider( return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE } -std::tuple> -withdraw( - Sandbox& view, - AccountID const& ammAccount, - AccountID const& account, - SLE const& ammSle, - STAmount const& amountBalance, - STAmount const& amountWithdraw, - std::optional const& amount2Withdraw, - STAmount const& lpTokensAMMBalance, - STAmount const& lpTokensWithdraw, - std::uint16_t tfee, - beast::Journal const& journal, - STTx const& tx, - bool withdrawAll) -{ - auto const lpTokens = ammLPHolds(view, ammSle, account, journal); - auto const expected = ammHolds( - view, - ammSle, - amountWithdraw.issue(), - std::nullopt, - FreezeHandling::fhZERO_IF_FROZEN, - journal); - // LCOV_EXCL_START - if (!expected) - return {expected.error(), STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - auto const [curBalance, curBalance2, _] = *expected; - (void)_; - - auto const - [amountWithdrawActual, amount2WithdrawActual, lpTokensWithdrawActual] = - [&]() -> std::tuple, STAmount> { - if (!withdrawAll) - return adjustAmountsByLPTokens( - amountBalance, - amountWithdraw, - amount2Withdraw, - lpTokensAMMBalance, - lpTokensWithdraw, - tfee, - false); - return std::make_tuple( - amountWithdraw, amount2Withdraw, lpTokensWithdraw); - }(); - - if (lpTokensWithdrawActual <= beast::zero || - lpTokensWithdrawActual > lpTokens) - { - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw, invalid LP tokens: " - << lpTokensWithdrawActual << " " << lpTokens << " " - << lpTokensAMMBalance; - return {tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, STAmount{}}; - } - - // Should not happen since the only LP on last withdraw - // has the balance set to the lp token trustline balance. - if (view.rules().enabled(fixAMMv1_1) && - lpTokensWithdrawActual > lpTokensAMMBalance) - { - // LCOV_EXCL_START - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw, unexpected LP tokens: " - << lpTokensWithdrawActual << " " << lpTokens << " " - << lpTokensAMMBalance; - return {tecINTERNAL, STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - } - - // Withdrawing one side of the pool - if ((amountWithdrawActual == curBalance && - amount2WithdrawActual != curBalance2) || - (amount2WithdrawActual == curBalance2 && - amountWithdrawActual != curBalance)) - { - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw one side of the pool " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; - } - - // May happen if withdrawing an amount close to one side of the pool - if (lpTokensWithdrawActual == lpTokensAMMBalance && - (amountWithdrawActual != curBalance || - amount2WithdrawActual != curBalance2)) - { - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw all tokens " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " curBalance2: " << amount2WithdrawActual.value_or(STAmount{0}) - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; - } - - // Withdrawing more than the pool's balance - if (amountWithdrawActual > curBalance || - amount2WithdrawActual > curBalance2) - { - JLOG(journal.debug()) - << "AMM Withdraw: withdrawing more than the pool's balance " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " curBalance2: " << curBalance2 << " " - << (amount2WithdrawActual ? *amount2WithdrawActual : STAmount{}) - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; - } - - // Withdraw amountWithdraw - auto res = accountSend( - view, - ammAccount, - account, - amountWithdrawActual, - journal, - WaiveTransferFee::Yes); - if (res != tesSUCCESS) - { - // LCOV_EXCL_START - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw " << amountWithdrawActual; - return {res, STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - } - - // Withdraw amount2Withdraw - if (amount2WithdrawActual) - { - res = accountSend( - view, - ammAccount, - account, - *amount2WithdrawActual, - journal, - WaiveTransferFee::Yes); - if (res != tesSUCCESS) - { - // LCOV_EXCL_START - JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw " - << *amount2WithdrawActual; - return {res, STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - } - } - - // Withdraw LP tokens - res = redeemIOU( - view, - account, - lpTokensWithdrawActual, - lpTokensWithdrawActual.issue(), - journal); - if (res != tesSUCCESS) - { - // LCOV_EXCL_START - JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw LPTokens"; - return {res, STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - } - - return std::make_tuple( - tesSUCCESS, - lpTokensAMMBalance - lpTokensWithdrawActual, - amountWithdrawActual, - amount2WithdrawActual); -} - -std::pair -deleteAMMAccountIfEmpty( - Sandbox& sb, - std::shared_ptr const ammSle, - STAmount const& lpTokenBalance, - Issue const& issue1, - Issue const& issue2, - beast::Journal const& journal) -{ - TER ter; - bool updateBalance = true; - if (lpTokenBalance == beast::zero) - { - ter = deleteAMMAccount(sb, issue1, issue2, journal); - if (ter != tesSUCCESS && ter != tecINCOMPLETE) - return {ter, false}; // LCOV_EXCL_LINE - else - updateBalance = (ter == tecINCOMPLETE); - } - - if (updateBalance) - { - ammSle->setFieldAmount(sfLPTokenBalance, lpTokenBalance); - sb.update(ammSle); - } - - return {ter, true}; -} } // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 82b7c78de3c..a003932f61a 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -84,12 +84,11 @@ AMMClawback::preflight(PreflightContext const& ctx) TER AMMClawback::preclaim(PreclaimContext const& ctx) { - auto const issuer = ctx.tx[sfAccount]; - auto const holder = ctx.tx[sfHolder]; auto const asset = ctx.tx[sfAsset]; auto const asset2 = ctx.tx[sfAsset2]; - auto const sleIssuer = ctx.view.read(keylet::account(issuer)); - auto const sleHolder = ctx.view.read(keylet::account(holder)); + auto const sleIssuer = ctx.view.read(keylet::account(ctx.tx[sfAccount])); + if (!sleIssuer) + return terNO_ACCOUNT; // LCOV_EXCL_LINE auto const ammSle = ctx.view.read(keylet::amm(asset, asset2)); if (!ammSle) @@ -107,15 +106,12 @@ AMMClawback::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; auto const flags = ctx.tx.getFlags(); - if (flags & tfClawTwoAssets) + if (flags & tfClawTwoAssets && asset.account != asset2.account) { - if (asset.account != asset2.account) - { - JLOG(ctx.j.trace()) - << "AMMClawback: tfClawTwoAssets can only be enabled when two " - "assets in the AMM pool are both issued by the issuer"; - return tecNO_PERMISSION; - } + JLOG(ctx.j.trace()) + << "AMMClawback: tfClawTwoAssets can only be enabled when two " + "assets in the AMM pool are both issued by the issuer"; + return tecNO_PERMISSION; } return tesSUCCESS; @@ -188,9 +184,8 @@ AMMClawback::applyGuts(Sandbox& sb) holdLPtokens, holdLPtokens, 0, - ctx_.journal, - ctx_.tx, - true); + WithdrawAll::Yes, + ctx_.journal); } else std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = @@ -207,7 +202,7 @@ AMMClawback::applyGuts(Sandbox& sb) if (result != tesSUCCESS) return result; // LCOV_EXCL_LINE - auto const res = deleteAMMAccountIfEmpty( + auto const res = AMMWithdraw::deleteAMMAccountIfEmpty( sb, ammSle, newLPTokenBalance, asset, asset2, j_); if (!res.second) return res.first; // LCOV_EXCL_LINE @@ -265,13 +260,12 @@ AMMClawback::equalWithdrawMatchingOneAmount( holdLPtokens, holdLPtokens, 0, - ctx_.journal, - ctx_.tx, - true); + WithdrawAll::Yes, + ctx_.journal); // Because we are doing a two-asset withdrawal, // tfee is actually not used, so pass tfee as 0. - return withdraw( + return AMMWithdraw::withdraw( sb, ammAccount, holder, @@ -282,9 +276,8 @@ AMMClawback::equalWithdrawMatchingOneAmount( lptAMMBalance, toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), 0, - ctx_.journal, - ctx_.tx, - false); + WithdrawAll::No, + ctx_.journal); } } // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index d9bd22a0097..c73b833f9c6 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -248,30 +248,21 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) { // Check if either of the assets is frozen, AMMDeposit is not allowed // if either asset is frozen - auto checkAsset = [&](std::optional const& asset) -> TER { - if (!asset.has_value()) - return temMALFORMED; // LCOV_EXCL_LINE - - if (isFrozen(ctx.view, accountID, asset.value())) + auto checkAsset = [&](Issue const& asset) -> bool { + if (isFrozen(ctx.view, accountID, asset)) { - JLOG(ctx.j.debug()) << "AMM Deposit: account is frozen, " - << to_string(accountID) << " " - << to_string(asset->currency); + JLOG(ctx.j.debug()) + << "AMM Deposit: account or currency is frozen, " + << to_string(accountID) << " " << to_string(asset.currency); - return tecFROZEN; + return false; } - return tesSUCCESS; + return true; }; - auto const asset = ctx.tx[~sfAsset]; - auto const asset2 = ctx.tx[~sfAsset2]; - - if (auto const ter = checkAsset(asset)) - return ter; - - if (auto const ter = checkAsset(asset2)) - return ter; + if (!checkAsset(ctx.tx[sfAsset]) || !checkAsset(ctx.tx[sfAsset2])) + return tecFROZEN; } auto const amount = ctx.tx[~sfAmount]; diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 9dd71d1f6f8..843384d3778 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -399,8 +399,6 @@ AMMWithdraw::applyGuts(Sandbox& sb) { TER result; STAmount newLPTokenBalance; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = equalWithdrawTokens( sb, @@ -413,9 +411,8 @@ AMMWithdraw::applyGuts(Sandbox& sb) lpTokens, *lpTokensWithdraw, tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); + isWithdrawAll(ctx_.tx), + ctx_.journal); return {result, newLPTokenBalance}; } // should not happen. @@ -456,6 +453,237 @@ AMMWithdraw::doApply() return result.first; } +std::pair +AMMWithdraw::withdraw( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee) +{ + TER ter; + STAmount newLPTokenBalance; + std::tie(ter, newLPTokenBalance, std::ignore, std::ignore) = withdraw( + view, + ammAccount, + account, + ammSle, + amountBalance, + amountWithdraw, + amount2Withdraw, + lpTokensAMMBalance, + lpTokensWithdraw, + tfee, + isWithdrawAll(ctx_.tx), + j_); + return {ter, newLPTokenBalance}; +} + +std::tuple> +AMMWithdraw::withdraw( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee, + WithdrawAll withdrawAll, + beast::Journal const& journal) +{ + auto const lpTokens = ammLPHolds(view, ammSle, account, journal); + auto const expected = ammHolds( + view, + ammSle, + amountWithdraw.issue(), + std::nullopt, + FreezeHandling::fhZERO_IF_FROZEN, + journal); + // LCOV_EXCL_START + if (!expected) + return {expected.error(), STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + auto const [curBalance, curBalance2, _] = *expected; + (void)_; + + auto const + [amountWithdrawActual, amount2WithdrawActual, lpTokensWithdrawActual] = + [&]() -> std::tuple, STAmount> { + if (withdrawAll == WithdrawAll::No) + return adjustAmountsByLPTokens( + amountBalance, + amountWithdraw, + amount2Withdraw, + lpTokensAMMBalance, + lpTokensWithdraw, + tfee, + false); + return std::make_tuple( + amountWithdraw, amount2Withdraw, lpTokensWithdraw); + }(); + + if (lpTokensWithdrawActual <= beast::zero || + lpTokensWithdrawActual > lpTokens) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw, invalid LP tokens: " + << lpTokensWithdrawActual << " " << lpTokens << " " + << lpTokensAMMBalance; + return {tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, STAmount{}}; + } + + // Should not happen since the only LP on last withdraw + // has the balance set to the lp token trustline balance. + if (view.rules().enabled(fixAMMv1_1) && + lpTokensWithdrawActual > lpTokensAMMBalance) + { + // LCOV_EXCL_START + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw, unexpected LP tokens: " + << lpTokensWithdrawActual << " " << lpTokens << " " + << lpTokensAMMBalance; + return {tecINTERNAL, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + // Withdrawing one side of the pool + if ((amountWithdrawActual == curBalance && + amount2WithdrawActual != curBalance2) || + (amount2WithdrawActual == curBalance2 && + amountWithdrawActual != curBalance)) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw one side of the pool " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // May happen if withdrawing an amount close to one side of the pool + if (lpTokensWithdrawActual == lpTokensAMMBalance && + (amountWithdrawActual != curBalance || + amount2WithdrawActual != curBalance2)) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw all tokens " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " curBalance2: " << amount2WithdrawActual.value_or(STAmount{0}) + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // Withdrawing more than the pool's balance + if (amountWithdrawActual > curBalance || + amount2WithdrawActual > curBalance2) + { + JLOG(journal.debug()) + << "AMM Withdraw: withdrawing more than the pool's balance " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " curBalance2: " << curBalance2 << " " + << (amount2WithdrawActual ? *amount2WithdrawActual : STAmount{}) + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // Withdraw amountWithdraw + auto res = accountSend( + view, + ammAccount, + account, + amountWithdrawActual, + journal, + WaiveTransferFee::Yes); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw " << amountWithdrawActual; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + // Withdraw amount2Withdraw + if (amount2WithdrawActual) + { + res = accountSend( + view, + ammAccount, + account, + *amount2WithdrawActual, + journal, + WaiveTransferFee::Yes); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw " + << *amount2WithdrawActual; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + } + + // Withdraw LP tokens + res = redeemIOU( + view, + account, + lpTokensWithdrawActual, + lpTokensWithdrawActual.issue(), + journal); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw LPTokens"; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + return std::make_tuple( + tesSUCCESS, + lpTokensAMMBalance - lpTokensWithdrawActual, + amountWithdrawActual, + amount2WithdrawActual); +} + +std::pair +AMMWithdraw::deleteAMMAccountIfEmpty( + Sandbox& sb, + std::shared_ptr const ammSle, + STAmount const& lpTokenBalance, + Issue const& issue1, + Issue const& issue2, + beast::Journal const& journal) +{ + TER ter; + bool updateBalance = true; + if (lpTokenBalance == beast::zero) + { + ter = deleteAMMAccount(sb, issue1, issue2, journal); + if (ter != tesSUCCESS && ter != tecINCOMPLETE) + return {ter, false}; // LCOV_EXCL_LINE + else + updateBalance = (ter == tecINCOMPLETE); + } + + if (updateBalance) + { + ammSle->setFieldAmount(sfLPTokenBalance, lpTokenBalance); + sb.update(ammSle); + } + + return {ter, true}; +} + /** Proportional withdrawal of pool assets for the amount of LPTokens. */ std::tuple> @@ -470,9 +698,8 @@ AMMWithdraw::equalWithdrawTokens( STAmount const& lpTokens, STAmount const& lpTokensWithdraw, std::uint16_t tfee, - beast::Journal const& journal, - STTx const& tx, - bool withdrawAll) + WithdrawAll withdrawAll, + beast::Journal const& journal) { try { @@ -490,9 +717,8 @@ AMMWithdraw::equalWithdrawTokens( lptAMMBalance, lpTokensWithdraw, tfee, - journal, - tx, - true); + WithdrawAll::Yes, + journal); } auto const frac = divide(lpTokensWithdraw, lptAMMBalance, noIssue()); @@ -518,9 +744,8 @@ AMMWithdraw::equalWithdrawTokens( lptAMMBalance, lpTokensWithdraw, tfee, - journal, - tx, - withdrawAll); + withdrawAll, + journal); } // LCOV_EXCL_START catch (std::exception const& e) @@ -569,36 +794,27 @@ AMMWithdraw::equalWithdrawLimit( STAmount const& amount2, std::uint16_t tfee) { - TER result; - STAmount newLPTokenBalance; auto frac = Number{amount} / amountBalance; auto const amount2Withdraw = amount2Balance * frac; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); if (amount2Withdraw <= amount2) { - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = - withdraw( - view, - ammAccount, - account_, - ammSle, - amountBalance, - amount, - toSTAmount(amount2.issue(), amount2Withdraw), - lptAMMBalance, - toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), - tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - return {result, newLPTokenBalance}; + return withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amount, + toSTAmount(amount2.issue(), amount2Withdraw), + lptAMMBalance, + toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), + tfee); } frac = Number{amount2} / amount2Balance; auto const amountWithdraw = amountBalance * frac; assert(amountWithdraw <= amount); - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = withdraw( + return withdraw( view, ammAccount, account_, @@ -608,11 +824,7 @@ AMMWithdraw::equalWithdrawLimit( amount2, lptAMMBalance, toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), - tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - return {result, newLPTokenBalance}; + tfee); } /** Withdraw single asset equivalent to the amount specified in Asset1Out. @@ -634,11 +846,7 @@ AMMWithdraw::singleWithdraw( if (tokens == beast::zero) return {tecAMM_FAILED, STAmount{}}; - TER result; - STAmount newLPTokenBalance; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = withdraw( + return withdraw( view, ammAccount, account_, @@ -648,12 +856,7 @@ AMMWithdraw::singleWithdraw( std::nullopt, lptAMMBalance, tokens, - tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - - return {result, newLPTokenBalance}; + tfee); } /** withdrawal of single asset specified in Asset1Out proportional @@ -681,27 +884,17 @@ AMMWithdraw::singleWithdrawTokens( withdrawByTokens(amountBalance, lptAMMBalance, lpTokensWithdraw, tfee); if (amount == beast::zero || amountWithdraw >= amount) { - TER result; - STAmount newLPTokenBalance; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = - withdraw( - view, - ammAccount, - account_, - ammSle, - amountBalance, - amountWithdraw, - std::nullopt, - lptAMMBalance, - lpTokensWithdraw, - tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - - return {result, newLPTokenBalance}; + return withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amountWithdraw, + std::nullopt, + lptAMMBalance, + lpTokensWithdraw, + tfee); } return {tecAMM_FAILED, STAmount{}}; @@ -756,30 +949,27 @@ AMMWithdraw::singleWithdrawEPrice( auto const amountWithdraw = toSTAmount(amount.issue(), tokens / ePrice); if (amount == beast::zero || amountWithdraw >= amount) { - TER result; - STAmount newLPTokenBalance; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = - withdraw( - view, - ammAccount, - account_, - ammSle, - amountBalance, - amountWithdraw, - std::nullopt, - lptAMMBalance, - toSTAmount(lptAMMBalance.issue(), tokens), - tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - - return {result, newLPTokenBalance}; + return withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amountWithdraw, + std::nullopt, + lptAMMBalance, + toSTAmount(lptAMMBalance.issue(), tokens), + tfee); } return {tecAMM_FAILED, STAmount{}}; } +WithdrawAll +AMMWithdraw::isWithdrawAll(STTx const& tx) +{ + if (tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll)) + return WithdrawAll::Yes; + return WithdrawAll::No; +} } // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.h b/src/xrpld/app/tx/detail/AMMWithdraw.h index e8147ec65d8..ddbf8f550ae 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.h +++ b/src/xrpld/app/tx/detail/AMMWithdraw.h @@ -62,6 +62,9 @@ class Sandbox; * @see [XLS30d:AMMWithdraw * transaction](https://github.com/XRPLF/XRPL-Standards/discussions/78) */ + +enum class WithdrawAll : bool { No = false, Yes }; + class AMMWithdraw : public Transactor { public: @@ -91,6 +94,7 @@ class AMMWithdraw : public Transactor * @param lpTokens current LPT balance * @param lpTokensWithdraw amount of tokens to withdraw * @param tfee trading fee in basis points + * @param withdrawAll if withdrawing all lptokens * @return */ static std::tuple> @@ -105,14 +109,75 @@ class AMMWithdraw : public Transactor STAmount const& lpTokens, STAmount const& lpTokensWithdraw, std::uint16_t tfee, - beast::Journal const& journal, - STTx const& tx, - bool withdrawAll); + WithdrawAll withdrawAll, + beast::Journal const& journal); + + /** Withdraw requested assets and token from AMM into LP account. + * Return new total LPToken balance and the withdrawn amounts for both + * assets. + * @param view + * @param ammAccount + * @param amountBalance current LP asset1 balance + * @param amountWithdraw asset1 withdraw amount + * @param amount2Withdraw asset2 withdraw amount + * @param lpTokensAMMBalance current AMM LPT balance + * @param lpTokensWithdraw amount of lptokens to withdraw + * @param tfee trading fee in basis points + * @param withdrawAll if withdraw all lptokens + * @return + */ + static std::tuple> + withdraw( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee, + WithdrawAll withdrawAll, + beast::Journal const& journal); + + static std::pair + deleteAMMAccountIfEmpty( + Sandbox& sb, + std::shared_ptr const ammSle, + STAmount const& lpTokenBalance, + Issue const& issue1, + Issue const& issue2, + beast::Journal const& journal); private: std::pair applyGuts(Sandbox& view); + /** Withdraw requested assets and token from AMM into LP account. + * Return new total LPToken balance. + * @param view + * @param ammAccount + * @param amountBalance current LP asset1 balance + * @param amountWithdraw asset1 withdraw amount + * @param amount2Withdraw asset2 withdraw amount + * @param lpTokensAMMBalance current AMM LPT balance + * @param lpTokensWithdraw amount of lptokens to withdraw + * @return + */ + std::pair + withdraw( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee); + /** Withdraw both assets (Asset1Out, Asset2Out) with the constraints * on the maximum amount of each asset that the trader is willing * to withdraw. The trading fee is not charged. @@ -201,6 +266,10 @@ class AMMWithdraw : public Transactor STAmount const& amount, STAmount const& ePrice, std::uint16_t tfee); + + /** Check from the flags if it's withdraw all */ + WithdrawAll + isWithdrawAll(STTx const& tx); }; } // namespace ripple From 2796e8daf395500254ca28dd86753bd632bbdb77 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Mon, 30 Sep 2024 21:11:42 -0400 Subject: [PATCH 05/13] use fhIGNORE_FREEZE for AMMClawback --- src/test/app/AMMClawback_test.cpp | 289 +++++++++++++++++++++++- src/test/app/AMM_test.cpp | 4 +- src/xrpld/app/misc/detail/AMMUtils.cpp | 2 - src/xrpld/app/tx/detail/AMMClawback.cpp | 6 +- src/xrpld/app/tx/detail/AMMWithdraw.cpp | 75 +++--- src/xrpld/app/tx/detail/AMMWithdraw.h | 29 ++- 6 files changed, 371 insertions(+), 34 deletions(-) diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index 812193932a5..0d4d0ebd8b4 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -1428,7 +1428,293 @@ class AMMClawback_test : public jtx::AMMTest ter(tecINTERNAL)); } -public: + void + testAssetFrozen(FeatureBitset features) + { + testcase("test assets frozen"); + using namespace jtx; + + // test individually frozen trustline. + { + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 3000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(3000))); + env.close(); + env.require(balance(alice, gw["USD"](3000))); + + // gw2 issues 3000 EUR to Alice. + auto const EUR = gw2["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw2, alice, EUR(3000))); + env.close(); + env.require(balance(alice, gw2["EUR"](3000))); + + // Alice creates AMM pool of EUR/USD. + AMM amm(env, alice, EUR(1000), USD(2000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + USD(2000), EUR(1000), IOUAmount{1414213562373095, -12})); + + // freeze trustline + env(trust(gw, alice["USD"](0), tfSetFreeze)); + env.close(); + + // gw clawback 1000 USD from the AMM pool. + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw["USD"](1000))); + env.require(balance(alice, gw2["EUR"](2500))); + BEAST_EXPECT(amm.expectBalances( + USD(1000), EUR(500), IOUAmount{7071067811865475, -13})); + + // Alice has half of its initial lptokens Left. + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13})); + + // gw clawback another 1000 USD from the AMM pool. The AMM pool will + // be empty and get deleted. + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + // Alice should still has 1000 USD because gw clawed back from the + // AMM pool. + env.require(balance(alice, gw["USD"](1000))); + env.require(balance(alice, gw2["EUR"](3000))); + + // amm is automatically deleted. + BEAST_EXPECT(!amm.ammExists()); + } + + // test individually frozen trustline of both USD and EUR currency. + { + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 3000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(3000))); + env.close(); + env.require(balance(alice, gw["USD"](3000))); + + // gw2 issues 3000 EUR to Alice. + auto const EUR = gw2["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw2, alice, EUR(3000))); + env.close(); + env.require(balance(alice, gw2["EUR"](3000))); + + // Alice creates AMM pool of EUR/USD. + AMM amm(env, alice, EUR(1000), USD(2000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + USD(2000), EUR(1000), IOUAmount{1414213562373095, -12})); + + // freeze trustlines + env(trust(gw, alice["USD"](0), tfSetFreeze)); + env(trust(gw2, alice["EUR"](0), tfSetFreeze)); + env.close(); + + // gw clawback 1000 USD from the AMM pool. + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw["USD"](1000))); + env.require(balance(alice, gw2["EUR"](2500))); + BEAST_EXPECT(amm.expectBalances( + USD(1000), EUR(500), IOUAmount{7071067811865475, -13})); + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13})); + } + + // test gw global freeze. + { + Env env(*this, features); + Account gw{"gateway"}; + Account gw2{"gateway2"}; + Account alice{"alice"}; + env.fund(XRP(1000000), gw, gw2, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 3000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(3000))); + env.close(); + env.require(balance(alice, gw["USD"](3000))); + + // gw2 issues 3000 EUR to Alice. + auto const EUR = gw2["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw2, alice, EUR(3000))); + env.close(); + env.require(balance(alice, gw2["EUR"](3000))); + + // Alice creates AMM pool of EUR/USD. + AMM amm(env, alice, EUR(1000), USD(2000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + USD(2000), EUR(1000), IOUAmount{1414213562373095, -12})); + + // global freeze + env(fset(gw, asfGlobalFreeze)); + env.close(); + + // gw clawback 1000 USD from the AMM pool. + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + ter(tesSUCCESS)); + env.close(); + + env.require(balance(alice, gw["USD"](1000))); + env.require(balance(alice, gw2["EUR"](2500))); + BEAST_EXPECT(amm.expectBalances( + USD(1000), EUR(500), IOUAmount{7071067811865475, -13})); + BEAST_EXPECT( + amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13})); + } + + // Test both assets are issued by the same issuer. And issuer sets + // global freeze. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + Account bob{"bob"}; + Account carol{"carol"}; + env.fund(XRP(1000000), gw, alice, bob, carol); + env.close(); + + // gw sets asfAllowTrustLineClawback. + 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(10000))); + env.trust(USD(100000), bob); + env(pay(gw, bob, USD(9000))); + env.trust(USD(100000), carol); + env(pay(gw, carol, USD(8000))); + env.close(); + + auto const EUR = gw["EUR"]; + env.trust(EUR(100000), alice); + env(pay(gw, alice, EUR(10000))); + env.trust(EUR(100000), bob); + env(pay(gw, bob, EUR(9000))); + env.trust(EUR(100000), carol); + env(pay(gw, carol, EUR(8000))); + env.close(); + + AMM amm(env, alice, EUR(2000), USD(8000), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT( + amm.expectBalances(USD(8000), EUR(2000), IOUAmount(4000))); + amm.deposit(bob, USD(4000), EUR(1000)); + BEAST_EXPECT( + amm.expectBalances(USD(12000), EUR(3000), IOUAmount(6000))); + amm.deposit(carol, USD(2000), EUR(500)); + BEAST_EXPECT( + amm.expectBalances(USD(14000), EUR(3500), IOUAmount(7000))); + + // global freeze + env(fset(gw, asfGlobalFreeze)); + env.close(); + + // gw clawback 1000 USD from carol. + env(amm::ammClawback(gw, carol, USD, EUR, USD(1000), std::nullopt), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT( + amm.expectBalances(USD(13000), EUR(3250), IOUAmount(6500))); + + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(4000))); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(2000))); + BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(500))); + BEAST_EXPECT(env.balance(alice, USD) == USD(2000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(bob, USD) == USD(5000)); + BEAST_EXPECT(env.balance(bob, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(carol, USD) == USD(6000)); + // 250 EUR goes back to carol. + BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); + + // gw clawback 1000 USD from bob with tfClawTwoAssets flag. + // then the corresponding EUR will also be clawed back + // by gw. + env(amm::ammClawback(gw, bob, USD, EUR, USD(1000), tfClawTwoAssets), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT( + amm.expectBalances(USD(12000), EUR(3000), IOUAmount(6000))); + + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(4000))); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(1500))); + BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(500))); + BEAST_EXPECT(env.balance(alice, USD) == USD(2000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(bob, USD) == USD(5000)); + // 250 EUR did not go back to bob because tfClawTwoAssets is set. + BEAST_EXPECT(env.balance(bob, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(carol, USD) == USD(6000)); + BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); + + // gw clawback all USD from alice and set tfClawTwoAssets. + env(amm::ammClawback( + gw, alice, USD, EUR, std::nullopt, tfClawTwoAssets), + ter(tesSUCCESS)); + env.close(); + BEAST_EXPECT( + amm.expectBalances(USD(4000), EUR(1000), IOUAmount(2000))); + + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); + BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(1500))); + BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(500))); + BEAST_EXPECT(env.balance(alice, USD) == USD(2000)); + BEAST_EXPECT(env.balance(alice, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(bob, USD) == USD(5000)); + BEAST_EXPECT(env.balance(bob, EUR) == EUR(8000)); + BEAST_EXPECT(env.balance(carol, USD) == USD(6000)); + BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); + } + } + void run() override { @@ -1442,6 +1728,7 @@ class AMMClawback_test : public jtx::AMMTest testAMMClawbackSameCurrency(all); testAMMClawbackIssuesEachOther(all); testNotHoldingLptoken(all); + testAssetFrozen(all); } }; BEAST_DEFINE_TESTSUITE(AMMClawback, app, ripple); diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index feee7eae603..1745751a259 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -6957,9 +6957,9 @@ struct AMM_test : public jtx::AMMTest env.fund(XRP(1'000), gw); fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}, Fund::Acct); env.close(); - AMM* amm = new AMM(env, alice, XRP(100), USD(100), ter(tesSUCCESS)); + AMM amm(env, alice, XRP(100), USD(100), ter(tesSUCCESS)); env(trust(gw, alice["USD"](0), tfSetFreeze)); - cb(*amm); + cb(amm); }; // Deposit two assets, one of which is frozen, diff --git a/src/xrpld/app/misc/detail/AMMUtils.cpp b/src/xrpld/app/misc/detail/AMMUtils.cpp index d5af0ee02ef..efc80cf17b6 100644 --- a/src/xrpld/app/misc/detail/AMMUtils.cpp +++ b/src/xrpld/app/misc/detail/AMMUtils.cpp @@ -16,14 +16,12 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ //============================================================================== -#include #include #include #include #include #include #include -#include namespace ripple { diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index a003932f61a..660a52d5c2e 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -152,7 +152,7 @@ AMMClawback::applyGuts(Sandbox& sb) *ammSle, asset, asset2, - FreezeHandling::fhZERO_IF_FROZEN, + FreezeHandling::fhIGNORE_FREEZE, ctx_.journal); if (!expected) @@ -184,6 +184,7 @@ AMMClawback::applyGuts(Sandbox& sb) holdLPtokens, holdLPtokens, 0, + FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::Yes, ctx_.journal); } @@ -222,7 +223,6 @@ AMMClawback::applyGuts(Sandbox& sb) // same issuer. auto const flags = ctx_.tx.getFlags(); if (flags & tfClawTwoAssets) - return rippleCredit(sb, holder, issuer, *amount2Withdraw, true, j_); return tesSUCCESS; @@ -260,6 +260,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( holdLPtokens, holdLPtokens, 0, + FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::Yes, ctx_.journal); @@ -276,6 +277,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( lptAMMBalance, toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), 0, + FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::No, ctx_.journal); } diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 843384d3778..36ce7325a58 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -397,23 +396,16 @@ AMMWithdraw::applyGuts(Sandbox& sb) tfee); if (subTxType & tfLPToken || subTxType & tfWithdrawAll) { - TER result; - STAmount newLPTokenBalance; - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = - equalWithdrawTokens( - sb, - *ammSle, - account_, - ammAccountID, - amountBalance, - amount2Balance, - lptAMMBalance, - lpTokens, - *lpTokensWithdraw, - tfee, - isWithdrawAll(ctx_.tx), - ctx_.journal); - return {result, newLPTokenBalance}; + return equalWithdrawTokens( + sb, + *ammSle, + ammAccountID, + amountBalance, + amount2Balance, + lptAMMBalance, + lpTokens, + *lpTokensWithdraw, + tfee); } // should not happen. // LCOV_EXCL_START @@ -457,7 +449,6 @@ std::pair AMMWithdraw::withdraw( Sandbox& view, AccountID const& ammAccount, - AccountID const& account, SLE const& ammSle, STAmount const& amountBalance, STAmount const& amountWithdraw, @@ -471,7 +462,7 @@ AMMWithdraw::withdraw( std::tie(ter, newLPTokenBalance, std::ignore, std::ignore) = withdraw( view, ammAccount, - account, + account_, ammSle, amountBalance, amountWithdraw, @@ -479,6 +470,7 @@ AMMWithdraw::withdraw( lpTokensAMMBalance, lpTokensWithdraw, tfee, + FreezeHandling::fhZERO_IF_FROZEN, isWithdrawAll(ctx_.tx), j_); return {ter, newLPTokenBalance}; @@ -496,6 +488,7 @@ AMMWithdraw::withdraw( STAmount const& lpTokensAMMBalance, STAmount const& lpTokensWithdraw, std::uint16_t tfee, + FreezeHandling freezeHandling, WithdrawAll withdrawAll, beast::Journal const& journal) { @@ -505,7 +498,7 @@ AMMWithdraw::withdraw( ammSle, amountWithdraw.issue(), std::nullopt, - FreezeHandling::fhZERO_IF_FROZEN, + freezeHandling, journal); // LCOV_EXCL_START if (!expected) @@ -655,6 +648,38 @@ AMMWithdraw::withdraw( amount2WithdrawActual); } +std::pair +AMMWithdraw::equalWithdrawTokens( + Sandbox& view, + SLE const& ammSle, + AccountID const& ammAccount, + STAmount const& amountBalance, + STAmount const& amount2Balance, + STAmount const& lptAMMBalance, + STAmount const& lpTokens, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee) +{ + TER ter; + STAmount newLPTokenBalance; + std::tie(ter, newLPTokenBalance, std::ignore, std::ignore) = + equalWithdrawTokens( + view, + ammSle, + account_, + ammAccount, + amountBalance, + amount2Balance, + lptAMMBalance, + lpTokens, + lpTokensWithdraw, + tfee, + FreezeHandling::fhZERO_IF_FROZEN, + isWithdrawAll(ctx_.tx), + ctx_.journal); + return {ter, newLPTokenBalance}; +} + std::pair AMMWithdraw::deleteAMMAccountIfEmpty( Sandbox& sb, @@ -698,6 +723,7 @@ AMMWithdraw::equalWithdrawTokens( STAmount const& lpTokens, STAmount const& lpTokensWithdraw, std::uint16_t tfee, + FreezeHandling freezeHanding, WithdrawAll withdrawAll, beast::Journal const& journal) { @@ -717,6 +743,7 @@ AMMWithdraw::equalWithdrawTokens( lptAMMBalance, lpTokensWithdraw, tfee, + freezeHanding, WithdrawAll::Yes, journal); } @@ -744,6 +771,7 @@ AMMWithdraw::equalWithdrawTokens( lptAMMBalance, lpTokensWithdraw, tfee, + freezeHanding, withdrawAll, journal); } @@ -801,7 +829,6 @@ AMMWithdraw::equalWithdrawLimit( return withdraw( view, ammAccount, - account_, ammSle, amountBalance, amount, @@ -817,7 +844,6 @@ AMMWithdraw::equalWithdrawLimit( return withdraw( view, ammAccount, - account_, ammSle, amountBalance, toSTAmount(amount.issue(), amountWithdraw), @@ -849,7 +875,6 @@ AMMWithdraw::singleWithdraw( return withdraw( view, ammAccount, - account_, ammSle, amountBalance, amount, @@ -887,7 +912,6 @@ AMMWithdraw::singleWithdrawTokens( return withdraw( view, ammAccount, - account_, ammSle, amountBalance, amountWithdraw, @@ -952,7 +976,6 @@ AMMWithdraw::singleWithdrawEPrice( return withdraw( view, ammAccount, - account_, ammSle, amountBalance, amountWithdraw, diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.h b/src/xrpld/app/tx/detail/AMMWithdraw.h index ddbf8f550ae..6694910e1a0 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.h +++ b/src/xrpld/app/tx/detail/AMMWithdraw.h @@ -21,6 +21,7 @@ #define RIPPLE_TX_AMMWITHDRAW_H_INCLUDED #include +#include namespace ripple { @@ -109,6 +110,7 @@ class AMMWithdraw : public Transactor STAmount const& lpTokens, STAmount const& lpTokensWithdraw, std::uint16_t tfee, + FreezeHandling freezeHanding, WithdrawAll withdrawAll, beast::Journal const& journal); @@ -138,6 +140,7 @@ class AMMWithdraw : public Transactor STAmount const& lpTokensAMMBalance, STAmount const& lpTokensWithdraw, std::uint16_t tfee, + FreezeHandling freezeHandling, WithdrawAll withdrawAll, beast::Journal const& journal); @@ -169,7 +172,6 @@ class AMMWithdraw : public Transactor withdraw( Sandbox& view, AccountID const& ammAccount, - AccountID const& account, SLE const& ammSle, STAmount const& amountBalance, STAmount const& amountWithdraw, @@ -178,6 +180,31 @@ class AMMWithdraw : public Transactor STAmount const& lpTokensWithdraw, std::uint16_t tfee); + /** Equal-asset withdrawal (LPTokens) of some AMM instance pools + * shares represented by the number of LPTokens . + * The trading fee is not charged. + * @param view + * @param ammAccount + * @param amountBalance current LP asset1 balance + * @param amount2Balance current LP asset2 balance + * @param lptAMMBalance current AMM LPT balance + * @param lpTokens current LPT balance + * @param lpTokensWithdraw amount of tokens to withdraw + * @param tfee trading fee in basis points + * @return + */ + std::pair + equalWithdrawTokens( + Sandbox& view, + SLE const& ammSle, + AccountID const& ammAccount, + STAmount const& amountBalance, + STAmount const& amount2Balance, + STAmount const& lptAMMBalance, + STAmount const& lpTokens, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee); + /** Withdraw both assets (Asset1Out, Asset2Out) with the constraints * on the maximum amount of each asset that the trader is willing * to withdraw. The trading fee is not charged. From e449a090f11ed3235b261fbe1f17751ecf020963 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Tue, 1 Oct 2024 11:44:47 -0400 Subject: [PATCH 06/13] change withdraw ammSle parameter order --- src/xrpld/app/tx/detail/AMMClawback.cpp | 10 ++++++---- src/xrpld/app/tx/detail/AMMClawback.h | 1 + src/xrpld/app/tx/detail/AMMWithdraw.cpp | 20 ++++++++++---------- src/xrpld/app/tx/detail/AMMWithdraw.h | 10 ++++++---- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 660a52d5c2e..5176d17051a 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -169,7 +169,6 @@ AMMClawback::applyGuts(Sandbox& sb) return tecINTERNAL; if (!clawAmount) - { // Because we are doing a two-asset withdrawal, // tfee is actually not used, so pass tfee as 0. std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = @@ -187,7 +186,6 @@ AMMClawback::applyGuts(Sandbox& sb) FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::Yes, ctx_.journal); - } else std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = equalWithdrawMatchingOneAmount( @@ -198,6 +196,7 @@ AMMClawback::applyGuts(Sandbox& sb) amountBalance, amount2Balance, lptAMMBalance, + holdLPtokens, *clawAmount); if (result != tesSUCCESS) @@ -221,6 +220,9 @@ AMMClawback::applyGuts(Sandbox& sb) // will claw the paired asset as well. We already checked if // tfClawTwoAssets is enabled, the two assets have to be issued by the // same issuer. + if (!amount2Withdraw) + return tecINTERNAL; // LCOV_EXCL_LINE + auto const flags = ctx_.tx.getFlags(); if (flags & tfClawTwoAssets) return rippleCredit(sb, holder, issuer, *amount2Withdraw, true, j_); @@ -237,6 +239,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( STAmount const& amountBalance, STAmount const& amount2Balance, STAmount const& lptAMMBalance, + STAmount const& holdLPtokens, STAmount const& amount) { auto frac = Number{amount} / amountBalance; @@ -244,7 +247,6 @@ AMMClawback::equalWithdrawMatchingOneAmount( auto const lpTokensWithdraw = toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac); - auto const holdLPtokens = ammLPHolds(sb, ammSle, holder, j_); if (lpTokensWithdraw > holdLPtokens) // if lptoken balance less than what the issuer intended to clawback, // clawback all the tokens. Because we are doing a two-asset withdrawal, @@ -268,9 +270,9 @@ AMMClawback::equalWithdrawMatchingOneAmount( // tfee is actually not used, so pass tfee as 0. return AMMWithdraw::withdraw( sb, + ammSle, ammAccount, holder, - ammSle, amountBalance, amount, toSTAmount(amount2Balance.issue(), amount2Withdraw), diff --git a/src/xrpld/app/tx/detail/AMMClawback.h b/src/xrpld/app/tx/detail/AMMClawback.h index 0978019e55c..fdcfc53e2ca 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.h +++ b/src/xrpld/app/tx/detail/AMMClawback.h @@ -66,6 +66,7 @@ class AMMClawback : public Transactor STAmount const& amountBalance, STAmount const& amount2Balance, STAmount const& lptAMMBalance, + STAmount const& holdLPtokens, STAmount const& amount); }; diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 36ce7325a58..0a6f3291b78 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -448,8 +448,8 @@ AMMWithdraw::doApply() std::pair AMMWithdraw::withdraw( Sandbox& view, - AccountID const& ammAccount, SLE const& ammSle, + AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& amountWithdraw, std::optional const& amount2Withdraw, @@ -461,9 +461,9 @@ AMMWithdraw::withdraw( STAmount newLPTokenBalance; std::tie(ter, newLPTokenBalance, std::ignore, std::ignore) = withdraw( view, + ammSle, ammAccount, account_, - ammSle, amountBalance, amountWithdraw, amount2Withdraw, @@ -479,9 +479,9 @@ AMMWithdraw::withdraw( std::tuple> AMMWithdraw::withdraw( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, AccountID const& account, - SLE const& ammSle, STAmount const& amountBalance, STAmount const& amountWithdraw, std::optional const& amount2Withdraw, @@ -734,9 +734,9 @@ AMMWithdraw::equalWithdrawTokens( { return withdraw( view, + ammSle, ammAccount, account, - ammSle, amountBalance, amountBalance, amount2Balance, @@ -762,9 +762,9 @@ AMMWithdraw::equalWithdrawTokens( return withdraw( view, + ammSle, ammAccount, account, - ammSle, amountBalance, withdrawAmount, withdraw2Amount, @@ -828,8 +828,8 @@ AMMWithdraw::equalWithdrawLimit( { return withdraw( view, - ammAccount, ammSle, + ammAccount, amountBalance, amount, toSTAmount(amount2.issue(), amount2Withdraw), @@ -843,8 +843,8 @@ AMMWithdraw::equalWithdrawLimit( assert(amountWithdraw <= amount); return withdraw( view, - ammAccount, ammSle, + ammAccount, amountBalance, toSTAmount(amount.issue(), amountWithdraw), amount2, @@ -874,8 +874,8 @@ AMMWithdraw::singleWithdraw( return withdraw( view, - ammAccount, ammSle, + ammAccount, amountBalance, amount, std::nullopt, @@ -911,8 +911,8 @@ AMMWithdraw::singleWithdrawTokens( { return withdraw( view, - ammAccount, ammSle, + ammAccount, amountBalance, amountWithdraw, std::nullopt, @@ -975,8 +975,8 @@ AMMWithdraw::singleWithdrawEPrice( { return withdraw( view, - ammAccount, ammSle, + ammAccount, amountBalance, amountWithdraw, std::nullopt, diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.h b/src/xrpld/app/tx/detail/AMMWithdraw.h index 6694910e1a0..f5b6b52e5ba 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.h +++ b/src/xrpld/app/tx/detail/AMMWithdraw.h @@ -118,7 +118,8 @@ class AMMWithdraw : public Transactor * Return new total LPToken balance and the withdrawn amounts for both * assets. * @param view - * @param ammAccount + * @param ammSle AMM ledger entry + * @param ammAccount AMM account * @param amountBalance current LP asset1 balance * @param amountWithdraw asset1 withdraw amount * @param amount2Withdraw asset2 withdraw amount @@ -131,9 +132,9 @@ class AMMWithdraw : public Transactor static std::tuple> withdraw( Sandbox& view, + SLE const& ammSle, AccountID const& ammAccount, AccountID const& account, - SLE const& ammSle, STAmount const& amountBalance, STAmount const& amountWithdraw, std::optional const& amount2Withdraw, @@ -160,7 +161,8 @@ class AMMWithdraw : public Transactor /** Withdraw requested assets and token from AMM into LP account. * Return new total LPToken balance. * @param view - * @param ammAccount + * @param ammSle AMM ledger entry + * @param ammAccount AMM account * @param amountBalance current LP asset1 balance * @param amountWithdraw asset1 withdraw amount * @param amount2Withdraw asset2 withdraw amount @@ -171,8 +173,8 @@ class AMMWithdraw : public Transactor std::pair withdraw( Sandbox& view, - AccountID const& ammAccount, SLE const& ammSle, + AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& amountWithdraw, std::optional const& amount2Withdraw, From 9f714bb5fdf18396635ea353dc6e80613f4f1e55 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Wed, 2 Oct 2024 17:09:45 -0400 Subject: [PATCH 07/13] resolve error type and other comments --- include/xrpl/protocol/TER.h | 2 - src/libxrpl/protocol/TER.cpp | 2 - src/test/app/AMMClawback_test.cpp | 139 ++++++++++-------------- src/test/jtx/AMM.h | 3 +- src/test/jtx/impl/AMM.cpp | 5 +- src/xrpld/app/tx/detail/AMMClawback.cpp | 6 +- 6 files changed, 65 insertions(+), 92 deletions(-) diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index afc12ab9a0d..aae3c7107bd 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -139,8 +139,6 @@ enum TEMcodes : TERUnderlyingType { temARRAY_EMPTY, temARRAY_TOO_LARGE, - temBAD_ASSET_AMOUNT, - temBAD_ASSET_ISSUER }; //------------------------------------------------------------------------------ diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index 5faf2479d83..917bbf26a9f 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/src/libxrpl/protocol/TER.cpp @@ -205,8 +205,6 @@ transResults() MAKE_ERROR(temXCHAIN_BRIDGE_BAD_REWARD_AMOUNT, "Malformed: Bad reward amount."), MAKE_ERROR(temARRAY_EMPTY, "Malformed: Array is empty."), MAKE_ERROR(temARRAY_TOO_LARGE, "Malformed: Array is too large."), - MAKE_ERROR(temBAD_ASSET_AMOUNT, "Malformed: Amount does not match Asset."), - MAKE_ERROR(temBAD_ASSET_ISSUER, "Malformed: Issuer does not match Asset."), MAKE_ERROR(terRETRY, "Retry transaction."), MAKE_ERROR(terFUNDS_SPENT, "DEPRECATED."), diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index 0d4d0ebd8b4..6c723cc0a7e 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -63,8 +63,7 @@ class AMMClawback_test : public jtx::AMMTest // The AMM account does not exist at all now. // It should return terNO_AMM error. - env(amm::ammClawback( - gw, alice, USD, EUR, std::nullopt, std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, std::nullopt), ter(terNO_AMM)); } @@ -91,12 +90,11 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); // Issuer can not clawback from himself. - env(amm::ammClawback(gw, gw, USD, XRP, std::nullopt, std::nullopt), + env(amm::ammClawback(gw, gw, USD, XRP, std::nullopt), ter(temMALFORMED)); // Holder can not clawback from himself. - env(amm::ammClawback( - alice, alice, USD, XRP, std::nullopt, std::nullopt), + env(amm::ammClawback(alice, alice, USD, XRP, std::nullopt), ter(temMALFORMED)); } @@ -122,15 +120,14 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); // The Asset's issuer field is alice, while the Account field is gw. - // This should return temBAD_ASSET_ISSUER because they do not match. + // This should return temMALFORMED because they do not match. env(amm::ammClawback( gw, alice, Issue{gw["USD"].currency, alice.id()}, XRP, - std::nullopt, std::nullopt), - ter(temBAD_ASSET_ISSUER)); + ter(temMALFORMED)); } // Test if the Amount field matches the Asset field. @@ -155,16 +152,15 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); // The Asset's issuer subfield is gw account and Amount's issuer - // subfield is alice account. Return temBAD_ASSET_AMOUNT because + // subfield is alice account. Return temBAD_AMOUNT because // they do not match. env(amm::ammClawback( gw, alice, USD, XRP, - STAmount{Issue{gw["USD"].currency, alice.id()}, 1}, - std::nullopt), - ter(temBAD_ASSET_AMOUNT)); + STAmount{Issue{gw["USD"].currency, alice.id()}, 1}), + ter(temBAD_AMOUNT)); } // Test if the Amount is invalid, which is less than zero. @@ -194,8 +190,7 @@ class AMMClawback_test : public jtx::AMMTest alice, USD, XRP, - STAmount{Issue{gw["USD"].currency, gw.id()}, -1}, - std::nullopt), + STAmount{Issue{gw["USD"].currency, gw.id()}, -1}), ter(temBAD_AMOUNT)); // Return temBAD_AMOUNT if the Amount value is 0. @@ -204,8 +199,7 @@ class AMMClawback_test : public jtx::AMMTest alice, USD, XRP, - STAmount{Issue{gw["USD"].currency, gw.id()}, 0}, - std::nullopt), + STAmount{Issue{gw["USD"].currency, gw.id()}, 0}), ter(temBAD_AMOUNT)); } @@ -231,8 +225,7 @@ class AMMClawback_test : public jtx::AMMTest // If asfAllowTrustLineClawback is not set, the issuer is not // allowed to send the AMMClawback transaction. - env(amm::ammClawback( - gw, alice, USD, XRP, std::nullopt, std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, std::nullopt), ter(tecNO_PERMISSION)); } @@ -258,8 +251,8 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); // Return temINVALID_FLAG when providing invalid flag. - env(amm::ammClawback( - gw, alice, USD, XRP, std::nullopt, tfTwoAssetIfEmpty), + env(amm::ammClawback(gw, alice, USD, XRP, std::nullopt), + txflags(tfTwoAssetIfEmpty), ter(temINVALID_FLAG)); } @@ -290,8 +283,8 @@ class AMMClawback_test : public jtx::AMMTest // but the issuer only issues USD in the pool. The issuer is not // allowed to set tfClawTwoAssets flag if he did not issue both // assts in the pool. - env(amm::ammClawback( - gw, alice, USD, XRP, std::nullopt, tfClawTwoAssets), + env(amm::ammClawback(gw, alice, USD, XRP, std::nullopt), + txflags(tfClawTwoAssets), ter(tecNO_PERMISSION)); } @@ -319,8 +312,7 @@ class AMMClawback_test : public jtx::AMMTest env.close(); // Clawback XRP is prohibited. - env(amm::ammClawback( - gw, alice, XRP, USD, std::nullopt, std::nullopt), + env(amm::ammClawback(gw, alice, XRP, USD, std::nullopt), ter(temMALFORMED)); } } @@ -352,8 +344,7 @@ class AMMClawback_test : public jtx::AMMTest // When featureAMMClawback is not enabled, AMMClawback is disabled. // Because when featureAMMClawback is disabled, we can not create // amm account, call amm::ammClawback directly for testing purpose. - env(amm::ammClawback( - gw, alice, USD, XRP, std::nullopt, std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, std::nullopt), ter(temDISABLED)); } } @@ -403,7 +394,7 @@ class AMMClawback_test : public jtx::AMMTest USD(2000), EUR(1000), IOUAmount{1414213562373095, -12})); // gw clawback 1000 USD from the AMM pool. - env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -430,7 +421,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback another 1000 USD from the AMM pool. The AMM pool will // be empty and get deleted. - env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -480,7 +471,7 @@ class AMMClawback_test : public jtx::AMMTest auto aliceXrpBalance = env.balance(alice, XRP); // gw clawback 1000 USD from the AMM pool. - env(amm::ammClawback(gw, alice, USD, XRP, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -505,7 +496,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback another 1000 USD from the AMM pool. The AMM pool will // be empty and get deleted. - env(amm::ammClawback(gw, alice, USD, XRP, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -570,7 +561,7 @@ class AMMClawback_test : public jtx::AMMTest USD(4000), EUR(5000), IOUAmount{4472135954999580, -12})); // gw clawback 1000 USD from the AMM pool - env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -594,7 +585,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectLPTokens(alice, IOUAmount{3354101966249685, -12})); // gw clawback another 500 USD from the AMM pool. - env(amm::ammClawback(gw, alice, USD, EUR, USD(500), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(500)), ter(tesSUCCESS)); env.close(); @@ -612,8 +603,7 @@ class AMMClawback_test : public jtx::AMMTest STAmount(EUR, UINT64_C(2874999999999999), -12)); // gw clawback small amount, 1 USD. - env(amm::ammClawback(gw, alice, USD, EUR, USD(1), std::nullopt), - ter(tesSUCCESS)); + env(amm::ammClawback(gw, alice, USD, EUR, USD(1)), ter(tesSUCCESS)); env.close(); // Another 1 USD / 1.25 EUR was withdrawn. @@ -630,7 +620,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 4000 USD, exceeding the current balance. We // will clawback all. - env(amm::ammClawback(gw, alice, USD, EUR, USD(4000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(4000)), ter(tesSUCCESS)); env.close(); @@ -713,7 +703,7 @@ class AMMClawback_test : public jtx::AMMTest auto bobXrpBalance = env.balance(bob, XRP); // gw clawback 500 USD from alice in amm - env(amm::ammClawback(gw, alice, USD, XRP, USD(500), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(500)), ter(tesSUCCESS)); env.close(); @@ -737,8 +727,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectLPTokens(bob, IOUAmount{1414213562373095, -9})); // gw clawback 10 USD from bob in amm. - env(amm::ammClawback(gw, bob, USD, XRP, USD(10), std::nullopt), - ter(tesSUCCESS)); + env(amm::ammClawback(gw, bob, USD, XRP, USD(10)), ter(tesSUCCESS)); env.close(); env.require(balance(alice, gw["USD"](5000))); @@ -757,7 +746,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectLPTokens(bob, IOUAmount{1400071426749365, -9})); // gw2 clawback 200 EUR from amm2. - env(amm::ammClawback(gw2, alice, EUR, XRP, EUR(200), std::nullopt), + env(amm::ammClawback(gw2, alice, EUR, XRP, EUR(200)), ter(tesSUCCESS)); env.close(); @@ -777,7 +766,7 @@ class AMMClawback_test : public jtx::AMMTest // gw claw back 1000 USD from alice in amm, which exceeds alice's // balance. This will clawback all the remaining LP tokens of alice // (corresponding 500 USD / 1000 XRP). - env(amm::ammClawback(gw, alice, USD, XRP, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -800,7 +789,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 1000 USD from bob in amm, which also exceeds bob's // balance in amm. All bob's lptoken in amm will be consumed, which // corresponds to 990 USD / 1980 XRP - env(amm::ammClawback(gw, bob, USD, XRP, USD(1000), std::nullopt), + env(amm::ammClawback(gw, bob, USD, XRP, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -821,7 +810,7 @@ class AMMClawback_test : public jtx::AMMTest // gw2 claw back 1000 EUR from alice in amm2, which exceeds alice's // balance. All alice's lptokens will be consumed, which corresponds // to 800EUR / 2400 XRP. - env(amm::ammClawback(gw2, alice, EUR, XRP, EUR(1000), std::nullopt), + env(amm::ammClawback(gw2, alice, EUR, XRP, EUR(1000)), ter(tesSUCCESS)); env.close(); @@ -847,7 +836,7 @@ class AMMClawback_test : public jtx::AMMTest // gw2 claw back 2000 EUR from bib in amm2, which exceeds bob's // balance. All bob's lptokens will be consumed, which corresponds // to 1000EUR / 3000 XRP. - env(amm::ammClawback(gw2, bob, EUR, XRP, EUR(2000), std::nullopt), + env(amm::ammClawback(gw2, bob, EUR, XRP, EUR(2000)), ter(tesSUCCESS)); env.close(); @@ -951,7 +940,7 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(carol, gw2["EUR"](2750))); // gw clawback all the bob's USD in amm. (2000 USD / 2500 EUR) - env(amm::ammClawback(gw, bob, USD, EUR, std::nullopt, std::nullopt), + env(amm::ammClawback(gw, bob, USD, EUR, std::nullopt), ter(tesSUCCESS)); env.close(); @@ -980,8 +969,7 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(carol, gw2["EUR"](2750))); // gw2 clawback all carol's EUR in amm. (1000 USD / 1250 EUR) - env(amm::ammClawback( - gw2, carol, EUR, USD, std::nullopt, std::nullopt), + env(amm::ammClawback(gw2, carol, EUR, USD, std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -995,8 +983,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(0))); // gw2 clawback all alice's EUR in amm. (4000 USD / 5000 EUR) - env(amm::ammClawback( - gw2, alice, EUR, USD, std::nullopt, std::nullopt), + env(amm::ammClawback(gw2, alice, EUR, USD, std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1044,8 +1031,7 @@ class AMMClawback_test : public jtx::AMMTest auto bobXrpBalance = env.balance(bob, XRP); // gw clawback all alice's USD in amm. (1000 USD / 200 XRP) - env(amm::ammClawback( - gw, alice, USD, XRP, std::nullopt, std::nullopt), + env(amm::ammClawback(gw, alice, USD, XRP, std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1055,7 +1041,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); // gw clawback all bob's USD in amm. (2000 USD / 400 XRP) - env(amm::ammClawback(gw, bob, USD, XRP, std::nullopt, std::nullopt), + env(amm::ammClawback(gw, bob, USD, XRP, std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1120,8 +1106,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectBalances(USD(14000), EUR(3500), IOUAmount(7000))); // gw clawback 1000 USD from carol. - env(amm::ammClawback(gw, carol, USD, EUR, USD(1000), std::nullopt), - ter(tesSUCCESS)); + env(amm::ammClawback(gw, carol, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( amm.expectBalances(USD(13000), EUR(3250), IOUAmount(6500))); @@ -1140,7 +1125,8 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 1000 USD from bob with tfClawTwoAssets flag. // then the corresponding EUR will also be clawed back // by gw. - env(amm::ammClawback(gw, bob, USD, EUR, USD(1000), tfClawTwoAssets), + env(amm::ammClawback(gw, bob, USD, EUR, USD(1000)), + txflags(tfClawTwoAssets), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( @@ -1158,8 +1144,8 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); // gw clawback all USD from alice and set tfClawTwoAssets. - env(amm::ammClawback( - gw, alice, USD, EUR, std::nullopt, tfClawTwoAssets), + env(amm::ammClawback(gw, alice, USD, EUR, std::nullopt), + txflags(tfClawTwoAssets), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances(USD(4000), EUR(1000), IOUAmount(2000))); @@ -1233,9 +1219,8 @@ class AMMClawback_test : public jtx::AMMTest alice, gw2["USD"], gw["USD"], - STAmount{Issue{gw2["USD"].currency, gw2.id()}, 500}, - std::nullopt), - ter(temBAD_ASSET_ISSUER)); + STAmount{Issue{gw2["USD"].currency, gw2.id()}, 500}), + ter(temMALFORMED)); // gw2 clawback 500 gw2[USD] from alice. env(amm::ammClawback( @@ -1243,8 +1228,7 @@ class AMMClawback_test : public jtx::AMMTest alice, gw2["USD"], gw["USD"], - STAmount{Issue{gw2["USD"].currency, gw2.id()}, 500}, - std::nullopt), + STAmount{Issue{gw2["USD"].currency, gw2.id()}, 500}), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1263,8 +1247,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(bob, gw2["USD"]) == gw2["USD"](2000)); // gw clawback all gw["USD"] from bob. - env(amm::ammClawback( - gw, bob, gw["USD"], gw2["USD"], std::nullopt, std::nullopt), + env(amm::ammClawback(gw, bob, gw["USD"], gw2["USD"], std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( @@ -1341,8 +1324,7 @@ class AMMClawback_test : public jtx::AMMTest amm.expectLPTokens(alice, IOUAmount{4242640687119285, -12})); // gw claws back 1000 USD from gw2. - env(amm::ammClawback(gw, gw2, USD, EUR, USD(1000), std::nullopt), - ter(tesSUCCESS)); + env(amm::ammClawback(gw, gw2, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( USD(5000), EUR(10000), IOUAmount{7071067811865475, -12})); @@ -1358,8 +1340,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); // gw2 claws back 1000 EUR from gw. - env(amm::ammClawback(gw2, gw, EUR, USD, EUR(1000), std::nullopt), - ter(tesSUCCESS)); + env(amm::ammClawback(gw2, gw, EUR, USD, EUR(1000)), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( USD(4500), @@ -1377,8 +1358,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); // gw2 claws back 4000 EUR from alice. - env(amm::ammClawback(gw2, alice, EUR, USD, EUR(4000), std::nullopt), - ter(tesSUCCESS)); + env(amm::ammClawback(gw2, alice, EUR, USD, EUR(4000)), ter(tesSUCCESS)); env.close(); BEAST_EXPECT(amm.expectBalances( USD(2500), @@ -1424,8 +1404,8 @@ class AMMClawback_test : public jtx::AMMTest // Alice did not deposit in the amm pool. So AMMClawback from Alice // will fail. - env(amm::ammClawback(gw, alice, USD, XRP, USD(1000), std::nullopt), - ter(tecINTERNAL)); + env(amm::ammClawback(gw, alice, USD, XRP, USD(1000)), + ter(tecAMM_BALANCE)); } void @@ -1474,7 +1454,7 @@ class AMMClawback_test : public jtx::AMMTest env.close(); // gw clawback 1000 USD from the AMM pool. - env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -1489,7 +1469,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback another 1000 USD from the AMM pool. The AMM pool will // be empty and get deleted. - env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -1543,7 +1523,7 @@ class AMMClawback_test : public jtx::AMMTest env.close(); // gw clawback 1000 USD from the AMM pool. - env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -1595,7 +1575,7 @@ class AMMClawback_test : public jtx::AMMTest env.close(); // gw clawback 1000 USD from the AMM pool. - env(amm::ammClawback(gw, alice, USD, EUR, USD(1000), std::nullopt), + env(amm::ammClawback(gw, alice, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); @@ -1658,7 +1638,7 @@ class AMMClawback_test : public jtx::AMMTest env.close(); // gw clawback 1000 USD from carol. - env(amm::ammClawback(gw, carol, USD, EUR, USD(1000), std::nullopt), + env(amm::ammClawback(gw, carol, USD, EUR, USD(1000)), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( @@ -1678,7 +1658,8 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 1000 USD from bob with tfClawTwoAssets flag. // then the corresponding EUR will also be clawed back // by gw. - env(amm::ammClawback(gw, bob, USD, EUR, USD(1000), tfClawTwoAssets), + env(amm::ammClawback(gw, bob, USD, EUR, USD(1000)), + txflags(tfClawTwoAssets), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( @@ -1696,8 +1677,8 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); // gw clawback all USD from alice and set tfClawTwoAssets. - env(amm::ammClawback( - gw, alice, USD, EUR, std::nullopt, tfClawTwoAssets), + env(amm::ammClawback(gw, alice, USD, EUR, std::nullopt), + txflags(tfClawTwoAssets), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( diff --git a/src/test/jtx/AMM.h b/src/test/jtx/AMM.h index e3c33a402f5..52039f74aea 100644 --- a/src/test/jtx/AMM.h +++ b/src/test/jtx/AMM.h @@ -445,8 +445,7 @@ ammClawback( Account const& holder, Issue const& asset, Issue const& asset2, - std::optional const& amount, - std::optional flags); + std::optional const& amount); } // namespace amm } // namespace jtx diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index 8431c1e7f21..8c59fbcf7cb 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -830,8 +830,7 @@ ammClawback( Account const& holder, Issue const& asset, Issue const& asset2, - std::optional const& amount, - std::optional flags) + std::optional const& amount) { Json::Value jv; jv[jss::TransactionType] = jss::AMMClawback; @@ -842,8 +841,6 @@ ammClawback( if (amount) jv[jss::Amount] = amount->getJson(JsonOptions::none); - if (flags) - jv[jss::Flags] = *flags; return jv; } } // namespace amm diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 5176d17051a..d7ae11af7c8 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -65,14 +65,14 @@ AMMClawback::preflight(PreflightContext const& ctx) { JLOG(ctx.j.trace()) << "AMMClawback: Asset's account does not " "match Account field."; - return temBAD_ASSET_ISSUER; + return temMALFORMED; } if (clawAmount && clawAmount->issue() != asset) { JLOG(ctx.j.trace()) << "AMMClawback: Amount's issuer/currency subfield " "does not match Asset field"; - return temBAD_ASSET_AMOUNT; + return temBAD_AMOUNT; } if (clawAmount && *clawAmount <= beast::zero) @@ -166,7 +166,7 @@ AMMClawback::applyGuts(Sandbox& sb) auto const holdLPtokens = ammLPHolds(sb, *ammSle, holder, j_); if (holdLPtokens == beast::zero) - return tecINTERNAL; + return tecAMM_BALANCE; if (!clawAmount) // Because we are doing a two-asset withdrawal, From 860d033a770de907bc3984038dbc5f2c5ff7340e Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Wed, 2 Oct 2024 22:26:30 -0400 Subject: [PATCH 08/13] Change temBAD_AMOUNT error message --- src/libxrpl/protocol/TER.cpp | 2 +- src/test/rpc/Status_test.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index 917bbf26a9f..3a35ccdaef8 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/src/libxrpl/protocol/TER.cpp @@ -159,7 +159,7 @@ transResults() MAKE_ERROR(temMALFORMED, "Malformed transaction."), MAKE_ERROR(temBAD_AMM_TOKENS, "Malformed: Invalid LPTokens."), - MAKE_ERROR(temBAD_AMOUNT, "Can only send positive amounts."), + MAKE_ERROR(temBAD_AMOUNT, "Malformed: Bad amount."), MAKE_ERROR(temBAD_CURRENCY, "Malformed: Bad currency."), MAKE_ERROR(temBAD_EXPIRATION, "Malformed: Bad expiration."), MAKE_ERROR(temBAD_FEE, "Invalid fee, negative or not XRP."), diff --git a/src/test/rpc/Status_test.cpp b/src/test/rpc/Status_test.cpp index 1ae8b23c66c..c68131e8131 100644 --- a/src/test/rpc/Status_test.cpp +++ b/src/test/rpc/Status_test.cpp @@ -76,7 +76,7 @@ class codeString_test : public beast::unit_test::suite { auto s = codeString(temBAD_AMOUNT); - expect(s == "temBAD_AMOUNT: Can only send positive amounts.", s); + expect(s == "temBAD_AMOUNT: Malformed: Bad amount.", s); } { @@ -176,7 +176,7 @@ class fillJson_test : public beast::unit_test::suite "temBAD_AMOUNT", temBAD_AMOUNT, {}, - "temBAD_AMOUNT: Can only send positive amounts."); + "temBAD_AMOUNT: Malformed: Bad amount."); expectFill( "rpcBAD_SYNTAX", From a15db1492f244d6d16710965189516caa834dd50 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Sun, 13 Oct 2024 22:47:53 -0400 Subject: [PATCH 09/13] Block unauthorized account to deposit into AMM pool --- src/test/app/AMM_test.cpp | 38 ++++++++++++++++++++++++++ src/xrpld/app/tx/detail/AMMDeposit.cpp | 20 ++++++++++---- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 1745751a259..8e764390e9a 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -1005,6 +1005,44 @@ struct AMM_test : public jtx::AMMTest }, {{USD(20'000), BTC(0.5)}}); + // Deposit unauthorized token. + { + Env env(*this, features); + env.fund(XRP(1000), gw, alice, bob); + env(fset(gw, asfRequireAuth)); + env.close(); + env(trust(gw, alice["USD"](100)), txflags(tfSetfAuth)); + env(trust(alice, gw["USD"](20))); + env.close(); + env(pay(gw, alice, gw["USD"](10))); + env.close(); + env(trust(gw, bob["USD"](100))); + env.close(); + + AMM amm(env, alice, XRP(10), gw["USD"](10), ter(tesSUCCESS)); + env.close(); + + if (features[featureAMMClawback]) + // if featureAMMClawback is enabled, bob can not deposit XRP + // because he's not authorized to hold the paired token + // gw["USD"]. + amm.deposit( + bob, + XRP(10), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecNO_AUTH)); + else + amm.deposit( + bob, + XRP(10), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tesSUCCESS)); + } + // Insufficient XRP balance testAMM([&](AMM& ammAlice, Env& env) { env.fund(XRP(1'000), bob); diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index c73b833f9c6..3448401eb79 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -248,21 +248,31 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) { // Check if either of the assets is frozen, AMMDeposit is not allowed // if either asset is frozen - auto checkAsset = [&](Issue const& asset) -> bool { + auto checkAsset = [&](Issue const& asset) -> TER { + if (auto const ter = requireAuth(ctx.view, asset, accountID)) + { + JLOG(ctx.j.debug()) + << "AMM Deposit: account is not authorized, " << asset; + return ter; + } + if (isFrozen(ctx.view, accountID, asset)) { JLOG(ctx.j.debug()) << "AMM Deposit: account or currency is frozen, " << to_string(accountID) << " " << to_string(asset.currency); - return false; + return tecFROZEN; } - return true; + return tesSUCCESS; }; - if (!checkAsset(ctx.tx[sfAsset]) || !checkAsset(ctx.tx[sfAsset2])) - return tecFROZEN; + if (auto const ter = checkAsset(ctx.tx[sfAsset])) + return ter; + + if (auto const ter = checkAsset(ctx.tx[sfAsset2])) + return ter; } auto const amount = ctx.tx[~sfAmount]; From fc577a551c0430951bdfd3110d59069c954755e7 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Sun, 27 Oct 2024 20:53:47 -0400 Subject: [PATCH 10/13] check if holder exists --- src/test/app/AMMClawback_test.cpp | 77 +++++++++++++++++++++++++ src/xrpld/app/tx/detail/AMMClawback.cpp | 3 + 2 files changed, 80 insertions(+) diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index 6c723cc0a7e..705a1274073 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -34,6 +34,30 @@ class AMMClawback_test : public jtx::AMMTest testcase("test invalid request"); using namespace jtx; + // Test if holder does not exist. + { + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(100000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + env.trust(USD(10000), alice); + env(pay(gw, alice, gw["USD"](100))); + + AMM amm(env, alice, XRP(100), USD(100)); + env.close(); + + env(amm::ammClawback( + gw, Account("unknown"), USD, XRP, std::nullopt), + ter(terNO_ACCOUNT)); + } + // Test if asset pair provided does not exist. This should // return terNO_AMM error. { @@ -1696,6 +1720,58 @@ class AMMClawback_test : public jtx::AMMTest } } + void + testSingleDepositAndClawback(FeatureBitset features) + { + testcase("test single depoit and clawback"); + using namespace jtx; + + // Test AMMClawback for USD/XRP pool. Claw back USD, and XRP goes back + // to the holder. + Env env(*this, features); + Account gw{"gateway"}; + Account alice{"alice"}; + env.fund(XRP(1000000000), gw, alice); + env.close(); + + // gw sets asfAllowTrustLineClawback. + env(fset(gw, asfAllowTrustLineClawback)); + env.close(); + env.require(flags(gw, asfAllowTrustLineClawback)); + + // gw issues 1000 USD to Alice. + auto const USD = gw["USD"]; + env.trust(USD(100000), alice); + env(pay(gw, alice, USD(1000))); + env.close(); + env.require(balance(alice, gw["USD"](1000))); + + // gw creates AMM pool of XRP/USD. + AMM amm(env, gw, XRP(100), USD(400), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances(USD(400), XRP(100), IOUAmount(200000))); + + amm.deposit(alice, USD(400)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + USD(800), XRP(100), IOUAmount{2828427124746190, -10})); + + auto aliceXrpBalance = env.balance(alice, XRP); + + env(amm::ammClawback(gw, alice, USD, XRP, USD(400)), ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + STAmount(USD, UINT64_C(5656854249492380), -13), + XRP(70.710678), + IOUAmount(200000))); + BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); + BEAST_EXPECT(expectLedgerEntryRoot( + env, alice, aliceXrpBalance + XRP(29.289322))); + } + void run() override { @@ -1710,6 +1786,7 @@ class AMMClawback_test : public jtx::AMMTest testAMMClawbackIssuesEachOther(all); testNotHoldingLptoken(all); testAssetFrozen(all); + testSingleDepositAndClawback(all); } }; BEAST_DEFINE_TESTSUITE(AMMClawback, app, ripple); diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index d7ae11af7c8..468a5a4c6a2 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -90,6 +90,9 @@ AMMClawback::preclaim(PreclaimContext const& ctx) if (!sleIssuer) return terNO_ACCOUNT; // LCOV_EXCL_LINE + if (!ctx.view.read(keylet::account(ctx.tx[sfHolder]))) + return terNO_ACCOUNT; + auto const ammSle = ctx.view.read(keylet::amm(asset, asset2)); if (!ammSle) { From 2d73bdac3b7521ea91916b7ca2bb3494a63527fe Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 1 Nov 2024 14:33:21 -0400 Subject: [PATCH 11/13] Change the numFeatures --- include/xrpl/protocol/Feature.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index eb975f39ae0..a2510c63000 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -80,7 +80,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 80; +static constexpr std::size_t numFeatures = 81; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated From 511047f6616dae2b6bbd60f76fd37ff097ee372d Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 1 Nov 2024 15:51:26 -0400 Subject: [PATCH 12/13] Revert "Change the numFeatures" This reverts commit 2d73bdac3b7521ea91916b7ca2bb3494a63527fe. --- include/xrpl/protocol/Feature.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index a2510c63000..eb975f39ae0 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -80,7 +80,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 81; +static constexpr std::size_t numFeatures = 80; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated From 25f9ba6c7577dee5a2068a9a4a4179dbc0d1eaf0 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 1 Nov 2024 16:13:24 -0400 Subject: [PATCH 13/13] resolve code conflict --- include/xrpl/protocol/Feature.h | 2 +- src/test/app/MPToken_test.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index eb975f39ae0..a2510c63000 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -80,7 +80,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 80; +static constexpr std::size_t numFeatures = 81; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index fa888faea17..9fdad6a0743 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -1443,6 +1443,17 @@ class MPToken_test : public beast::unit_test::suite }; ammBid(sfBidMin); ammBid(sfBidMax); + // AMMClawback + { + Json::Value jv; + jv[jss::TransactionType] = jss::AMMClawback; + jv[jss::Account] = alice.human(); + jv[jss::Holder] = carol.human(); + jv[jss::Asset] = to_json(xrpIssue()); + jv[jss::Asset2] = to_json(USD.issue()); + jv[jss::Amount] = mpt.getJson(JsonOptions::none); + test(jv, jss::Amount.c_str()); + } // CheckCash auto checkCash = [&](SField const& field) { Json::Value jv;