Merge branch 'develop' into Bronek/vault_enable

include/xrpl/beast/utility/rngfill.h

@@ -48,8 +48,10 @@ rngfill(void* buffer, std::size_t bytes, Generator& g)
 
 #ifdef __GNUC__
     // gcc 11.1 (falsely) warns about an array-bounds overflow in release mode.
+    // gcc 12.1 (also falsely) warns about an string overflow in release mode.
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Warray-bounds"
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
 #endif
 
     if (bytes > 0)

include/xrpl/protocol/detail/features.macro

@@ -35,6 +35,7 @@
 // If you add an amendment here, then do not forget to increment `numFeatures`
 // in include/xrpl/protocol/Feature.h.
 
+XRPL_FIX    (AMMClawbackRounding,        Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(TokenEscrow,                Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (EnforceNFTokenTrustlineV2,  Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (AMMv1_3,                    Supported::yes, VoteBehavior::DefaultNo)

src/libxrpl/protocol/TxMeta.cpp

@@ -185,6 +185,18 @@ TxMeta::getAffectedAccounts() const
                         {
                             auto issuer = lim->getIssuer();
 
+                            if (issuer.isNonZero())
+                                list.insert(issuer);
+                        }
+                    }
+                    else if (field.getFName() == sfMPTokenIssuanceID)
+                    {
+                        auto mptID =
+                            dynamic_cast<STBitString<192> const*>(&field);
+                        if (mptID != nullptr)
+                        {
+                            auto issuer = MPTIssue(mptID->value()).getIssuer();
+
                             if (issuer.isNonZero())
                                 list.insert(issuer);
                         }

src/test/app/TxQ_test.cpp

@@ -99,40 +99,6 @@ class TxQPosNegFlows_test : public beast::unit_test::suite
         return calcMedFeeLevel(feeLevel, feeLevel);
     }
 
-    static std::unique_ptr<Config>
-    makeConfig(
-        std::map<std::string, std::string> extraTxQ = {},
-        std::map<std::string, std::string> extraVoting = {})
-    {
-        auto p = test::jtx::envconfig();
-        auto& section = p->section("transaction_queue");
-        section.set("ledgers_in_queue", "2");
-        section.set("minimum_queue_size", "2");
-        section.set("min_ledgers_to_compute_size_limit", "3");
-        section.set("max_ledger_counts_to_store", "100");
-        section.set("retry_sequence_percent", "25");
-        section.set("normal_consensus_increase_percent", "0");
-
-        for (auto const& [k, v] : extraTxQ)
-            section.set(k, v);
-
-        // Some tests specify different fee settings that are enabled by
-        // a FeeVote
-        if (!extraVoting.empty())
-        {
-            auto& votingSection = p->section("voting");
-            for (auto const& [k, v] : extraVoting)
-            {
-                votingSection.set(k, v);
-            }
-
-            // In order for the vote to occur, we must run as a validator
-            p->section("validation_seed")
-                .legacy("shUwVw52ofnCUX5m7kPTKzJdr4HEH");
-        }
-        return p;
-    }
-
     std::size_t
     initFee(
         jtx::Env& env,

src/test/jtx/envconfig.h

@@ -127,6 +127,11 @@ addGrpcConfigWithSecureGateway(
     std::unique_ptr<Config>,
     std::string const& secureGateway);
 
+std::unique_ptr<Config>
+makeConfig(
+    std::map<std::string, std::string> extraTxQ = {},
+    std::map<std::string, std::string> extraVoting = {});
+
 }  // namespace jtx
 }  // namespace test
 }  // namespace ripple

src/test/jtx/impl/envconfig.cpp

@@ -140,6 +140,39 @@ addGrpcConfigWithSecureGateway(
     return cfg;
 }
 
+std::unique_ptr<Config>
+makeConfig(
+    std::map<std::string, std::string> extraTxQ,
+    std::map<std::string, std::string> extraVoting)
+{
+    auto p = test::jtx::envconfig();
+    auto& section = p->section("transaction_queue");
+    section.set("ledgers_in_queue", "2");
+    section.set("minimum_queue_size", "2");
+    section.set("min_ledgers_to_compute_size_limit", "3");
+    section.set("max_ledger_counts_to_store", "100");
+    section.set("retry_sequence_percent", "25");
+    section.set("normal_consensus_increase_percent", "0");
+
+    for (auto const& [k, v] : extraTxQ)
+        section.set(k, v);
+
+    // Some tests specify different fee settings that are enabled by
+    // a FeeVote
+    if (!extraVoting.empty())
+    {
+        auto& votingSection = p->section("voting");
+        for (auto const& [k, v] : extraVoting)
+        {
+            votingSection.set(k, v);
+        }
+
+        // In order for the vote to occur, we must run as a validator
+        p->section("validation_seed").legacy("shUwVw52ofnCUX5m7kPTKzJdr4HEH");
+    }
+    return p;
+}
+
 }  // namespace jtx
 }  // namespace test
 }  // namespace ripple

