From f7735eace40558968119e04c752009bb7c4e09d8 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Mon, 11 Nov 2024 15:30:16 -0500 Subject: [PATCH] Fix adjust by tokens deposit --- src/test/app/AMM_test.cpp | 518 ++++++++++++++++--------- src/xrpld/app/tx/detail/AMMDeposit.cpp | 4 +- 2 files changed, 335 insertions(+), 187 deletions(-) diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index c7d1c74f39c..2a40e7493a1 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -1419,24 +1419,13 @@ struct AMM_test : public jtx::AMMTest }); // Single deposit: 1000 USD - testAMM( - [&](AMM& ammAlice, Env& env) { - ammAlice.deposit(carol, USD(1'000)); - if (!env.current()->rules().enabled(fixAMMv1_3)) - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), - STAmount{USD, UINT64_C(10'999'99999999999), -11}, - IOUAmount{10'488'088'48170151, -8})); - else - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), - USD(11'000), - IOUAmount{10'488'088'48170151, -8})); - }, - std::nullopt, - 0, - std::nullopt, - {all, all - fixAMMv1_3}); + testAMM([&](AMM& ammAlice, Env& env) { + ammAlice.deposit(carol, USD(1'000)); + BEAST_EXPECT(ammAlice.expectBalances( + XRP(10'000), + STAmount{USD, UINT64_C(10'999'99999999999), -11}, + IOUAmount{10'488'088'48170151, -8})); + }); // Single deposit: 1000 XRP testAMM([&](AMM& ammAlice, Env&) { @@ -1529,25 +1518,14 @@ struct AMM_test : public jtx::AMMTest IOUAmount{1'000'000'000049999, -8})); BEAST_EXPECT(ammAlice.expectLPTokens(carol, IOUAmount{49999, -8})); }); - testAMM( - [&](AMM& ammAlice, Env& env) { - ammAlice.deposit(carol, STAmount{USD, 1, -10}); - if (!env.current()->rules().enabled(fixAMMv1_3)) - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), - STAmount{USD, UINT64_C(10'000'00000000008), -11}, - IOUAmount{10'000'000'00000004, -8})); - else - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), - STAmount{USD, UINT64_C(10'000'0000000001), -10}, - IOUAmount{10'000'000'00000004, -8})); - BEAST_EXPECT(ammAlice.expectLPTokens(carol, IOUAmount{4, -8})); - }, - std::nullopt, - 0, - std::nullopt, - {all, all - fixAMMv1_3}); + testAMM([&](AMM& ammAlice, Env& env) { + ammAlice.deposit(carol, STAmount{USD, 1, -10}); + BEAST_EXPECT(ammAlice.expectBalances( + XRP(10'000), + STAmount{USD, UINT64_C(10'000'00000000008), -11}, + IOUAmount{10'000'000'00000004, -8})); + BEAST_EXPECT(ammAlice.expectLPTokens(carol, IOUAmount{4, -8})); + }); // Issuer create/deposit for (auto const& feat : {all, all - fixAMMv1_3}) @@ -1561,40 +1539,23 @@ struct AMM_test : public jtx::AMMTest BEAST_EXPECT(ammGw.expectBalances( XRP(11'000), USD(11'000), IOUAmount{11'000'000})); ammGw.deposit(gw, USD(1'000)); - if (!env.current()->rules().enabled(fixAMMv1_3)) - BEAST_EXPECT(ammGw.expectBalances( - XRP(11'000), - STAmount{USD, UINT64_C(11'999'99999999998), -11}, - IOUAmount{11'489'125'29307605, -8})); - else - BEAST_EXPECT(ammGw.expectBalances( - XRP(11'000), - USD(12'000), - IOUAmount{11'489'125'29307605, -8})); + BEAST_EXPECT(ammGw.expectBalances( + XRP(11'000), + STAmount{USD, UINT64_C(11'999'99999999998), -11}, + IOUAmount{11'489'125'29307605, -8})); } // Issuer deposit - testAMM( - [&](AMM& ammAlice, Env& env) { - ammAlice.deposit(gw, 1'000'000); - BEAST_EXPECT(ammAlice.expectBalances( - XRP(11'000), USD(11'000), IOUAmount{11'000'000})); - ammAlice.deposit(gw, USD(1'000)); - if (!env.current()->rules().enabled(fixAMMv1_3)) - BEAST_EXPECT(ammAlice.expectBalances( - XRP(11'000), - STAmount{USD, UINT64_C(11'999'99999999998), -11}, - IOUAmount{11'489'125'29307605, -8})); - else - BEAST_EXPECT(ammAlice.expectBalances( - XRP(11'000), - USD(12'000), - IOUAmount{11'489'125'29307605, -8})); - }, - std::nullopt, - 0, - std::nullopt, - {all, all - fixAMMv1_3}); + testAMM([&](AMM& ammAlice, Env& env) { + ammAlice.deposit(gw, 1'000'000); + BEAST_EXPECT(ammAlice.expectBalances( + XRP(11'000), USD(11'000), IOUAmount{11'000'000})); + ammAlice.deposit(gw, USD(1'000)); + BEAST_EXPECT(ammAlice.expectBalances( + XRP(11'000), + STAmount{USD, UINT64_C(11'999'99999999998), -11}, + IOUAmount{11'489'125'29307605, -8})); + }); // Min deposit testAMM([&](AMM& ammAlice, Env& env) { @@ -1639,33 +1600,22 @@ struct AMM_test : public jtx::AMMTest BEAST_EXPECT(ammAlice.expectBalances( XRP(11'000), USD(10'000), IOUAmount{10'488'088'48170151, -8})); }); - testAMM( - [&](AMM& ammAlice, Env& env) { - // Single deposit by asset - ammAlice.deposit( - carol, - 488'088, - USD(1'000), - std::nullopt, - std::nullopt, - tfSingleAsset, - std::nullopt, - std::nullopt); - if (!env.current()->rules().enabled(fixAMMv1_3)) - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), - STAmount{USD, UINT64_C(10'999'99999999999), -11}, - IOUAmount{10'488'088'48170151, -8})); - else - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), - USD(11'000), - IOUAmount{10'488'088'48170151, -8})); - }, - std::nullopt, - 0, - std::nullopt, - {all, all - fixAMMv1_3}); + testAMM([&](AMM& ammAlice, Env& env) { + // Single deposit by asset + ammAlice.deposit( + carol, + 488'088, + USD(1'000), + std::nullopt, + std::nullopt, + tfSingleAsset, + std::nullopt, + std::nullopt); + BEAST_EXPECT(ammAlice.expectBalances( + XRP(10'000), + STAmount{USD, UINT64_C(10'999'99999999999), -11}, + IOUAmount{10'488'088'48170151, -8})); + }); } void @@ -2339,46 +2289,24 @@ struct AMM_test : public jtx::AMMTest }); // Single deposit 1000USD, withdraw all tokens in USD - testAMM( - [&](AMM& ammAlice, Env& env) { - ammAlice.deposit(carol, USD(1'000)); - ammAlice.withdrawAll(carol, USD(0)); - if (!env.enabled(fixAMMv1_3)) - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), USD(10'000), IOUAmount{10'000'000, 0})); - else - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), - STAmount{USD, UINT64_C(10'000'00000000001), -11}, - IOUAmount{10'000'000, 0})); - BEAST_EXPECT( - ammAlice.expectLPTokens(carol, IOUAmount(beast::Zero()))); - }, - std::nullopt, - 0, - std::nullopt, - {all, all - fixAMMv1_3}); + testAMM([&](AMM& ammAlice, Env& env) { + ammAlice.deposit(carol, USD(1'000)); + ammAlice.withdrawAll(carol, USD(0)); + BEAST_EXPECT(ammAlice.expectBalances( + XRP(10'000), USD(10'000), IOUAmount{10'000'000, 0})); + BEAST_EXPECT( + ammAlice.expectLPTokens(carol, IOUAmount(beast::Zero()))); + }); // Single deposit 1000USD, withdraw all tokens in XRP - testAMM( - [&](AMM& ammAlice, Env& env) { - ammAlice.deposit(carol, USD(1'000)); - ammAlice.withdrawAll(carol, XRP(0)); - if (!env.enabled(fixAMMv1_3)) - BEAST_EXPECT(ammAlice.expectBalances( - XRPAmount(9'090'909'091), - STAmount{USD, UINT64_C(10'999'99999999999), -11}, - IOUAmount{10'000'000, 0})); - else - BEAST_EXPECT(ammAlice.expectBalances( - XRPAmount(9'090'909'091), - USD(11'000), - IOUAmount{10'000'000, 0})); - }, - std::nullopt, - 0, - std::nullopt, - {all, all - fixAMMv1_3}); + testAMM([&](AMM& ammAlice, Env& env) { + ammAlice.deposit(carol, USD(1'000)); + ammAlice.withdrawAll(carol, XRP(0)); + BEAST_EXPECT(ammAlice.expectBalances( + XRPAmount(9'090'909'091), + STAmount{USD, UINT64_C(10'999'99999999999), -11}, + IOUAmount{10'000'000, 0})); + }); // Single deposit/withdraw by the same account testAMM( @@ -2398,7 +2326,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT(ammAlice.expectBalances( XRPAmount(10'000'000'001), - STAmount{USD, UINT64_C(10'000'00000000003), -11}, + USD(10'000), ammAlice.tokens())); BEAST_EXPECT(ammAlice.expectLPTokens(carol, IOUAmount{0})); }, @@ -2409,27 +2337,16 @@ struct AMM_test : public jtx::AMMTest // Single deposit by different accounts and then withdraw // in reverse. - testAMM( - [&](AMM& ammAlice, Env& env) { - auto const carolTokens = ammAlice.deposit(carol, USD(1'000)); - auto const aliceTokens = ammAlice.deposit(alice, USD(1'000)); - ammAlice.withdraw(alice, aliceTokens, USD(0)); - ammAlice.withdraw(carol, carolTokens, USD(0)); - if (!env.enabled(fixAMMv1_3)) - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), USD(10'000), ammAlice.tokens())); - else - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), - STAmount{USD, UINT64_C(10'000'00000000002), -11}, - ammAlice.tokens())); - BEAST_EXPECT(ammAlice.expectLPTokens(carol, IOUAmount{0})); - BEAST_EXPECT(ammAlice.expectLPTokens(alice, ammAlice.tokens())); - }, - std::nullopt, - 0, - std::nullopt, - {all, all - fixAMMv1_3}); + testAMM([&](AMM& ammAlice, Env& env) { + auto const carolTokens = ammAlice.deposit(carol, USD(1'000)); + auto const aliceTokens = ammAlice.deposit(alice, USD(1'000)); + ammAlice.withdraw(alice, aliceTokens, USD(0)); + ammAlice.withdraw(carol, carolTokens, USD(0)); + BEAST_EXPECT(ammAlice.expectBalances( + XRP(10'000), USD(10'000), ammAlice.tokens())); + BEAST_EXPECT(ammAlice.expectLPTokens(carol, IOUAmount{0})); + BEAST_EXPECT(ammAlice.expectLPTokens(alice, ammAlice.tokens())); + }); // Equal deposit 10%, withdraw all tokens testAMM([&](AMM& ammAlice, Env&) { @@ -3247,7 +3164,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT( env.balance(carol, USD) == - STAmount(USD, UINT64_C(29'499'00572620529), -11)); + STAmount(USD, UINT64_C(29'499'00572620545), -11)); if (!features[fixAMMv1_3]) BEAST_EXPECT( env.balance(bob, USD) == @@ -3255,7 +3172,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT( env.balance(bob, USD) == - STAmount(USD, UINT64_C(18'999'00572616184), -11)); + STAmount(USD, UINT64_C(18'999'00572616195), -11)); if (!features[fixAMMv1_3]) BEAST_EXPECT( env.balance(ed, USD) == @@ -3263,7 +3180,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT( env.balance(ed, USD) == - STAmount(USD, UINT64_C(18'999'00572611832), -11)); + STAmount(USD, UINT64_C(18'999'00572611841), -11)); // USD pool is slightly higher because of the fees. if (!features[fixAMMv1_3]) BEAST_EXPECT(ammAlice.expectBalances( @@ -3273,7 +3190,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT(ammAlice.expectBalances( XRPAmount{13'000'000'003}, - STAmount(USD, UINT64_C(13'002'98282151455), -11), + STAmount(USD, UINT64_C(13'002'98282151419), -11), ammTokens)); } ammTokens = ammAlice.getLPTokensBalance(); @@ -3323,7 +3240,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT( env.balance(dan, USD) == - STAmount(USD, UINT64_C(19'490'05672274384), -11)); + STAmount(USD, UINT64_C(19'490'056722744), -9)); // USD pool gains more in dan's fees. if (!features[fixAMMv1_3]) BEAST_EXPECT(ammAlice.expectBalances( @@ -3333,7 +3250,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT(ammAlice.expectBalances( XRPAmount{13'000'000'003}, - STAmount{USD, UINT64_C(13'012'92609877071), -11}, + STAmount{USD, UINT64_C(13'012'92609877019), -11}, ammTokens)); // Discounted fee payment ammAlice.deposit(carol, USD(100)); @@ -3346,7 +3263,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT(ammAlice.expectBalances( XRPAmount{13'000'000'003}, - STAmount{USD, UINT64_C(13'112'92609877071), -11}, + STAmount{USD, UINT64_C(13'112'92609877019), -11}, ammTokens)); env(pay(carol, bob, USD(100)), path(~USD), @@ -3362,7 +3279,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT(ammAlice.expectBalances( XRPAmount{13'100'000'671}, - STAmount{USD, UINT64_C(13'012'92609877071), -11}, + STAmount{USD, UINT64_C(13'012'92609877019), -11}, ammTokens)); } // Payment with the trading fee @@ -3389,29 +3306,21 @@ struct AMM_test : public jtx::AMMTest { BEAST_EXPECT(ammAlice.expectBalances( XRPAmount{13'000'000'671}, - STAmount{USD, UINT64_C(13'114'03663044985), -11}, + STAmount{USD, UINT64_C(13'114'03663044932), -11}, ammTokens)); } // Auction slot expired, no discounted fee env.close(seconds(TOTAL_TIME_SLOT_SECS + 1)); // clock is parent's based env.close(); - if (!features[fixAMMv1_1] && !features[fixAMMv1_3]) + if (!features[fixAMMv1_1] || features[fixAMMv1_3]) BEAST_EXPECT( env.balance(carol, USD) == STAmount(USD, UINT64_C(29'399'00572620545), -11)); - else if (features[fixAMMv1_1] && !features[fixAMMv1_3]) - { + else if (!features[fixAMMv1_3]) BEAST_EXPECT( env.balance(carol, USD) == STAmount(USD, UINT64_C(29'399'00572620544), -11)); - } - else - { - BEAST_EXPECT( - env.balance(carol, USD) == - STAmount(USD, UINT64_C(29'399'00572620529), -11)); - } ammTokens = ammAlice.getLPTokensBalance(); for (int i = 0; i < 10; ++i) { @@ -3444,10 +3353,10 @@ struct AMM_test : public jtx::AMMTest { BEAST_EXPECT( env.balance(carol, USD) == - STAmount(USD, UINT64_C(29'389'06197177097), -11)); + STAmount(USD, UINT64_C(29'389'06197177128), -11)); BEAST_EXPECT(ammAlice.expectBalances( XRPAmount{13'000'000'671}, - STAmount{USD, UINT64_C(13'123'98038488417), -11}, + STAmount{USD, UINT64_C(13'123'98038488349), -11}, ammTokens)); } env(pay(carol, bob, USD(100)), path(~USD), sendmax(XRP(110))); @@ -3473,7 +3382,7 @@ struct AMM_test : public jtx::AMMTest { BEAST_EXPECT(ammAlice.expectBalances( XRPAmount(13'100'824'793), - STAmount{USD, UINT64_C(13'023'98038488417), -11}, + STAmount{USD, UINT64_C(13'023'98038488349), -11}, ammTokens)); } }, @@ -5175,7 +5084,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT( env.balance(carol, USD) == - STAmount(USD, UINT64_C(30'443'43891402712), -11)); + STAmount(USD, UINT64_C(30'443'43891402713), -11)); // carol pays ~4008 LPTokens in fees or ~0.5% of the no-fee // LPTokens if (!features[fixAMMv1_1] && !features[fixAMMv1_3]) @@ -5494,7 +5403,7 @@ struct AMM_test : public jtx::AMMTest else BEAST_EXPECT(ammAlice.expectBalances( XRP(10'000), - STAmount{USD, UINT64_C(10'000'0000000041), -10}, + STAmount{USD, UINT64_C(10'000'000000001), -9}, IOUAmount{10'000'000})); BEAST_EXPECT(expectLine(env, ben, USD(1'500'000))); BEAST_EXPECT(expectLine(env, simon, USD(1'500'000))); @@ -5508,10 +5417,7 @@ struct AMM_test : public jtx::AMMTest else if (features[fixAMMv1_1] && !features[fixAMMv1_3]) BEAST_EXPECT(expectLine(env, carol, USD(30'000))); else - BEAST_EXPECT(expectLine( - env, - carol, - STAmount{USD, UINT64_C(29'999'99999999991), -11})); + BEAST_EXPECT(expectLine(env, carol, USD(30'000))); BEAST_EXPECT(expectLine(env, ed, USD(1'500'000))); BEAST_EXPECT(expectLine(env, paul, USD(1'500'000))); if (!features[fixAMMv1_1] && !features[fixAMMv1_3]) @@ -5528,7 +5434,7 @@ struct AMM_test : public jtx::AMMTest BEAST_EXPECT(expectLine( env, nataly, - STAmount{USD, UINT64_C(1'499'999'999999996), -9})); + STAmount{USD, UINT64_C(1'499'999'999999998), -9})); ammAlice.withdrawAll(alice); BEAST_EXPECT(!ammAlice.ammExists()); if (!features[fixAMMv1_1] && !features[fixAMMv1_3]) @@ -5542,7 +5448,7 @@ struct AMM_test : public jtx::AMMTest BEAST_EXPECT(expectLine( env, alice, - STAmount{USD, UINT64_C(30'000'0000000041), -10})); + STAmount{USD, UINT64_C(30'000'000000001), -9})); // alice XRP balance is 30,000initial - 50 ammcreate fee - // 10drops fee BEAST_EXPECT(accountBalance(env, alice) == "29949999990"); @@ -7476,6 +7382,246 @@ struct AMM_test : public jtx::AMMTest test(false); } + void + testDepositRounding() + { + testcase("Deposit Rounding"); + using namespace jtx; + + // single asset deposits into IOU/IOU AMM pool + // both LPTokens and asset-amounts are rounded down in AMMHelpers + // .cpp:adjustAmountsByLPTokens. The AMM Invariant is violated by ~1e-6 + for (STAmount const& depositEuro : + {/*STAmount(EUR, 1, 1), + STAmount(EUR, 1, 2), + STAmount(EUR, 1, 5),*/ + STAmount(EUR, 1, -3), + /*STAmount(EUR, 1, -6), + STAmount(EUR, 1, -9)*/}) + { + testAMM( + [&](AMM& ammAlice, Env& env) { + fund( + env, + gw, + {bob}, + XRP(10'000'000), + {GBP(100'000), EUR(100'000)}, + Fund::Acct); + env.close(); + + ammAlice.deposit( + DepositArg{.account = bob, .asset1In = depositEuro}); + }, + {{GBP(30'000), EUR(30'000)}}); + } + return; + // two-asset proportional deposit (1:1 pool ratio) + testAMM( + [&](AMM& ammAlice, Env& env) { + fund( + env, + gw, + {bob}, + XRP(10'000'000), + {GBP(100'000), EUR(100'000)}, + Fund::Acct); + env.close(); + + // although the following inputs are crafted to cause rounding, + // the deposit transaction does not violate the AMM invariants + const STAmount depositEuro{ + EUR, UINT64_C(10'1234567890123456), -16}; + const STAmount depositGBP{ + GBP, UINT64_C(10'1234567890123456), -16}; + + ammAlice.deposit(DepositArg{ + .account = bob, + .asset1In = depositEuro, + .asset2In = depositGBP}); + }, + {{GBP(30'000), EUR(30'000)}}); + + // two-asset proportional deposit (1:3 pool ratio) + // Since the pool assets are not in identical quantities, computation + // of resulting LPTokens could cause rounding adjustments + for (const int exponent : {1, 2, 3, 4, -3, -6, -9}) + { + testAMM( + [&](AMM& ammAlice, Env& env) { + fund( + env, + gw, + {bob}, + XRP(10'000'000), + {GBP(100'000), EUR(100'000)}, + Fund::Acct); + env.close(); + + const STAmount depositEuro{EUR, 1, exponent}; + const STAmount depositGBP{GBP, 1, exponent}; + + ammAlice.deposit(DepositArg{ + .account = bob, + .asset1In = depositEuro, + .asset2In = depositGBP}); + }, + {{GBP(10'000), EUR(30'000)}}); + } + + // tfLPToken based deposits + // Observation: adjustLPTokens does not make any changes to LPTokens + // in this deposit mode. No rounding-based violations are seen either. + testAMM( + [&](AMM& ammAlice, Env& env) { + fund( + env, + gw, + {bob}, + XRP(10'000'000), + {GBP(100'000), EUR(100'000)}, + Fund::Acct); + env.close(); + + ammAlice.deposit(DepositArg{ + .account = bob, + .tokens = IOUAmount{10'1234567890123456, -16}}); + }, + {{GBP(7'000), EUR(30'000)}}); + + // tfOneAssetLPToken deposits -- AMM invariant is violated + // This error isn't observed with inputs of 10, 100. + // ammAssetsIn is responsible for handling the LPTokens and amount1 + // for this deposit mode. + for (const unsigned int tokens : {1'000, 10'000}) + { + testAMM( + [&](AMM& ammAlice, Env& env) { + fund( + env, + gw, + {bob}, + XRP(10'000'000), + {GBP(100'000), EUR(1'000'000)}, + Fund::Acct); + env.close(); + + ammAlice.deposit(DepositArg{ + .account = bob, + .tokens = tokens, + .asset1In = STAmount{EUR, 1, 6}}); + }, + {{GBP(7'000), EUR(30'000)}}); + } + + // Single deposit with EP not exceeding specified: + // 1'000 GBP with EP not to exceed 5 (GBP/TokensOut) + testAMM( + [&](AMM& ammAlice, Env& env) { + fund( + env, + gw, + {bob}, + XRP(10'000'000), + {GBP(100'000), EUR(100'000)}, + Fund::Acct); + env.close(); + + ammAlice.deposit( + bob, GBP(1'000), std::nullopt, STAmount{GBP, 5}); + }, + {{GBP(30'000), EUR(30'000)}}); + } + + void + testWithdrawRounding() + { + testcase("Withdraw Rounding"); + + using namespace jtx; + + // tfLPToken mode + testAMM( + [&](AMM& ammAlice, Env& env) { ammAlice.withdraw(alice, 1'000); }, + {{GBP(7'000), EUR(30'000)}}); + + // tfWithdrawAll mode + testAMM( + [&](AMM& ammAlice, Env& env) { + ammAlice.withdraw( + WithdrawArg{.account = alice, .flags = tfWithdrawAll}); + }, + {{GBP(7'000), EUR(30'000)}}); + + // tfTwoAsset withdraw mode + testAMM( + [&](AMM& ammAlice, Env& env) { + ammAlice.withdraw(WithdrawArg{ + .account = alice, + .asset1Out = STAmount{GBP, 3'500}, + .asset2Out = STAmount{EUR, 15'000}, + .flags = tfTwoAsset}); + }, + {{GBP(7'000), EUR(30'000)}}); + + // tfSingleAsset withdraw mode + // Note: This test fails with 0 trading fees, but doesn't fail if + // trading fees is set to 1'000 -- I suspect the compound operations + // in AMMHelpers.cpp:withdrawByTokens compensate for the rounding errors + testAMM( + [&](AMM& ammAlice, Env& env) { + ammAlice.withdraw(WithdrawArg{ + .account = alice, + .asset1Out = STAmount{GBP, 1'234}, + .flags = tfSingleAsset}); + }, + {{GBP(7'000), EUR(30'000)}}); + + // tfOneAssetWithdrawAll mode + testAMM( + [&](AMM& ammAlice, Env& env) { + fund( + env, + gw, + {bob}, + XRP(10'000'000), + {GBP(100'000), EUR(100'000)}, + Fund::Acct); + env.close(); + + ammAlice.deposit(DepositArg{ + .account = bob, .asset1In = STAmount{GBP, 3'456}}); + + ammAlice.withdraw(WithdrawArg{ + .account = bob, + .asset1Out = STAmount{GBP, 1'000}, + .flags = tfOneAssetWithdrawAll}); + }, + {{GBP(7'000), EUR(30'000)}}); + + // tfOneAssetLPToken mode + testAMM( + [&](AMM& ammAlice, Env& env) { + ammAlice.withdraw(WithdrawArg{ + .account = alice, + .tokens = 1'000, + .asset1Out = STAmount{GBP, 100}, + .flags = tfOneAssetLPToken}); + }, + {{GBP(7'000), EUR(30'000)}}); + + // tfLimitLPToken mode + testAMM( + [&](AMM& ammAlice, Env& env) { + ammAlice.withdraw(WithdrawArg{ + .account = alice, + .asset1Out = STAmount{GBP, 100}, + .maxEP = IOUAmount{2}, + .flags = tfLimitLPToken}); + }, + {{GBP(7'000), EUR(30'000)}}); + } + void run() override { @@ -7538,6 +7684,8 @@ struct AMM_test : public jtx::AMMTest all - fixAMMv1_1 - fixAMMv1_3 - featureAMMClawback); testDepositAndWithdrawRoundingV2(all); testDepositAndWithdrawRoundingV2(all - fixAMMv1_3); + // testDepositRounding(); + // testWithdrawRounding(); } }; diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index 625104a4be3..633df42f70b 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -816,7 +816,7 @@ AMMDeposit::singleDeposit( view, ammAccount, amountBalance, - amount, + amountDeposit, std::nullopt, lptAMMBalance, tokens, @@ -980,7 +980,7 @@ AMMDeposit::singleDepositEPrice( view, ammAccount, amountBalance, - amountDeposit, + amountDepositActual, std::nullopt, lptAMMBalance, tokens,