Merge branch 'develop' into dangell7/batch-v1

- add early `sfBatchSigners` size check
- fix log nomenclature

.github/workflows/reusable-build-test-config.yml

@@ -76,7 +76,7 @@ jobs:
     name: ${{ inputs.config_name }}
     runs-on: ${{ fromJSON(inputs.runs_on) }}
     container: ${{ inputs.image != '' && inputs.image || null }}
-    timeout-minutes: ${{ inputs.sanitizers != '' && 360 || 60 }}
+    timeout-minutes: 60
     env:
       # Use a namespace to keep the objects separate for each configuration.
       CCACHE_NAMESPACE: ${{ inputs.config_name }}
@@ -204,14 +204,8 @@ jobs:
 
       - name: Set sanitizer options
         if: ${{ !inputs.build_only && env.SANITIZERS_ENABLED == 'true' }}
-        env:
-          CONFIG_NAME: ${{ inputs.config_name }}
         run: |
-          ASAN_OPTS="halt_on_error=0:use_sigaltstack=0:print_stacktrace=1:detect_container_overflow=0:detect_stack_use_after_return=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp"
-          if [[ "${CONFIG_NAME}" == *gcc* ]]; then
-            ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0"
-          fi
-          echo "ASAN_OPTIONS=${ASAN_OPTS}" >> ${GITHUB_ENV}
+          echo "ASAN_OPTIONS=print_stacktrace=1:detect_container_overflow=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" >> ${GITHUB_ENV}
           echo "TSAN_OPTIONS=second_deadlock_stack=1:halt_on_error=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV}
           echo "UBSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV}
           echo "LSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV}
@@ -235,21 +229,8 @@ jobs:
         env:
           BUILD_NPROC: ${{ steps.nproc.outputs.nproc }}
         run: |
-          set -o pipefail
-          ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log
+          ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}"
 
-      - name: Show test failure summary
-        if: ${{ failure() && !inputs.build_only }}
-        working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }}
-        run: |
-          if [ ! -f unittest.log ]; then
-            echo "unittest.log not found; embedded tests may not have run."
-            exit 0
-          fi
-
-          if ! grep -E "failed" unittest.log; then
-            echo "Log present but no failure lines found in unittest.log."
-          fi
       - name: Debug failure (Linux)
         if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }}
         run: |

cmake/XrplConfig.cmake

@@ -17,10 +17,12 @@ find_dependency(Boost
                 chrono
                 container
                 context
+                coroutine
                 date_time
                 filesystem
                 program_options
                 regex
+                system
                 thread)
 #[=========================================================[
   OpenSSL

cmake/XrplInterface.cmake

