From 6510b72d513c542bb67277140267ba61480472ab Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Sun, 22 Sep 2024 21:11:11 -0400 Subject: [PATCH] resolve coverage --- include/xrpl/protocol/TxFlags.h | 2 +- src/test/app/AMMClawback_test.cpp | 366 ++++++++++++------------ src/test/app/AMM_test.cpp | 1 + src/xrpld/app/misc/detail/AMMUtils.cpp | 12 +- src/xrpld/app/tx/detail/AMMClawback.cpp | 54 ++-- src/xrpld/app/tx/detail/AMMClawback.h | 2 +- src/xrpld/app/tx/detail/AMMDeposit.cpp | 2 +- src/xrpld/app/tx/detail/AMMWithdraw.cpp | 2 + 8 files changed, 222 insertions(+), 219 deletions(-) diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index 9b0aea298d8..4cf04f4f42a 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/include/xrpl/protocol/TxFlags.h @@ -182,7 +182,7 @@ constexpr std::uint32_t tfWithdrawMask = ~(tfUniversal | tfWithdrawSubTx); constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfDepositSubTx); // AMMClawback flags: -constexpr std::uint32_t tfClawTwoAssets = 0x00010000; +constexpr std::uint32_t tfClawTwoAssets = 0x00000001; constexpr std::uint32_t tfAMMClawbackMask = ~(tfUniversal | tfClawTwoAssets); // BridgeModify flags: diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index 5b2c01e2941..67ca3f6a577 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -34,8 +34,47 @@ class AMMClawback_test : public jtx::AMMTest testcase("test invalid request"); using namespace jtx; - // Test if the AMMAccount provided is not an AMM account. This should + // 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"}; @@ -43,12 +82,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(10000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 100 USD to Alice + // gw issues 100 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(1000), alice); env(pay(gw, alice, USD(100))); @@ -56,9 +95,9 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); - // gateway send invalid request - // by passing alice account as AMMAccount, this should return - // terNO_AMM + // 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)); @@ -73,12 +112,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(10000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 100 USD to Alice + // gw issues 100 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(1000), alice); env(pay(gw, alice, USD(100))); @@ -86,7 +125,7 @@ class AMMClawback_test : public jtx::AMMTest AMM amm(env, gw, XRP(100), USD(100), ter(tesSUCCESS)); - // issuer can not clawback from himself + // Issuer can not clawback from himself. env(ammClawback( gw, gw, USD, std::nullopt, amm.ammAccount(), std::nullopt), ter(temMALFORMED)); @@ -100,12 +139,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(10000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 100 USD to Alice + // gw issues 100 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(1000), alice); env(pay(gw, alice, USD(100))); @@ -133,12 +172,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(10000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 100 USD to Alice + // gw issues 100 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(1000), alice); env(pay(gw, alice, USD(100))); @@ -167,12 +206,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(10000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 100 USD to Alice + // gw issues 100 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(1000), alice); env(pay(gw, alice, USD(100))); @@ -189,6 +228,16 @@ class AMMClawback_test : public jtx::AMMTest 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 @@ -200,7 +249,7 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(10000), gw, alice); env.close(); - // gateway issues 100 USD to Alice + // gw issues 100 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(1000), alice); env(pay(gw, alice, USD(100))); @@ -208,7 +257,7 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(alice, gw["USD"](100))); env.require(balance(gw, alice["USD"](-100))); - // create AMM pool of XRP/USD + // 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 @@ -231,12 +280,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(10000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 100 USD to Alice + // gw issues 100 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(1000), alice); env(pay(gw, alice, USD(100))); @@ -257,6 +306,38 @@ class AMMClawback_test : public jtx::AMMTest 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. { @@ -266,18 +347,18 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(10000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 100 USD to Alice + // 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 + // 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, @@ -294,7 +375,7 @@ class AMMClawback_test : public jtx::AMMTest ter(tecNO_PERMISSION)); } - // test clawing back xrp will return error + // Test clawing back XRP is being prohibited. { Env env(*this, features); Account gw{"gateway"}; @@ -302,22 +383,22 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 3000 USD to Alice + // 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 + // Alice creates AMM pool of XRP/USD. AMM amm(env, alice, XRP(1000), USD(2000), ter(tesSUCCESS)); env.close(); - // clawback XRP is prohibited. + // Clawback XRP is prohibited. env(ammClawback( gw, alice, @@ -342,12 +423,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 3000 USD to Alice + // gw issues 3000 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(100000), alice); env(pay(gw, alice, USD(3000))); @@ -378,12 +459,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, gw2, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 3000 USD to Alice + // gw issues 3000 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(100000), alice); env(pay(gw, alice, USD(3000))); @@ -391,7 +472,7 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(gw, alice["USD"](-3000))); env.require(balance(alice, gw["USD"](3000))); - // gateway2 issues 3000 EUR to Alice + // gw2 issues 3000 EUR to Alice. auto const EUR = gw2["EUR"]; env.trust(EUR(100000), alice); env(pay(gw2, alice, EUR(3000))); @@ -399,21 +480,16 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(gw2, alice["EUR"](-3000))); env.require(balance(alice, gw2["EUR"](3000))); - // Alice creates AMM pool of EUR/USD + // 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 + // gw clawback 1000 USD from the AMM pool. env(ammClawback( - gw, - alice, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -441,12 +517,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, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -473,12 +544,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 3000 USD to Alice + // gw issues 3000 USD to Alice. auto const USD = gw["USD"]; env.trust(USD(100000), alice); env(pay(gw, alice, USD(3000))); @@ -486,7 +557,7 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(gw, alice["USD"](-3000))); env.require(balance(alice, gw["USD"](3000))); - // Alice creates AMM pool of XRP/USD + // Alice creates AMM pool of XRP/USD. AMM amm(env, alice, XRP(1000), USD(2000), ter(tesSUCCESS)); env.close(); @@ -494,14 +565,10 @@ class AMMClawback_test : public jtx::AMMTest USD(2000), XRP(1000), IOUAmount{1414213562373095, -9})); auto aliceXrpBalance = env.balance(alice, XRP); - // gw clawback 1000 USD from the AMM pool + + // gw clawback 1000 USD from the AMM pool. env(ammClawback( - gw, - alice, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -527,12 +594,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, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -570,19 +632,19 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, gw2, alice); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 6000 USD to Alice + // 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))); - // gateway2 issues 6000 EUR to Alice + // gw2 issues 6000 EUR to Alice. auto const EUR = gw2["EUR"]; env.trust(EUR(100000), alice); env(pay(gw2, alice, EUR(6000))); @@ -598,12 +660,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 1000 USD from the AMM pool env(ammClawback( - gw, - alice, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -628,12 +685,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback another 500 USD from the AMM pool. env(ammClawback( - gw, - alice, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 500}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(500), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -650,18 +702,13 @@ class AMMClawback_test : public jtx::AMMTest env.balance(alice, EUR) == STAmount(EUR, UINT64_C(2874999999999999), -12)); - // gw clawback small amount, 1 USD + // gw clawback small amount, 1 USD. env(ammClawback( - gw, - alice, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(1), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); - // another 1 USD / 1.25 EUR was withdrawn. + // Another 1 USD / 1.25 EUR was withdrawn. env.require(balance(alice, gw["USD"](2000))); BEAST_EXPECT(amm.expectBalances( @@ -673,15 +720,10 @@ class AMMClawback_test : public jtx::AMMTest env.balance(alice, EUR) == STAmount(EUR, UINT64_C(2876249999999998), -12)); - // gw clawback a large amount, exceeding the current balance. We + // gw clawback 4000 USD, exceeding the current balance. We // will clawback all. env(ammClawback( - gw, - alice, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 4000}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(4000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -710,17 +752,17 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, gw2, alice, bob); env.close(); - // gw sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gw2 sets asfAllowTrustLineClawback + // gw2 sets asfAllowTrustLineClawback. env(fset(gw2, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw2, asfAllowTrustLineClawback)); - // gateway issues 6000 USD to Alice and 5000 USD to Bob. + // 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))); @@ -728,7 +770,7 @@ class AMMClawback_test : public jtx::AMMTest env(pay(gw, bob, USD(5000))); env.close(); - // gateway2 issues 5000 EUR to Alice and 4000 EUR to Bob. + // 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))); @@ -765,12 +807,7 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 500 USD from alice in amm env(ammClawback( - gw, - alice, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 500}, - amm.ammAccount(), - std::nullopt), + gw, alice, USD, USD(500), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -779,10 +816,10 @@ class AMMClawback_test : public jtx::AMMTest // back from the AMM pool, so she still has 5000 USD. env.require(balance(alice, gw["USD"](5000))); - // bob's balance is not changed. + // Bob's balance is not changed. env.require(balance(bob, gw["USD"](4000))); - // alice gets 1000 XRP back. + // Alice gets 1000 XRP back. BEAST_EXPECT( expectLedgerEntryRoot(env, alice, aliceXrpBalance + XRP(1000))); @@ -795,19 +832,14 @@ class AMMClawback_test : public jtx::AMMTest // gw clawback 10 USD from bob in amm. env(ammClawback( - gw, - bob, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 10}, - amm.ammAccount(), - std::nullopt), + 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. + // Bob gets 20 XRP back. BEAST_EXPECT( expectLedgerEntryRoot(env, bob, bobXrpBalance + XRP(20))); BEAST_EXPECT(amm.expectBalances( @@ -819,21 +851,16 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT( amm.expectLPTokens(bob, IOUAmount{1400071426749365, -9})); - // gw2 claw back 200 EUR from amm2 + // gw2 clawback 200 EUR from amm2. env(ammClawback( - gw2, - alice, - EUR, - STAmount{Issue{gw2["EUR"].currency, gw2.id()}, 200}, - amm2.ammAccount(), - std::nullopt), + 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. + // Alice gets 600 XRP back. BEAST_EXPECT(expectLedgerEntryRoot( env, alice, aliceXrpBalance + XRP(1000) + XRP(600))); BEAST_EXPECT(amm2.expectBalances( @@ -847,21 +874,14 @@ class AMMClawback_test : public jtx::AMMTest // balance. This will clawback all the remaining LP tokens of alice // (corresponding 500 USD / 1000 XRP). env(ammClawback( - gw, - alice, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - std::nullopt), + 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))); - // auto aliceXrpBalance3 = env.balance(alice, XRP); - // auto bobXrpBalance2 = env.balance(bob, XRP); - // alice gets 1000 XRP back. + // Alice gets 1000 XRP back. BEAST_EXPECT(expectLedgerEntryRoot( env, alice, @@ -878,12 +898,7 @@ class AMMClawback_test : public jtx::AMMTest // balance in amm. All bob's lptoken in amm will be consumed, which // corresponds to 990 USD / 1980 XRP env(ammClawback( - gw, - bob, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - std::nullopt), + gw, bob, USD, USD(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); @@ -897,7 +912,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(expectLedgerEntryRoot( env, bob, bobXrpBalance + XRP(20) + XRP(1980))); - // now neither alice nor bob has any lptoken in amm. + // Now neither alice nor bob has any lptoken in amm. BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(0))); @@ -908,7 +923,7 @@ class AMMClawback_test : public jtx::AMMTest gw2, alice, EUR, - STAmount{Issue{gw2["EUR"].currency, gw2.id()}, 1000}, + EUR(1000), amm2.ammAccount(), std::nullopt), ter(tesSUCCESS)); @@ -917,7 +932,7 @@ class AMMClawback_test : public jtx::AMMTest 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 + // Alice gets another 2400 XRP back, bob's XRP balance remains the // same. BEAST_EXPECT(expectLedgerEntryRoot( env, @@ -927,7 +942,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(expectLedgerEntryRoot( env, bob, bobXrpBalance + XRP(20) + XRP(1980))); - // alice now does not have any lptoken in amm2 + // Alice now does not have any lptoken in amm2 BEAST_EXPECT(amm2.expectLPTokens(alice, IOUAmount(0))); BEAST_EXPECT(amm2.expectBalances( @@ -937,19 +952,14 @@ class AMMClawback_test : public jtx::AMMTest // balance. All bob's lptokens will be consumed, which corresponds // to 1000EUR / 3000 XRP. env(ammClawback( - gw2, - bob, - EUR, - STAmount{Issue{gw2["EUR"].currency, gw2.id()}, 2000}, - amm2.ammAccount(), - std::nullopt), + 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 + // Bob gets another 3000 XRP back. Alice's XRP balance remains the // same. BEAST_EXPECT(expectLedgerEntryRoot( env, @@ -959,7 +969,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(expectLedgerEntryRoot( env, bob, bobXrpBalance + XRP(20) + XRP(1980) + XRP(3000))); - // neither alice nor bob has any lptoken in amm2 + // Neither alice nor bob has any lptoken in amm2 BEAST_EXPECT(amm2.expectLPTokens(alice, IOUAmount(0))); BEAST_EXPECT(amm2.expectLPTokens(bob, IOUAmount(0))); @@ -986,17 +996,17 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, gw2, alice, bob, carol); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gw2 sets asfAllowTrustLineClawback + // gw2 sets asfAllowTrustLineClawback. env(fset(gw2, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw2, asfAllowTrustLineClawback)); - // gateway issues 6000 USD to Alice, 5000 USD to Bob, and 4000 USD + // 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); @@ -1007,7 +1017,7 @@ class AMMClawback_test : public jtx::AMMTest env(pay(gw, carol, USD(4000))); env.close(); - // gateway2 issues 6000 EUR to Alice and 5000 EUR to Bob and 4000 + // 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); @@ -1045,7 +1055,7 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(carol, gw["USD"](3000))); env.require(balance(carol, gw2["EUR"](2750))); - // clawback all the bob's USD in amm. (2000 USD / 2500 EUR) + // 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)); @@ -1062,7 +1072,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT( amm.expectLPTokens(carol, IOUAmount{1118033988749895, -12})); - // bob will get 2500 EUR back. + // Bob will get 2500 EUR back. env.require(balance(alice, gw["USD"](2000))); env.require(balance(alice, gw2["EUR"](1000))); BEAST_EXPECT( @@ -1075,7 +1085,7 @@ class AMMClawback_test : public jtx::AMMTest env.require(balance(carol, gw["USD"](3000))); env.require(balance(carol, gw2["EUR"](2750))); - // clawback all carol's EUR in amm. (1000 USD / 1250 EUR) + // gw2 clawback all carol's EUR in amm. (1000 USD / 1250 EUR) env(ammClawback( gw2, carol, @@ -1095,7 +1105,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(amm.expectLPTokens(bob, IOUAmount(0))); BEAST_EXPECT(amm.expectLPTokens(carol, IOUAmount(0))); - // clawback all alice's EUR in amm. (4000 USD / 5000 EUR) + // gw2 clawback all alice's EUR in amm. (4000 USD / 5000 EUR) env(ammClawback( gw2, alice, @@ -1126,7 +1136,7 @@ class AMMClawback_test : public jtx::AMMTest env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gateway issues 600000 USD to Alice and 500000 USD to Bob. + // 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))); @@ -1149,7 +1159,7 @@ class AMMClawback_test : public jtx::AMMTest auto aliceXrpBalance = env.balance(alice, XRP); auto bobXrpBalance = env.balance(bob, XRP); - // clawback all alice's USD in amm. (1000 USD / 200 XRP) + // gw clawback all alice's USD in amm. (1000 USD / 200 XRP) env(ammClawback( gw, alice, @@ -1165,7 +1175,7 @@ class AMMClawback_test : public jtx::AMMTest expectLedgerEntryRoot(env, alice, aliceXrpBalance + XRP(200))); BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount(0))); - // clawback all bob's USD in amm. (2000 USD / 400 XRP) + // 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)); @@ -1197,7 +1207,7 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, alice, bob, carol); env.close(); - // gateway sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); @@ -1231,14 +1241,9 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT( amm.expectBalances(USD(14000), EUR(3500), IOUAmount(7000))); - // claw back 1000 USD from carol + // gw clawback 1000 USD from carol. env(ammClawback( - gw, - carol, - USD, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - std::nullopt), + gw, carol, USD, USD(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( @@ -1255,16 +1260,11 @@ class AMMClawback_test : public jtx::AMMTest // 250 EUR goes back to carol. BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); - // claw back 1000 USD from bob with tfClawTwoAssets flag. + // 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, - STAmount{Issue{gw["USD"].currency, gw.id()}, 1000}, - amm.ammAccount(), - tfClawTwoAssets), + gw, bob, USD, USD(1000), amm.ammAccount(), tfClawTwoAssets), ter(tesSUCCESS)); env.close(); BEAST_EXPECT( @@ -1281,7 +1281,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(carol, USD) == USD(6000)); BEAST_EXPECT(env.balance(carol, EUR) == EUR(7750)); - // clawback all USD from alice and set tfClawTwoAssets + // gw clawback all USD from alice and set tfClawTwoAssets. env(ammClawback( gw, alice, @@ -1322,12 +1322,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, gw2, alice, bob); env.close(); - // gw sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gw2 sets asfAllowTrustLineClawback + // gw2 sets asfAllowTrustLineClawback. env(fset(gw2, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw2, asfAllowTrustLineClawback)); @@ -1356,7 +1356,7 @@ class AMMClawback_test : public jtx::AMMTest gw2["USD"](4500), IOUAmount{3674234614174767, -12})); - // issuer does not match with asset. + // Issuer does not match with asset. env(ammClawback( gw, alice, @@ -1414,7 +1414,7 @@ class AMMClawback_test : public jtx::AMMTest 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. + // Bob gets 3000 gw2["USD"] back and now his balance is 5000. BEAST_EXPECT(env.balance(bob, gw2["USD"]) == gw2["USD"](5000)); } @@ -1433,12 +1433,12 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, gw2, alice); env.close(); - // gw sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); - // gw2 sets asfAllowTrustLineClawback + // gw2 sets asfAllowTrustLineClawback. env(fset(gw2, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw2, asfAllowTrustLineClawback)); @@ -1474,7 +1474,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT( amm.expectLPTokens(alice, IOUAmount{4242640687119285, -12})); - // gw claws back 1000 USD from gw2 + // gw claws back 1000 USD from gw2. env(ammClawback( gw, gw2, USD, USD(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); @@ -1492,7 +1492,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(gw, EUR) == EUR(4000)); BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); - // gw2 claws back 1000 EUR from gw + // gw2 claws back 1000 EUR from gw. env(ammClawback( gw2, gw, EUR, EUR(1000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); @@ -1512,7 +1512,7 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(env.balance(gw, EUR) == EUR(4000)); BEAST_EXPECT(env.balance(gw2, USD) == USD(3000)); - // gw2 claws back 4000 EUR from alice + // gw2 claws back 4000 EUR from alice. env(ammClawback( gw2, alice, EUR, EUR(4000), amm.ammAccount(), std::nullopt), ter(tesSUCCESS)); @@ -1547,7 +1547,7 @@ class AMMClawback_test : public jtx::AMMTest env.fund(XRP(1000000), gw, alice); env.close(); - // gw sets asfAllowTrustLineClawback + // gw sets asfAllowTrustLineClawback. env(fset(gw, asfAllowTrustLineClawback)); env.close(); env.require(flags(gw, asfAllowTrustLineClawback)); diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index fbeede3149e..26c18d3f4b4 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -7030,6 +7030,7 @@ struct AMM_test : public jtx::AMMTest testInvalidInstance(); testInstanceCreate(); testInvalidDeposit(all); + testInvalidDeposit(all - featureAMMClawback); testDeposit(); testInvalidWithdraw(); testWithdraw(); diff --git a/src/xrpld/app/misc/detail/AMMUtils.cpp b/src/xrpld/app/misc/detail/AMMUtils.cpp index 7c20bb26326..d5cf3503433 100644 --- a/src/xrpld/app/misc/detail/AMMUtils.cpp +++ b/src/xrpld/app/misc/detail/AMMUtils.cpp @@ -457,7 +457,9 @@ withdraw( FreezeHandling::fhZERO_IF_FROZEN, journal); if (!expected) + // LCOV_EXCL_START return {expected.error(), STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_END auto const [curBalance, curBalance2, _] = *expected; (void)_; @@ -492,11 +494,13 @@ withdraw( 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 @@ -551,9 +555,11 @@ withdraw( 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 @@ -568,9 +574,11 @@ withdraw( 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 } } @@ -583,8 +591,10 @@ withdraw( 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( @@ -609,7 +619,7 @@ deleteAMMAccountIfEmpty( { ter = deleteAMMAccount(sb, issue1, issue2, journal); if (ter != tesSUCCESS && ter != tecINCOMPLETE) - return {ter, false}; + return {ter, false}; // LCOV_EXCL_LINE else updateBalance = (ter == tecINCOMPLETE); } diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 0090c35591a..4512e860d30 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -40,7 +40,7 @@ AMMClawback::preflight(PreflightContext const& ctx) return temDISABLED; if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; + return ret; // LCOV_EXCL_LINE if (ctx.tx.getFlags() & tfAMMClawbackMask) return temINVALID_FLAG; @@ -75,7 +75,7 @@ AMMClawback::preflight(PreflightContext const& ctx) return temBAD_ASSET_AMOUNT; } - if (clawAmount && (*clawAmount < beast::zero || isXRP(*clawAmount))) + if (clawAmount && *clawAmount <= beast::zero) return temBAD_AMOUNT; return preflight2(ctx); @@ -117,11 +117,7 @@ AMMClawback::preclaim(PreclaimContext const& ctx) auto const sleAMM = ctx.view.read(keylet::amm(ammID)); if (!sleAMM) - { - JLOG(ctx.j.trace()) - << "AMMClawback: can not find AMM with ammID: " << ammID; - return tecINTERNAL; - } + return tecINTERNAL; // LCOV_EXCL_LINE STIssue const& asset = sleAMM->getFieldIssue(sfAsset); STIssue const& asset2 = sleAMM->getFieldIssue(sfAsset2); @@ -153,14 +149,14 @@ AMMClawback::doApply() { Sandbox sb(&ctx_.view()); - auto const result = applyGuts(sb); - if (result.second) + auto const ter = applyGuts(sb); + if (ter == tesSUCCESS) sb.apply(ctx_.rawView()); - return result.first; + return ter; } -std::pair +TER AMMClawback::applyGuts(Sandbox& sb) { std::optional const clawAmount = ctx_.tx[~sfAmount]; @@ -172,18 +168,16 @@ AMMClawback::applyGuts(Sandbox& sb) auto const sleAMMAccount = ctx_.view().read(keylet::account(ammAccount)); // should not happen. checked in preclaim. - // LCOV_EXCL_START if (!sleAMMAccount) - return {terNO_AMM, false}; + return terNO_AMM; // LCOV_EXCL_LINE auto const ammID = sleAMMAccount->getFieldH256(sfAMMID); if (!ammID) - return {tecINTERNAL, false}; + return tecINTERNAL; // LCOV_EXCL_LINE auto ammSle = sb.peek(keylet::amm(ammID)); if (!ammSle) - return {tecINTERNAL, false}; - // LCOV_EXCL_STOP + return tecINTERNAL; // LCOV_EXCL_LINE auto const tfee = getTradingFee(ctx_.view(), *ammSle, ammAccount); Issue const& issue1 = ammSle->getFieldIssue(sfAsset).issue(); @@ -202,7 +196,7 @@ AMMClawback::applyGuts(Sandbox& sb) ctx_.journal); if (!expected) - return {expected.error(), false}; + return expected.error(); // LCOV_EXCL_LINE auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; TER result; @@ -212,7 +206,7 @@ AMMClawback::applyGuts(Sandbox& sb) auto const holdLPtokens = ammLPHolds(sb, *ammSle, holder, j_); if (holdLPtokens == beast::zero) - return {tecINTERNAL, false}; + return tecINTERNAL; if (!clawAmount) { @@ -246,12 +240,12 @@ AMMClawback::applyGuts(Sandbox& sb) tfee); if (result != tesSUCCESS) - return {result, false}; + return result; // LCOV_EXCL_LINE auto const res = deleteAMMAccountIfEmpty( sb, ammSle, newLPTokenBalance, issue1, issue2, j_); if (!res.second) - return {res.first, false}; + return res.first; // LCOV_EXCL_LINE JLOG(ctx_.journal.trace()) << "AMM Withdraw during AMMClawback: lptoken new balance: " @@ -260,22 +254,18 @@ AMMClawback::applyGuts(Sandbox& sb) auto const ter = rippleCredit(sb, holder, issuer, amountWithdraw, true, j_); if (ter != tesSUCCESS) - return {ter, false}; + 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) - { - // 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 ter = - rippleCredit(sb, holder, issuer, *amount2Withdraw, true, j_); - if (ter != tesSUCCESS) - return {ter, false}; - } - return {tesSUCCESS, true}; + return rippleCredit(sb, holder, issuer, *amount2Withdraw, true, j_); + + return tesSUCCESS; } std::tuple> diff --git a/src/xrpld/app/tx/detail/AMMClawback.h b/src/xrpld/app/tx/detail/AMMClawback.h index 874ab0c63ea..fcb56af1dc0 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.h +++ b/src/xrpld/app/tx/detail/AMMClawback.h @@ -43,7 +43,7 @@ class AMMClawback : public Transactor doApply() override; private: - std::pair + TER applyGuts(Sandbox& view); /** Withdraw both assets by providing maximum amount of asset1, diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index fe40b699477..d9bd22a0097 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -250,7 +250,7 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) // if either asset is frozen auto checkAsset = [&](std::optional const& asset) -> TER { if (!asset.has_value()) - return temMALFORMED; + return temMALFORMED; // LCOV_EXCL_LINE if (isFrozen(ctx.view, accountID, asset.value())) { diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 8f4a0e84511..9dd71d1f6f8 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -430,8 +430,10 @@ AMMWithdraw::applyGuts(Sandbox& sb) 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()) << " "