diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index fde1e9fb2c..346c5023a5 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.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 diff --git a/BUILD.md b/BUILD.md index 1930c3cefe..e2c72820b4 100644 --- a/BUILD.md +++ b/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 -possible to re-run the `coverage` target without rebuilding the `rippled` binary -(since it is simply a dependency of the coverage report target). It is also possible -to select only specific tests for the purpose of the coverage report, by setting -the `coverage_test` variable in `cmake` +(to store coverage capture data). Since `rippled` binary is simply a dependency of the +coverage report target, it is possible to re-run the `coverage` target without +rebuilding the `rippled` binary. Note, running of the unit tests before the `coverage` +target is left to the developer. Each such run will append to the coverage data +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: ``` diff --git a/cmake/CodeCoverage.cmake b/cmake/CodeCoverage.cmake index c2b66c9cac..e1b44e656d 100644 --- a/cmake/CodeCoverage.cmake +++ b/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 - set(GCOVR_EXEC_TESTS_CMD - ${Coverage_EXECUTABLE} ${Coverage_EXECUTABLE_ARGS} - ) + # 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") - message(STATUS "Command to run tests: ") - string(REPLACE ";" " " GCOVR_EXEC_TESTS_CMD_SPACED "${GCOVR_EXEC_TESTS_CMD}") - message(STATUS "${GCOVR_EXEC_TESTS_CMD_SPACED}") + 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}") diff --git a/cmake/XrplCov.cmake b/cmake/XrplCov.cmake index 288f74c0ef..b212d60b64 100644 --- a/cmake/XrplCov.cmake +++ b/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$<$:=${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) diff --git a/cmake/XrplSettings.cmake b/cmake/XrplSettings.cmake index be339c135e..a16513afc5 100644 --- a/cmake/XrplSettings.cmake +++ b/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) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 53da9eb38d..e282919311 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/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) diff --git a/src/test/app/AccountDelete_test.cpp b/src/test/app/AccountDelete_test.cpp index 9328b4e017..9d5d1cd877 100644 --- a/src/test/app/AccountDelete_test.cpp +++ b/src/test/app/AccountDelete_test.cpp @@ -268,12 +268,8 @@ public: testcase("Owned types"); - // We want to test both... - // o Old-style PayChannels without a recipient backlink as well as - // 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}; + // We want to test PayChannels with the backlink. + Env env{*this, testable_amendments()}; 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 - // destination. So with the PayChannel in place becky should be - // able to delete her account, but alice should not. + // With the PayChannel in place becky and alice should not be + // able to delete her account auto const beckyBalance{env.balance(becky)}; env(acctdelete(alice, gw), fee(acctDelFee), ter(tecHAS_OBLIGATIONS)); - env(acctdelete(becky, gw), fee(acctDelFee)); - verifyDeliveredAmount(env, beckyBalance - acctDelFee); + env(acctdelete(becky, gw), fee(acctDelFee), ter(tecHAS_OBLIGATIONS)); 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 - // destination. - 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. + // gw creates a PayChannel with alice as the destination, 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(); diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 650542f827..1756b8fdbf 100644 --- a/src/test/app/PayChan_test.cpp +++ b/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(ownerDirCount(*env.current(), bob) == 0); - 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); + BEAST_EXPECT(inOwnerDir(*env.current(), bob, chanSle)); + BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 1); - // 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 - ? features - : features - fixPayChanRecipientOwnerDir; - Env env{*this, amd}; + Env env{*this, features}; 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( - env, - bob, - carol, - withOwnerDirFix ? TER(tecHAS_OBLIGATIONS) : TER(tesSUCCESS)); + rmAccount(env, bob, carol, TER(tecHAS_OBLIGATIONS)); 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); - } + 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; // 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( - 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); + auto const preAlice = env.balance(alice); + env(fund(alice, chan, XRP(1000))); 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); - } + 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 diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index 880fdb2948..a2374698d9 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/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() diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index 27b8380f5f..a6d9996b89 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/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]; - page && view.rules().enabled(fixPayChanRecipientOwnerDir)) + if (auto const page = (*slep)[~sfDestinationNode]) { 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));