From 09c4e35c519db086c216b7e2663069d692f9bac2 Mon Sep 17 00:00:00 2001
From: TimothyBanks <tbanks@ripple.com>
Date: Thu, 10 Sep 2026 09:40:55 -0400
Subject: [PATCH] chore: Address code review comments

---
 src/tests/libxrpl/helpers/TxTest.h            | 36 +++++--------------
 .../libxrpl/tx/wasm/fixtures/EscrowWasm.h     |  8 -----
 .../tx/wasm/transactor/BytecodePreflight.cpp  |  2 --
 3 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/src/tests/libxrpl/helpers/TxTest.h b/src/tests/libxrpl/helpers/TxTest.h
index 165a91b1d1..9a26050eb5 100644
--- a/src/tests/libxrpl/helpers/TxTest.h
+++ b/src/tests/libxrpl/helpers/TxTest.h
@@ -240,37 +240,19 @@ public:
      * @tparam T A type derived from TransactionBuilderBase.
      * @param builder The transaction builder.
      * @param signer The account to sign with.
+     * @param fee The fee to pay. The 10 drop default is below what some transactions
+     *            require: an `EscrowCreate` carrying `sfBytecode` owes
+     *            `base * 10 + 5 * bytecodeBytes` (`EscrowCreate::calculateBaseFee`), and an
+     *            `EscrowFinish` carrying `sfGas` owes the allowance priced at `gasPrice`.
+     *            Those submissions would fail on the fee rather than on whatever they meant
+     *            to test, so they must pass one explicitly.
      * @return TxResult containing the result code, applied status, and metadata.
      */
     template <typename T>
         requires std::
             derived_from<std::decay_t<T>, transactions::TransactionBuilderBase<std::decay_t<T>>>
         [[nodiscard]] TxResult
-        submit(T&& builder, Account const& signer)
-    {
-        return submit(std::forward<T>(builder), signer, XRPAmount{10});
-    }
-
-    /**
-     * @brief Submit a transaction from a builder, paying an explicit fee.
-     *
-     * The overload above pays a flat 10 drops, which is below what some transactions
-     * require: an `EscrowCreate` carrying `sfBytecode` owes `base * 10 + 5 * bytecodeBytes`
-     * (`EscrowCreate::calculateBaseFee`), and an `EscrowFinish` carrying `sfGas` owes the
-     * allowance priced at `gasPrice`. Those submissions would fail on the fee rather than on
-     * whatever they meant to test.
-     *
-     * @tparam T A type derived from TransactionBuilderBase.
-     * @param builder The transaction builder.
-     * @param signer The account to sign with.
-     * @param fee The fee to pay.
-     * @return TxResult containing the result code, applied status, and metadata.
-     */
-    template <typename T>
-        requires std::
-            derived_from<std::decay_t<T>, transactions::TransactionBuilderBase<std::decay_t<T>>>
-        [[nodiscard]] TxResult
-        submit(T&& builder, Account const& signer, XRPAmount fee)
+        submit(T&& builder, Account const& signer, XRPAmount fee = XRPAmount{10})
     {
         auto const& obj = builder.getSTObject();
         auto accountId = obj[sfAccount];
@@ -297,14 +279,14 @@ public:
      * @tparam T A type derived from TransactionBuilderBase.
      * @param builder The transaction builder.
      * @param signer The account to sign with.
-     * @param fee The fee to pay.
+     * @param fee The fee to pay; see `submit` for when the default is not enough.
      * @return The result code and the metadata produced by the close.
      */
     template <typename T>
         requires std::
             derived_from<std::decay_t<T>, transactions::TransactionBuilderBase<std::decay_t<T>>>
         [[nodiscard]] ClosedResult
-        submitAndClose(T&& builder, Account const& signer, XRPAmount fee)
+        submitAndClose(T&& builder, Account const& signer, XRPAmount fee = XRPAmount{10})
     {
         auto const result = submit(std::forward<T>(builder), signer, fee);
         close();
diff --git a/src/tests/libxrpl/tx/wasm/fixtures/EscrowWasm.h b/src/tests/libxrpl/tx/wasm/fixtures/EscrowWasm.h
index 0f2e8f898a..2d67a80f36 100644
--- a/src/tests/libxrpl/tx/wasm/fixtures/EscrowWasm.h
+++ b/src/tests/libxrpl/tx/wasm/fixtures/EscrowWasm.h
@@ -24,14 +24,6 @@ inline constexpr auto kReadsLedgerSqn = std::string_view{R"wat(
     (i32.const 5)))
 )wat"};
 
-// Returns 0, which `EscrowFinish` reads as a contract-defined rejection.
-inline constexpr auto kRejects = std::string_view{R"wat(
-(module
-  (memory (export "memory") 1)
-  (func (export "escrow_finish") (result i32)
-    (i32.const 0)))
-)wat"};
-
 // Traps. A fault rather than a rejection: no return code, and nothing it wrote survives.
 inline constexpr auto kTraps = std::string_view{R"wat(
 (module
diff --git a/src/tests/libxrpl/tx/wasm/transactor/BytecodePreflight.cpp b/src/tests/libxrpl/tx/wasm/transactor/BytecodePreflight.cpp
index 086c7aea3b..428cdf6e08 100644
--- a/src/tests/libxrpl/tx/wasm/transactor/BytecodePreflight.cpp
+++ b/src/tests/libxrpl/tx/wasm/transactor/BytecodePreflight.cpp
@@ -158,8 +158,6 @@ TEST_F(BytecodePreflight, BytecodeWithoutACancelTimeIsRefused)
     EXPECT_EQ(env.submit(withFinish, alice, fee).ter, temBAD_EXPIRATION);
 }
 
-// The success side, and the reason this file could not exist before: these cases need a
-// module that actually passes screening, which the old compiled fixtures stopped doing.
 TEST_F(BytecodePreflight, BytecodeWithACancelTimeIsAccepted)
 {
     auto env = TxTest{};