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

fix: Refactor Batch Transaction IDs (#7736)

Browse Source 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Mayukha Vadari <mvadari@ripple.com>
This commit is contained in:
Denis Angell
2026-07-16 09:15:59 -04:00
committed by GitHub
parent b42cde3e85
commit 69b70d7a0d
9 changed files with 353 additions and 267 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
include/xrpl/protocol/STObject.h
View File

@@ -229,13 +229,6 @@ public:
[[nodiscard]] AccountID
getAccountID(SField const& field) const;
/**
* The account responsible for the authorization: the delegate when
* sfDelegate is present, otherwise the account.
*/
[[nodiscard]] AccountID
getInitiator() const;
[[nodiscard]] Blob
getFieldVL(SField const& field) const;
[[nodiscard]] STAmount const&

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

@@ -142,9 +142,26 @@ public:
TxnSql status,
std::string const& escapedMetaData) const;
[[nodiscard]] std::vector<uint256> const&
/**
* The IDs of the inner transactions of a Batch.
*/
[[nodiscard]] std::vector<uint256>
getBatchTransactionIDs() const;
/**
* The inner transactions of a Batch, built and validated at construction.
* Always seated for Batch STTx instances (construction throws if oversized).
*/
[[nodiscard]] std::vector<std::shared_ptr<STTx const>> const&
getBatchTransactions() const;
/**
* The account responsible for the authorization: the delegate when
* sfDelegate is present, otherwise the account.
*/
[[nodiscard]] AccountID
getInitiator() const;
[[nodiscard]] AccountID
getFeePayerID() const;
@@ -166,13 +183,16 @@ private:
checkMultiSign(Rules const& rules, STObject const& sigObject) const;
[[nodiscard]] std::expected<void, std::string>
checkBatchSingleSign(STObject const& batchSigner) const;
checkBatchSingleSign(STObject const& batchSigner, std::vector<uint256> const& txIds) const;
[[nodiscard]] std::expected<void, std::string>
checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const;
checkBatchMultiSign(
STObject const& batchSigner,
Rules const& rules,
std::vector<uint256> const& txIds) const;
void
buildBatchTxnIds();
buildBatchTxns();
STBase*
copy(std::size_t n, void* buf) const override;
@@ -180,11 +200,11 @@ private:
move(std::size_t n, void* buf) override;
friend class detail::STVar;
std::optional<std::vector<uint256>> batchTxnIds_;
std::optional<std::vector<std::shared_ptr<STTx const>>> batchTxns_;
};
bool
passesLocalChecks(STObject const& st, std::string&);
passesLocalChecks(STTx const& tx, std::string&);
/**
* Sterilize a transaction.

8
include/xrpl/tx/transactors/system/Batch.h
View File

@@ -12,6 +12,7 @@
#include <array>
#include <cstdint>
#include <optional>
namespace xrpl {
@@ -39,6 +40,9 @@ public:
static NotTEC
checkSign(PreclaimContext const& ctx);
static TER
preclaim(PreclaimContext const& ctx);
TER
doApply() override;
@@ -76,6 +80,10 @@ private:
// only be reached through Batch::checkSign.
static NotTEC
checkBatchSign(PreclaimContext const& ctx);
// nullopt on overflow or oversized signer arrays.
static std::optional<XRPAmount>
calculateBaseFeeImpl(ReadView const& view, STTx const& tx);
};
} // namespace xrpl

14
src/libxrpl/protocol/STObject.cpp
View File

@@ -633,20 +633,6 @@ STObject::getAccountID(SField const& field) const
return getFieldByValue<STAccount>(field);
}
AccountID
STObject::getInitiator() const
{
// If sfDelegate is present, the delegate account is the initiator
// note: if a delegate is specified, its authorization to act on behalf of the account is
// enforced in `Transactor::invokeCheckPermission`
// cryptographic signature validity is checked separately (e.g., in `Transactor::checkSign`)
if (isFieldPresent(sfDelegate))
return getAccountID(sfDelegate);
// Default initiator
return getAccountID(sfAccount);
}
Blob
STObject::getFieldVL(SField const& field) const
{

180
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*
@@ -279,12 +279,9 @@ STTx::checkSign(Rules const& rules) const
return std::unexpected("Sponsor: " + ret.error());
}
// 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))
// Verify batch signer signatures here so they are cached with the rest
// of signature checking.
if (isFieldPresent(sfBatchSigners))
{
if (auto const ret = checkBatchSign(rules); !ret)
return ret;
@@ -307,11 +304,28 @@ 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.");
// Defensive.
if (!batchTxns_)
{
// LCOV_EXCL_START
UNREACHABLE("STTx::checkBatchSign : batch transactions not built");
return std::unexpected("Missing inner transactions.");
// LCOV_EXCL_STOP
}
auto const txIds = getBatchTransactionIDs();
for (auto const& signer : signers)
{
Blob const& signingPubKey = signer.getFieldVL(sfSigningPubKey);
auto const result = signingPubKey.empty() ? checkBatchMultiSign(signer, rules)
: checkBatchSingleSign(signer);
auto const result = signingPubKey.empty() ? checkBatchMultiSign(signer, rules, txIds)
: checkBatchSingleSign(signer, txIds);
if (!result)
return result;
@@ -441,12 +455,11 @@ STTx::checkSingleSign(STObject const& sigObject) const
}
std::expected<void, std::string>
STTx::checkBatchSingleSign(STObject const& batchSigner) const
STTx::checkBatchSingleSign(STObject const& batchSigner, std::vector<uint256> const& txIds) const
{
XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::checkBatchSingleSign : batch transaction");
Serializer msg;
serializeBatch(
msg, getAccountID(sfAccount), getSeqValue(), getFlags(), getBatchTransactionIDs());
serializeBatch(msg, getAccountID(sfAccount), getSeqValue(), getFlags(), txIds);
finishMultiSigningData(batchSigner.getAccountID(sfAccount), msg);
return singleSignHelper(batchSigner, msg.slice());
}
@@ -529,7 +542,10 @@ multiSignHelper(
}
std::expected<void, std::string>
STTx::checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const
STTx::checkBatchMultiSign(
STObject const& batchSigner,
Rules const& rules,
std::vector<uint256> const& txIds) const
{
XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::checkBatchMultiSign : batch transaction");
// We can ease the computational load inside the loop a bit by
@@ -537,8 +553,7 @@ STTx::checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const
// with the stuff that stays constant from signature to signature.
auto const batchSignerAccount = batchSigner.getAccountID(sfAccount);
Serializer dataStart;
serializeBatch(
dataStart, getAccountID(sfAccount), getSeqValue(), getFlags(), getBatchTransactionIDs());
serializeBatch(dataStart, getAccountID(sfAccount), getSeqValue(), getFlags(), txIds);
dataStart.addBitString(batchSignerAccount);
return multiSignHelper(
batchSigner,
@@ -577,38 +592,79 @@ 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 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().
auto& ids = batchTxnIds_.emplace();
ids.reserve(raw.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).
auto& txns = batchTxns_.emplace();
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.");
txns.push_back(std::make_shared<STTx const>(STObject{rb}));
}
}
std::vector<uint256> const&
std::vector<uint256>
STTx::getBatchTransactionIDs() const
{
XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::getBatchTransactionIDs : batch transaction");
auto const& txns = getBatchTransactions();
std::vector<uint256> ids;
ids.reserve(txns.size());
for (auto const& stx : txns)
ids.push_back(stx->getTransactionID());
return ids;
}
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(
batchTxnIds_.has_value(), "STTx::getBatchTransactionIDs : batch transaction IDs built");
XRPL_ASSERT(
batchTxnIds_->size() == getFieldArray(sfRawTransactions).size(),
"STTx::getBatchTransactionIDs : batch transaction IDs size mismatch");
// NOLINTNEXTLINE(bugprone-unchecked-optional-access): guarded by assert above
return *batchTxnIds_;
batchTxns_->size() == getFieldArray(sfRawTransactions).size(),
"STTx::getBatchTransactions : batch transactions size mismatch");
return *batchTxns_;
}
AccountID
STTx::getInitiator() const
{
// If sfDelegate is present, the delegate account is the initiator
// note: if a delegate is specified, its authorization to act on behalf of the account is
// enforced in `Transactor::invokeCheckPermission`
// cryptographic signature validity is checked separately (e.g., in `Transactor::checkSign`)
if (isFieldPresent(sfDelegate))
return getAccountID(sfDelegate);
// Default initiator
return getAccountID(sfAccount);
}
AccountID
@@ -754,86 +810,62 @@ invalidMPTAmountInTx(STObject const& tx)
}
static bool
isBatchRawTransactionOkay(STObject const& st, std::string& reason)
isBatchRawTransactionOkay(STTx const& tx, std::string& reason)
{
if (!st.isFieldPresent(sfRawTransactions))
if (!tx.isFieldPresent(sfRawTransactions))
return true;
// sfRawTransactions only appears on a Batch. passesLocalChecks runs on
// unverified user and peer input, so reject (rather than assert) a non-batch
// transaction that carries it.
if (st.getFieldU16(sfTransactionType) != ttBATCH)
if (tx.getTxnType() != ttBATCH)
{
reason = "Only Batch transactions may contain raw transactions.";
return false;
}
if (st.isFieldPresent(sfBatchSigners) &&
st.getFieldArray(sfBatchSigners).size() > kMaxBatchSigners)
if (tx.isFieldPresent(sfBatchSigners) &&
tx.getFieldArray(sfBatchSigners).size() > kMaxBatchSigners)
{
reason = "Batch Signers array exceeds max entries.";
reason = "BatchSigners array exceeds max entries.";
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 (auto const& inner : tx.getBatchTransactions())
{
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(*inner, reason))
return false;
}
}
return true;
}
bool
passesLocalChecks(STObject const& st, std::string& reason)
passesLocalChecks(STTx const& tx, std::string& reason)
{
if (!isMemoOkay(st, reason))
if (!isMemoOkay(tx, reason))
return false;
if (!isAccountFieldOkay(st))
if (!isAccountFieldOkay(tx))
{
reason = "An account field is invalid.";
return false;
}
if (isPseudoTx(st))
if (isPseudoTx(tx))
{
reason = "Cannot submit pseudo transactions.";
return false;
}
if (invalidMPTAmountInTx(st))
if (invalidMPTAmountInTx(tx))
{
reason = "Amount can not be MPT.";
return false;
}
if (!isBatchRawTransactionOkay(st, reason))
if (!isBatchRawTransactionOkay(tx, 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");

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

@@ -15,7 +15,6 @@
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STObject.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/SystemParameters.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/TxFormats.h>
@@ -28,6 +27,7 @@
#include <cstddef>
#include <cstdint>
#include <limits>
#include <optional>
#include <unordered_map>
#include <unordered_set>
#include <utility>
@@ -46,17 +46,11 @@ namespace xrpl {
*
* @param view The ledger view providing fee and state information.
* @param tx The batch transaction to calculate the fee for.
* @return XRPAmount The total base fee required for the batch transaction.
*
* @throws std::overflow_error If any fee calculation would overflow the
* XRPAmount type.
* @throws std::length_error If the number of inner transactions or signers
* exceeds the allowed maximum.
* @throws std::invalid_argument If an inner transaction is itself a batch
* transaction.
* @return XRPAmount The total base fee required for the batch transaction,
* or std::nullopt on failure (overflow, oversized arrays).
*/
XRPAmount
Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
std::optional<XRPAmount>
Batch::calculateBaseFeeImpl(ReadView const& view, STTx const& tx)
{
XRPAmount const maxAmount{std::numeric_limits<XRPAmount::value_type>::max()};
@@ -67,49 +61,26 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
if (baseFee > maxAmount - view.fees().base)
{
JLOG(debugLog().error()) << "BatchTrace: Base fee overflow detected.";
return XRPAmount{kInitialXrp};
return std::nullopt;
}
// LCOV_EXCL_STOP
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.";
return XRPAmount{kInitialXrp};
JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in txnFees calculation.";
return std::nullopt;
}
// 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
@@ -122,7 +93,7 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
if (signers.size() > kMaxBatchSigners)
{
JLOG(debugLog().error()) << "BatchTrace: Batch Signers array exceeds max entries.";
return XRPAmount{kInitialXrp};
return std::nullopt;
}
// LCOV_EXCL_STOP
@@ -140,7 +111,7 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
{
JLOG(debugLog().error())
<< "BatchTrace: Nested Signers array exceeds max entries.";
return kInitialXrp;
return std::nullopt;
}
// LCOV_EXCL_STOP
signerCount += nestedSigners.size();
@@ -152,7 +123,7 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
if (signerCount > 0 && view.fees().base > maxAmount / signerCount)
{
JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in signerCount calculation.";
return XRPAmount{kInitialXrp};
return std::nullopt;
}
// LCOV_EXCL_STOP
@@ -162,13 +133,13 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
if (signerFees > maxAmount - txnFees)
{
JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in signerFees calculation.";
return XRPAmount{kInitialXrp};
return std::nullopt;
}
XRPAmount const innerFees = txnFees + signerFees;
if (innerFees > maxAmount - batchBase)
{
JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in total fee calculation.";
return XRPAmount{kInitialXrp};
return std::nullopt;
}
// LCOV_EXCL_STOP
@@ -176,6 +147,24 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
return innerFees + batchBase;
}
XRPAmount
Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
{
if (auto const fee = calculateBaseFeeImpl(view, tx))
return *fee;
// The fee could not be computed, so return a placeholder the account can
// pay; preclaim rejects the transaction with tecINSUFF_FEE.
return view.fees().base; // LCOV_EXCL_LINE
}
TER
Batch::preclaim(PreclaimContext const& ctx)
{
if (!calculateBaseFeeImpl(ctx.view, ctx.tx))
return tecINSUFF_FEE; // LCOV_EXCL_LINE
return tesSUCCESS;
}
std::uint32_t
Batch::getFlagsMask(PreflightContext const& ctx)
{
@@ -246,13 +235,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 +275,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 +288,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 +408,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();

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

@@ -56,7 +56,6 @@
#include <xrpl/protocol/SecretKey.h>
#include <xrpl/protocol/Serializer.h>
#include <xrpl/protocol/Sign.h>
#include <xrpl/protocol/SystemParameters.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/TxFormats.h>
@@ -72,6 +71,7 @@
#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <exception>
#include <map>
#include <memory>
#include <optional>
@@ -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.
@@ -5567,8 +5571,9 @@ class Batch_test : public beast::unit_test::Suite
batch::Inner(pay(alice, bob, XRP(10)), seq + 1),
batch::Inner(pay(alice, bob, XRP(5)), seq + 2),
batch::Sig(std::vector<Reg>(kMaxBatchSigners + 1, bob)));
// Failure paths fall back to the ledger base fee.
XRPAmount const txBaseFee = getBaseFee(jtx);
BEAST_EXPECT(txBaseFee == XRPAmount(kInitialXrp));
BEAST_EXPECT(txBaseFee == env.current()->fees().base);
}
// good:

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

