diff --git a/src/ripple/app/misc/impl/AMMUtils.cpp b/src/ripple/app/misc/impl/AMMUtils.cpp index d766bc508b6..cf4da2c5d85 100644 --- a/src/ripple/app/misc/impl/AMMUtils.cpp +++ b/src/ripple/app/misc/impl/AMMUtils.cpp @@ -60,9 +60,12 @@ ammHolds( *optIssue2, std::make_optional(std::make_pair(issue1, issue2)))) { + // This error can only be hit if the AMM is corrupted + // LCOV_EXCL_START JLOG(j.debug()) << "ammHolds: Invalid optIssue1 or optIssue2 " << *optIssue1 << " " << *optIssue2; return std::nullopt; + // LCOV_EXCL_STOP } return std::make_optional(std::make_pair(*optIssue1, *optIssue2)); } @@ -74,9 +77,12 @@ ammHolds( return std::make_optional(std::make_pair(issue1, issue2)); else if (checkIssue == issue2) return std::make_optional(std::make_pair(issue2, issue1)); + // Unreachable unless AMM corrupted. + // LCOV_EXCL_START JLOG(j.debug()) << "ammHolds: Invalid " << label << " " << checkIssue; return std::nullopt; + // LCOV_EXCL_STOP }; if (optIssue1) { @@ -84,7 +90,8 @@ ammHolds( } else if (optIssue2) { - return singleIssue(*optIssue2, "optIssue2"); + // Cannot have Amount2 without Amount. + return singleIssue(*optIssue2, "optIssue2"); // LCOV_EXCL_LINE } return std::make_optional(std::make_pair(issue1, issue2)); }(); @@ -210,19 +217,23 @@ deleteAMMTrustLines( // Should only have the trustlines if (nodeType != LedgerEntryType::ltRIPPLE_STATE) { + // LCOV_EXCL_START JLOG(j.error()) << "deleteAMMTrustLines: deleting non-trustline " << nodeType; return {tecINTERNAL, SkipEntry::No}; + // LCOV_EXCL_STOP } // Trustlines must have zero balance if (sleItem->getFieldAmount(sfBalance) != beast::zero) { + // LCOV_EXCL_START JLOG(j.error()) << "deleteAMMTrustLines: deleting trustline with " "non-zero balance."; return {tecINTERNAL, SkipEntry::No}; + // LCOV_EXCL_STOP } return { @@ -243,18 +254,22 @@ deleteAMMAccount( auto ammSle = sb.peek(keylet::amm(asset, asset2)); if (!ammSle) { + // LCOV_EXCL_START JLOG(j.error()) << "deleteAMMAccount: AMM object does not exist " << asset << " " << asset2; return tecINTERNAL; + // LCOV_EXCL_STOP } auto const ammAccountID = (*ammSle)[sfAccount]; auto sleAMMRoot = sb.peek(keylet::account(ammAccountID)); if (!sleAMMRoot) { + // LCOV_EXCL_START JLOG(j.error()) << "deleteAMMAccount: AMM account does not exist " << to_string(ammAccountID); return tecINTERNAL; + // LCOV_EXCL_STOP } if (auto const ter = @@ -266,14 +281,18 @@ deleteAMMAccount( if (!sb.dirRemove( ownerDirKeylet, (*ammSle)[sfOwnerNode], ammSle->key(), false)) { + // LCOV_EXCL_START JLOG(j.error()) << "deleteAMMAccount: failed to remove dir link"; return tecINTERNAL; + // LCOV_EXCL_STOP } if (sb.exists(ownerDirKeylet) && !sb.emptyDirDelete(ownerDirKeylet)) { + // LCOV_EXCL_START JLOG(j.error()) << "deleteAMMAccount: cannot delete root dir node of " << toBase58(ammAccountID); return tecINTERNAL; + // LCOV_EXCL_STOP } sb.erase(ammSle); @@ -322,11 +341,11 @@ initializeFeeAuctionVote( if (tfee != 0) ammSle->setFieldU16(sfTradingFee, tfee); else if (ammSle->isFieldPresent(sfTradingFee)) - ammSle->makeFieldAbsent(sfTradingFee); + ammSle->makeFieldAbsent(sfTradingFee); // LCOV_EXCL_LINE if (auto const dfee = tfee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION) auctionSlot.setFieldU16(sfDiscountedFee, dfee); else if (auctionSlot.isFieldPresent(sfDiscountedFee)) - auctionSlot.makeFieldAbsent(sfDiscountedFee); + auctionSlot.makeFieldAbsent(sfDiscountedFee); // LCOV_EXCL_LINE } } // namespace ripple diff --git a/src/ripple/app/paths/AMMOffer.h b/src/ripple/app/paths/AMMOffer.h index 426ba96a772..e3fb41e220b 100644 --- a/src/ripple/app/paths/AMMOffer.h +++ b/src/ripple/app/paths/AMMOffer.h @@ -74,9 +74,6 @@ class AMMOffer Issue const& issueIn() const; - Issue const& - issueOut() const; - AccountID const& owner() const; diff --git a/src/ripple/app/paths/impl/AMMOffer.cpp b/src/ripple/app/paths/impl/AMMOffer.cpp index 759851b7afe..697bac9c790 100644 --- a/src/ripple/app/paths/impl/AMMOffer.cpp +++ b/src/ripple/app/paths/impl/AMMOffer.cpp @@ -44,13 +44,6 @@ AMMOffer::issueIn() const return ammLiquidity_.issueIn(); } -template -Issue const& -AMMOffer::issueOut() const -{ - return ammLiquidity_.issueOut(); -} - template AccountID const& AMMOffer::owner() const diff --git a/src/ripple/app/tx/impl/AMMDeposit.cpp b/src/ripple/app/tx/impl/AMMDeposit.cpp index 5ea49f5445c..74cba160edc 100644 --- a/src/ripple/app/tx/impl/AMMDeposit.cpp +++ b/src/ripple/app/tx/impl/AMMDeposit.cpp @@ -186,7 +186,7 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) FreezeHandling::fhIGNORE_FREEZE, ctx.j); if (!expected) - return expected.error(); + return expected.error(); // LCOV_EXCL_LINE auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; if (ctx.tx.getFlags() & tfTwoAssetIfEmpty) { @@ -194,8 +194,10 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) return tecAMM_NOT_EMPTY; if (amountBalance != beast::zero || amount2Balance != beast::zero) { + // LCOV_EXCL_START JLOG(ctx.j.debug()) << "AMM Deposit: tokens balance is not zero."; return tecINTERNAL; + // LCOV_EXCL_STOP } } else @@ -205,9 +207,11 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) if (amountBalance <= beast::zero || amount2Balance <= beast::zero || lptAMMBalance < beast::zero) { + // LCOV_EXCL_START JLOG(ctx.j.debug()) << "AMM Deposit: reserves or tokens balance is zero."; return tecINTERNAL; + // LCOV_EXCL_STOP } } @@ -254,10 +258,12 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) if (auto const ter = requireAuth(ctx.view, amount->issue(), accountID)) { + // LCOV_EXCL_START JLOG(ctx.j.debug()) << "AMM Deposit: account is not authorized, " << amount->issue(); return ter; + // LCOV_EXCL_STOP } // AMM account or currency frozen if (isFrozen(ctx.view, ammAccountID, amount->issue())) @@ -339,7 +345,7 @@ AMMDeposit::applyGuts(Sandbox& sb) auto const lpTokensDeposit = ctx_.tx[~sfLPTokenOut]; auto ammSle = sb.peek(keylet::amm(ctx_.tx[sfAsset], ctx_.tx[sfAsset2])); if (!ammSle) - return {tecINTERNAL, false}; + return {tecINTERNAL, false}; // LCOV_EXCL_LINE auto const ammAccountID = (*ammSle)[sfAccount]; auto const expected = ammHolds( @@ -350,7 +356,7 @@ AMMDeposit::applyGuts(Sandbox& sb) FreezeHandling::fhZERO_IF_FROZEN, ctx_.journal); if (!expected) - return {expected.error(), false}; + return {expected.error(), false}; // LCOV_EXCL_LINE auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; auto const tfee = (lptAMMBalance == beast::zero) ? ctx_.tx[~sfTradingFee].value_or(0) @@ -421,8 +427,10 @@ AMMDeposit::applyGuts(Sandbox& sb) lptAMMBalance.issue(), tfee); // should not happen. + // LCOV_EXCL_START JLOG(j_.error()) << "AMM Deposit: invalid options."; return std::make_pair(tecINTERNAL, STAmount{}); + // LCOV_EXCL_STOP }(); if (result == tesSUCCESS) @@ -621,10 +629,12 @@ AMMDeposit::equalDepositTokens( } catch (std::exception const& e) { + // LCOV_EXCL_START JLOG(j_.error()) << "AMMDeposit::equalDepositTokens exception " << e.what(); + return {tecINTERNAL, STAmount{}}; + // LCOV_EXCL_STOP } - return {tecINTERNAL, STAmount{}}; } /** Proportional deposit of pool assets with the constraints on the maximum diff --git a/src/ripple/app/tx/impl/AMMWithdraw.cpp b/src/ripple/app/tx/impl/AMMWithdraw.cpp index 58fc6b549ce..839c3b433aa 100644 --- a/src/ripple/app/tx/impl/AMMWithdraw.cpp +++ b/src/ripple/app/tx/impl/AMMWithdraw.cpp @@ -201,7 +201,7 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) { JLOG(ctx.j.debug()) << "AMM Withdraw: reserves or tokens balance is zero."; - return tecINTERNAL; + return tecINTERNAL; // LCOV_EXCL_LINE } auto const ammAccountID = ammSle->getAccountID(sfAccount); @@ -300,11 +300,11 @@ AMMWithdraw::applyGuts(Sandbox& sb) auto const ePrice = ctx_.tx[~sfEPrice]; auto ammSle = sb.peek(keylet::amm(ctx_.tx[sfAsset], ctx_.tx[sfAsset2])); if (!ammSle) - return {tecINTERNAL, false}; + return {tecINTERNAL, false}; // LCOV_EXCL_LINE auto const ammAccountID = (*ammSle)[sfAccount]; auto const accountSle = sb.read(keylet::account(ammAccountID)); if (!accountSle) - return {tecINTERNAL, false}; + return {tecINTERNAL, false}; // LCOV_EXCL_LINE auto const lpTokens = ammLPHolds(ctx_.view(), *ammSle, ctx_.tx[sfAccount], ctx_.journal); auto const lpTokensWithdraw = @@ -372,8 +372,10 @@ AMMWithdraw::applyGuts(Sandbox& sb) *lpTokensWithdraw, tfee); // should not happen. + // LCOV_EXCL_START JLOG(j_.error()) << "AMM Withdraw: invalid options."; return std::make_pair(tecINTERNAL, STAmount{}); + // LCOV_EXCL_STOP }(); if (result != tesSUCCESS) @@ -431,7 +433,7 @@ AMMWithdraw::withdraw( auto const ammSle = ctx_.view().read(keylet::amm(ctx_.tx[sfAsset], ctx_.tx[sfAsset2])); if (!ammSle) - return {tecINTERNAL, STAmount{}}; + return {tecINTERNAL, STAmount{}}; // LCOV_EXCL_LINE auto const lpTokens = ammLPHolds(view, *ammSle, account_, ctx_.journal); auto const expected = ammHolds( view, @@ -612,12 +614,14 @@ AMMWithdraw::equalWithdrawTokens( lpTokensWithdraw, tfee); } + // LCOV_EXCL_START catch (std::exception const& e) { JLOG(j_.error()) << "AMMWithdraw::equalWithdrawTokens exception " << e.what(); } return {tecINTERNAL, STAmount{}}; + // LCOV_EXCL_STOP } /** All assets withdrawal with the constraints on the maximum amount diff --git a/src/ripple/protocol/STIssue.h b/src/ripple/protocol/STIssue.h index 38ca136e017..223798a8911 100644 --- a/src/ripple/protocol/STIssue.h +++ b/src/ripple/protocol/STIssue.h @@ -56,9 +56,6 @@ class STIssue final : public STBase, CountedObject SerializedTypeID getSType() const override; - std::string - getText() const override; - Json::Value getJson(JsonOptions) const override; void @@ -71,9 +68,6 @@ class STIssue final : public STBase, CountedObject isDefault() const override; private: - static std::unique_ptr - construct(SerialIter&, SField const& name); - STBase* copy(std::size_t n, void* buf) const override; STBase* diff --git a/src/ripple/protocol/impl/STIssue.cpp b/src/ripple/protocol/impl/STIssue.cpp index 878a5b4c71b..356af438659 100644 --- a/src/ripple/protocol/impl/STIssue.cpp +++ b/src/ripple/protocol/impl/STIssue.cpp @@ -65,12 +65,6 @@ STIssue::getSType() const return STI_ISSUE; } -std::string -STIssue::getText() const -{ - return issue_.getText(); -} - Json::Value STIssue::getJson(JsonOptions) const { return to_json(issue_); @@ -97,12 +91,6 @@ STIssue::isDefault() const return issue_ == xrpIssue(); } -std::unique_ptr -STIssue::construct(SerialIter& sit, SField const& name) -{ - return std::make_unique(sit, name); -} - STBase* STIssue::copy(std::size_t n, void* buf) const { diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 91e8ffa53dc..449a795c228 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -42,6 +42,9 @@ namespace ripple { namespace test { +/** + * Tests of AMM that use offers too. + */ struct AMMExtended_test : public jtx::AMMTest { private: @@ -3614,6 +3617,8 @@ struct AMMExtended_test : public jtx::AMMTest int const signerListOwners{features[featureMultiSignReserve] ? 2 : 5}; env.require(owners(alice, signerListOwners + 0)); + const msig ms{becky, bogie}; + // Multisign all AMM transactions AMM ammAlice( env, @@ -3625,7 +3630,7 @@ struct AMMExtended_test : public jtx::AMMTest ammCrtFee(env).drops(), std::nullopt, std::nullopt, - msig(becky, bogie), + ms, ter(tesSUCCESS)); BEAST_EXPECT(ammAlice.expectBalances( XRP(10'000), USD(10'000), ammAlice.tokens())); @@ -3641,7 +3646,7 @@ struct AMMExtended_test : public jtx::AMMTest ammAlice.vote({}, 1'000); BEAST_EXPECT(ammAlice.expectTradingFee(1'000)); - ammAlice.bid(alice, 100); + env(ammAlice.bid({.account = alice, .bidMin = 100}), ms).close(); BEAST_EXPECT(ammAlice.expectAuctionSlot(100, 0, IOUAmount{4'000})); // 4000 tokens burnt BEAST_EXPECT(ammAlice.expectBalances( diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 12c0cc64cc2..26a4ba1b4c7 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -37,6 +38,10 @@ namespace ripple { namespace test { +/** + * Basic tests of AMM that do not use offers. + * Tests incorporating offers are in `AMMExtended_test`. + */ struct AMM_test : public jtx::AMMTest { private: @@ -81,10 +86,8 @@ struct AMM_test : public jtx::AMMTest env.fund(XRP(30'000), gw, alice); env.close(); env(fset(gw, asfRequireAuth)); - env.close(); - env.trust(USD(30'000), alice); - env.close(); - env(trust(gw, alice["USD"](30'000)), txflags(tfSetfAuth)); + env(trust(alice, gw["USD"](30'000), 0)); + env(trust(gw, alice["USD"](0), tfSetfAuth)); env.close(); env(pay(gw, alice, USD(10'000))); env.close(); @@ -117,6 +120,14 @@ struct AMM_test : public jtx::AMMTest }, std::nullopt, 1'000); + + // Make sure asset comparison works. + BEAST_EXPECT( + STIssue(sfAsset, STAmount(XRP(2'000)).issue()) == + STIssue(sfAsset, STAmount(XRP(2'000)).issue())); + BEAST_EXPECT( + STIssue(sfAsset, STAmount(XRP(2'000)).issue()) != + STIssue(sfAsset, STAmount({USD(2'000)}).issue())); } void @@ -599,7 +610,12 @@ struct AMM_test : public jtx::AMMTest USD(100), STAmount{USD, 1, -1}, std::nullopt}, - }; + {tfTwoAssetIfEmpty | tfLPToken, + std::nullopt, + XRP(100), + USD(100), + STAmount{USD, 1, -1}, + std::nullopt}}; for (auto const& it : invalidOptions) { ammAlice.deposit( @@ -615,6 +631,19 @@ struct AMM_test : public jtx::AMMTest ter(temMALFORMED)); } + { + // bad preflight1 + Json::Value jv = Json::objectValue; + jv[jss::Account] = alice.human(); + jv[jss::TransactionType] = jss::AMMDeposit; + jv[jss::Asset] = + STIssue(sfAsset, XRP).getJson(JsonOptions::none); + jv[jss::Asset2] = + STIssue(sfAsset, USD).getJson(JsonOptions::none); + jv[jss::Fee] = "-1"; + env(jv, ter(temBAD_FEE)); + } + // Invalid tokens ammAlice.deposit( alice, 0, std::nullopt, std::nullopt, ter(temBAD_AMM_TOKENS)); @@ -625,6 +654,33 @@ struct AMM_test : public jtx::AMMTest std::nullopt, ter(temBAD_AMM_TOKENS)); + { + Json::Value jv = Json::objectValue; + jv[jss::Account] = alice.human(); + jv[jss::TransactionType] = jss::AMMDeposit; + jv[jss::Asset] = + STIssue(sfAsset, XRP).getJson(JsonOptions::none); + jv[jss::Asset2] = + STIssue(sfAsset, USD).getJson(JsonOptions::none); + jv[jss::LPTokenOut] = + USD(100).value().getJson(JsonOptions::none); + jv[jss::Flags] = tfLPToken; + env(jv, ter(temBAD_AMM_TOKENS)); + } + + // Invalid trading fee + ammAlice.deposit( + carol, + std::nullopt, + XRP(200), + USD(200), + std::nullopt, + tfTwoAssetIfEmpty, + std::nullopt, + std::nullopt, + 10'000, + ter(temBAD_FEE)); + // Invalid tokens - bogus currency { auto const iss1 = Issue{Currency(0xabc), gw.id()}; @@ -821,6 +877,13 @@ struct AMM_test : public jtx::AMMTest 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)); }); // Individually frozen (AMM) account @@ -858,6 +921,50 @@ struct AMM_test : public jtx::AMMTest ter(tecFROZEN)); }); + // Individually frozen (AMM) account with IOU/IOU AMM + testAMM( + [&](AMM& ammAlice, Env& env) { + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env(trust(gw, carol["BTC"](0), tfSetFreeze)); + env.close(); + 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(); + // Cannot deposit non-frozen token + ammAlice.deposit( + carol, + 1'000'000, + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + ammAlice.deposit( + carol, + USD(100), + BTC(0.01), + std::nullopt, + std::nullopt, + ter(tecFROZEN)); + }, + {{USD(20'000), BTC(0.5)}}); + // Insufficient XRP balance testAMM([&](AMM& ammAlice, Env& env) { env.fund(XRP(1'000), bob); @@ -947,6 +1054,15 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, ter(tecINSUF_RESERVE_LINE)); + + env(offer(carol, XRP(100), USD(103))); + ammAlice.deposit( + carol, + USD(100), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecINSUF_RESERVE_LINE)); } // Insufficient reserve, IOU/IOU @@ -1359,6 +1475,48 @@ struct AMM_test : public jtx::AMMTest using namespace jtx; + testAMM( + [&](AMM& ammAlice, Env& env) { + WithdrawArg args{ + .asset1Out = XRP(100), + .err = ter(tecAMM_BALANCE), + }; + ammAlice.withdraw(args); + }, + {{XRP(99), USD(99)}}); + + testAMM( + [&](AMM& ammAlice, Env& env) { + WithdrawArg args{ + .asset1Out = USD(100), + .err = ter(tecAMM_BALANCE), + }; + ammAlice.withdraw(args); + }, + {{XRP(99), USD(99)}}); + + { + Env env{*this}; + env.fund(XRP(30'000), gw, alice, bob); + env.close(); + env(fset(gw, asfRequireAuth)); + env(trust(alice, gw["USD"](30'000), 0)); + env(trust(gw, alice["USD"](0), tfSetfAuth)); + // Bob trusts Gateway to owe him USD... + env(trust(bob, gw["USD"](30'000), 0)); + // ...but Gateway does not authorize Bob to hold its USD. + env.close(); + env(pay(gw, alice, USD(10'000))); + env.close(); + AMM ammAlice(env, alice, XRP(10'000), USD(10'000)); + WithdrawArg args{ + .account = bob, + .asset1Out = USD(100), + .err = ter(tecNO_AUTH), + }; + ammAlice.withdraw(args); + } + testAMM([&](AMM& ammAlice, Env& env) { // Invalid flags ammAlice.withdraw( @@ -2294,142 +2452,156 @@ struct AMM_test : public jtx::AMMTest using namespace jtx; using namespace std::chrono; + // burn all the LPTokens through a AMMBid transaction + { + Env env(*this); + fund(env, gw, {alice}, XRP(2'000), {USD(2'000)}); + AMM amm(env, gw, XRP(1'000), USD(1'000), false, 1'000); + + // auction slot is owned by the creator of the AMM i.e. gw + BEAST_EXPECT(amm.expectAuctionSlot(100, 0, IOUAmount{0})); + + // gw attempts to burn all her LPTokens through a bid transaction + // this transaction fails because AMMBid transaction can not burn + // all the outstanding LPTokens + env(amm.bid({ + .account = gw, + .bidMin = 1'000'000, + }), + ter(tecAMM_INVALID_TOKENS)); + } + + // burn all the LPTokens through a AMMBid transaction + { + Env env(*this); + fund(env, gw, {alice}, XRP(2'000), {USD(2'000)}); + AMM amm(env, gw, XRP(1'000), USD(1'000), false, 1'000); + + // auction slot is owned by the creator of the AMM i.e. gw + BEAST_EXPECT(amm.expectAuctionSlot(100, 0, IOUAmount{0})); + + // gw burns all but one of its LPTokens through a bid transaction + // this transaction suceeds because the bid price is less than + // the total outstanding LPToken balance + env(amm.bid({ + .account = gw, + .bidMin = STAmount{amm.lptIssue(), UINT64_C(999'999)}, + }), + ter(tesSUCCESS)) + .close(); + + // gw must own the auction slot + BEAST_EXPECT(amm.expectAuctionSlot(100, 0, IOUAmount{999'999})); + + // 999'999 tokens are burned, only 1 LPToken is owned by gw + BEAST_EXPECT( + amm.expectBalances(XRP(1'000), USD(1'000), IOUAmount{1})); + + // gw owns only 1 LPToken in its balance + BEAST_EXPECT(Number{amm.getLPTokensBalance(gw)} == 1); + + // gw attempts to burn the last of its LPTokens in an AMMBid + // transaction. This transaction fails because it would burn all + // the remaining LPTokens + env(amm.bid({ + .account = gw, + .bidMin = 1, + }), + ter(tecAMM_INVALID_TOKENS)); + } + testAMM([&](AMM& ammAlice, Env& env) { // Invalid flags - ammAlice.bid( - carol, - 0, - std::nullopt, - {}, - tfWithdrawAll, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMin = 0, + .flags = tfWithdrawAll, + }), ter(temINVALID_FLAG)); ammAlice.deposit(carol, 1'000'000); // Invalid Bid price <= 0 for (auto bid : {0, -100}) { - ammAlice.bid( - carol, - bid, - std::nullopt, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMin = bid, + }), ter(temBAD_AMOUNT)); - ammAlice.bid( - carol, - std::nullopt, - bid, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMax = bid, + }), ter(temBAD_AMOUNT)); } // Invlaid Min/Max combination - ammAlice.bid( - carol, - 200, - 100, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMin = 200, + .bidMax = 100, + }), ter(tecAMM_INVALID_TOKENS)); // Invalid Account Account bad("bad"); env.memoize(bad); - ammAlice.bid( - bad, - std::nullopt, - 100, - {}, - std::nullopt, + env(ammAlice.bid({ + .account = bad, + .bidMax = 100, + }), seq(1), - std::nullopt, ter(terNO_ACCOUNT)); // Account is not LP Account const dan("dan"); env.fund(XRP(1'000), dan); - ammAlice.bid( - dan, - 100, - std::nullopt, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = dan, + .bidMin = 100, + }), ter(tecAMM_INVALID_TOKENS)); - ammAlice.bid( - dan, - std::nullopt, - std::nullopt, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = dan, + }), ter(tecAMM_INVALID_TOKENS)); // Auth account is invalid. - ammAlice.bid( - carol, - 100, - std::nullopt, - {bob}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMin = 100, + .authAccounts = {bob}, + }), ter(terNO_ACCOUNT)); // Invalid Assets - ammAlice.bid( - alice, - std::nullopt, - 100, - {}, - std::nullopt, - std::nullopt, - {{USD, GBP}}, + env(ammAlice.bid({ + .account = alice, + .bidMax = 100, + .assets = {{USD, GBP}}, + }), ter(terNO_AMM)); // Invalid Min/Max issue - ammAlice.bid( - alice, - std::nullopt, - STAmount{USD, 100}, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = alice, + .bidMax = STAmount{USD, 100}, + }), ter(temBAD_AMM_TOKENS)); - ammAlice.bid( - alice, - STAmount{USD, 100}, - std::nullopt, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = alice, + .bidMin = STAmount{USD, 100}, + }), ter(temBAD_AMM_TOKENS)); }); // Invalid AMM testAMM([&](AMM& ammAlice, Env& env) { ammAlice.withdrawAll(alice); - ammAlice.bid( - alice, - std::nullopt, - 100, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = alice, + .bidMax = 100, + }), ter(terNO_AMM)); }); @@ -2442,14 +2614,11 @@ struct AMM_test : public jtx::AMMTest env.fund(XRP(1'000), bob, ed, bill, scott, james); env.close(); ammAlice.deposit(carol, 1'000'000); - ammAlice.bid( - carol, - 100, - std::nullopt, - {bob, ed, bill, scott, james}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMin = 100, + .authAccounts = {bob, ed, bill, scott, james}, + }), ter(temMALFORMED)); }); @@ -2458,35 +2627,25 @@ struct AMM_test : public jtx::AMMTest fund(env, gw, {bob}, XRP(1'000), {USD(100)}, Fund::Acct); ammAlice.deposit(carol, 1'000'000); ammAlice.deposit(bob, 10); - ammAlice.bid( - carol, - 1'000'001, - std::nullopt, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMin = 1'000'001, + }), ter(tecAMM_INVALID_TOKENS)); - ammAlice.bid( - carol, - std::nullopt, - 1'000'001, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMax = 1'000'001, + }), ter(tecAMM_INVALID_TOKENS)); - ammAlice.bid(carol, 1'000); + env(ammAlice.bid({ + .account = carol, + .bidMin = 1'000, + })); BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount{1'000})); // Slot purchase price is more than 1000 but bob only has 10 tokens - ammAlice.bid( - bob, - std::nullopt, - std::nullopt, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = bob, + }), ter(tecAMM_INVALID_TOKENS)); }); @@ -2500,17 +2659,13 @@ struct AMM_test : public jtx::AMMTest env.trust(STAmount{lpIssue, 50}, bob); env(pay(gw, alice, STAmount{lpIssue, 100})); env(pay(gw, bob, STAmount{lpIssue, 50})); - amm.bid(alice, 100); + env(amm.bid({.account = alice, .bidMin = 100})); // Alice doesn't have any more tokens, but // she still owns the slot. - amm.bid( - bob, - std::nullopt, - 50, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(amm.bid({ + .account = bob, + .bidMax = 50, + }), ter(tecAMM_FAILED)); } } @@ -2527,7 +2682,7 @@ struct AMM_test : public jtx::AMMTest // Bid 110 tokens. Pay bidMin. testAMM([&](AMM& ammAlice, Env& env) { ammAlice.deposit(carol, 1'000'000); - ammAlice.bid(carol, 110); + env(ammAlice.bid({.account = carol, .bidMin = 110})); BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount{110})); // 110 tokens are burned. BEAST_EXPECT(ammAlice.expectBalances( @@ -2538,12 +2693,12 @@ struct AMM_test : public jtx::AMMTest testAMM([&](AMM& ammAlice, Env& env) { ammAlice.deposit(carol, 1'000'000); // Bid exactly 110. Pay 110 because the pay price is < 110. - ammAlice.bid(carol, 110, 110); + env(ammAlice.bid({.account = carol, .bidMin = 110, .bidMax = 110})); BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount{110})); BEAST_EXPECT(ammAlice.expectBalances( XRP(11'000), USD(11'000), IOUAmount{10'999'890})); // Bid exactly 180-200. Pay 180 because the pay price is < 180. - ammAlice.bid(alice, 180, 200); + env(ammAlice.bid({.account = alice, .bidMin = 180, .bidMax = 200})); BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount{180})); BEAST_EXPECT(ammAlice.expectBalances( XRP(11'000), USD(11'000), IOUAmount{10'999'814'5, -1})); @@ -2553,43 +2708,36 @@ struct AMM_test : public jtx::AMMTest testAMM([&](AMM& ammAlice, Env& env) { ammAlice.deposit(carol, 1'000'000); // Bid, pay bidMin. - ammAlice.bid(carol, 110); + env(ammAlice.bid({.account = carol, .bidMin = 110})); BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount{110})); fund(env, gw, {bob}, {USD(10'000)}, Fund::Acct); ammAlice.deposit(bob, 1'000'000); // Bid, pay the computed price. - ammAlice.bid(bob); + env(ammAlice.bid({.account = bob})); BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount(1155, -1))); // Bid bidMax fails because the computed price is higher. - ammAlice.bid( - carol, - std::nullopt, - 120, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMax = 120, + }), ter(tecAMM_FAILED)); // Bid MaxSlotPrice succeeds - pay computed price - ammAlice.bid(carol, std::nullopt, 600); + env(ammAlice.bid({.account = carol, .bidMax = 600})); BEAST_EXPECT( ammAlice.expectAuctionSlot(0, 0, IOUAmount{121'275, -3})); // Bid Min/MaxSlotPrice fails because the computed price is not in // range - ammAlice.bid( - carol, - 10, - 100, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(ammAlice.bid({ + .account = carol, + .bidMin = 10, + .bidMax = 100, + }), ter(tecAMM_FAILED)); // Bid Min/MaxSlotPrice succeeds - pay computed price - ammAlice.bid(carol, 100, 600); + env(ammAlice.bid({.account = carol, .bidMin = 100, .bidMax = 600})); BEAST_EXPECT( ammAlice.expectAuctionSlot(0, 0, IOUAmount{127'33875, -5})); }); @@ -2604,23 +2752,23 @@ struct AMM_test : public jtx::AMMTest XRP(12'000), USD(12'000), IOUAmount{12'000'000, 0})); // Initial state. Pay bidMin. - ammAlice.bid(carol, 110); + env(ammAlice.bid({.account = carol, .bidMin = 110})).close(); BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount{110})); // 1st Interval after close, price for 0th interval. - ammAlice.bid(bob); + env(ammAlice.bid({.account = bob})); env.close(seconds(AUCTION_SLOT_INTERVAL_DURATION + 1)); BEAST_EXPECT( ammAlice.expectAuctionSlot(0, 1, IOUAmount{1'155, -1})); // 10th Interval after close, price for 1st interval. - ammAlice.bid(carol); + env(ammAlice.bid({.account = carol})); env.close(seconds(10 * AUCTION_SLOT_INTERVAL_DURATION + 1)); BEAST_EXPECT( ammAlice.expectAuctionSlot(0, 10, IOUAmount{121'275, -3})); // 20th Interval (expired) after close, price for 10th interval. - ammAlice.bid(bob); + env(ammAlice.bid({.account = bob})); env.close(seconds( AUCTION_SLOT_TIME_INTERVALS * AUCTION_SLOT_INTERVAL_DURATION + 1)); @@ -2628,7 +2776,7 @@ struct AMM_test : public jtx::AMMTest 0, std::nullopt, IOUAmount{127'33875, -5})); // 0 Interval. - ammAlice.bid(carol, 110); + env(ammAlice.bid({.account = carol, .bidMin = 110})).close(); BEAST_EXPECT( ammAlice.expectAuctionSlot(0, std::nullopt, IOUAmount{110})); // ~321.09 tokens burnt on bidding fees. @@ -2650,7 +2798,11 @@ struct AMM_test : public jtx::AMMTest ammAlice.deposit(carol, 500'000); ammAlice.deposit(dan, 500'000); auto ammTokens = ammAlice.getLPTokensBalance(); - ammAlice.bid(carol, 120, std::nullopt, {bob, ed}); + env(ammAlice.bid({ + .account = carol, + .bidMin = 120, + .authAccounts = {bob, ed}, + })); auto const slotPrice = IOUAmount{5'200}; ammTokens -= slotPrice; BEAST_EXPECT(ammAlice.expectAuctionSlot(100, 0, slotPrice)); @@ -2760,10 +2912,10 @@ struct AMM_test : public jtx::AMMTest 1'000); // Bid tiny amount - testAMM([&](AMM& ammAlice, Env&) { + testAMM([&](AMM& ammAlice, Env& env) { // Bid a tiny amount auto const tiny = Number{STAmount::cMinValue, STAmount::cMinOffset}; - ammAlice.bid(alice, IOUAmount{tiny}); + env(ammAlice.bid({.account = alice, .bidMin = IOUAmount{tiny}})); // Auction slot purchase price is equal to the tiny amount // since the minSlotPrice is 0 with no trading fee. BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount{tiny})); @@ -2771,8 +2923,10 @@ struct AMM_test : public jtx::AMMTest BEAST_EXPECT(ammAlice.expectBalances( XRP(10'000), USD(10'000), ammAlice.tokens())); // Bid the tiny amount - ammAlice.bid( - alice, IOUAmount{STAmount::cMinValue, STAmount::cMinOffset}); + env(ammAlice.bid({ + .account = alice, + .bidMin = IOUAmount{STAmount::cMinValue, STAmount::cMinOffset}, + })); // Pay slightly higher price BEAST_EXPECT(ammAlice.expectAuctionSlot( 0, 0, IOUAmount{tiny * Number{105, -2}})); @@ -2783,14 +2937,22 @@ struct AMM_test : public jtx::AMMTest // Reset auth account testAMM([&](AMM& ammAlice, Env& env) { - ammAlice.bid(alice, IOUAmount{100}, std::nullopt, {carol}); + env(ammAlice.bid({ + .account = alice, + .bidMin = IOUAmount{100}, + .authAccounts = {carol}, + })); BEAST_EXPECT(ammAlice.expectAuctionSlot({carol})); - ammAlice.bid(alice, IOUAmount{100}); + env(ammAlice.bid({.account = alice, .bidMin = IOUAmount{100}})); BEAST_EXPECT(ammAlice.expectAuctionSlot({})); Account bob("bob"); Account dan("dan"); fund(env, {bob, dan}, XRP(1'000)); - ammAlice.bid(alice, IOUAmount{100}, std::nullopt, {bob, dan}); + env(ammAlice.bid({ + .account = alice, + .bidMin = IOUAmount{100}, + .authAccounts = {bob, dan}, + })); BEAST_EXPECT(ammAlice.expectAuctionSlot({bob, dan})); }); @@ -2805,7 +2967,7 @@ struct AMM_test : public jtx::AMMTest env(pay(gw, alice, STAmount{lpIssue, 500})); env(pay(gw, bob, STAmount{lpIssue, 50})); // Alice doesn't have anymore lp tokens - amm.bid(alice, 500); + env(amm.bid({.account = alice, .bidMin = 500})); BEAST_EXPECT(amm.expectAuctionSlot(100, 0, IOUAmount{500})); BEAST_EXPECT(expectLine(env, alice, STAmount{lpIssue, 0})); // But trades with the discounted fee since she still owns the slot. @@ -2822,6 +2984,57 @@ struct AMM_test : public jtx::AMMTest STAmount{USD, UINT64_C(1'010'10090898081), -11}, IOUAmount{1'004'487'562112089, -9})); } + + // preflight tests + { + Env env(*this); + fund(env, gw, {alice, bob}, XRP(2'000), {USD(2'000)}); + AMM amm(env, gw, XRP(1'000), USD(1'010), false, 1'000); + Json::Value tx = amm.bid({.account = alice, .bidMin = 500}); + + { + auto jtx = env.jt(tx, seq(1), fee(10)); + env.app().config().features.erase(featureAMM); + PreflightContext pfctx( + env.app(), + *jtx.stx, + env.current()->rules(), + tapNONE, + env.journal); + auto pf = AMMBid::preflight(pfctx); + BEAST_EXPECT(pf == temDISABLED); + env.app().config().features.insert(featureAMM); + } + + { + auto jtx = env.jt(tx, seq(1), fee(10)); + jtx.jv["TxnSignature"] = "deadbeef"; + jtx.stx = env.ust(jtx); + PreflightContext pfctx( + env.app(), + *jtx.stx, + env.current()->rules(), + tapNONE, + env.journal); + auto pf = AMMBid::preflight(pfctx); + BEAST_EXPECT(pf != tesSUCCESS); + } + + { + auto jtx = env.jt(tx, seq(1), fee(10)); + jtx.jv["Asset2"]["currency"] = "XRP"; + jtx.jv["Asset2"].removeMember("issuer"); + jtx.stx = env.ust(jtx); + PreflightContext pfctx( + env.app(), + *jtx.stx, + env.current()->rules(), + tapNONE, + env.journal); + auto pf = AMMBid::preflight(pfctx); + BEAST_EXPECT(pf == temBAD_AMM_TOKENS); + } + } } void @@ -3601,7 +3814,7 @@ struct AMM_test : public jtx::AMMTest ammAlice.vote(carol, 0); BEAST_EXPECT(ammAlice.expectTradingFee(0)); // Carol bids - ammAlice.bid(carol, 100); + env(ammAlice.bid({.account = carol, .bidMin = 100})); BEAST_EXPECT(ammAlice.expectLPTokens(carol, IOUAmount{4'999'900})); BEAST_EXPECT(ammAlice.expectAuctionSlot(0, 0, IOUAmount{100})); BEAST_EXPECT(accountBalance(env, carol) == "22499999960"); @@ -3693,6 +3906,15 @@ struct AMM_test : public jtx::AMMTest Env env{*this, feature}; fund(env, gw, {alice}, {USD(1'000)}, Fund::All); AMM amm(env, alice, XRP(1'000), USD(1'000), ter(temDISABLED)); + + env(amm.bid({.bidMax = 1000}), ter(temMALFORMED)); + env(amm.bid({}), ter(temDISABLED)); + amm.vote(VoteArg{.tfee = 100, .err = ter(temDISABLED)}); + amm.withdraw(WithdrawArg{.tokens = 100, .err = ter(temMALFORMED)}); + amm.withdraw(WithdrawArg{.err = ter(temDISABLED)}); + amm.deposit( + DepositArg{.asset1In = USD(100), .err = ter(temDISABLED)}); + amm.ammDelete(alice, ter(temDISABLED)); } } @@ -4312,14 +4534,10 @@ struct AMM_test : public jtx::AMMTest // Bid,Vote,Deposit,Withdraw,SetTrust failing with // tecAMM_EMPTY. Deposit succeeds with tfTwoAssetIfEmpty option. - amm.bid( - alice, - 1000, - std::nullopt, - {}, - std::nullopt, - std::nullopt, - std::nullopt, + env(amm.bid({ + .account = alice, + .bidMin = 1000, + }), ter(tecAMM_EMPTY)); amm.vote( std::nullopt, @@ -4391,6 +4609,9 @@ struct AMM_test : public jtx::AMMTest amm.ammDelete(alice); BEAST_EXPECT(!amm.ammExists()); BEAST_EXPECT(!env.le(keylet::ownerDir(amm.ammAccount()))); + + // Try redundant delete + amm.ammDelete(alice, ter(terNO_AMM)); } } @@ -4889,6 +5110,65 @@ struct AMM_test : public jtx::AMMTest false); } + void + testMalformed() + { + using namespace jtx; + + testAMM([&](AMM& ammAlice, Env& env) { + WithdrawArg args{ + .flags = tfSingleAsset, + .err = ter(temMALFORMED), + }; + ammAlice.withdraw(args); + }); + + testAMM([&](AMM& ammAlice, Env& env) { + WithdrawArg args{ + .flags = tfOneAssetLPToken, + .err = ter(temMALFORMED), + }; + ammAlice.withdraw(args); + }); + + testAMM([&](AMM& ammAlice, Env& env) { + WithdrawArg args{ + .flags = tfLimitLPToken, + .err = ter(temMALFORMED), + }; + ammAlice.withdraw(args); + }); + + testAMM([&](AMM& ammAlice, Env& env) { + WithdrawArg args{ + .asset1Out = XRP(100), + .asset2Out = XRP(100), + .err = ter(temBAD_AMM_TOKENS), + }; + ammAlice.withdraw(args); + }); + + testAMM([&](AMM& ammAlice, Env& env) { + WithdrawArg args{ + .asset1Out = XRP(100), + .asset2Out = BAD(100), + .err = ter(temBAD_CURRENCY), + }; + ammAlice.withdraw(args); + }); + + testAMM([&](AMM& ammAlice, Env& env) { + Json::Value jv; + jv[jss::TransactionType] = jss::AMMWithdraw; + jv[jss::Flags] = tfLimitLPToken; + jv[jss::Account] = alice.human(); + ammAlice.setTokens(jv); + XRP(100).value().setJson(jv[jss::Amount]); + USD(100).value().setJson(jv[jss::EPrice]); + env(jv, ter(temBAD_AMM_TOKENS)); + }); + } + void testFixOverflowOffer() { @@ -5164,7 +5444,7 @@ struct AMM_test : public jtx::AMMTest } void - testCore() + testAll() { testInvalidInstance(); testInstanceCreate(); @@ -5190,13 +5470,14 @@ struct AMM_test : public jtx::AMMTest testAMMID(); testSelection(); testFixDefaultInnerObj(); + testMalformed(); testFixOverflowOffer(); } void run() override { - testCore(); + testAll(); } }; diff --git a/src/test/jtx/AMM.h b/src/test/jtx/AMM.h index 4c6e8d78a4e..b98065a077e 100644 --- a/src/test/jtx/AMM.h +++ b/src/test/jtx/AMM.h @@ -113,9 +113,7 @@ struct BidArg std::optional> bidMax = std::nullopt; std::vector authAccounts = {}; std::optional flags = std::nullopt; - std::optional seq = std::nullopt; std::optional> assets = std::nullopt; - std::optional err = std::nullopt; }; /** Convenience class to test AMM functionality. @@ -317,19 +315,7 @@ class AMM void vote(VoteArg const& arg); - void - bid(std::optional const& account, - std::optional> const& bidMin = - std::nullopt, - std::optional> const& bidMax = - std::nullopt, - std::vector const& authAccounts = {}, - std::optional const& flags = std::nullopt, - std::optional const& seq = std::nullopt, - std::optional> const& assets = std::nullopt, - std::optional const& ter = std::nullopt); - - void + Json::Value bid(BidArg const& arg); AccountID const& @@ -391,12 +377,12 @@ class AMM return ammID_; } -private: void setTokens( Json::Value& jv, std::optional> const& assets = std::nullopt); +private: AccountID create( std::uint32_t tfee = 0, diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index 72cac29040a..69640179b11 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -521,17 +521,18 @@ class Env /** Apply funclets and submit. */ /** @{ */ template - void + Env& apply(JsonValue&& jv, FN const&... fN) { submit(jt(std::forward(jv), fN...)); + return *this; } template - void + Env& operator()(JsonValue&& jv, FN const&... fN) { - apply(std::forward(jv), fN...); + return apply(std::forward(jv), fN...); } /** @} */ @@ -661,6 +662,13 @@ class Env } /** @} */ + /** Create a STTx from a JTx without sanitizing + Use to inject bogus values into test transactions by first + editing the JSON. + */ + std::shared_ptr + ust(JTx const& jt); + protected: int trace_ = 0; TestStopwatch stopwatch_; diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index 2d5ce90d306..eddb9691693 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -656,16 +656,8 @@ AMM::vote(VoteArg const& arg) return vote(arg.account, arg.tfee, arg.flags, arg.seq, arg.assets, arg.err); } -void -AMM::bid( - std::optional const& account, - std::optional> const& bidMin, - std::optional> const& bidMax, - std::vector const& authAccounts, - std::optional const& flags, - std::optional const& seq, - std::optional> const& assets, - std::optional const& ter) +Json::Value +AMM::bid(BidArg const& arg) { if (auto const amm = env_.current()->read(keylet::amm(asset1_.issue(), asset2_.issue()))) @@ -684,8 +676,9 @@ AMM::bid( bidMax_ = std::nullopt; Json::Value jv; - jv[jss::Account] = account ? account->human() : creatorAccount_.human(); - setTokens(jv, assets); + jv[jss::Account] = + arg.account ? arg.account->human() : creatorAccount_.human(); + setTokens(jv, arg.assets); auto getBid = [&](auto const& bid) { if (std::holds_alternative(bid)) return STAmount{lptIssue_, std::get(bid)}; @@ -694,22 +687,22 @@ AMM::bid( else return std::get(bid); }; - if (bidMin) + if (arg.bidMin) { - STAmount saTokens = getBid(*bidMin); + STAmount saTokens = getBid(*arg.bidMin); saTokens.setJson(jv[jss::BidMin]); bidMin_ = saTokens.iou(); } - if (bidMax) + if (arg.bidMax) { - STAmount saTokens = getBid(*bidMax); + STAmount saTokens = getBid(*arg.bidMax); saTokens.setJson(jv[jss::BidMax]); bidMax_ = saTokens.iou(); } - if (authAccounts.size() > 0) + if (arg.authAccounts.size() > 0) { Json::Value accounts(Json::arrayValue); - for (auto const& account : authAccounts) + for (auto const& account : arg.authAccounts) { Json::Value acct; Json::Value authAcct; @@ -719,26 +712,12 @@ AMM::bid( } jv[jss::AuthAccounts] = accounts; } - if (flags) - jv[jss::Flags] = *flags; + if (arg.flags) + jv[jss::Flags] = *arg.flags; jv[jss::TransactionType] = jss::AMMBid; if (fee_ != 0) jv[jss::Fee] = std::to_string(fee_); - submit(jv, seq, ter); -} - -void -AMM::bid(BidArg const& arg) -{ - return bid( - arg.account, - arg.bidMin, - arg.bidMax, - arg.authAccounts, - arg.flags, - arg.seq, - arg.assets, - arg.err); + return jv; } void diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 6496f7df1d2..17d2d511846 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -472,6 +472,32 @@ Env::st(JTx const& jt) return nullptr; } +std::shared_ptr +Env::ust(JTx const& jt) +{ + // The parse must succeed, since we + // generated the JSON ourselves. + std::optional obj; + try + { + obj = jtx::parse(jt.jv); + } + catch (jtx::parse_error const&) + { + test.log << "Exception: parse_error\n" << pretty(jt.jv) << std::endl; + Rethrow(); + } + + try + { + return std::make_shared(std::move(*obj)); + } + catch (std::exception const&) + { + } + return nullptr; +} + Json::Value Env::do_rpc( unsigned apiVersion, diff --git a/src/test/jtx/multisign.h b/src/test/jtx/multisign.h index ab9996e415d..0617ea2fcbb 100644 --- a/src/test/jtx/multisign.h +++ b/src/test/jtx/multisign.h @@ -20,6 +20,7 @@ #ifndef RIPPLE_TEST_JTX_MULTISIGN_H_INCLUDED #define RIPPLE_TEST_JTX_MULTISIGN_H_INCLUDED +#include #include #include #include @@ -99,7 +100,9 @@ class msig msig(std::vector signers_); template - explicit msig(AccountType&& a0, Accounts&&... aN) + requires std::convertible_to explicit msig( + AccountType&& a0, + Accounts&&... aN) : msig{std::vector{ std::forward(a0), std::forward(aN)...}} diff --git a/src/test/rpc/AMMInfo_test.cpp b/src/test/rpc/AMMInfo_test.cpp index 1d9642539a1..cfcb672c032 100644 --- a/src/test/rpc/AMMInfo_test.cpp +++ b/src/test/rpc/AMMInfo_test.cpp @@ -130,7 +130,8 @@ class AMMInfo_test : public jtx::AMMTestBase Account ed("ed"); Account bill("bill"); env.fund(XRP(1000), bob, ed, bill); - ammAlice.bid(alice, 100, std::nullopt, {carol, bob, ed, bill}); + env(ammAlice.bid( + {.bidMin = 100, .authAccounts = {carol, bob, ed, bill}})); BEAST_EXPECT(ammAlice.expectAmmRpcInfo( XRP(80000), USD(80000),