fix: Inner batch transactions never have valid signatures (#6069 )

- Introduces amendment `fixBatchInnerSigs` - Update Batch unit tests - Fix all the Env instantiations to _use_ the "features" parameter. - testInnerSubmitRPC runs with Batch enabled and disabled. - Add a test to testInnerSubmitRPC for a correctly signed tx incorrectly using the tfInnerBatchTxn flag. - Generalize the submitAndValidate lambda in testInnerSubmitRPC. - With the fix amendment, a transaction never reaches the transaction engine (Transactor and derived classes.) - Test submitting a pseudo-transaction. Stopped before reaching the transaction engine, but with different errors. - The tests verify that without the amendment, a transaction with tfInnerBatchTxn is immediately rejected. Without the amendment, things are safe. The amendment just makes things safer and more future-proof.
fix: Reorder Batch Preflight Errors (#6176 )
2026-01-10 17:55:23 +00:00 · 2026-01-10 00:05:31 -05:00 · 2026-01-09 21:59:23 -05:00
6 changed files with 263 additions and 102 deletions
--- a/include/xrpl/protocol/detail/features.macro
+++ b/include/xrpl/protocol/detail/features.macro
@@ -32,6 +32,7 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.

+XRPL_FIX    (BatchInnerSigs,             Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocol,            Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (DirectoryLimit,             Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (IncludeKeyletFields,        Supported::yes, VoteBehavior::DefaultNo)
--- a/src/test/app/Batch_test.cpp
+++ b/src/test/app/Batch_test.cpp
@@ -166,15 +166,21 @@ class Batch_test : public beast::unit_test::suite
    void
    testEnable(FeatureBitset features)
    {
-        testcase("enabled");
-
        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");
+
            auto const amend = withBatch ? features : features - featureBatch;
-            test::jtx::Env env{*this, envconfig(), amend};
+
+            test::jtx::Env env{*this, amend};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -197,7 +203,7 @@ class Batch_test : public beast::unit_test::suite

            // tfInnerBatchTxn
            // If the feature is disabled, the transaction fails with
-            // temINVALID_FLAG If the feature is enabled, the transaction fails
+            // temINVALID_FLAG. If the feature is enabled, the transaction fails
            // early in checkValidity()
            {
                auto const txResult =
@@ -223,7 +229,7 @@ class Batch_test : public beast::unit_test::suite
        //----------------------------------------------------------------------
        // preflight

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -420,6 +426,38 @@ class Batch_test : public beast::unit_test::suite
            env.close();
        }

+        // temBAD_FEE: Inner txn with negative fee
+        {
+            auto const seq = env.seq(alice);
+            auto const batchFee = batch::calcBatchFee(env, 0, 2);
+            auto tx1 = batch::inner(pay(alice, bob, XRP(1)), seq + 1);
+            tx1[jss::Fee] = "-1";
+            env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                tx1,
+                batch::inner(pay(alice, bob, XRP(2)), seq + 2),
+                ter(temBAD_FEE));
+            env.close();
+        }
+
+        // temBAD_FEE: Inner txn with non-integer fee
+        {
+            auto const seq = env.seq(alice);
+            auto const batchFee = batch::calcBatchFee(env, 0, 2);
+            auto tx1 = batch::inner(pay(alice, bob, XRP(1)), seq + 1);
+            tx1[jss::Fee] = "1.5";
+            try
+            {
+                env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                    tx1,
+                    batch::inner(pay(alice, bob, XRP(2)), seq + 2));
+                fail("Expected parse_error for fractional fee");
+            }
+            catch (jtx::parse_error const&)
+            {
+                BEAST_EXPECT(true);
+            }
+        }
+
        // temSEQ_AND_TICKET: Batch: inner txn cannot have both Sequence
        // and TicketSequence.
        {
@@ -603,7 +641,7 @@ class Batch_test : public beast::unit_test::suite
        //----------------------------------------------------------------------
        // preclaim

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -844,7 +882,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -935,7 +973,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -1173,7 +1211,7 @@ class Batch_test : public beast::unit_test::suite

        // Bad Fee Without Signer
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1195,7 +1233,7 @@ class Batch_test : public beast::unit_test::suite

        // Bad Fee With MultiSign
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1222,7 +1260,7 @@ class Batch_test : public beast::unit_test::suite

        // Bad Fee With MultiSign + BatchSigners
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1251,7 +1289,7 @@ class Batch_test : public beast::unit_test::suite

        // Bad Fee With MultiSign + BatchSigners.Signers
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1283,7 +1321,7 @@ class Batch_test : public beast::unit_test::suite

        // Bad Fee With BatchSigners
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1307,7 +1345,7 @@ class Batch_test : public beast::unit_test::suite

        // Bad Fee Dynamic Fee Calculation
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1347,7 +1385,7 @@ class Batch_test : public beast::unit_test::suite

        // telENV_RPC_FAILED: Batch: txns array exceeds 8 entries.
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1372,7 +1410,7 @@ class Batch_test : public beast::unit_test::suite

        // temARRAY_TOO_LARGE: Batch: txns array exceeds 8 entries.
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1405,7 +1443,7 @@ class Batch_test : public beast::unit_test::suite

        // telENV_RPC_FAILED: Batch: signers array exceeds 8 entries.
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1424,7 +1462,7 @@ class Batch_test : public beast::unit_test::suite

        // temARRAY_TOO_LARGE: Batch: signers array exceeds 8 entries.
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -1458,7 +1496,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -1594,7 +1632,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -1826,7 +1864,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -2048,7 +2086,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -2234,14 +2272,26 @@ class Batch_test : public beast::unit_test::suite
    }

    void
-    testInnerSubmitRPC(FeatureBitset features)
+    doTestInnerSubmitRPC(FeatureBitset features, bool withBatch)
    {
-        testcase("inner submit rpc");
+        bool const withInnerSigFix = features[fixBatchInnerSigs];
+
+        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;

        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, amend};
+        if (!BEAST_EXPECT(amend[featureBatch] == withBatch))
+            return;

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -2249,75 +2299,170 @@ class Batch_test : public beast::unit_test::suite
        env.fund(XRP(10000), alice, bob);
        env.close();

-        auto submitAndValidate = [&](Slice const& slice) {
-            auto const jrr = env.rpc("submit", strHex(slice))[jss::result];
-            BEAST_EXPECT(
-                jrr[jss::status] == "error" &&
-                jrr[jss::error] == "invalidTransaction" &&
-                jrr[jss::error_exception] ==
-                    "fails local checks: Malformed: Invalid inner batch "
-                    "transaction.");
-            env.close();
-        };
+        auto submitAndValidate =
+            [&](std::string caseName,
+                Slice const& slice,
+                int line,
+                std::optional<std::string> expectedEnabled = std::nullopt,
+                std::optional<std::string> expectedDisabled = std::nullopt,
+                bool expectInvalidFlag = false) {
+                testcase << testName << caseName
+                         << (expectInvalidFlag
+                                 ? " - Expected to reach tx engine!"
+                                 : "");
+                auto const jrr = env.rpc("submit", strHex(slice))[jss::result];
+                auto const expected = withBatch
+                    ? expectedEnabled.value_or(
+                          "fails local checks: Malformed: Invalid inner batch "
+                          "transaction.")
+                    : expectedDisabled.value_or(
+                          "fails local checks: Empty SigningPubKey.");
+                if (expectInvalidFlag)
+                {
+                    expect(
+                        jrr[jss::status] == "success" &&
+                            jrr[jss::engine_result] == "temINVALID_FLAG",
+                        pretty(jrr),
+                        __FILE__,
+                        line);
+                }
+                else
+                {
+                    expect(
+                        jrr[jss::status] == "error" &&
+                            jrr[jss::error] == "invalidTransaction" &&
+                            jrr[jss::error_exception] == expected,
+                        pretty(jrr),
+                        __FILE__,
+                        line);
+                }
+                env.close();
+            };

        // Invalid RPC Submission: TxnSignature
-        // - has `TxnSignature` field
+        // + has `TxnSignature` field
        // - has no `SigningPubKey` field
        // - has no `Signers` field
-        // - has `tfInnerBatchTxn` flag
+        // + has `tfInnerBatchTxn` flag
        {
            auto txn = batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
            txn[sfTxnSignature] = "DEADBEEF";
            STParsedJSONObject parsed("test", txn.getTxn());
            Serializer s;
            parsed.object->add(s);
-            submitAndValidate(s.slice());
+            submitAndValidate("TxnSignature set", s.slice(), __LINE__);
        }

        // Invalid RPC Submission: SigningPubKey
        // - has no `TxnSignature` field
-        // - has `SigningPubKey` field
+        // + has `SigningPubKey` field
        // - has no `Signers` field
-        // - has `tfInnerBatchTxn` flag
+        // + has `tfInnerBatchTxn` flag
        {
            auto txn = batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
            txn[sfSigningPubKey] = strHex(alice.pk());
            STParsedJSONObject parsed("test", txn.getTxn());
            Serializer s;
            parsed.object->add(s);
-            submitAndValidate(s.slice());
+            submitAndValidate(
+                "SigningPubKey set",
+                s.slice(),
+                __LINE__,
+                std::nullopt,
+                "fails local checks: Invalid signature.");
        }

        // Invalid RPC Submission: Signers
        // - has no `TxnSignature` field
-        // - has empty `SigningPubKey` field
-        // - has `Signers` field
-        // - has `tfInnerBatchTxn` flag
+        // + has empty `SigningPubKey` field
+        // + has `Signers` field
+        // + has `tfInnerBatchTxn` flag
        {
            auto txn = batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
            txn[sfSigners] = Json::arrayValue;
            STParsedJSONObject parsed("test", txn.getTxn());
            Serializer s;
            parsed.object->add(s);
-            submitAndValidate(s.slice());
+            submitAndValidate(
+                "Signers set",
+                s.slice(),
+                __LINE__,
+                std::nullopt,
+                "fails local checks: Invalid Signers array size.");
+        }
+
+        {
+            // Fully signed inner batch transaction
+            auto const txn =
+                batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
+            auto const jt = env.jt(txn.getTxn());
+
+            STParsedJSONObject parsed("test", jt.jv);
+            Serializer s;
+            parsed.object->add(s);
+            submitAndValidate(
+                "Fully signed",
+                s.slice(),
+                __LINE__,
+                std::nullopt,
+                std::nullopt,
+                !withBatch);
        }

        // Invalid RPC Submission: tfInnerBatchTxn
        // - has no `TxnSignature` field
-        // - has empty `SigningPubKey` field
+        // + has empty `SigningPubKey` field
        // - has no `Signers` field
-        // - has `tfInnerBatchTxn` flag
+        // + has `tfInnerBatchTxn` flag
        {
            auto txn = batch::inner(pay(alice, bob, XRP(1)), env.seq(alice));
            STParsedJSONObject parsed("test", txn.getTxn());
            Serializer s;
            parsed.object->add(s);
-            auto const jrr = env.rpc("submit", strHex(s.slice()))[jss::result];
-            BEAST_EXPECT(
-                jrr[jss::status] == "success" &&
-                jrr[jss::engine_result] == "temINVALID_FLAG");
+            submitAndValidate(
+                "No signing fields set",
+                s.slice(),
+                __LINE__,
+                "fails local checks: Empty SigningPubKey.",
+                "fails local checks: Empty SigningPubKey.",
+                withBatch && !withInnerSigFix);
+        }

-            env.close();
+        // Invalid RPC Submission: tfInnerBatchTxn pseudo-transaction
+        // - has no `TxnSignature` field
+        // + has empty `SigningPubKey` field
+        // - has no `Signers` field
+        // + has `tfInnerBatchTxn` flag
+        {
+            STTx amendTx(
+                ttAMENDMENT, [seq = env.closed()->info().seq + 1](auto& obj) {
+                    obj.setAccountID(sfAccount, AccountID());
+                    obj.setFieldH256(sfAmendment, fixBatchInnerSigs);
+                    obj.setFieldU32(sfLedgerSequence, seq);
+                    obj.setFieldU32(sfFlags, tfInnerBatchTxn);
+                });
+            auto txn = batch::inner(
+                amendTx.getJson(JsonOptions::none), env.seq(alice));
+            STParsedJSONObject parsed("test", txn.getTxn());
+            Serializer s;
+            parsed.object->add(s);
+            submitAndValidate(
+                "Pseudo-transaction",
+                s.slice(),
+                __LINE__,
+                withInnerSigFix
+                    ? "fails local checks: Empty SigningPubKey."
+                    : "fails local checks: Cannot submit pseudo transactions.",
+                "fails local checks: Empty SigningPubKey.");
+        }
+    }
+
+    void
+    testInnerSubmitRPC(FeatureBitset features)
+    {
+        for (bool const withBatch : {true, false})
+        {
+            doTestInnerSubmitRPC(features, withBatch);
        }
    }

@@ -2329,7 +2474,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -2376,7 +2521,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -2429,7 +2574,7 @@ class Batch_test : public beast::unit_test::suite

        // tfIndependent: account delete success
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -2470,7 +2615,7 @@ class Batch_test : public beast::unit_test::suite

        // tfIndependent: account delete fails
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -2515,7 +2660,7 @@ class Batch_test : public beast::unit_test::suite

        // tfAllOrNothing: account delete fails
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -2567,7 +2712,6 @@ class Batch_test : public beast::unit_test::suite

        test::jtx::Env env{
            *this,
-            envconfig(),
            features | featureSingleAssetVault | featureLendingProtocol |
                featureMPTokensV1};

@@ -2762,7 +2906,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -2875,7 +3019,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -2933,7 +3077,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -2995,7 +3139,7 @@ class Batch_test : public beast::unit_test::suite
            using namespace test::jtx;
            using namespace std::literals;

-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -3044,7 +3188,7 @@ class Batch_test : public beast::unit_test::suite
            using namespace test::jtx;
            using namespace std::literals;

-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -3092,7 +3236,7 @@ class Batch_test : public beast::unit_test::suite
            using namespace test::jtx;
            using namespace std::literals;

-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -3155,7 +3299,7 @@ class Batch_test : public beast::unit_test::suite
            // overwritten by the payment in the batch transaction. Because the
            // terPRE_SEQ is outside of the batch this noop transaction will ge
            // reapplied in the following ledger
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob, carol);
            env.close();

@@ -3202,7 +3346,7 @@ class Batch_test : public beast::unit_test::suite
            // IMPORTANT: The batch txn is applied first, then the noop txn.
            // Because of this ordering, the noop txn is not applied and is
            // overwritten by the payment in the batch transaction.
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob);
            env.close();

