Compare commits

...

13 Commits

Author SHA1 Message Date
Denis Angell
ddcf08593b clang-format 2026-03-18 02:17:17 +01:00
Denis Angell
d0b54a0a5a Merge branch 'develop' into dangell7/batch-v1 2026-03-18 02:08:20 +01:00
Denis Angell
c3cff1ed5c review comments 2026-03-18 02:04:57 +01:00
Denis Angell
d892f0e73d Update src/libxrpl/tx/transactors/Batch.cpp
Co-authored-by: Mayukha Vadari <mvadari@ripple.com>
2026-03-18 02:00:43 +01:00
Denis Angell
200f93a287 Merge branch 'develop' into dangell7/batch-v1 2026-02-26 23:41:34 +01:00
Denis Angell
90d2eb839a reverse negated else 2026-02-26 22:27:24 +01:00
Denis Angell
359a94b766 add account id to batch serialization 2026-02-26 15:12:49 +01:00
Denis Angell
5e39c1d80c clang-format 2026-02-26 15:12:49 +01:00
Denis Angell
25abc8ffae minor adjustments 2026-02-26 14:03:13 +01:00
Denis Angell
a669dcec87 misc review fix
- add early `sfBatchSigners` size check
- fix log nomenclature
2026-02-26 14:03:01 +01:00
Denis Angell
4f5a3241de move checkBatchSign 2026-02-26 14:03:01 +01:00
Denis Angell
c729a26dd3 defensive check for nested Signers array size 2026-02-26 14:02:52 +01:00
Denis Angell
69084a6ff5 feature BatchV1_1 2026-02-26 14:02:51 +01:00
21 changed files with 290 additions and 165 deletions

View File

@@ -28,7 +28,7 @@ inline constexpr struct open_ledger_t
/** Batch view construction tag.
Views constructed with this tag are part of a stack of views
used during batch transaction applied.
used during batch transaction application.
*/
inline constexpr struct batch_view_t
{

View File

@@ -1,3 +1,5 @@
#pragma once
#include <xrpl/protocol/HashPrefix.h>
#include <xrpl/protocol/STVector256.h>
#include <xrpl/protocol/Serializer.h>

View File

@@ -153,13 +153,16 @@ private:
Expected<void, std::string>
checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const;
void
buildBatchTxnIds();
STBase*
copy(std::size_t n, void* buf) const override;
STBase*
move(std::size_t n, void* buf) override;
friend class detail::STVar;
mutable std::vector<uint256> batchTxnIds_;
std::vector<uint256> batchTxnIds_;
};
bool

View File