src/test/rpc/AccountTx_test.cpp

@@ -18,9 +18,12 @@
 //==============================================================================
 
 #include <test/jtx.h>
+#include <test/jtx/envconfig.h>
 
 #include <xrpl/beast/unit_test.h>
+#include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/protocol/ErrorCodes.h>
+#include <xrpl/protocol/TxFlags.h>
 #include <xrpl/protocol/jss.h>
 
 #include <boost/container/flat_set.hpp>
@@ -753,6 +756,85 @@ class AccountTx_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testMPT()
+    {
+        testcase("MPT");
+
+        using namespace test::jtx;
+        using namespace std::chrono_literals;
+
+        auto cfg = makeConfig();
+        cfg->FEES.reference_fee = 10;
+        Env env(*this, std::move(cfg));
+
+        Account const alice{"alice"};
+        Account const bob{"bob"};
+        Account const carol{"carol"};
+
+        MPTTester mptAlice(env, alice, {.holders = {bob, carol}});
+
+        // check the latest mpt-related txn is in alice's account history
+        auto const checkAliceAcctTx = [&](size_t size,
+                                          Json::StaticString txType) {
+            Json::Value params;
+            params[jss::account] = alice.human();
+            params[jss::limit] = 100;
+            auto const jv =
+                env.rpc("json", "account_tx", to_string(params))[jss::result];
+
+            BEAST_EXPECT(jv[jss::transactions].size() == size);
+            auto const& tx0(jv[jss::transactions][0u][jss::tx]);
+            BEAST_EXPECT(tx0[jss::TransactionType] == txType);
+
+            std::string const txHash{
+                env.tx()->getJson(JsonOptions::none)[jss::hash].asString()};
+            BEAST_EXPECT(tx0[jss::hash] == txHash);
+        };
+
+        // alice creates issuance
+        mptAlice.create(
+            {.ownerCount = 1,
+             .holderCount = 0,
+             .flags = tfMPTCanClawback | tfMPTRequireAuth | tfMPTCanTransfer});
+
+        checkAliceAcctTx(3, jss::MPTokenIssuanceCreate);
+
+        // bob creates a MPToken;
+        mptAlice.authorize({.account = bob});
+        checkAliceAcctTx(4, jss::MPTokenAuthorize);
+        env.close();
+
+        // TODO: windows pipeline fails validation for the hardcoded ledger hash
+        // due to having different test config, it can be uncommented after
+        // figuring out what happened
+        //
+        // ledger hash should be fixed regardless any change to account history
+        // BEAST_EXPECT(
+        //     to_string(env.closed()->info().hash) ==
+        //     "0BD507BB87D3C0E73B462485E6E381798A8C82FC49BF17FE39C60E08A1AF035D");
+
+        // alice authorizes bob
+        mptAlice.authorize({.account = alice, .holder = bob});
+        checkAliceAcctTx(5, jss::MPTokenAuthorize);
+
+        // carol creates a MPToken;
+        mptAlice.authorize({.account = carol});
+        checkAliceAcctTx(6, jss::MPTokenAuthorize);
+
+        // alice authorizes carol
+        mptAlice.authorize({.account = alice, .holder = carol});
+        checkAliceAcctTx(7, jss::MPTokenAuthorize);
+
+        // alice pays bob 100 tokens
+        mptAlice.pay(alice, bob, 100);
+        checkAliceAcctTx(8, jss::Payment);
+
+        // bob pays carol 10 tokens
+        mptAlice.pay(bob, carol, 10);
+        checkAliceAcctTx(9, jss::Payment);
+    }
+
 public:
     void
     run() override
@@ -761,6 +843,7 @@ public:
             std::bind_front(&AccountTx_test::testParameters, this));
         testContents();
         testAccountDelete();
+        testMPT();
     }
 };
 BEAST_DEFINE_TESTSUITE(AccountTx, rpc, ripple);

src/xrpld/app/misc/AMMUtils.h

@@ -125,6 +125,17 @@ isOnlyLiquidityProvider(
     Issue const& ammIssue,
     AccountID const& lpAccount);
 
+/** Due to rounding, the LPTokenBalance of the last LP might
+ * not match the LP's trustline balance. If it's within the tolerance,
+ * update LPTokenBalance to match the LP's trustline balance.
+ */
+Expected<bool, TER>
+verifyAndAdjustLPTokenBalance(
+    Sandbox& sb,
+    STAmount const& lpTokens,
+    std::shared_ptr<SLE>& ammSle,
+    AccountID const& account);
+
 }  // namespace ripple
 
 #endif  // RIPPLE_APP_MISC_AMMUTILS_H_INCLUDED

src/xrpld/app/misc/detail/AMMUtils.cpp

@@ -17,6 +17,7 @@
 */
 //==============================================================================
 
+#include <xrpld/app/misc/AMMHelpers.h>
 #include <xrpld/app/misc/AMMUtils.h>
 #include <xrpld/ledger/Sandbox.h>
 
@@ -464,4 +465,32 @@ isOnlyLiquidityProvider(
     return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
 }
 
+Expected<bool, TER>
+verifyAndAdjustLPTokenBalance(
+    Sandbox& sb,
+    STAmount const& lpTokens,
+    std::shared_ptr<SLE>& ammSle,
+    AccountID const& account)
+{
+    if (auto const res = isOnlyLiquidityProvider(sb, lpTokens.issue(), account);
+        !res)
+        return Unexpected<TER>(res.error());
+    else if (res.value())
+    {
+        if (withinRelativeDistance(
+                lpTokens,
+                ammSle->getFieldAmount(sfLPTokenBalance),
+                Number{1, -3}))
+        {
+            ammSle->setFieldAmount(sfLPTokenBalance, lpTokens);
+            sb.update(ammSle);
+        }
+        else
+        {
+            return Unexpected<TER>(tecAMM_INVALID_TOKENS);
+        }
+    }
+    return true;
+}
+
 }  // namespace ripple

src/xrpld/app/tx/detail/AMMClawback.cpp

@@ -151,6 +151,20 @@ AMMClawback::applyGuts(Sandbox& sb)
     if (!accountSle)
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
+    if (sb.rules().enabled(fixAMMClawbackRounding))
+    {
+        // retrieve LP token balance inside the amendment gate to avoid
+        // inconsistent error behavior
+        auto const lpTokenBalance = ammLPHolds(sb, *ammSle, holder, j_);
+        if (lpTokenBalance == beast::zero)
+            return tecAMM_BALANCE;
+
+        if (auto const res = verifyAndAdjustLPTokenBalance(
+                sb, lpTokenBalance, ammSle, holder);
+            !res)
+            return res.error();  // LCOV_EXCL_LINE
+    }
+
     auto const expected = ammHolds(
         sb,
         *ammSle,
@@ -248,10 +262,11 @@ AMMClawback::equalWithdrawMatchingOneAmount(
     STAmount const& amount)
 {
     auto frac = Number{amount} / amountBalance;
-    auto const amount2Withdraw = amount2Balance * frac;
+    auto amount2Withdraw = amount2Balance * frac;
 
     auto const lpTokensWithdraw =
         toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac);
+
     if (lpTokensWithdraw > holdLPtokens)
         // if lptoken balance less than what the issuer intended to clawback,
         // clawback all the tokens. Because we are doing a two-asset withdrawal,
@@ -272,6 +287,42 @@ AMMClawback::equalWithdrawMatchingOneAmount(
             mPriorBalance,
             ctx_.journal);
 
+    auto const& rules = sb.rules();
+    if (rules.enabled(fixAMMClawbackRounding))
+    {
+        auto tokensAdj =
+            getRoundedLPTokens(rules, lptAMMBalance, frac, IsDeposit::No);
+
+        // LCOV_EXCL_START
+        if (tokensAdj == beast::zero)
+            return {
+                tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, std::nullopt};
+        // LCOV_EXCL_STOP
+
+        frac = adjustFracByTokens(rules, lptAMMBalance, tokensAdj, frac);
+        auto amount2Rounded =
+            getRoundedAsset(rules, amount2Balance, frac, IsDeposit::No);
+
+        auto amountRounded =
+            getRoundedAsset(rules, amountBalance, frac, IsDeposit::No);
+
+        return AMMWithdraw::withdraw(
+            sb,
+            ammSle,
+            ammAccount,
+            holder,
+            amountBalance,
+            amountRounded,
+            amount2Rounded,
+            lptAMMBalance,
+            tokensAdj,
+            0,
+            FreezeHandling::fhIGNORE_FREEZE,
+            WithdrawAll::No,
+            mPriorBalance,
+            ctx_.journal);
+    }
+
     // Because we are doing a two-asset withdrawal,
     // tfee is actually not used, so pass tfee as 0.
     return AMMWithdraw::withdraw(

src/xrpld/app/tx/detail/AMMWithdraw.cpp

@@ -311,24 +311,9 @@ AMMWithdraw::applyGuts(Sandbox& sb)
     if (sb.rules().enabled(fixAMMv1_1))
     {
         if (auto const res =
-                isOnlyLiquidityProvider(sb, lpTokens.issue(), account_);
+                verifyAndAdjustLPTokenBalance(sb, lpTokens, ammSle, account_);
             !res)
             return {res.error(), false};
-        else if (res.value())
-        {
-            if (withinRelativeDistance(
-                    lpTokens,
-                    ammSle->getFieldAmount(sfLPTokenBalance),
-                    Number{1, -3}))
-            {
-                ammSle->setFieldAmount(sfLPTokenBalance, lpTokens);
-                sb.update(ammSle);
-            }
-            else
-            {
-                return {tecAMM_INVALID_TOKENS, false};
-            }
-        }
     }
 
     auto const tfee = getTradingFee(ctx_.view(), *ammSle, account_);