@@ -3244,7 +3388,7 @@ class Batch_test : public beast::unit_test::suite
            // IMPORTANT: The batch txn is applied first, then the noop txn.
            // Because of this ordering, the noop txn is not applied and is
            // overwritten by the payment in the batch transaction.
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob);
            env.close();

@@ -3281,7 +3425,7 @@ class Batch_test : public beast::unit_test::suite

        // Outer Batch terPRE_SEQ
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob, carol);
            env.close();

@@ -3339,7 +3483,7 @@ class Batch_test : public beast::unit_test::suite
            // IMPORTANT: The batch txn is applied first, then the noop txn.
            // Because of this ordering, the noop txn is not applied and is
            // overwritten by the payment in the batch transaction.
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob);
            env.close();

@@ -3388,7 +3532,7 @@ class Batch_test : public beast::unit_test::suite
            // IMPORTANT: The batch txn is applied first, then the noop txn.
            // Because of this ordering, the noop txn is not applied and is
            // overwritten by the payment in the batch transaction.
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob);
            env.close();

@@ -3450,7 +3594,7 @@ class Batch_test : public beast::unit_test::suite
            // batch will run in the close ledger process. The batch will be
            // allied and then retry this transaction in the current ledger.

-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob);
            env.close();

@@ -3497,7 +3641,7 @@ class Batch_test : public beast::unit_test::suite

        // Create Object Before Batch Txn
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob);
            env.close();