@@ -15,10 +15,9 @@
// Add new amendments to the top of this list.
// Keep it sorted in reverse chronological order.
XRPL_FEATURE(BatchV1_1, Supported::no, VoteBehavior::DefaultNo)
XRPL_FIX (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (BatchInnerSigs, Supported::no, VoteBehavior::DefaultNo)
XRPL_FEATURE(LendingProtocol, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(PermissionDelegationV1_1, Supported::no, VoteBehavior::DefaultNo)
XRPL_FIX (DirectoryLimit, Supported::yes, VoteBehavior::DefaultNo)
@@ -32,7 +31,6 @@ XRPL_FEATURE(TokenEscrow, Supported::yes, VoteBehavior::DefaultNo
XRPL_FIX (EnforceNFTokenTrustlineV2, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (AMMv1_3, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(PermissionedDEX, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(Batch, Supported::no, VoteBehavior::DefaultNo)
XRPL_FEATURE(SingleAssetVault, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (PayChanCancelAfter, Supported::yes, VoteBehavior::DefaultNo)
// Check flags in Credential transactions

View File

@@ -936,7 +936,7 @@ TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback,
#endif
TRANSACTION(ttBATCH, 71, Batch,
Delegation::notDelegable,
featureBatch,
featureBatchV1_1,
noPriv,
({
{sfRawTransactions, soeREQUIRED},

View File

@@ -122,7 +122,7 @@ private:
ApplyFlags flags_;
std::optional<ApplyViewImpl> view_;
// The ID of the batch transaction we are executing under, if seated.
// The ID of the batch transaction we are executing under, if set.
std::optional<uint256 const> parentBatchId_;
};

View File

@@ -161,9 +161,6 @@ public:
static NotTEC
checkSign(PreclaimContext const& ctx);
static NotTEC
checkBatchSign(PreclaimContext const& ctx);
// Returns the fee in fee units, not scaled for load.
static XRPAmount
calculateBaseFee(ReadView const& view, STTx const& tx);
@@ -292,14 +289,7 @@ protected:
std::optional<T> value,
unit::ValueUnit<Unit, T> min = unit::ValueUnit<Unit, T>{});
private:
std::pair<TER, XRPAmount>
reset(XRPAmount fee);
TER
consumeSeqProxy(SLE::pointer const& sleAccount);
TER
payFee();
protected:
static NotTEC
checkSingleSign(
ReadView const& view,
@@ -315,6 +305,15 @@ private:
STObject const& sigObject,
beast::Journal const j);
private:
std::pair<TER, XRPAmount>
reset(XRPAmount fee);
TER
consumeSeqProxy(SLE::pointer const& sleAccount);
TER
payFee();
void trapTransaction(uint256) const;
/** Performs early sanity checks on the account and fee fields.

View File

@@ -1,7 +1,5 @@
#pragma once
#include <xrpl/basics/Log.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/tx/Transactor.h>
namespace xrpl {
@@ -27,6 +25,9 @@ public:
static NotTEC
preflightSigValidated(PreflightContext const& ctx);
static NotTEC
checkBatchSign(PreclaimContext const& ctx);
static NotTEC
checkSign(PreclaimContext const& ctx);

View File

@@ -73,6 +73,7 @@ STTx::STTx(STObject&& object) : STObject(std::move(object))
tx_type_ = safe_cast<TxType>(getFieldU16(sfTransactionType));
applyTemplate(getTxFormat(tx_type_)->getSOTemplate()); // may throw
tid_ = getHash(HashPrefix::transactionID);
buildBatchTxnIds();
}
STTx::STTx(SerialIter& sit) : STObject(sfTransaction)
@@ -89,6 +90,7 @@ STTx::STTx(SerialIter& sit) : STObject(sfTransaction)
applyTemplate(getTxFormat(tx_type_)->getSOTemplate()); // May throw
tid_ = getHash(HashPrefix::transactionID);
buildBatchTxnIds();
}
STTx::STTx(TxType type, std::function<void(STObject&)> assembler) : STObject(sfTransaction)
@@ -106,6 +108,7 @@ STTx::STTx(TxType type, std::function<void(STObject&)> assembler) : STObject(sfT
LogicError("Transaction type was mutated during assembly");
tid_ = getHash(HashPrefix::transactionID);
buildBatchTxnIds();
}
STBase*
@@ -292,6 +295,8 @@ STTx::checkBatchSign(Rules const& rules) const
JLOG(debugLog().fatal()) << "not a batch transaction";
return Unexpected("Not a batch transaction.");
}
if (!isFieldPresent(sfBatchSigners))
return Unexpected("Missing BatchSigners field.");
STArray const& signers{getFieldArray(sfBatchSigners)};
for (auto const& signer : signers)
{
@@ -306,9 +311,8 @@ STTx::checkBatchSign(Rules const& rules) const
}
catch (std::exception const& e)
{
JLOG(debugLog().error()) << "Batch signature check failed: " << e.what();
return Unexpected(std::string("Internal batch signature check failure: ") + e.what());
}
return Unexpected("Internal batch signature check failure.");
}
Json::Value
@@ -430,6 +434,7 @@ STTx::checkBatchSingleSign(STObject const& batchSigner) const
{
Serializer msg;
serializeBatch(msg, getFlags(), getBatchTransactionIDs());
finishMultiSigningData(batchSigner.getAccountID(sfAccount), msg);
return singleSignHelper(batchSigner, msg.slice());
}
@@ -502,7 +507,7 @@ multiSignHelper(
if (!validSig)
return Unexpected(
std::string("Invalid signature on account ") + toBase58(accountID) +
errorWhat.value_or("") + ".");
(errorWhat ? ": " + *errorWhat : "") + ".");
}
// All signatures verified.
return {};
@@ -550,41 +555,24 @@ STTx::checkMultiSign(Rules const& rules, STObject const& sigObject) const
rules);
}
/**
* @brief Retrieves a batch of transaction IDs from the STTx.
*
* This function returns a vector of transaction IDs by extracting them from
* the field array `sfRawTransactions` within the STTx. If the batch
* transaction IDs have already been computed and cached in `batchTxnIds_`,
* it returns the cached vector. Otherwise, it computes the transaction IDs,
* caches them, and then returns the vector.
*
* @return A vector of `uint256` containing the batch transaction IDs.
*
* @note The function asserts that the `sfRawTransactions` field array is not
* empty and that the size of the computed batch transaction IDs matches the
* size of the `sfRawTransactions` field array.
*/
void
STTx::buildBatchTxnIds()
{
if (tx_type_ != ttBATCH || !isFieldPresent(sfRawTransactions))
return;
auto const& raw = getFieldArray(sfRawTransactions);
batchTxnIds_.reserve(raw.size());
for (STObject const& rb : raw)
batchTxnIds_.push_back(rb.getHash(HashPrefix::transactionID));
}
std::vector<uint256> const&
STTx::getBatchTransactionIDs() const
{
XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::getBatchTransactionIDs : not a batch transaction");
XRPL_ASSERT(
getFieldArray(sfRawTransactions).size() != 0,
"STTx::getBatchTransactionIDs : empty raw transactions");
// The list of inner ids is built once, then reused on subsequent calls.
// After the list is built, it must always have the same size as the array
// `sfRawTransactions`. The assert below verifies that.
if (batchTxnIds_.size() == 0)
{
for (STObject const& rb : getFieldArray(sfRawTransactions))
batchTxnIds_.push_back(rb.getHash(HashPrefix::transactionID));
}
XRPL_ASSERT(
batchTxnIds_.size() == getFieldArray(sfRawTransactions).size(),
"STTx::getBatchTransactionIDs : batch transaction IDs size mismatch");
!batchTxnIds_.empty(), "STTx::getBatchTransactionIDs : batch transaction IDs not built");
return batchTxnIds_;
}
@@ -740,7 +728,7 @@ isRawTransactionOkay(STObject const& st, std::string& reason)
reason = "Raw Transactions array exceeds max entries.";
return false;
}
for (STObject raw : rawTxns)
for (auto raw : rawTxns)
{
try
{
@@ -752,6 +740,9 @@ isRawTransactionOkay(STObject const& st, std::string& reason)
}
raw.applyTemplate(getTxFormat(tt)->getSOTemplate());
if (!passesLocalChecks(raw, reason))
return false;
}
catch (std::exception const& e)
{

View File

@@ -175,12 +175,16 @@ Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask)
if (ctx.tx.getSeqProxy().isTicket() && ctx.tx.isFieldPresent(sfAccountTxnID))
return temINVALID;
if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatch))
if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatchV1_1))
return temINVALID_FLAG;
if (ctx.rules.enabled(featureBatchV1_1) &&
ctx.tx.isFlag(tfInnerBatchTxn) != ctx.parentBatchId.has_value())
return temINVALID_INNER_BATCH;
XRPL_ASSERT(
ctx.tx.isFlag(tfInnerBatchTxn) == ctx.parentBatchId.has_value() ||
!ctx.rules.enabled(featureBatch),
!ctx.rules.enabled(featureBatchV1_1),
"Inner batch transaction must have a parent batch ID.");
return tesSUCCESS;
@@ -196,13 +200,13 @@ Transactor::preflight2(PreflightContext const& ctx)
return *ret;
// It should be impossible for the InnerBatchTxn flag to be set without
// featureBatch being enabled
// featureBatchV1_1 being enabled
XRPL_ASSERT_PARTS(
!ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatch),
!ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatchV1_1),
"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(featureBatchV1_1))
return tesSUCCESS;
// Do not add any checks after this point that are relevant for
// batch inner transactions. They will be skipped.
@@ -631,7 +635,7 @@ Transactor::checkSign(
auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey);
// Ignore signature check on batch inner transactions
if (parentBatchId && view.rules().enabled(featureBatch))
if (parentBatchId && view.rules().enabled(featureBatchV1_1))
{
// Defensive Check: These values are also checked in Batch::preflight
if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||
@@ -683,50 +687,6 @@ Transactor::checkSign(PreclaimContext const& ctx)
return checkSign(ctx.view, ctx.flags, ctx.parentBatchId, idAccount, ctx.tx, ctx.j);
}
NotTEC
Transactor::checkBatchSign(PreclaimContext const& ctx)
{
NotTEC ret = tesSUCCESS;
STArray const& signers{ctx.tx.getFieldArray(sfBatchSigners)};
for (auto const& signer : signers)
{
auto const idAccount = signer.getAccountID(sfAccount);
Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey);
if (pkSigner.empty())
{
if (ret = checkMultiSign(ctx.view, ctx.flags, idAccount, signer, ctx.j);
!isTesSuccess(ret))
return ret;
}
else
{
// LCOV_EXCL_START
if (!publicKeyType(makeSlice(pkSigner)))
return tefBAD_AUTH;
// LCOV_EXCL_STOP
auto const idSigner = calcAccountID(PublicKey(makeSlice(pkSigner)));
auto const sleAccount = ctx.view.read(keylet::account(idAccount));
// A batch can include transactions from an un-created account ONLY
// when the account master key is the signer
if (!sleAccount)
{
if (idAccount != idSigner)
return tefBAD_AUTH;
return tesSUCCESS;
}
if (ret = checkSingleSign(ctx.view, idSigner, idAccount, sleAccount, ctx.j);
!isTesSuccess(ret))
return ret;
}
}
return ret;
}
NotTEC
Transactor::checkSingleSign(
ReadView const& view,

View File

@@ -24,29 +24,12 @@ checkValidity(HashRouter& router, STTx const& tx, Rules const& rules)
auto const flags = router.getFlags(id);
// Ignore signature check on batch inner transactions
if (tx.isFlag(tfInnerBatchTxn) && rules.enabled(featureBatch))
if (tx.isFlag(tfInnerBatchTxn) && rules.enabled(featureBatchV1_1))
{
// Defensive Check: These values are also checked in Batch::preflight
if (tx.isFieldPresent(sfTxnSignature) || !tx.getSigningPubKey().empty() ||
tx.isFieldPresent(sfSigners))
return {Validity::SigBad, "Malformed: Invalid inner batch transaction."};
// 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)
{
std::string reason;
if (!passesLocalChecks(tx, reason))
{
router.setFlags(id, SF_LOCALBAD);
return {Validity::SigGoodOnly, reason};
}
router.setFlags(id, SF_SIGGOOD);
return {Validity::Valid, ""};
}
}
if (any(flags & SF_SIGBAD))

View File

@@ -26,7 +26,7 @@ LoanSet::preflight(PreflightContext const& ctx)
auto const& tx = ctx.tx;
// Special case for Batch inner transactions
if (tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatch) &&
if (tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatchV1_1) &&
!tx.isFieldPresent(sfCounterparty))
{
auto const parentBatchId = ctx.parentBatchId.value_or(uint256{0});

View File

@@ -63,9 +63,9 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
}
// LCOV_EXCL_STOP
for (STObject txn : txns)
for (auto const& txn : txns)
{
STTx const stx = STTx{std::move(txn)};
STTx const stx = STTx{STObject(txn)};
// LCOV_EXCL_START
if (stx.getTxnType() == ttBATCH)
@@ -107,7 +107,18 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
if (signer.isFieldPresent(sfTxnSignature))
signerCount += 1;
else if (signer.isFieldPresent(sfSigners))
signerCount += signer.getFieldArray(sfSigners).size();
{
auto const& nestedSigners = signer.getFieldArray(sfSigners);
// LCOV_EXCL_START
if (nestedSigners.size() > STTx::maxMultiSigners)
{
JLOG(debugLog().error())
<< "BatchTrace: Nested Signers array exceeds max entries.";
return XRPAmount{INITIAL_XRP};
}
// LCOV_EXCL_STOP
signerCount += nestedSigners.size();
}
}
}
@@ -205,6 +216,14 @@ Batch::preflight(PreflightContext const& ctx)
return temARRAY_TOO_LARGE;
}
if (ctx.tx.isFieldPresent(sfBatchSigners) &&
ctx.tx.getFieldArray(sfBatchSigners).size() > maxBatchTxCount)
{
JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:"
<< "signers array exceeds 8 entries.";
return temARRAY_TOO_LARGE;
}
// Validation Inner Batch Txns
std::unordered_set<uint256> uniqueHashes;
std::unordered_map<AccountID, std::unordered_set<std::uint32_t>> accountSeqTicket;
@@ -237,9 +256,9 @@ Batch::preflight(PreflightContext const& ctx)
return tesSUCCESS;
};
for (STObject rb : rawTxns)
for (auto const& rb : rawTxns)
{
STTx const stx = STTx{std::move(rb)};
STTx const stx = STTx{STObject(rb)};
auto const hash = stx.getTransactionID();
if (!uniqueHashes.emplace(hash).second)
{
@@ -426,7 +445,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
if (requiredSigners.erase(signerAccount) == 0)
{
JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "
<< "no account signature for inner txn.";
<< "extra signer provided: " << signerAccount;
return temBAD_SIGNER;
}
}
@@ -451,6 +470,52 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
return tesSUCCESS;
}
NotTEC
Batch::checkBatchSign(PreclaimContext const& ctx)
{
NotTEC ret = tesSUCCESS;
STArray const& signers{ctx.tx.getFieldArray(sfBatchSigners)};
for (auto const& signer : signers)
{
auto const idAccount = signer.getAccountID(sfAccount);
Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey);
if (pkSigner.empty())
{
if (ret = checkMultiSign(ctx.view, ctx.flags, idAccount, signer, ctx.j);
!isTesSuccess(ret))
return ret;
}
else
{
if (!publicKeyType(makeSlice(pkSigner)))
return tefBAD_AUTH; // LCOV_EXCL_LINE
auto const idSigner = calcAccountID(PublicKey(makeSlice(pkSigner)));
auto const sleAccount = ctx.view.read(keylet::account(idAccount));
if (sleAccount)
{
if (isPseudoAccount(sleAccount))
return tefBAD_AUTH;
if (ret = checkSingleSign(ctx.view, idSigner, idAccount, sleAccount, ctx.j);
!isTesSuccess(ret))
return ret;
}
else
{
if (idAccount != idSigner)
return tefBAD_AUTH;
// A batch can include transactions from an un-created account ONLY
// when the account master key is the signer
}
}
}
return ret;
}
/**
* @brief Checks the validity of signatures for a batch transaction.
*
@@ -459,7 +524,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
* corresponding error code.
*
* Next, it verifies the batch-specific signature requirements by calling
* Transactor::checkBatchSign. If this check fails, it also returns the
* Batch::checkBatchSign. If this check fails, it also returns the
* corresponding error code.
*
* If both checks succeed, the function returns tesSUCCESS.
@@ -474,8 +539,11 @@ Batch::checkSign(PreclaimContext const& ctx)
if (auto ret = Transactor::checkSign(ctx); !isTesSuccess(ret))
return ret;
if (auto ret = Transactor::checkBatchSign(ctx); !isTesSuccess(ret))
return ret;
if (ctx.tx.isFieldPresent(sfBatchSigners))
{
if (auto ret = checkBatchSign(ctx); !isTesSuccess(ret))
return ret;
}
return tesSUCCESS;
}

View File

@@ -13,6 +13,7 @@
#include <xrpl/protocol/jss.h>
#include <xrpl/server/NetworkOPs.h>
#include <xrpl/tx/apply.h>
#include <xrpl/tx/transactors/payment/Payment.h>
#include <xrpl/tx/transactors/system/Batch.h>
namespace xrpl {
@@ -141,14 +142,11 @@ class Batch_test : public beast::unit_test::suite
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");
testcase << "enabled: Batch " << (withBatch ? "enabled" : "disabled");
auto const amend = withBatch ? features : features - featureBatch;
auto const amend = withBatch ? features : features - featureBatchV1_1;
test::jtx::Env env{*this, amend};
@@ -372,6 +370,25 @@ class Batch_test : public beast::unit_test::suite
env.close();
}
// temINVALID_INNER_BATCH: tfInnerBatchTxn set but no parentBatchId.
{
auto jtx = env.jt(pay(alice, bob, XRP(1)), txflags(tfInnerBatchTxn));
PreflightContext pfCtx(
env.app(), *jtx.stx, env.current()->rules(), tapNONE, env.journal);
auto const pf = Transactor::invokePreflight<Payment>(pfCtx);
BEAST_EXPECT(pf == temINVALID_INNER_BATCH);
}
// temINVALID_INNER_BATCH: parentBatchId set but tfInnerBatchTxn not
// set.
{
auto jtx = env.jt(pay(alice, bob, XRP(1)));
PreflightContext pfCtx(
env.app(), *jtx.stx, uint256{1}, env.current()->rules(), tapBATCH, env.journal);
auto const pf = Transactor::invokePreflight<Payment>(pfCtx);
BEAST_EXPECT(pf == temINVALID_INNER_BATCH);
}
// temBAD_FEE: Batch: inner txn must have a fee of 0.
{
auto const seq = env.seq(alice);
@@ -553,6 +570,7 @@ class Batch_test : public beast::unit_test::suite
Serializer msg;
serializeBatch(msg, tfAllOrNothing, jt.stx->getBatchTransactionIDs());
finishMultiSigningData(bob.id(), msg);
auto const sig = xrpl::sign(bob.pk(), bob.sk(), msg.slice());
jt.jv[sfBatchSigners.jsonName][0u][sfBatchSigner.jsonName][sfAccount.jsonName] =
bob.human();
@@ -912,6 +930,47 @@ class Batch_test : public beast::unit_test::suite
env(jt.jv, batch::sig(bob), ter(telENV_RPC_FAILED));
env.close();
}
// Invalid: inner txn with MPT amount on tx type that doesn't
// support MPT (OfferCreate TakerPays)
{
MPTIssue issue(makeMptID(1, alice));
STAmount mptAmt{issue, UINT64_C(100)};
auto const batchFee = batch::calcBatchFee(env, 0, 2);
auto const seq = env.seq(alice);
Json::Value tx1;
tx1[jss::TransactionType] = jss::OfferCreate;
tx1[jss::Account] = alice.human();
tx1[jss::TakerPays] = mptAmt.getJson(JsonOptions::none);
tx1[jss::TakerGets] = XRP(10).value().getJson(JsonOptions::none);
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(tx1, seq + 1),
batch::inner(pay(alice, bob, XRP(1)), seq + 2),
ter(telENV_RPC_FAILED));
env.close();
}
// Invalid: inner txn with invalid memo (non-URL-safe MemoType)
{
auto const batchFee = batch::calcBatchFee(env, 0, 2);
auto const seq = env.seq(alice);
auto tx1 = pay(alice, bob, XRP(10));
auto& ma = tx1["Memos"];
auto& mi = ma[ma.size()];
auto& m = mi["Memo"];
m["MemoType"] = strHex(std::string("\x01\x02\x03", 3));
m["MemoData"] = strHex(std::string("test"));
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(tx1, seq + 1),
batch::inner(pay(alice, bob, XRP(1)), seq + 2),
ter(telENV_RPC_FAILED));
env.close();
}
}
void
@@ -1405,7 +1464,7 @@ class Batch_test : public beast::unit_test::suite
env.close();
}
// temARRAY_TOO_LARGE: Batch: signers array exceeds 8 entries.
// temARRAY_TOO_LARGE: Batch preflight: signers array exceeds 8 entries.
{
test::jtx::Env env{*this, features};
@@ -2191,22 +2250,16 @@ class Batch_test : public beast::unit_test::suite
void
doTestInnerSubmitRPC(FeatureBitset features, bool withBatch)
{
bool const withInnerSigFix = features[fixBatchInnerSigs];
std::string const testName =
std::string("inner submit rpc: batch ") + (withBatch ? "enabled" : "disabled") + ": ";
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;
auto const amend = withBatch ? features : features - featureBatchV1_1;
using namespace test::jtx;
using namespace std::literals;
test::jtx::Env env{*this, amend};
if (!BEAST_EXPECT(amend[featureBatch] == withBatch))
if (!BEAST_EXPECT(amend[featureBatchV1_1] == withBatch))
return;
auto const alice = Account("alice");
@@ -2328,8 +2381,7 @@ class Batch_test : public beast::unit_test::suite
s.slice(),
__LINE__,
"fails local checks: Empty SigningPubKey.",
"fails local checks: Empty SigningPubKey.",
withBatch && !withInnerSigFix);
"fails local checks: Empty SigningPubKey.");
}
// Invalid RPC Submission: tfInnerBatchTxn pseudo-transaction
@@ -2340,7 +2392,7 @@ class Batch_test : public beast::unit_test::suite
{
STTx amendTx(ttAMENDMENT, [seq = env.closed()->header().seq + 1](auto& obj) {
obj.setAccountID(sfAccount, AccountID());
obj.setFieldH256(sfAmendment, fixBatchInnerSigs);
obj.setFieldH256(sfAmendment, featureBatchV1_1);
obj.setFieldU32(sfLedgerSequence, seq);
obj.setFieldU32(sfFlags, tfInnerBatchTxn);
});
@@ -2352,8 +2404,7 @@ class Batch_test : public beast::unit_test::suite
"Pseudo-transaction",
s.slice(),
__LINE__,
withInnerSigFix ? "fails local checks: Empty SigningPubKey."
: "fails local checks: Cannot submit pseudo transactions.",
"fails local checks: Empty SigningPubKey.",
"fails local checks: Empty SigningPubKey.");
}
}
@@ -2414,6 +2465,53 @@ class Batch_test : public beast::unit_test::suite
BEAST_EXPECT(env.balance(bob) == XRP(1000));
}
void
testCheckAllSignatures(FeatureBitset features)
{
testcase("check all signatures");
using namespace test::jtx;
using namespace std::literals;
// Verifies that checkBatchSign validates all signers even when an
// unfunded account (signed with its master key) appears first in the
// sorted signer list. A funded account with an invalid signature must
// still be rejected with tefBAD_AUTH.
test::jtx::Env env{*this, features};
auto const alice = Account("alice");
// "aaa" sorts before other accounts alphabetically, ensuring the
// unfunded account is checked first in the sorted signer list
auto const unfunded = Account("aaa");
auto const carol = Account("carol");
env.fund(XRP(10000), alice, carol);
env.close();
// Verify sort order: unfunded.id() < carol.id()
BEAST_EXPECT(unfunded.id() < carol.id());
auto const seq = env.seq(alice);
auto const ledSeq = env.current()->seq();
auto const batchFee = batch::calcBatchFee(env, 2, 3);
// The batch includes:
// 1. alice pays unfunded (to create unfunded's account)
// 2. unfunded does a noop (signed by unfunded's master key - valid)
// 3. carol pays alice (signed by alice's key - INVALID since alice is
// not carol's regular key)
//
// checkBatchSign must validate all signers regardless of order.
// This must fail with tefBAD_AUTH.
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(pay(alice, unfunded, XRP(100)), seq + 1),
batch::inner(noop(unfunded), ledSeq),
batch::inner(pay(carol, alice, XRP(1000)), env.seq(carol)),
batch::sig(unfunded, Reg{carol, alice}),
ter(tefBAD_AUTH));
env.close();
}
void
testAccountSet(FeatureBitset features)
{
@@ -4329,6 +4427,7 @@ class Batch_test : public beast::unit_test::suite
testIndependent(features);
testInnerSubmitRPC(features);
testAccountActivation(features);
testCheckAllSignatures(features);
testAccountSet(features);
testAccountDelete(features);
testLoan(features);
@@ -4355,7 +4454,6 @@ public:
using namespace test::jtx;
auto const sa = testable_amendments();
testWithFeats(sa - fixBatchInnerSigs);
testWithFeats(sa);
}
};

View File

@@ -2,14 +2,13 @@
#include <test/jtx/Account.h>
#include <test/jtx/Env.h>
#include <test/jtx/SignerUtils.h>
#include <test/jtx/amount.h>
#include <test/jtx/owners.h>
#include <test/jtx/tags.h>
#include <xrpl/protocol/TxFlags.h>
#include "test/jtx/SignerUtils.h"
#include <concepts>
#include <cstdint>
#include <optional>
@@ -34,7 +33,6 @@ class inner
{
private:
Json::Value txn_;
std::uint32_t seq_;
std::optional<std::uint32_t> ticket_;
public:
@@ -42,10 +40,10 @@ public:
Json::Value const& txn,
std::uint32_t const& sequence,
std::optional<std::uint32_t> const& ticket = std::nullopt)
: txn_(txn), seq_(sequence), ticket_(ticket)
: txn_(txn), ticket_(ticket)
{
txn_[jss::SigningPubKey] = "";
txn_[jss::Sequence] = seq_;
txn_[jss::Sequence] = sequence;
txn_[jss::Fee] = "0";
txn_[jss::Flags] = txn_[jss::Flags].asUInt() | tfInnerBatchTxn;

View File

@@ -74,6 +74,7 @@ sig::operator()(Env& env, JTx& jt) const
Serializer msg;
serializeBatch(msg, stx.getFlags(), stx.getBatchTransactionIDs());
finishMultiSigningData(e.acct.id(), msg);
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
auto const sig = xrpl::sign(*publicKeyType(e.sig.pk().slice()), e.sig.sk(), msg.slice());
jo[sfTxnSignature.getJsonName()] = strHex(Slice{sig.data(), sig.size()});

View File

@@ -118,7 +118,7 @@ class Feature_test : public beast::unit_test::suite
// or removed, swap out for any other feature.
BEAST_EXPECT(
featureToName(fixRemoveNFTokenAutoTrustLine) == "fixRemoveNFTokenAutoTrustLine");
BEAST_EXPECT(featureToName(featureBatch) == "Batch");
BEAST_EXPECT(featureToName(featureBatchV1_1) == "BatchV1_1");
BEAST_EXPECT(featureToName(featureDID) == "DID");
BEAST_EXPECT(featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields");
BEAST_EXPECT(featureToName(featureTokenEscrow) == "TokenEscrow");

View File

@@ -395,6 +395,20 @@ class Simulate_test : public beast::unit_test::suite
BEAST_EXPECT(
resp[jss::result][jss::error_message] == "Transaction should not be signed.");
}
{
// tfInnerBatchTxn flag on top-level transaction
Json::Value params;
Json::Value tx_json = Json::objectValue;
tx_json[jss::TransactionType] = jss::AccountSet;
tx_json[jss::Account] = env.master.human();
tx_json[jss::Flags] = tfInnerBatchTxn;
params[jss::tx_json] = tx_json;
auto const resp = env.rpc("json", "simulate", to_string(params));
BEAST_EXPECT(
resp[jss::result][jss::error_message] ==
"tfInnerBatchTxn flag is not allowed on top-level transactions.");
}
}
void
@@ -451,7 +465,7 @@ class Simulate_test : public beast::unit_test::suite
auto jt = env.jtnofill(
batch::outer(alice, env.seq(alice), batchFee, tfAllOrNothing),
batch::inner(pay(alice, bob, XRP(10)), seq + 1),
batch::inner(pay(alice, bob, XRP(10)), seq + 1));
batch::inner(pay(alice, bob, XRP(10)), seq + 2));
jt.jv.removeMember(jss::TxnSignature);
Json::Value params;

View File

@@ -1113,7 +1113,8 @@ NetworkOPsImp::submitTransaction(std::shared_ptr<STTx const> const& iTrans)
}
// Enforce Network bar for batch txn
if (iTrans->isFlag(tfInnerBatchTxn) && m_ledgerMaster.getValidatedRules().enabled(featureBatch))
if (iTrans->isFlag(tfInnerBatchTxn) &&
m_ledgerMaster.getValidatedRules().enabled(featureBatchV1_1))
{
JLOG(m_journal.error()) << "Submitted transaction invalid: tfInnerBatchTxn flag present.";
return;
@@ -1179,7 +1180,7 @@ NetworkOPsImp::preProcessTransaction(std::shared_ptr<Transaction>& transaction)
// under no circumstances will we ever accept an inner txn within a batch
// txn from the network.
auto const sttx = *transaction->getSTransaction();
if (sttx.isFlag(tfInnerBatchTxn) && view->rules().enabled(featureBatch))
if (sttx.isFlag(tfInnerBatchTxn) && view->rules().enabled(featureBatchV1_1))
{
transaction->setStatus(INVALID);
transaction->setResult(temINVALID_FLAG);

View File

@@ -1291,7 +1291,7 @@ PeerImp::handleTransaction(
// Charge strongly for attempting to relay a txn with tfInnerBatchTxn
// LCOV_EXCL_START
/*
There is no need to check whether the featureBatch amendment is
There is no need to check whether the featureBatchV1_1 amendment is
enabled.
* If the `tfInnerBatchTxn` flag is set, and the amendment is
@@ -2740,7 +2740,7 @@ PeerImp::checkTransaction(
// charge strongly for relaying batch txns
// LCOV_EXCL_START
/*
There is no need to check whether the featureBatch amendment is
There is no need to check whether the featureBatchV1_1 amendment is
enabled.
* If the `tfInnerBatchTxn` flag is set, and the amendment is

View File

@@ -14,6 +14,7 @@
#include <xrpl/protocol/NFTSyntheticSerializer.h>
#include <xrpl/protocol/RPCErr.h>
#include <xrpl/protocol/STParsedJSON.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/resource/Fees.h>
#include <xrpl/tx/apply.h>
@@ -332,6 +333,13 @@ doSimulate(RPC::JsonContext& context)
return RPC::make_error(rpcNOT_IMPL);
}
// Reject transactions with the tfInnerBatchTxn flag.
if (stTx->isFlag(tfInnerBatchTxn))
{
return RPC::make_error(
rpcINVALID_PARAMS, "tfInnerBatchTxn flag is not allowed on top-level transactions.");
}
std::string reason;
auto transaction = std::make_shared<Transaction>(stTx, reason, context.app);
// Actually run the transaction through the transaction processor