From 10928c9a39eb940fa82d4d801d05933e3ff4ca16 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Fri, 5 Jan 2024 15:05:49 -0500 Subject: [PATCH 01/44] Authorization --- .../app/tx/impl/MPTokenIssuanceCreate.cpp | 2 + src/ripple/ledger/impl/View.cpp | 14 ++ src/test/app/MPToken_test.cpp | 143 +++++++++++++++++- 3 files changed, 154 insertions(+), 5 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp b/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp index dd8a90355fe..5a8abf34f19 100644 --- a/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp +++ b/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp @@ -55,6 +55,8 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) return temMALFORMED; } + // TODO: check if maximumAmount is within 63 bit range + return preflight2(ctx); } diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 9364d611c8a..dced3e8bf46 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1736,6 +1736,14 @@ rippleMPTCredit( auto const mptokenID = keylet::mptoken(mptID.key, uSenderID); if (auto sle = view.peek(mptokenID)) { + // Ensure sender is authorized if the MPT requires auth. + // It's possible that the issuer unauthorized a token holder + // who is holding some MPT + if (auto const sleIssuance = view.peek(mptID); + (sleIssuance->getFlags() & lsfMPTRequireAuth) && + !(sle->getFlags() & lsfMPTAuthorized)) + return tecNO_AUTH; + auto const amt = sle->getFieldU64(sfMPTAmount); auto const pay = saAmount.mpt().mpt(); if (amt >= pay) @@ -1773,6 +1781,12 @@ rippleMPTCredit( auto const mptokenID = keylet::mptoken(mptID.key, uReceiverID); if (auto sle = view.peek(mptokenID)) { + // Ensure receiver is authorized if the MPT requires auth + if (auto const sleIssuance = view.peek(mptID); + (sleIssuance->getFlags() & lsfMPTRequireAuth) && + !(sle->getFlags() & lsfMPTAuthorized)) + return tecNO_AUTH; + sle->setFieldU64( sfMPTAmount, sle->getFieldU64(sfMPTAmount) + saAmount.mpt().mpt()); diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index aae5d66f028..f5c3a492d4a 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -210,6 +210,7 @@ class MPToken_test : public beast::unit_test::suite BEAST_EXPECT(env.ownerCount(alice) == 0); auto const id = getMptID(alice.id(), env.seq(alice)); + auto const mpt = ripple::MPT(env.seq(alice), alice.id()); env(mpt::create(alice)); env.close(); @@ -219,7 +220,27 @@ class MPToken_test : public beast::unit_test::suite env(mpt::destroy(bob, id), ter(tecNO_PERMISSION)); env.close(); - // TODO: add test when OutstandingAmount is non zero + // Make sure that issuer can't delete issuance when it still has + // outstanding balance + { + // bob now holds a mptoken object + env(mpt::authorize(bob, id, std::nullopt)); + env.close(); + + BEAST_EXPECT(env.ownerCount(bob) == 1); + + // alice pays bob 100 tokens + env( + pay(alice, + bob, + ripple::test::jtx::MPT(alice.name(), mpt)(100))); + env.close(); + BEAST_EXPECT(checkMPTokenAmount( + env, keylet::mptIssuance(id).key, bob, 100)); + + env(mpt::destroy(alice, id), ter(tecHAS_OBLIGATIONS)); + env.close(); + } } } @@ -327,6 +348,7 @@ class MPToken_test : public beast::unit_test::suite BEAST_EXPECT(env.ownerCount(alice) == 0); auto const id = getMptID(alice.id(), env.seq(alice)); + auto const mpt = ripple::MPT(env.seq(alice), alice.id()); env(mpt::create(alice)); env.close(); @@ -359,7 +381,33 @@ class MPToken_test : public beast::unit_test::suite env(mpt::authorize(bob, id, std::nullopt), ter(tecMPTOKEN_EXISTS)); env.close(); - // TODO: check where mptoken balance is nonzero + // Check that bob cannot delete CFToken when his balance is non-zero + { + // alice pays bob 100 tokens + env( + pay(alice, + bob, + ripple::test::jtx::MPT(alice.name(), mpt)(100))); + env.close(); + BEAST_EXPECT(checkMPTokenAmount( + env, keylet::mptIssuance(id).key, bob, 100)); + + // bob tries to delete his CFToken, but fails since he still + // holds tokens + env(mpt::authorize(bob, id, std::nullopt), + txflags(tfMPTUnauthorize), + ter(tecHAS_OBLIGATIONS)); + env.close(); + + // bob pays back alice 100 tokens + env( + pay(bob, + alice, + ripple::test::jtx::MPT(alice.name(), mpt)(100))); + env.close(); + BEAST_EXPECT(checkMPTokenAmount( + env, keylet::mptIssuance(id).key, bob, 0)); + } env(mpt::authorize(bob, id, std::nullopt), txflags(tfMPTUnauthorize)); @@ -600,9 +648,6 @@ class MPToken_test : public beast::unit_test::suite BEAST_EXPECT(env.ownerCount(bob) == 0); } - - // TODO: test allowlisting cases where bob tries to send tokens - // without being authorized. } void @@ -963,6 +1008,94 @@ class MPToken_test : public beast::unit_test::suite BEAST_EXPECT(checkMPTokenOutstandingAmount( env, keylet::mptIssuance(id).key, 100)); } + + // If allowlisting is enabled, Payment fails if the receiver is not + // authorized + { + Env env{*this, features}; + Account const alice("alice"); // issuer + Account const bob("bob"); // holder + + env.fund(XRP(10000), alice, bob); + env.close(); + + BEAST_EXPECT(env.ownerCount(alice) == 0); + + auto const seq = env.seq(alice); + auto const id = getMptID(alice.id(), seq); + auto const mpt = ripple::MPT(seq, alice.id()); + + env(mpt::create(alice), txflags(tfMPTRequireAuth)); + env.close(); + + BEAST_EXPECT(env.ownerCount(alice) == 1); + BEAST_EXPECT(env.ownerCount(bob) == 0); + + env(mpt::authorize(bob, id, std::nullopt)); + env.close(); + + env(pay(alice, bob, ripple::test::jtx::MPT(alice.name(), mpt)(100)), + ter(tecNO_AUTH)); + env.close(); + BEAST_EXPECT( + checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 0)); + BEAST_EXPECT(checkMPTokenOutstandingAmount( + env, keylet::mptIssuance(id).key, 0)); + } + + // If allowlisting is enabled, Payment fails if the sender is not + // authorized + { + Env env{*this, features}; + Account const alice("alice"); // issuer + Account const bob("bob"); // holder + + env.fund(XRP(10000), alice, bob); + env.close(); + + BEAST_EXPECT(env.ownerCount(alice) == 0); + + auto const seq = env.seq(alice); + auto const id = getMptID(alice.id(), seq); + auto const mpt = ripple::MPT(seq, alice.id()); + + env(mpt::create(alice), txflags(tfMPTRequireAuth)); + env.close(); + + BEAST_EXPECT(env.ownerCount(alice) == 1); + BEAST_EXPECT(env.ownerCount(bob) == 0); + + // bob creates an empty MPToken + env(mpt::authorize(bob, id, std::nullopt)); + env.close(); + + // alice authorizes bob to hold funds + env(mpt::authorize(alice, id, bob)); + env.close(); + + // alice sends 100 MPT to bob + env(pay( + alice, bob, ripple::test::jtx::MPT(alice.name(), mpt)(100))); + env.close(); + BEAST_EXPECT( + checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 100)); + BEAST_EXPECT(checkMPTokenOutstandingAmount( + env, keylet::mptIssuance(id).key, 100)); + + // alice UNAUTHORIZES bob + env(mpt::authorize(alice, id, bob), txflags(tfMPTUnauthorize)); + env.close(); + + // bob fails to send back to alice because he is no longer authorize + // to move his funds! + env(pay(bob, alice, ripple::test::jtx::MPT(bob.name(), mpt)(100)), + ter(tecNO_AUTH)); + env.close(); + BEAST_EXPECT( + checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 100)); + BEAST_EXPECT(checkMPTokenOutstandingAmount( + env, keylet::mptIssuance(id).key, 100)); + } } void From c8d638b7a079203d858847b2c9d49c4ab281df24 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Mon, 8 Jan 2024 09:41:21 -0500 Subject: [PATCH 02/44] modify require auth --- src/ripple/ledger/impl/View.cpp | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index dced3e8bf46..c6d4bcb98fa 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1563,10 +1563,8 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account) sle && sle->getFieldU32(sfFlags) & lsfMPTRequireAuth) { auto const mptokenID = keylet::mptoken(mptID.key, account); - if (auto const tokSle = view.read(mptokenID)) - { - // TODO no lsfAuthorized as in specs - } + if (auto const tokSle = view.read(mptokenID); (sle->getFlags() & lsfMPTRequireAuth) && !(tokSle->getFlags() & lsfMPTAuthorized)) + return TER{tecNO_AUTH}; } return tesSUCCESS; } @@ -1718,6 +1716,14 @@ rippleMPTCredit( STAmount saAmount, beast::Journal j) { + if (auto const ter = requireAuth(view, saAmount.issue(), uSenderID); + ter != tesSUCCESS) + return ter; + + if (auto const ter = requireAuth(view, saAmount.issue(), uReceiverID); + ter != tesSUCCESS) + return ter; + auto const mptID = keylet::mptIssuance(saAmount.getAsset()); if (uSenderID == saAmount.getIssuer()) { @@ -1736,14 +1742,6 @@ rippleMPTCredit( auto const mptokenID = keylet::mptoken(mptID.key, uSenderID); if (auto sle = view.peek(mptokenID)) { - // Ensure sender is authorized if the MPT requires auth. - // It's possible that the issuer unauthorized a token holder - // who is holding some MPT - if (auto const sleIssuance = view.peek(mptID); - (sleIssuance->getFlags() & lsfMPTRequireAuth) && - !(sle->getFlags() & lsfMPTAuthorized)) - return tecNO_AUTH; - auto const amt = sle->getFieldU64(sfMPTAmount); auto const pay = saAmount.mpt().mpt(); if (amt >= pay) @@ -1781,12 +1779,6 @@ rippleMPTCredit( auto const mptokenID = keylet::mptoken(mptID.key, uReceiverID); if (auto sle = view.peek(mptokenID)) { - // Ensure receiver is authorized if the MPT requires auth - if (auto const sleIssuance = view.peek(mptID); - (sleIssuance->getFlags() & lsfMPTRequireAuth) && - !(sle->getFlags() & lsfMPTAuthorized)) - return tecNO_AUTH; - sle->setFieldU64( sfMPTAmount, sle->getFieldU64(sfMPTAmount) + saAmount.mpt().mpt()); From 78a6a93b313a015eb4c8f973d1d3ea6b90cf50e3 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Mon, 8 Jan 2024 09:41:54 -0500 Subject: [PATCH 03/44] clang --- src/ripple/ledger/impl/View.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index c6d4bcb98fa..521207c76b4 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1563,7 +1563,9 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account) sle && sle->getFieldU32(sfFlags) & lsfMPTRequireAuth) { auto const mptokenID = keylet::mptoken(mptID.key, account); - if (auto const tokSle = view.read(mptokenID); (sle->getFlags() & lsfMPTRequireAuth) && !(tokSle->getFlags() & lsfMPTAuthorized)) + if (auto const tokSle = view.read(mptokenID); + (sle->getFlags() & lsfMPTRequireAuth) && + !(tokSle->getFlags() & lsfMPTAuthorized)) return TER{tecNO_AUTH}; } return tesSUCCESS; From f8d0795e401dc1f90e8c7cd4d072e68bad57b84a Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Mon, 8 Jan 2024 15:06:02 -0500 Subject: [PATCH 04/44] Move requireAuth call --- src/ripple/app/tx/impl/Payment.cpp | 8 ++++++++ src/ripple/ledger/impl/View.cpp | 8 -------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index 10b69fd582d..d41422203c7 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -437,6 +437,14 @@ Payment::doApply() } else if (saDstAmount.isMPT()) { + if (auto const ter = requireAuth(view(), saDstAmount.issue(), account_); + ter != tesSUCCESS) + return ter; + + if (auto const ter = requireAuth(view(), saDstAmount.issue(), uDstAccountID); + ter != tesSUCCESS) + return ter; + PaymentSandbox pv(&view()); auto const res = rippleMPTCredit(pv, account_, uDstAccountID, saDstAmount, j_); diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 521207c76b4..a24d1dda016 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1718,14 +1718,6 @@ rippleMPTCredit( STAmount saAmount, beast::Journal j) { - if (auto const ter = requireAuth(view, saAmount.issue(), uSenderID); - ter != tesSUCCESS) - return ter; - - if (auto const ter = requireAuth(view, saAmount.issue(), uReceiverID); - ter != tesSUCCESS) - return ter; - auto const mptID = keylet::mptIssuance(saAmount.getAsset()); if (uSenderID == saAmount.getIssuer()) { From 31726d9dfd461b37bf6c1d5981349566a612e5dd Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Mon, 8 Jan 2024 15:07:52 -0500 Subject: [PATCH 05/44] clang --- src/ripple/app/tx/impl/Payment.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index d41422203c7..7bda0a02003 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -441,7 +441,8 @@ Payment::doApply() ter != tesSUCCESS) return ter; - if (auto const ter = requireAuth(view(), saDstAmount.issue(), uDstAccountID); + if (auto const ter = + requireAuth(view(), saDstAmount.issue(), uDstAccountID); ter != tesSUCCESS) return ter; From 9807aae704d94f6f0a676e1170d4c98646fe961e Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Mon, 8 Jan 2024 17:57:46 -0500 Subject: [PATCH 06/44] Resolve MPT/Currency inconsistencies, use accountSend() for direct MPT payment. --- src/ripple/app/ledger/OrderBookDB.cpp | 7 ++----- src/ripple/app/paths/impl/AmountSpec.h | 10 +++++----- src/ripple/app/paths/impl/PaySteps.cpp | 25 ++++++++++++------------- src/ripple/app/tx/impl/CreateOffer.cpp | 1 - src/ripple/app/tx/impl/Payment.cpp | 3 +-- src/ripple/app/tx/impl/SetTrust.cpp | 2 +- src/ripple/protocol/Book.h | 2 +- src/ripple/protocol/Issue.h | 11 +++++------ src/ripple/protocol/impl/Issue.cpp | 20 ++++++++++++-------- src/ripple/rpc/handlers/Subscribe.cpp | 1 - 10 files changed, 39 insertions(+), 43 deletions(-) diff --git a/src/ripple/app/ledger/OrderBookDB.cpp b/src/ripple/app/ledger/OrderBookDB.cpp index 5001d099012..479570f4ec7 100644 --- a/src/ripple/app/ledger/OrderBookDB.cpp +++ b/src/ripple/app/ledger/OrderBookDB.cpp @@ -114,15 +114,12 @@ OrderBookDB::update(std::shared_ptr const& ledger) sle->getFieldH256(sfRootIndex) == sle->key()) { Book book; - // TODO update for MPT once supported in the offers Currency currency; AccountID account; - currency = static_cast( - sle->getFieldH160(sfTakerPaysCurrency)); + currency = sle->getFieldH160(sfTakerPaysCurrency); account = sle->getFieldH160(sfTakerPaysIssuer); book.in = std::make_pair(currency, account); - currency = static_cast( - sle->getFieldH160(sfTakerGetsCurrency)); + currency = sle->getFieldH160(sfTakerGetsCurrency); account = sle->getFieldH160(sfTakerGetsIssuer); book.out = std::make_pair(currency, account); diff --git a/src/ripple/app/paths/impl/AmountSpec.h b/src/ripple/app/paths/impl/AmountSpec.h index b780871157b..61d9dd6c0a8 100644 --- a/src/ripple/app/paths/impl/AmountSpec.h +++ b/src/ripple/app/paths/impl/AmountSpec.h @@ -42,7 +42,7 @@ struct AmountSpec IOUAmount iou = {}; }; std::optional issuer; - std::optional currency; + std::optional asset; friend std::ostream& operator<<(std::ostream& stream, AmountSpec const& amt) @@ -53,8 +53,8 @@ struct AmountSpec stream << to_string(amt.xrp); else stream << to_string(amt.iou); - if (amt.currency) - stream << "/(" << *amt.currency << ")"; + if (amt.asset) + stream << "/(" << *amt.asset << ")"; if (amt.issuer) stream << "/" << *amt.issuer << ""; return stream; @@ -217,7 +217,7 @@ toAmountSpec(STAmount const& amt) else result.iou = IOUAmount(sMant, amt.exponent()); result.issuer = amt.issue().account(); - result.currency = amt.issue().asset(); + result.asset = amt.issue().asset(); } return result; @@ -238,7 +238,7 @@ toAmountSpec(EitherAmount const& ea, std::optional const& a) { AmountSpec r; r.native = (!a || isXRP(*a)); - r.currency = a; + r.asset = a; assert(ea.native == r.native); if (r.is_mpt) { diff --git a/src/ripple/app/paths/impl/PaySteps.cpp b/src/ripple/app/paths/impl/PaySteps.cpp index d571aa29e02..228b967e21b 100644 --- a/src/ripple/app/paths/impl/PaySteps.cpp +++ b/src/ripple/app/paths/impl/PaySteps.cpp @@ -186,11 +186,11 @@ toStrand( } Issue curIssue = [&] { - auto const& asset = - sendMaxIssue ? sendMaxIssue->asset() : deliver.asset(); - if (isXRP(asset)) + auto const& currency = static_cast( + sendMaxIssue ? sendMaxIssue->asset() : deliver.asset()); + if (isXRP(currency)) return xrpIssue(); - return Issue{asset, src}; + return Issue{currency, src}; }(); auto hasCurrency = [](STPathElement const pe) { @@ -304,11 +304,10 @@ toStrand( if (cur->hasCurrency()) { - Currency currency; - currency = cur->getCurrency(); - if (isXRP(currency)) - curIssue.setIssuer(xrpAccount()); - curIssue = std::make_pair(currency, curIssue.account()); + Currency const currency = cur->getCurrency(); + AccountID const account = + isXRP(currency) ? xrpAccount() : curIssue.account(); + curIssue = std::make_pair(currency, account); } if (cur->isAccount() && next->isAccount()) @@ -422,11 +421,11 @@ toStrand( auto curAcc = src; auto curIss = [&] { - auto& asset = - sendMaxIssue ? sendMaxIssue->asset() : deliver.asset(); - if (isXRP(asset)) + auto const& currency = static_cast( + sendMaxIssue ? sendMaxIssue->asset() : deliver.asset()); + if (isXRP(currency)) return xrpIssue(); - return Issue{asset, src}; + return Issue{currency, src}; }(); for (auto const& s : result) diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index 662722fdb9b..a5bc020ac61 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -1185,7 +1185,6 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) bool const bookExisted = static_cast(sb.peek(dir)); auto const bookNode = sb.dirAppend(dir, offer_index, [&](SLE::ref sle) { - // TODO add MPT once offers are supported for MPT sle->setFieldH160( sfTakerPaysCurrency, static_cast(saTakerPays.issue().asset())); diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index 7bda0a02003..55e9ee76343 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -91,7 +91,6 @@ Payment::preflight(PreflightContext const& ctx) auto const& uSrcCurrency = maxSourceAmount.getAsset(); auto const& uDstCurrency = saDstAmount.getAsset(); - // isZero() is XRP. FIX! bool const bXRPDirect = uSrcCurrency.isXRP() && uDstCurrency.isXRP(); bool const bMPTDirect = uSrcCurrency.isMPT() && uDstCurrency.isMPT(); bool const bDirect = bXRPDirect || bMPTDirect; @@ -448,7 +447,7 @@ Payment::doApply() PaymentSandbox pv(&view()); auto const res = - rippleMPTCredit(pv, account_, uDstAccountID, saDstAmount, j_); + accountSend(pv, account_, uDstAccountID, saDstAmount, j_); pv.apply(ctx_.rawView()); return res; } diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index fb3ed782456..b8cb584cc7d 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -102,7 +102,7 @@ SetTrust::preclaim(PreclaimContext const& ctx) auto const saLimitAmount = ctx.tx[sfLimitAmount]; - auto const currency = saLimitAmount.getAsset(); + auto const currency = static_cast(saLimitAmount.getAsset()); auto const uDstAccountID = saLimitAmount.getIssuer(); if (ctx.view.rules().enabled(fixTrustLinesToSelf)) diff --git a/src/ripple/protocol/Book.h b/src/ripple/protocol/Book.h index 546321ab2c0..649f11caf4a 100644 --- a/src/ripple/protocol/Book.h +++ b/src/ripple/protocol/Book.h @@ -67,7 +67,7 @@ reversed(Book const& book); /** Equality comparison. */ /** @{ */ -[[nodiscard]] inline bool +[[nodiscard]] inline constexpr bool operator==(Book const& lhs, Book const& rhs) { return (lhs.in == rhs.in) && (lhs.out == rhs.out); diff --git a/src/ripple/protocol/Issue.h b/src/ripple/protocol/Issue.h index edbddf263f6..e20704208af 100644 --- a/src/ripple/protocol/Issue.h +++ b/src/ripple/protocol/Issue.h @@ -36,8 +36,6 @@ namespace ripple { class Issue { private: - // using IOU = std::pair; - // std::variant issue_; Asset asset_{}; std::optional account_{std::nullopt}; @@ -157,14 +155,15 @@ void hash_append(Hasher& h, Issue const& r) { using beast::hash_append; - std::visit( - [&](auto&& arg) { hash_append(h, arg, r.account()); }, - r.asset().asset()); + if (r.isMPT()) + hash_append(h, std::get(r.asset().asset())); + else + hash_append(h, std::get(r.asset().asset()), r.account()); } /** Equality comparison. */ /** @{ */ -[[nodiscard]] inline bool +[[nodiscard]] inline constexpr bool operator==(Issue const& lhs, Issue const& rhs) { return (lhs.asset() == rhs.asset()) && diff --git a/src/ripple/protocol/impl/Issue.cpp b/src/ripple/protocol/impl/Issue.cpp index 319f4d0b8c2..3901f3f5c7f 100644 --- a/src/ripple/protocol/impl/Issue.cpp +++ b/src/ripple/protocol/impl/Issue.cpp @@ -94,22 +94,26 @@ issueFromJson(Json::Value const& v) bool const isMPT = v.isMember(jss::mpt_issuance_id); - Json::Value const assetStr = - isMPT ? v[jss::mpt_issuance_id] : v[jss::currency]; + assert(!isMPT); + if (isMPT) + Throw("issueFromJson MPT is not supported"); + + Json::Value const curStr = v[jss::currency]; Json::Value const issStr = v[jss::issuer]; - if (!assetStr.isString()) + if (!curStr.isString()) { - Throw("issueFromJson asset must be a string Json value"); + Throw( + "issueFromJson currency must be a string Json value"); } - Asset asset = to_currency(assetStr.asString()); - if (asset == badCurrency() || asset == noCurrency()) + auto const currency = to_currency(curStr.asString()); + if (currency == badCurrency() || currency == noCurrency()) { Throw("issueFromJson currency must be a valid currency"); } - if (asset.isXRP()) + if (isXRP(currency)) { if (!issStr.isNull()) { @@ -129,7 +133,7 @@ issueFromJson(Json::Value const& v) Throw("issueFromJson issuer must be a valid account"); } - return Issue{asset, *issuer}; + return Issue{currency, *issuer}; } std::ostream& diff --git a/src/ripple/rpc/handlers/Subscribe.cpp b/src/ripple/rpc/handlers/Subscribe.cpp index 609f603c0f6..fa868039b65 100644 --- a/src/ripple/rpc/handlers/Subscribe.cpp +++ b/src/ripple/rpc/handlers/Subscribe.cpp @@ -250,7 +250,6 @@ doSubscribe(RPC::JsonContext& context) Json::Value taker_pays = j[jss::taker_pays]; Json::Value taker_gets = j[jss::taker_gets]; - // TODO Add MPT once supported in OFFERS // Parse mandatory currency. Currency inCurrency; if (!taker_pays.isMember(jss::currency) || From 3f2101d277c6fe89227d4680a83932112220c3eb Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Mon, 8 Jan 2024 18:10:37 -0500 Subject: [PATCH 07/44] Fix constexpr --- src/ripple/protocol/Asset.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ripple/protocol/Asset.h b/src/ripple/protocol/Asset.h index 56e515dbed5..b2fb37814bb 100644 --- a/src/ripple/protocol/Asset.h +++ b/src/ripple/protocol/Asset.h @@ -132,42 +132,42 @@ class Asset return std::holds_alternative(a1.asset_) == std::holds_alternative(a2.asset_); } - friend bool + friend constexpr bool operator==(Currency const& c, Asset const& a) noexcept { return a.isCurrency() && c == (Currency&)a; } - friend bool + friend constexpr bool operator==(Asset const& a, Currency const& c) noexcept { return c == a; } - friend bool + friend constexpr bool operator==(Asset const& a1, Asset const& a2) noexcept { return comparable(a1, a2) && a1.asset_ == a2.asset_; } - friend bool + friend constexpr bool operator!=(Asset const& a1, Asset const& a2) noexcept { return !(a1 == a2); } - friend bool + friend constexpr bool operator<(Asset const& a1, Asset const& a2) noexcept { return comparable(a1, a2) && a1.asset_ < a2.asset_; } - friend bool + friend constexpr bool operator>(Asset const& a1, Asset const& a2) noexcept { return a2 < a1; } - friend bool + friend constexpr bool operator<=(Asset const& a1, Asset const& a2) noexcept { return !(a2 < a1); } - friend bool + friend constexpr bool operator>=(Asset const& a1, Asset const& a2) noexcept { return !(a1 < a2); From 22909fce8f4779b1a16d0c2525f6adb714b509b0 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Mon, 8 Jan 2024 18:15:44 -0500 Subject: [PATCH 08/44] Fix constexpr --- src/ripple/protocol/Asset.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/Asset.h b/src/ripple/protocol/Asset.h index b2fb37814bb..458cf629be3 100644 --- a/src/ripple/protocol/Asset.h +++ b/src/ripple/protocol/Asset.h @@ -126,7 +126,7 @@ class Asset return std::get(asset_); } - friend bool + friend constexpr bool comparable(Asset const& a1, Asset const& a2) { return std::holds_alternative(a1.asset_) == From f959d151683dfae1f7bb5f64378bd7781f065b8f Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 9 Jan 2024 10:41:12 -0500 Subject: [PATCH 09/44] MaxAmt --- src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp | 12 ++++++++++-- src/test/app/MPToken_test.cpp | 8 ++++++++ src/test/jtx/impl/mpt.cpp | 13 +++++++++++-- src/test/jtx/mpt.h | 2 +- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp b/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp index 5a8abf34f19..8711ee4514a 100644 --- a/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp +++ b/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp @@ -55,8 +55,16 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) return temMALFORMED; } - // TODO: check if maximumAmount is within 63 bit range - + // Check if maximumAmount is within 63 bit range + if (auto const maxAmt = ctx.tx[~sfMaximumAmount]) + { + if (maxAmt == 0) + return temMALFORMED; + + // TODO: Improve this check and move the constant elsewhere (STAmount?) + if (maxAmt > 0x7FFFFFFFFFFFFFFFull) + return temMALFORMED; + } return preflight2(ctx); } diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index f5c3a492d4a..48cf5666fa8 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -123,6 +123,14 @@ class MPToken_test : public beast::unit_test::suite // empty metadata returns error env(mpt::create(alice, 100, 0, 0, ""), ter(temMALFORMED)); env.close(); + + // MaximumAmout of 0 returns error + env(mpt::create(alice, 0, 1, 1, "test"), ter(temMALFORMED)); + env.close(); + + // MaximumAmount larger than 63 bit returns errpr + env(mpt::create(alice, 0xFFFFFFFFFFFFFFF0ull, 0, 0, "test"), ter(temMALFORMED)); + env.close(); } } diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 5426dae4369..cf40aed1c98 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -27,6 +27,13 @@ namespace jtx { namespace mpt { +static std::array +uint64ToByteArray(std::uint64_t value) { + std::array result; + std::memcpy(result.data(), &value, sizeof(value)); + return result; +} + Json::Value create(jtx::Account const& account) { @@ -39,7 +46,7 @@ create(jtx::Account const& account) Json::Value create( jtx::Account const& account, - std::uint32_t const maxAmt, + std::uint64_t const maxAmt, std::uint8_t const assetScale, std::uint16_t transferFee, std::string metadata) @@ -47,10 +54,12 @@ create( Json::Value jv; jv[sfAccount.jsonName] = account.human(); jv[sfTransactionType.jsonName] = jss::MPTokenIssuanceCreate; - jv[sfMaximumAmount.jsonName] = maxAmt; jv[sfAssetScale.jsonName] = assetScale; jv[sfTransferFee.jsonName] = transferFee; jv[sfMPTokenMetadata.jsonName] = strHex(metadata); + + // convert maxAmt to hex string, since json doesn't accept 64-bit int + jv[sfMaximumAmount.jsonName] = strHex(uint64ToByteArray(maxAmt)); return jv; } diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 8b164f0dacb..7f77c89ca34 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -38,7 +38,7 @@ create(jtx::Account const& account); Json::Value create( jtx::Account const& account, - std::uint32_t const maxAmt, + std::uint64_t const maxAmt, std::uint8_t const assetScale, std::uint16_t transferFee, std::string metadata); From baca32e08c3fe478086e9c13f0b0ee6ba81010c2 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 9 Jan 2024 13:47:18 -0500 Subject: [PATCH 10/44] Fix test --- src/test/app/MPToken_test.cpp | 2 +- src/test/jtx/impl/mpt.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 48cf5666fa8..c1427b0a614 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -153,7 +153,7 @@ class MPToken_test : public beast::unit_test::suite BEAST_EXPECT(env.ownerCount(alice) == 0); auto const id = getMptID(alice, env.seq(alice)); - env(mpt::create(alice, 100, 1, 10, "123"), + env(mpt::create(alice, 0x7FFFFFFFFFFFFFFF, 1, 10, "123"), txflags( tfMPTCanLock | tfMPTRequireAuth | tfMPTCanEscrow | tfMPTCanTrade | tfMPTCanTransfer | tfMPTCanClawback)); diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index cf40aed1c98..62d56cf9316 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -29,6 +29,7 @@ namespace mpt { static std::array uint64ToByteArray(std::uint64_t value) { + value = boost::endian::native_to_big(value); std::array result; std::memcpy(result.data(), &value, sizeof(value)); return result; From b1513c2d0bf644a4e9f534c0925e1f0d1d5c6298 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 9 Jan 2024 15:14:00 -0500 Subject: [PATCH 11/44] clang --- src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp | 2 +- src/test/app/MPToken_test.cpp | 3 ++- src/test/jtx/impl/mpt.cpp | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp b/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp index 8711ee4514a..3f220dd5f64 100644 --- a/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp +++ b/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp @@ -60,7 +60,7 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) { if (maxAmt == 0) return temMALFORMED; - + // TODO: Improve this check and move the constant elsewhere (STAmount?) if (maxAmt > 0x7FFFFFFFFFFFFFFFull) return temMALFORMED; diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index c1427b0a614..6cd700c3e5b 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -129,7 +129,8 @@ class MPToken_test : public beast::unit_test::suite env.close(); // MaximumAmount larger than 63 bit returns errpr - env(mpt::create(alice, 0xFFFFFFFFFFFFFFF0ull, 0, 0, "test"), ter(temMALFORMED)); + env(mpt::create(alice, 0xFFFFFFFFFFFFFFF0ull, 0, 0, "test"), + ter(temMALFORMED)); env.close(); } } diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 62d56cf9316..5653d3e70fc 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -27,8 +27,9 @@ namespace jtx { namespace mpt { -static std::array -uint64ToByteArray(std::uint64_t value) { +static std::array +uint64ToByteArray(std::uint64_t value) +{ value = boost::endian::native_to_big(value); std::array result; std::memcpy(result.data(), &value, sizeof(value)); From 8bbb22cca4110d6bfc71f53e2ee579f6d75da60a Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 10 Jan 2024 10:20:40 -0500 Subject: [PATCH 12/44] Add MPTTester class to streamline MPToken unit-tests --- src/test/app/MPToken_test.cpp | 930 ++++++++++----------------- src/test/jtx/impl/mpt.cpp | 205 ++++-- src/test/jtx/mpt.h | 197 +++++- src/test/rpc/AccountObjects_test.cpp | 11 +- src/test/rpc/LedgerRPC_test.cpp | 9 +- 5 files changed, 653 insertions(+), 699 deletions(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 6cd700c3e5b..c26df7b7b41 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -87,51 +87,61 @@ class MPToken_test : public beast::unit_test::suite { testcase("Create Validate"); using namespace test::jtx; + Account const alice("alice"); // test preflight of MPTokenIssuanceCreate { // If the MPT amendment is not enabled, you should not be able to // create MPTokenIssuances Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); // issuer - - env.fund(XRP(10000), alice); - env.close(); + MPTTester mptAlice(env, alice); - BEAST_EXPECT(env.ownerCount(alice) == 0); - - env(mpt::create(alice), ter(temDISABLED)); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); + mptAlice.create({.ownerCount = 0, .err = temDISABLED}); env.enableFeature(featureMPTokensV1); - env(mpt::create(alice), txflags(0x00000001), ter(temINVALID_FLAG)); - env.close(); + mptAlice.create({.flags = 0x00000001, .err = temINVALID_FLAG}); // tries to set a txfee while not enabling in the flag - env(mpt::create(alice, 100, 0, 1, "test"), ter(temMALFORMED)); - env.close(); + mptAlice.create( + {.maxAmt = 100, + .assetScale = 0, + .transferFee = 1, + .metadata = "test", + .err = temMALFORMED}); // tries to set a txfee while not enabling transfer - env(mpt::create(alice, 100, 0, maxTransferFee + 1, "test"), - txflags(tfMPTCanTransfer), - ter(temBAD_MPTOKEN_TRANSFER_FEE)); - env.close(); + mptAlice.create( + {.maxAmt = 100, + .assetScale = 0, + .transferFee = maxTransferFee + 1, + .metadata = "test", + .flags = tfMPTCanTransfer, + .err = temBAD_MPTOKEN_TRANSFER_FEE}); // empty metadata returns error - env(mpt::create(alice, 100, 0, 0, ""), ter(temMALFORMED)); - env.close(); + mptAlice.create( + {.maxAmt = 100, + .assetScale = 0, + .transferFee = 0, + .metadata = "", + .err = temMALFORMED}); // MaximumAmout of 0 returns error - env(mpt::create(alice, 0, 1, 1, "test"), ter(temMALFORMED)); - env.close(); - - // MaximumAmount larger than 63 bit returns errpr - env(mpt::create(alice, 0xFFFFFFFFFFFFFFF0ull, 0, 0, "test"), - ter(temMALFORMED)); - env.close(); + mptAlice.create( + {.maxAmt = 0, + .assetScale = 1, + .transferFee = 1, + .metadata = "test", + .err = temMALFORMED}); + + // MaximumAmount larger than 63 bit returns error + mptAlice.create( + {.maxAmt = 0xFFFFFFFFFFFFFFF0ull, + .assetScale = 0, + .transferFee = 0, + .metadata = "test", + .err = temMALFORMED}); } } @@ -141,30 +151,24 @@ class MPToken_test : public beast::unit_test::suite testcase("Create Enabled"); using namespace test::jtx; + Account const alice("alice"); { // If the MPT amendment IS enabled, you should be able to create // MPTokenIssuances Env env{*this, features}; - Account const alice("alice"); // issuer - - env.fund(XRP(10000), alice); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice, env.seq(alice)); - env(mpt::create(alice, 0x7FFFFFFFFFFFFFFF, 1, 10, "123"), - txflags( - tfMPTCanLock | tfMPTRequireAuth | tfMPTCanEscrow | - tfMPTCanTrade | tfMPTCanTransfer | tfMPTCanClawback)); - env.close(); - - BEAST_EXPECT(checkMPTokenIssuanceFlags( - env, - keylet::mptIssuance(id).key, + MPTTester mptAlice(env, alice); + mptAlice.create( + {.maxAmt = 0x7FFFFFFFFFFFFFFF, + .assetScale = 1, + .transferFee = 10, + .metadata = "123", + .flags = tfMPTCanLock | tfMPTRequireAuth | tfMPTCanEscrow | + tfMPTCanTrade | tfMPTCanTransfer | tfMPTCanClawback}); + + BEAST_EXPECT(mptAlice.checkFlags( lsfMPTCanLock | lsfMPTRequireAuth | lsfMPTCanEscrow | - lsfMPTCanTrade | lsfMPTCanTransfer | lsfMPTCanClawback)); + lsfMPTCanTrade | lsfMPTCanTransfer | lsfMPTCanClawback)); BEAST_EXPECT(env.ownerCount(alice) == 1); } @@ -176,79 +180,47 @@ class MPToken_test : public beast::unit_test::suite testcase("Destroy Validate"); using namespace test::jtx; + Account const alice("alice"); + Account const bob("bob"); // MPTokenIssuanceDestroy (preflight) { Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); // issuer - - env.fund(XRP(10000), alice); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice.id(), env.seq(alice)); - env(mpt::destroy(alice, id), ter(temDISABLED)); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); + MPTTester mptAlice(env, alice); + mptAlice.destroy({.ownerCount = 0, .err = temDISABLED}); env.enableFeature(featureMPTokensV1); - env(mpt::destroy(alice, id), - txflags(0x00000001), - ter(temINVALID_FLAG)); - env.close(); + mptAlice.destroy({.flags = 0x00000001, .err = temINVALID_FLAG}); } // MPTokenIssuanceDestroy (preclaim) { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - - env.fund(XRP(10000), alice, bob); - env.close(); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - BEAST_EXPECT(env.ownerCount(alice) == 0); + mptAlice.destroy( + {.id = getMptID(alice.id(), env.seq(alice)), + .ownerCount = 0, + .err = tecOBJECT_NOT_FOUND}); - auto const fakeID = getMptID(alice.id(), env.seq(alice)); - - env(mpt::destroy(alice, fakeID), ter(tecOBJECT_NOT_FOUND)); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mpt = ripple::MPT(env.seq(alice), alice.id()); - env(mpt::create(alice)); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 1); + mptAlice.create({.ownerCount = 1}); // a non-issuer tries to destroy a mptissuance they didn't issue - env(mpt::destroy(bob, id), ter(tecNO_PERMISSION)); - env.close(); + mptAlice.destroy({.issuer = &bob, .err = tecNO_PERMISSION}); // Make sure that issuer can't delete issuance when it still has // outstanding balance { // bob now holds a mptoken object - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 1); // alice pays bob 100 tokens - env( - pay(alice, - bob, - ripple::test::jtx::MPT(alice.name(), mpt)(100))); - env.close(); - BEAST_EXPECT(checkMPTokenAmount( - env, keylet::mptIssuance(id).key, bob, 100)); + mptAlice.pay(alice, bob, 100); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); - env(mpt::destroy(alice, id), ter(tecHAS_OBLIGATIONS)); - env.close(); + mptAlice.destroy({.err = tecHAS_OBLIGATIONS}); } } } @@ -259,26 +231,16 @@ class MPToken_test : public beast::unit_test::suite testcase("Destroy Enabled"); using namespace test::jtx; + Account const alice("alice"); // If the MPT amendment IS enabled, you should be able to destroy // MPTokenIssuances Env env{*this, features}; - Account const alice("alice"); // issuer - - env.fund(XRP(10000), alice); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice.id(), env.seq(alice)); - env(mpt::create(alice)); - env.close(); + MPTTester mptAlice(env, alice); - BEAST_EXPECT(env.ownerCount(alice) == 1); + mptAlice.create({.ownerCount = 1}); - env(mpt::destroy(alice, id)); - env.close(); - BEAST_EXPECT(env.ownerCount(alice) == 0); + mptAlice.destroy({.ownerCount = 0}); } void @@ -287,145 +249,94 @@ class MPToken_test : public beast::unit_test::suite testcase("Validate authorize transaction"); using namespace test::jtx; + Account const alice("alice"); + Account const bob("bob"); // Validate fields in MPTokenAuthorize (preflight) { Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - auto const id = getMptID(alice.id(), env.seq(alice)); - - env(mpt::authorize(bob, id, std::nullopt), ter(temDISABLED)); - env.close(); + mptAlice.authorize({.account = &bob, .err = temDISABLED}); env.enableFeature(featureMPTokensV1); - env(mpt::create(alice)); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 1); + mptAlice.create({.ownerCount = 1}); - env(mpt::authorize(bob, id, std::nullopt), - txflags(0x00000002), - ter(temINVALID_FLAG)); - env.close(); + mptAlice.authorize( + {.account = &bob, .flags = 0x00000002, .err = temINVALID_FLAG}); - env(mpt::authorize(bob, id, bob), ter(temMALFORMED)); - env.close(); + mptAlice.authorize( + {.account = &bob, .holder = &bob, .err = temMALFORMED}); - env(mpt::authorize(alice, id, alice), ter(temMALFORMED)); - env.close(); + mptAlice.authorize({.holder = &alice, .err = temMALFORMED}); } - // Try authorizing when MPTokenIssuance doesnt exist in MPTokenAuthorize - // (preclaim) + // Try authorizing when MPTokenIssuance doesn't exist in + // MPTokenAuthorize (preclaim) { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - auto const id = getMptID(alice.id(), env.seq(alice)); - - env(mpt::authorize(alice, id, bob), ter(tecOBJECT_NOT_FOUND)); - env.close(); + mptAlice.authorize({.holder = &bob, .err = tecOBJECT_NOT_FOUND}); - env(mpt::authorize(bob, id, std::nullopt), - ter(tecOBJECT_NOT_FOUND)); - env.close(); + mptAlice.authorize({.account = &bob, .err = tecOBJECT_NOT_FOUND}); } // Test bad scenarios without allowlisting in MPTokenAuthorize // (preclaim) { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mpt = ripple::MPT(env.seq(alice), alice.id()); - env(mpt::create(alice)); - env.close(); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, keylet::mptIssuance(id).key, 0)); + mptAlice.create({.ownerCount = 1}); - BEAST_EXPECT(env.ownerCount(alice) == 1); + BEAST_EXPECT(mptAlice.checkFlags(0)); // bob submits a tx with a holder field - env(mpt::authorize(bob, id, alice), ter(temMALFORMED)); - env.close(); + mptAlice.authorize( + {.account = &bob, .holder = &alice, .err = temMALFORMED}); - env(mpt::authorize(bob, id, bob), ter(temMALFORMED)); - env.close(); + mptAlice.authorize( + {.account = &bob, .holder = &bob, .err = temMALFORMED}); - env(mpt::authorize(alice, id, alice), ter(temMALFORMED)); - env.close(); + mptAlice.authorize({.holder = &alice, .err = temMALFORMED}); // the mpt does not enable allowlisting - env(mpt::authorize(alice, id, bob), ter(tecNO_AUTH)); - env.close(); + mptAlice.authorize({.holder = &bob, .err = tecNO_AUTH}); // bob now holds a mptoken object - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 1); // bob cannot create the mptoken the second time - env(mpt::authorize(bob, id, std::nullopt), ter(tecMPTOKEN_EXISTS)); - env.close(); + mptAlice.authorize({.account = &bob, .err = tecMPTOKEN_EXISTS}); - // Check that bob cannot delete CFToken when his balance is non-zero + // Check that bob cannot delete CFToken when his balance is + // non-zero { // alice pays bob 100 tokens - env( - pay(alice, - bob, - ripple::test::jtx::MPT(alice.name(), mpt)(100))); - env.close(); - BEAST_EXPECT(checkMPTokenAmount( - env, keylet::mptIssuance(id).key, bob, 100)); + mptAlice.pay(alice, bob, 100); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); // bob tries to delete his CFToken, but fails since he still // holds tokens - env(mpt::authorize(bob, id, std::nullopt), - txflags(tfMPTUnauthorize), - ter(tecHAS_OBLIGATIONS)); - env.close(); + mptAlice.authorize( + {.account = &bob, + .flags = tfMPTUnauthorize, + .err = tecHAS_OBLIGATIONS}); // bob pays back alice 100 tokens - env( - pay(bob, - alice, - ripple::test::jtx::MPT(alice.name(), mpt)(100))); - env.close(); - BEAST_EXPECT(checkMPTokenAmount( - env, keylet::mptIssuance(id).key, bob, 0)); + mptAlice.pay(bob, alice, 100); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); } - env(mpt::authorize(bob, id, std::nullopt), - txflags(tfMPTUnauthorize)); - env.close(); + mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); - env(mpt::authorize(bob, id, std::nullopt), - txflags(tfMPTUnauthorize), - ter(tecNO_ENTRY)); - env.close(); + mptAlice.authorize( + {.account = &bob, + .flags = tfMPTUnauthorize, + .err = tecNO_ENTRY}); BEAST_EXPECT(env.ownerCount(bob) == 0); } @@ -436,70 +347,48 @@ class MPToken_test : public beast::unit_test::suite Account const alice("alice"); // issuer Account const bob("bob"); // holder Account const cindy("cindy"); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mptIssuance = keylet::mptIssuance(id); - env(mpt::create(alice), txflags(tfMPTRequireAuth)); - env.close(); - - BEAST_EXPECT(checkMPTokenIssuanceFlags( - env, mptIssuance.key, lsfMPTRequireAuth)); + mptAlice.create({.ownerCount = 1, .flags = tfMPTRequireAuth}); - BEAST_EXPECT(env.ownerCount(alice) == 1); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTRequireAuth)); // alice submits a tx without specifying a holder's account - env(mpt::authorize(alice, id, std::nullopt), ter(temMALFORMED)); - env.close(); + mptAlice.authorize({.err = temMALFORMED}); - // alice submits a tx to authorize a holder that hasn't created a - // mptoken yet - env(mpt::authorize(alice, id, bob), ter(tecNO_ENTRY)); - env.close(); + // alice submits a tx to authorize a holder that hasn't created + // a mptoken yet + mptAlice.authorize({.holder = &bob, .err = tecNO_ENTRY}); // alice specifys a holder acct that doesn't exist - env(mpt::authorize(alice, id, cindy), ter(tecNO_DST)); - env.close(); + mptAlice.authorize({.holder = &cindy, .err = tecNO_DST}); // bob now holds a mptoken object - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 1); - BEAST_EXPECT( - checkMPTokenFlags(env, keylet::mptIssuance(id).key, bob, 0)); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); // alice tries to unauthorize bob. // although tx is successful, // but nothing happens because bob hasn't been authorized yet - env(mpt::authorize(alice, id, bob), txflags(tfMPTUnauthorize)); - env.close(); - BEAST_EXPECT(checkMPTokenFlags(env, mptIssuance.key, bob, 0)); + mptAlice.authorize({.holder = &bob, .flags = tfMPTUnauthorize}); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); // alice authorizes bob // make sure bob's mptoken has set lsfMPTAuthorized - env(mpt::authorize(alice, id, bob)); - env.close(); - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTAuthorized)); + mptAlice.authorize({.holder = &bob}); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTAuthorized, &bob)); // alice tries authorizes bob again. // tx is successful, but bob is already authorized, // so no changes - env(mpt::authorize(alice, id, bob)); - env.close(); - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTAuthorized)); + mptAlice.authorize({.holder = &bob}); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTAuthorized, &bob)); // bob deletes his mptoken - env(mpt::authorize(bob, id, std::nullopt), - txflags(tfMPTUnauthorize)); - env.close(); + mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); BEAST_EXPECT(env.ownerCount(bob) == 0); } @@ -510,50 +399,38 @@ class MPToken_test : public beast::unit_test::suite auto const acctReserve = env.current()->fees().accountReserve(0); auto const incReserve = env.current()->fees().increment; - Account const alice("alice"); - Account const bob("bob"); - - env.fund(XRP(10000), alice); - env.fund(acctReserve + XRP(1), bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id1 = getMptID(alice.id(), env.seq(alice)); - env(mpt::create(alice)); - env.close(); + MPTTester mptAlice1( + env, + alice, + {.holders = {&bob}, + .xrpHolders = acctReserve + XRP(1).value().xrp()}); + mptAlice1.create(); - auto const id2 = getMptID(alice.id(), env.seq(alice)); - env(mpt::create(alice)); - env.close(); + MPTTester mptAlice2(env, alice, {.fund = false}); + mptAlice2.create(); - auto const id3 = getMptID(alice.id(), env.seq(alice)); - env(mpt::create(alice)); - env.close(); + MPTTester mptAlice3(env, alice, {.fund = false}); + mptAlice3.create(); BEAST_EXPECT(env.ownerCount(alice) == 3); // first mpt for free - env(mpt::authorize(bob, id1, std::nullopt)); - env.close(); + mptAlice1.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 1); // second mpt free - env(mpt::authorize(bob, id2, std::nullopt)); - env.close(); + mptAlice2.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 2); - env(mpt::authorize(bob, id3, std::nullopt), - ter(tecINSUFFICIENT_RESERVE)); - env.close(); + mptAlice3.authorize( + {.account = &bob, .err = tecINSUFFICIENT_RESERVE}); env(pay( env.master, bob, drops(incReserve + incReserve + incReserve))); env.close(); - env(mpt::authorize(bob, id3, std::nullopt)); - env.close(); + mptAlice3.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 3); } @@ -565,40 +442,31 @@ class MPToken_test : public beast::unit_test::suite testcase("Authorize Enabled"); using namespace test::jtx; + Account const alice("alice"); + Account const bob("bob"); // Basic authorization without allowlisting { Env env{*this, features}; - Account const alice("alice"); - Account const bob("bob"); - - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); // alice create mptissuance without allowisting - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mptIssuance = keylet::mptIssuance(id); - env(mpt::create(alice)); - env.close(); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create(); - BEAST_EXPECT(checkMPTokenIssuanceFlags(env, mptIssuance.key, 0)); + BEAST_EXPECT(mptAlice.checkFlags(0)); BEAST_EXPECT(env.ownerCount(alice) == 1); // bob creates a mptoken - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 1); - BEAST_EXPECT(checkMPTokenFlags(env, mptIssuance.key, bob, 0)); - BEAST_EXPECT(checkMPTokenAmount(env, mptIssuance.key, bob, 0)); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); // bob deletes his mptoken - env(mpt::authorize(bob, id, std::nullopt), - txflags(tfMPTUnauthorize)); - env.close(); + mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); BEAST_EXPECT(env.ownerCount(bob) == 0); } @@ -606,54 +474,40 @@ class MPToken_test : public beast::unit_test::suite // With allowlisting { Env env{*this, features}; - Account const alice("alice"); - Account const bob("bob"); - - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); // alice creates a mptokenissuance that requires authorization - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mptIssuance = keylet::mptIssuance(id); - env(mpt::create(alice), txflags(tfMPTRequireAuth)); - env.close(); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create({.flags = tfMPTRequireAuth}); - BEAST_EXPECT(checkMPTokenIssuanceFlags( - env, mptIssuance.key, lsfMPTRequireAuth)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTRequireAuth)); BEAST_EXPECT(env.ownerCount(alice) == 1); // bob creates a mptoken - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 1); - BEAST_EXPECT(checkMPTokenFlags(env, mptIssuance.key, bob, 0)); - BEAST_EXPECT(checkMPTokenAmount(env, mptIssuance.key, bob, 0)); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); // alice authorizes bob - env(mpt::authorize(alice, id, bob)); - env.close(); + mptAlice.authorize({.account = &alice, .holder = &bob}); // make sure bob's mptoken has lsfMPTAuthorized set - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTAuthorized)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTAuthorized, &bob)); // Unauthorize bob's mptoken - env(mpt::authorize(alice, id, bob), txflags(tfMPTUnauthorize)); - env.close(); + mptAlice.authorize( + {.account = &alice, .holder = &bob, .flags = tfMPTUnauthorize}); // ensure bob's mptoken no longer has lsfMPTAuthorized set - BEAST_EXPECT(checkMPTokenFlags(env, mptIssuance.key, bob, 0)); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); BEAST_EXPECT(env.ownerCount(bob) == 1); - env(mpt::authorize(bob, id, std::nullopt), - txflags(tfMPTUnauthorize)); - env.close(); + mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); BEAST_EXPECT(env.ownerCount(bob) == 0); } @@ -665,154 +519,121 @@ class MPToken_test : public beast::unit_test::suite testcase("Validate set transaction"); using namespace test::jtx; + Account const alice("alice"); // issuer + Account const bob("bob"); // holder + Account const cindy("cindy"); // Validate fields in MPTokenIssuanceSet (preflight) { Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - - env.fund(XRP(10000), alice, bob); - env.close(); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mptIssuance = keylet::mptIssuance(id); - - env(mpt::set(bob, id, std::nullopt), ter(temDISABLED)); - env.close(); + mptAlice.set({.account = &bob, .err = temDISABLED}); env.enableFeature(featureMPTokensV1); - env(mpt::create(alice)); - env.close(); + mptAlice.create(); - BEAST_EXPECT(checkMPTokenIssuanceFlags(env, mptIssuance.key, 0)); + BEAST_EXPECT(mptAlice.checkFlags(0)); BEAST_EXPECT(env.ownerCount(alice) == 1); BEAST_EXPECT(env.ownerCount(bob) == 0); - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 1); // test invalid flag - env(mpt::set(alice, id, std::nullopt), - txflags(0x00000008), - ter(temINVALID_FLAG)); - env.close(); + mptAlice.set( + {.account = &alice, + .flags = 0x00000008, + .err = temINVALID_FLAG}); // set both lock and unlock flags at the same time will fail - env(mpt::set(alice, id, std::nullopt), - txflags(tfMPTLock | tfMPTUnlock), - ter(temINVALID_FLAG)); - env.close(); - - // if the holder is the same as the acct that submitted the tx, tx - // fails - env(mpt::set(alice, id, alice), - txflags(tfMPTLock), - ter(temMALFORMED)); - env.close(); + mptAlice.set( + {.account = &alice, + .flags = tfMPTLock | tfMPTUnlock, + .err = temINVALID_FLAG}); + + // if the holder is the same as the acct that submitted the tx, + // tx fails + mptAlice.set( + {.account = &alice, + .holder = &alice, + .flags = tfMPTLock, + .err = temMALFORMED}); } // Validate fields in MPTokenIssuanceSet (preclaim) // test when a mptokenissuance has disabled locking { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - Account const cindy("cindy"); - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mptIssuance = keylet::mptIssuance(id); - - env(mpt::create(alice)); // no locking - env.close(); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - BEAST_EXPECT(checkMPTokenIssuanceFlags(env, mptIssuance.key, 0)); + mptAlice.create({.ownerCount = 1}); - BEAST_EXPECT(env.ownerCount(alice) == 1); + BEAST_EXPECT(mptAlice.checkFlags(0)); // alice tries to lock a mptissuance that has disabled locking - env(mpt::set(alice, id, std::nullopt), - txflags(tfMPTLock), - ter(tecNO_PERMISSION)); - env.close(); + mptAlice.set( + {.account = &alice, + .flags = tfMPTLock, + .err = tecNO_PERMISSION}); // alice tries to unlock mptissuance that has disabled locking - env(mpt::set(alice, id, std::nullopt), - txflags(tfMPTUnlock), - ter(tecNO_PERMISSION)); - env.close(); - - // issuer tries to lock a bob's mptoken that has disabled locking - env(mpt::set(alice, id, bob), - txflags(tfMPTLock), - ter(tecNO_PERMISSION)); - env.close(); - - // issuer tries to unlock a bob's mptoken that has disabled locking - env(mpt::set(alice, id, bob), - txflags(tfMPTUnlock), - ter(tecNO_PERMISSION)); - env.close(); + mptAlice.set( + {.account = &alice, + .flags = tfMPTUnlock, + .err = tecNO_PERMISSION}); + + // issuer tries to lock a bob's mptoken that has disabled + // locking + mptAlice.set( + {.account = &alice, + .holder = &bob, + .flags = tfMPTLock, + .err = tecNO_PERMISSION}); + + // issuer tries to unlock a bob's mptoken that has disabled + // locking + mptAlice.set( + {.account = &alice, + .holder = &bob, + .flags = tfMPTUnlock, + .err = tecNO_PERMISSION}); } // Validate fields in MPTokenIssuanceSet (preclaim) // test when mptokenissuance has enabled locking { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - Account const cindy("cindy"); - - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - auto const badID = getMptID(alice.id(), env.seq(alice)); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); // alice trying to set when the mptissuance doesn't exist yet - env(mpt::set(alice, badID, std::nullopt), - txflags(tfMPTLock), - ter(tecOBJECT_NOT_FOUND)); - env.close(); - - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mptIssuance = keylet::mptIssuance(id); + mptAlice.set( + {.id = getMptID(alice.id(), env.seq(alice)), + .flags = tfMPTLock, + .err = tecOBJECT_NOT_FOUND}); // create a mptokenissuance with locking - env(mpt::create(alice), txflags(tfMPTCanLock)); - env.close(); - - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, mptIssuance.key, lsfMPTCanLock)); + mptAlice.create({.ownerCount = 1, .flags = tfMPTCanLock}); - BEAST_EXPECT(env.ownerCount(alice) == 1); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); // a non-issuer acct tries to set the mptissuance - env(mpt::set(bob, id, std::nullopt), - txflags(tfMPTLock), - ter(tecNO_PERMISSION)); - env.close(); + mptAlice.set( + {.account = &bob, .flags = tfMPTLock, .err = tecNO_PERMISSION}); // trying to set a holder who doesn't have a mptoken - env(mpt::set(alice, id, bob), - txflags(tfMPTLock), - ter(tecOBJECT_NOT_FOUND)); - env.close(); + mptAlice.set( + {.holder = &bob, + .flags = tfMPTLock, + .err = tecOBJECT_NOT_FOUND}); // trying to set a holder who doesn't exist - env(mpt::set(alice, id, cindy), txflags(tfMPTLock), ter(tecNO_DST)); - env.close(); + mptAlice.set( + {.holder = &cindy, .flags = tfMPTLock, .err = tecNO_DST}); } } @@ -828,128 +649,91 @@ class MPToken_test : public beast::unit_test::suite Account const alice("alice"); // issuer Account const bob("bob"); // holder - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - - auto const id = getMptID(alice.id(), env.seq(alice)); - auto const mptIssuance = keylet::mptIssuance(id); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); // create a mptokenissuance with locking - env(mpt::create(alice), txflags(tfMPTCanLock)); - env.close(); + mptAlice.create({.flags = tfMPTCanLock}); - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, mptIssuance.key, lsfMPTCanLock)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); BEAST_EXPECT(env.ownerCount(alice) == 1); BEAST_EXPECT(env.ownerCount(bob) == 0); - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); BEAST_EXPECT(env.ownerCount(bob) == 1); - env.close(); // both the mptissuance and mptoken are not locked - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, mptIssuance.key, lsfMPTCanLock)); - BEAST_EXPECT(checkMPTokenFlags(env, mptIssuance.key, bob, 0)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); // locks bob's mptoken - env(mpt::set(alice, id, bob), txflags(tfMPTLock)); - env.close(); + mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, mptIssuance.key, lsfMPTCanLock)); - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); // trying to lock bob's mptoken again will still succeed // but no changes to the objects - env(mpt::set(alice, id, bob), txflags(tfMPTLock)); - env.close(); + mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); // no changes to the objects - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, mptIssuance.key, lsfMPTCanLock)); - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); // alice locks the mptissuance - env(mpt::set(alice, id, std::nullopt), txflags(tfMPTLock)); - env.close(); + mptAlice.set({.account = &alice, .flags = tfMPTLock}); // now both the mptissuance and mptoken are locked up - BEAST_EXPECT(checkMPTokenIssuanceFlags( - env, mptIssuance.key, lsfMPTCanLock | lsfMPTLocked)); - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock | lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); // alice tries to lock up both mptissuance and mptoken again // it will not change the flags and both will remain locked. - env(mpt::set(alice, id, std::nullopt), txflags(tfMPTLock)); - env.close(); - env(mpt::set(alice, id, bob), txflags(tfMPTLock)); - env.close(); + mptAlice.set({.account = &alice, .flags = tfMPTLock}); + mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); // now both the mptissuance and mptoken remain locked up - BEAST_EXPECT(checkMPTokenIssuanceFlags( - env, mptIssuance.key, lsfMPTCanLock | lsfMPTLocked)); - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock | lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); // alice unlocks bob's mptoken - env(mpt::set(alice, id, bob), txflags(tfMPTUnlock)); - env.close(); + mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTUnlock}); // only mptissuance is locked - BEAST_EXPECT(checkMPTokenIssuanceFlags( - env, mptIssuance.key, lsfMPTCanLock | lsfMPTLocked)); - BEAST_EXPECT(checkMPTokenFlags(env, mptIssuance.key, bob, 0)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock | lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); // locks up bob's mptoken again - env(mpt::set(alice, id, bob), txflags(tfMPTLock)); - env.close(); + mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); // now both the mptissuance and mptokens are locked up - BEAST_EXPECT(checkMPTokenIssuanceFlags( - env, mptIssuance.key, lsfMPTCanLock | lsfMPTLocked)); - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock | lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); // alice unlocks mptissuance - env(mpt::set(alice, id, std::nullopt), txflags(tfMPTUnlock)); - env.close(); + mptAlice.set({.account = &alice, .flags = tfMPTUnlock}); // now mptissuance is unlocked - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, mptIssuance.key, lsfMPTCanLock)); - BEAST_EXPECT( - checkMPTokenFlags(env, mptIssuance.key, bob, lsfMPTLocked)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); // alice unlocks bob's mptoken - env(mpt::set(alice, id, bob), txflags(tfMPTUnlock)); - env.close(); + mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTUnlock}); // both mptissuance and bob's mptoken are unlocked - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, mptIssuance.key, lsfMPTCanLock)); - BEAST_EXPECT(checkMPTokenFlags(env, mptIssuance.key, bob, 0)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); // alice unlocks mptissuance and bob's mptoken again despite that // they are already unlocked. Make sure this will not change the // flags - env(mpt::set(alice, id, bob), txflags(tfMPTUnlock)); - env.close(); - env(mpt::set(alice, id, std::nullopt), txflags(tfMPTUnlock)); - env.close(); + mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTUnlock}); + mptAlice.set({.account = &alice, .flags = tfMPTUnlock}); // both mptissuance and bob's mptoken remain unlocked - BEAST_EXPECT( - checkMPTokenIssuanceFlags(env, mptIssuance.key, lsfMPTCanLock)); - BEAST_EXPECT(checkMPTokenFlags(env, mptIssuance.key, bob, 0)); + BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); } void @@ -958,23 +742,15 @@ class MPToken_test : public beast::unit_test::suite testcase("Payment"); using namespace test::jtx; + Account const alice("alice"); // issuer + Account const bob("bob"); // holder + Account const carol("carol"); // holder { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - Account const carol("carol"); // holder - env.fund(XRP(10000), alice, bob, carol); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); + MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); - auto const seq = env.seq(alice); - auto const id = getMptID(alice.id(), seq); - auto const mpt = ripple::MPT(seq, alice.id()); - - env(mpt::create(alice)); - env.close(); + mptAlice.create(); BEAST_EXPECT(env.ownerCount(alice) == 1); BEAST_EXPECT(env.ownerCount(bob) == 0); @@ -983,127 +759,78 @@ class MPToken_test : public beast::unit_test::suite // env(mpt::authorize(alice, id.key, std::nullopt)); // env.close(); - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); - env(mpt::authorize(carol, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); + mptAlice.authorize({.account = &carol}); // issuer to holder - env(pay( - alice, bob, ripple::test::jtx::MPT(alice.name(), mpt)(100))); - env.close(); - BEAST_EXPECT( - checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 100)); - BEAST_EXPECT(checkMPTokenOutstandingAmount( - env, keylet::mptIssuance(id).key, 100)); + mptAlice.pay(alice, bob, 100); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); + BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(100)); // holder to issuer - env(pay(bob, alice, ripple::test::jtx::MPT(bob.name(), mpt)(100))); - env.close(); - BEAST_EXPECT( - checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 0)); - BEAST_EXPECT(checkMPTokenOutstandingAmount( - env, keylet::mptIssuance(id).key, 0)); + mptAlice.pay(bob, alice, 100); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); + BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(0)); // holder to holder - env(pay( - alice, bob, ripple::test::jtx::MPT(alice.name(), mpt)(100))); - env(pay(bob, carol, ripple::test::jtx::MPT(alice.name(), mpt)(50))); - env.close(); - BEAST_EXPECT( - checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 50)); - BEAST_EXPECT(checkMPTokenAmount( - env, keylet::mptIssuance(id).key, carol, 50)); - BEAST_EXPECT(checkMPTokenOutstandingAmount( - env, keylet::mptIssuance(id).key, 100)); + mptAlice.pay(alice, bob, 100); + mptAlice.pay(bob, carol, 50); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 50)); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(carol, 50)); + BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(100)); } // If allowlisting is enabled, Payment fails if the receiver is not // authorized { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - auto const seq = env.seq(alice); - auto const id = getMptID(alice.id(), seq); - auto const mpt = ripple::MPT(seq, alice.id()); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - env(mpt::create(alice), txflags(tfMPTRequireAuth)); - env.close(); + mptAlice.create({.flags = tfMPTRequireAuth}); BEAST_EXPECT(env.ownerCount(alice) == 1); BEAST_EXPECT(env.ownerCount(bob) == 0); - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); - env(pay(alice, bob, ripple::test::jtx::MPT(alice.name(), mpt)(100)), - ter(tecNO_AUTH)); - env.close(); - BEAST_EXPECT( - checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 0)); - BEAST_EXPECT(checkMPTokenOutstandingAmount( - env, keylet::mptIssuance(id).key, 0)); + mptAlice.pay(alice, bob, 100, tecNO_AUTH); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); + BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(0)); } // If allowlisting is enabled, Payment fails if the sender is not // authorized { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - - env.fund(XRP(10000), alice, bob); - env.close(); - - BEAST_EXPECT(env.ownerCount(alice) == 0); - auto const seq = env.seq(alice); - auto const id = getMptID(alice.id(), seq); - auto const mpt = ripple::MPT(seq, alice.id()); + MPTTester mptAlice(env, alice, {.holders = {&bob}}); - env(mpt::create(alice), txflags(tfMPTRequireAuth)); - env.close(); + mptAlice.create({.flags = tfMPTRequireAuth}); BEAST_EXPECT(env.ownerCount(alice) == 1); BEAST_EXPECT(env.ownerCount(bob) == 0); // bob creates an empty MPToken - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); // alice authorizes bob to hold funds - env(mpt::authorize(alice, id, bob)); - env.close(); + mptAlice.authorize({.account = &alice, .holder = &bob}); // alice sends 100 MPT to bob - env(pay( - alice, bob, ripple::test::jtx::MPT(alice.name(), mpt)(100))); - env.close(); - BEAST_EXPECT( - checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 100)); - BEAST_EXPECT(checkMPTokenOutstandingAmount( - env, keylet::mptIssuance(id).key, 100)); + mptAlice.pay(alice, bob, 100); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); + BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(100)); // alice UNAUTHORIZES bob - env(mpt::authorize(alice, id, bob), txflags(tfMPTUnauthorize)); - env.close(); - - // bob fails to send back to alice because he is no longer authorize - // to move his funds! - env(pay(bob, alice, ripple::test::jtx::MPT(bob.name(), mpt)(100)), - ter(tecNO_AUTH)); - env.close(); - BEAST_EXPECT( - checkMPTokenAmount(env, keylet::mptIssuance(id).key, bob, 100)); - BEAST_EXPECT(checkMPTokenOutstandingAmount( - env, keylet::mptIssuance(id).key, 100)); + mptAlice.authorize( + {.account = &alice, .holder = &bob, .flags = tfMPTUnauthorize}); + + // bob fails to send back to alice because he is no longer + // authorize to move his funds! + mptAlice.pay(bob, alice, 100, tecNO_AUTH); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); + BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(100)); } } @@ -1115,19 +842,11 @@ class MPToken_test : public beast::unit_test::suite Env env{*this, features}; Account const alice("alice"); // issuer - env.fund(XRP(10000), alice); - env.close(); - - auto const mpt = ripple::MPT(env.seq(alice), alice.id()); + MPTTester mptAlice(env, alice); - env(mpt::create(alice)); - env.close(); + mptAlice.create(); - env(offer( - alice, - ripple::test::jtx::MPT(alice.name(), mpt)(100), - XRP(100)), - ter(temINVALID)); + env(offer(alice, mptAlice.mpt(100), XRP(100)), ter(temINVALID)); env.close(); BEAST_EXPECT(expectOffers(env, alice, 0)); @@ -1145,13 +864,9 @@ class MPToken_test : public beast::unit_test::suite Account const alice{"alice"}; Env env{*this, features}; - env.fund(XRP(10000), alice); - env.close(); - - auto const id = getMptID(alice.id(), env.seq(alice)); + MPTTester mptAlice(env, alice); - env(mpt::create(alice)); - env.close(); + mptAlice.create(); std::string const txHash{ env.tx()->getJson(JsonOptions::none)[jss::hash].asString()}; @@ -1160,7 +875,8 @@ class MPToken_test : public beast::unit_test::suite // Expect mpt_issuance_id field BEAST_EXPECT(meta.isMember(jss::mpt_issuance_id)); - BEAST_EXPECT(meta[jss::mpt_issuance_id] == to_string(id)); + BEAST_EXPECT( + meta[jss::mpt_issuance_id] == to_string(mptAlice.issuanceID())); } void @@ -1177,13 +893,9 @@ class MPToken_test : public beast::unit_test::suite Env env{*this, features}; Account const alice("alice"); // issuer - env.fund(XRP(10000), alice); - env.close(); - - auto const id = getMptID(alice.id(), env.seq(alice)); + MPTTester mptAlice(env, alice); - env(mpt::create(alice)); - env.close(); + mptAlice.create(); // create accounts that will create MPTokens for (auto i = 0; i < expectCount; i++) @@ -1193,8 +905,7 @@ class MPToken_test : public beast::unit_test::suite env.close(); // a holder creates a mptoken - env(mpt::authorize(bob, id, std::nullopt)); - env.close(); + mptAlice.authorize({.account = &bob}); } // Checks mpt_holder query responses @@ -1203,12 +914,14 @@ class MPToken_test : public beast::unit_test::suite Json::Value allHolders(Json::arrayValue); std::string marker; - // The do/while collects results until no marker is returned. + // The do/while collects results until no marker is + // returned. do { - Json::Value mptHolders = [&env, &id, &marker]() { + Json::Value mptHolders = [&env, &mptAlice, &marker]() { Json::Value params; - params[jss::mpt_issuance_id] = to_string(id); + params[jss::mpt_issuance_id] = + to_string(mptAlice.issuanceID()); if (!marker.empty()) params[jss::marker] = marker; @@ -1365,7 +1078,8 @@ class MPToken_test : public beast::unit_test::suite testMPTInvalidInTx(all); // Test parsed MPTokenIssuanceID in API response metadata - // TODO: This test exercises the parsing logic of mptID in `tx`, but, + // TODO: This test exercises the parsing logic of mptID in `tx`, + // but, // mptID is also parsed in different places like `account_tx`, // `subscribe`, `ledger`. We should create test for these // occurances (lower prioirity). diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 5653d3e70fc..697cfef1fd8 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -25,8 +25,6 @@ namespace ripple { namespace test { namespace jtx { -namespace mpt { - static std::array uint64ToByteArray(std::uint64_t value) { @@ -36,77 +34,190 @@ uint64ToByteArray(std::uint64_t value) return result; } -Json::Value -create(jtx::Account const& account) +std::unordered_map +MPTTester::makeHolders(std::vector const& holders) { - Json::Value jv; - jv[sfAccount.jsonName] = account.human(); - jv[sfTransactionType.jsonName] = jss::MPTokenIssuanceCreate; - return jv; + std::unordered_map accounts; + for (auto const& h : holders) + { + assert(h && holders_.find(h->human()) == accounts.cend()); + accounts.emplace(h->human(), h); + } + return accounts; } -Json::Value -create( - jtx::Account const& account, - std::uint64_t const maxAmt, - std::uint8_t const assetScale, - std::uint16_t transferFee, - std::string metadata) +MPTTester::MPTTester(Env& env, Account const& issuer, MPTConstr const& arg) + : env_(env) + , issuer_(issuer) + , holders_(makeHolders(arg.holders)) + , close_(arg.close) { + if (arg.fund) + { + env_.fund(arg.xrp, issuer_); + for (auto it : holders_) + env_.fund(arg.xrpHolders, *it.second); + } + if (close_) + env.close(); + if (arg.fund) + { + env_.require(owners(issuer_, 0)); + for (auto it : holders_) + { + assert(issuer_.id() != it.second->id()); + env_.require(owners(*it.second, 0)); + } + } +} + +void +MPTTester::create(const MPTCreate& arg) +{ + if (sequence_) + Throw("MPT can't be reused"); + sequence_ = env_.seq(issuer_); + id_ = getMptID(issuer_.id(), *sequence_); + issuanceID_ = keylet::mptIssuance(*id_).key; + mpt_ = std::make_pair(*sequence_, issuer_.id()); Json::Value jv; - jv[sfAccount.jsonName] = account.human(); + jv[sfAccount.jsonName] = issuer_.human(); jv[sfTransactionType.jsonName] = jss::MPTokenIssuanceCreate; - jv[sfAssetScale.jsonName] = assetScale; - jv[sfTransferFee.jsonName] = transferFee; - jv[sfMPTokenMetadata.jsonName] = strHex(metadata); + if (arg.assetScale) + jv[sfAssetScale.jsonName] = *arg.assetScale; + if (arg.transferFee) + jv[sfTransferFee.jsonName] = *arg.transferFee; + if (arg.metadata) + jv[sfMPTokenMetadata.jsonName] = strHex(*arg.metadata); // convert maxAmt to hex string, since json doesn't accept 64-bit int - jv[sfMaximumAmount.jsonName] = strHex(uint64ToByteArray(maxAmt)); - return jv; + if (arg.maxAmt) + jv[sfMaximumAmount.jsonName] = strHex(uint64ToByteArray(*arg.maxAmt)); + submit(arg, jv); } -Json::Value -destroy(jtx::Account const& account, uint192 const& id) +void +MPTTester::destroy(MPTDestroy const& arg) { Json::Value jv; - jv[sfAccount.jsonName] = account.human(); - jv[sfMPTokenIssuanceID.jsonName] = to_string(id); + if (arg.issuer) + jv[sfAccount.jsonName] = arg.issuer->human(); + else + jv[sfAccount.jsonName] = issuer_.human(); + if (arg.id) + jv[sfMPTokenIssuanceID.jsonName] = to_string(*arg.id); + else + jv[sfMPTokenIssuanceID.jsonName] = to_string(*id_); jv[sfTransactionType.jsonName] = jss::MPTokenIssuanceDestroy; - return jv; + submit(arg, jv); } -Json::Value -authorize( - jtx::Account const& account, - uint192 const& issuanceID, - std::optional const& holder) +Account const& +MPTTester::holder(std::string const& holder_) const +{ + auto const& it = holders_.find(holder_); + assert(it != holders_.cend()); + if (it == holders_.cend()) + Throw("Holder is not found"); + return *it->second; +} + +void +MPTTester::authorize(MPTAuthorize const& arg) { Json::Value jv; - jv[sfAccount.jsonName] = account.human(); + if (arg.account) + jv[sfAccount.jsonName] = arg.account->human(); + else + jv[sfAccount.jsonName] = issuer_.human(); jv[sfTransactionType.jsonName] = jss::MPTokenAuthorize; - jv[sfMPTokenIssuanceID.jsonName] = to_string(issuanceID); - if (holder) - jv[sfMPTokenHolder.jsonName] = holder->human(); - - return jv; + jv[sfMPTokenIssuanceID.jsonName] = to_string(*id_); + if (arg.holder) + jv[sfMPTokenHolder.jsonName] = arg.holder->human(); + submit(arg, jv); } -Json::Value -set(jtx::Account const& account, - uint192 const& issuanceID, - std::optional const& holder) +void +MPTTester::set(MPTSet const& arg) { Json::Value jv; - jv[sfAccount.jsonName] = account.human(); + if (arg.account) + jv[sfAccount.jsonName] = arg.account->human(); + else + jv[sfAccount.jsonName] = issuer_.human(); jv[sfTransactionType.jsonName] = jss::MPTokenIssuanceSet; - jv[sfMPTokenIssuanceID.jsonName] = to_string(issuanceID); - if (holder) - jv[sfMPTokenHolder.jsonName] = holder->human(); + if (arg.id) + jv[sfMPTokenIssuanceID.jsonName] = to_string(*arg.id); + else + jv[sfMPTokenIssuanceID.jsonName] = to_string(*id_); + if (arg.holder) + jv[sfMPTokenHolder.jsonName] = arg.holder->human(); + submit(arg, jv); +} + +bool +MPTTester::forObject( + std::function const& cb, + AccountP holder_) const +{ + auto const key = [&]() { + if (holder_) + return keylet::mptoken(*issuanceID_, holder_->id()); + return keylet::mptIssuance(*issuanceID_); + }(); + if (auto const sle = env_.le(key)) + return cb(sle); + return false; +} - return jv; +[[nodiscard]] bool +MPTTester::checkMPTokenAmount( + Account const& holder_, + std::uint64_t expectedAmount) const +{ + return forObject( + [&](SLEP const& sle) { return expectedAmount == (*sle)[sfMPTAmount]; }, + &holder_); } -} // namespace mpt +[[nodiscard]] bool +MPTTester::checkMPTokenOutstandingAmount(std::uint64_t expectedAmount) const +{ + return forObject([&](SLEP const& sle) { + return expectedAmount == (*sle)[sfOutstandingAmount]; + }); +} + +[[nodiscard]] bool +MPTTester::checkFlags(uint32_t const expectedFlags, AccountP holder_) const +{ + return forObject( + [&](SLEP const& sle) { return expectedFlags == sle->getFlags(); }, + holder_); +} + +void +MPTTester::pay( + Account const& src, + Account const& dest, + std::uint64_t amount, + std::optional err) +{ + assert(mpt_); + if (err) + env_(jtx::pay(src, dest, mpt(amount)), ter(*err)); + else + env_(jtx::pay(src, dest, mpt(amount))); + if (close_) + env_.close(); +} + +PrettyAmount +MPTTester::mpt(std::uint64_t amount) const +{ + assert(mpt_); + return ripple::test::jtx::MPT(issuer_.name(), *mpt_)(amount); +} } // namespace jtx } // namespace test diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 7f77c89ca34..210bb71e261 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -20,7 +20,9 @@ #ifndef RIPPLE_TEST_JTX_MPT_H_INCLUDED #define RIPPLE_TEST_JTX_MPT_H_INCLUDED -#include +#include +#include +#include #include @@ -28,38 +30,167 @@ namespace ripple { namespace test { namespace jtx { -namespace mpt { - -/** Issue a MPT with default fields. */ -Json::Value -create(jtx::Account const& account); - -/** Issue a MPT with user-defined fields. */ -Json::Value -create( - jtx::Account const& account, - std::uint64_t const maxAmt, - std::uint8_t const assetScale, - std::uint16_t transferFee, - std::string metadata); - -/** Destroy a MPT. */ -Json::Value -destroy(Account const& account, uint192 const& id); - -/** Authorize a MPT. */ -Json::Value -authorize( - jtx::Account const& account, - uint192 const& issuanceID, - std::optional const& holder); - -/** Set a MPT. */ -Json::Value -set(jtx::Account const& account, - uint192 const& issuanceID, - std::optional const& holder); -} // namespace mpt +namespace { +using AccountP = Account const*; +} + +struct MPTConstr +{ + std::vector holders = {}; + PrettyAmount const& xrp = XRP(10'000); + PrettyAmount const& xrpHolders = XRP(10'000); + bool fund = true; + bool close = true; +}; + +struct MPTCreate +{ + std::optional maxAmt = std::nullopt; + std::optional assetScale = std::nullopt; + std::optional transferFee = std::nullopt; + std::optional metadata = std::nullopt; + std::optional ownerCount = std::nullopt; + bool fund = true; + std::uint32_t flags = 0; + std::optional err = std::nullopt; +}; + +struct MPTDestroy +{ + AccountP issuer = nullptr; + std::optional id = std::nullopt; + std::optional ownerCount = std::nullopt; + std::uint32_t flags = 0; + std::optional err = std::nullopt; +}; + +struct MPTAuthorize +{ + AccountP account = nullptr; + AccountP holder = nullptr; + std::optional ownerCount = std::nullopt; + std::uint32_t flags = 0; + std::optional err = std::nullopt; +}; + +struct MPTSet +{ + AccountP account = nullptr; + AccountP holder = nullptr; + std::optional id = std::nullopt; + std::optional ownerCount = std::nullopt; + std::uint32_t flags = 0; + std::optional err = std::nullopt; +}; + +class MPTTester +{ + Env& env_; + Account const& issuer_; + std::unordered_map const holders_; + std::optional sequence_; + std::optional id_; + std::optional issuanceID_; + std::optional mpt_; + bool close_; + +public: + MPTTester(Env& env, Account const& issuer, MPTConstr const& constr = {}); + + void + create(MPTCreate const& arg = MPTCreate{}); + + void + destroy(MPTDestroy const& arg = MPTDestroy{}); + + void + authorize(MPTAuthorize const& arg = MPTAuthorize{}); + + void + set(MPTSet const& set = {}); + + [[nodiscard]] bool + checkMPTokenAmount(Account const& holder, std::uint64_t expectedAmount) + const; + + [[nodiscard]] bool + checkMPTokenOutstandingAmount(std::uint64_t expectedAmount) const; + + [[nodiscard]] bool + checkFlags(uint32_t const expectedFlags, AccountP holder = nullptr) const; + + Account const& + issuer() const + { + return issuer_; + } + Account const& + holder(std::string const& h) const; + + void + pay(Account const& src, + Account const& dest, + std::uint64_t amount, + std::optional err = std::nullopt); + + PrettyAmount + mpt(std::uint64_t amount) const; + + uint256 const& + issuanceKey() const + { + assert(issuanceID_); + return *issuanceID_; + } + + uint192 const& + issuanceID() const + { + assert(id_); + return *id_; + } + +private: + using SLEP = std::shared_ptr; + bool + forObject( + std::function const& cb, + AccountP holder = nullptr) const; + + template + void + submit(A const& arg, Json::Value const& jv) + { + if (arg.err) + { + if (arg.flags) + env_(jv, txflags(arg.flags), ter(*arg.err)); + else + env_(jv, ter(*arg.err)); + } + else if (arg.flags) + env_(jv, txflags(arg.flags)); + else + env_(jv); + if constexpr (std::is_same_v) + { + if (env_.ter() != tesSUCCESS) + { + sequence_.reset(); + id_.reset(); + issuanceID_.reset(); + mpt_.reset(); + } + } + if (close_) + env_.close(); + if (arg.ownerCount) + env_.require(owners(issuer_, *arg.ownerCount)); + } + + std::unordered_map + makeHolders(std::vector const& holders); +}; } // namespace jtx } // namespace test diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 44175341ce9..b7fcb670ed5 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -904,14 +904,14 @@ class AccountObjects_test : public beast::unit_test::suite } { // alice creates MPToken that is going to be used by gw - auto const issuanceID = getMptID(alice, env.seq(alice)); - env(mpt::create(alice)); - env.close(); + MPTTester mptAlice(env, alice, {.fund = false}); + mptAlice.create(); // gw creates a MPToken that we can look for in the ledger. Json::Value jvMPToken; jvMPToken[jss::TransactionType] = jss::MPTokenAuthorize; - jvMPToken[jss::MPTokenIssuanceID] = to_string(issuanceID); + jvMPToken[jss::MPTokenIssuanceID] = + to_string(mptAlice.issuanceID()); jvMPToken[jss::Account] = gw.human(); env(jvMPToken); env.close(); @@ -921,7 +921,8 @@ class AccountObjects_test : public beast::unit_test::suite BEAST_EXPECT(acct_objs_is_size(resp, 1)); auto const& mptoken = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT( - mptoken[sfMPTokenIssuanceID.jsonName] == to_string(issuanceID)); + mptoken[sfMPTokenIssuanceID.jsonName] == + to_string(mptAlice.issuanceID())); BEAST_EXPECT(mptoken[sfAccount.jsonName] == gw.human()); } // Make gw multisigning by adding a signerList. diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 04e88ffda13..44eb1c3cb33 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -1590,19 +1590,16 @@ class LedgerRPC_test : public beast::unit_test::suite Env env{*this}; Account const alice{"alice"}; - env.fund(XRP(10000), alice); - env.close(); + MPTTester mptAlice(env, alice); - auto const id = getMptID(alice, env.seq(alice)); - env(mpt::create(alice)); - env.close(); + mptAlice.create(); std::string const ledgerHash{to_string(env.closed()->info().hash)}; { // Request the MPTokenIssuance using its ID. Json::Value jvParams; - jvParams[jss::mpt_issuance_id] = to_string(id); + jvParams[jss::mpt_issuance_id] = to_string(mptAlice.issuanceID()); jvParams[jss::ledger_hash] = ledgerHash; Json::Value const jrr = env.rpc( "json", "ledger_entry", to_string(jvParams))[jss::result]; From 72b7bbc4966944eab8de603eb345c26e896cdbb6 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 10 Jan 2024 11:40:09 -0500 Subject: [PATCH 13/44] Change issuanceID_ to issuanceKey_ --- src/test/jtx/impl/mpt.cpp | 6 +++--- src/test/jtx/mpt.h | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 697cfef1fd8..8e811813c6e 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -78,7 +78,7 @@ MPTTester::create(const MPTCreate& arg) Throw("MPT can't be reused"); sequence_ = env_.seq(issuer_); id_ = getMptID(issuer_.id(), *sequence_); - issuanceID_ = keylet::mptIssuance(*id_).key; + issuanceKey_ = keylet::mptIssuance(*id_).key; mpt_ = std::make_pair(*sequence_, issuer_.id()); Json::Value jv; jv[sfAccount.jsonName] = issuer_.human(); @@ -162,8 +162,8 @@ MPTTester::forObject( { auto const key = [&]() { if (holder_) - return keylet::mptoken(*issuanceID_, holder_->id()); - return keylet::mptIssuance(*issuanceID_); + return keylet::mptoken(*issuanceKey_, holder_->id()); + return keylet::mptIssuance(*issuanceKey_); }(); if (auto const sle = env_.le(key)) return cb(sle); diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 210bb71e261..ad874eaa9e3 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -90,7 +90,7 @@ class MPTTester std::unordered_map const holders_; std::optional sequence_; std::optional id_; - std::optional issuanceID_; + std::optional issuanceKey_; std::optional mpt_; bool close_; @@ -139,8 +139,8 @@ class MPTTester uint256 const& issuanceKey() const { - assert(issuanceID_); - return *issuanceID_; + assert(issuanceKey_); + return *issuanceKey_; } uint192 const& @@ -178,7 +178,7 @@ class MPTTester { sequence_.reset(); id_.reset(); - issuanceID_.reset(); + issuanceKey_.reset(); mpt_.reset(); } } From 9f91bb2621e30120ee5bcef7d532e6b72ab4f8f2 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 10 Jan 2024 14:19:33 -0500 Subject: [PATCH 14/44] Bundle owner/holderCount into the transaction calls. --- src/test/app/MPToken_test.cpp | 108 +++++++++++----------------------- src/test/jtx/mpt.h | 9 +++ 2 files changed, 42 insertions(+), 75 deletions(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index c26df7b7b41..9bcba7a094f 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -163,14 +163,13 @@ class MPToken_test : public beast::unit_test::suite .assetScale = 1, .transferFee = 10, .metadata = "123", + .ownerCount = 1, .flags = tfMPTCanLock | tfMPTRequireAuth | tfMPTCanEscrow | tfMPTCanTrade | tfMPTCanTransfer | tfMPTCanClawback}); BEAST_EXPECT(mptAlice.checkFlags( lsfMPTCanLock | lsfMPTRequireAuth | lsfMPTCanEscrow | lsfMPTCanTrade | lsfMPTCanTransfer | lsfMPTCanClawback)); - - BEAST_EXPECT(env.ownerCount(alice) == 1); } } @@ -212,9 +211,7 @@ class MPToken_test : public beast::unit_test::suite // outstanding balance { // bob now holds a mptoken object - mptAlice.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 1); + mptAlice.authorize({.account = &bob, .holderCount = 1}); // alice pays bob 100 tokens mptAlice.pay(alice, bob, 100); @@ -305,9 +302,7 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize({.holder = &bob, .err = tecNO_AUTH}); // bob now holds a mptoken object - mptAlice.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 1); + mptAlice.authorize({.account = &bob, .holderCount = 1}); // bob cannot create the mptoken the second time mptAlice.authorize({.account = &bob, .err = tecMPTOKEN_EXISTS}); @@ -336,9 +331,8 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize( {.account = &bob, .flags = tfMPTUnauthorize, + .holderCount = 0, .err = tecNO_ENTRY}); - - BEAST_EXPECT(env.ownerCount(bob) == 0); } // Test bad scenarios with allow-listing in MPTokenAuthorize (preclaim) @@ -364,9 +358,7 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize({.holder = &cindy, .err = tecNO_DST}); // bob now holds a mptoken object - mptAlice.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 1); + mptAlice.authorize({.account = &bob, .holderCount = 1}); BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); @@ -388,9 +380,8 @@ class MPToken_test : public beast::unit_test::suite BEAST_EXPECT(mptAlice.checkFlags(lsfMPTAuthorized, &bob)); // bob deletes his mptoken - mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); - - BEAST_EXPECT(env.ownerCount(bob) == 0); + mptAlice.authorize( + {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize}); } // Test mptoken reserve requirement - first two mpts free (doApply) @@ -410,18 +401,13 @@ class MPToken_test : public beast::unit_test::suite mptAlice2.create(); MPTTester mptAlice3(env, alice, {.fund = false}); - mptAlice3.create(); - - BEAST_EXPECT(env.ownerCount(alice) == 3); + mptAlice3.create({.ownerCount = 3}); // first mpt for free - mptAlice1.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 1); + mptAlice1.authorize({.account = &bob, .holderCount = 1}); // second mpt free - mptAlice2.authorize({.account = &bob}); - BEAST_EXPECT(env.ownerCount(bob) == 2); + mptAlice2.authorize({.account = &bob, .holderCount = 2}); mptAlice3.authorize( {.account = &bob, .err = tecINSUFFICIENT_RESERVE}); @@ -430,9 +416,7 @@ class MPToken_test : public beast::unit_test::suite env.master, bob, drops(incReserve + incReserve + incReserve))); env.close(); - mptAlice3.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 3); + mptAlice3.authorize({.account = &bob, .holderCount = 3}); } } @@ -451,24 +435,19 @@ class MPToken_test : public beast::unit_test::suite // alice create mptissuance without allowisting MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.create(); + mptAlice.create({.ownerCount = 1}); BEAST_EXPECT(mptAlice.checkFlags(0)); - BEAST_EXPECT(env.ownerCount(alice) == 1); - // bob creates a mptoken - mptAlice.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 1); + mptAlice.authorize({.account = &bob, .holderCount = 1}); BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); // bob deletes his mptoken - mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); - - BEAST_EXPECT(env.ownerCount(bob) == 0); + mptAlice.authorize( + {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize}); } // With allowlisting @@ -478,16 +457,12 @@ class MPToken_test : public beast::unit_test::suite // alice creates a mptokenissuance that requires authorization MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.create({.flags = tfMPTRequireAuth}); + mptAlice.create({.ownerCount = 1, .flags = tfMPTRequireAuth}); BEAST_EXPECT(mptAlice.checkFlags(lsfMPTRequireAuth)); - BEAST_EXPECT(env.ownerCount(alice) == 1); - // bob creates a mptoken - mptAlice.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 1); + mptAlice.authorize({.account = &bob, .holderCount = 1}); BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); @@ -500,16 +475,16 @@ class MPToken_test : public beast::unit_test::suite // Unauthorize bob's mptoken mptAlice.authorize( - {.account = &alice, .holder = &bob, .flags = tfMPTUnauthorize}); + {.account = &alice, + .holder = &bob, + .holderCount = 1, + .flags = tfMPTUnauthorize}); // ensure bob's mptoken no longer has lsfMPTAuthorized set BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - BEAST_EXPECT(env.ownerCount(bob) == 1); - - mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); - - BEAST_EXPECT(env.ownerCount(bob) == 0); + mptAlice.authorize( + {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize}); } } @@ -531,16 +506,11 @@ class MPToken_test : public beast::unit_test::suite env.enableFeature(featureMPTokensV1); - mptAlice.create(); + mptAlice.create({.ownerCount = 1, .holderCount = 0}); BEAST_EXPECT(mptAlice.checkFlags(0)); - BEAST_EXPECT(env.ownerCount(alice) == 1); - BEAST_EXPECT(env.ownerCount(bob) == 0); - - mptAlice.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 1); + mptAlice.authorize({.account = &bob, .holderCount = 1}); // test invalid flag mptAlice.set( @@ -652,16 +622,12 @@ class MPToken_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {&bob}}); // create a mptokenissuance with locking - mptAlice.create({.flags = tfMPTCanLock}); + mptAlice.create( + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanLock}); BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - BEAST_EXPECT(env.ownerCount(alice) == 1); - BEAST_EXPECT(env.ownerCount(bob) == 0); - - mptAlice.authorize({.account = &bob}); - - BEAST_EXPECT(env.ownerCount(bob) == 1); + mptAlice.authorize({.account = &bob, .holderCount = 1}); // both the mptissuance and mptoken are not locked BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); @@ -750,11 +716,7 @@ class MPToken_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); - mptAlice.create(); - - BEAST_EXPECT(env.ownerCount(alice) == 1); - BEAST_EXPECT(env.ownerCount(bob) == 0); - BEAST_EXPECT(env.ownerCount(carol) == 0); + mptAlice.create({.ownerCount = 1, .holderCount = 0}); // env(mpt::authorize(alice, id.key, std::nullopt)); // env.close(); @@ -787,10 +749,8 @@ class MPToken_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.create({.flags = tfMPTRequireAuth}); - - BEAST_EXPECT(env.ownerCount(alice) == 1); - BEAST_EXPECT(env.ownerCount(bob) == 0); + mptAlice.create( + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTRequireAuth}); mptAlice.authorize({.account = &bob}); @@ -806,10 +766,8 @@ class MPToken_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.create({.flags = tfMPTRequireAuth}); - - BEAST_EXPECT(env.ownerCount(alice) == 1); - BEAST_EXPECT(env.ownerCount(bob) == 0); + mptAlice.create( + {.ownerCount = 1, .holderCount = 0, .flags = tfMPTRequireAuth}); // bob creates an empty MPToken mptAlice.authorize({.account = &bob}); diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index ad874eaa9e3..729d3cc7271 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -50,6 +50,7 @@ struct MPTCreate std::optional transferFee = std::nullopt; std::optional metadata = std::nullopt; std::optional ownerCount = std::nullopt; + std::optional holderCount = std::nullopt; bool fund = true; std::uint32_t flags = 0; std::optional err = std::nullopt; @@ -60,6 +61,7 @@ struct MPTDestroy AccountP issuer = nullptr; std::optional id = std::nullopt; std::optional ownerCount = std::nullopt; + std::optional holderCount = std::nullopt; std::uint32_t flags = 0; std::optional err = std::nullopt; }; @@ -69,6 +71,7 @@ struct MPTAuthorize AccountP account = nullptr; AccountP holder = nullptr; std::optional ownerCount = std::nullopt; + std::optional holderCount = std::nullopt; std::uint32_t flags = 0; std::optional err = std::nullopt; }; @@ -79,6 +82,7 @@ struct MPTSet AccountP holder = nullptr; std::optional id = std::nullopt; std::optional ownerCount = std::nullopt; + std::optional holderCount = std::nullopt; std::uint32_t flags = 0; std::optional err = std::nullopt; }; @@ -186,6 +190,11 @@ class MPTTester env_.close(); if (arg.ownerCount) env_.require(owners(issuer_, *arg.ownerCount)); + if (arg.holderCount) + { + for (auto it : holders_) + env_.require(owners(*it.second, *arg.holderCount)); + } } std::unordered_map From 2865aaed81a8d73d7d7e0065401d76f9a6388e31 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 10 Jan 2024 15:13:43 -0500 Subject: [PATCH 15/44] Move some asset methods to the source file * Remove unused sequence_ * Fix nullopt id_ * Fix Windows compile --- Builds/CMake/RippledCore.cmake | 1 + src/ripple/protocol/Asset.h | 68 ++++-------------------- src/ripple/protocol/impl/Asset.cpp | 84 ++++++++++++++++++++++++++++++ src/test/app/MPToken_test.cpp | 25 ++++++--- src/test/jtx/impl/mpt.cpp | 22 ++++++-- src/test/jtx/mpt.h | 3 +- 6 files changed, 132 insertions(+), 71 deletions(-) create mode 100644 src/ripple/protocol/impl/Asset.cpp diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index c371b59e1bd..84371a3dac4 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -75,6 +75,7 @@ target_sources (xrpl_core PRIVATE #]===============================] src/ripple/protocol/impl/AccountID.cpp src/ripple/protocol/impl/AMMCore.cpp + src/ripple/protocol/impl/Asset.cpp src/ripple/protocol/impl/Book.cpp src/ripple/protocol/impl/BuildInfo.cpp src/ripple/protocol/impl/ErrorCodes.cpp diff --git a/src/ripple/protocol/Asset.h b/src/ripple/protocol/Asset.h index 458cf629be3..355d9a9a856 100644 --- a/src/ripple/protocol/Asset.h +++ b/src/ripple/protocol/Asset.h @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ /* This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2023 Ripple Labs Inc. + Copyright (c) 2024 Ripple Labs Inc. Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above @@ -78,16 +78,7 @@ class Asset } void - addBitString(Serializer& s) const - { - if (isCurrency()) - s.addBitString(std::get(asset_)); - else - { - s.add32(std::get(asset_).first); - s.addBitString(std::get(asset_).second); - } - } + addBitString(Serializer& s) const; bool empty() const @@ -104,27 +95,15 @@ class Asset } template - requires(std::is_same_v || std::is_same_v) - T const* get() const + requires(std::is_same_v || std::is_same_v) + T const* get() const { return std::get_if(asset_); } - operator Currency const &() const - { - assert(std::holds_alternative(asset_)); - if (!std::holds_alternative(asset_)) - Throw("Invalid Currency cast"); - return std::get(asset_); - } + operator Currency const&() const; - operator MPT const &() const - { - assert(std::holds_alternative(asset_)); - if (!std::holds_alternative(asset_)) - Throw("Invalid MPT cast"); - return std::get(asset_); - } + operator MPT const&() const; friend constexpr bool comparable(Asset const& a1, Asset const& a2) @@ -172,37 +151,12 @@ class Asset { return !(a1 < a2); } - friend inline constexpr std::weak_ordering - operator<=>(Asset const& lhs, Asset const& rhs) - { - assert(lhs.isCurrency() == rhs.isCurrency()); - if (lhs.isCurrency() != rhs.isCurrency()) - Throw("Invalid Asset comparison"); - if (lhs.isCurrency()) - return std::get(lhs.asset_) <=> - std::get(rhs.asset_); - if (auto const c{ - std::get(lhs.asset_).second <=> - std::get(rhs.asset_).second}; - c != 0) - return c; - return std::get(lhs.asset_).first <=> - std::get(rhs.asset_).first; - } + friend constexpr std::weak_ordering + operator<=>(Asset const& lhs, Asset const& rhs); + friend std::string - to_string(Asset const& a) - { - if (a.isCurrency()) - return to_string((Currency&)a); - // TODO, common getMptID() - uint192 u; - auto const sequence = - boost::endian::native_to_big(std::get(a.asset_).first); - auto const& account = std::get(a.asset_).second; - memcpy(u.data(), &sequence, sizeof(sequence)); - memcpy(u.data() + sizeof(sequence), account.data(), sizeof(account)); - return to_string(u); - } + to_string(Asset const& a); + friend bool isXRP(Asset const& a) { diff --git a/src/ripple/protocol/impl/Asset.cpp b/src/ripple/protocol/impl/Asset.cpp new file mode 100644 index 00000000000..604e3316464 --- /dev/null +++ b/src/ripple/protocol/impl/Asset.cpp @@ -0,0 +1,84 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include + +namespace ripple { + +void +Asset::addBitString(ripple::Serializer& s) const +{ + if (isCurrency()) + s.addBitString(std::get(asset_)); + else + { + s.add32(std::get(asset_).first); + s.addBitString(std::get(asset_).second); + } +} + +Asset::operator Currency const&() const +{ + assert(std::holds_alternative(asset_)); + if (!std::holds_alternative(asset_)) + Throw("Invalid Currency cast"); + return std::get(asset_); +} + +Asset::operator MPT const&() const +{ + assert(std::holds_alternative(asset_)); + if (!std::holds_alternative(asset_)) + Throw("Invalid MPT cast"); + return std::get(asset_); +} + +constexpr std::weak_ordering +operator<=>(Asset const& lhs, Asset const& rhs) +{ + assert(lhs.isCurrency() == rhs.isCurrency()); + if (lhs.isCurrency() != rhs.isCurrency()) + Throw("Invalid Asset comparison"); + if (lhs.isCurrency()) + return std::get(lhs.asset_) <=> + std::get(rhs.asset_); + if (auto const c{ + std::get(lhs.asset_).second <=> + std::get(rhs.asset_).second}; + c != 0) + return c; + return std::get(lhs.asset_).first <=> std::get(rhs.asset_).first; +} + +std::string +to_string(Asset const& a) +{ + if (a.isCurrency()) + return to_string((Currency&)a); + // TODO, common getMptID() + uint192 u; + auto const sequence = + boost::endian::native_to_big(std::get(a.asset_).first); + auto const& account = std::get(a.asset_).second; + memcpy(u.data(), &sequence, sizeof(sequence)); + memcpy(u.data() + sizeof(sequence), account.data(), sizeof(account)); + return to_string(u); +} + +} // namespace ripple diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 9bcba7a094f..0fcb64ef21b 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -185,11 +185,13 @@ class MPToken_test : public beast::unit_test::suite { Env env{*this, features - featureMPTokensV1}; MPTTester mptAlice(env, alice); - mptAlice.destroy({.ownerCount = 0, .err = temDISABLED}); + auto const id = getMptID(alice, env.seq(alice)); + mptAlice.destroy({.ownerCount = 0, .id = id, .err = temDISABLED}); env.enableFeature(featureMPTokensV1); - mptAlice.destroy({.flags = 0x00000001, .err = temINVALID_FLAG}); + mptAlice.destroy( + {.flags = 0x00000001, .id = id, .err = temINVALID_FLAG}); } // MPTokenIssuanceDestroy (preclaim) @@ -253,7 +255,10 @@ class MPToken_test : public beast::unit_test::suite Env env{*this, features - featureMPTokensV1}; MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.authorize({.account = &bob, .err = temDISABLED}); + mptAlice.authorize( + {.account = &bob, + .id = getMptID(alice, env.seq(alice)), + .err = temDISABLED}); env.enableFeature(featureMPTokensV1); @@ -273,10 +278,13 @@ class MPToken_test : public beast::unit_test::suite { Env env{*this, features}; MPTTester mptAlice(env, alice, {.holders = {&bob}}); + auto const id = getMptID(alice, env.seq(alice)); - mptAlice.authorize({.holder = &bob, .err = tecOBJECT_NOT_FOUND}); + mptAlice.authorize( + {.holder = &bob, .id = id, .err = tecOBJECT_NOT_FOUND}); - mptAlice.authorize({.account = &bob, .err = tecOBJECT_NOT_FOUND}); + mptAlice.authorize( + {.account = &bob, .id = id, .err = tecOBJECT_NOT_FOUND}); } // Test bad scenarios without allowlisting in MPTokenAuthorize @@ -330,8 +338,8 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize( {.account = &bob, - .flags = tfMPTUnauthorize, .holderCount = 0, + .flags = tfMPTUnauthorize, .err = tecNO_ENTRY}); } @@ -502,7 +510,10 @@ class MPToken_test : public beast::unit_test::suite Env env{*this, features - featureMPTokensV1}; MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.set({.account = &bob, .err = temDISABLED}); + mptAlice.set( + {.account = &bob, + .id = getMptID(alice, env.seq(alice)), + .err = temDISABLED}); env.enableFeature(featureMPTokensV1); diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 8e811813c6e..e831e4e9546 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -74,12 +74,11 @@ MPTTester::MPTTester(Env& env, Account const& issuer, MPTConstr const& arg) void MPTTester::create(const MPTCreate& arg) { - if (sequence_) + if (issuanceKey_) Throw("MPT can't be reused"); - sequence_ = env_.seq(issuer_); - id_ = getMptID(issuer_.id(), *sequence_); + mpt_ = std::make_pair(env_.seq(issuer_), issuer_.id()); + id_ = getMptID(issuer_.id(), mpt_->first); issuanceKey_ = keylet::mptIssuance(*id_).key; - mpt_ = std::make_pair(*sequence_, issuer_.id()); Json::Value jv; jv[sfAccount.jsonName] = issuer_.human(); jv[sfTransactionType.jsonName] = jss::MPTokenIssuanceCreate; @@ -107,7 +106,10 @@ MPTTester::destroy(MPTDestroy const& arg) if (arg.id) jv[sfMPTokenIssuanceID.jsonName] = to_string(*arg.id); else + { + assert(id_); jv[sfMPTokenIssuanceID.jsonName] = to_string(*id_); + } jv[sfTransactionType.jsonName] = jss::MPTokenIssuanceDestroy; submit(arg, jv); } @@ -131,7 +133,13 @@ MPTTester::authorize(MPTAuthorize const& arg) else jv[sfAccount.jsonName] = issuer_.human(); jv[sfTransactionType.jsonName] = jss::MPTokenAuthorize; - jv[sfMPTokenIssuanceID.jsonName] = to_string(*id_); + if (arg.id) + jv[sfMPTokenIssuanceID.jsonName] = to_string(*arg.id); + else + { + assert(id_); + jv[sfMPTokenIssuanceID.jsonName] = to_string(*id_); + } if (arg.holder) jv[sfMPTokenHolder.jsonName] = arg.holder->human(); submit(arg, jv); @@ -149,7 +157,10 @@ MPTTester::set(MPTSet const& arg) if (arg.id) jv[sfMPTokenIssuanceID.jsonName] = to_string(*arg.id); else + { + assert(id_); jv[sfMPTokenIssuanceID.jsonName] = to_string(*id_); + } if (arg.holder) jv[sfMPTokenHolder.jsonName] = arg.holder->human(); submit(arg, jv); @@ -160,6 +171,7 @@ MPTTester::forObject( std::function const& cb, AccountP holder_) const { + assert(issuanceKey_); auto const key = [&]() { if (holder_) return keylet::mptoken(*issuanceKey_, holder_->id()); diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 729d3cc7271..135bee64aba 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -70,6 +70,7 @@ struct MPTAuthorize { AccountP account = nullptr; AccountP holder = nullptr; + std::optional id = std::nullopt; std::optional ownerCount = std::nullopt; std::optional holderCount = std::nullopt; std::uint32_t flags = 0; @@ -92,7 +93,6 @@ class MPTTester Env& env_; Account const& issuer_; std::unordered_map const holders_; - std::optional sequence_; std::optional id_; std::optional issuanceKey_; std::optional mpt_; @@ -180,7 +180,6 @@ class MPTTester { if (env_.ter() != tesSUCCESS) { - sequence_.reset(); id_.reset(); issuanceKey_.reset(); mpt_.reset(); From 984585e3a2e4c3431c1ca7525b29706257a96a35 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 10 Jan 2024 15:18:16 -0500 Subject: [PATCH 16/44] Fix clang format --- src/ripple/protocol/Asset.h | 8 ++++---- src/ripple/protocol/impl/Asset.cpp | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ripple/protocol/Asset.h b/src/ripple/protocol/Asset.h index 355d9a9a856..5da2215c0df 100644 --- a/src/ripple/protocol/Asset.h +++ b/src/ripple/protocol/Asset.h @@ -95,15 +95,15 @@ class Asset } template - requires(std::is_same_v || std::is_same_v) - T const* get() const + requires(std::is_same_v || std::is_same_v) + T const* get() const { return std::get_if(asset_); } - operator Currency const&() const; + operator Currency const &() const; - operator MPT const&() const; + operator MPT const &() const; friend constexpr bool comparable(Asset const& a1, Asset const& a2) diff --git a/src/ripple/protocol/impl/Asset.cpp b/src/ripple/protocol/impl/Asset.cpp index 604e3316464..ed484de4437 100644 --- a/src/ripple/protocol/impl/Asset.cpp +++ b/src/ripple/protocol/impl/Asset.cpp @@ -33,7 +33,7 @@ Asset::addBitString(ripple::Serializer& s) const } } -Asset::operator Currency const&() const +Asset::operator Currency const &() const { assert(std::holds_alternative(asset_)); if (!std::holds_alternative(asset_)) @@ -41,7 +41,7 @@ Asset::operator Currency const&() const return std::get(asset_); } -Asset::operator MPT const&() const +Asset::operator MPT const &() const { assert(std::holds_alternative(asset_)); if (!std::holds_alternative(asset_)) From 516f4bd05562ac09f313dccca0c87e1d28874627 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 10 Jan 2024 17:06:35 -0500 Subject: [PATCH 17/44] Fix Windows build --- src/test/app/MPToken_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 0fcb64ef21b..11a92e866eb 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -186,12 +186,12 @@ class MPToken_test : public beast::unit_test::suite Env env{*this, features - featureMPTokensV1}; MPTTester mptAlice(env, alice); auto const id = getMptID(alice, env.seq(alice)); - mptAlice.destroy({.ownerCount = 0, .id = id, .err = temDISABLED}); + mptAlice.destroy({.id = id, .ownerCount = 0, .err = temDISABLED}); env.enableFeature(featureMPTokensV1); mptAlice.destroy( - {.flags = 0x00000001, .id = id, .err = temINVALID_FLAG}); + {.id = id, .flags = 0x00000001, .err = temINVALID_FLAG}); } // MPTokenIssuanceDestroy (preclaim) From ae75af7c6e8e8094d2dfab55c98c427d52897305 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Thu, 11 Jan 2024 15:58:09 -0500 Subject: [PATCH 18/44] Handle MPT freeze * Bundle checkFlags into mpt create * Bundle amount balances check into mpt pay --- src/ripple/app/misc/NetworkOPs.cpp | 8 +- src/ripple/app/tx/impl/CreateCheck.cpp | 2 +- src/ripple/app/tx/impl/CreateOffer.cpp | 4 +- src/ripple/ledger/View.h | 14 +-- src/ripple/ledger/impl/View.cpp | 109 ++++++++++++----------- src/test/app/MPToken_test.cpp | 117 +++++-------------------- src/test/jtx/impl/mpt.cpp | 57 +++++++++++- src/test/jtx/mpt.h | 69 +++++++++++---- 8 files changed, 196 insertions(+), 184 deletions(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index baa6bead60c..3cb07588541 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -4332,8 +4332,8 @@ NetworkOPsImp::getBookPage( ReadView const& view = *lpLedger; - bool const bGlobalFreeze = isGlobalFrozen(view, book.out.account()) || - isGlobalFrozen(view, book.in.account()); + bool const bGlobalFreeze = + isGlobalFrozen(view, book.out) || isGlobalFrozen(view, book.in); bool bDone = false; bool bDirectAdvance = true; @@ -4530,8 +4530,8 @@ NetworkOPsImp::getBookPage( auto const rate = transferRate(lesActive, book.out.account); - const bool bGlobalFreeze = lesActive.isGlobalFrozen(book.out.account) || - lesActive.isGlobalFrozen(book.in.account); + const bool bGlobalFreeze = + lesActive.isGlobalFrozen(book.out) || lesActive.isGlobalFrozen(book.in); while (iLimit-- > 0 && obIterator.nextOffer()) { diff --git a/src/ripple/app/tx/impl/CreateCheck.cpp b/src/ripple/app/tx/impl/CreateCheck.cpp index 4542a57a6cc..8c8bb01cefb 100644 --- a/src/ripple/app/tx/impl/CreateCheck.cpp +++ b/src/ripple/app/tx/impl/CreateCheck.cpp @@ -115,7 +115,7 @@ CreateCheck::preclaim(PreclaimContext const& ctx) { // The currency may not be globally frozen AccountID const& issuerId{sendMax.getIssuer()}; - if (isGlobalFrozen(ctx.view, issuerId)) + if (isGlobalFrozen(ctx.view, sendMax.issue())) { JLOG(ctx.j.warn()) << "Creating a check for frozen asset"; return tecFROZEN; diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index a5bc020ac61..12ae77c65be 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -148,8 +148,8 @@ CreateOffer::preclaim(PreclaimContext const& ctx) auto viewJ = ctx.app.journal("View"); - if (isGlobalFrozen(ctx.view, uPaysIssuerID) || - isGlobalFrozen(ctx.view, uGetsIssuerID)) + if (isGlobalFrozen(ctx.view, saTakerPays.issue()) || + isGlobalFrozen(ctx.view, saTakerGets.issue())) { JLOG(ctx.j.debug()) << "Offer involves frozen asset"; return tecFROZEN; diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index 207c09a9c3c..b0a65cebdfd 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -79,23 +79,13 @@ hasExpired(ReadView const& view, std::optional const& exp); enum FreezeHandling { fhIGNORE_FREEZE, fhZERO_IF_FROZEN }; [[nodiscard]] bool -isGlobalFrozen(ReadView const& view, AccountID const& issuer); - -[[nodiscard]] bool -isIndividualFrozen( - ReadView const& view, - AccountID const& account, - Currency const& currency, - AccountID const& issuer); +isGlobalFrozen(ReadView const& view, Issue const& issue); [[nodiscard]] inline bool isIndividualFrozen( ReadView const& view, AccountID const& account, - Issue const& issue) -{ - return isIndividualFrozen(view, account, issue.asset(), issue.account()); -} + Issue const& issue); [[nodiscard]] bool isFrozen( diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index a24d1dda016..f9298248c07 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -168,11 +168,16 @@ hasExpired(ReadView const& view, std::optional const& exp) } bool -isGlobalFrozen(ReadView const& view, AccountID const& issuer) +isGlobalFrozen(ReadView const& view, Issue const& issue) { - if (isXRP(issuer)) + if (isXRP(issue)) return false; - if (auto const sle = view.read(keylet::account(issuer))) + if (issue.isMPT()) + { + if (auto const sle = view.read(keylet::mptIssuance(issue.asset()))) + return sle->getFlags() & lsfMPTLocked; + } + else if (auto const sle = view.read(keylet::account(issue.account()))) return sle->isFlag(lsfGlobalFreeze); return false; } @@ -181,18 +186,27 @@ bool isIndividualFrozen( ReadView const& view, AccountID const& account, - Currency const& currency, - AccountID const& issuer) + Issue const& issue) { - if (isXRP(currency)) + if (isXRP(issue)) return false; - if (issuer != account) + if (issue.account() != account) { + // Check if the issuer locked MPT + if (issue.isMPT()) + { + if (auto const sle = + view.read(keylet::mptoken(issue.asset(), account))) + return sle->getFlags() & lsfMPTLocked; + } // Check if the issuer froze the line - auto const sle = view.read(keylet::line(account, issuer, currency)); - if (sle && - sle->isFlag((issuer > account) ? lsfHighFreeze : lsfLowFreeze)) - return true; + else if ( + auto const sle = view.read( + keylet::line(account, issue.account(), issue.asset()))) + { + return sle->isFlag( + (issue.account() > account) ? lsfHighFreeze : lsfLowFreeze); + } } return false; } @@ -206,20 +220,12 @@ isFrozen( Asset const& asset, AccountID const& issuer) { - if (isXRP(asset) || asset.isMPT()) + Issue const issue = asset.isMPT() ? Issue{asset} : Issue{asset, issuer}; + if (isXRP(issue)) return false; - auto sle = view.read(keylet::account(issuer)); - if (sle && sle->isFlag(lsfGlobalFreeze)) + if (isGlobalFrozen(view, issue)) return true; - if (issuer != account) - { - // Check if the issuer froze the line - sle = view.read(keylet::line(account, issuer, asset)); - if (sle && - sle->isFlag((issuer > account) ? lsfHighFreeze : lsfLowFreeze)) - return true; - } - return false; + return isIndividualFrozen(view, account, issue); } STAmount @@ -231,46 +237,49 @@ accountHolds( FreezeHandling zeroIfFrozen, beast::Journal j) { - STAmount amount; if (isXRP(asset)) { return {xrpLiquid(view, account, 0, j)}; } - if (asset.isMPT()) + STAmount amount; + Issue const issue = asset.isMPT() ? Issue{asset} : Issue{asset, issuer}; + amount.clear(issue); + + // Lambda returns sle if the object exists and + // the asset is not frozen/locked + auto getSle = [&]() -> std::shared_ptr { + Keylet const key = asset.isMPT() ? keylet::mptoken(asset, account) + : keylet::line(account, issuer, asset); + auto const sle = view.read(key); + if (!sle || + ((zeroIfFrozen == fhZERO_IF_FROZEN) && + isFrozen(view, account, issue))) + return nullptr; + else + return sle; + }; + + if (auto const sle = getSle()) { - Issue iss{asset}; - if (auto const sle = view.read(keylet::mptoken(asset, account))) + // Return balance on MPT or trust line modulo freeze + if (asset.isMPT()) { auto const amt = sle->getFieldU64(sfMPTAmount); auto const locked = sle->getFieldU64(sfLockedAmount); if (amt > locked) - return STAmount{iss, amt - locked}; + amount = STAmount{Issue{asset}, amt - locked}; } - return STAmount{iss, 0}; - } - - // IOU: Return balance on trust line modulo freeze - auto const sle = view.read(keylet::line(account, issuer, asset)); - if (!sle) - { - amount.clear({asset, issuer}); - } - else if ( - (zeroIfFrozen == fhZERO_IF_FROZEN) && - isFrozen(view, account, asset, issuer)) - { - amount.clear(Issue(asset, issuer)); - } - else - { - amount = sle->getFieldAmount(sfBalance); - if (account > issuer) + else { - // Put balance in account terms. - amount.negate(); + amount = sle->getFieldAmount(sfBalance); + if (account > issuer) + { + // Put balance in account terms. + amount.negate(); + } + amount.setIssuer(issuer); } - amount.setIssuer(issuer); } JLOG(j.trace()) << "accountHolds:" << " account=" << to_string(account) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 11a92e866eb..fe35306b1e4 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -25,63 +25,6 @@ namespace ripple { class MPToken_test : public beast::unit_test::suite { - [[nodiscard]] bool - checkMPTokenAmount( - test::jtx::Env const& env, - ripple::uint256 const mptIssuanceid, - test::jtx::Account const& holder, - std::uint64_t expectedAmount) - { - auto const sleMpt = env.le(keylet::mptoken(mptIssuanceid, holder)); - if (!sleMpt) - return false; - - std::uint64_t const amount = (*sleMpt)[sfMPTAmount]; - return amount == expectedAmount; - } - - [[nodiscard]] bool - checkMPTokenOutstandingAmount( - test::jtx::Env const& env, - ripple::uint256 const mptIssuanceid, - std::uint64_t expectedAmount) - { - auto const sleMpt = env.le(keylet::mptIssuance(mptIssuanceid)); - if (!sleMpt) - return false; - - std::uint64_t const amount = (*sleMpt)[sfOutstandingAmount]; - return amount == expectedAmount; - } - - [[nodiscard]] bool - checkMPTokenIssuanceFlags( - test::jtx::Env const& env, - ripple::uint256 const mptIssuanceid, - uint32_t const expectedFlags) - { - auto const sleMptIssuance = env.le(keylet::mptIssuance(mptIssuanceid)); - if (!sleMptIssuance) - return false; - - uint32_t const mptIssuanceFlags = sleMptIssuance->getFlags(); - return expectedFlags == mptIssuanceFlags; - } - - [[nodiscard]] bool - checkMPTokenFlags( - test::jtx::Env const& env, - ripple::uint256 const mptIssuanceid, - test::jtx::Account const& holder, - uint32_t const expectedFlags) - { - auto const sleMpt = env.le(keylet::mptoken(mptIssuanceid, holder)); - if (!sleMpt) - return false; - uint32_t const mptFlags = sleMpt->getFlags(); - return mptFlags == expectedFlags; - } - void testCreateValidation(FeatureBitset features) { @@ -166,10 +109,6 @@ class MPToken_test : public beast::unit_test::suite .ownerCount = 1, .flags = tfMPTCanLock | tfMPTRequireAuth | tfMPTCanEscrow | tfMPTCanTrade | tfMPTCanTransfer | tfMPTCanClawback}); - - BEAST_EXPECT(mptAlice.checkFlags( - lsfMPTCanLock | lsfMPTRequireAuth | lsfMPTCanEscrow | - lsfMPTCanTrade | lsfMPTCanTransfer | lsfMPTCanClawback)); } } @@ -217,7 +156,6 @@ class MPToken_test : public beast::unit_test::suite // alice pays bob 100 tokens mptAlice.pay(alice, bob, 100); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); mptAlice.destroy({.err = tecHAS_OBLIGATIONS}); } @@ -250,6 +188,7 @@ class MPToken_test : public beast::unit_test::suite using namespace test::jtx; Account const alice("alice"); Account const bob("bob"); + Account const cindy("cindy"); // Validate fields in MPTokenAuthorize (preflight) { Env env{*this, features - featureMPTokensV1}; @@ -295,8 +234,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.create({.ownerCount = 1}); - BEAST_EXPECT(mptAlice.checkFlags(0)); - // bob submits a tx with a holder field mptAlice.authorize( {.account = &bob, .holder = &alice, .err = temMALFORMED}); @@ -320,7 +257,6 @@ class MPToken_test : public beast::unit_test::suite { // alice pays bob 100 tokens mptAlice.pay(alice, bob, 100); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); // bob tries to delete his CFToken, but fails since he still // holds tokens @@ -331,7 +267,6 @@ class MPToken_test : public beast::unit_test::suite // bob pays back alice 100 tokens mptAlice.pay(bob, alice, 100); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); } mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); @@ -346,15 +281,10 @@ class MPToken_test : public beast::unit_test::suite // Test bad scenarios with allow-listing in MPTokenAuthorize (preclaim) { Env env{*this, features}; - Account const alice("alice"); // issuer - Account const bob("bob"); // holder - Account const cindy("cindy"); MPTTester mptAlice(env, alice, {.holders = {&bob}}); mptAlice.create({.ownerCount = 1, .flags = tfMPTRequireAuth}); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTRequireAuth)); - // alice submits a tx without specifying a holder's account mptAlice.authorize({.err = temMALFORMED}); @@ -445,8 +375,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.create({.ownerCount = 1}); - BEAST_EXPECT(mptAlice.checkFlags(0)); - // bob creates a mptoken mptAlice.authorize({.account = &bob, .holderCount = 1}); @@ -467,8 +395,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.create({.ownerCount = 1, .flags = tfMPTRequireAuth}); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTRequireAuth)); - // bob creates a mptoken mptAlice.authorize({.account = &bob, .holderCount = 1}); @@ -519,8 +445,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.create({.ownerCount = 1, .holderCount = 0}); - BEAST_EXPECT(mptAlice.checkFlags(0)); - mptAlice.authorize({.account = &bob, .holderCount = 1}); // test invalid flag @@ -553,8 +477,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.create({.ownerCount = 1}); - BEAST_EXPECT(mptAlice.checkFlags(0)); - // alice tries to lock a mptissuance that has disabled locking mptAlice.set( {.account = &alice, @@ -600,8 +522,6 @@ class MPToken_test : public beast::unit_test::suite // create a mptokenissuance with locking mptAlice.create({.ownerCount = 1, .flags = tfMPTCanLock}); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - // a non-issuer acct tries to set the mptissuance mptAlice.set( {.account = &bob, .flags = tfMPTLock, .err = tecNO_PERMISSION}); @@ -636,8 +556,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanLock}); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - mptAlice.authorize({.account = &bob, .holderCount = 1}); // both the mptissuance and mptoken are not locked @@ -737,20 +655,13 @@ class MPToken_test : public beast::unit_test::suite // issuer to holder mptAlice.pay(alice, bob, 100); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); - BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(100)); // holder to issuer mptAlice.pay(bob, alice, 100); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); - BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(0)); // holder to holder mptAlice.pay(alice, bob, 100); mptAlice.pay(bob, carol, 50); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 50)); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(carol, 50)); - BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(100)); } // If allowlisting is enabled, Payment fails if the receiver is not @@ -766,8 +677,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize({.account = &bob}); mptAlice.pay(alice, bob, 100, tecNO_AUTH); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); - BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(0)); } // If allowlisting is enabled, Payment fails if the sender is not @@ -788,8 +697,6 @@ class MPToken_test : public beast::unit_test::suite // alice sends 100 MPT to bob mptAlice.pay(alice, bob, 100); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); - BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(100)); // alice UNAUTHORIZES bob mptAlice.authorize( @@ -798,8 +705,26 @@ class MPToken_test : public beast::unit_test::suite // bob fails to send back to alice because he is no longer // authorize to move his funds! mptAlice.pay(bob, alice, 100, tecNO_AUTH); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 100)); - BEAST_EXPECT(mptAlice.checkMPTokenOutstandingAmount(100)); + } + + // Payer doesn't have enough funds + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); + + mptAlice.create({.ownerCount = 1}); + + mptAlice.authorize({.account = &bob}); + mptAlice.authorize({.account = &carol}); + + mptAlice.pay(alice, bob, 100); + + // Pay to another holder + mptAlice.pay(bob, carol, 101, tecINSUFFICIENT_FUNDS); + + // Pay to the issuer + mptAlice.pay(bob, alice, 101, tecINSUFFICIENT_FUNDS); } } diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index e831e4e9546..226db6cd0c6 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -34,6 +34,18 @@ uint64ToByteArray(std::uint64_t value) return result; } +void +mptflags::operator()(Env& env) const +{ + env.test.expect(tester_.checkFlags(flags_)); +} + +void +mptpay::operator()(Env& env) const +{ + env.test.expect(amount_ == tester_.getAmount(account_)); +} + std::unordered_map MPTTester::makeHolders(std::vector const& holders) { @@ -92,7 +104,14 @@ MPTTester::create(const MPTCreate& arg) // convert maxAmt to hex string, since json doesn't accept 64-bit int if (arg.maxAmt) jv[sfMaximumAmount.jsonName] = strHex(uint64ToByteArray(*arg.maxAmt)); - submit(arg, jv); + if (submit(arg, jv) != tesSUCCESS) + { + id_.reset(); + issuanceKey_.reset(); + mpt_.reset(); + } + else if (arg.flags) + env_.require(mptflags(*this, *arg.flags)); } void @@ -216,12 +235,31 @@ MPTTester::pay( std::optional err) { assert(mpt_); + auto const srcAmt = getAmount(src); + auto const destAmt = getAmount(dest); if (err) env_(jtx::pay(src, dest, mpt(amount)), ter(*err)); else env_(jtx::pay(src, dest, mpt(amount))); + if (env_.ter() != tesSUCCESS) + amount = 0; if (close_) env_.close(); + if (src == issuer_) + { + env_.require(mptpay(*this, src, srcAmt + amount)); + env_.require(mptpay(*this, dest, destAmt + amount)); + } + else if (dest == issuer_) + { + env_.require(mptpay(*this, src, srcAmt - amount)); + env_.require(mptpay(*this, dest, destAmt - amount)); + } + else + { + env_.require(mptpay(*this, src, srcAmt - amount)); + env_.require(mptpay(*this, dest, destAmt + amount)); + } } PrettyAmount @@ -231,6 +269,23 @@ MPTTester::mpt(std::uint64_t amount) const return ripple::test::jtx::MPT(issuer_.name(), *mpt_)(amount); } +std::uint64_t +MPTTester::getAmount(Account const& account) const +{ + if (account == issuer_) + { + if (auto const sle = env_.le(keylet::mptIssuance(*issuanceKey_))) + return sle->getFieldU64(sfOutstandingAmount); + } + else + { + if (auto const sle = + env_.le(keylet::mptoken(*issuanceKey_, account.id()))) + return sle->getFieldU64(sfMPTAmount); + } + return 0; +} + } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 135bee64aba..9f3ddc34068 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -34,6 +34,43 @@ namespace { using AccountP = Account const*; } +class MPTTester; + +// Check flags settings on MPT create +class mptflags +{ +private: + MPTTester& tester_; + std::uint32_t flags_; + +public: + mptflags(MPTTester& tester, std::uint32_t flags) + : tester_(tester), flags_(flags) + { + } + + void + operator()(Env& env) const; +}; + +// Check mptissuance or mptoken amount balances on payment +class mptpay +{ +private: + MPTTester const& tester_; + Account const& account_; + std::uint64_t const amount_; + +public: + mptpay(MPTTester& tester, Account const& account, std::uint64_t amount) + : tester_(tester), account_(account), amount_(amount) + { + } + + void + operator()(Env& env) const; +}; + struct MPTConstr { std::vector holders = {}; @@ -52,7 +89,7 @@ struct MPTCreate std::optional ownerCount = std::nullopt; std::optional holderCount = std::nullopt; bool fund = true; - std::uint32_t flags = 0; + std::optional flags = {0}; std::optional err = std::nullopt; }; @@ -62,7 +99,7 @@ struct MPTDestroy std::optional id = std::nullopt; std::optional ownerCount = std::nullopt; std::optional holderCount = std::nullopt; - std::uint32_t flags = 0; + std::optional flags = std::nullopt; std::optional err = std::nullopt; }; @@ -73,7 +110,7 @@ struct MPTAuthorize std::optional id = std::nullopt; std::optional ownerCount = std::nullopt; std::optional holderCount = std::nullopt; - std::uint32_t flags = 0; + std::optional flags = std::nullopt; std::optional err = std::nullopt; }; @@ -84,7 +121,7 @@ struct MPTSet std::optional id = std::nullopt; std::optional ownerCount = std::nullopt; std::optional holderCount = std::nullopt; - std::uint32_t flags = 0; + std::optional flags = std::nullopt; std::optional err = std::nullopt; }; @@ -154,6 +191,9 @@ class MPTTester return *id_; } + std::uint64_t + getAmount(Account const& account) const; + private: using SLEP = std::shared_ptr; bool @@ -162,29 +202,21 @@ class MPTTester AccountP holder = nullptr) const; template - void + TER submit(A const& arg, Json::Value const& jv) { if (arg.err) { - if (arg.flags) - env_(jv, txflags(arg.flags), ter(*arg.err)); + if (arg.flags && arg.flags > 0) + env_(jv, txflags(*arg.flags), ter(*arg.err)); else env_(jv, ter(*arg.err)); } - else if (arg.flags) - env_(jv, txflags(arg.flags)); + else if (arg.flags && arg.flags > 0) + env_(jv, txflags(*arg.flags)); else env_(jv); - if constexpr (std::is_same_v) - { - if (env_.ter() != tesSUCCESS) - { - id_.reset(); - issuanceKey_.reset(); - mpt_.reset(); - } - } + auto const err = env_.ter(); if (close_) env_.close(); if (arg.ownerCount) @@ -194,6 +226,7 @@ class MPTTester for (auto it : holders_) env_.require(owners(*it.second, *arg.holderCount)); } + return err; } std::unordered_map From 19a949b741fa18361c4eedabe75035159a66524f Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Fri, 12 Jan 2024 12:47:54 -0500 Subject: [PATCH 19/44] update ledger entry --- src/ripple/rpc/handlers/LedgerEntry.cpp | 46 +++++++++++++++++- src/test/rpc/LedgerRPC_test.cpp | 62 +++++++++++++++++++++++-- 2 files changed, 102 insertions(+), 6 deletions(-) diff --git a/src/ripple/rpc/handlers/LedgerEntry.cpp b/src/ripple/rpc/handlers/LedgerEntry.cpp index 63f87389473..1d35624dca0 100644 --- a/src/ripple/rpc/handlers/LedgerEntry.cpp +++ b/src/ripple/rpc/handlers/LedgerEntry.cpp @@ -598,11 +598,11 @@ doLedgerEntry(RPC::JsonContext& context) else uNodeIndex = keylet::did(*account).key; } - else if (context.params.isMember(jss::mpt_issuance_id)) + else if (context.params.isMember(jss::mpt_issuance)) { expectedType = ltMPTOKEN_ISSUANCE; auto const unparsedMPTIssuanceID = - context.params[jss::mpt_issuance_id]; + context.params[jss::mpt_issuance]; if (unparsedMPTIssuanceID.isString()) { uint192 mptIssuanceID; @@ -619,6 +619,48 @@ doLedgerEntry(RPC::JsonContext& context) jvResult[jss::error] = "malformedRequest"; } } + else if (context.params.isMember(jss::mptoken)) + { + expectedType = ltMPTOKEN; + if (!context.params[jss::mptoken].isObject()) + { + if (!uNodeIndex.parseHex(context.params[jss::mptoken].asString())) + { + uNodeIndex = beast::zero; + jvResult[jss::error] = "malformedRequest"; + } + } + else if ( + !context.params[jss::mptoken].isMember(jss::mpt_issuance_id) || + !context.params[jss::mptoken].isMember(jss::account)) + { + jvResult[jss::error] = "malformedRequest"; + } + else + { + try + { + auto const mptIssuanceIdStr = + context.params[jss::mptoken][jss::mpt_issuance_id].asString(); + + uint192 mptIssuanceID; + if (!mptIssuanceID.parseHex(mptIssuanceIdStr)) + Throw("Cannot parse mpt_issuance_id"); + + auto const account = parseBase58( + context.params[jss::mptoken][jss::account].asString()); + + if (!account || account->isZero()) + jvResult[jss::error] = "malformedAddress"; + else + uNodeIndex = keylet::mptoken(mptIssuanceID, *account).key; + } + catch (std::runtime_error const&) + { + jvResult[jss::error] = "malformedRequest"; + } + } + } else { if (context.params.isMember("params") && diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 44eb1c3cb33..e54c3fab430 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -1582,7 +1582,7 @@ class LedgerRPC_test : public beast::unit_test::suite } void - testLedgerEntryMPTIssuanceID() + testLedgerEntryMPTIssuance() { testcase("ledger_entry Request MPTokenIssuance"); using namespace test::jtx; @@ -1599,7 +1599,7 @@ class LedgerRPC_test : public beast::unit_test::suite { // Request the MPTokenIssuance using its ID. Json::Value jvParams; - jvParams[jss::mpt_issuance_id] = to_string(mptAlice.issuanceID()); + jvParams[jss::mpt_issuance] = to_string(mptAlice.issuanceID()); jvParams[jss::ledger_hash] = ledgerHash; Json::Value const jrr = env.rpc( "json", "ledger_entry", to_string(jvParams))[jss::result]; @@ -1608,7 +1608,7 @@ class LedgerRPC_test : public beast::unit_test::suite { // Request an index that is not a MPTokenIssuacne. Json::Value jvParams; - jvParams[jss::mpt_issuance_id] = ledgerHash; + jvParams[jss::mpt_issuance] = ledgerHash; jvParams[jss::ledger_hash] = ledgerHash; Json::Value const jrr = env.rpc( "json", "ledger_entry", to_string(jvParams))[jss::result]; @@ -1616,6 +1616,59 @@ class LedgerRPC_test : public beast::unit_test::suite } } + void + testLedgerEntryMPToken() + { + testcase("ledger_entry Request MPToken"); + using namespace test::jtx; + using namespace std::literals::chrono_literals; + Env env{*this}; + Account const alice{"alice"}; + Account const bob{"bob"}; + + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create(); + std::string ledgerHash{to_string(env.closed()->info().hash)}; + + // MPToken can be requested in two ways: + // 1. an object that has MPTIssuanceID and holder + // 2. ledger index string of the MPToken object + { + mptAlice.authorize({.account = &bob, .holderCount = 1}); + ledgerHash = to_string(env.closed()->info().hash); + + // Request the MPToken using its issuanceID and holder. + Json::Value jvObjParams; + jvObjParams[jss::mptoken] = Json::objectValue; + jvObjParams[jss::mptoken][jss::account] = bob.human(); + jvObjParams[jss::mptoken][jss::mpt_issuance_id] = to_string(mptAlice.issuanceID()); + jvObjParams[jss::ledger_hash] = ledgerHash; + Json::Value const jrr1 = env.rpc( + "json", "ledger_entry", to_string(jvObjParams))[jss::result]; + BEAST_EXPECT(jrr1[jss::node][sfAccount.jsonName] == bob.human()); + + auto const mptokenIndex = jrr1[jss::node][jss::index]; + + // Request the same MPToken using the ledger index + Json::Value jvStrParams; + jvStrParams[jss::mptoken] = mptokenIndex; + jvStrParams[jss::ledger_hash] = ledgerHash; + Json::Value const jrr2 = env.rpc( + "json", "ledger_entry", to_string(jvStrParams))[jss::result]; + BEAST_EXPECT(jrr2[jss::node][sfAccount.jsonName] == bob.human()); + } + { + // Request an index that is not a MPToken. + Json::Value jvParams; + jvParams[jss::mptoken] = ledgerHash; + jvParams[jss::ledger_hash] = ledgerHash; + Json::Value const jrr = env.rpc( + "json", "ledger_entry", to_string(jvParams))[jss::result]; + checkErrorValue(jrr, "entryNotFound", ""); + } + } + void testLedgerEntryInvalidParams(unsigned int apiVersion) { @@ -2339,7 +2392,8 @@ class LedgerRPC_test : public beast::unit_test::suite testQueue(); testLedgerAccountsOption(); testLedgerEntryDID(); - testLedgerEntryMPTIssuanceID(); + testLedgerEntryMPTIssuance(); + testLedgerEntryMPToken(); test::jtx::forAllApiVersions(std::bind_front( &LedgerRPC_test::testLedgerEntryInvalidParams, this)); From 9a7afa5a05226bf185cbb55797144b453f92f4ed Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Fri, 12 Jan 2024 12:48:39 -0500 Subject: [PATCH 20/44] clang --- src/ripple/rpc/handlers/LedgerEntry.cpp | 12 ++++++++---- src/test/rpc/LedgerRPC_test.cpp | 5 +++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/ripple/rpc/handlers/LedgerEntry.cpp b/src/ripple/rpc/handlers/LedgerEntry.cpp index 1d35624dca0..c7f629a329f 100644 --- a/src/ripple/rpc/handlers/LedgerEntry.cpp +++ b/src/ripple/rpc/handlers/LedgerEntry.cpp @@ -624,7 +624,8 @@ doLedgerEntry(RPC::JsonContext& context) expectedType = ltMPTOKEN; if (!context.params[jss::mptoken].isObject()) { - if (!uNodeIndex.parseHex(context.params[jss::mptoken].asString())) + if (!uNodeIndex.parseHex( + context.params[jss::mptoken].asString())) { uNodeIndex = beast::zero; jvResult[jss::error] = "malformedRequest"; @@ -641,11 +642,13 @@ doLedgerEntry(RPC::JsonContext& context) try { auto const mptIssuanceIdStr = - context.params[jss::mptoken][jss::mpt_issuance_id].asString(); + context.params[jss::mptoken][jss::mpt_issuance_id] + .asString(); uint192 mptIssuanceID; if (!mptIssuanceID.parseHex(mptIssuanceIdStr)) - Throw("Cannot parse mpt_issuance_id"); + Throw( + "Cannot parse mpt_issuance_id"); auto const account = parseBase58( context.params[jss::mptoken][jss::account].asString()); @@ -653,7 +656,8 @@ doLedgerEntry(RPC::JsonContext& context) if (!account || account->isZero()) jvResult[jss::error] = "malformedAddress"; else - uNodeIndex = keylet::mptoken(mptIssuanceID, *account).key; + uNodeIndex = + keylet::mptoken(mptIssuanceID, *account).key; } catch (std::runtime_error const&) { diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index e54c3fab430..5422c221af2 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -1632,7 +1632,7 @@ class LedgerRPC_test : public beast::unit_test::suite std::string ledgerHash{to_string(env.closed()->info().hash)}; // MPToken can be requested in two ways: - // 1. an object that has MPTIssuanceID and holder + // 1. an object that has MPTIssuanceID and holder // 2. ledger index string of the MPToken object { mptAlice.authorize({.account = &bob, .holderCount = 1}); @@ -1642,7 +1642,8 @@ class LedgerRPC_test : public beast::unit_test::suite Json::Value jvObjParams; jvObjParams[jss::mptoken] = Json::objectValue; jvObjParams[jss::mptoken][jss::account] = bob.human(); - jvObjParams[jss::mptoken][jss::mpt_issuance_id] = to_string(mptAlice.issuanceID()); + jvObjParams[jss::mptoken][jss::mpt_issuance_id] = + to_string(mptAlice.issuanceID()); jvObjParams[jss::ledger_hash] = ledgerHash; Json::Value const jrr1 = env.rpc( "json", "ledger_entry", to_string(jvObjParams))[jss::result]; From 7e94fa0526a3770d6097d8436bdfeda0108c537f Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Fri, 12 Jan 2024 14:40:39 -0500 Subject: [PATCH 21/44] Add lock check on pay, bundle flag check into set --- src/ripple/app/tx/impl/CreateOffer.cpp | 2 - src/ripple/app/tx/impl/Payment.cpp | 11 ++++ src/ripple/ledger/View.h | 2 +- src/test/app/MPToken_test.cpp | 73 ++++++++++++++------------ src/test/jtx/impl/mpt.cpp | 29 +++++++++- src/test/jtx/mpt.h | 5 +- 6 files changed, 80 insertions(+), 42 deletions(-) diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index 12ae77c65be..e3fae15f0cd 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -136,8 +136,6 @@ CreateOffer::preclaim(PreclaimContext const& ctx) auto const& uPaysIssuerID = saTakerPays.getIssuer(); auto const& uPaysCurrency = saTakerPays.getAsset(); - auto const& uGetsIssuerID = saTakerGets.getIssuer(); - auto const cancelSequence = ctx.tx[~sfOfferSequence]; auto const sleCreator = ctx.view.read(keylet::account(id)); diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index 55e9ee76343..89e90bd0cb0 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -445,6 +445,17 @@ Payment::doApply() ter != tesSUCCESS) return ter; + auto const& issue = saDstAmount.issue(); + auto const& issuer = issue.account(); + // If globally/individually locked then + // can't send between holders + // holder can send back to issuer + // issuer can send to holder + if (account_ != issuer && uDstAccountID != issuer && + (isFrozen(view(), account_, issue) || + isFrozen(view(), uDstAccountID, issue))) + return tecFROZEN; + PaymentSandbox pv(&view()); auto const res = accountSend(pv, account_, uDstAccountID, saDstAmount, j_); diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index b0a65cebdfd..70446bc258e 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -81,7 +81,7 @@ enum FreezeHandling { fhIGNORE_FREEZE, fhZERO_IF_FROZEN }; [[nodiscard]] bool isGlobalFrozen(ReadView const& view, Issue const& issue); -[[nodiscard]] inline bool +[[nodiscard]] bool isIndividualFrozen( ReadView const& view, AccountID const& account, diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index fe35306b1e4..f2b27a81bdd 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -565,70 +565,35 @@ class MPToken_test : public beast::unit_test::suite // locks bob's mptoken mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); - // trying to lock bob's mptoken again will still succeed // but no changes to the objects mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); - // no changes to the objects - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); - // alice locks the mptissuance mptAlice.set({.account = &alice, .flags = tfMPTLock}); - // now both the mptissuance and mptoken are locked up - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock | lsfMPTLocked)); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); - // alice tries to lock up both mptissuance and mptoken again // it will not change the flags and both will remain locked. mptAlice.set({.account = &alice, .flags = tfMPTLock}); mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); - // now both the mptissuance and mptoken remain locked up - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock | lsfMPTLocked)); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); - // alice unlocks bob's mptoken mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTUnlock}); - // only mptissuance is locked - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock | lsfMPTLocked)); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - // locks up bob's mptoken again mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); - // now both the mptissuance and mptokens are locked up - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock | lsfMPTLocked)); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); - // alice unlocks mptissuance mptAlice.set({.account = &alice, .flags = tfMPTUnlock}); - // now mptissuance is unlocked - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTLocked, &bob)); - // alice unlocks bob's mptoken mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTUnlock}); - // both mptissuance and bob's mptoken are unlocked - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - // alice unlocks mptissuance and bob's mptoken again despite that // they are already unlocked. Make sure this will not change the // flags mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTUnlock}); mptAlice.set({.account = &alice, .flags = tfMPTUnlock}); - - // both mptissuance and bob's mptoken remain unlocked - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); } void @@ -726,6 +691,44 @@ class MPToken_test : public beast::unit_test::suite // Pay to the issuer mptAlice.pay(bob, alice, 101, tecINSUFFICIENT_FUNDS); } + + // MPT is locked + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); + + mptAlice.create({.ownerCount = 1, .flags = tfMPTCanLock}); + + mptAlice.authorize({.account = &bob}); + mptAlice.authorize({.account = &carol}); + + mptAlice.pay(alice, bob, 100); + mptAlice.pay(alice, carol, 100); + + // Global lock + mptAlice.set({.account = &alice, .flags = tfMPTLock}); + // Can't send between holders + mptAlice.pay(bob, carol, 1, tecFROZEN); + mptAlice.pay(carol, bob, 2, tecFROZEN); + // Issuer can send + mptAlice.pay(alice, bob, 3); + // Holder can send back to issuer + mptAlice.pay(bob, alice, 4); + + // Global unlock + mptAlice.set({.account = &alice, .flags = tfMPTUnlock}); + // Individual lock + mptAlice.set( + {.account = &alice, .holder = &bob, .flags = tfMPTLock}); + // Can't send between holders + mptAlice.pay(bob, carol, 5, tecFROZEN); + mptAlice.pay(carol, bob, 6, tecFROZEN); + // Issuer can send + mptAlice.pay(alice, bob, 7); + // Holder can send back to issuer + mptAlice.pay(bob, alice, 8); + } } void diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 226db6cd0c6..a1ff282d7d3 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -37,7 +37,7 @@ uint64ToByteArray(std::uint64_t value) void mptflags::operator()(Env& env) const { - env.test.expect(tester_.checkFlags(flags_)); + env.test.expect(tester_.checkFlags(flags_, holder_)); } void @@ -182,7 +182,32 @@ MPTTester::set(MPTSet const& arg) } if (arg.holder) jv[sfMPTokenHolder.jsonName] = arg.holder->human(); - submit(arg, jv); + if (submit(arg, jv) == tesSUCCESS && arg.flags.value_or(0)) + { + std::uint32_t flags = 0; + auto require = [&](AccountP holder, bool unchanged) { + assert(forObject( + [&](SLEP const& sle) { + flags = sle->getFlags(); + return true; + }, + holder)); + if (!unchanged) + { + if (*arg.flags & tfMPTLock) + flags |= lsfMPTLocked; + else if (*arg.flags & tfMPTUnlock) + flags &= ~lsfMPTLocked; + else + assert(0); + } + env_.require(mptflags(*this, flags, holder)); + }; + if (arg.account) + require(nullptr, arg.holder != nullptr); + if (arg.holder) + require(arg.holder, false); + } } bool diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 9f3ddc34068..b7a90391640 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -42,10 +42,11 @@ class mptflags private: MPTTester& tester_; std::uint32_t flags_; + AccountP holder_; public: - mptflags(MPTTester& tester, std::uint32_t flags) - : tester_(tester), flags_(flags) + mptflags(MPTTester& tester, std::uint32_t flags, AccountP holder = nullptr) + : tester_(tester), flags_(flags), holder_(holder) { } From d34508eb5bbc42634c793cbc1c0bd45bbc020d9e Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Mon, 15 Jan 2024 11:33:17 -0500 Subject: [PATCH 22/44] remove mpt dir --- Builds/CMake/RippledCore.cmake | 1 - src/ripple/app/tx/impl/MPTokenAuthorize.cpp | 18 -- src/ripple/protocol/SField.h | 1 - src/ripple/protocol/impl/LedgerFormats.cpp | 2 - src/ripple/protocol/impl/SField.cpp | 1 - src/ripple/rpc/handlers/MPTHolders.cpp | 164 ------------------- src/ripple/rpc/impl/Handler.cpp | 2 +- src/test/app/MPToken_test.cpp | 172 -------------------- 8 files changed, 1 insertion(+), 360 deletions(-) delete mode 100644 src/ripple/rpc/handlers/MPTHolders.cpp diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 84371a3dac4..915ecc0b412 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -682,7 +682,6 @@ target_sources (rippled PRIVATE src/ripple/rpc/handlers/BlackList.cpp src/ripple/rpc/handlers/BookOffers.cpp src/ripple/rpc/handlers/CanDelete.cpp - src/ripple/rpc/handlers/MPTHolders.cpp src/ripple/rpc/handlers/Connect.cpp src/ripple/rpc/handlers/ConsensusInfo.cpp src/ripple/rpc/handlers/CrawlShards.cpp diff --git a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp index 529aa0499d2..fb32c55ca80 100644 --- a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp +++ b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp @@ -182,13 +182,6 @@ MPTokenAuthorize::doApply() false)) return tecINTERNAL; - if (!view().dirRemove( - keylet::mpt_dir(mptIssuanceID), - (*sleMpt)[sfMPTokenNode], - sleMpt->key(), - false)) - return tecINTERNAL; - adjustOwnerCount(view(), sleAcct, -1, j_); view().erase(sleMpt); @@ -214,22 +207,11 @@ MPTokenAuthorize::doApply() if (!ownerNode) return tecDIR_FULL; - auto const mptNode = view().dirInsert( - keylet::mpt_dir(mptIssuanceID), - mptokenKey, - [&mptIssuanceID](std::shared_ptr const& sle) { - (*sle)[sfMPTokenIssuanceID] = mptIssuanceID; - }); - - if (!mptNode) - return tecDIR_FULL; - auto mptoken = std::make_shared(mptokenKey); (*mptoken)[sfAccount] = account_; (*mptoken)[sfMPTokenIssuanceID] = mptIssuanceID; (*mptoken)[sfFlags] = 0; (*mptoken)[sfOwnerNode] = *ownerNode; - (*mptoken)[sfMPTokenNode] = *mptNode; view().insert(mptoken); // Update owner count. diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 08f90237973..6a9eb31723d 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -451,7 +451,6 @@ extern SF_UINT64 const sfCookie; extern SF_UINT64 const sfServerVersion; extern SF_UINT64 const sfNFTokenOfferNode; extern SF_UINT64 const sfEmitBurden; -extern SF_UINT64 const sfMPTokenNode; // 64-bit integers (uncommon) extern SF_UINT64 const sfHookOn; diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index c20af507cf2..d7f3387b47e 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -74,7 +74,6 @@ LedgerFormats::LedgerFormats() {sfIndexNext, soeOPTIONAL}, {sfIndexPrevious, soeOPTIONAL}, {sfNFTokenID, soeOPTIONAL}, - {sfMPTokenIssuanceID, soeOPTIONAL}, }, commonFields); @@ -366,7 +365,6 @@ LedgerFormats::LedgerFormats() {sfMPTAmount, soeDEFAULT}, {sfLockedAmount, soeDEFAULT}, {sfOwnerNode, soeREQUIRED}, - {sfMPTokenNode, soeREQUIRED}, {sfPreviousTxnID, soeREQUIRED}, {sfPreviousTxnLgrSeq, soeREQUIRED} }, diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index fea6f010527..3b3f309d382 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -180,7 +180,6 @@ CONSTRUCT_TYPED_SFIELD(sfCookie, "Cookie", UINT64, CONSTRUCT_TYPED_SFIELD(sfServerVersion, "ServerVersion", UINT64, 11); CONSTRUCT_TYPED_SFIELD(sfNFTokenOfferNode, "NFTokenOfferNode", UINT64, 12); CONSTRUCT_TYPED_SFIELD(sfEmitBurden, "EmitBurden", UINT64, 13); -CONSTRUCT_TYPED_SFIELD(sfMPTokenNode, "MPTokenNode", UINT64, 14); // 64-bit integers (uncommon) CONSTRUCT_TYPED_SFIELD(sfHookOn, "HookOn", UINT64, 16); diff --git a/src/ripple/rpc/handlers/MPTHolders.cpp b/src/ripple/rpc/handlers/MPTHolders.cpp deleted file mode 100644 index 64cb3f4f25b..00000000000 --- a/src/ripple/rpc/handlers/MPTHolders.cpp +++ /dev/null @@ -1,164 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2024 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace ripple { - -static void -appendMPTHolderJson( - Application const& app, - std::shared_ptr const& mpt, - Json::Value& holders) -{ - Json::Value& obj(holders.append(Json::objectValue)); - - obj[jss::mptoken_index] = to_string(mpt->key()); - obj[jss::flags] = (*mpt)[sfFlags]; - obj[jss::account] = toBase58(mpt->getAccountID(sfAccount)); - obj[jss::mpt_amount] = - STUInt64{(*mpt)[sfMPTAmount]}.getJson(JsonOptions::none); - - if ((*mpt)[sfLockedAmount]) - obj[jss::locked_amount] = - STUInt64{(*mpt)[sfLockedAmount]}.getJson(JsonOptions::none); -} - -// { -// mpt_issuance_id: -// ledger_hash : -// ledger_index : -// limit: integer // optional -// marker: opaque // optional, resume previous query -// } -static Json::Value -enumerateMPTHolders( - RPC::JsonContext& context, - uint192 const& mptIssuanceID, - Keylet const& directory) -{ - unsigned int limit; - if (auto err = readLimitField(limit, RPC::Tuning::mptHolders, context)) - return *err; - - std::shared_ptr ledger; - - if (auto result = RPC::lookupLedger(ledger, context); !ledger) - return result; - - if (!ledger->exists(directory)) - return rpcError(rpcOBJECT_NOT_FOUND); - - Json::Value result; - result[jss::mpt_issuance_id] = to_string(mptIssuanceID); - - Json::Value& jsonHolders(result[jss::holders] = Json::arrayValue); - - std::vector> holders; - unsigned int reserve(limit); - uint256 startAfter; - std::uint64_t startHint = 0; - - if (context.params.isMember(jss::marker)) - { - // We have a start point. Use limit - 1 from the result and use the - // very last one for the resume. - Json::Value const& marker(context.params[jss::marker]); - - if (!marker.isString()) - return RPC::expected_field_error(jss::marker, "string"); - - if (!startAfter.parseHex(marker.asString())) - return rpcError(rpcINVALID_PARAMS); - - auto const sle = ledger->read(keylet::mptoken(startAfter)); - - if (!sle || mptIssuanceID != sle->getFieldH192(sfMPTokenIssuanceID)) - return rpcError(rpcINVALID_PARAMS); - - startHint = sle->getFieldU64(sfMPTokenNode); - appendMPTHolderJson(context.app, sle, jsonHolders); - holders.reserve(reserve); - } - else - { - // We have no start point, limit should be one higher than requested. - holders.reserve(++reserve); - } - - if (!forEachItemAfter( - *ledger, - directory, - startAfter, - startHint, - reserve, - [&holders](std::shared_ptr const& mptoken) { - if (mptoken->getType() == ltMPTOKEN) - { - holders.emplace_back(mptoken); - return true; - } - - return false; - })) - { - return rpcError(rpcINVALID_PARAMS); - } - - if (holders.size() == reserve) - { - result[jss::limit] = limit; - result[jss::marker] = to_string(holders.back()->key()); - holders.pop_back(); - } - - for (auto const& mpt : holders) - appendMPTHolderJson(context.app, mpt, jsonHolders); - - context.loadType = Resource::feeMediumBurdenRPC; - return result; -} - -Json::Value -doMPTHolders(RPC::JsonContext& context) -{ - if (!context.params.isMember(jss::mpt_issuance_id)) - return RPC::missing_field_error(jss::mpt_issuance_id); - - uint192 mptIssuanceID; - - if (!mptIssuanceID.parseHex( - context.params[jss::mpt_issuance_id].asString())) - return RPC::invalid_field_error(jss::mpt_issuance_id); - - return enumerateMPTHolders( - context, mptIssuanceID, keylet::mpt_dir(mptIssuanceID)); -} - -} // namespace ripple diff --git a/src/ripple/rpc/impl/Handler.cpp b/src/ripple/rpc/impl/Handler.cpp index 4cccc060180..3c9cb463def 100644 --- a/src/ripple/rpc/impl/Handler.cpp +++ b/src/ripple/rpc/impl/Handler.cpp @@ -95,7 +95,7 @@ Handler const handlerArray[]{ {"book_changes", byRef(&doBookChanges), Role::USER, NO_CONDITION}, {"book_offers", byRef(&doBookOffers), Role::USER, NO_CONDITION}, {"can_delete", byRef(&doCanDelete), Role::ADMIN, NO_CONDITION}, - {"mpt_holders", byRef(&doMPTHolders), Role::USER, NO_CONDITION}, + //{"mpt_holders", byRef(&doMPTHolders), Role::ADMIN, NO_CONDITION}, {"channel_authorize", byRef(&doChannelAuthorize), Role::USER, NO_CONDITION}, {"channel_verify", byRef(&doChannelVerify), Role::USER, NO_CONDITION}, {"connect", byRef(&doConnect), Role::ADMIN, NO_CONDITION}, diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index f2b27a81bdd..dd3fcece849 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -776,175 +776,6 @@ class MPToken_test : public beast::unit_test::suite meta[jss::mpt_issuance_id] == to_string(mptAlice.issuanceID())); } - void - testMPTHoldersAPI(FeatureBitset features) - { - testcase("MPT Holders"); - using namespace test::jtx; - - // a lambda that checks API correctness given different numbers of - // MPToken - auto checkMPTokens = [&](int expectCount, - int expectMarkerCount, - int line) { - Env env{*this, features}; - Account const alice("alice"); // issuer - - MPTTester mptAlice(env, alice); - - mptAlice.create(); - - // create accounts that will create MPTokens - for (auto i = 0; i < expectCount; i++) - { - Account const bob{std::string("bob") + std::to_string(i)}; - env.fund(XRP(1000), bob); - env.close(); - - // a holder creates a mptoken - mptAlice.authorize({.account = &bob}); - } - - // Checks mpt_holder query responses - { - int markerCount = 0; - Json::Value allHolders(Json::arrayValue); - std::string marker; - - // The do/while collects results until no marker is - // returned. - do - { - Json::Value mptHolders = [&env, &mptAlice, &marker]() { - Json::Value params; - params[jss::mpt_issuance_id] = - to_string(mptAlice.issuanceID()); - - if (!marker.empty()) - params[jss::marker] = marker; - return env.rpc( - "json", "mpt_holders", to_string(params)); - }(); - - // If there are mptokens we get an error - if (expectCount == 0) - { - if (expect( - mptHolders.isMember(jss::result), - "expected \"result\"", - __FILE__, - line)) - { - if (expect( - mptHolders[jss::result].isMember( - jss::error), - "expected \"error\"", - __FILE__, - line)) - { - expect( - mptHolders[jss::result][jss::error] - .asString() == "objectNotFound", - "expected \"objectNotFound\"", - __FILE__, - line); - } - } - break; - } - - marker.clear(); - if (expect( - mptHolders.isMember(jss::result), - "expected \"result\"", - __FILE__, - line)) - { - Json::Value& result = mptHolders[jss::result]; - - if (result.isMember(jss::marker)) - { - ++markerCount; - marker = result[jss::marker].asString(); - } - - if (expect( - result.isMember(jss::holders), - "expected \"holders\"", - __FILE__, - line)) - { - Json::Value& someHolders = result[jss::holders]; - for (std::size_t i = 0; i < someHolders.size(); ++i) - allHolders.append(someHolders[i]); - } - } - } while (!marker.empty()); - - // Verify the contents of allHolders makes sense. - expect( - allHolders.size() == expectCount, - "Unexpected returned offer count", - __FILE__, - line); - expect( - markerCount == expectMarkerCount, - "Unexpected marker count", - __FILE__, - line); - std::optional globalFlags; - std::set mptIndexes; - std::set holderAddresses; - for (Json::Value const& holder : allHolders) - { - // The flags on all found offers should be the same. - if (!globalFlags) - globalFlags = holder[jss::flags].asInt(); - - expect( - *globalFlags == holder[jss::flags].asInt(), - "Inconsistent flags returned", - __FILE__, - line); - - // The test conditions should produce unique indexes and - // amounts for all holders. - mptIndexes.insert(holder[jss::mptoken_index].asString()); - holderAddresses.insert(holder[jss::account].asString()); - } - - expect( - mptIndexes.size() == expectCount, - "Duplicate indexes returned?", - __FILE__, - line); - expect( - holderAddresses.size() == expectCount, - "Duplicate addresses returned?", - __FILE__, - line); - } - }; - - // Test 1 MPToken - checkMPTokens(1, 0, __LINE__); - - // Test 10 MPTokens - checkMPTokens(10, 0, __LINE__); - - // Test 200 MPTokens - checkMPTokens(200, 0, __LINE__); - - // Test 201 MPTokens - checkMPTokens(201, 1, __LINE__); - - // Test 400 MPTokens - checkMPTokens(400, 1, __LINE__); - - // Test 401 MPTokesn - checkMPTokens(401, 2, __LINE__); - } - public: void run() override @@ -981,9 +812,6 @@ class MPToken_test : public beast::unit_test::suite // `subscribe`, `ledger`. We should create test for these // occurances (lower prioirity). testTxJsonMetaFields(all); - - // Test mpt_holders - testMPTHoldersAPI(all); } }; From ee7b08e6c2bf711ae248af2c0e9552e11e4d7a2e Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 16 Jan 2024 14:34:23 -0500 Subject: [PATCH 23/44] Fix bug --- src/ripple/app/tx/impl/MPTokenAuthorize.cpp | 254 +++++++++++--------- src/test/app/MPToken_test.cpp | 26 ++ 2 files changed, 161 insertions(+), 119 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp index fb32c55ca80..776402c8ced 100644 --- a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp +++ b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp @@ -46,11 +46,6 @@ MPTokenAuthorize::preflight(PreflightContext const& ctx) TER MPTokenAuthorize::preclaim(PreclaimContext const& ctx) { - auto const sleMptIssuance = - ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID])); - if (!sleMptIssuance) - return tecOBJECT_NOT_FOUND; - auto const accountID = ctx.tx[sfAccount]; auto const txFlags = ctx.tx.getFlags(); auto const holderID = ctx.tx[~sfMPTokenHolder]; @@ -58,56 +53,78 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) if (holderID && !(ctx.view.exists(keylet::account(*holderID)))) return tecNO_DST; - std::uint32_t const mptIssuanceFlags = sleMptIssuance->getFieldU32(sfFlags); + std::shared_ptr sleMptIssuance; - // If tx is submitted by issuer, they would either try to do the following - // for allowlisting: - // 1. authorize an account - // 2. unauthorize an account + // if non-issuer account submits this tx, then they are trying either: + // 1. Unauthorize/delete MPToken + // 2. Use/create MPToken // - // Note: `accountID` is issuer's account - // `holderID` is holder's account - if (accountID == (*sleMptIssuance)[sfIssuer]) - { - // If tx is submitted by issuer, it only applies for MPT with - // lsfMPTRequireAuth set - if (!(mptIssuanceFlags & lsfMPTRequireAuth)) - return tecNO_AUTH; - - if (!holderID) + // Note: `accountID` is holder's account + // `holderID` is NOT used + if (!holderID){ + std::shared_ptr sleMpt = + ctx.view.read(keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], accountID)); + + // There is an edge case where holder deletes MPT after issuance has already been destroyed. + // So we must check for unauthorize before fetching the MPTIssuance object(since it doesn't exist) + + // if holder wants to delete/unauthorize a mpt + if (txFlags & tfMPTUnauthorize) + { + if (!sleMpt) + return tecNO_ENTRY; + + if ((*sleMpt)[sfMPTAmount] != 0) + return tecHAS_OBLIGATIONS; + + return tesSUCCESS; + } + + // Now test when the holder wants to hold/create/authorize a new MPT + sleMptIssuance = + ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID])); + + if (!sleMptIssuance) + return tecOBJECT_NOT_FOUND; + + if (accountID == (*sleMptIssuance)[sfIssuer]) return temMALFORMED; - if (!ctx.view.exists( - keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], *holderID))) - return tecNO_ENTRY; + // if holder wants to use and create a mpt + if (sleMpt) + return tecMPTOKEN_EXISTS; return tesSUCCESS; } - // if non-issuer account submits this tx, then they are trying either: - // 1. Unauthorize/delete MPToken - // 2. Use/create MPToken + sleMptIssuance = + ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID])); + if (!sleMptIssuance) + return tecOBJECT_NOT_FOUND; + + std::uint32_t const mptIssuanceFlags = sleMptIssuance->getFieldU32(sfFlags); + + // If tx is submitted by issuer, they would either try to do the following + // for allowlisting: + // 1. authorize an account + // 2. unauthorize an account // - // Note: `accountID` is holder's account - // `holderID` is NOT used - if (holderID) + // Note: `accountID` is issuer's account + // `holderID` is holder's account + if (accountID != (*sleMptIssuance)[sfIssuer]) return temMALFORMED; - std::shared_ptr sleMpt = - ctx.view.read(keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], accountID)); + if (!holderID) + return temMALFORMED; - // if holder wants to delete/unauthorize a mpt - if (txFlags & tfMPTUnauthorize) - { - if (!sleMpt) - return tecNO_ENTRY; + // If tx is submitted by issuer, it only applies for MPT with + // lsfMPTRequireAuth set + if (!(mptIssuanceFlags & lsfMPTRequireAuth)) + return tecNO_AUTH; - if ((*sleMpt)[sfMPTAmount] != 0) - return tecHAS_OBLIGATIONS; - } - // if holder wants to use and create a mpt - else if (sleMpt) - return tecMPTOKEN_EXISTS; + if (!ctx.view.exists( + keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], *holderID))) + return tecNO_ENTRY; return tesSUCCESS; } @@ -116,10 +133,6 @@ TER MPTokenAuthorize::doApply() { auto const mptIssuanceID = ctx_.tx[sfMPTokenIssuanceID]; - auto const sleMptIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); - if (!sleMptIssuance) - return tecINTERNAL; - auto const sleAcct = view().peek(keylet::account(account_)); if (!sleAcct) return tecINTERNAL; @@ -127,96 +140,99 @@ MPTokenAuthorize::doApply() auto const holderID = ctx_.tx[~sfMPTokenHolder]; auto const txFlags = ctx_.tx.getFlags(); - // If the account that submitted this tx is the issuer of the MPT - // Note: `account_` is issuer's account - // `holderID` is holder's account - if (account_ == (*sleMptIssuance)[sfIssuer]) - { - if (!holderID) - return tecINTERNAL; - - auto const sleMpt = - view().peek(keylet::mptoken(mptIssuanceID, *holderID)); - if (!sleMpt) - return tecINTERNAL; - - std::uint32_t const flagsIn = sleMpt->getFieldU32(sfFlags); - std::uint32_t flagsOut = flagsIn; - - // Issuer wants to unauthorize the holder, unset lsfMPTAuthorized on - // their MPToken - if (txFlags & tfMPTUnauthorize) - flagsOut &= ~lsfMPTAuthorized; - // Issuer wants to authorize a holder, set lsfMPTAuthorized on their - // MPToken - else - flagsOut |= lsfMPTAuthorized; - - if (flagsIn != flagsOut) - sleMpt->setFieldU32(sfFlags, flagsOut); - - view().update(sleMpt); - return tesSUCCESS; - } - // If the account that submitted the tx is a holder // Note: `account_` is holder's account // `holderID` is NOT used - if (holderID) - return tecINTERNAL; + if (!holderID){ + // When a holder wants to unauthorize/delete a MPT, the ledger must + // - delete mptokenKey from both owner and mpt directories + // - delete the MPToken + if (txFlags & tfMPTUnauthorize) + { + auto const mptokenKey = keylet::mptoken(mptIssuanceID, account_); + auto const sleMpt = view().peek(mptokenKey); + if (!sleMpt) + return tecINTERNAL; + + if (!view().dirRemove( + keylet::ownerDir(account_), + (*sleMpt)[sfOwnerNode], + sleMpt->key(), + false)) + return tecINTERNAL; + + adjustOwnerCount(view(), sleAcct, -1, j_); + + view().erase(sleMpt); + return tesSUCCESS; + } + + // A potential holder wants to authorize/hold a mpt, the ledger must: + // - add the new mptokenKey to both the owner and mpt directries + // - create the MPToken object for the holder + std::uint32_t const uOwnerCount = sleAcct->getFieldU32(sfOwnerCount); + XRPAmount const reserveCreate( + (uOwnerCount < 2) ? XRPAmount(beast::zero) + : view().fees().accountReserve(uOwnerCount + 1)); + + if (mPriorBalance < reserveCreate) + return tecINSUFFICIENT_RESERVE; - // When a holder wants to unauthorize/delete a MPT, the ledger must - // - delete mptokenKey from both owner and mpt directories - // - delete the MPToken - if (txFlags & tfMPTUnauthorize) - { auto const mptokenKey = keylet::mptoken(mptIssuanceID, account_); - auto const sleMpt = view().peek(mptokenKey); - if (!sleMpt) - return tecINTERNAL; - if (!view().dirRemove( - keylet::ownerDir(account_), - (*sleMpt)[sfOwnerNode], - sleMpt->key(), - false)) - return tecINTERNAL; + auto const ownerNode = view().dirInsert( + keylet::ownerDir(account_), mptokenKey, describeOwnerDir(account_)); - adjustOwnerCount(view(), sleAcct, -1, j_); + if (!ownerNode) + return tecDIR_FULL; + + auto mptoken = std::make_shared(mptokenKey); + (*mptoken)[sfAccount] = account_; + (*mptoken)[sfMPTokenIssuanceID] = mptIssuanceID; + (*mptoken)[sfFlags] = 0; + (*mptoken)[sfOwnerNode] = *ownerNode; + view().insert(mptoken); + + // Update owner count. + adjustOwnerCount(view(), sleAcct, 1, j_); - view().erase(sleMpt); return tesSUCCESS; } - // A potential holder wants to authorize/hold a mpt, the ledger must: - // - add the new mptokenKey to both the owner and mpt directries - // - create the MPToken object for the holder - std::uint32_t const uOwnerCount = sleAcct->getFieldU32(sfOwnerCount); - XRPAmount const reserveCreate( - (uOwnerCount < 2) ? XRPAmount(beast::zero) - : view().fees().accountReserve(uOwnerCount + 1)); - - if (mPriorBalance < reserveCreate) - return tecINSUFFICIENT_RESERVE; + std::shared_ptr sleMptIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); + if (!sleMptIssuance) + return tecINTERNAL; - auto const mptokenKey = keylet::mptoken(mptIssuanceID, account_); + // If the account that submitted this tx is the issuer of the MPT + // Note: `account_` is issuer's account + // `holderID` is holder's account + if (account_ != (*sleMptIssuance)[sfIssuer]) + return tecINTERNAL; + + if (!holderID) + return tecINTERNAL; - auto const ownerNode = view().dirInsert( - keylet::ownerDir(account_), mptokenKey, describeOwnerDir(account_)); + auto const sleMpt = + view().peek(keylet::mptoken(mptIssuanceID, *holderID)); + if (!sleMpt) + return tecINTERNAL; - if (!ownerNode) - return tecDIR_FULL; + std::uint32_t const flagsIn = sleMpt->getFieldU32(sfFlags); + std::uint32_t flagsOut = flagsIn; - auto mptoken = std::make_shared(mptokenKey); - (*mptoken)[sfAccount] = account_; - (*mptoken)[sfMPTokenIssuanceID] = mptIssuanceID; - (*mptoken)[sfFlags] = 0; - (*mptoken)[sfOwnerNode] = *ownerNode; - view().insert(mptoken); + // Issuer wants to unauthorize the holder, unset lsfMPTAuthorized on + // their MPToken + if (txFlags & tfMPTUnauthorize) + flagsOut &= ~lsfMPTAuthorized; + // Issuer wants to authorize a holder, set lsfMPTAuthorized on their + // MPToken + else + flagsOut |= lsfMPTAuthorized; - // Update owner count. - adjustOwnerCount(view(), sleAcct, 1, j_); + if (flagsIn != flagsOut) + sleMpt->setFieldU32(sfFlags, flagsOut); + view().update(sleMpt); return tesSUCCESS; } diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index dd3fcece849..9f3749224f0 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -241,6 +241,10 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize( {.account = &bob, .holder = &bob, .err = temMALFORMED}); + // alice tries to hold onto her own token + mptAlice.authorize({.account= &alice, .err = temMALFORMED}); + + // alice tries to authorize herself mptAlice.authorize({.holder = &alice, .err = temMALFORMED}); // the mpt does not enable allowlisting @@ -420,6 +424,28 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize( {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize}); } + + // Holder can have dangling MPToken even if issuance has been destroyed. + // Make sure they can still delete/unauthorize the MPToken + { + Env env{*this, features}; + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create({.ownerCount = 1}); + + // bob creates a mptoken + mptAlice.authorize({.account = &bob, .holderCount = 1}); + + BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); + BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); + + // alice deletes her issuance + mptAlice.destroy({.ownerCount = 0}); + + // bob can delete his mptoken even though issuance is no longer existent + mptAlice.authorize( + {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize}); + } } void From be824d857a150be1b0d691e458053e60c9094b9f Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 16 Jan 2024 15:02:12 -0500 Subject: [PATCH 24/44] comment --- src/ripple/app/tx/impl/MPTokenAuthorize.cpp | 2 +- src/test/app/MPToken_test.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp index 776402c8ced..6d109cfbce8 100644 --- a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp +++ b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp @@ -168,7 +168,7 @@ MPTokenAuthorize::doApply() } // A potential holder wants to authorize/hold a mpt, the ledger must: - // - add the new mptokenKey to both the owner and mpt directries + // - add the new mptokenKey to the owner directory // - create the MPToken object for the holder std::uint32_t const uOwnerCount = sleAcct->getFieldU32(sfOwnerCount); XRPAmount const reserveCreate( diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 9f3749224f0..c83b871910f 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -273,8 +273,10 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(bob, alice, 100); } + // bob deletes/unauthorizes his MPToken mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); + // bob receives error when he tries to delete his MPToken that has already been deleted mptAlice.authorize( {.account = &bob, .holderCount = 0, From fdd28b3c4b8184e143c1cc15a4ebe931b9a62216 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 16 Jan 2024 15:03:10 -0500 Subject: [PATCH 25/44] clang --- src/ripple/app/tx/impl/MPTokenAuthorize.cpp | 31 +++++++++++---------- src/test/app/MPToken_test.cpp | 10 ++++--- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp index 6d109cfbce8..e51ba75b70d 100644 --- a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp +++ b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp @@ -61,12 +61,14 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) // // Note: `accountID` is holder's account // `holderID` is NOT used - if (!holderID){ - std::shared_ptr sleMpt = - ctx.view.read(keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], accountID)); + if (!holderID) + { + std::shared_ptr sleMpt = ctx.view.read( + keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], accountID)); - // There is an edge case where holder deletes MPT after issuance has already been destroyed. - // So we must check for unauthorize before fetching the MPTIssuance object(since it doesn't exist) + // There is an edge case where holder deletes MPT after issuance has + // already been destroyed. So we must check for unauthorize before + // fetching the MPTIssuance object(since it doesn't exist) // if holder wants to delete/unauthorize a mpt if (txFlags & tfMPTUnauthorize) @@ -76,17 +78,17 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) if ((*sleMpt)[sfMPTAmount] != 0) return tecHAS_OBLIGATIONS; - + return tesSUCCESS; } // Now test when the holder wants to hold/create/authorize a new MPT - sleMptIssuance = + sleMptIssuance = ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID])); if (!sleMptIssuance) return tecOBJECT_NOT_FOUND; - + if (accountID == (*sleMptIssuance)[sfIssuer]) return temMALFORMED; @@ -143,7 +145,8 @@ MPTokenAuthorize::doApply() // If the account that submitted the tx is a holder // Note: `account_` is holder's account // `holderID` is NOT used - if (!holderID){ + if (!holderID) + { // When a holder wants to unauthorize/delete a MPT, the ledger must // - delete mptokenKey from both owner and mpt directories // - delete the MPToken @@ -173,7 +176,7 @@ MPTokenAuthorize::doApply() std::uint32_t const uOwnerCount = sleAcct->getFieldU32(sfOwnerCount); XRPAmount const reserveCreate( (uOwnerCount < 2) ? XRPAmount(beast::zero) - : view().fees().accountReserve(uOwnerCount + 1)); + : view().fees().accountReserve(uOwnerCount + 1)); if (mPriorBalance < reserveCreate) return tecINSUFFICIENT_RESERVE; @@ -199,7 +202,8 @@ MPTokenAuthorize::doApply() return tesSUCCESS; } - std::shared_ptr sleMptIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); + std::shared_ptr sleMptIssuance = + view().read(keylet::mptIssuance(mptIssuanceID)); if (!sleMptIssuance) return tecINTERNAL; @@ -208,12 +212,11 @@ MPTokenAuthorize::doApply() // `holderID` is holder's account if (account_ != (*sleMptIssuance)[sfIssuer]) return tecINTERNAL; - + if (!holderID) return tecINTERNAL; - auto const sleMpt = - view().peek(keylet::mptoken(mptIssuanceID, *holderID)); + auto const sleMpt = view().peek(keylet::mptoken(mptIssuanceID, *holderID)); if (!sleMpt) return tecINTERNAL; diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index c83b871910f..58ff736a55f 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -242,7 +242,7 @@ class MPToken_test : public beast::unit_test::suite {.account = &bob, .holder = &bob, .err = temMALFORMED}); // alice tries to hold onto her own token - mptAlice.authorize({.account= &alice, .err = temMALFORMED}); + mptAlice.authorize({.account = &alice, .err = temMALFORMED}); // alice tries to authorize herself mptAlice.authorize({.holder = &alice, .err = temMALFORMED}); @@ -276,7 +276,8 @@ class MPToken_test : public beast::unit_test::suite // bob deletes/unauthorizes his MPToken mptAlice.authorize({.account = &bob, .flags = tfMPTUnauthorize}); - // bob receives error when he tries to delete his MPToken that has already been deleted + // bob receives error when he tries to delete his MPToken that has + // already been deleted mptAlice.authorize( {.account = &bob, .holderCount = 0, @@ -434,7 +435,7 @@ class MPToken_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {&bob}}); mptAlice.create({.ownerCount = 1}); - + // bob creates a mptoken mptAlice.authorize({.account = &bob, .holderCount = 1}); @@ -444,7 +445,8 @@ class MPToken_test : public beast::unit_test::suite // alice deletes her issuance mptAlice.destroy({.ownerCount = 0}); - // bob can delete his mptoken even though issuance is no longer existent + // bob can delete his mptoken even though issuance is no longer + // existent mptAlice.authorize( {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize}); } From db5032ca2f895af7888f21d8548cd4717544d145 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 16 Jan 2024 18:10:25 -0500 Subject: [PATCH 26/44] comment --- src/ripple/app/tx/impl/MPTokenAuthorize.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp index e51ba75b70d..bfbeaf34f9e 100644 --- a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp +++ b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp @@ -148,7 +148,7 @@ MPTokenAuthorize::doApply() if (!holderID) { // When a holder wants to unauthorize/delete a MPT, the ledger must - // - delete mptokenKey from both owner and mpt directories + // - delete mptokenKey from owner directory // - delete the MPToken if (txFlags & tfMPTUnauthorize) { From ff6c3dde52d499e1b7f4c5576cf660f7be6604e2 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 16 Jan 2024 18:47:48 -0500 Subject: [PATCH 27/44] exceed max amt --- src/ripple/ledger/impl/View.cpp | 5 +++++ src/ripple/protocol/TER.h | 3 ++- src/ripple/protocol/impl/TER.cpp | 1 + src/test/app/MPToken_test.cpp | 17 +++++++++++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index f9298248c07..b78b1ffe96f 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1735,6 +1735,11 @@ rippleMPTCredit( sle->setFieldU64( sfOutstandingAmount, sle->getFieldU64(sfOutstandingAmount) + saAmount.mpt().mpt()); + + // TODO: Replace maxAmt const with a variable + if (sle->getFieldU64(sfOutstandingAmount) > (*sle)[~sfMaximumAmount].value_or(0x7FFFFFFFFFFFFFFFull)) + return tecMPT_MAX_AMOUNT_EXCEEDED; + view.update(sle); } else diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 94a512b3dd8..0e7cd8b2906 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -332,7 +332,8 @@ enum TECcodes : TERUnderlyingType { tecXCHAIN_BAD_PUBLIC_KEY_ACCOUNT_PAIR = 185, tecXCHAIN_CREATE_ACCOUNT_DISABLED = 186, tecEMPTY_DID = 187, - tecMPTOKEN_EXISTS = 188 + tecMPTOKEN_EXISTS = 188, + tecMPT_MAX_AMOUNT_EXCEEDED = 189 }; diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index b254a29f990..c15d6a4c4b4 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -112,6 +112,7 @@ transResults() MAKE_ERROR(tecXCHAIN_CREATE_ACCOUNT_DISABLED, "This bridge does not support account creation."), MAKE_ERROR(tecEMPTY_DID, "The DID object did not have a URI or DIDDocument field."), MAKE_ERROR(tecMPTOKEN_EXISTS, "The account already owns the MPToken object."), + MAKE_ERROR(tecMPT_MAX_AMOUNT_EXCEEDED, "The MPT's maximum amount is exceeded."), MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."), MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."), diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 58ff736a55f..cb037839038 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -759,6 +759,23 @@ class MPToken_test : public beast::unit_test::suite // Holder can send back to issuer mptAlice.pay(bob, alice, 8); } + + // Issuer fails trying to send more than the maximum amount allowed + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create({.ownerCount = 1, .holderCount = 0, .maxAmt = 100}); + + mptAlice.authorize({.account = &bob}); + + // issuer sends holder the max amount allowed + mptAlice.pay(alice, bob, 100); + + // issuer tries to exceed max amount + mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); + } } void From 593898d7ba1d013e70fdd419bccb184ae5f3f872 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 16 Jan 2024 18:49:25 -0500 Subject: [PATCH 28/44] clang --- src/ripple/ledger/impl/View.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index b78b1ffe96f..7fbe3e5728b 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1737,7 +1737,8 @@ rippleMPTCredit( sle->getFieldU64(sfOutstandingAmount) + saAmount.mpt().mpt()); // TODO: Replace maxAmt const with a variable - if (sle->getFieldU64(sfOutstandingAmount) > (*sle)[~sfMaximumAmount].value_or(0x7FFFFFFFFFFFFFFFull)) + if (sle->getFieldU64(sfOutstandingAmount) > + (*sle)[~sfMaximumAmount].value_or(0x7FFFFFFFFFFFFFFFull)) return tecMPT_MAX_AMOUNT_EXCEEDED; view.update(sle); From 8080db980f5cf2f22708c33a80086f23d021acb6 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 16 Jan 2024 19:20:49 -0500 Subject: [PATCH 29/44] use auto const --- src/ripple/app/tx/impl/MPTokenAuthorize.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp index bfbeaf34f9e..4bba77695a1 100644 --- a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp +++ b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp @@ -53,8 +53,6 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) if (holderID && !(ctx.view.exists(keylet::account(*holderID)))) return tecNO_DST; - std::shared_ptr sleMptIssuance; - // if non-issuer account submits this tx, then they are trying either: // 1. Unauthorize/delete MPToken // 2. Use/create MPToken @@ -83,7 +81,7 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) } // Now test when the holder wants to hold/create/authorize a new MPT - sleMptIssuance = + auto const sleMptIssuance = ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID])); if (!sleMptIssuance) @@ -99,7 +97,7 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) return tesSUCCESS; } - sleMptIssuance = + auto const sleMptIssuance = ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID])); if (!sleMptIssuance) return tecOBJECT_NOT_FOUND; @@ -202,7 +200,7 @@ MPTokenAuthorize::doApply() return tesSUCCESS; } - std::shared_ptr sleMptIssuance = + auto const sleMptIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); if (!sleMptIssuance) return tecINTERNAL; From 691dd79b81da3765077805af7b17a10f92083a51 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Tue, 16 Jan 2024 19:22:58 -0500 Subject: [PATCH 30/44] clang --- src/ripple/app/tx/impl/MPTokenAuthorize.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp index 4bba77695a1..5378d764a6b 100644 --- a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp +++ b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp @@ -200,8 +200,7 @@ MPTokenAuthorize::doApply() return tesSUCCESS; } - auto const sleMptIssuance = - view().read(keylet::mptIssuance(mptIssuanceID)); + auto const sleMptIssuance = view().read(keylet::mptIssuance(mptIssuanceID)); if (!sleMptIssuance) return tecINTERNAL; From 36f83f382bf777051671d86028f6fd7aef525327 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 08:53:41 -0500 Subject: [PATCH 31/44] default maxAmt test --- src/test/app/MPToken_test.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index cb037839038..3953685c75b 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -776,6 +776,24 @@ class MPToken_test : public beast::unit_test::suite // issuer tries to exceed max amount mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); } + + // TODO: This test is currently failing! Modify the STAmount to change the range + // Issuer fails trying to send more than the default maximum amount allowed + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob}}); + + mptAlice.create({.ownerCount = 1, .holderCount = 0}); + + mptAlice.authorize({.account = &bob}); + + // issuer sends holder the default max amount allowed + mptAlice.pay(alice, bob, 0x7FFFFFFFFFFFFFFFull); + + // issuer tries to exceed max amount + mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); + } } void From 3fb60a3583145485cac28bcaf2019cedf43404c4 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 08:55:14 -0500 Subject: [PATCH 32/44] clang --- src/test/app/MPToken_test.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 3953685c75b..b6b216eed03 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -777,8 +777,10 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); } - // TODO: This test is currently failing! Modify the STAmount to change the range - // Issuer fails trying to send more than the default maximum amount allowed + // TODO: This test is currently failing! Modify the STAmount to change + // the range + // Issuer fails trying to send more than the default maximum + // amount allowed { Env env{*this, features}; From d8e824593129b6191d626f8acdf2f0225837e07e Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 12:19:04 -0500 Subject: [PATCH 33/44] Address comments --- src/ripple/app/tx/impl/MPTokenAuthorize.cpp | 18 +++++------------- src/test/app/MPToken_test.cpp | 4 ++-- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp index 5378d764a6b..86821266328 100644 --- a/src/ripple/app/tx/impl/MPTokenAuthorize.cpp +++ b/src/ripple/app/tx/impl/MPTokenAuthorize.cpp @@ -47,7 +47,6 @@ TER MPTokenAuthorize::preclaim(PreclaimContext const& ctx) { auto const accountID = ctx.tx[sfAccount]; - auto const txFlags = ctx.tx.getFlags(); auto const holderID = ctx.tx[~sfMPTokenHolder]; if (holderID && !(ctx.view.exists(keylet::account(*holderID)))) @@ -69,10 +68,10 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) // fetching the MPTIssuance object(since it doesn't exist) // if holder wants to delete/unauthorize a mpt - if (txFlags & tfMPTUnauthorize) + if (ctx.tx.getFlags() & tfMPTUnauthorize) { if (!sleMpt) - return tecNO_ENTRY; + return tecOBJECT_NOT_FOUND; if ((*sleMpt)[sfMPTAmount] != 0) return tecHAS_OBLIGATIONS; @@ -114,9 +113,6 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) if (accountID != (*sleMptIssuance)[sfIssuer]) return temMALFORMED; - if (!holderID) - return temMALFORMED; - // If tx is submitted by issuer, it only applies for MPT with // lsfMPTRequireAuth set if (!(mptIssuanceFlags & lsfMPTRequireAuth)) @@ -124,7 +120,7 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx) if (!ctx.view.exists( keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], *holderID))) - return tecNO_ENTRY; + return tecOBJECT_NOT_FOUND; return tesSUCCESS; } @@ -138,7 +134,6 @@ MPTokenAuthorize::doApply() return tecINTERNAL; auto const holderID = ctx_.tx[~sfMPTokenHolder]; - auto const txFlags = ctx_.tx.getFlags(); // If the account that submitted the tx is a holder // Note: `account_` is holder's account @@ -148,7 +143,7 @@ MPTokenAuthorize::doApply() // When a holder wants to unauthorize/delete a MPT, the ledger must // - delete mptokenKey from owner directory // - delete the MPToken - if (txFlags & tfMPTUnauthorize) + if (ctx_.tx.getFlags() & tfMPTUnauthorize) { auto const mptokenKey = keylet::mptoken(mptIssuanceID, account_); auto const sleMpt = view().peek(mptokenKey); @@ -210,9 +205,6 @@ MPTokenAuthorize::doApply() if (account_ != (*sleMptIssuance)[sfIssuer]) return tecINTERNAL; - if (!holderID) - return tecINTERNAL; - auto const sleMpt = view().peek(keylet::mptoken(mptIssuanceID, *holderID)); if (!sleMpt) return tecINTERNAL; @@ -222,7 +214,7 @@ MPTokenAuthorize::doApply() // Issuer wants to unauthorize the holder, unset lsfMPTAuthorized on // their MPToken - if (txFlags & tfMPTUnauthorize) + if (ctx_.tx.getFlags() & tfMPTUnauthorize) flagsOut &= ~lsfMPTAuthorized; // Issuer wants to authorize a holder, set lsfMPTAuthorized on their // MPToken diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index b6b216eed03..44b5bbcf011 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -282,7 +282,7 @@ class MPToken_test : public beast::unit_test::suite {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize, - .err = tecNO_ENTRY}); + .err = tecOBJECT_NOT_FOUND}); } // Test bad scenarios with allow-listing in MPTokenAuthorize (preclaim) @@ -297,7 +297,7 @@ class MPToken_test : public beast::unit_test::suite // alice submits a tx to authorize a holder that hasn't created // a mptoken yet - mptAlice.authorize({.holder = &bob, .err = tecNO_ENTRY}); + mptAlice.authorize({.holder = &bob, .err = tecOBJECT_NOT_FOUND}); // alice specifys a holder acct that doesn't exist mptAlice.authorize({.holder = &cindy, .err = tecNO_DST}); From 9e43eb2cc0775899d2e86628ebf5c39c9dc13fd9 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 12:19:27 -0500 Subject: [PATCH 34/44] clang --- src/test/app/MPToken_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 44b5bbcf011..24dd761ac47 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -778,7 +778,7 @@ class MPToken_test : public beast::unit_test::suite } // TODO: This test is currently failing! Modify the STAmount to change - // the range + // the range // Issuer fails trying to send more than the default maximum // amount allowed { From 791a008fbaeb475084d8d37d11d2a436a73dc4da Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 10:54:02 -0500 Subject: [PATCH 35/44] Invariants and error code --- src/ripple/app/tx/impl/InvariantCheck.cpp | 199 +++++++++++----------- src/ripple/app/tx/impl/InvariantCheck.h | 2 + src/ripple/app/tx/impl/Payment.cpp | 2 +- src/ripple/ledger/impl/View.cpp | 2 +- src/ripple/protocol/Protocol.h | 3 + src/ripple/protocol/TER.h | 3 +- src/ripple/protocol/impl/TER.cpp | 1 + src/test/app/MPToken_test.cpp | 10 +- 8 files changed, 119 insertions(+), 103 deletions(-) diff --git a/src/ripple/app/tx/impl/InvariantCheck.cpp b/src/ripple/app/tx/impl/InvariantCheck.cpp index 05a403863d3..968a8ae1653 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.cpp +++ b/src/ripple/app/tx/impl/InvariantCheck.cpp @@ -815,6 +815,9 @@ ValidMPTIssuance::visitEntry( mptIssuancesDeleted_++; else if (!before) mptIssuancesCreated_++; + + if ((*after)[sfOutstandingAmount] > (*after)[~sfMaximumAmount].value_or(maxMPTokenAmount)) + amountExceededMax_ = true; } if (after && after->getType() == ltMPTOKEN) @@ -834,116 +837,118 @@ ValidMPTIssuance::finalize( ReadView const& _view, beast::Journal const& j) { - if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_CREATE && result == tesSUCCESS) - { - if (mptIssuancesCreated_ == 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " - "succeeded without creating a MPT issuance"; - } - else if (mptIssuancesDeleted_ != 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " - "succeeded while removing MPT issuances"; - } - else if (mptIssuancesCreated_ > 1) + if (result == tesSUCCESS){ + if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_CREATE) { - JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " - "succeeded but created multiple issuances"; - } - - return mptIssuancesCreated_ == 1 && mptIssuancesDeleted_ == 0; - } + if (mptIssuancesCreated_ == 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " + "succeeded without creating a MPT issuance"; + } + else if (mptIssuancesDeleted_ != 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " + "succeeded while removing MPT issuances"; + } + else if (mptIssuancesCreated_ > 1) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " + "succeeded but created multiple issuances"; + } - if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_DESTROY && result == tesSUCCESS) - { - if (mptIssuancesDeleted_ == 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " - "succeeded without removing a MPT issuance"; - } - else if (mptIssuancesCreated_ > 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " - "succeeded while creating MPT issuances"; - } - else if (mptIssuancesDeleted_ > 1) - { - JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " - "succeeded but deleted multiple issuances"; + return mptIssuancesCreated_ == 1 && mptIssuancesDeleted_ == 0; } - return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 1; - } - - if (tx.getTxnType() == ttMPTOKEN_AUTHORIZE && result == tesSUCCESS) - { - bool const submittedByIssuer = tx.isFieldPresent(sfMPTokenHolder); - - if (mptIssuancesCreated_ > 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT authorize " - "succeeded but created MPT issuances"; - return false; - } - else if (mptIssuancesDeleted_ > 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT authorize " - "succeeded but deleted issuances"; - return false; - } - else if ( - submittedByIssuer && (mptokensCreated_ > 0 || mptokensDeleted_ > 0)) + if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_DESTROY) { - JLOG(j.fatal()) - << "Invariant failed: MPT authorize submitted by issuer " - "succeeded but created/deleted mptokens"; - return false; + if (mptIssuancesDeleted_ == 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " + "succeeded without removing a MPT issuance"; + } + else if (mptIssuancesCreated_ > 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " + "succeeded while creating MPT issuances"; + } + else if (mptIssuancesDeleted_ > 1) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " + "succeeded but deleted multiple issuances"; + } + + return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 1; } - else if ( - !submittedByIssuer && (mptokensCreated_ + mptokensDeleted_ != 1)) + + if (tx.getTxnType() == ttMPTOKEN_AUTHORIZE) { - // if the holder submitted this tx, then a mptoken must be either - // created or deleted. - JLOG(j.fatal()) - << "Invariant failed: MPT authorize submitted by holder " - "succeeded but created/deleted bad number of mptokens"; - return false; - } + bool const submittedByIssuer = tx.isFieldPresent(sfMPTokenHolder); - return true; - } + if (mptIssuancesCreated_ > 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT authorize " + "succeeded but created MPT issuances"; + return false; + } + else if (mptIssuancesDeleted_ > 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT authorize " + "succeeded but deleted issuances"; + return false; + } + else if ( + submittedByIssuer && (mptokensCreated_ > 0 || mptokensDeleted_ > 0)) + { + JLOG(j.fatal()) + << "Invariant failed: MPT authorize submitted by issuer " + "succeeded but created/deleted mptokens"; + return false; + } + else if ( + !submittedByIssuer && (mptokensCreated_ + mptokensDeleted_ != 1)) + { + // if the holder submitted this tx, then a mptoken must be either + // created or deleted. + JLOG(j.fatal()) + << "Invariant failed: MPT authorize submitted by holder " + "succeeded but created/deleted bad number of mptokens"; + return false; + } - if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_SET && result == tesSUCCESS) - { - if (mptIssuancesDeleted_ > 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT issuance set " - "succeeded while removing MPT issuances"; - } - else if (mptIssuancesCreated_ > 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT issuance set " - "succeeded while creating MPT issuances"; - } - else if (mptokensDeleted_ > 0) - { - JLOG(j.fatal()) << "Invariant failed: MPT issuance set " - "succeeded while removing MPTokens"; + return true; } - else if (mptokensCreated_ > 0) + + if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_SET ) { - JLOG(j.fatal()) << "Invariant failed: MPT issuance set " - "succeeded while creating MPTokens"; - } + if (mptIssuancesDeleted_ > 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance set " + "succeeded while removing MPT issuances"; + } + else if (mptIssuancesCreated_ > 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance set " + "succeeded while creating MPT issuances"; + } + else if (mptokensDeleted_ > 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance set " + "succeeded while removing MPTokens"; + } + else if (mptokensCreated_ > 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT issuance set " + "succeeded while creating MPTokens"; + } - return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 0 && - mptokensCreated_ == 0 && mptokensDeleted_ == 0; + return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 0 && + mptokensCreated_ == 0 && mptokensDeleted_ == 0; + } } if (mptIssuancesCreated_ != 0) { - JLOG(j.fatal()) << "Invariant failed: a MPT issuance was created"; + JLOG(j.fatal()) << "Invariant failed: a MPT issuance was created"; } else if (mptIssuancesDeleted_ != 0) { @@ -957,9 +962,13 @@ ValidMPTIssuance::finalize( { JLOG(j.fatal()) << "Invariant failed: a MPToken was deleted"; } + else if (amountExceededMax_) + { + JLOG(j.fatal()) << "Invariant failed: OutstandingAmount exceeded MaximumAmount"; + } return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 0 && - mptokensCreated_ == 0 && mptokensDeleted_ == 0; + mptokensCreated_ == 0 && mptokensDeleted_ == 0 && !amountExceededMax_; } } // namespace ripple diff --git a/src/ripple/app/tx/impl/InvariantCheck.h b/src/ripple/app/tx/impl/InvariantCheck.h index d30cc413b2d..e17eda9310c 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.h +++ b/src/ripple/app/tx/impl/InvariantCheck.h @@ -426,6 +426,8 @@ class ValidMPTIssuance std::uint32_t mptokensCreated_ = 0; std::uint32_t mptokensDeleted_ = 0; + bool amountExceededMax_ = false; + public: void visitEntry( diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index 89e90bd0cb0..53dbf1201d1 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -454,7 +454,7 @@ Payment::doApply() if (account_ != issuer && uDstAccountID != issuer && (isFrozen(view(), account_, issue) || isFrozen(view(), uDstAccountID, issue))) - return tecFROZEN; + return tecMPT_LOCKED; PaymentSandbox pv(&view()); auto const res = diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 7fbe3e5728b..00b873d4345 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1738,7 +1738,7 @@ rippleMPTCredit( // TODO: Replace maxAmt const with a variable if (sle->getFieldU64(sfOutstandingAmount) > - (*sle)[~sfMaximumAmount].value_or(0x7FFFFFFFFFFFFFFFull)) + (*sle)[~sfMaximumAmount].value_or(maxMPTokenAmount)) return tecMPT_MAX_AMOUNT_EXCEEDED; view.update(sle); diff --git a/src/ripple/protocol/Protocol.h b/src/ripple/protocol/Protocol.h index 94092696e7b..80fd05c1223 100644 --- a/src/ripple/protocol/Protocol.h +++ b/src/ripple/protocol/Protocol.h @@ -98,6 +98,9 @@ std::size_t constexpr maxDomainLength = 256; /** The maximum length of MPTokenMetadata */ std::size_t constexpr maxMPTokenMetadataLength = 1024; +/** The maximum amount of MPTokenIssuance */ +std::uint64_t constexpr maxMPTokenAmount = 0x7FFFFFFFFFFFFFFFull; + /** A ledger index. */ using LedgerIndex = std::uint32_t; diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 0e7cd8b2906..8e8d8097fc7 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -333,7 +333,8 @@ enum TECcodes : TERUnderlyingType { tecXCHAIN_CREATE_ACCOUNT_DISABLED = 186, tecEMPTY_DID = 187, tecMPTOKEN_EXISTS = 188, - tecMPT_MAX_AMOUNT_EXCEEDED = 189 + tecMPT_MAX_AMOUNT_EXCEEDED = 189, + tecMPT_LOCKED = 190 }; diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index c15d6a4c4b4..9eaf86f5f76 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -113,6 +113,7 @@ transResults() MAKE_ERROR(tecEMPTY_DID, "The DID object did not have a URI or DIDDocument field."), MAKE_ERROR(tecMPTOKEN_EXISTS, "The account already owns the MPToken object."), MAKE_ERROR(tecMPT_MAX_AMOUNT_EXCEEDED, "The MPT's maximum amount is exceeded."), + MAKE_ERROR(tecMPT_LOCKED, "MPT is locked by the issuer."), MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."), MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."), diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 24dd761ac47..f84bdea526f 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -739,8 +739,8 @@ class MPToken_test : public beast::unit_test::suite // Global lock mptAlice.set({.account = &alice, .flags = tfMPTLock}); // Can't send between holders - mptAlice.pay(bob, carol, 1, tecFROZEN); - mptAlice.pay(carol, bob, 2, tecFROZEN); + mptAlice.pay(bob, carol, 1, tecMPT_LOCKED); + mptAlice.pay(carol, bob, 2, tecMPT_LOCKED); // Issuer can send mptAlice.pay(alice, bob, 3); // Holder can send back to issuer @@ -752,8 +752,8 @@ class MPToken_test : public beast::unit_test::suite mptAlice.set( {.account = &alice, .holder = &bob, .flags = tfMPTLock}); // Can't send between holders - mptAlice.pay(bob, carol, 5, tecFROZEN); - mptAlice.pay(carol, bob, 6, tecFROZEN); + mptAlice.pay(bob, carol, 5, tecMPT_LOCKED); + mptAlice.pay(carol, bob, 6, tecMPT_LOCKED); // Issuer can send mptAlice.pay(alice, bob, 7); // Holder can send back to issuer @@ -791,7 +791,7 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize({.account = &bob}); // issuer sends holder the default max amount allowed - mptAlice.pay(alice, bob, 0x7FFFFFFFFFFFFFFFull); + mptAlice.pay(alice, bob, maxMPTokenAmount); // issuer tries to exceed max amount mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); From 5043094165d135831e57069e13a4de639ecb268b Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 12:30:48 -0500 Subject: [PATCH 36/44] clang --- src/ripple/app/tx/impl/InvariantCheck.cpp | 53 +++++++++++++---------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/ripple/app/tx/impl/InvariantCheck.cpp b/src/ripple/app/tx/impl/InvariantCheck.cpp index 968a8ae1653..325ab836ef4 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.cpp +++ b/src/ripple/app/tx/impl/InvariantCheck.cpp @@ -815,8 +815,9 @@ ValidMPTIssuance::visitEntry( mptIssuancesDeleted_++; else if (!before) mptIssuancesCreated_++; - - if ((*after)[sfOutstandingAmount] > (*after)[~sfMaximumAmount].value_or(maxMPTokenAmount)) + + if ((*after)[sfOutstandingAmount] > + (*after)[~sfMaximumAmount].value_or(maxMPTokenAmount)) amountExceededMax_ = true; } @@ -837,23 +838,24 @@ ValidMPTIssuance::finalize( ReadView const& _view, beast::Journal const& j) { - if (result == tesSUCCESS){ + if (result == tesSUCCESS) + { if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_CREATE) { if (mptIssuancesCreated_ == 0) { JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " - "succeeded without creating a MPT issuance"; + "succeeded without creating a MPT issuance"; } else if (mptIssuancesDeleted_ != 0) { JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " - "succeeded while removing MPT issuances"; + "succeeded while removing MPT issuances"; } else if (mptIssuancesCreated_ > 1) { JLOG(j.fatal()) << "Invariant failed: MPT issuance creation " - "succeeded but created multiple issuances"; + "succeeded but created multiple issuances"; } return mptIssuancesCreated_ == 1 && mptIssuancesDeleted_ == 0; @@ -864,17 +866,17 @@ ValidMPTIssuance::finalize( if (mptIssuancesDeleted_ == 0) { JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " - "succeeded without removing a MPT issuance"; + "succeeded without removing a MPT issuance"; } else if (mptIssuancesCreated_ > 0) { JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " - "succeeded while creating MPT issuances"; + "succeeded while creating MPT issuances"; } else if (mptIssuancesDeleted_ > 1) { JLOG(j.fatal()) << "Invariant failed: MPT issuance deletion " - "succeeded but deleted multiple issuances"; + "succeeded but deleted multiple issuances"; } return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 1; @@ -887,58 +889,60 @@ ValidMPTIssuance::finalize( if (mptIssuancesCreated_ > 0) { JLOG(j.fatal()) << "Invariant failed: MPT authorize " - "succeeded but created MPT issuances"; + "succeeded but created MPT issuances"; return false; } else if (mptIssuancesDeleted_ > 0) { JLOG(j.fatal()) << "Invariant failed: MPT authorize " - "succeeded but deleted issuances"; + "succeeded but deleted issuances"; return false; } else if ( - submittedByIssuer && (mptokensCreated_ > 0 || mptokensDeleted_ > 0)) + submittedByIssuer && + (mptokensCreated_ > 0 || mptokensDeleted_ > 0)) { JLOG(j.fatal()) << "Invariant failed: MPT authorize submitted by issuer " - "succeeded but created/deleted mptokens"; + "succeeded but created/deleted mptokens"; return false; } else if ( - !submittedByIssuer && (mptokensCreated_ + mptokensDeleted_ != 1)) + !submittedByIssuer && + (mptokensCreated_ + mptokensDeleted_ != 1)) { - // if the holder submitted this tx, then a mptoken must be either - // created or deleted. + // if the holder submitted this tx, then a mptoken must be + // either created or deleted. JLOG(j.fatal()) << "Invariant failed: MPT authorize submitted by holder " - "succeeded but created/deleted bad number of mptokens"; + "succeeded but created/deleted bad number of mptokens"; return false; } return true; } - if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_SET ) + if (tx.getTxnType() == ttMPTOKEN_ISSUANCE_SET) { if (mptIssuancesDeleted_ > 0) { JLOG(j.fatal()) << "Invariant failed: MPT issuance set " - "succeeded while removing MPT issuances"; + "succeeded while removing MPT issuances"; } else if (mptIssuancesCreated_ > 0) { JLOG(j.fatal()) << "Invariant failed: MPT issuance set " - "succeeded while creating MPT issuances"; + "succeeded while creating MPT issuances"; } else if (mptokensDeleted_ > 0) { JLOG(j.fatal()) << "Invariant failed: MPT issuance set " - "succeeded while removing MPTokens"; + "succeeded while removing MPTokens"; } else if (mptokensCreated_ > 0) { JLOG(j.fatal()) << "Invariant failed: MPT issuance set " - "succeeded while creating MPTokens"; + "succeeded while creating MPTokens"; } return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 0 && @@ -948,7 +952,7 @@ ValidMPTIssuance::finalize( if (mptIssuancesCreated_ != 0) { - JLOG(j.fatal()) << "Invariant failed: a MPT issuance was created"; + JLOG(j.fatal()) << "Invariant failed: a MPT issuance was created"; } else if (mptIssuancesDeleted_ != 0) { @@ -964,7 +968,8 @@ ValidMPTIssuance::finalize( } else if (amountExceededMax_) { - JLOG(j.fatal()) << "Invariant failed: OutstandingAmount exceeded MaximumAmount"; + JLOG(j.fatal()) + << "Invariant failed: OutstandingAmount exceeded MaximumAmount"; } return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 0 && From ded84f828b373e412b7bc6d554c96e81fcb643f9 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 13:53:36 -0500 Subject: [PATCH 37/44] Remove TODO, and replace a missing const --- src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp | 3 +-- src/ripple/ledger/impl/View.cpp | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp b/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp index 3f220dd5f64..9a2e27addf7 100644 --- a/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp +++ b/src/ripple/app/tx/impl/MPTokenIssuanceCreate.cpp @@ -61,8 +61,7 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx) if (maxAmt == 0) return temMALFORMED; - // TODO: Improve this check and move the constant elsewhere (STAmount?) - if (maxAmt > 0x7FFFFFFFFFFFFFFFull) + if (maxAmt > maxMPTokenAmount) return temMALFORMED; } return preflight2(ctx); diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 00b873d4345..7cedbe79ec2 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1736,7 +1736,6 @@ rippleMPTCredit( sfOutstandingAmount, sle->getFieldU64(sfOutstandingAmount) + saAmount.mpt().mpt()); - // TODO: Replace maxAmt const with a variable if (sle->getFieldU64(sfOutstandingAmount) > (*sle)[~sfMaximumAmount].value_or(maxMPTokenAmount)) return tecMPT_MAX_AMOUNT_EXCEEDED; From 1f87af0a8dfa7d8020f4f483ed363aa7eb19943d Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 13:58:52 -0500 Subject: [PATCH 38/44] remove uneeded invariant --- src/ripple/app/tx/impl/InvariantCheck.cpp | 11 +---------- src/ripple/app/tx/impl/InvariantCheck.h | 2 -- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/ripple/app/tx/impl/InvariantCheck.cpp b/src/ripple/app/tx/impl/InvariantCheck.cpp index 325ab836ef4..da01f6e2c7b 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.cpp +++ b/src/ripple/app/tx/impl/InvariantCheck.cpp @@ -815,10 +815,6 @@ ValidMPTIssuance::visitEntry( mptIssuancesDeleted_++; else if (!before) mptIssuancesCreated_++; - - if ((*after)[sfOutstandingAmount] > - (*after)[~sfMaximumAmount].value_or(maxMPTokenAmount)) - amountExceededMax_ = true; } if (after && after->getType() == ltMPTOKEN) @@ -966,14 +962,9 @@ ValidMPTIssuance::finalize( { JLOG(j.fatal()) << "Invariant failed: a MPToken was deleted"; } - else if (amountExceededMax_) - { - JLOG(j.fatal()) - << "Invariant failed: OutstandingAmount exceeded MaximumAmount"; - } return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 0 && - mptokensCreated_ == 0 && mptokensDeleted_ == 0 && !amountExceededMax_; + mptokensCreated_ == 0 && mptokensDeleted_ == 0; } } // namespace ripple diff --git a/src/ripple/app/tx/impl/InvariantCheck.h b/src/ripple/app/tx/impl/InvariantCheck.h index e17eda9310c..d30cc413b2d 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.h +++ b/src/ripple/app/tx/impl/InvariantCheck.h @@ -426,8 +426,6 @@ class ValidMPTIssuance std::uint32_t mptokensCreated_ = 0; std::uint32_t mptokensDeleted_ = 0; - bool amountExceededMax_ = false; - public: void visitEntry( From c3fc6c9a807fc2916dd91a1abc0089f14da91939 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Wed, 17 Jan 2024 14:35:58 -0500 Subject: [PATCH 39/44] fix windows - reorder maxAmt --- src/test/app/MPToken_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index f84bdea526f..aa13f01a4f3 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -766,7 +766,7 @@ class MPToken_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.create({.ownerCount = 1, .holderCount = 0, .maxAmt = 100}); + mptAlice.create({.maxAmt = 100, .ownerCount = 1, .holderCount = 0}); mptAlice.authorize({.account = &bob}); From 94cbc20786bb934b6482a1309bd3ffe24f3adb60 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Thu, 18 Jan 2024 14:24:07 -0500 Subject: [PATCH 40/44] Bundle flag and balance checks into authorize --- src/test/app/MPToken_test.cpp | 24 ------------------------ src/test/jtx/impl/mpt.cpp | 27 +++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index aa13f01a4f3..87ad670f285 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -305,24 +305,19 @@ class MPToken_test : public beast::unit_test::suite // bob now holds a mptoken object mptAlice.authorize({.account = &bob, .holderCount = 1}); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - // alice tries to unauthorize bob. // although tx is successful, // but nothing happens because bob hasn't been authorized yet mptAlice.authorize({.holder = &bob, .flags = tfMPTUnauthorize}); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); // alice authorizes bob // make sure bob's mptoken has set lsfMPTAuthorized mptAlice.authorize({.holder = &bob}); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTAuthorized, &bob)); // alice tries authorizes bob again. // tx is successful, but bob is already authorized, // so no changes mptAlice.authorize({.holder = &bob}); - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTAuthorized, &bob)); // bob deletes his mptoken mptAlice.authorize( @@ -385,9 +380,6 @@ class MPToken_test : public beast::unit_test::suite // bob creates a mptoken mptAlice.authorize({.account = &bob, .holderCount = 1}); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); - // bob deletes his mptoken mptAlice.authorize( {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize}); @@ -405,15 +397,9 @@ class MPToken_test : public beast::unit_test::suite // bob creates a mptoken mptAlice.authorize({.account = &bob, .holderCount = 1}); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); - // alice authorizes bob mptAlice.authorize({.account = &alice, .holder = &bob}); - // make sure bob's mptoken has lsfMPTAuthorized set - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTAuthorized, &bob)); - // Unauthorize bob's mptoken mptAlice.authorize( {.account = &alice, @@ -421,9 +407,6 @@ class MPToken_test : public beast::unit_test::suite .holderCount = 1, .flags = tfMPTUnauthorize}); - // ensure bob's mptoken no longer has lsfMPTAuthorized set - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - mptAlice.authorize( {.account = &bob, .holderCount = 0, .flags = tfMPTUnauthorize}); } @@ -439,9 +422,6 @@ class MPToken_test : public beast::unit_test::suite // bob creates a mptoken mptAlice.authorize({.account = &bob, .holderCount = 1}); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - BEAST_EXPECT(mptAlice.checkMPTokenAmount(bob, 0)); - // alice deletes her issuance mptAlice.destroy({.ownerCount = 0}); @@ -588,10 +568,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize({.account = &bob, .holderCount = 1}); - // both the mptissuance and mptoken are not locked - BEAST_EXPECT(mptAlice.checkFlags(lsfMPTCanLock)); - BEAST_EXPECT(mptAlice.checkFlags(0, &bob)); - // locks bob's mptoken mptAlice.set({.account = &alice, .holder = &bob, .flags = tfMPTLock}); diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index a1ff282d7d3..3dc52fd87a6 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -161,7 +161,24 @@ MPTTester::authorize(MPTAuthorize const& arg) } if (arg.holder) jv[sfMPTokenHolder.jsonName] = arg.holder->human(); - submit(arg, jv); + if (submit(arg, jv) == tesSUCCESS) + { + if (arg.account == nullptr || *arg.account == issuer_) + { + // issuer un-authorizes the holder + if (arg.flags.value_or(0) == tfMPTUnauthorize) + env_.require(mptflags(*this, 0, arg.holder)); + // issuer authorizes the holder + else + env_.require(mptflags(*this, lsfMPTAuthorized, arg.holder)); + } + else if (arg.flags.value_or(0) == 0) + { + // holder creates a token + env_.require(mptflags(*this, 0, arg.account)); + env_.require(mptpay(*this, *arg.account, 0)); + } + } } void @@ -248,7 +265,13 @@ MPTTester::checkMPTokenOutstandingAmount(std::uint64_t expectedAmount) const MPTTester::checkFlags(uint32_t const expectedFlags, AccountP holder_) const { return forObject( - [&](SLEP const& sle) { return expectedFlags == sle->getFlags(); }, + [&](SLEP const& sle) { + auto const flags = sle->getFlags(); + if (flags != expectedFlags) + std::cout << flags << " " << expectedFlags << " " + << (std::uint64_t)holder_ << std::endl; + return expectedFlags == flags; + }, holder_); } From 873f9765a485ce5beb833e7bf13411b31ee8cb0d Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Thu, 18 Jan 2024 17:42:53 -0500 Subject: [PATCH 41/44] comment test --- src/test/app/MPToken_test.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index aa13f01a4f3..5ef38366711 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -781,21 +781,21 @@ class MPToken_test : public beast::unit_test::suite // the range // Issuer fails trying to send more than the default maximum // amount allowed - { - Env env{*this, features}; + // { + // Env env{*this, features}; - MPTTester mptAlice(env, alice, {.holders = {&bob}}); + // MPTTester mptAlice(env, alice, {.holders = {&bob}}); - mptAlice.create({.ownerCount = 1, .holderCount = 0}); + // mptAlice.create({.ownerCount = 1, .holderCount = 0}); - mptAlice.authorize({.account = &bob}); + // mptAlice.authorize({.account = &bob}); - // issuer sends holder the default max amount allowed - mptAlice.pay(alice, bob, maxMPTokenAmount); + // // issuer sends holder the default max amount allowed + // mptAlice.pay(alice, bob, maxMPTokenAmount); - // issuer tries to exceed max amount - mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); - } + // // issuer tries to exceed max amount + // mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); + // } } void From a81d19d31644bae1bd47d2e59b40c415f644454a Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Fri, 19 Jan 2024 17:31:06 -0500 Subject: [PATCH 42/44] Fix the transfer fee and other changes * Add transfer fee unit-test * Refactor flags handling * Check no MPToken on authorization failure --- src/ripple/ledger/impl/View.cpp | 14 ++++++- src/test/app/MPToken_test.cpp | 32 +++++++++++++++ src/test/jtx/impl/mpt.cpp | 70 +++++++++++++++++++++++---------- src/test/jtx/mpt.h | 17 ++++++++ 4 files changed, 111 insertions(+), 22 deletions(-) diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 7cedbe79ec2..ac7648c4a01 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1167,8 +1167,20 @@ rippleSend( saAmount, transferRateMPT( view, static_cast(saAmount.getAsset()))); - return rippleMPTCredit(view, uSenderID, uReceiverID, saActual, j); + + JLOG(j.debug()) << "rippleSend> " << to_string(uSenderID) << " - > " + << to_string(uReceiverID) + << " : deliver=" << saAmount.getFullText() + << " cost=" << saActual.getFullText(); + + if (auto const terResult = + rippleMPTCredit(view, issuer, uReceiverID, saAmount, j); + terResult != tesSUCCESS) + return terResult; + else + return rippleMPTCredit(view, uSenderID, issuer, saActual, j); } + return tecINTERNAL; } diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 5af78b749b9..b634d334616 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -772,6 +772,38 @@ class MPToken_test : public beast::unit_test::suite // // issuer tries to exceed max amount // mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); // } + + // Transfer fee + { + Env env{*this, features}; + + MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); + + // Transfer fee is 10% + mptAlice.create( + {.ownerCount = 1, + .holderCount = 0, + .transferFee = 10'000, + .flags = tfMPTCanTransfer}); + + // Holders create MPToken + mptAlice.authorize({.account = &bob}); + mptAlice.authorize({.account = &carol}); + + // Payment between the issuer and the holder, no transfer fee. + mptAlice.pay(alice, bob, 2'000); + + // Payment between the holder and the issuer, no transfer fee. + mptAlice.pay(alice, bob, 1'000); + + // Payment between the holders. The sender doesn't have + // enough funds to cover the transfer fee. + mptAlice.pay(bob, carol, 1'000); + + // Payment between the holders. The sender pays 10% transfer fee. + std::cout << "test transfer" << std::endl; + mptAlice.pay(bob, carol, 100); + } } void diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 3dc52fd87a6..540b0414615 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -46,6 +46,12 @@ mptpay::operator()(Env& env) const env.test.expect(amount_ == tester_.getAmount(account_)); } +void +requireAny::operator()(Env& env) const +{ + env.test.expect(cb_()); +} + std::unordered_map MPTTester::makeHolders(std::vector const& holders) { @@ -163,22 +169,37 @@ MPTTester::authorize(MPTAuthorize const& arg) jv[sfMPTokenHolder.jsonName] = arg.holder->human(); if (submit(arg, jv) == tesSUCCESS) { + // Issuer authorizes if (arg.account == nullptr || *arg.account == issuer_) { + auto const flags = getFlags(arg.holder); // issuer un-authorizes the holder if (arg.flags.value_or(0) == tfMPTUnauthorize) - env_.require(mptflags(*this, 0, arg.holder)); + env_.require(mptflags(*this, flags, arg.holder)); // issuer authorizes the holder else - env_.require(mptflags(*this, lsfMPTAuthorized, arg.holder)); + env_.require( + mptflags(*this, flags | lsfMPTAuthorized, arg.holder)); } + // Holder authorizes else if (arg.flags.value_or(0) == 0) { + auto const flags = getFlags(arg.account); // holder creates a token - env_.require(mptflags(*this, 0, arg.account)); + env_.require(mptflags(*this, flags, arg.account)); env_.require(mptpay(*this, *arg.account, 0)); } } + else if ( + arg.account != nullptr && *arg.account != issuer_ && + arg.flags.value_or(0) == 0) + { + // Verify MPToken doesn't exist if holder failed authorizing + env_.require(requireAny([&]() -> bool { + return env_.le(keylet::mptoken(*issuanceKey_, arg.account->id())) == + nullptr; + })); + } } void @@ -201,14 +222,8 @@ MPTTester::set(MPTSet const& arg) jv[sfMPTokenHolder.jsonName] = arg.holder->human(); if (submit(arg, jv) == tesSUCCESS && arg.flags.value_or(0)) { - std::uint32_t flags = 0; auto require = [&](AccountP holder, bool unchanged) { - assert(forObject( - [&](SLEP const& sle) { - flags = sle->getFlags(); - return true; - }, - holder)); + auto flags = getFlags(holder); if (!unchanged) { if (*arg.flags & tfMPTLock) @@ -262,17 +277,9 @@ MPTTester::checkMPTokenOutstandingAmount(std::uint64_t expectedAmount) const } [[nodiscard]] bool -MPTTester::checkFlags(uint32_t const expectedFlags, AccountP holder_) const +MPTTester::checkFlags(uint32_t const expectedFlags, AccountP holder) const { - return forObject( - [&](SLEP const& sle) { - auto const flags = sle->getFlags(); - if (flags != expectedFlags) - std::cout << flags << " " << expectedFlags << " " - << (std::uint64_t)holder_ << std::endl; - return expectedFlags == flags; - }, - holder_); + return expectedFlags == getFlags(holder); } void @@ -285,6 +292,7 @@ MPTTester::pay( assert(mpt_); auto const srcAmt = getAmount(src); auto const destAmt = getAmount(dest); + auto const outstnAmt = getAmount(issuer_); if (err) env_(jtx::pay(src, dest, mpt(amount)), ter(*err)); else @@ -305,8 +313,15 @@ MPTTester::pay( } else { - env_.require(mptpay(*this, src, srcAmt - amount)); + STAmount const saAmount = {Issue{*mpt_}, amount}; + STAmount const saActual = + multiply(saAmount, transferRateMPT(*env_.current(), *mpt_)); + // Sender pays the transfer fee if any + env_.require(mptpay(*this, src, srcAmt - saActual.mpt().mpt())); env_.require(mptpay(*this, dest, destAmt + amount)); + // Outstanding amount is reduced by the transfer fee if any + env_.require(mptpay( + *this, issuer_, outstnAmt - (saActual - saAmount).mpt().mpt())); } } @@ -334,6 +349,19 @@ MPTTester::getAmount(Account const& account) const return 0; } +std::uint32_t +MPTTester::getFlags(ripple::test::jtx::AccountP holder) const +{ + std::uint32_t flags = 0; + assert(forObject( + [&](SLEP const& sle) { + flags = sle->getFlags(); + return true; + }, + holder)); + return flags; +} + } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index b7a90391640..12b384c2902 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -72,6 +72,20 @@ class mptpay operator()(Env& env) const; }; +class requireAny +{ +private: + std::function cb_; + +public: + requireAny(std::function const& cb) : cb_(cb) + { + } + + void + operator()(Env& env) const; +}; + struct MPTConstr { std::vector holders = {}; @@ -232,6 +246,9 @@ class MPTTester std::unordered_map makeHolders(std::vector const& holders); + + std::uint32_t + getFlags(AccountP holder) const; }; } // namespace jtx From 1314e78b91d9f47d69714cc08d14bffbcfd94521 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Mon, 22 Jan 2024 10:52:06 -0500 Subject: [PATCH 43/44] fix failed test --- src/test/app/MPToken_test.cpp | 18 +++++++++++++----- src/test/jtx/impl/mpt.cpp | 29 +++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index b634d334616..cdc5bf06d7e 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -40,8 +40,12 @@ class MPToken_test : public beast::unit_test::suite MPTTester mptAlice(env, alice); mptAlice.create({.ownerCount = 0, .err = temDISABLED}); + } - env.enableFeature(featureMPTokensV1); + // test preflight of MPTokenIssuanceCreate + { + Env env{*this, features}; + MPTTester mptAlice(env, alice); mptAlice.create({.flags = 0x00000001, .err = temINVALID_FLAG}); @@ -189,7 +193,7 @@ class MPToken_test : public beast::unit_test::suite Account const alice("alice"); Account const bob("bob"); Account const cindy("cindy"); - // Validate fields in MPTokenAuthorize (preflight) + // Validate amendment enable in MPTokenAuthorize (preflight) { Env env{*this, features - featureMPTokensV1}; MPTTester mptAlice(env, alice, {.holders = {&bob}}); @@ -198,8 +202,12 @@ class MPToken_test : public beast::unit_test::suite {.account = &bob, .id = getMptID(alice, env.seq(alice)), .err = temDISABLED}); + } - env.enableFeature(featureMPTokensV1); + // Validate fields in MPTokenAuthorize (preflight) + { + Env env{*this, features}; + MPTTester mptAlice(env, alice, {.holders = {&bob}}); mptAlice.create({.ownerCount = 1}); @@ -256,13 +264,13 @@ class MPToken_test : public beast::unit_test::suite // bob cannot create the mptoken the second time mptAlice.authorize({.account = &bob, .err = tecMPTOKEN_EXISTS}); - // Check that bob cannot delete CFToken when his balance is + // Check that bob cannot delete MPToken when his balance is // non-zero { // alice pays bob 100 tokens mptAlice.pay(alice, bob, 100); - // bob tries to delete his CFToken, but fails since he still + // bob tries to delete his MPToken, but fails since he still // holds tokens mptAlice.authorize( {.account = &bob, diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 540b0414615..23cb0923aed 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -112,6 +112,11 @@ MPTTester::create(const MPTCreate& arg) jv[sfMaximumAmount.jsonName] = strHex(uint64ToByteArray(*arg.maxAmt)); if (submit(arg, jv) != tesSUCCESS) { + // Verify issuance doesn't exist + env_.require(requireAny([&]() -> bool { + return env_.le(keylet::mptIssuance(*id_)) == nullptr; + })); + id_.reset(); issuanceKey_.reset(); mpt_.reset(); @@ -167,7 +172,7 @@ MPTTester::authorize(MPTAuthorize const& arg) } if (arg.holder) jv[sfMPTokenHolder.jsonName] = arg.holder->human(); - if (submit(arg, jv) == tesSUCCESS) + if (auto const result = submit(arg, jv); result == tesSUCCESS) { // Issuer authorizes if (arg.account == nullptr || *arg.account == issuer_) @@ -194,11 +199,23 @@ MPTTester::authorize(MPTAuthorize const& arg) arg.account != nullptr && *arg.account != issuer_ && arg.flags.value_or(0) == 0) { - // Verify MPToken doesn't exist if holder failed authorizing - env_.require(requireAny([&]() -> bool { - return env_.le(keylet::mptoken(*issuanceKey_, arg.account->id())) == - nullptr; - })); + if (result == tecMPTOKEN_EXISTS) + { + // Verify that MPToken already exists + env_.require(requireAny([&]() -> bool { + return env_.le(keylet::mptoken( + *issuanceKey_, arg.account->id())) != nullptr; + })); + } + else + { + // Verify MPToken doesn't exist if holder failed authorizing(unless + // it already exists) + env_.require(requireAny([&]() -> bool { + return env_.le(keylet::mptoken( + *issuanceKey_, arg.account->id())) == nullptr; + })); + } } } From c3c68fc6a8b33f8f4a27efee27b02d03535f1634 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Mon, 22 Jan 2024 14:11:07 -0500 Subject: [PATCH 44/44] [FOLD] Fix initialization list, remove unneeded logging --- src/test/app/MPToken_test.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index cdc5bf06d7e..574b04e8639 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -789,9 +789,9 @@ class MPToken_test : public beast::unit_test::suite // Transfer fee is 10% mptAlice.create( - {.ownerCount = 1, + {.transferFee = 10'000, + .ownerCount = 1, .holderCount = 0, - .transferFee = 10'000, .flags = tfMPTCanTransfer}); // Holders create MPToken @@ -809,7 +809,6 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(bob, carol, 1'000); // Payment between the holders. The sender pays 10% transfer fee. - std::cout << "test transfer" << std::endl; mptAlice.pay(bob, carol, 100); } }