diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 5c8d2aa198..932668c16f 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -16,6 +16,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_FEATURE(PermissionDelegationV1_1, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (DirectoryLimit, Supported::yes, VoteBehavior::DefaultNo) diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index 6fbec52a93..68bf7e833b 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -148,15 +148,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"); @@ -179,7 +185,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 = @@ -205,7 +211,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"); @@ -617,7 +623,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"); @@ -858,7 +864,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"); @@ -949,7 +955,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"); @@ -1187,7 +1193,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"); @@ -1209,7 +1215,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"); @@ -1236,7 +1242,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"); @@ -1265,7 +1271,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"); @@ -1297,7 +1303,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"); @@ -1321,7 +1327,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"); @@ -1361,7 +1367,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"); @@ -1386,7 +1392,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"); @@ -1419,7 +1425,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"); @@ -1438,7 +1444,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"); @@ -1472,7 +1478,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"); @@ -1608,7 +1614,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"); @@ -1840,7 +1846,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"); @@ -2062,7 +2068,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"); @@ -2248,14 +2254,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"); @@ -2263,75 +2281,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 expectedEnabled = std::nullopt, + std::optional 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()->header().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); } } @@ -2343,7 +2456,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"); @@ -2390,7 +2503,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"); @@ -2443,7 +2556,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"); @@ -2484,7 +2597,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"); @@ -2529,7 +2642,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"); @@ -2581,7 +2694,6 @@ class Batch_test : public beast::unit_test::suite test::jtx::Env env{ *this, - envconfig(), features | featureSingleAssetVault | featureLendingProtocol | featureMPTokensV1}; @@ -2776,7 +2888,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"); @@ -2889,7 +3001,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"); @@ -2947,7 +3059,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"); @@ -3009,7 +3121,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"); @@ -3058,7 +3170,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"); @@ -3106,7 +3218,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"); @@ -3169,7 +3281,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(); @@ -3216,7 +3328,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(); @@ -3258,7 +3370,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(); @@ -3295,7 +3407,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(); @@ -3353,7 +3465,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(); @@ -3402,7 +3514,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(); @@ -3464,7 +3576,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(); @@ -3511,7 +3623,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(); @@ -3558,7 +3670,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(); @@ -3605,7 +3717,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"); @@ -3644,7 +3756,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"); @@ -3725,6 +3837,7 @@ class Batch_test : public beast::unit_test::suite *this, makeSmallQueueConfig( {{"minimum_txn_in_ledger_standalone", "2"}}), + features, nullptr, beast::severities::kError}; @@ -3785,6 +3898,7 @@ class Batch_test : public beast::unit_test::suite *this, makeSmallQueueConfig( {{"minimum_txn_in_ledger_standalone", "2"}}), + features, nullptr, beast::severities::kError}; @@ -3892,7 +4006,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"); @@ -3937,7 +4051,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"); @@ -3989,7 +4103,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"); @@ -4038,7 +4152,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); @@ -4094,7 +4208,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"}; @@ -4134,7 +4248,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"}; @@ -4182,7 +4296,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); @@ -4259,7 +4373,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"); @@ -4384,6 +4498,7 @@ public: { using namespace test::jtx; auto const sa = testable_amendments(); + testWithFeats(sa - fixBatchInnerSigs); testWithFeats(sa); } }; diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 6a00354b15..2422ec4ae6 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1681,7 +1681,7 @@ NetworkOPsImp::apply(std::unique_lock& 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; diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 851712fe90..a834f7c6c3 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -204,8 +204,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. diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index 5209c46f8f..a75f0cc967 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -41,15 +41,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))