Merge branch 'develop' into pratik/Add-sanitizers-to-CI-builds

Signed-off-by: Pratik Mankawde <pmankawde@ripple.com>

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

@@ -179,6 +179,24 @@ jobs:
             -C "${BUILD_TYPE}" \
             -j "${PARALLELISM}"
 
+      - name: Run the embedded tests
+        if: ${{ !inputs.build_only }}
+        working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', inputs.build_dir, inputs.build_type) || inputs.build_dir }}
+        shell: bash
+        env:
+          BUILD_NPROC: ${{ steps.nproc.outputs.nproc }}
+        run: |
+          ./rippled --unittest --unittest-jobs "${BUILD_NPROC}"
+
+      - name: Debug failure (Linux)
+        if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }}
+        shell: bash
+        run: |
+          echo "IPv4 local port range:"
+          cat /proc/sys/net/ipv4/ip_local_port_range
+          echo "Netstat:"
+          netstat -an
+
       - name: Prepare coverage report
         if: ${{ !inputs.build_only && env.ENABLED_COVERAGE == 'true' }}
         working-directory: ${{ inputs.build_dir }}
@@ -204,23 +222,3 @@ jobs:
           plugins: noop
           token: ${{ secrets.CODECOV_TOKEN }}
           verbose: true
-
-      - name: Run the embedded tests
-        if: ${{ !inputs.build_only && env.ENABLED_COVERAGE != 'true' }}
-        working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', inputs.build_dir, inputs.build_type) || inputs.build_dir }}
-        shell: bash
-        env:
-          BUILD_NPROC: ${{ steps.nproc.outputs.nproc }}
-          ASAN_OPTIONS: "suppressions=${{ github.workspace }}/external/asan_suppressions.txt"
-          TSAN_OPTIONS: "suppressions=${{ github.workspace }}/external/tsan_suppressions.txt"
-        run: |
-          ./rippled --unittest --unittest-jobs "${BUILD_NPROC}"
-
-      - name: Debug failure (Linux)
-        if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }}
-        shell: bash
-        run: |
-          echo "IPv4 local port range:"
-          cat /proc/sys/net/ipv4/ip_local_port_range
-          echo "Netstat:"
-          netstat -an

BUILD.md

@@ -505,18 +505,18 @@ A coverage report is created when the following steps are completed, in order:
 
 1. `rippled` binary built with instrumentation data, enabled by the `coverage`
    option mentioned above
-2. completed run of unit tests, which populates coverage capture data
+2. completed one or more run of the unit tests, which populates coverage capture data
 3. completed run of the `gcovr` tool (which internally invokes either `gcov` or `llvm-cov`)
    to assemble both instrumentation data and the coverage capture data into a coverage report
 
-The above steps are automated into a single target `coverage`. The instrumented
+The last step of the above is automated into a single target `coverage`. The instrumented
 `rippled` binary can also be used for regular development or testing work, at
 the cost of extra disk space utilization and a small performance hit
-(to store coverage capture). In case of a spurious failure of unit tests, it is
+(to store coverage capture data). Since `rippled` binary is simply a dependency of the
-possible to re-run the `coverage` target without rebuilding the `rippled` binary
+coverage report target, it is possible to re-run the `coverage` target without
-(since it is simply a dependency of the coverage report target). It is also possible
+rebuilding the `rippled` binary. Note, running of the unit tests before the `coverage`
-to select only specific tests for the purpose of the coverage report, by setting
+target is left to the developer. Each such run will append to the coverage data
-the `coverage_test` variable in `cmake`
+collected in the build directory.
 
 The default coverage report format is `html-details`, but the user
 can override it to any of the formats listed in `Builds/CMake/CodeCoverage.cmake`
@@ -525,11 +525,6 @@ to generate more than one format at a time by setting the `coverage_extra_args`
 variable in `cmake`. The specific command line used to run the `gcovr` tool will be
 displayed if the `CODE_COVERAGE_VERBOSE` variable is set.
 