@@ -22,7 +22,7 @@ target_compile_definitions(
               BOOST_FILESYSTEM_NO_DEPRECATED
               >
               $<$<NOT:$<BOOL:${boost_show_deprecated}>>:
-              BOOST_COROUTINES2_NO_DEPRECATION_WARNING
+              BOOST_COROUTINES_NO_DEPRECATION_WARNING
               BOOST_BEAST_ALLOW_DEPRECATED
               BOOST_FILESYSTEM_DEPRECATED
               >

cmake/deps/Boost.cmake

@@ -4,12 +4,13 @@ include(XrplSanitizers)
 find_package(Boost REQUIRED
              COMPONENTS chrono
                         container
-                        context
+                        coroutine
                         date_time
                         filesystem
                         json
                         program_options
                         regex
+                        system
                         thread)
 
 add_library(xrpl_boost INTERFACE)
@@ -20,7 +21,7 @@ target_link_libraries(
     INTERFACE Boost::headers
               Boost::chrono
               Boost::container
-              Boost::context
+              Boost::coroutine
               Boost::date_time
               Boost::filesystem
               Boost::json
@@ -31,35 +32,14 @@ target_link_libraries(
 if (Boost_COMPILER)
     target_link_libraries(xrpl_boost INTERFACE Boost::disable_autolinking)
 endif ()
-
-# GCC 14+ has a false positive -Wuninitialized warning in Boost.Coroutine2's
-# state.hpp when compiled with -O3. This is due to GCC's intentional behavior
-# change (Bug #98871, #119388) where warnings from inlined system header code
-# are no longer suppressed by -isystem. The warning occurs in operator|= in
-# boost/coroutine2/detail/state.hpp when inlined from push_control_block::destroy().
-# See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119388
-if (is_gcc AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 14)
-    target_compile_options(xrpl_boost INTERFACE -Wno-uninitialized)
+if (SANITIZERS_ENABLED AND is_clang)
+    # TODO: gcc does not support -fsanitize-blacklist...can we do something else for gcc ?
+    if (NOT Boost_INCLUDE_DIRS AND TARGET Boost::headers)
+        get_target_property(Boost_INCLUDE_DIRS Boost::headers INTERFACE_INCLUDE_DIRECTORIES)
+    endif ()
+    message(STATUS "Adding [${Boost_INCLUDE_DIRS}] to sanitizer blacklist")
+    file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/san_bl.txt "src:${Boost_INCLUDE_DIRS}/*")
+    target_compile_options(
+        opts INTERFACE # ignore boost headers for sanitizing
+                       -fsanitize-blacklist=${CMAKE_CURRENT_BINARY_DIR}/san_bl.txt)
 endif ()
-
-# Boost.Context's ucontext backend has ASAN fiber-switching annotations
-# (start/finish_switch_fiber) that are compiled in when BOOST_USE_ASAN is defined.
-# This tells ASAN about coroutine stack switches, preventing false positive
-# stack-use-after-scope errors. BOOST_USE_UCONTEXT ensures the ucontext backend
-# is selected (fcontext does not support ASAN annotations).
-# These defines must match what Boost was compiled with (see conan/profiles/sanitizers).
-if (enable_asan)
-    target_compile_definitions(xrpl_boost INTERFACE BOOST_USE_ASAN BOOST_USE_UCONTEXT)
-endif ()
-
-# if (SANITIZERS_ENABLED AND is_clang)
-#     # TODO: gcc does not support -fsanitize-blacklist...can we do something else for gcc ?
-#     if (NOT Boost_INCLUDE_DIRS AND TARGET Boost::headers)
-#         get_target_property(Boost_INCLUDE_DIRS Boost::headers INTERFACE_INCLUDE_DIRECTORIES)
-#     endif ()
-#     message(STATUS "Adding [${Boost_INCLUDE_DIRS}] to sanitizer blacklist")
-#     file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/san_bl.txt "src:${Boost_INCLUDE_DIRS}/*")
-#     target_compile_options(
-#         opts INTERFACE # ignore boost headers for sanitizing
-#                        -fsanitize-blacklist=${CMAKE_CURRENT_BINARY_DIR}/san_bl.txt)
-# endif ()

conan/profiles/sanitizers

@@ -7,21 +7,16 @@ include(default)
     {% if compiler == "gcc" %}
         {% if "address" in sanitizers or "thread" in sanitizers or "undefinedbehavior" in sanitizers %}
             {% set sanitizer_list = [] %}
-            {% set defines = [] %}
             {% set model_code = "" %}
             {% set extra_cxxflags = ["-fno-omit-frame-pointer", "-O1", "-Wno-stringop-overflow"] %}
 
             {% if "address" in sanitizers %}
                 {% set _ = sanitizer_list.append("address") %}
                 {% set model_code = "-mcmodel=large" %}
-                {% set _ = defines.append("BOOST_USE_ASAN")%}
-                {% set _ = defines.append("BOOST_USE_UCONTEXT")%}
             {% elif "thread" in sanitizers %}
                 {% set _ = sanitizer_list.append("thread") %}
                 {% set model_code = "-mcmodel=medium" %}
                 {% set _ = extra_cxxflags.append("-Wno-tsan") %}
-                {% set _ = defines.append("BOOST_USE_TSAN")%}
-                {% set _ = defines.append("BOOST_USE_UCONTEXT")%}
             {% endif %}
 
             {% if "undefinedbehavior" in sanitizers %}
@@ -34,22 +29,16 @@ include(default)
             tools.build:cxxflags+=['{{sanitizer_flags}} {{" ".join(extra_cxxflags)}}']
             tools.build:sharedlinkflags+=['{{sanitizer_flags}}']
             tools.build:exelinkflags+=['{{sanitizer_flags}}']
-            tools.build:defines+={{defines}}
         {% endif %}
     {% elif compiler == "apple-clang" or compiler == "clang" %}
         {% if "address" in sanitizers or "thread" in sanitizers or "undefinedbehavior" in sanitizers %}
             {% set sanitizer_list = [] %}
-            {% set defines = [] %}
             {% set extra_cxxflags = ["-fno-omit-frame-pointer", "-O1"] %}
 
             {% if "address" in sanitizers %}
                 {% set _ = sanitizer_list.append("address") %}
-                {% set _ = defines.append("BOOST_USE_ASAN")%}
-                {% set _ = defines.append("BOOST_USE_UCONTEXT")%}
             {% elif "thread" in sanitizers %}
                 {% set _ = sanitizer_list.append("thread") %}
-                {% set _ = defines.append("BOOST_USE_TSAN")%}
-                {% set _ = defines.append("BOOST_USE_UCONTEXT")%}
             {% endif %}
 
             {% if "undefinedbehavior" in sanitizers %}
@@ -63,24 +52,8 @@ include(default)
             tools.build:cxxflags+=['{{sanitizer_flags}} {{" ".join(extra_cxxflags)}}']
             tools.build:sharedlinkflags+=['{{sanitizer_flags}}']
             tools.build:exelinkflags+=['{{sanitizer_flags}}']
-            tools.build:defines+={{defines}}
         {% endif %}
     {% endif %}
 {% endif %}
 
-tools.info.package_id:confs+=["tools.build:cxxflags", "tools.build:exelinkflags", "tools.build:sharedlinkflags", "tools.build:defines"]
-
-[options]
-{% if sanitizers %}
-    {% if "address" in sanitizers %}
-        # Build Boost.Context with ucontext backend (not fcontext) so that
-        # ASAN fiber-switching annotations (__sanitizer_start/finish_switch_fiber)
-        # are compiled into the library. fcontext (assembly) has no ASAN support.
-        # define=BOOST_USE_ASAN=1 is critical: it must be defined when building
-        # Boost.Context itself so the ucontext backend compiles in the ASAN annotations.
-        boost/*:extra_b2_flags=context-impl=ucontext address-sanitizer=on define=BOOST_USE_ASAN=1
-        boost/*:without_context=False
-        # Boost stacktrace fails to build with some sanitizers
-        boost/*:without_stacktrace=True
-    {% endif %}
-{% endif %}
+tools.info.package_id:confs+=["tools.build:cxxflags", "tools.build:exelinkflags", "tools.build:sharedlinkflags"]

conanfile.py

@@ -1,5 +1,4 @@
 import re
-import os
 
 from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout
 
@@ -58,9 +57,6 @@ class Xrpl(ConanFile):
         "tests": False,
         "unity": False,
         "xrpld": False,
-        "boost/*:without_context": False,
-        "boost/*:without_coroutine": True,
-        "boost/*:without_coroutine2": False,
         "date/*:header_only": True,
         "ed25519/*:shared": False,
         "grpc/*:shared": False,
@@ -129,14 +125,6 @@ class Xrpl(ConanFile):
             self.options["boost"].visibility = "global"
         if self.settings.compiler in ["clang", "gcc"]:
             self.options["boost"].without_cobalt = True
-        self.options["boost"].without_context = False
-        self.options["boost"].without_coroutine = True
-        self.options["boost"].without_coroutine2 = False
-        # Check if environment variable exists
-        if "SANITIZERS" in os.environ:
-            sanitizers = os.environ["SANITIZERS"]
-            if "address" in sanitizers.lower():
-                self.default_options["fPIC"] = False
 
     def requirements(self):
         # Conan 2 requires transitive headers to be specified
@@ -208,8 +196,7 @@ class Xrpl(ConanFile):
             "boost::headers",
             "boost::chrono",
             "boost::container",
-            "boost::context",
-            "boost::coroutine2",
+            "boost::coroutine",
             "boost::date_time",
             "boost::filesystem",
             "boost::json",

cspell.config.yaml

@@ -99,7 +99,6 @@ words:
   - endmacro
   - exceptioned
   - Falco
-  - fcontext
   - finalizers
   - firewalled
   - fmtdur

include/xrpl/core/Coro.ipp

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <xrpl/basics/ByteUtilities.h>
+
 namespace xrpl {
 
 template <class F>
@@ -9,18 +11,16 @@ JobQueue::Coro::Coro(Coro_create_t, JobQueue& jq, JobType type, std::string cons
     , name_(name)
     , running_(false)
     , coro_(
-          // Stack size of 1MB wasn't sufficient for deep calls. ASAN tests flagged the issue. Hence
-          // increasing the size to 1.5MB.
-          boost::context::protected_fixedsize_stack(1536 * 1024),
           [this, fn = std::forward<F>(f)](
-              boost::coroutines2::asymmetric_coroutine<void>::push_type& do_yield) {
+              boost::coroutines::asymmetric_coroutine<void>::push_type& do_yield) {
               yield_ = &do_yield;
               yield();
               fn(shared_from_this());
 #ifndef NDEBUG
               finished_ = true;
 #endif
-          })
+          },
+          boost::coroutines::attributes(megabytes(1)))
 {
 }
 

include/xrpl/core/JobQueue.h

@@ -7,8 +7,7 @@
 #include <xrpl/core/detail/Workers.h>
 #include <xrpl/json/json_value.h>
 
-#include <boost/context/protected_fixedsize_stack.hpp>
-#include <boost/coroutine2/all.hpp>
+#include <boost/coroutine/all.hpp>
 
 #include <set>
 
@@ -49,8 +48,8 @@ public:
         std::mutex mutex_;
         std::mutex mutex_run_;
         std::condition_variable cv_;
-        boost::coroutines2::coroutine<void>::pull_type coro_;
-        boost::coroutines2::coroutine<void>::push_type* yield_;
+        boost::coroutines::asymmetric_coroutine<void>::pull_type coro_;
+        boost::coroutines::asymmetric_coroutine<void>::push_type* yield_;
 #ifndef NDEBUG
         bool finished_ = false;
 #endif

include/xrpl/protocol/detail/features.macro

@@ -15,10 +15,9 @@
 
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
-
+XRPL_FEATURE(BatchV1_1,                  Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX   (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo)
-XRPL_FIX    (BatchInnerSigs,             Supported::no, VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocol,            Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionDelegationV1_1,   Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (DirectoryLimit,             Supported::yes, VoteBehavior::DefaultNo)
@@ -32,7 +31,6 @@ XRPL_FEATURE(TokenEscrow,                Supported::yes, VoteBehavior::DefaultNo
 XRPL_FIX    (EnforceNFTokenTrustlineV2,  Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (AMMv1_3,                    Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionedDEX,            Supported::yes, VoteBehavior::DefaultNo)
-XRPL_FEATURE(Batch,                      Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(SingleAssetVault,           Supported::yes,  VoteBehavior::DefaultNo)
 XRPL_FIX    (PayChanCancelAfter,         Supported::yes, VoteBehavior::DefaultNo)
 // Check flags in Credential transactions

include/xrpl/protocol/detail/transactions.macro

@@ -918,7 +918,7 @@ TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback,
 #endif
 TRANSACTION(ttBATCH, 71, Batch,
     Delegation::notDelegable,
-    featureBatch,
+    featureBatchV1_1,
     noPriv,
     ({
     {sfRawTransactions, soeREQUIRED},

include/xrpl/tx/Transactor.h

@@ -162,9 +162,6 @@ public:
     static NotTEC
     checkSign(PreclaimContext const& ctx);
 
-    static NotTEC
-    checkBatchSign(PreclaimContext const& ctx);
-
     // Returns the fee in fee units, not scaled for load.
     static XRPAmount
     calculateBaseFee(ReadView const& view, STTx const& tx);
@@ -293,14 +290,7 @@ protected:
         std::optional<T> value,
         unit::ValueUnit<Unit, T> min = unit::ValueUnit<Unit, T>{});
 
-private:
-    std::pair<TER, XRPAmount>
-    reset(XRPAmount fee);
-
-    TER
-    consumeSeqProxy(SLE::pointer const& sleAccount);
-    TER
-    payFee();
+protected:
     static NotTEC
     checkSingleSign(
         ReadView const& view,
@@ -316,6 +306,15 @@ private:
         STObject const& sigObject,
         beast::Journal const j);
 
+private:
+    std::pair<TER, XRPAmount>
+    reset(XRPAmount fee);
+
+    TER
+    consumeSeqProxy(SLE::pointer const& sleAccount);
+    TER
+    payFee();
+
     void trapTransaction(uint256) const;
 
     /** Performs early sanity checks on the account and fee fields.

include/xrpl/tx/transactors/Batch.h

@@ -27,6 +27,9 @@ public:
     static NotTEC
     preflightSigValidated(PreflightContext const& ctx);
 
+    static NotTEC
+    checkBatchSign(PreclaimContext const& ctx);
+
     static NotTEC
     checkSign(PreclaimContext const& ctx);
 

sanitizers/suppressions/asan.supp

@@ -1,7 +1,15 @@
 # The idea is to empty this file gradually by fixing the underlying issues and removing suppressions.
 #
 # ASAN_OPTIONS="print_stacktrace=1:detect_container_overflow=0:suppressions=sanitizers/suppressions/asan.supp:halt_on_error=0"
+#
+# The detect_container_overflow=0 option disables false positives from:
+# - Boost intrusive containers (slist_iterator.hpp, hashtable.hpp, aged_unordered_container.h)
+# - Boost context/coroutine stack switching (Workers.cpp, thread.h)
+#
+# See: https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow
 
+# Boost
+interceptor_name:boost/asio
 
 # Leaks in Doctest tests: xrpl.test.*
 interceptor_name:src/libxrpl/net/HTTPClient.cpp
@@ -12,25 +20,6 @@ interceptor_name:xrpl/net/HTTPClient.h
 interceptor_name:xrpl/net/HTTPClientSSLContext.h
 interceptor_name:xrpl/net/RegisterSSLCerts.h
 
-# Boost.Context fiber/coroutine false positives
-# ASan doesn't fully support makecontext/swapcontext (see first warning in output)
-# The "attempting free on address which was not malloc()-ed" errors are false positives
-# caused by Boost.Context's fiber stack management.
-#
-# Suppress bad-free errors in Boost.Context fiber/coroutine stack deallocation
-# These are triggered when fiber stacks are deallocated but ASan doesn't recognize them
-# as having been allocated via malloc (because they use mmap via boost's stack allocator)
-# We ignore these files from intrumentation already on Clang, but on GCC, we need to suppress the issues.
-
-#interceptor_via_fun:swapcontext
-#interceptor_via_fun:makecontext
-#interceptor_via_fun:boost::context::basic_fixedsize_stack*deallocate
-#interceptor_via_fun:boost::context::fiber::~fiber
-#interceptor_name:boost/context/fiber_ucontext.hpp
-#interceptor_name:boost/context/fixedsize_stack.hpp
-#interceptor_name:Coro.ipp
-
-
 # Suppress false positive stack-buffer errors in thread stack allocation
 # Related to ASan's __asan_handle_no_return warnings (github.com/google/sanitizers/issues/189)
 # These occur during multi-threaded test initialization on macOS

src/libxrpl/protocol/STTx.cpp

@@ -278,6 +278,8 @@ STTx::checkBatchSign(Rules const& rules) const
             JLOG(debugLog().fatal()) << "not a batch transaction";
             return Unexpected("Not a batch transaction.");
         }
+        if (!isFieldPresent(sfBatchSigners))
+            return Unexpected("Missing BatchSigners field.");
         STArray const& signers{getFieldArray(sfBatchSigners)};
         for (auto const& signer : signers)
         {
@@ -292,9 +294,8 @@ STTx::checkBatchSign(Rules const& rules) const
     }
     catch (std::exception const& e)
     {
-        JLOG(debugLog().error()) << "Batch signature check failed: " << e.what();
+        return Unexpected(std::string("Internal batch signature check failure: ") + e.what());
     }
-    return Unexpected("Internal batch signature check failure.");
 }
 
 Json::Value
@@ -416,6 +417,7 @@ STTx::checkBatchSingleSign(STObject const& batchSigner) const
 {
     Serializer msg;
     serializeBatch(msg, getFlags(), getBatchTransactionIDs());
+    finishMultiSigningData(batchSigner.getAccountID(sfAccount), msg);
     return singleSignHelper(batchSigner, msg.slice());
 }
 
@@ -488,7 +490,7 @@ multiSignHelper(
         if (!validSig)
             return Unexpected(
                 std::string("Invalid signature on account ") + toBase58(accountID) +
-                errorWhat.value_or("") + ".");
+                (errorWhat ? ": " + *errorWhat : "") + ".");
     }
     // All signatures verified.
     return {};

src/libxrpl/tx/Transactor.cpp

@@ -175,12 +175,12 @@ Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask)
     if (ctx.tx.getSeqProxy().isTicket() && ctx.tx.isFieldPresent(sfAccountTxnID))
         return temINVALID;
 
-    if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatch))
+    if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatchV1_1))
         return temINVALID_FLAG;
 
     XRPL_ASSERT(
         ctx.tx.isFlag(tfInnerBatchTxn) == ctx.parentBatchId.has_value() ||
-            !ctx.rules.enabled(featureBatch),
+            !ctx.rules.enabled(featureBatchV1_1),
         "Inner batch transaction must have a parent batch ID.");
 
     return tesSUCCESS;
@@ -196,13 +196,13 @@ Transactor::preflight2(PreflightContext const& ctx)
         return *ret;
 
     // It should be impossible for the InnerBatchTxn flag to be set without
-    // featureBatch being enabled
+    // featureBatchV1_1 being enabled
     XRPL_ASSERT_PARTS(
-        !ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatch),
+        !ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatchV1_1),
         "xrpl::Transactor::preflight2",
         "InnerBatch flag only set if feature enabled");
     // Skip signature check on batch inner transactions
-    if (ctx.tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatch))
+    if (ctx.tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatchV1_1))
         return tesSUCCESS;
     // Do not add any checks after this point that are relevant for
     // batch inner transactions. They will be skipped.
@@ -647,7 +647,7 @@ Transactor::checkSign(
 
     auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey);
     // Ignore signature check on batch inner transactions
-    if (parentBatchId && view.rules().enabled(featureBatch))
+    if (parentBatchId && view.rules().enabled(featureBatchV1_1))
     {
         // Defensive Check: These values are also checked in Batch::preflight
         if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||
@@ -699,50 +699,6 @@ Transactor::checkSign(PreclaimContext const& ctx)
     return checkSign(ctx.view, ctx.flags, ctx.parentBatchId, idAccount, ctx.tx, ctx.j);
 }
 
-NotTEC
-Transactor::checkBatchSign(PreclaimContext const& ctx)
-{
-    NotTEC ret = tesSUCCESS;
-    STArray const& signers{ctx.tx.getFieldArray(sfBatchSigners)};
-    for (auto const& signer : signers)
-    {
-        auto const idAccount = signer.getAccountID(sfAccount);
-
-        Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey);
-        if (pkSigner.empty())
-        {
-            if (ret = checkMultiSign(ctx.view, ctx.flags, idAccount, signer, ctx.j);
-                !isTesSuccess(ret))
-                return ret;
-        }
-        else
-        {
-            // LCOV_EXCL_START
-            if (!publicKeyType(makeSlice(pkSigner)))
-                return tefBAD_AUTH;
-            // LCOV_EXCL_STOP
-
-            auto const idSigner = calcAccountID(PublicKey(makeSlice(pkSigner)));
-            auto const sleAccount = ctx.view.read(keylet::account(idAccount));
-
-            // A batch can include transactions from an un-created account ONLY
-            // when the account master key is the signer
-            if (!sleAccount)
-            {
-                if (idAccount != idSigner)
-                    return tefBAD_AUTH;
-
-                return tesSUCCESS;
-            }
-
-            if (ret = checkSingleSign(ctx.view, idSigner, idAccount, sleAccount, ctx.j);
-                !isTesSuccess(ret))
-                return ret;
-        }
-    }
-    return ret;
-}
-
 NotTEC
 Transactor::checkSingleSign(
     ReadView const& view,

src/libxrpl/tx/apply.cpp

@@ -24,29 +24,12 @@ checkValidity(HashRouter& router, STTx const& tx, Rules const& rules)
     auto const flags = router.getFlags(id);
 
     // Ignore signature check on batch inner transactions
-    if (tx.isFlag(tfInnerBatchTxn) && rules.enabled(featureBatch))
+    if (tx.isFlag(tfInnerBatchTxn) && rules.enabled(featureBatchV1_1))
     {
         // Defensive Check: These values are also checked in Batch::preflight
         if (tx.isFieldPresent(sfTxnSignature) || !tx.getSigningPubKey().empty() ||
             tx.isFieldPresent(sfSigners))
             return {Validity::SigBad, "Malformed: Invalid inner batch transaction."};
-
-        // This block should probably have never been included in the
-        // original `Batch` implementation. An inner transaction never
-        // has a valid signature.
-        bool const neverValid = rules.enabled(fixBatchInnerSigs);
-        if (!neverValid)
-        {
-            std::string reason;
-            if (!passesLocalChecks(tx, reason))
-            {
-                router.setFlags(id, SF_LOCALBAD);
-                return {Validity::SigGoodOnly, reason};
-            }
-
-            router.setFlags(id, SF_SIGGOOD);
-            return {Validity::Valid, ""};
-        }
     }
 
     if (any(flags & SF_SIGBAD))

src/libxrpl/tx/transactors/Batch.cpp

@@ -107,7 +107,18 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
             if (signer.isFieldPresent(sfTxnSignature))
                 signerCount += 1;
             else if (signer.isFieldPresent(sfSigners))
-                signerCount += signer.getFieldArray(sfSigners).size();
+            {
+                auto const& nestedSigners = signer.getFieldArray(sfSigners);
+                // LCOV_EXCL_START
+                if (nestedSigners.size() > STTx::maxMultiSigners)
+                {
+                    JLOG(debugLog().error())
+                        << "BatchTrace: Nested Signers array exceeds max entries.";
+                    return XRPAmount{INITIAL_XRP};
+                }
+                // LCOV_EXCL_STOP
+                signerCount += nestedSigners.size();
+            }
         }
     }
 
@@ -205,6 +216,14 @@ Batch::preflight(PreflightContext const& ctx)
         return temARRAY_TOO_LARGE;
     }
 
+    if (ctx.tx.isFieldPresent(sfBatchSigners) &&
+        ctx.tx.getFieldArray(sfBatchSigners).size() > maxBatchTxCount)
+    {
+        JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:"
+                            << "signers array exceeds 8 entries.";
+        return temARRAY_TOO_LARGE;
+    }
+
     // Validation Inner Batch Txns
     std::unordered_set<uint256> uniqueHashes;
     std::unordered_map<AccountID, std::unordered_set<std::uint32_t>> accountSeqTicket;
@@ -426,7 +445,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
             if (requiredSigners.erase(signerAccount) == 0)
             {
                 JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "
-                                    << "no account signature for inner txn.";
+                                    << "extra signer provided: " << signerAccount;
                 return temBAD_SIGNER;
             }
         }
@@ -451,6 +470,54 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
     return tesSUCCESS;
 }
 
+NotTEC
+Batch::checkBatchSign(PreclaimContext const& ctx)
+{
+    NotTEC ret = tesSUCCESS;
+    STArray const& signers{ctx.tx.getFieldArray(sfBatchSigners)};
+    for (auto const& signer : signers)
+    {
+        auto const idAccount = signer.getAccountID(sfAccount);
+
+        Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey);
+        if (pkSigner.empty())
+        {
+            if (ret = checkMultiSign(ctx.view, ctx.flags, idAccount, signer, ctx.j);
+                !isTesSuccess(ret))
+                return ret;
+        }
+        else
+        {
+            // LCOV_EXCL_START
+            if (!publicKeyType(makeSlice(pkSigner)))
+                return tefBAD_AUTH;
+            // LCOV_EXCL_STOP
+
+            auto const idSigner = calcAccountID(PublicKey(makeSlice(pkSigner)));
+            auto const sleAccount = ctx.view.read(keylet::account(idAccount));
+
+            if (sleAccount)
+            {
+                if (isPseudoAccount(sleAccount))
+                    return tefBAD_AUTH;
+
+                if (ret = checkSingleSign(ctx.view, idSigner, idAccount, sleAccount, ctx.j);
+                    !isTesSuccess(ret))
+                    return ret;
+            }
+            else
+            {
+                if (idAccount != idSigner)
+                    return tefBAD_AUTH;
+
+                // A batch can include transactions from an un-created account ONLY
+                // when the account master key is the signer
+            }
+        }
+    }
+    return ret;
+}
+
 /**
  * @brief Checks the validity of signatures for a batch transaction.
  *
@@ -459,7 +526,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
  * corresponding error code.
  *
  * Next, it verifies the batch-specific signature requirements by calling
- * Transactor::checkBatchSign. If this check fails, it also returns the
+ * Batch::checkBatchSign. If this check fails, it also returns the
  * corresponding error code.
  *
  * If both checks succeed, the function returns tesSUCCESS.
@@ -474,8 +541,11 @@ Batch::checkSign(PreclaimContext const& ctx)
     if (auto ret = Transactor::checkSign(ctx); !isTesSuccess(ret))
         return ret;
 
-    if (auto ret = Transactor::checkBatchSign(ctx); !isTesSuccess(ret))
-        return ret;
+    if (ctx.tx.isFieldPresent(sfBatchSigners))
+    {
+        if (auto ret = checkBatchSign(ctx); !isTesSuccess(ret))
+            return ret;
+    }
 
     return tesSUCCESS;
 }

src/libxrpl/tx/transactors/Lending/LoanSet.cpp

@@ -26,7 +26,7 @@ LoanSet::preflight(PreflightContext const& ctx)
     auto const& tx = ctx.tx;
 
     // Special case for Batch inner transactions
-    if (tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatch) &&
+    if (tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatchV1_1) &&
         !tx.isFieldPresent(sfCounterparty))
     {
         auto const parentBatchId = ctx.parentBatchId.value_or(uint256{0});

src/test/app/Batch_test.cpp

@@ -141,14 +141,11 @@ class Batch_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        bool const withInnerSigFix = features[fixBatchInnerSigs];
-
         for (bool const withBatch : {true, false})
         {
-            testcase << "enabled: Batch " << (withBatch ? "enabled" : "disabled")
-                     << ", Inner Sig Fix: " << (withInnerSigFix ? "enabled" : "disabled");
+            testcase << "enabled: Batch " << (withBatch ? "enabled" : "disabled");
 
-            auto const amend = withBatch ? features : features - featureBatch;
+            auto const amend = withBatch ? features : features - featureBatchV1_1;
 
             test::jtx::Env env{*this, amend};
 
@@ -553,6 +550,7 @@ class Batch_test : public beast::unit_test::suite
 
             Serializer msg;
             serializeBatch(msg, tfAllOrNothing, jt.stx->getBatchTransactionIDs());
+            finishMultiSigningData(bob.id(), msg);
             auto const sig = xrpl::sign(bob.pk(), bob.sk(), msg.slice());
             jt.jv[sfBatchSigners.jsonName][0u][sfBatchSigner.jsonName][sfAccount.jsonName] =
                 bob.human();
@@ -1405,7 +1403,7 @@ class Batch_test : public beast::unit_test::suite
             env.close();
         }
 
-        // temARRAY_TOO_LARGE: Batch: signers array exceeds 8 entries.
+        // temARRAY_TOO_LARGE: Batch preflight: signers array exceeds 8 entries.
         {
             test::jtx::Env env{*this, features};
 
@@ -2191,22 +2189,16 @@ class Batch_test : public beast::unit_test::suite
     void
     doTestInnerSubmitRPC(FeatureBitset features, bool withBatch)
     {
-        bool const withInnerSigFix = features[fixBatchInnerSigs];
+        std::string const testName =
+            std::string("inner submit rpc: batch ") + (withBatch ? "enabled" : "disabled") + ": ";
 
-        std::string const testName = [&]() {
-            std::stringstream ss;
-            ss << "inner submit rpc: batch " << (withBatch ? "enabled" : "disabled")
-               << ", inner sig fix: " << (withInnerSigFix ? "enabled" : "disabled") << ": ";
-            return ss.str();
-        }();
-
-        auto const amend = withBatch ? features : features - featureBatch;
+        auto const amend = withBatch ? features : features - featureBatchV1_1;
 
         using namespace test::jtx;
         using namespace std::literals;
 
         test::jtx::Env env{*this, amend};
-        if (!BEAST_EXPECT(amend[featureBatch] == withBatch))
+        if (!BEAST_EXPECT(amend[featureBatchV1_1] == withBatch))
             return;
 
         auto const alice = Account("alice");
@@ -2328,8 +2320,7 @@ class Batch_test : public beast::unit_test::suite
                 s.slice(),
                 __LINE__,
                 "fails local checks: Empty SigningPubKey.",
-                "fails local checks: Empty SigningPubKey.",
-                withBatch && !withInnerSigFix);
+                "fails local checks: Empty SigningPubKey.");
         }
 
         // Invalid RPC Submission: tfInnerBatchTxn pseudo-transaction
@@ -2340,7 +2331,7 @@ class Batch_test : public beast::unit_test::suite
         {
             STTx amendTx(ttAMENDMENT, [seq = env.closed()->header().seq + 1](auto& obj) {
                 obj.setAccountID(sfAccount, AccountID());
-                obj.setFieldH256(sfAmendment, fixBatchInnerSigs);
+                obj.setFieldH256(sfAmendment, featureBatchV1_1);
                 obj.setFieldU32(sfLedgerSequence, seq);
                 obj.setFieldU32(sfFlags, tfInnerBatchTxn);
             });
@@ -2352,8 +2343,7 @@ class Batch_test : public beast::unit_test::suite
                 "Pseudo-transaction",
                 s.slice(),
                 __LINE__,
-                withInnerSigFix ? "fails local checks: Empty SigningPubKey."
-                                : "fails local checks: Cannot submit pseudo transactions.",
+                "fails local checks: Empty SigningPubKey.",
                 "fails local checks: Empty SigningPubKey.");
         }
     }
@@ -2414,6 +2404,53 @@ class Batch_test : public beast::unit_test::suite
         BEAST_EXPECT(env.balance(bob) == XRP(1000));
     }
 
+    void
+    testCheckAllSignatures(FeatureBitset features)
+    {
+        testcase("check all signatures");
+
+        using namespace test::jtx;
+        using namespace std::literals;
+
+        // Verifies that checkBatchSign validates all signers even when an
+        // unfunded account (signed with its master key) appears first in the
+        // sorted signer list. A funded account with an invalid signature must
+        // still be rejected with tefBAD_AUTH.
+
+        test::jtx::Env env{*this, features};
+
+        auto const alice = Account("alice");
+        // "aaa" sorts before other accounts alphabetically, ensuring the
+        // unfunded account is checked first in the sorted signer list
+        auto const unfunded = Account("aaa");
+        auto const carol = Account("carol");
+        env.fund(XRP(10000), alice, carol);
+        env.close();
+
+        // Verify sort order: unfunded.id() < carol.id()
+        BEAST_EXPECT(unfunded.id() < carol.id());
+
+        auto const seq = env.seq(alice);
+        auto const ledSeq = env.current()->seq();
+        auto const batchFee = batch::calcBatchFee(env, 2, 3);
+
+        // The batch includes:
+        // 1. alice pays unfunded (to create unfunded's account)
+        // 2. unfunded does a noop (signed by unfunded's master key - valid)
+        // 3. carol pays alice (signed by alice's key - INVALID since alice is
+        //    not carol's regular key)
+        //
+        // checkBatchSign must validate all signers regardless of order.
+        // This must fail with tefBAD_AUTH.
+        env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
+            batch::inner(pay(alice, unfunded, XRP(100)), seq + 1),
+            batch::inner(noop(unfunded), ledSeq),
+            batch::inner(pay(carol, alice, XRP(1000)), env.seq(carol)),
+            batch::sig(unfunded, Reg{carol, alice}),
+            ter(tefBAD_AUTH));
+        env.close();
+    }
+
     void
     testAccountSet(FeatureBitset features)
     {
@@ -4331,6 +4368,7 @@ class Batch_test : public beast::unit_test::suite
         testIndependent(features);
         testInnerSubmitRPC(features);
         testAccountActivation(features);
+        testCheckAllSignatures(features);
         testAccountSet(features);
         testAccountDelete(features);
         testLoan(features);
@@ -4356,7 +4394,6 @@ public:
     {
         using namespace test::jtx;
         auto const sa = testable_amendments();
-        testWithFeats(sa - fixBatchInnerSigs);
         testWithFeats(sa);
     }
 };

src/test/app/Vault_test.cpp

@@ -5340,20 +5340,20 @@ class Vault_test : public beast::unit_test::suite
             env.close();
 
             // 2. Mantissa larger than uint64 max
-            env.set_parse_failure_expected(true);
             try
             {
                 tx[sfAssetsMaximum] = "18446744073709551617e5";  // uint64 max + 1
                 env(tx, THISLINE);
-                BEAST_EXPECTS(false, "Expected parse_error for mantissa larger than uint64 max");
+                BEAST_EXPECT(false);
             }
             catch (parse_error const& e)
             {
                 using namespace std::string_literals;
                 BEAST_EXPECT(
-                    e.what() == "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."s);
+                    e.what() ==
+                    "invalidParamsField 'tx_json.AssetsMaximum' has invalid "
+                    "data."s);
             }
-            env.set_parse_failure_expected(false);
         }
     }
 

src/test/jtx/impl/batch.cpp

@@ -74,6 +74,7 @@ sig::operator()(Env& env, JTx& jt) const
 
         Serializer msg;
         serializeBatch(msg, stx.getFlags(), stx.getBatchTransactionIDs());
+        finishMultiSigningData(e.acct.id(), msg);
         auto const sig = xrpl::sign(*publicKeyType(e.sig.pk().slice()), e.sig.sk(), msg.slice());
         jo[sfTxnSignature.getJsonName()] = strHex(Slice{sig.data(), sig.size()});
     }

src/test/rpc/Feature_test.cpp

@@ -116,7 +116,7 @@ class Feature_test : public beast::unit_test::suite
         // or removed, swap out for any other feature.
         BEAST_EXPECT(
             featureToName(fixRemoveNFTokenAutoTrustLine) == "fixRemoveNFTokenAutoTrustLine");
-        BEAST_EXPECT(featureToName(featureBatch) == "Batch");
+        BEAST_EXPECT(featureToName(featureBatchV1_1) == "BatchV1_1");
         BEAST_EXPECT(featureToName(featureDID) == "DID");
         BEAST_EXPECT(featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields");
         BEAST_EXPECT(featureToName(featureTokenEscrow) == "TokenEscrow");

src/xrpld/app/misc/NetworkOPs.cpp

@@ -1119,7 +1119,8 @@ NetworkOPsImp::submitTransaction(std::shared_ptr<STTx const> const& iTrans)
     }
 
     // Enforce Network bar for batch txn
-    if (iTrans->isFlag(tfInnerBatchTxn) && m_ledgerMaster.getValidatedRules().enabled(featureBatch))
+    if (iTrans->isFlag(tfInnerBatchTxn) &&
+        m_ledgerMaster.getValidatedRules().enabled(featureBatchV1_1))
     {
         JLOG(m_journal.error()) << "Submitted transaction invalid: tfInnerBatchTxn flag present.";
         return;
@@ -1185,7 +1186,7 @@ NetworkOPsImp::preProcessTransaction(std::shared_ptr<Transaction>& transaction)
     // under no circumstances will we ever accept an inner txn within a batch
     // txn from the network.
     auto const sttx = *transaction->getSTransaction();
-    if (sttx.isFlag(tfInnerBatchTxn) && view->rules().enabled(featureBatch))
+    if (sttx.isFlag(tfInnerBatchTxn) && view->rules().enabled(featureBatchV1_1))
     {
         transaction->setStatus(INVALID);
         transaction->setResult(temINVALID_FLAG);

src/xrpld/overlay/detail/PeerImp.cpp

@@ -1291,7 +1291,7 @@ PeerImp::handleTransaction(
         // Charge strongly for attempting to relay a txn with tfInnerBatchTxn
         // LCOV_EXCL_START
         /*
-           There is no need to check whether the featureBatch amendment is
+           There is no need to check whether the featureBatchV1_1 amendment is
            enabled.
 
            * If the `tfInnerBatchTxn` flag is set, and the amendment is
@@ -2740,7 +2740,7 @@ PeerImp::checkTransaction(
         // charge strongly for relaying batch txns
         // LCOV_EXCL_START
         /*
-           There is no need to check whether the featureBatch amendment is
+           There is no need to check whether the featureBatchV1_1 amendment is
            enabled.
 
            * If the `tfInnerBatchTxn` flag is set, and the amendment is