ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-06-06 18:26:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: enforce sorted batch signers

Browse Source
This commit is contained in:
Denis Angell
2026-04-21 07:03:03 +02:00
parent c27613e1df
commit 477532edfe
2 changed files with 50 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
src/libxrpl/tx/transactors/system/Batch.cpp
View File

@@ -408,7 +408,6 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
}
// Validation Batch Signers
std::unordered_set<AccountID> batchSigners;
if (ctx.tx.isFieldPresent(sfBatchSigners))
{
STArray const& signers = ctx.tx.getFieldArray(sfBatchSigners);
@@ -421,11 +420,8 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
return temARRAY_TOO_LARGE;
}
// Add batch signers to the set to ensure all signer accounts are
// unique. Meanwhile, remove signer accounts from the set of inner
// transaction accounts (`requiredSigners`). By the end of the loop,
// `requiredSigners` should be empty, indicating that all inner
// accounts are matched with signers.
// Batch signers must be in strictly ascending order by AccountID.
AccountID lastBatchSigner(beast::zero);
for (auto const& signer : signers)
{
AccountID const signerAccount = signer.getAccountID(sfAccount);
@@ -436,13 +432,21 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
return temBAD_SIGNER;
}
if (!batchSigners.insert(signerAccount).second)
if (lastBatchSigner == signerAccount)
{
JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "
<< "duplicate signer found: " << signerAccount;
return temREDUNDANT;
return temBAD_SIGNER;
}
if (lastBatchSigner > signerAccount)
{
JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "
<< "unsorted signers array: " << signerAccount;
return temBAD_SIGNER;
}
lastBatchSigner = signerAccount;
// Check that the batch signer is in the required signers set.
// Remove it if it does, as it can be crossed off the list.
if (requiredSigners.erase(signerAccount) == 0)

40
src/test/app/Batch_test.cpp
View File

@@ -522,7 +522,7 @@ class Batch_test : public beast::unit_test::suite
env.close();
}
// temREDUNDANT: Batch: duplicate signer found
// temBAD_SIGNER: Batch: duplicate signer (caught by ascending order check)
{
auto const seq = env.seq(alice);
auto const batchFee = batch::calcBatchFee(env, 2, 2);
@@ -530,7 +530,7 @@ class Batch_test : public beast::unit_test::suite
batch::inner(pay(alice, bob, XRP(10)), seq + 1),
batch::inner(pay(bob, alice, XRP(5)), env.seq(bob)),
batch::sig(bob, bob),
ter(temREDUNDANT));
ter(temBAD_SIGNER));
env.close();
}
@@ -4459,6 +4459,41 @@ class Batch_test : public beast::unit_test::suite
}
}
void
testUnsortedBatchSigners(FeatureBitset features)
{
testcase("unsorted batch signers");
using namespace test::jtx;
Env env{*this, features};
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const carol = Account("carol");
env.fund(XRP(10000), alice, bob, carol);
env.close();
auto const seq = env.seq(alice);
auto const bobSeq = env.seq(bob);
auto const carolSeq = env.seq(carol);
auto const batchFee = batch::calcBatchFee(env, 2, 2);
auto jt = env.jt(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(pay(bob, alice, XRP(10)), bobSeq),
batch::inner(pay(carol, alice, XRP(5)), carolSeq),
batch::sig(bob, carol));
auto const s0 = jt.jv[sfBatchSigners.jsonName][0u];
auto const s1 = jt.jv[sfBatchSigners.jsonName][1u];
jt.jv[sfBatchSigners.jsonName][0u] = s1;
jt.jv[sfBatchSigners.jsonName][1u] = s0;
env(jt.jv, ter(temBAD_SIGNER));
env.close();
}
void
testWithFeats(FeatureBitset features)
{
@@ -4494,6 +4529,7 @@ class Batch_test : public beast::unit_test::suite
testValidateRPCResponse(features);
testBatchCalculateBaseFee(features);
testStandaloneInnerBatchFlag(features);
testUnsortedBatchSigners(features);
}
public: