ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-23 23:20:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: validate Batch inner transactions at construction

Browse Source
This commit is contained in:
Denis Angell
2026-07-13 10:32:56 -04:00
parent 62634463f4
commit 7161a00742
7 changed files with 275 additions and 189 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
include/xrpl/protocol/STTx.h
View File

@@ -145,6 +145,13 @@ public:
[[nodiscard]] std::vector<uint256> const&
getBatchTransactionIDs() const;
/**
* The inner transactions of a Batch, built and validated at construction.
* Seated only when the batch is within the size cap.
*/
[[nodiscard]] std::vector<std::shared_ptr<STTx const>> const&
getBatchTransactions() const;
[[nodiscard]] AccountID
getFeePayerID() const;
@@ -172,7 +179,7 @@ private:
checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const;
void
buildBatchTxnIds();
buildBatchTxns();
STBase*
copy(std::size_t n, void* buf) const override;
@@ -181,6 +188,7 @@ private:
friend class detail::STVar;
std::optional<std::vector<uint256>> batchTxnIds_;
std::optional<std::vector<std::shared_ptr<STTx const>>> batchTxns_;
};
bool

122
src/libxrpl/protocol/STTx.cpp
View File

@@ -72,7 +72,7 @@ STTx::STTx(STObject&& object)
{
applyTemplate(getTxFormat(txType_)->getSOTemplate()); // may throw
tid_ = getHash(HashPrefix::TransactionId);
buildBatchTxnIds();
buildBatchTxns();
}
STTx::STTx(SerialIter& sit) : STObject(sfTransaction)
@@ -89,7 +89,7 @@ STTx::STTx(SerialIter& sit) : STObject(sfTransaction)
applyTemplate(getTxFormat(txType_)->getSOTemplate()); // May throw
tid_ = getHash(HashPrefix::TransactionId);
buildBatchTxnIds();
buildBatchTxns();
}
STTx::STTx(TxType type, std::function<void(STObject&)> assembler) : STObject(sfTransaction)
@@ -110,7 +110,7 @@ STTx::STTx(TxType type, std::function<void(STObject&)> assembler) : STObject(sfT
logicError("Transaction type was mutated during assembly");
tid_ = getHash(HashPrefix::TransactionId);
buildBatchTxnIds();
buildBatchTxns();
}
STBase*
@@ -281,10 +281,14 @@ STTx::checkSign(Rules const& rules) const
// Verify the batch signer signatures here too, so they are cached with the
// rest of signature checking (checkValidity / SF_SIGGOOD) and stay out of
// the transaction engine. Gated on a batch (batchTxnIds_ seated) that
// actually carries signers; a batch whose inners are all from the outer
// account has no sfBatchSigners and needs no signer crypto.
if (batchTxnIds_ && isFieldPresent(sfBatchSigners))
// the transaction engine. Keyed only on sfBatchSigners being present (which
// only a Batch can carry). Deliberately NOT gated on batchTxnIds_ or any
// other transaction data: signature checking must never be skipped based on
// whether some size check passed, otherwise an oversized batch could slip
// through as validly signed by leaning on the later local / preflight
// checks (which are bypassed on several paths). A batch whose inners are all
// from the outer account has no sfBatchSigners and needs no signer crypto.
if (isFieldPresent(sfBatchSigners))
{
if (auto const ret = checkBatchSign(rules); !ret)
return ret;
@@ -307,6 +311,24 @@ STTx::checkBatchSign(Rules const& rules) const
if (!isFieldPresent(sfBatchSigners))
return std::unexpected("Missing BatchSigners field."); // LCOV_EXCL_LINE
STArray const& signers{getFieldArray(sfBatchSigners)};
// Bound signature verification to the protocol cap. This runs in
// checkValidity (via checkSign) at relay / submit time, BEFORE preflight
// and passesLocalChecks enforce the cap. Without this guard a malicious
// peer could put an oversized sfBatchSigners array in a 1 MB blob and
// force one signature verification per entry before any of those checks
// (or the fee charge) runs.
if (signers.size() > kMaxBatchSigners)
return std::unexpected("BatchSigners array exceeds max entries.");
// The signers sign over the inner transaction ids, which are seated at
// construction whenever sfRawTransactions is present (required for a
// Batch). Guard against a future format change that made it optional.
if (!batchTxnIds_)
{
// LCOV_EXCL_START
UNREACHABLE("STTx::checkBatchSign : batch transaction IDs not built");
return std::unexpected("Missing inner transactions.");
// LCOV_EXCL_STOP
}
for (auto const& signer : signers)
{
Blob const& signingPubKey = signer.getFieldVL(sfSigningPubKey);
@@ -577,25 +599,45 @@ STTx::checkMultiSign(Rules const& rules, STObject const& sigObject) const
}
void
STTx::buildBatchTxnIds()
STTx::buildBatchTxns()
{
// Precondition: the template must have been applied first, so the fields
// (including sfRawTransactions) are canonical before the inner txns are
// hashed. The constructors call this immediately after applying the
// template; isFree() being false confirms a template is set.
XRPL_ASSERT(!isFree(), "STTx::buildBatchTxnIds : template applied");
if (getTxnType() != ttBATCH || !isFieldPresent(sfRawTransactions))
XRPL_ASSERT(!isFree(), "STTx::buildBatchTxns : template applied");
if (getTxnType() != ttBATCH)
return;
// A Batch always seats its inner ids and transactions here, so every
// downstream consumer can rely on them. sfRawTransactions is required by the
// format (applyTemplate rejects a Batch without it); this guards a future
// change that made it optional.
if (!isFieldPresent(sfRawTransactions))
{
// LCOV_EXCL_START
UNREACHABLE("STTx::buildBatchTxns : missing RawTransactions");
Throw<std::runtime_error>("Batch has no RawTransactions.");
// LCOV_EXCL_STOP
}
auto const& raw = getFieldArray(sfRawTransactions);
if (raw.size() > kMaxBatchTxCount)
Throw<std::runtime_error>("Batch has too many inner transactions.");
// Seated for any batch with raw transactions. The count is validated in
// preflight and at the relay boundary, so build every id here; this keeps
// the invariant batchTxnIds_->size() == rawTransactions.size().
// Build and validate each inner as an STTx once. A malformed inner throws;
// a nested batch is rejected before building it (a batch cannot contain a
// batch, and building one would recurse). ids are cached alongside the
// transactions so checkBatchSign, which signs over them once per signer,
// does not reallocate.
auto& ids = batchTxnIds_.emplace();
auto& txns = batchTxns_.emplace();
ids.reserve(raw.size());
txns.reserve(raw.size());
for (STObject const& rb : raw)
ids.push_back(rb.getHash(HashPrefix::TransactionId));
{
if (rb.getFieldU16(sfTransactionType) == ttBATCH)
Throw<std::runtime_error>("Batch inner transaction cannot be a Batch.");
auto stx = std::make_shared<STTx const>(STObject{rb});
ids.push_back(stx->getTransactionID());
txns.push_back(std::move(stx));
}
}
std::vector<uint256> const&
@@ -611,6 +653,18 @@ STTx::getBatchTransactionIDs() const
return *batchTxnIds_;
}
std::vector<std::shared_ptr<STTx const>> const&
STTx::getBatchTransactions() const
{
XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::getBatchTransactions : batch transaction");
XRPL_ASSERT(
batchTxns_.has_value(), "STTx::getBatchTransactions : batch transactions built");
XRPL_ASSERT(
batchTxns_->size() == getFieldArray(sfRawTransactions).size(),
"STTx::getBatchTransactions : batch transactions size mismatch");
return *batchTxns_;
}
AccountID
STTx::getFeePayerID() const
{
@@ -775,36 +829,12 @@ isBatchRawTransactionOkay(STObject const& st, std::string& reason)
return false;
}
auto const& rawTxns = st.getFieldArray(sfRawTransactions);
if (rawTxns.size() > kMaxBatchTxCount)
// Inner structure (type, template, no nesting, count) is validated when the
// batch STTx is constructed; here we only run each inner's local checks.
for (STObject const& raw : st.getFieldArray(sfRawTransactions))
{
reason = "Raw Transactions array exceeds max entries.";
return false;
}
for (STObject raw : rawTxns)
{
try
{
auto const tt = safeCast<TxType>(raw.getFieldU16(sfTransactionType));
if (tt == ttBATCH)
{
reason = "Raw Transactions may not contain batch transactions.";
return false;
}
raw.applyTemplate(getTxFormat(tt)->getSOTemplate());
// passesLocalChecks recurses back into isBatchRawTransactionOkay,
// but an inner can never be a batch (rejected above), so the
// recursion terminates at depth 1.
if (!passesLocalChecks(raw, reason))
return false;
}
catch (std::exception const& e)
{
reason = e.what();
if (!passesLocalChecks(raw, reason))
return false;
}
}
return true;
}