-By default, the code coverage tool runs parallel unit tests with `--unittest-jobs`
-set to the number of available CPU cores. This may cause spurious test
-errors on Apple. Developers can override the number of unit test jobs with
-the `coverage_test_parallelism` variable in `cmake`.
-
 Example use with some cmake variables set:
 
 ```

cmake/CodeCoverage.cmake

@@ -109,6 +109,9 @@
 #     - add a new function add_code_coverage_to_target
 #     - remove some unused code
 #
+# 2025-11-11, Bronek Kozicki
+#     - make EXECUTABLE and EXECUTABLE_ARGS optional
+#
 # USAGE:
 #
 # 1. Copy this file into your cmake modules path.
@@ -317,6 +320,10 @@ function(setup_target_for_coverage_gcovr)
         set(Coverage_FORMAT xml)
     endif()
 
+    if(NOT DEFINED Coverage_EXECUTABLE AND DEFINED Coverage_EXECUTABLE_ARGS)
+        message(FATAL_ERROR "EXECUTABLE_ARGS must not be set if EXECUTABLE is not set")
+    endif()
+
     if("--output" IN_LIST GCOVR_ADDITIONAL_ARGS)
         message(FATAL_ERROR "Unsupported --output option detected in GCOVR_ADDITIONAL_ARGS! Aborting...")
     else()
@@ -398,17 +405,18 @@ function(setup_target_for_coverage_gcovr)
     endforeach()
 
     # Set up commands which will be run to generate coverage data
-    # Run tests
+    # If EXECUTABLE is not set, the user is expected to run the tests manually
+    # before running the coverage target NAME
+    if(DEFINED Coverage_EXECUTABLE)
         set(GCOVR_EXEC_TESTS_CMD
             ${Coverage_EXECUTABLE} ${Coverage_EXECUTABLE_ARGS}
         )
+    endif()
 
     # Create folder
     if(DEFINED GCOVR_CREATE_FOLDER)
         set(GCOVR_FOLDER_CMD
             ${CMAKE_COMMAND} -E make_directory ${GCOVR_CREATE_FOLDER})
-    else()
-        set(GCOVR_FOLDER_CMD echo) # dummy
     endif()
 
     # Running gcovr
@@ -425,11 +433,13 @@ function(setup_target_for_coverage_gcovr)
     if(CODE_COVERAGE_VERBOSE)
         message(STATUS "Executed command report")
 
+        if(NOT "${GCOVR_EXEC_TESTS_CMD}" STREQUAL "")
             message(STATUS "Command to run tests: ")
             string(REPLACE ";" " " GCOVR_EXEC_TESTS_CMD_SPACED "${GCOVR_EXEC_TESTS_CMD}")
             message(STATUS "${GCOVR_EXEC_TESTS_CMD_SPACED}")
+        endif()
 
-        if(NOT GCOVR_FOLDER_CMD STREQUAL "echo")
+        if(NOT "${GCOVR_FOLDER_CMD}" STREQUAL "")
             message(STATUS "Command to create a folder: ")
             string(REPLACE ";" " " GCOVR_FOLDER_CMD_SPACED "${GCOVR_FOLDER_CMD}")
             message(STATUS "${GCOVR_FOLDER_CMD_SPACED}")

cmake/XrplCov.cmake

@@ -11,6 +11,9 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
   return()
 endif()
 
+include(ProcessorCount)
+ProcessorCount(PROCESSOR_COUNT)
+
 include(CodeCoverage)
 
 # The instructions for these commands come from the `CodeCoverage` module,
@@ -26,15 +29,13 @@ list(APPEND GCOVR_ADDITIONAL_ARGS
   --exclude-throw-branches
   --exclude-noncode-lines
   --exclude-unreachable-branches -s
-  -j ${coverage_test_parallelism})
+  -j ${PROCESSOR_COUNT})
 
 setup_target_for_coverage_gcovr(
   NAME coverage
   FORMAT ${coverage_format}
-  EXECUTABLE xrpld
-  EXECUTABLE_ARGS --unittest$<$<BOOL:${coverage_test}>:=${coverage_test}> --unittest-jobs ${coverage_test_parallelism} --quiet --unittest-log
   EXCLUDE "src/test" "src/tests" "include/xrpl/beast/test" "include/xrpl/beast/unit_test" "${CMAKE_BINARY_DIR}/pb-xrpl.libpb"
-  DEPENDENCIES xrpld
+  DEPENDENCIES xrpld xrpl.tests
 )
 
 add_code_coverage_to_target(opts INTERFACE)

cmake/XrplSettings.cmake

@@ -51,17 +51,10 @@ if(is_gcc OR is_clang)
 
   option(coverage "Generates coverage info." OFF)
   option(profile "Add profiling flags" OFF)
-  set(coverage_test_parallelism "${PROCESSOR_COUNT}" CACHE STRING
-    "Unit tests parallelism for the purpose of coverage report.")
   set(coverage_format "html-details" CACHE STRING
     "Output format of the coverage report.")
   set(coverage_extra_args "" CACHE STRING
     "Additional arguments to pass to gcovr.")
-  set(coverage_test "" CACHE STRING
-    "On gcc & clang, the specific unit test(s) to run for coverage. Default is all tests.")
-  if(coverage_test AND NOT coverage)
-    set(coverage ON CACHE BOOL "gcc/clang only" FORCE)
-  endif()
   option(wextra "compile with extra gcc/clang warnings enabled" ON)
 else()
   set(profile OFF CACHE BOOL "gcc/clang only" FORCE)

include/xrpl/protocol/detail/features.macro

@@ -68,7 +68,6 @@ XRPL_FEATURE(TicketBatch,                Supported::yes, VoteBehavior::DefaultYe
 XRPL_FEATURE(NegativeUNL,                Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(RequireFullyCanonicalSig,   Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(DeletableAccounts,          Supported::yes, VoteBehavior::DefaultYes)
-XRPL_FIX    (PayChanRecipientOwnerDir,   Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(MultiSignReserve,           Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(DepositPreauth,             Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(Checks,                     Supported::yes, VoteBehavior::DefaultYes)
@@ -109,9 +108,10 @@ XRPL_RETIRE(fix1623)
 XRPL_RETIRE(fix1781)
 XRPL_RETIRE(fixAmendmentMajorityCalc)
 XRPL_RETIRE(fixCheckThreading)
+XRPL_RETIRE(fixMasterKeyAsRegularKey)
 XRPL_RETIRE(fixNonFungibleTokensV1_2)
 XRPL_RETIRE(fixNFTokenRemint)
-XRPL_RETIRE(fixMasterKeyAsRegularKey)
+XRPL_RETIRE(fixPayChanRecipientOwnerDir)
 XRPL_RETIRE(fixQualityUpperBound)
 XRPL_RETIRE(fixReducedOffersV1)
 XRPL_RETIRE(fixRmSmallIncreasedQOffers)

src/test/app/AccountDelete_test.cpp

@@ -268,12 +268,8 @@ public:
 
         testcase("Owned types");
 
-        // We want to test both...
+        // We want to test PayChannels with the backlink.
-        //  o Old-style PayChannels without a recipient backlink as well as
+        Env env{*this, testable_amendments()};
-        //  o New-styled PayChannels with the backlink.
-        // So we start the test using old-style PayChannels.  Then we pass
-        // the amendment to get new-style PayChannels.
-        Env env{*this, testable_amendments() - fixPayChanRecipientOwnerDir};
         Account const alice("alice");
         Account const becky("becky");
         Account const gw("gw");
@@ -374,16 +370,14 @@ public:
             alice, becky, XRP(57), 4s, env.now() + 2s, alice.pk()));
         env.close();
 
-        // An old-style PayChannel does not add a back link from the
+        // With the PayChannel in place becky and alice should not be
-        // destination.  So with the PayChannel in place becky should be
+        // able to delete her account
-        // able to delete her account, but alice should not.
         auto const beckyBalance{env.balance(becky)};
         env(acctdelete(alice, gw), fee(acctDelFee), ter(tecHAS_OBLIGATIONS));
-        env(acctdelete(becky, gw), fee(acctDelFee));
+        env(acctdelete(becky, gw), fee(acctDelFee), ter(tecHAS_OBLIGATIONS));
-        verifyDeliveredAmount(env, beckyBalance - acctDelFee);
         env.close();
 
-        // Alice cancels her PayChannel which will leave her with only offers
+        // Alice cancels her PayChannel, which will leave her with only offers
         // in her directory.
 
         // Lambda to close a PayChannel.
@@ -401,14 +395,8 @@ public:
         env(payChanClose(alice, alicePayChanKey, alice.pk()));
         env.close();
 
-        // Now enable the amendment so PayChannels add a backlink from the
+        // gw creates a PayChannel with alice as the destination, this should
-        // destination.
+        // prevent alice from deleting her account.
-        env.enableFeature(fixPayChanRecipientOwnerDir);
-        env.close();
-
-        // gw creates a PayChannel with alice as the destination.  With the
-        // amendment passed this should prevent alice from deleting her
-        // account.
         Keylet const gwPayChanKey{keylet::payChan(gw, alice, env.seq(gw))};
 
         env(payChanCreate(gw, alice, XRP(68), 4s, env.now() + 2s, alice.pk()));
@@ -430,84 +418,6 @@ public:
         env.close();
     }
 
-    void
-    testResurrection()
-    {
-        // Create an account with an old-style PayChannel.  Delete the
-        // destination of the PayChannel then resurrect the destination.
-        // The PayChannel should still work.
-        using namespace jtx;
-
-        testcase("Resurrection");
-
-        // We need an old-style PayChannel that doesn't provide a backlink
-        // from the destination.  So don't enable the amendment with that fix.
-        Env env{*this, testable_amendments() - fixPayChanRecipientOwnerDir};
-        Account const alice("alice");
-        Account const becky("becky");
-
-        env.fund(XRP(10000), alice, becky);
-        env.close();
-
-        // Verify that becky's account root is present.
-        Keylet const beckyAcctKey{keylet::account(becky.id())};
-        BEAST_EXPECT(env.closed()->exists(beckyAcctKey));
-
-        using namespace std::chrono_literals;
-        Keylet const payChanKey{keylet::payChan(alice, becky, env.seq(alice))};
-        auto const payChanXRP = XRP(37);
-
-        env(payChanCreate(
-            alice, becky, payChanXRP, 4s, env.now() + 1h, alice.pk()));
-        env.close();
-        BEAST_EXPECT(env.closed()->exists(payChanKey));
-
-        // Close enough ledgers to be able to delete becky's account.
-        incLgrSeqForAccDel(env, becky);
-
-        auto const beckyPreDelBalance{env.balance(becky)};
-
-        auto const acctDelFee{drops(env.current()->fees().increment)};
-        env(acctdelete(becky, alice), fee(acctDelFee));
-        verifyDeliveredAmount(env, beckyPreDelBalance - acctDelFee);
-        env.close();
-
-        // Verify that becky's account root is gone.
-        BEAST_EXPECT(!env.closed()->exists(beckyAcctKey));
-
-        // All it takes is a large enough XRP payment to resurrect
-        // becky's account.  Try too small a payment.
-        env(pay(alice,
-                becky,
-                drops(env.current()->fees().accountReserve(0)) - XRP(1)),
-            ter(tecNO_DST_INSUF_XRP));
-        env.close();
-
-        // Actually resurrect becky's account.
-        env(pay(alice, becky, XRP(10)));
-        env.close();
-
-        // becky's account root should be back.
-        BEAST_EXPECT(env.closed()->exists(beckyAcctKey));
-        BEAST_EXPECT(env.balance(becky) == XRP(10));
-
-        // becky's resurrected account can be the destination of alice's
-        // PayChannel.
-        auto payChanClaim = [&]() {
-            Json::Value jv;
-            jv[jss::TransactionType] = jss::PaymentChannelClaim;
-            jv[jss::Account] = alice.human();
-            jv[sfChannel.jsonName] = to_string(payChanKey.key);
-            jv[sfBalance.jsonName] =
-                payChanXRP.value().getJson(JsonOptions::none);
-            return jv;
-        };
-        env(payChanClaim());
-        env.close();
-
-        BEAST_EXPECT(env.balance(becky) == XRP(10) + payChanXRP);
-    }
-
     void
     testAmendmentEnable()
     {
@@ -1238,7 +1148,6 @@ public:
         testBasics();
         testDirectories();
         testOwnedTypes();
-        testResurrection();
         testAmendmentEnable();
         testTooManyOffers();
         testImplicitlyCreatedTrustline();

src/test/app/PayChan_test.cpp

@@ -1820,36 +1820,6 @@ struct PayChan_test : public beast::unit_test::suite
             return std::distance(ownerDir.begin(), ownerDir.end());
         };
 
-        {
-            // Test without adding the paychan to the recipient's owner
-            // directory
-            Env env(*this, features - fixPayChanRecipientOwnerDir);
-            env.fund(XRP(10000), alice, bob);
-            env(create(alice, bob, XRP(1000), settleDelay, pk));
-            env.close();
-            auto const [chan, chanSle] =
-                channelKeyAndSle(*env.current(), alice, bob);
-            BEAST_EXPECT(inOwnerDir(*env.current(), alice, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), alice) == 1);
-            BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
-            if (features[fixIncludeKeyletFields])
-            {
-                BEAST_EXPECT((*chanSle)[sfSequence] == env.seq(alice) - 1);
-            }
-            else
-            {
-                BEAST_EXPECT(!chanSle->isFieldPresent(sfSequence));
-            }
-            // close the channel
-            env(claim(bob, chan), txflags(tfClose));
-            BEAST_EXPECT(!channelExists(*env.current(), chan));
-            BEAST_EXPECT(!inOwnerDir(*env.current(), alice, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), alice) == 0);
-            BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
-        }
-
         {
             // Test with adding the paychan to the recipient's owner directory
             Env env{*this, features};
@@ -1874,7 +1844,7 @@ struct PayChan_test : public beast::unit_test::suite
         {
             // Test removing paychans created before adding to the recipient's
             // owner directory
-            Env env(*this, features - fixPayChanRecipientOwnerDir);
+            Env env(*this, features);
             env.fund(XRP(10000), alice, bob);
             // create the channel before the amendment activates
             env(create(alice, bob, XRP(1000), settleDelay, pk));
@@ -1883,21 +1853,9 @@ struct PayChan_test : public beast::unit_test::suite
                 channelKeyAndSle(*env.current(), alice, bob);
             BEAST_EXPECT(inOwnerDir(*env.current(), alice, chanSle));
             BEAST_EXPECT(ownerDirCount(*env.current(), alice) == 1);
-            BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
+            BEAST_EXPECT(inOwnerDir(*env.current(), bob, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
+            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 1);
-            env.enableFeature(fixPayChanRecipientOwnerDir);
-            env.close();
-            BEAST_EXPECT(
-                env.current()->rules().enabled(fixPayChanRecipientOwnerDir));
-            // These checks look redundant, but if you don't `close` after the
-            // `create` these checks will fail. I believe this is due to the
-            // create running with one set of amendments initially, then with a
-            // different set with the ledger closes (tho I haven't dug into it)
-            BEAST_EXPECT(inOwnerDir(*env.current(), alice, chanSle));
-            BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
-            BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
 
-            // close the channel after the amendment activates
             env(claim(bob, chan), txflags(tfClose));
             BEAST_EXPECT(!channelExists(*env.current(), chan));
             BEAST_EXPECT(!inOwnerDir(*env.current(), alice, chanSle));
@@ -1939,12 +1897,8 @@ struct PayChan_test : public beast::unit_test::suite
         auto const bob = Account("bob");
         auto const carol = Account("carol");
 
-        for (bool const withOwnerDirFix : {false, true})
         {
-            auto const amd = withOwnerDirFix
+            Env env{*this, features};
-                ? features
-                : features - fixPayChanRecipientOwnerDir;
-            Env env{*this, amd};
             env.fund(XRP(10000), alice, bob, carol);
             env.close();
 
@@ -1959,11 +1913,7 @@ struct PayChan_test : public beast::unit_test::suite
 
             rmAccount(env, alice, carol, tecHAS_OBLIGATIONS);
             // can only remove bob if the channel isn't in their owner direcotry
-            rmAccount(
+            rmAccount(env, bob, carol, TER(tecHAS_OBLIGATIONS));
-                env,
-                bob,
-                carol,
-                withOwnerDirFix ? TER(tecHAS_OBLIGATIONS) : TER(tesSUCCESS));
 
             auto const feeDrops = env.current()->fees().base;
             auto chanBal = channelBalance(*env.current(), chan);
@@ -1978,152 +1928,21 @@ struct PayChan_test : public beast::unit_test::suite
             assert(reqBal <= chanAmt);
 
             // claim should fail if the dst was removed
-            if (withOwnerDirFix)
-            {
             env(claim(alice, chan, reqBal, authAmt));
             env.close();
             BEAST_EXPECT(channelBalance(*env.current(), chan) == reqBal);
             BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
             BEAST_EXPECT(env.balance(bob) == preBob + delta);
             chanBal = reqBal;
-            }
-            else
-            {
-                auto const preAlice = env.balance(alice);
-                env(claim(alice, chan, reqBal, authAmt), ter(tecNO_DST));
-                env.close();
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == chanBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob);
-                BEAST_EXPECT(env.balance(alice) == preAlice - feeDrops);
-            }
 
             // fund should fail if the dst was removed
-            if (withOwnerDirFix)
-            {
             auto const preAlice = env.balance(alice);
             env(fund(alice, chan, XRP(1000)));
             env.close();
-                BEAST_EXPECT(
+            BEAST_EXPECT(env.balance(alice) == preAlice - XRP(1000) - feeDrops);
-                    env.balance(alice) == preAlice - XRP(1000) - feeDrops);
             BEAST_EXPECT(
                 channelAmount(*env.current(), chan) == chanAmt + XRP(1000));
             chanAmt = chanAmt + XRP(1000);
-            }
-            else
-            {
-                auto const preAlice = env.balance(alice);
-                env(fund(alice, chan, XRP(1000)), ter(tecNO_DST));
-                env.close();
-                BEAST_EXPECT(env.balance(alice) == preAlice - feeDrops);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-            }
-
-            {
-                // Owner closes, will close after settleDelay
-                env(claim(alice, chan), txflags(tfClose));
-                env.close();
-                // settle delay hasn't ellapsed. Channels should exist.
-                BEAST_EXPECT(channelExists(*env.current(), chan));
-                auto const closeTime = env.current()->info().parentCloseTime;
-                auto const minExpiration = closeTime + settleDelay;
-                env.close(minExpiration);
-                env(claim(alice, chan), txflags(tfClose));
-                BEAST_EXPECT(!channelExists(*env.current(), chan));
-            }
-        }
-
-        {
-            // test resurrected account
-            Env env{*this, features - fixPayChanRecipientOwnerDir};
-            env.fund(XRP(10000), alice, bob, carol);
-            env.close();
-
-            // Create a channel from alice to bob
-            auto const pk = alice.pk();
-            auto const settleDelay = 100s;
-            auto const chan = channel(alice, bob, env.seq(alice));
-            env(create(alice, bob, XRP(1000), settleDelay, pk));
-            env.close();
-            BEAST_EXPECT(channelBalance(*env.current(), chan) == XRP(0));
-            BEAST_EXPECT(channelAmount(*env.current(), chan) == XRP(1000));
-
-            // Since `fixPayChanRecipientOwnerDir` is not active, can remove bob
-            rmAccount(env, bob, carol);
-            BEAST_EXPECT(!env.closed()->exists(keylet::account(bob.id())));
-
-            auto const feeDrops = env.current()->fees().base;
-            auto chanBal = channelBalance(*env.current(), chan);
-            auto chanAmt = channelAmount(*env.current(), chan);
-            BEAST_EXPECT(chanBal == XRP(0));
-            BEAST_EXPECT(chanAmt == XRP(1000));
-            auto preBob = env.balance(bob);
-            auto const delta = XRP(50);
-            auto reqBal = chanBal + delta;
-            auto authAmt = reqBal + XRP(100);
-            assert(reqBal <= chanAmt);
-
-            {
-                // claim should fail, since bob doesn't exist
-                auto const preAlice = env.balance(alice);
-                env(claim(alice, chan, reqBal, authAmt), ter(tecNO_DST));
-                env.close();
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == chanBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob);
-                BEAST_EXPECT(env.balance(alice) == preAlice - feeDrops);
-            }
-
-            {
-                // fund should fail, sincebob doesn't exist
-                auto const preAlice = env.balance(alice);
-                env(fund(alice, chan, XRP(1000)), ter(tecNO_DST));
-                env.close();
-                BEAST_EXPECT(env.balance(alice) == preAlice - feeDrops);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-            }
-
-            // resurrect bob
-            env(pay(alice, bob, XRP(20)));
-            env.close();
-            BEAST_EXPECT(env.closed()->exists(keylet::account(bob.id())));
-
-            {
-                // alice should be able to claim
-                preBob = env.balance(bob);
-                reqBal = chanBal + delta;
-                authAmt = reqBal + XRP(100);
-                env(claim(alice, chan, reqBal, authAmt));
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == reqBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob + delta);
-                chanBal = reqBal;
-            }
-
-            {
-                // bob should be able to claim
-                preBob = env.balance(bob);
-                reqBal = chanBal + delta;
-                authAmt = reqBal + XRP(100);
-                auto const sig =
-                    signClaimAuth(alice.pk(), alice.sk(), chan, authAmt);
-                env(claim(bob, chan, reqBal, authAmt, Slice(sig), alice.pk()));
-                BEAST_EXPECT(channelBalance(*env.current(), chan) == reqBal);
-                BEAST_EXPECT(channelAmount(*env.current(), chan) == chanAmt);
-                BEAST_EXPECT(env.balance(bob) == preBob + delta - feeDrops);
-                chanBal = reqBal;
-            }
-
-            {
-                // alice should be able to fund
-                auto const preAlice = env.balance(alice);
-                env(fund(alice, chan, XRP(1000)));
-                BEAST_EXPECT(
-                    env.balance(alice) == preAlice - XRP(1000) - feeDrops);
-                BEAST_EXPECT(
-                    channelAmount(*env.current(), chan) == chanAmt + XRP(1000));
-                chanAmt = chanAmt + XRP(1000);
-            }
 
             {
                 // Owner closes, will close after settleDelay

src/tests/libxrpl/CMakeLists.txt

@@ -3,6 +3,9 @@ include(XrplAddTest)
 # Test requirements.
 find_package(doctest REQUIRED)
 
+# Custom target for all tests defined in this file
+add_custom_target(xrpl.tests)
+
 # Common library dependencies for the rest of the tests.
 add_library(xrpl.imports.test INTERFACE)
 target_link_libraries(xrpl.imports.test INTERFACE doctest::doctest xrpl.libxrpl)
@@ -10,13 +13,19 @@ target_link_libraries(xrpl.imports.test INTERFACE doctest::doctest xrpl.libxrpl)
 # One test for each module.
 xrpl_add_test(basics)
 target_link_libraries(xrpl.test.basics PRIVATE xrpl.imports.test)
+add_dependencies(xrpl.tests xrpl.test.basics)
+
 xrpl_add_test(crypto)
 target_link_libraries(xrpl.test.crypto PRIVATE xrpl.imports.test)
+add_dependencies(xrpl.tests xrpl.test.crypto)
+
 xrpl_add_test(json)
 target_link_libraries(xrpl.test.json PRIVATE xrpl.imports.test)
+add_dependencies(xrpl.tests xrpl.test.json)
 
 # Network unit tests are currently not supported on Windows
 if(NOT WIN32)
   xrpl_add_test(net)
   target_link_libraries(xrpl.test.net PRIVATE xrpl.imports.test)
+  add_dependencies(xrpl.tests xrpl.test.net)
 endif()

src/xrpld/app/tx/detail/PayChan.cpp

@@ -116,8 +116,7 @@ closeChannel(
     }
 
     // Remove PayChan from recipient's owner directory, if present.
-    if (auto const page = (*slep)[~sfDestinationNode];
+    if (auto const page = (*slep)[~sfDestinationNode])
-        page && view.rules().enabled(fixPayChanRecipientOwnerDir))
     {
         auto const dst = (*slep)[sfDestination];
         if (!view.dirRemove(keylet::ownerDir(dst), *page, key, true))
@@ -284,7 +283,6 @@ PayChanCreate::doApply()
     }
 
     // Add PayChan to the recipient's owner directory
-    if (ctx_.view().rules().enabled(fixPayChanRecipientOwnerDir))
     {
         auto const page = ctx_.view().dirInsert(
             keylet::ownerDir(dst), payChanKeylet, describeOwnerDir(dst));