diff --git a/include/xrpl/protocol/Protocol.h b/include/xrpl/protocol/Protocol.h index 6a96b2ccbe..4472da4256 100644 --- a/include/xrpl/protocol/Protocol.h +++ b/include/xrpl/protocol/Protocol.h @@ -307,4 +307,14 @@ constexpr std::size_t kPermissionMaxSize = 10; /** The maximum number of transactions that can be in a batch. */ constexpr std::size_t kMaxBatchTxCount = 8; +/** The maximum number of batch signers. + * + * A single inner transaction can require more than one signer: the inner + * account's authorizer plus, for some transaction types, a counterparty + * and/or a sponsor. Bounding the signers array at the inner-transaction + * count would therefore reject otherwise-valid batches. Allow up to three + * distinct signers per inner transaction. + */ +constexpr std::size_t kMaxBatchSigners = kMaxBatchTxCount * 3; + } // namespace xrpl diff --git a/src/libxrpl/tx/transactors/system/Batch.cpp b/src/libxrpl/tx/transactors/system/Batch.cpp index f3795ad271..3b0149cfe0 100644 --- a/src/libxrpl/tx/transactors/system/Batch.cpp +++ b/src/libxrpl/tx/transactors/system/Batch.cpp @@ -117,7 +117,7 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) auto const& signers = tx.getFieldArray(sfBatchSigners); // LCOV_EXCL_START - if (signers.size() > kMaxBatchTxCount) + if (signers.size() > kMaxBatchSigners) { JLOG(debugLog().error()) << "BatchTrace: Batch Signers array exceeds max entries."; return XRPAmount{kInitialXrp}; @@ -241,10 +241,10 @@ Batch::preflight(PreflightContext const& ctx) } if (ctx.tx.isFieldPresent(sfBatchSigners) && - ctx.tx.getFieldArray(sfBatchSigners).size() > kMaxBatchTxCount) + ctx.tx.getFieldArray(sfBatchSigners).size() > kMaxBatchSigners) { JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:" - << "signers array exceeds 8 entries."; + << "signers array exceeds " << kMaxBatchSigners << " entries."; return temARRAY_TOO_LARGE; } @@ -433,10 +433,10 @@ Batch::preflightSigValidated(PreflightContext const& ctx) STArray const& signers = ctx.tx.getFieldArray(sfBatchSigners); // Check that the batch signers array is not too large. - if (signers.size() > kMaxBatchTxCount) + if (signers.size() > kMaxBatchSigners) { JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: " - << "signers array exceeds " << kMaxBatchTxCount << " entries."; + << "signers array exceeds " << kMaxBatchSigners << " entries."; return temARRAY_TOO_LARGE; } diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index ad2bf2154c..88fd9b7a83 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -556,16 +556,16 @@ class Batch_test : public beast::unit_test::Suite env.close(); } - // DEFENSIVE: temARRAY_TOO_LARGE: Batch: signers array exceeds 8 - // entries. + // DEFENSIVE: temARRAY_TOO_LARGE: Batch: signers array exceeds + // kMaxBatchSigners entries. // ACTUAL: telENV_RPC_FAILED: isRawTransactionOkay() { auto const seq = env.seq(alice); - auto const batchFee = batch::calcBatchFee(env, 9, 2); + auto const batchFee = batch::calcBatchFee(env, kMaxBatchSigners + 1, 2); env(batch::outer(alice, seq, batchFee, tfAllOrNothing), batch::Inner(pay(alice, bob, XRP(10)), seq + 1), batch::Inner(pay(alice, bob, XRP(5)), seq + 2), - batch::Sig(bob, carol, alice, bob, carol, alice, bob, carol, alice, alice), + batch::Sig(std::vector(kMaxBatchSigners + 1, bob)), Ter(telENV_RPC_FAILED)); env.close(); } @@ -1530,7 +1530,8 @@ class Batch_test : public beast::unit_test::Suite }); } - // telENV_RPC_FAILED: Batch: signers array exceeds 8 entries. + // telENV_RPC_FAILED: Batch: signers array exceeds kMaxBatchSigners + // entries. { test::jtx::Env env{*this, features}; @@ -1540,16 +1541,17 @@ class Batch_test : public beast::unit_test::Suite env.close(); auto const aliceSeq = env.seq(alice); - auto const batchFee = batch::calcBatchFee(env, 9, 2); + auto const batchFee = batch::calcBatchFee(env, kMaxBatchSigners + 1, 2); env(batch::outer(alice, aliceSeq, batchFee, tfAllOrNothing), batch::Inner(pay(alice, bob, XRP(10)), aliceSeq + 1), batch::Inner(pay(alice, bob, XRP(5)), aliceSeq + 2), - batch::Sig(bob, bob, bob, bob, bob, bob, bob, bob, bob, bob), + batch::Sig(std::vector(kMaxBatchSigners + 1, bob)), Ter(telENV_RPC_FAILED)); env.close(); } - // temARRAY_TOO_LARGE: Batch preflight: signers array exceeds 8 entries. + // temARRAY_TOO_LARGE: Batch preflight: signers array exceeds + // kMaxBatchSigners entries. { test::jtx::Env env{*this, features}; @@ -1558,13 +1560,13 @@ class Batch_test : public beast::unit_test::Suite env.fund(XRP(10000), alice, bob); env.close(); - auto const batchFee = batch::calcBatchFee(env, 0, 9); + auto const batchFee = batch::calcBatchFee(env, kMaxBatchSigners + 1, 2); auto const aliceSeq = env.seq(alice); auto jt = env.jtnofill( batch::outer(alice, aliceSeq, batchFee, tfAllOrNothing), batch::Inner(pay(alice, bob, XRP(10)), aliceSeq + 1), batch::Inner(pay(alice, bob, XRP(5)), aliceSeq + 2), - batch::Sig(bob, bob, bob, bob, bob, bob, bob, bob, bob, bob)); + batch::Sig(std::vector(kMaxBatchSigners + 1, bob))); env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal j) { auto const result = xrpl::apply(env.app(), view, *jt.stx, TapNone, j); @@ -5424,7 +5426,7 @@ class Batch_test : public beast::unit_test::Suite batch::outer(alice, seq, batchFee, tfAllOrNothing), batch::Inner(pay(alice, bob, XRP(10)), seq + 1), batch::Inner(pay(alice, bob, XRP(5)), seq + 2), - batch::Sig(bob, carol, alice, bob, carol, alice, bob, carol, alice, alice)); + batch::Sig(std::vector(kMaxBatchSigners + 1, bob))); XRPAmount const txBaseFee = getBaseFee(jtx); BEAST_EXPECT(txBaseFee == XRPAmount(kInitialXrp)); }