@@ -21,6 +21,7 @@
#include <xrpl.pb.h>
#include <cstdint>
#include <cstring>
#include <exception>
#include <memory>
@@ -50,15 +51,10 @@ public:
run() override
{
testMalformedSerializedForm();
testcase("secp256k1 signatures");
testSTTx(KeyType::Secp256k1);
testcase("ed25519 signatures");
testSTTx(KeyType::Ed25519);
testcase("STObject constructor errors");
testObjectCtorErrors();
testBatchInnerCtorErrors();
}
void
@@ -1328,6 +1324,8 @@ public:
void
testSTTx(KeyType keyType)
{
testcase(std::string(to_string(keyType)) + " signatures");
auto const keypair = randomKeyPair(keyType);
STTx j(ttACCOUNT_SET, [&keypair](auto& obj) {
@@ -1382,6 +1380,8 @@ public:
void
testObjectCtorErrors()
{
testcase("STObject constructor errors");
auto const kp1 = randomKeyPair(KeyType::Secp256k1);
auto const id1 = calcAccountID(kp1.first);
@@ -1463,6 +1463,75 @@ public:
BEAST_EXPECT(got == "Field 'Fee' is required but missing.");
}
}
void
testBatchInnerCtorErrors()
{
testcase("Batch inner transaction validation");
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 errorMsg;
try
{
STTx{makeBatch(makeInner(ttPAYMENT))};
}
catch (std::exception const& err)
{
errorMsg = err.what();
}
BEAST_EXPECT(errorMsg.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 errorMsg;
try
{
STTx{makeBatch(makeInner(60000))};
}
catch (std::exception const& err)
{
errorMsg = err.what();
}
BEAST_EXPECT(matches(errorMsg.c_str(), "Invalid transaction type 60000"));
}
}
};
class InnerObjectFormatsSerializer_test : public beast::unit_test::Suite