feat: TEMP DRAFT: Convert CMake UNIT_TEST_REFERENCE_FEE to command line unittest-fee

.github/scripts/strategy-matrix/generate.py

@@ -40,6 +40,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
         # The default CMake target is 'all' for Linux and MacOS and 'install'
         # for Windows, but it can get overridden for certain configurations.
         cmake_target = "install" if os["distro_name"] == "windows" else "all"
+        unittest_args = ""
 
         # We build and test all configurations by default, except for Windows in
         # Debug, because it is too slow, as well as when code coverage is
@@ -67,7 +68,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
                         and build_type == "Release"
                         and architecture["platform"] == "linux/amd64"
                     ):
-                        cmake_args = f"-DUNIT_TEST_REFERENCE_FEE=500 {cmake_args}"
+                        unittest_args = f"{unittest_args} --unittest-fee=500"
                         skip = False
                     if (
                         f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-15"
@@ -87,7 +88,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
                         and build_type == "Release"
                         and architecture["platform"] == "linux/amd64"
                     ):
-                        cmake_args = f"-DUNIT_TEST_REFERENCE_FEE=1000 {cmake_args}"
+                        unittest_args = f"{unittest_args} --unittest-fee=1000"
                         skip = False
                     if (
                         f"{os['compiler_name']}-{os['compiler_version']}" == "clang-20"
@@ -245,6 +246,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
                 {
                     "config_name": config_name + "-asan-ubsan",
                     "cmake_args": cmake_args,
+                    "unittest_args": unittest_args,
                     "cmake_target": cmake_target,
                     "build_only": build_only,
                     "build_type": build_type,
@@ -260,6 +262,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
                     {
                         "config_name": config_name + "-tsan-ubsan",
                         "cmake_args": cmake_args,
+                        "unittest_args": unittest_args,
                         "cmake_target": cmake_target,
                         "build_only": build_only,
                         "build_type": build_type,
@@ -273,6 +276,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
                 {
                     "config_name": config_name,
                     "cmake_args": cmake_args,
+                    "unittest_args": unittest_args,
                     "cmake_target": cmake_target,
                     "build_only": build_only,
                     "build_type": build_type,

.github/workflows/reusable-build-test-config.yml

@@ -25,6 +25,12 @@ on:
         type: string
         default: ""
 
+      unittest_args:
+        description: "Additional arguments to pass to rippled when running tests"
+        required: false
+        type: string
+        default: ""
+
       cmake_target:
         description: "The CMake target to build."
         required: true
@@ -260,11 +266,12 @@ jobs:
         working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }}
         env:
           BUILD_NPROC: ${{ steps.nproc.outputs.nproc }}
+          UNITTEST_ARGS: ${{ inputs.unittest_args }}
         run: |
           set -o pipefail
           # Coverage builds are slower due to instrumentation; use fewer parallel jobs to avoid flakiness
           [ "$COVERAGE_ENABLED" = "true" ] && BUILD_NPROC=$(( BUILD_NPROC - 2 ))
-          ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log
+          ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" ${UNITTEST_ARGS} 2>&1 | tee unittest.log
 
       - name: Show test failure summary
         if: ${{ failure() && !inputs.build_only }}

.github/workflows/reusable-build-test.yml

@@ -53,6 +53,7 @@ jobs:
       build_type: ${{ matrix.build_type }}
       ccache_enabled: ${{ inputs.ccache_enabled }}
       cmake_args: ${{ matrix.cmake_args }}
+      unittest_args: ${{ matrix.unittest_args }}
       cmake_target: ${{ matrix.cmake_target }}
       runs_on: ${{ toJSON(matrix.architecture.runner) }}
       image: ${{ contains(matrix.architecture.platform, 'linux') && format('ghcr.io/xrplf/ci/{0}-{1}:{2}-{3}-sha-{4}', matrix.os.distro_name, matrix.os.distro_version, matrix.os.compiler_name, matrix.os.compiler_version, matrix.os.image_sha) || '' }}

cmake/XrplCore.cmake

@@ -239,10 +239,6 @@ if(xrpld)
     add_executable(xrpld)
     if(tests)
         target_compile_definitions(xrpld PUBLIC ENABLE_TESTS)
-        target_compile_definitions(
-            xrpld
-            PRIVATE UNIT_TEST_REFERENCE_FEE=${UNIT_TEST_REFERENCE_FEE}
-        )
     endif()
     target_include_directories(
         xrpld

cmake/XrplSettings.cmake

@@ -23,12 +23,6 @@ option(assert "Enables asserts, even in release builds" OFF)
 option(xrpld "Build xrpld" ON)
 
 option(tests "Build tests" ON)
-if(tests)
-    # This setting allows making a separate workflow to test fees other than default 10
-    if(NOT UNIT_TEST_REFERENCE_FEE)
-        set(UNIT_TEST_REFERENCE_FEE "10" CACHE STRING "")
-    endif()
-endif()
 
 option(unity "Creates a build using UNITY support in cmake." OFF)
 if(unity)

include/xrpl/beast/unit_test/runner.h

@@ -9,6 +9,7 @@
 #include <boost/assert.hpp>
 
 #include <mutex>
+#include <optional>
 #include <string>
 
 namespace beast {
@@ -22,6 +23,7 @@ namespace unit_test {
 class runner
 {
     std::string arg_;
+    std::optional<std::int64_t> referenceFee_;
     bool default_ = false;
     bool failed_ = false;
     bool cond_ = false;
@@ -54,6 +56,24 @@ class runner
         return arg_;
     }
 
+    /** Set the reference fee (in drops) for tests.
+
+        If provided, this value is used in every suite that
+        does not override it.
+    */
+    void
+    referenceFee(std::int64_t fee)
+    {
+        referenceFee_ = fee;
+    }
+
+    /** Returns the reference fee, if any. */
+    std::optional<std::int64_t> const&
+    referenceFee() const
+    {
+        return referenceFee_;
+    }
+
     /** Run the specified suite.
         @return `true` if any conditions failed.
     */

include/xrpl/beast/unit_test/suite.h

@@ -281,6 +281,16 @@ class suite
         return runner_->arg();
     }
 
+    /** Return the reference fee associated with the runner. */
+    std::optional<std::int64_t>
+    referenceFee() const
+    {
+        assert(runner_);
+        if (!runner_)
+            return {};
+        return runner_->referenceFee();
+    }
+
     // DEPRECATED
     // @return `true` if the test condition indicates success(a false value)
     template <class Condition, class String>

include/xrpl/ledger/helpers/NFTokenHelpers.h

@@ -20,6 +20,10 @@ removeTokenOffersWithLimit(
     Keylet const& directory,
     std::size_t maxDeletableOffers);
 
+/** Returns tesSUCCESS if NFToken has few enough offers that it can be burned */
+TER
+notTooManyOffers(ReadView const& view, uint256 const& nftokenID);
+
 /** Finds the specified token in the owner's token directory. */
 std::optional<STObject>
 findToken(ReadView const& view, AccountID const& owner, uint256 const& nftokenID);

src/libxrpl/ledger/helpers/NFTokenHelpers.cpp

@@ -607,6 +607,33 @@ removeTokenOffersWithLimit(ApplyView& view, Keylet const& directory, std::size_t
     return deletedOffersCount;
 }
 
+TER
+notTooManyOffers(ReadView const& view, uint256 const& nftokenID)
+{
+    std::size_t totalOffers = 0;
+
+    {
+        Dir const buys(view, keylet::nft_buys(nftokenID));
+        for (auto iter = buys.begin(); iter != buys.end(); iter.next_page())
+        {
+            totalOffers += iter.page_size();
+            if (totalOffers > maxDeletableTokenOfferEntries)
+                return tefTOO_BIG;
+        }
+    }
+
+    {
+        Dir const sells(view, keylet::nft_sells(nftokenID));
+        for (auto iter = sells.begin(); iter != sells.end(); iter.next_page())
+        {
+            totalOffers += iter.page_size();
+            if (totalOffers > maxDeletableTokenOfferEntries)
+                return tefTOO_BIG;
+        }
+    }
+    return tesSUCCESS;
+}
+
 bool
 deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer)
 {

src/test/app/AMM_test.cpp

@@ -5043,13 +5043,7 @@ struct AMM_test : public jtx::AMMTest
         FeatureBitset const all{testable_amendments()};
 
         {
-            Env env(
-                *this,
-                envconfig([](std::unique_ptr<Config> cfg) {
-                    cfg->FEES.reference_fee = XRPAmount(1);
-                    return cfg;
-                }),
-                all);
+            Env env(*this, all, XRPAmount(1));
             fund(env, gw, {alice}, XRP(20'000), {USD(10'000)});
             AMM amm(env, gw, XRP(10'000), USD(10'000));
             for (auto i = 0; i < maxDeletableAMMTrustLines + 10; ++i)
@@ -5101,13 +5095,7 @@ struct AMM_test : public jtx::AMMTest
         }
 
         {
-            Env env(
-                *this,
-                envconfig([](std::unique_ptr<Config> cfg) {
-                    cfg->FEES.reference_fee = XRPAmount(1);
-                    return cfg;
-                }),
-                all);
+            Env env(*this, all, XRPAmount(1));
             fund(env, gw, {alice}, XRP(20'000), {USD(10'000)});
             AMM amm(env, gw, XRP(10'000), USD(10'000));
             for (auto i = 0; i < (maxDeletableAMMTrustLines * 2) + 10; ++i)

src/test/app/FeeVote_test.cpp

@@ -635,7 +635,7 @@ class FeeVote_test : public beast::unit_test::suite
         Env env(*this, testable_amendments() | featureXRPFees);
 
         // establish what the current fees are
-        BEAST_EXPECT(env.current()->fees().base == XRPAmount{UNIT_TEST_REFERENCE_FEE});
+        BEAST_EXPECT(env.current()->fees().base == env.app().config().FEES.reference_fee);
         BEAST_EXPECT(env.current()->fees().reserve == XRPAmount{200'000'000});
         BEAST_EXPECT(env.current()->fees().increment == XRPAmount{50'000'000});
 

src/test/app/LedgerMaster_test.cpp

@@ -14,8 +14,6 @@ class LedgerMaster_test : public beast::unit_test::suite
         using namespace jtx;
         return envconfig([&](std::unique_ptr<Config> cfg) {
             cfg->NETWORK_ID = networkID;
-            // This test relies on ledger hash so must lock it to fee 10.
-            cfg->FEES.reference_fee = 10;
             return cfg;
         });
     }
@@ -28,7 +26,8 @@ class LedgerMaster_test : public beast::unit_test::suite
         using namespace test::jtx;
         using namespace std::literals;
 
-        test::jtx::Env env{*this, makeNetworkConfig(11111)};
+        // This test relies on ledger hash so must lock it to fee 10.
+        test::jtx::Env env{*this, makeNetworkConfig(11111), XRPAmount(10)};
 
         auto const alice = Account("alice");
         env.fund(XRP(1000), alice);

src/test/app/MPToken_test.cpp

@@ -2060,9 +2060,7 @@ class MPToken_test : public beast::unit_test::suite
 
         Account const alice{"alice"};
 
-        auto cfg = envconfig();
-        cfg->FEES.reference_fee = 10;
-        Env env{*this, std::move(cfg), features};
+        Env env{*this, features, XRPAmount(10)};
         MPTTester mptAlice(env, alice);
 
         mptAlice.create();

src/test/app/Regression_test.cpp

@@ -152,11 +152,13 @@ struct Regression_test : public beast::unit_test::suite
     {
         testcase("Autofilled fee should use the escalated fee");
         using namespace jtx;
-        Env env(*this, envconfig([](std::unique_ptr<Config> cfg) {
-            cfg->section("transaction_queue").set("minimum_txn_in_ledger_standalone", "3");
-            cfg->FEES.reference_fee = 10;
-            return cfg;
-        }));
+        Env env(
+            *this,
+            envconfig([](std::unique_ptr<Config> cfg) {
+                cfg->section("transaction_queue").set("minimum_txn_in_ledger_standalone", "3");
+                return cfg;
+            }),
+            XRPAmount(10));
         Env_ss envs(env);
 
         auto const alice = Account("alice");

src/test/app/TxQ_test.cpp

@@ -1229,8 +1229,7 @@ class TxQPosNegFlows_test : public beast::unit_test::suite
         testcase("tie breaking");
 
         auto cfg = makeConfig({{"minimum_txn_in_ledger_standalone", "4"}});
-        cfg->FEES.reference_fee = 10;
-        Env env(*this, std::move(cfg));
+        Env env(*this, std::move(cfg), XRPAmount(10));
 
         auto alice = Account("alice");
         auto bob = Account("bob");
@@ -2621,8 +2620,7 @@ class TxQPosNegFlows_test : public beast::unit_test::suite
             {{"minimum_txn_in_ledger_standalone", "1"},
              {"ledgers_in_queue", "10"},
              {"maximum_txn_per_account", "11"}});
-        cfg->FEES.reference_fee = 10;
-        Env env(*this, std::move(cfg));
+        Env env(*this, std::move(cfg), XRPAmount(10));
 
         auto const baseFee = env.current()->fees().base.drops();