From d83cf86defa1aab53bde8713f94efeb98d6d42dd Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Thu, 25 Jun 2026 09:02:00 -0400 Subject: [PATCH] feat: add signature cache --- include/xrpl/core/HashRouter.h | 8 ++- src/libxrpl/tx/transactors/system/Batch.cpp | 26 +++++--- src/test/app/Batch_test.cpp | 68 +++++++++++++++++++++ 3 files changed, 92 insertions(+), 10 deletions(-) diff --git a/include/xrpl/core/HashRouter.h b/include/xrpl/core/HashRouter.h index d36b8aee6e..83d6ef8189 100644 --- a/include/xrpl/core/HashRouter.h +++ b/include/xrpl/core/HashRouter.h @@ -19,14 +19,16 @@ enum class HashRouterFlags : std::uint16_t { HELD = 0x08, // Held by LedgerMaster after potential processing failure TRUSTED = 0x10, // Comes from a trusted source - // Private flags (used internally in apply.cpp) - // Do not attempt to read, set, or reuse. + // Private slots: each is claimed by one subsystem to cache a per-tx result. + // Don't read or reuse a slot you don't own; the meaning lives at the + // constant's definition (e.g. apply.cpp, EscrowFinish.cpp, Batch.cpp). PRIVATE1 = 0x0100, PRIVATE2 = 0x0200, PRIVATE3 = 0x0400, PRIVATE4 = 0x0800, PRIVATE5 = 0x1000, - PRIVATE6 = 0x2000 + PRIVATE6 = 0x2000, + PRIVATE7 = 0x4000 }; constexpr HashRouterFlags diff --git a/src/libxrpl/tx/transactors/system/Batch.cpp b/src/libxrpl/tx/transactors/system/Batch.cpp index 95489e0480..62ca7eef59 100644 --- a/src/libxrpl/tx/transactors/system/Batch.cpp +++ b/src/libxrpl/tx/transactors/system/Batch.cpp @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include #include #include @@ -36,6 +38,10 @@ namespace xrpl { +// Set on a batch's tx id once its signer signatures verify, so +// preflightSigValidated can skip the expensive re-check on later preflights. +constexpr HashRouterFlags kSfBatchSigGood = HashRouterFlags::PRIVATE7; + /** * @brief Calculates the total base fee for a batch transaction. * @@ -490,14 +496,20 @@ Batch::preflightSigValidated(PreflightContext const& ctx) ++matched; } - // Check the batch signers signatures. - auto const sigResult = ctx.tx.checkBatchSign(ctx.rules); - - if (!sigResult) + // Cache the expensive signer-signature crypto per tx id. Safe because + // the tx id covers sfBatchSigners and checkBatchSign is rules- + // independent (revisit the key if that changes). Good-only: failures + // aren't cached, so a verdict can't become sticky. + auto& router = ctx.registry.get().getHashRouter(); + if (!any(router.getFlags(parentBatchId) & kSfBatchSigGood)) { - JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: " - << "invalid batch txn signature: " << sigResult.error(); - return temBAD_SIGNATURE; + if (auto const sigResult = ctx.tx.checkBatchSign(ctx.rules); !sigResult) + { + JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: " + << "invalid batch txn signature: " << sigResult.error(); + return temBAD_SIGNATURE; + } + router.setFlags(parentBatchId, kSfBatchSigGood); } } diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index c7a54d403b..3892e5d712 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -5873,6 +5873,73 @@ class Batch_test : public beast::unit_test::Suite env.close(); } + void + testBatchSigCache(FeatureBitset features) + { + testcase("batch signature caching"); + + using namespace test::jtx; + + // Mirrors Batch.cpp's file-local kSfBatchSigGood; keep in sync. + constexpr HashRouterFlags kSfBatchSigGood = HashRouterFlags::PRIVATE7; + + // Valid batch: alice (outer) + an inner from bob, who co-signs. + auto buildValidBatch = [](Env& env) { + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const seq = env.seq(alice); + auto const batchFee = batch::calcBatchFee(env, 1, 2); + return env.jt( + batch::outer(alice, seq, batchFee, tfAllOrNothing), + batch::Inner(pay(alice, bob, XRP(1)), seq + 1), + batch::Inner(pay(bob, alice, XRP(2)), env.seq(bob)), + batch::Sig(bob)); + }; + + // WRITE: a valid batch records "good" on its tx id. + { + Env env{*this, features}; + env.fund(XRP(10000), Account("alice"), Account("bob")); + env.close(); + + auto jt = buildValidBatch(env); + auto const txid = jt.stx->getTransactionID(); + + BEAST_EXPECT(!any(env.app().getHashRouter().getFlags(txid) & kSfBatchSigGood)); + env(jt, Ter(tesSUCCESS)); + BEAST_EXPECT(any(env.app().getHashRouter().getFlags(txid) & kSfBatchSigGood)); + env.close(); + } + + // READ: corrupt only a signer's signature (outer sig + signer key + // untouched), so just the crypto would fail. Caught when uncached... + { + Env env{*this, features}; + env.fund(XRP(10000), Account("alice"), Account("bob")); + env.close(); + + auto jt = buildValidBatch(env); + jt.jv[sfBatchSigners.jsonName][0u][sfBatchSigner.jsonName][sfTxnSignature.jsonName] = + "00"; + env(jt.jv, Ter(temBAD_SIGNATURE)); + env.close(); + } + { + // ...but a planted "good" skips the crypto, so it applies. + Env env{*this, features}; + env.fund(XRP(10000), Account("alice"), Account("bob")); + env.close(); + + auto jt = buildValidBatch(env); + jt.jv[sfBatchSigners.jsonName][0u][sfBatchSigner.jsonName][sfTxnSignature.jsonName] = + "00"; + auto const txid = STTx{parse(jt.jv)}.getTransactionID(); + env.app().getHashRouter().setFlags(txid, kSfBatchSigGood); + env(jt.jv, Ter(tesSUCCESS)); + env.close(); + } + } + void testWithFeats(FeatureBitset features) { @@ -5911,6 +5978,7 @@ class Batch_test : public beast::unit_test::Suite testStandaloneInnerBatchFlag(features); testOuterBinding(features); testUnsortedBatchSigners(features); + testBatchSigCache(features); } public: