From 4879ffcd74c85f6d60b628ee6a538c957a71efc4 Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Tue, 24 Sep 2024 15:33:35 -0400 Subject: [PATCH] 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 | 232 +++++++++--------------- 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, 134 insertions(+), 233 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..c952403f289 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,46 +63,16 @@ class AMMClawback_test : public jtx::AMMTest // The AMM account does not exist at all now. // It should return terNO_AMM error. - env(ammClawback( + env(amm::ammClawback( gw, alice, USD, + EUR, 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. { @@ -126,8 +96,8 @@ 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 +124,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 +158,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 +190,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,46 +232,12 @@ class AMMClawback_test : public jtx::AMMTest // If asfAllowTrustLineClawback is not set, the issuer is not // allowed to send the AMMClawback transaction. - env(ammClawback( + env(amm::ammClawback( gw, alice, USD, + XRP, 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)); } @@ -328,12 +264,12 @@ 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( + env(amm::ammClawback( gw, alice, USD, + XRP, std::nullopt, - amm.ammAccount(), tfTwoAssetIfEmpty), ter(temINVALID_FLAG)); } @@ -365,12 +301,12 @@ 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( + env(amm::ammClawback( gw, alice, USD, + XRP, std::nullopt, - amm.ammAccount(), tfClawTwoAssets), ter(tecNO_PERMISSION)); } @@ -399,12 +335,12 @@ class AMMClawback_test : public jtx::AMMTest env.close(); // Clawback XRP is prohibited. - env(ammClawback( + env(amm::ammClawback( gw, alice, XRP, + USD, std::nullopt, - amm.ammAccount(), std::nullopt), ter(temMALFORMED)); } @@ -436,9 +372,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 +424,8 @@ 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 +452,8 @@ 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 +503,8 @@ 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 +529,8 @@ 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 +595,8 @@ 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 +620,8 @@ 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 +639,8 @@ 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 +658,8 @@ 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 +742,8 @@ 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 +767,8 @@ 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 +788,8 @@ 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 +809,8 @@ 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 +833,8 @@ 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,12 +855,12 @@ 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( + env(amm::ammClawback( gw2, alice, EUR, + XRP, EUR(1000), - amm2.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -951,8 +887,8 @@ 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 +992,8 @@ 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,12 +1022,12 @@ 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( + env(amm::ammClawback( gw2, carol, EUR, + USD, std::nullopt, - amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1106,12 +1042,12 @@ 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( + env(amm::ammClawback( gw2, alice, EUR, + USD, std::nullopt, - amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1160,12 +1096,12 @@ 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( + env(amm::ammClawback( gw, alice, USD, + XRP, std::nullopt, - amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1176,8 +1112,8 @@ 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 +1178,8 @@ 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 +1199,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(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,12 +1218,12 @@ 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( + env(amm::ammClawback( gw, alice, USD, + EUR, std::nullopt, - amm.ammAccount(), tfClawTwoAssets), ter(tesSUCCESS)); env.close(); @@ -1357,22 +1293,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,12 +1328,12 @@ 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( + env(amm::ammClawback( gw, bob, gw["USD"], + gw2["USD"], std::nullopt, - amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -1475,8 +1411,8 @@ 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 +1429,8 @@ 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 +1449,8 @@ 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 +1497,8 @@ 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