@@ -3544,7 +3688,7 @@ class Batch_test : public beast::unit_test::suite
            // batch will run in the close ledger process. The batch will be
            // applied and then retry this transaction in the current ledger.

-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            env.fund(XRP(10000), alice, bob);
            env.close();

@@ -3591,7 +3735,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};

        auto const alice = Account("alice");
        auto const bob = Account("bob");
@@ -3630,7 +3774,7 @@ class Batch_test : public beast::unit_test::suite
        using namespace test::jtx;
        using namespace std::literals;

-        test::jtx::Env env{*this, envconfig()};
+        test::jtx::Env env{*this, features};
        XRPAmount const baseFee = env.current()->fees().base;

        auto const alice = Account("alice");
@@ -3711,6 +3855,7 @@ class Batch_test : public beast::unit_test::suite
                *this,
                makeSmallQueueConfig(
                    {{"minimum_txn_in_ledger_standalone", "2"}}),
+                features,
                nullptr,
                beast::severities::kError};

@@ -3771,6 +3916,7 @@ class Batch_test : public beast::unit_test::suite
                *this,
                makeSmallQueueConfig(
                    {{"minimum_txn_in_ledger_standalone", "2"}}),
+                features,
                nullptr,
                beast::severities::kError};

@@ -3878,7 +4024,7 @@ class Batch_test : public beast::unit_test::suite

        // delegated non atomic inner
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -3923,7 +4069,7 @@ class Batch_test : public beast::unit_test::suite

        // delegated atomic inner
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -3975,7 +4121,7 @@ class Batch_test : public beast::unit_test::suite
        // this also makes sure tfInnerBatchTxn won't block delegated AccountSet
        // with granular permission
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};

            auto const alice = Account("alice");
            auto const bob = Account("bob");
@@ -4024,7 +4170,7 @@ class Batch_test : public beast::unit_test::suite
        // this also makes sure tfInnerBatchTxn won't block delegated
        // MPTokenIssuanceSet with granular permission
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            Account alice{"alice"};
            Account bob{"bob"};
            env.fund(XRP(100000), alice, bob);
@@ -4080,7 +4226,7 @@ class Batch_test : public beast::unit_test::suite
        // this also makes sure tfInnerBatchTxn won't block delegated TrustSet
        // with granular permission
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            Account gw{"gw"};
            Account alice{"alice"};
            Account bob{"bob"};
@@ -4120,7 +4266,7 @@ class Batch_test : public beast::unit_test::suite

        // inner transaction not authorized by the delegating account.
        {
-            test::jtx::Env env{*this, envconfig()};
+            test::jtx::Env env{*this, features};
            Account gw{"gw"};
            Account alice{"alice"};
            Account bob{"bob"};
@@ -4169,7 +4315,7 @@ class Batch_test : public beast::unit_test::suite
        testcase("Validate RPC response");

        using namespace jtx;
-        Env env(*this);
+        Env env(*this, features);
        Account const alice("alice");
        Account const bob("bob");
        env.fund(XRP(10000), alice, bob);
@@ -4246,7 +4392,7 @@ class Batch_test : public beast::unit_test::suite
    testBatchCalculateBaseFee(FeatureBitset features)
    {
        using namespace jtx;
-        Env env(*this);
+        Env env(*this, features);
        Account const alice("alice");
        Account const bob("bob");
        Account const carol("carol");
@@ -4371,6 +4517,7 @@ public:
    {
        using namespace test::jtx;
        auto const sa = testable_amendments();
+        testWithFeats(sa - fixBatchInnerSigs);
        testWithFeats(sa);
    }
 };
--- a/src/xrpld/app/misc/NetworkOPs.cpp
+++ b/src/xrpld/app/misc/NetworkOPs.cpp
@@ -1696,7 +1696,7 @@ NetworkOPsImp::apply(std::unique_lock<std::mutex>& batchLock)
                    // only be set if the Batch feature is enabled. If Batch is
                    // not enabled, the flag is always invalid, so don't relay
                    // it regardless.
-                    !sttx.isFlag(tfInnerBatchTxn))
+                    !(sttx.isFlag(tfInnerBatchTxn)))
                {
                    protocol::TMTransaction tx;
                    Serializer s;
--- a/src/xrpld/app/tx/detail/Batch.cpp
+++ b/src/xrpld/app/tx/detail/Batch.cpp
@@ -324,6 +324,16 @@ Batch::preflight(PreflightContext const& ctx)
            }
        }