4
src/libxrpl/tx/apply.cpp
View File

@@ -177,9 +177,9 @@ applyBatchTransactions(
int applied = 0;
for (STObject rb : batchTxn.getFieldArray(sfRawTransactions))
for (auto const& stx : batchTxn.getBatchTransactions())
{
auto const result = applyOneTransaction(STTx{std::move(rb)});
auto const result = applyOneTransaction(*stx);
XRPL_ASSERT(
result.applied == (isTesSuccess(result.ter) || isTecClaim(result.ter)),
"Outer Batch failure, inner transaction should not be applied");

15
src/libxrpl/tx/applySteps.cpp
View File

@@ -10,6 +10,7 @@
#include <xrpl/protocol/Rules.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/SeqProxy.h>
#include <xrpl/protocol/SystemParameters.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/XRPAmount.h>
#include <xrpl/tx/ApplyContext.h>
@@ -419,7 +420,19 @@ preclaim(PreflightResult const& preflightResult, ServiceRegistry& registry, Open
XRPAmount
calculateBaseFee(ReadView const& view, STTx const& tx)
{
return invokeCalculateBaseFee(view, tx);
try
{
return invokeCalculateBaseFee(view, tx);
}
catch (std::exception const& e)
{
// LCOV_EXCL_START
// A fee calc should not throw; return an unpayable fee so the
// transaction fails the fee check.
JLOG(debugLog().error()) << "apply (calculateBaseFee): " << e.what();
return XRPAmount{kInitialXrp};
// LCOV_EXCL_STOP
}
}
XRPAmount

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

@@ -73,43 +73,20 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
XRPAmount const batchBase = view.fees().base + baseFee;
// Calculate the Inner Txn Fees
// Calculate the Inner Txn Fees. Inners are built and validated (count,
// no nesting) at construction, so they are reused here directly.
XRPAmount txnFees{0};
if (tx.isFieldPresent(sfRawTransactions))
for (auto const& stx : tx.getBatchTransactions())
{
auto const& txns = tx.getFieldArray(sfRawTransactions);
auto const fee = xrpl::calculateBaseFee(view, *stx);
// LCOV_EXCL_START
if (txns.size() > kMaxBatchTxCount)
if (txnFees > maxAmount - fee)
{
JLOG(debugLog().error()) << "BatchTrace: Raw Transactions array exceeds max entries.";
JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in txnFees calculation.";
return XRPAmount{kInitialXrp};
}
// LCOV_EXCL_STOP
for (STObject txn : txns)
{
STTx const stx = STTx{std::move(txn)};
// LCOV_EXCL_START
if (stx.getTxnType() == ttBATCH)
{
JLOG(debugLog().error()) << "BatchTrace: Inner Batch transaction found.";
return XRPAmount{kInitialXrp};
}
// LCOV_EXCL_STOP
auto const fee = xrpl::calculateBaseFee(view, stx);
// LCOV_EXCL_START
if (txnFees > maxAmount - fee)
{
JLOG(debugLog().error())
<< "BatchTrace: XRPAmount overflow in txnFees calculation.";
return XRPAmount{kInitialXrp};
}
// LCOV_EXCL_STOP
txnFees += fee;
}
txnFees += fee;
}
// Calculate the Signers/BatchSigners Fees
@@ -140,7 +117,7 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
{
JLOG(debugLog().error())
<< "BatchTrace: Nested Signers array exceeds max entries.";
return kInitialXrp;
return XRPAmount{kInitialXrp};
}
// LCOV_EXCL_STOP
signerCount += nestedSigners.size();
@@ -246,13 +223,6 @@ Batch::preflight(PreflightContext const& ctx)
return temARRAY_EMPTY;
}
if (rawTxns.size() > kMaxBatchTxCount)
{
JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:"
<< "txns array exceeds 8 entries.";
return temARRAY_TOO_LARGE;
}
if (ctx.tx.isFieldPresent(sfBatchSigners) &&
ctx.tx.getFieldArray(sfBatchSigners).size() > kMaxBatchSigners)
{
@@ -293,9 +263,9 @@ Batch::preflight(PreflightContext const& ctx)
return tesSUCCESS;
};
for (STObject rb : rawTxns)
for (auto const& stxPtr : ctx.tx.getBatchTransactions())
{
STTx const stx = STTx{std::move(rb)};
STTx const& stx = *stxPtr;
auto const hash = stx.getTransactionID();
if (!uniqueHashes.emplace(hash).second)
{
@@ -306,14 +276,6 @@ Batch::preflight(PreflightContext const& ctx)
}
auto const txType = stx.getFieldU16(sfTransactionType);
if (txType == ttBATCH)
{
JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "
<< "batch cannot have an inner batch txn. "
<< "txID: " << hash;
return temINVALID;
}
if (std::ranges::any_of(
kDisabledTxTypes, [txType](auto const& disabled) { return txType == disabled; }))
{
@@ -434,15 +396,14 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
ctx.tx.getTxnType() == ttBATCH, "xrpl::Batch::preflightSigValidated : batch transaction");
auto const parentBatchId = ctx.tx.getTransactionID();
auto const outerAccount = ctx.tx.getAccountID(sfAccount);
auto const& rawTxns = ctx.tx.getFieldArray(sfRawTransactions);
// Accounts that must sign the batch: each inner authorizer and counterparty
// (excluding the outer account), sorted and de-duplicated to match against
// the ascending, unique batch signers.
std::vector<AccountID> requiredSigners;
requiredSigners.reserve(kMaxBatchSigners);
for (STObject const& rb : rawTxns)
for (auto const& stxPtr : ctx.tx.getBatchTransactions())
{
STTx const& rb = *stxPtr;
// A delegated inner is signed by the delegate, not the account holder,
// so the delegate is the required signer when present.
AccountID const authorizer = rb.getInitiator();

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

@@ -313,8 +313,8 @@ class Batch_test : public beast::unit_test::Suite
env.close();
}
// DEFENSIVE: temARRAY_TOO_LARGE: Batch: txns array exceeds 8 entries.
// ACTUAL: telENV_RPC_FAILED: isRawTransactionOkay()
// An oversized batch (more than kMaxBatchTxCount inners) fails STTx
// construction, so the transaction cannot be built.
{
auto const seq = env.seq(alice);
auto const batchFee = batch::calcBatchFee(env, 0, 9);
@@ -328,7 +328,7 @@ class Batch_test : public beast::unit_test::Suite
batch::Inner(pay(alice, bob, XRP(1)), seq + 7),
batch::Inner(pay(alice, bob, XRP(1)), seq + 8),
batch::Inner(pay(alice, bob, XRP(1)), seq + 9),
Ter(telENV_RPC_FAILED));
Ter(temMALFORMED));
env.close();
}
@@ -345,15 +345,15 @@ class Batch_test : public beast::unit_test::Suite
env.close();
}
// DEFENSIVE: temINVALID: Batch: batch cannot have inner batch txn.
// ACTUAL: telENV_RPC_FAILED: isRawTransactionOkay()
// A batch may not contain a batch: the nested inner fails STTx
// construction, so the transaction cannot be built.
{
auto const seq = env.seq(alice);
auto const batchFee = batch::calcBatchFee(env, 0, 2);
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::Inner(batch::outer(alice, seq, batchFee, tfAllOrNothing), seq),
batch::Inner(pay(alice, bob, XRP(1)), seq + 2),
Ter(telENV_RPC_FAILED));
Ter(temMALFORMED));
env.close();
}
@@ -938,80 +938,41 @@ class Batch_test : public beast::unit_test::Suite
env.fund(XRP(10000), alice, bob);
// An inner missing a required field can no longer be submitted: the
// outer STTx builds and validates each inner at construction, so
// building the batch (as signing does) throws. Returns true if the
// build fails.
auto batchCtorFails = [&](json::StaticString const& field) -> bool {
auto const batchFee = batch::calcBatchFee(env, 1, 2);
auto const seq = env.seq(alice);
auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1);
tx1.removeMember(field);
try
{
// Env::st swallows the construction failure and yields a null
// stx, so a malformed inner shows up as no transaction built.
auto const jt = env.jtnofill(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
tx1,
batch::Inner(pay(alice, bob, XRP(10)), seq + 2));
return jt.stx == nullptr;
}
catch (std::exception const&)
{
return true;
}
};
// Invalid: sfTransactionType
{
auto const batchFee = batch::calcBatchFee(env, 1, 2);
auto const seq = env.seq(alice);
auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1);
tx1.removeMember(jss::TransactionType);
auto jt = env.jtnofill(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
tx1,
batch::Inner(pay(alice, bob, XRP(10)), seq + 2));
env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED));
env.close();
}
BEAST_EXPECT(batchCtorFails(jss::TransactionType));
// Invalid: sfAccount
{
auto const batchFee = batch::calcBatchFee(env, 1, 2);
auto const seq = env.seq(alice);
auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1);
tx1.removeMember(jss::Account);
auto jt = env.jtnofill(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
tx1,
batch::Inner(pay(alice, bob, XRP(10)), seq + 2));
env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED));
env.close();
}
BEAST_EXPECT(batchCtorFails(jss::Account));
// Invalid: sfSequence
{
auto const batchFee = batch::calcBatchFee(env, 1, 2);
auto const seq = env.seq(alice);
auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1);
tx1.removeMember(jss::Sequence);
auto jt = env.jtnofill(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
tx1,
batch::Inner(pay(alice, bob, XRP(10)), seq + 2));
env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED));
env.close();
}
BEAST_EXPECT(batchCtorFails(jss::Sequence));
// Invalid: sfFee
{
auto const batchFee = batch::calcBatchFee(env, 1, 2);
auto const seq = env.seq(alice);
auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1);
tx1.removeMember(jss::Fee);
auto jt = env.jtnofill(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
tx1,
batch::Inner(pay(alice, bob, XRP(10)), seq + 2));
env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED));
env.close();
}
BEAST_EXPECT(batchCtorFails(jss::Fee));
// Invalid: sfSigningPubKey
{
auto const batchFee = batch::calcBatchFee(env, 1, 2);
auto const seq = env.seq(alice);
auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1);
tx1.removeMember(jss::SigningPubKey);
auto jt = env.jtnofill(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
tx1,
batch::Inner(pay(alice, bob, XRP(10)), seq + 2));
env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED));
env.close();
}
BEAST_EXPECT(batchCtorFails(jss::SigningPubKey));
// Inner OfferCreate with MPT TakerPays. Valid under featureMPTokensV2.
{
@@ -1512,11 +1473,13 @@ class Batch_test : public beast::unit_test::Suite
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
Ter(telENV_RPC_FAILED));
Ter(temMALFORMED));
env.close();
}
// temARRAY_TOO_LARGE: Batch: txns array exceeds 8 entries.
// An oversized batch (more than kMaxBatchTxCount inners) fails STTx
// construction, so it never reaches apply or checkValidity - Env::st
// swallows the failure and yields a null stx.
{
Env env{*this, features};
@@ -1539,11 +1502,44 @@ class Batch_test : public beast::unit_test::Suite
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq));
env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal j) {
auto const result = xrpl::apply(env.app(), view, *jt.stx, TapNone, j);
BEAST_EXPECT(!result.applied && result.ter == temARRAY_TOO_LARGE);
return result.applied;
});
BEAST_EXPECT(jt.stx == nullptr);
}
// An oversized batch cannot slip through as validly signed even when it
// carries a BatchSigners array: the oversized inner array fails STTx
// construction before any signature is checked, so the batch can't be
// built at all (building it - as signing does - throws).
{
Env env{*this, features};
auto const alice = Account("alice");
auto const bob = Account("bob");
env.fund(XRP(10000), alice, bob);
env.close();
auto const aliceSeq = env.seq(alice);
auto const batchFee = batch::calcBatchFee(env, kMaxBatchSigners + 1, 9);
bool threw = false;
try
{
env.jtnofill(
batch::outer(alice, aliceSeq, batchFee, tfAllOrNothing),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Inner(pay(alice, bob, XRP(1)), aliceSeq),
batch::Sig(std::vector<Reg>(kMaxBatchSigners + 1, bob)));
}
catch (std::exception const&)
{
threw = true;
}
BEAST_EXPECT(threw);
}
// Regression: the relay-boundary local check (isBatchRawTransactionOkay)
@@ -1598,6 +1594,14 @@ class Batch_test : public beast::unit_test::Suite
BEAST_EXPECT(!result.applied && result.ter == temARRAY_TOO_LARGE);
return result.applied;
});
// Regression (uncapped batch-signer verification): the relay
// boundary (checkValidity) rejects the oversized signers array via
// the checkBatchSign guard, BEFORE verifying a single signature.
auto const [valid, reason] =
xrpl::checkValidity(env.app().getHashRouter(), *jt.stx, env.current()->rules());
BEAST_EXPECT(valid == xrpl::Validity::SigBad);
BEAST_EXPECT(reason == "BatchSigners array exceeds max entries.");
}
}
@@ -5524,7 +5528,8 @@ class Batch_test : public beast::unit_test::Suite
return Batch::calculateBaseFee(*env.current(), *jtx.stx);
};
// bad: Inner Batch transaction found
// bad: a batch may not contain a batch - the nested inner fails STTx
// construction, so the transaction cannot be built.
{
auto const seq = env.seq(alice);
XRPAmount const batchFee = batch::calcBatchFee(env, 0, 2);
@@ -5532,11 +5537,11 @@ class Batch_test : public beast::unit_test::Suite
batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::Inner(batch::outer(alice, seq, batchFee, tfAllOrNothing), seq),
batch::Inner(pay(alice, bob, XRP(1)), seq + 2));
XRPAmount const txBaseFee = getBaseFee(jtx);
BEAST_EXPECT(txBaseFee == XRPAmount(kInitialXrp));
BEAST_EXPECT(jtx.stx == nullptr);
}
// bad: Raw Transactions array exceeds max entries.
// bad: an oversized batch (more than kMaxBatchTxCount inners) fails
// STTx construction, so it cannot be built.
{
auto const seq = env.seq(alice);
XRPAmount const batchFee = batch::calcBatchFee(env, 0, 2);
@@ -5553,8 +5558,7 @@ class Batch_test : public beast::unit_test::Suite
batch::Inner(pay(alice, bob, XRP(1)), seq + 8),
batch::Inner(pay(alice, bob, XRP(1)), seq + 9));
XRPAmount const txBaseFee = getBaseFee(jtx);
BEAST_EXPECT(txBaseFee == XRPAmount(kInitialXrp));
BEAST_EXPECT(jtx.stx == nullptr);
}
// bad: Signers array exceeds max entries.

