chore: Bump version to 3.4.1 and make pkg_release 2

.github/workflows/reusable-package.yml

@@ -25,7 +25,7 @@ on:
         description: "The base URL of the Nexus instance hosting the deb and rpm repositories."
         required: false
         type: string
-        default: https://packages.xrplf.org
+        default: https://packages-upload.xrplf.org
 
     secrets:
       remote_username:
@@ -112,7 +112,7 @@ jobs:
         env:
           PACKAGE_TYPE: ${{ matrix.package_type }}
           PACKAGE_VARIANT: ${{ matrix.package_variant }}
-          PKG_RELEASE: ${{ steps.release_info.outputs.pkg_release }}
+          PKG_RELEASE: ${{ startsWith(github.ref, 'refs/tags/') && '2' || steps.release_info.outputs.pkg_release }}
           CHANNEL: ${{ steps.release_info.outputs.channel }}
         run: |
           ./package/build_pkg.py \

include/xrpl/basics/MathUtilities.h

@@ -3,9 +3,84 @@
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
+#include <cstdint>
+#include <limits>
+#include <optional>
 
 namespace xrpl {
 
+/**
+ * Add two signed 64-bit integers, returning std::nullopt when the exact
+ * mathematical sum is not representable in std::int64_t.
+ */
+[[nodiscard]] constexpr std::optional<std::int64_t>
+checkedAdd(std::int64_t a, std::int64_t b) noexcept
+{
+    using L = std::numeric_limits<std::int64_t>;
+
+    if ((b > 0 && a > L::max() - b) || (b < 0 && a < L::min() - b))
+        return std::nullopt;
+
+    return a + b;
+}
+
+/**
+ * Subtract two signed 64-bit integers, returning std::nullopt when the exact
+ * mathematical difference is not representable in std::int64_t.
+ */
+[[nodiscard]] constexpr std::optional<std::int64_t>
+checkedSub(std::int64_t a, std::int64_t b) noexcept
+{
+    using L = std::numeric_limits<std::int64_t>;
+
+    if ((b > 0 && a < L::min() + b) || (b < 0 && a > L::max() + b))
+        return std::nullopt;
+
+    return a - b;
+}
+
+static_assert(checkedAdd(0, 0) == 0);
+static_assert(checkedAdd(1, -1) == 0);
+static_assert(checkedAdd(-5, 2) == -3);
+static_assert(!checkedAdd(std::numeric_limits<std::int64_t>::max(), 1).has_value());
+static_assert(!checkedAdd(std::numeric_limits<std::int64_t>::min(), -1).has_value());
+static_assert(
+    checkedAdd(std::numeric_limits<std::int64_t>::max() - 1, 1) ==
+    std::numeric_limits<std::int64_t>::max());
+static_assert(
+    checkedAdd(
+        std::numeric_limits<std::int64_t>::min(),
+        std::numeric_limits<std::int64_t>::max()) == -1);
+static_assert(
+    checkedAdd(
+        std::numeric_limits<std::int64_t>::max(),
+        std::numeric_limits<std::int64_t>::min()) == -1);
+static_assert(
+    !checkedAdd(std::numeric_limits<std::int64_t>::max(), std::numeric_limits<std::int64_t>::max())
+         .has_value());
+static_assert(
+    !checkedAdd(std::numeric_limits<std::int64_t>::min(), std::numeric_limits<std::int64_t>::min())
+         .has_value());
+
+static_assert(checkedSub(0, 0) == 0);
+static_assert(checkedSub(1, 1) == 0);
+static_assert(checkedSub(-5, 2) == -7);
+static_assert(checkedSub(-5, -2) == -3);
+static_assert(!checkedSub(std::numeric_limits<std::int64_t>::min(), 1).has_value());
+static_assert(!checkedSub(std::numeric_limits<std::int64_t>::max(), -1).has_value());
+static_assert(
+    checkedSub(std::numeric_limits<std::int64_t>::min() + 1, 1) ==
+    std::numeric_limits<std::int64_t>::min());
+static_assert(
+    checkedSub(-1, std::numeric_limits<std::int64_t>::max()) ==
+    std::numeric_limits<std::int64_t>::min());
+static_assert(
+    !checkedSub(std::numeric_limits<std::int64_t>::max(), std::numeric_limits<std::int64_t>::min())
+         .has_value());
+static_assert(
+    !checkedSub(std::numeric_limits<std::int64_t>::min(), std::numeric_limits<std::int64_t>::max())
+         .has_value());
+
 /**
  * Calculate one number divided by another number in percentage.
  * The result is rounded up to the next integer, and capped in the range [0,100]

include/xrpl/protocol/detail/features.macro

@@ -15,6 +15,7 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
+XRPL_FIX    (BatchV1_2,                   Supported::Yes, VoteBehavior::DefaultYes)
 XRPL_FIX    (Cleanup3_4_0,                Supported::Yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(Sponsor,                     Supported::Yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(BatchV1_1,                   Supported::Yes, VoteBehavior::DefaultNo)

include/xrpl/tx/invariants/InvariantCheck.h

@@ -18,6 +18,8 @@
 #include <xrpl/tx/invariants/SponsorshipInvariant.h>
 #include <xrpl/tx/invariants/VaultInvariant.h>
 
+#include <boost/multiprecision/cpp_int.hpp>
+
 #include <cstdint>
 #include <set>
 #include <string>
@@ -139,7 +141,7 @@ public:
  */
 class XRPNotCreated
 {
-    std::int64_t drops_ = 0;
+    boost::multiprecision::int128_t drops_ = 0;
 
 public:
     void

include/xrpl/tx/paths/detail/Steps.h

@@ -1,6 +1,8 @@
 #pragma once
 
+#include <xrpl/basics/MathUtilities.h>
 #include <xrpl/basics/base_uint.h>
+#include <xrpl/basics/contract.h>
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/beast/utility/Zero.h>
 #include <xrpl/protocol/AccountID.h>
@@ -23,6 +25,7 @@
 #include <ostream>
 #include <stdexcept>
 #include <string>
+#include <type_traits>
 #include <utility>
 #include <vector>
 
@@ -502,6 +505,46 @@ public:
 };
 /** @endcond */
 
+/** @cond INTERNAL */
+template <class T>
+[[nodiscard]] std::optional<T>
+checkedStepAddOpt(T const& lhs, T const& rhs)
+{
+    if constexpr (std::is_same_v<T, XRPAmount>)
+    {
+        if (auto const r = checkedAdd(lhs.drops(), rhs.drops()))
+            return XRPAmount{*r};
+        return std::nullopt;
+    }
+    else if constexpr (std::is_same_v<T, MPTAmount>)
+    {
+        if (auto const r = checkedAdd(lhs.value(), rhs.value()))
+            return MPTAmount{*r};
+        return std::nullopt;
+    }
+    else if constexpr (std::is_same_v<T, IOUAmount>)
+    {
+        // IOUAmount is Number-backed and throws on overflow.
+        return lhs + rhs;
+    }
+    else
+    {
+        // A new amount type must decide explicitly how to add; do not fall back
+        // to an unchecked add.
+        static_assert(sizeof(T) == 0, "checkedStepAddOpt: unsupported amount type");
+    }
+}
+
+template <class T>
+[[nodiscard]] T
+checkedStepAdd(T const& lhs, T const& rhs)
+{
+    if (auto const r = checkedStepAddOpt(lhs, rhs))
+        return *r;
+    Throw<FlowException>(tecPATH_DRY);
+}
+/** @endcond */
+
 /** @cond INTERNAL */
 // Check equal with tolerance
 bool

include/xrpl/tx/paths/detail/StrandFlow.h

@@ -31,7 +31,6 @@
 #include <cstdint>
 #include <iterator>
 #include <memory>
-#include <numeric>
 #include <optional>
 #include <tuple>
 #include <type_traits>
@@ -646,11 +645,21 @@ flow(
     boost::container::flat_multiset<TOutAmt> savedOuts;
     savedOuts.reserve(maxTries);
 
-    auto sum = [](auto const& col) {
+    // Returns std::nullopt if the aggregate overflows; callers treat that as a
+    // dry path.
+    auto sum = [](auto const& col) -> std::optional<std::decay_t<decltype(*col.begin())>> {
         using TResult = std::decay_t<decltype(*col.begin())>;
         if (col.empty())
             return TResult{beast::kZero};
-        return std::accumulate(col.begin() + 1, col.end(), *col.begin());
+        TResult total = *col.begin();
+        for (auto it = col.begin() + 1; it != col.end(); ++it)
+        {
+            auto const next = checkedStepAddOpt(total, *it);
+            if (!next)
+                return std::nullopt;
+            total = *next;
+        }
+        return total;
     };
 
     // These offers only need to be removed if the payment is not
@@ -749,9 +758,17 @@ flow(
         {
             savedIns.insert(best->in);
             savedOuts.insert(best->out);
-            remainingOut = outReq - sum(savedOuts);
+            auto const sumOut = sum(savedOuts);
+            if (!sumOut)
+                return {tecPATH_DRY, std::move(ofrsToRmOnFail)};
+            remainingOut = outReq - *sumOut;
             if (sendMax)
-                remainingIn = *sendMax - sum(savedIns);
+            {
+                auto const sumIn = sum(savedIns);
+                if (!sumIn)
+                    return {tecPATH_DRY, std::move(ofrsToRmOnFail)};
+                remainingIn = *sendMax - *sumIn;
+            }
 
             if (flowDebugInfo)
             {
@@ -786,8 +803,12 @@ flow(
             break;
     }
 
-    auto const actualOut = sum(savedOuts);
-    auto const actualIn = sum(savedIns);
+    auto const actualOutOpt = sum(savedOuts);
+    auto const actualInOpt = sum(savedIns);
+    if (!actualOutOpt || !actualInOpt)
+        return {tecPATH_DRY, std::move(ofrsToRmOnFail)};
+    auto const actualOut = *actualOutOpt;
+    auto const actualIn = *actualInOpt;
 
     JLOG(j.trace()) << "Total flow: in: " << to_string(actualIn)
                     << " out: " << to_string(actualOut);

src/libxrpl/ledger/helpers/TokenHelpers.cpp

@@ -1,6 +1,7 @@
 #include <xrpl/ledger/helpers/TokenHelpers.h>
 
 #include <xrpl/basics/Log.h>
+#include <xrpl/basics/MathUtilities.h>
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/beast/utility/Zero.h>
 #include <xrpl/beast/utility/instrumentation.h>
@@ -1155,6 +1156,10 @@ accountSendMultiIOU(
 
         if (receiver)
         {
+            // Confirm the running debit will not overflow before crediting.
+            if (!checkedAdd(takeFromSender.xrp().drops(), amount.xrp().drops()))
+                return tecINTERNAL;
+
             // Increment XRP balance.
             auto const rcvBal = receiver->getFieldAmount(sfBalance);
             receiver->setFieldAmount(sfBalance, rcvBal + amount);
@@ -1162,7 +1167,7 @@ accountSendMultiIOU(
 
             view.update(receiver);
 
-            // Take what is actually sent
+            // Take what is actually sent.
             takeFromSender += amount;
         }
 
@@ -1438,6 +1443,8 @@ directSendNoLimitMultiMPT(
             }
 
             // Direct send: redeeming MPTs and/or sending own MPTs.
+            if (!checkedAdd(actual.mpt().value(), amount.mpt().value()))
+                return tecINTERNAL;
             if (auto const ter = directSendNoFeeMPT(view, senderID, receiverID, amount, j);
                 !isTesSuccess(ter))
                 return ter;
@@ -1451,6 +1458,10 @@ directSendNoLimitMultiMPT(
         STAmount const actualSend = (waiveFee == WaiveTransferFee::Yes)
             ? amount
             : multiply(amount, transferRate(view, amount.get<MPTIssue>().getMptID()));
+        // actual is a superset of takeFromSender, so checking it before both add
+        // sites also protects the debit accumulator.
+        if (!checkedAdd(actual.mpt().value(), actualSend.mpt().value()))
+            return tecINTERNAL;
         actual += actualSend;
         takeFromSender += actualSend;
 

src/libxrpl/protocol/BuildInfo.cpp

@@ -23,7 +23,7 @@ namespace {
 //------------------------------------------------------------------------------
 // clang-format off
 // NOLINTNEXTLINE(readability-identifier-naming)
-char const* const versionString = "3.4.0"
+char const* const versionString = "3.4.1"
     // clang-format on
     ;
 

src/libxrpl/tx/invariants/InvariantCheck.cpp

@@ -210,6 +210,9 @@ XRPNotCreated::finalize(
     ReadView const&,
     beast::Journal const& j) const
 {
+    // drops_ is a full-width running total, so a value that would have wrapped a
+    // 64-bit accumulator is caught here.
+
     // The net change should never be positive, as this would mean that the
     // transaction created XRP out of thin air. That's not possible.
     if (drops_ > 0)

src/libxrpl/tx/paths/BookStep.cpp

@@ -1060,7 +1060,10 @@ sum(TCollection const& col)
     using TResult = std::decay_t<decltype(*col.begin())>;
     if (col.empty())
         return TResult{beast::kZero};
-    return std::accumulate(col.begin() + 1, col.end(), *col.begin());
+    return std::accumulate(
+        col.begin() + 1, col.end(), *col.begin(), [](TResult const& a, TResult const& b) {
+            return checkedStepAdd(a, b);
+        });
 };
 
 template <class TIn, class TOut, class TDerived>

src/libxrpl/tx/transactors/system/Batch.cpp

@@ -9,6 +9,7 @@
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/helpers/SponsorHelpers.h>
 #include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STAccount.h>  // IWYU pragma: keep
@@ -194,6 +195,7 @@ Batch::getFlagsMask(PreflightContext const& ctx)
  * - The batch must contain at least two and no more than the maximum allowed
  * inner transactions.
  * - Each inner transaction must:
+ *   - Be wrapped in a RawTransaction object (with fixBatchV1_2).
  *   - Be unique within the batch.
  *   - Not itself be a Batch transaction.
  *   - Have the tfInnerBatchTxn flag set.
@@ -287,6 +289,14 @@ Batch::preflight(PreflightContext const& ctx)
     for (auto const& stxPtr : ctx.tx.getBatchTransactions())
     {
         STTx const& stx = *stxPtr;
+
+        if (ctx.rules.enabled(fixBatchV1_2) && stx.getFName() != sfRawTransaction)
+        {
+            JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:"
+                                << "txns array may contain only RawTransaction objects.";
+            return temMALFORMED;
+        }
+
         auto const hash = stx.getTransactionID();
         if (!uniqueHashes.emplace(hash).second)
         {

src/test/app/Batch_test.cpp

@@ -5896,6 +5896,81 @@ class Batch_test : public beast::unit_test::Suite
         }
     }
 
+    void
+    testWrappedInnerSubmission(FeatureBitset features)
+    {
+        testcase("wrapper field submission");
+
+        using namespace test::jtx;
+
+        // Object fields with no InnerObjectFormats template, used in place of
+        // sfRawTransaction as the wrapper of each inner transaction.
+        for (SField const* wrapper :
+             {&sfRawTransaction,
+              &sfCreatedNode,
+              &sfModifiedNode,
+              &sfDeletedNode,
+              &sfTemplateEntry,
+              &sfEmitDetails,
+              &sfMemo,
+              &sfFinalFields,
+              &sfNewFields,
+              &sfPreviousFields,
+              &sfTransactionMetaData})
+        {
+            bool const poisoned = (wrapper != &sfRawTransaction);
+
+            auto const alice = Account("alice");
+            auto const bob = Account("bob");
+
+            auto wrap = [&](std::uint32_t s) {
+                json::Value inner = pay(alice, bob, XRP(1));
+                inner[jss::SigningPubKey] = "";
+                inner[jss::Sequence] = s;
+                inner[jss::Fee] = "0";
+                inner[jss::Flags] = tfInnerBatchTxn;
+
+                json::Value wrapped;
+                wrapped[wrapper->jsonName] = inner;
+                return wrapped;
+            };
+
+            auto submit = [&](Env& env, TER expected) {
+                env.fund(XRP(10000), alice, bob);
+                env.close();
+
+                auto const preBob = env.balance(bob);
+                auto const batchFee = batch::calcBatchFee(env, 0, 2);
+                auto const seq = env.seq(alice);
+
+                auto jv = batch::outer(alice, seq, batchFee, tfAllOrNothing);
+                jv[jss::RawTransactions][0u] = wrap(seq + 1);
+                jv[jss::RawTransactions][1u] = wrap(seq + 2);
+
+                env(jv, Ter(expected));
+                env.close();
+
+                return env.balance(bob) == preBob + XRP(2);
+            };
+
+            // Without fixBatchV1_2 every wrapper executes.
+            {
+                Env env{*this, features - fixBatchV1_2};
+                BEAST_EXPECTS(
+                    submit(env, tesSUCCESS),
+                    wrapper->getName() + " did not execute without fixBatchV1_2");
+            }
+
+            // With fixBatchV1_2 only sfRawTransaction is accepted.
+            {
+                Env env{*this, features | fixBatchV1_2};
+                bool const delivered = submit(env, poisoned ? TER{temMALFORMED} : TER{tesSUCCESS});
+                BEAST_EXPECTS(
+                    delivered == !poisoned, wrapper->getName() + " wrong result with fixBatchV1_2");
+            }
+        }
+    }
+
     void
     testWithFeats(FeatureBitset features)
     {
@@ -5935,6 +6010,7 @@ class Batch_test : public beast::unit_test::Suite
         testOuterBinding(features);
         testUnsortedBatchSigners(features);
         testBatchSigCache(features);
+        testWrappedInnerSubmission(features);
     }
 
 public: