From 10928c9a39eb940fa82d4d801d05933e3ff4ca16 Mon Sep 17 00:00:00 2001 From: Shawn Xie Date: Fri, 5 Jan 2024 15:05:49 -0500 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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;