70
src/test/protocol/STTx_test.cpp
View File

@@ -59,6 +59,9 @@ public:
testcase("STObject constructor errors");
testObjectCtorErrors();
testcase("Batch inner transaction validation");
testBatchInnerCtorErrors();
}
void
@@ -1463,6 +1466,73 @@ public:
BEAST_EXPECT(got == "Field 'Fee' is required but missing.");
}
}
void
testBatchInnerCtorErrors()
{
auto const kp1 = randomKeyPair(KeyType::Secp256k1);
auto const id1 = calcAccountID(kp1.first);
auto const kp2 = randomKeyPair(KeyType::Secp256k1);
auto const id2 = calcAccountID(kp2.first);
// A raw inner transaction object of the given transaction type.
auto makeInner = [&](std::uint16_t txType) {
STObject inner(sfRawTransaction);
inner.setFieldU16(sfTransactionType, txType);
inner.setAccountID(sfAccount, id1);
inner.setAccountID(sfDestination, id2);
inner.setFieldAmount(sfAmount, STAmount(10000000000ull));
inner.setFieldAmount(sfFee, STAmount(0ull));
inner.setFieldU32(sfSequence, 1);
inner.setFieldVL(sfSigningPubKey, Slice(kp1.first.data(), kp1.first.size()));
return inner;
};
// An outer Batch STObject wrapping the given inner.
auto makeBatch = [&](STObject inner) {
STArray rawTxns(sfRawTransactions);
rawTxns.push_back(std::move(inner));
STObject batch(sfGeneric);
batch.setFieldU16(sfTransactionType, ttBATCH);
batch.setAccountID(sfAccount, id1);
batch.setFieldAmount(sfFee, STAmount(20ull));
batch.setFieldU32(sfSequence, 1);
batch.setFieldVL(sfSigningPubKey, Slice(kp1.first.data(), kp1.first.size()));
batch.setFieldArray(sfRawTransactions, rawTxns);
return batch;
};
{
// A batch whose inner is a well-formed transaction constructs.
std::string got;
try
{
STTx{makeBatch(makeInner(ttPAYMENT))};
}
catch (std::exception const& err)
{
got = err.what();
}
BEAST_EXPECT(got.empty());
}
{
// A batch whose inner carries an unregistered transaction type is
// rejected at construction, rather than surviving as a raw STObject
// and throwing later from an unprotected fee-calculation path.
std::string got;
try
{
STTx{makeBatch(makeInner(60000))};
}
catch (std::exception const& err)
{
got = err.what();
}
BEAST_EXPECT(matches(got.c_str(), "Invalid transaction type 60000"));
}
}
};
class InnerObjectFormatsSerializer_test : public beast::unit_test::Suite