+        // Check that the Fee is native asset (XRP) and zero
+        if (auto const fee = stx.getFieldAmount(sfFee);
+            !fee.native() || fee.xrp() != beast::zero)
+        {
+            JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "
+                                << "inner txn must have a fee of 0. "
+                                << "txID: " << hash;
+            return temBAD_FEE;
+        }
+
        auto const innerAccount = stx.getAccountID(sfAccount);
        if (auto const preflightResult = ripple::preflight(
                ctx.app, ctx.rules, parentBatchId, stx, tapBATCH, ctx.j);
@@ -336,16 +346,6 @@ Batch::preflight(PreflightContext const& ctx)
            return temINVALID_INNER_BATCH;
        }

-        // Check that the fee is zero
-        if (auto const fee = stx.getFieldAmount(sfFee);
-            !fee.native() || fee.xrp() != beast::zero)
-        {
-            JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "
-                                << "inner txn must have a fee of 0. "
-                                << "txID: " << hash;
-            return temBAD_FEE;
-        }
-
        // Check that Sequence and TicketSequence are not both present
        if (stx.isFieldPresent(sfTicketSequence) &&
            stx.getFieldU32(sfSequence) != 0)
--- a/src/xrpld/app/tx/detail/Transactor.cpp
+++ b/src/xrpld/app/tx/detail/Transactor.cpp
@@ -231,8 +231,14 @@ Transactor::preflight2(PreflightContext const& ctx)
        // regardless of success or failure
        return *ret;

+    // It should be impossible for the InnerBatchTxn flag to be set without
+    // featureBatch being enabled
+    XRPL_ASSERT_PARTS(
+        !ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatch),
+        "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(featureBatch))
        return tesSUCCESS;
    // Do not add any checks after this point that are relevant for
    // batch inner transactions. They will be skipped.
--- a/src/xrpld/app/tx/detail/apply.cpp
+++ b/src/xrpld/app/tx/detail/apply.cpp
@@ -60,15 +60,22 @@ checkValidity(
                Validity::SigBad,
                "Malformed: Invalid inner batch transaction."};

-        std::string reason;
-        if (!passesLocalChecks(tx, reason))
+        // 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)
        {
-            router.setFlags(id, SF_LOCALBAD);
-            return {Validity::SigGoodOnly, reason};
-        }
+            std::string reason;
+            if (!passesLocalChecks(tx, reason))
+            {
+                router.setFlags(id, SF_LOCALBAD);
+                return {Validity::SigGoodOnly, reason};
+            }

-        router.setFlags(id, SF_SIGGOOD);
-        return {Validity::Valid, ""};
+            router.setFlags(id, SF_SIGGOOD);
+            return {Validity::Valid, ""};
+        }
    }

    if (any(flags & SF_SIGBAD))