From cc348e7745aa571b220adbf9e187c56c61b563ce Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Tue, 30 Jun 2026 09:15:21 -0400 Subject: [PATCH] chore: address review nits (safeCast, whitespace, comments) --- include/xrpl/protocol/Protocol.h | 4 ++-- include/xrpl/tx/ApplyContext.h | 4 ++++ src/libxrpl/protocol/STTx.cpp | 2 +- src/libxrpl/tx/transactors/system/Batch.cpp | 5 ++--- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/xrpl/protocol/Protocol.h b/include/xrpl/protocol/Protocol.h index 6901f9823e..ba6347dfa7 100644 --- a/include/xrpl/protocol/Protocol.h +++ b/include/xrpl/protocol/Protocol.h @@ -310,8 +310,8 @@ constexpr std::size_t kPermissionMaxSize = 10; /** The maximum number of transactions that can be in a batch. */ constexpr std::size_t kMaxBatchTxCount = 8; - - /** The maximum number of batch signers. */ + +/** The maximum number of batch signers. */ constexpr std::size_t kMaxBatchSigners = kMaxBatchTxCount * 3; /** Length of a secp256k1 scalar in bytes. */ diff --git a/include/xrpl/tx/ApplyContext.h b/include/xrpl/tx/ApplyContext.h index fb8df9292b..c98b3f82c5 100644 --- a/include/xrpl/tx/ApplyContext.h +++ b/include/xrpl/tx/ApplyContext.h @@ -24,6 +24,10 @@ public: ApplyFlags flags, beast::Journal journal = beast::Journal{beast::Journal::getNullSink()}); + // Convenience constructor used only by tests that build an ApplyContext + // directly (e.g. invariant checks). Production always uses the parentBatchId + // constructor above; this one fixes parentBatchId to std::nullopt and so is + // never valid for a batch inner (hence the TapBatch assert). explicit ApplyContext( ServiceRegistry& registry, OpenView& base, diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index b95c93d655..f7b89942ef 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -755,7 +755,7 @@ isBatchRawTransactionOkay(STObject const& st, std::string& reason) // 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 (safeCast(st.getFieldU16(sfTransactionType)) != ttBATCH) + if (st.getFieldU16(sfTransactionType) != ttBATCH) { reason = "Only Batch transactions may contain raw transactions."; return false; diff --git a/src/libxrpl/tx/transactors/system/Batch.cpp b/src/libxrpl/tx/transactors/system/Batch.cpp index 4cb44513bd..e1cd323766 100644 --- a/src/libxrpl/tx/transactors/system/Batch.cpp +++ b/src/libxrpl/tx/transactors/system/Batch.cpp @@ -484,9 +484,8 @@ Batch::preflightSigValidated(PreflightContext const& ctx) } } - // Every required signer must be matched. This is cheaper than the signature - // check below, so verify it first. Also covers sfBatchSigners being absent - // while inner txns require signers (numReqSignersMatched stays 0). + // Every required signer must be matched. Also covers sfBatchSigners being + // absent while inner txns require signers (numReqSignersMatched stays 0). if (numReqSignersMatched != requiredSigners.size()) { JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "