From 673fb06c7519c9b8c9e319f1699024b88faa3b7c Mon Sep 17 00:00:00 2001 From: Jingchen Date: Wed, 5 Nov 2025 14:56:20 +0000 Subject: [PATCH 01/19] refactor: Retire fixTrustLinesToSelf amendment (#5989) Amendments activated for more than 2 years can be retired. This change retires the fixTrustLinesToSelf amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/rpc/Feature_test.cpp | 3 +- src/xrpld/app/tx/detail/Change.cpp | 85 --------------------- src/xrpld/app/tx/detail/Change.h | 3 - src/xrpld/app/tx/detail/SetTrust.cpp | 40 +--------- 5 files changed, 5 insertions(+), 128 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 92f05f8149..6ac33f3e78 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -62,7 +62,6 @@ XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FIX (TrustLinesToSelf, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) @@ -119,6 +118,7 @@ XRPL_RETIRE(fixReducedOffersV1) XRPL_RETIRE(fixRmSmallIncreasedQOffers) XRPL_RETIRE(fixSTAmountCanonicalize) XRPL_RETIRE(fixTakerDryOfferRemoval) +XRPL_RETIRE(fixTrustLinesToSelf) XRPL_RETIRE(CryptoConditions) XRPL_RETIRE(Escrow) XRPL_RETIRE(EnforceInvariants) diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 0f5cf65f72..1a53f39f60 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -121,7 +121,8 @@ class Feature_test : public beast::unit_test::suite // Test a random sampling of the variables. If any of these get retired // or removed, swap out for any other feature. BEAST_EXPECT( - featureToName(fixTrustLinesToSelf) == "fixTrustLinesToSelf"); + featureToName(fixRemoveNFTokenAutoTrustLine) == + "fixRemoveNFTokenAutoTrustLine"); BEAST_EXPECT(featureToName(featureFlow) == "Flow"); BEAST_EXPECT(featureToName(featureNegativeUNL) == "NegativeUNL"); BEAST_EXPECT( diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index 92b0eb2bb2..4832287c2e 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -147,88 +147,6 @@ Change::preCompute() account_ == beast::zero, "ripple::Change::preCompute : zero account"); } -void -Change::activateTrustLinesToSelfFix() -{ - JLOG(j_.warn()) << "fixTrustLinesToSelf amendment activation code starting"; - - auto removeTrustLineToSelf = [this](Sandbox& sb, uint256 id) { - auto tl = sb.peek(keylet::child(id)); - - if (tl == nullptr) - { - JLOG(j_.warn()) << id << ": Unable to locate trustline"; - return true; - } - - if (tl->getType() != ltRIPPLE_STATE) - { - JLOG(j_.warn()) << id << ": Unexpected type " - << static_cast(tl->getType()); - return true; - } - - auto const& lo = tl->getFieldAmount(sfLowLimit); - auto const& hi = tl->getFieldAmount(sfHighLimit); - - if (lo != hi) - { - JLOG(j_.warn()) << id << ": Trustline doesn't meet requirements"; - return true; - } - - if (auto const page = tl->getFieldU64(sfLowNode); !sb.dirRemove( - keylet::ownerDir(lo.getIssuer()), page, tl->key(), false)) - { - JLOG(j_.error()) << id << ": failed to remove low entry from " - << toBase58(lo.getIssuer()) << ":" << page - << " owner directory"; - return false; - } - - if (auto const page = tl->getFieldU64(sfHighNode); !sb.dirRemove( - keylet::ownerDir(hi.getIssuer()), page, tl->key(), false)) - { - JLOG(j_.error()) << id << ": failed to remove high entry from " - << toBase58(hi.getIssuer()) << ":" << page - << " owner directory"; - return false; - } - - if (tl->getFlags() & lsfLowReserve) - adjustOwnerCount( - sb, sb.peek(keylet::account(lo.getIssuer())), -1, j_); - - if (tl->getFlags() & lsfHighReserve) - adjustOwnerCount( - sb, sb.peek(keylet::account(hi.getIssuer())), -1, j_); - - sb.erase(tl); - - JLOG(j_.warn()) << "Successfully deleted trustline " << id; - - return true; - }; - - using namespace std::literals; - - Sandbox sb(&view()); - - if (removeTrustLineToSelf( - sb, - uint256{ - "2F8F21EFCAFD7ACFB07D5BB04F0D2E18587820C7611305BB674A64EAB0FA71E1"sv}) && - removeTrustLineToSelf( - sb, - uint256{ - "326035D5C0560A9DA8636545DD5A1B0DFCFF63E68D491B5522B767BB00564B1A"sv})) - { - JLOG(j_.warn()) << "fixTrustLinesToSelf amendment activation code " - "executed successfully"; - sb.apply(ctx_.rawView()); - } -} - TER Change::applyAmendment() { @@ -305,9 +223,6 @@ Change::applyAmendment() amendments.push_back(amendment); amendmentObject->setFieldV256(sfAmendments, amendments); - if (amendment == fixTrustLinesToSelf) - activateTrustLinesToSelfFix(); - ctx_.app.getAmendmentTable().enable(amendment); if (!ctx_.app.getAmendmentTable().isSupported(amendment)) diff --git a/src/xrpld/app/tx/detail/Change.h b/src/xrpld/app/tx/detail/Change.h index d71c5baeb5..9ff37b1515 100644 --- a/src/xrpld/app/tx/detail/Change.h +++ b/src/xrpld/app/tx/detail/Change.h @@ -29,9 +29,6 @@ public: preclaim(PreclaimContext const& ctx); private: - void - activateTrustLinesToSelfFix(); - TER applyAmendment(); diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp index 2b10b94374..dcbdbc013f 100644 --- a/src/xrpld/app/tx/detail/SetTrust.cpp +++ b/src/xrpld/app/tx/detail/SetTrust.cpp @@ -195,29 +195,8 @@ SetTrust::preclaim(PreclaimContext const& ctx) auto const currency = saLimitAmount.getCurrency(); auto const uDstAccountID = saLimitAmount.getIssuer(); - if (ctx.view.rules().enabled(fixTrustLinesToSelf)) - { - if (id == uDstAccountID) - return temDST_IS_SRC; - } - else - { - if (id == uDstAccountID) - { - // Prevent trustline to self from being created, - // unless one has somehow already been created - // (in which case doApply will clean it up). - auto const sleDelete = - ctx.view.read(keylet::line(id, uDstAccountID, currency)); - - if (!sleDelete) - { - JLOG(ctx.j.trace()) - << "Malformed transaction: Can not extend credit to self."; - return temDST_IS_SRC; - } - } - } + if (id == uDstAccountID) + return temDST_IS_SRC; // This might be nullptr auto const sleDst = ctx.view.read(keylet::account(uDstAccountID)); @@ -403,21 +382,6 @@ SetTrust::doApply() auto viewJ = ctx_.app.journal("View"); - // Trust lines to self are impossible but because of the old bug there - // are two on 19-02-2022. This code was here to allow those trust lines - // to be deleted. The fixTrustLinesToSelf fix amendment will remove them - // when it enables so this code will no longer be needed. - if (!view().rules().enabled(fixTrustLinesToSelf) && - account_ == uDstAccountID) - { - return trustDelete( - view(), - view().peek(keylet::line(account_, uDstAccountID, currency)), - account_, - uDstAccountID, - viewJ); - } - SLE::pointer sleDst = view().peek(keylet::account(uDstAccountID)); if (!sleDst) From 28a1f909383952c12273ba11dff58d125563bfac Mon Sep 17 00:00:00 2001 From: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> Date: Wed, 5 Nov 2025 18:08:10 -0500 Subject: [PATCH 02/19] fix: domain order book insertion #5998 --- src/xrpld/app/ledger/OrderBookDB.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xrpld/app/ledger/OrderBookDB.cpp b/src/xrpld/app/ledger/OrderBookDB.cpp index da4bc1d28f..1a407d0d3d 100644 --- a/src/xrpld/app/ledger/OrderBookDB.cpp +++ b/src/xrpld/app/ledger/OrderBookDB.cpp @@ -106,7 +106,7 @@ OrderBookDB::update(std::shared_ptr const& ledger) book.domain = (*sle)[~sfDomainID]; if (book.domain) - domainBooks_[{book.in, *book.domain}].insert(book.out); + domainBooks[{book.in, *book.domain}].insert(book.out); else allBooks[book.in].insert(book.out); From 173f9f7bb0f3939e756c3afea8caf3338c97e51c Mon Sep 17 00:00:00 2001 From: Bart Date: Thu, 6 Nov 2025 09:06:45 +0000 Subject: [PATCH 03/19] chore: Removes unnecessary creation of symlink in CMake install file (#6009) --- .github/scripts/rename/cmake.sh | 4 ++++ cmake/XrplInstall.cmake | 7 +------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/scripts/rename/cmake.sh b/.github/scripts/rename/cmake.sh index fba65e88ea..43089b476d 100755 --- a/.github/scripts/rename/cmake.sh +++ b/.github/scripts/rename/cmake.sh @@ -81,5 +81,9 @@ elif ! grep -q '"rippled"' cmake/XrplCore.cmake; then mv cmake.tmp cmake/XrplCore.cmake fi +# Remove the symlink that previously pointed from 'ripple' to 'xrpl' but now is +# no longer needed. +${SED_COMMAND} -z -i -E 's@install\(CODE.+CMAKE_INSTALL_INCLUDEDIR}/xrpl\)\n"\)@install(CODE "set(CMAKE_MODULE_PATH \\"${CMAKE_MODULE_PATH}\\")")@' cmake/XrplInstall.cmake + popd echo "Renaming complete." diff --git a/cmake/XrplInstall.cmake b/cmake/XrplInstall.cmake index b53b03e1cd..371dcdf216 100644 --- a/cmake/XrplInstall.cmake +++ b/cmake/XrplInstall.cmake @@ -37,12 +37,7 @@ install( DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}" ) -install(CODE " - set(CMAKE_MODULE_PATH \"${CMAKE_MODULE_PATH}\") - include(create_symbolic_link) - create_symbolic_link(xrpl \ - \$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}/xrpl) -") +install(CODE "set(CMAKE_MODULE_PATH \"${CMAKE_MODULE_PATH}\")") install (EXPORT XrplExports FILE XrplTargets.cmake From c39f9c561ce75818988e1205ab3f9811e7501a99 Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Fri, 7 Nov 2025 02:51:12 -0800 Subject: [PATCH 04/19] chore: Point xrpld symlink to rippled (#6012) As part of renaming ripple(d) to xrpl(d), the xrpld symlink was made to point to itself instead of to the rippled binary. This change fixes the symlink. --- .github/scripts/rename/cmake.sh | 7 +++++-- cmake/XrplInstall.cmake | 4 +--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/scripts/rename/cmake.sh b/.github/scripts/rename/cmake.sh index 43089b476d..0f88fa5de2 100755 --- a/.github/scripts/rename/cmake.sh +++ b/.github/scripts/rename/cmake.sh @@ -74,16 +74,19 @@ if grep -q '"xrpld"' cmake/XrplCore.cmake; then # The script has been rerun, so just restore the name of the binary. ${SED_COMMAND} -i 's/"xrpld"/"rippled"/' cmake/XrplCore.cmake elif ! grep -q '"rippled"' cmake/XrplCore.cmake; then - ghead -n -1 cmake/XrplCore.cmake > cmake.tmp + ${HEAD_COMMAND} -n -1 cmake/XrplCore.cmake > cmake.tmp echo ' # For the time being, we will keep the name of the binary as it was.' >> cmake.tmp echo ' set_target_properties(xrpld PROPERTIES OUTPUT_NAME "rippled")' >> cmake.tmp tail -1 cmake/XrplCore.cmake >> cmake.tmp mv cmake.tmp cmake/XrplCore.cmake fi +# Restore the symlink from 'xrpld' to 'rippled'. +${SED_COMMAND} -i -E 's@create_symbolic_link\(xrpld@create_symbolic_link(rippled@' cmake/XrplInstall.cmake + # Remove the symlink that previously pointed from 'ripple' to 'xrpl' but now is # no longer needed. -${SED_COMMAND} -z -i -E 's@install\(CODE.+CMAKE_INSTALL_INCLUDEDIR}/xrpl\)\n"\)@install(CODE "set(CMAKE_MODULE_PATH \\"${CMAKE_MODULE_PATH}\\")")@' cmake/XrplInstall.cmake +${SED_COMMAND} -z -i -E 's@install\(CODE.+CMAKE_INSTALL_INCLUDEDIR}/xrpl\)\n"\)\n+@@' cmake/XrplInstall.cmake popd echo "Renaming complete." diff --git a/cmake/XrplInstall.cmake b/cmake/XrplInstall.cmake index 371dcdf216..05ace6eeea 100644 --- a/cmake/XrplInstall.cmake +++ b/cmake/XrplInstall.cmake @@ -37,8 +37,6 @@ install( DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}" ) -install(CODE "set(CMAKE_MODULE_PATH \"${CMAKE_MODULE_PATH}\")") - install (EXPORT XrplExports FILE XrplTargets.cmake NAMESPACE Xrpl:: @@ -69,7 +67,7 @@ if (is_root_project AND TARGET xrpld) install(CODE " set(CMAKE_MODULE_PATH \"${CMAKE_MODULE_PATH}\") include(create_symbolic_link) - create_symbolic_link(xrpld${suffix} \ + create_symbolic_link(rippled${suffix} \ \$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/xrpld${suffix}) ") endif () From 8d2dff2e483e4799bf2f5988e78df115e5a00976 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 10 Nov 2025 15:32:08 +0000 Subject: [PATCH 05/19] chore: Unify build & test, add ctest to coverage (#6013) This change unifies the build and test jobs into a single job, and adds `ctest` to coverage reporting. The mechanics of coverage reporting is slightly complex and most of it is encapsulated in the `coverage` target. The status quo way of preparing coverage reports involves running a single target `cmake --build . --target coverage`, which does three things: * Build the `rippled` binary (via target dependency) * Prepare coverage reports: * Run `./rippled -u` unit tests. * Gather test output and build reports. This makes it awkward to add an additional `ctest` step between build and coverage reporting steps. The better solution is to split `coverage` target into separate build, followed by `ctest`, followed by test generation. Luckily, the `coverage` target has been designed specifically to support such case; it does not need to build `rippled`, it's just a dependency. Similarly it allows additional tests to be run before gathering test outputs; in principle we could even strip it from running tests and run them separately instead. This means we can keep build, `ctest` and generation of coverage reports as separate steps, as long as the state of build directory is fully (including file timestamps, additional coverage files etc.) preserved between the steps. This means that in order to run `ctest` for coverage reporting we need to integrate build and test into a single job, which this change does. --- .github/scripts/strategy-matrix/generate.py | 2 - .../workflows/reusable-build-test-config.yml | 186 +++++++++++++++--- .github/workflows/reusable-build.yml | 154 --------------- .github/workflows/reusable-test.yml | 111 ----------- cmake/XrplAddTest.cmake | 16 -- src/tests/libxrpl/CMakeLists.txt | 8 +- 6 files changed, 169 insertions(+), 308 deletions(-) delete mode 100644 .github/workflows/reusable-build.yml delete mode 100644 .github/workflows/reusable-test.yml diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index 025d553b5e..c762d61ccd 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -138,8 +138,6 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: # Unity on linux/amd64 if f'{os['compiler_name']}-{os['compiler_version']}' == 'gcc-15' and build_type == 'Debug' and '-Dunity=OFF' in cmake_args and architecture['platform'] == 'linux/amd64': cmake_args = f'-Dcoverage=ON -Dcoverage_format=xml -DCODE_COVERAGE_VERBOSE=ON -DCMAKE_C_FLAGS=-O0 -DCMAKE_CXX_FLAGS=-O0 {cmake_args}' - cmake_target = 'coverage' - build_only = True # Generate a unique name for the configuration, e.g. macos-arm64-debug # or debian-bookworm-gcc-12-amd64-release-unity. diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index a59dbda71b..4a8bc71321 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -7,19 +7,23 @@ on: description: "The directory where to build." required: true type: string + build_only: description: 'Whether to only build or to build and test the code ("true", "false").' required: true type: boolean + build_type: description: 'The build type to use ("Debug", "Release").' type: string required: true + cmake_args: description: "Additional arguments to pass to CMake." required: false type: string default: "" + cmake_target: description: "The CMake target to build." type: string @@ -29,6 +33,7 @@ on: description: Runner to run the job on as a JSON string required: true type: string + image: description: "The image to run in (leave empty to run natively)" required: true @@ -51,27 +56,162 @@ on: required: true jobs: - build: - uses: ./.github/workflows/reusable-build.yml - with: - build_dir: ${{ inputs.build_dir }} - build_type: ${{ inputs.build_type }} - cmake_args: ${{ inputs.cmake_args }} - cmake_target: ${{ inputs.cmake_target }} - runs_on: ${{ inputs.runs_on }} - image: ${{ inputs.image }} - config_name: ${{ inputs.config_name }} - nproc_subtract: ${{ inputs.nproc_subtract }} - secrets: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + build-and-test: + name: ${{ inputs.config_name }} + runs-on: ${{ fromJSON(inputs.runs_on) }} + container: ${{ inputs.image != '' && inputs.image || null }} + timeout-minutes: 60 + env: + ENABLED_VOIDSTAR: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }} + ENABLED_COVERAGE: ${{ contains(inputs.cmake_args, '-Dcoverage=ON') }} + steps: + - name: Cleanup workspace + if: ${{ runner.os == 'macOS' }} + uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e - test: - needs: build - uses: ./.github/workflows/reusable-test.yml - with: - run_tests: ${{ !inputs.build_only }} - verify_voidstar: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }} - runs_on: ${{ inputs.runs_on }} - image: ${{ inputs.image }} - config_name: ${{ inputs.config_name }} - nproc_subtract: ${{ inputs.nproc_subtract }} + - name: Checkout repository + uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + + - name: Prepare runner + uses: XRPLF/actions/.github/actions/prepare-runner@99685816bb60a95a66852f212f382580e180df3a + with: + disable_ccache: false + + - name: Print build environment + uses: ./.github/actions/print-env + + - name: Get number of processors + uses: XRPLF/actions/.github/actions/get-nproc@046b1620f6bfd6cd0985dc82c3df02786801fe0a + id: nproc + with: + subtract: ${{ inputs.nproc_subtract }} + + - name: Setup Conan + uses: ./.github/actions/setup-conan + + - name: Build dependencies + uses: ./.github/actions/build-deps + with: + build_dir: ${{ inputs.build_dir }} + build_nproc: ${{ steps.nproc.outputs.nproc }} + build_type: ${{ inputs.build_type }} + # Set the verbosity to "quiet" for Windows to avoid an excessive + # amount of logs. For other OSes, the "verbose" logs are more useful. + log_verbosity: ${{ runner.os == 'Windows' && 'quiet' || 'verbose' }} + + - name: Configure CMake + shell: bash + working-directory: ${{ inputs.build_dir }} + env: + BUILD_TYPE: ${{ inputs.build_type }} + CMAKE_ARGS: ${{ inputs.cmake_args }} + run: | + cmake \ + -G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ + -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ + ${CMAKE_ARGS} \ + .. + + - name: Build the binary + shell: bash + working-directory: ${{ inputs.build_dir }} + env: + BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} + BUILD_TYPE: ${{ inputs.build_type }} + CMAKE_TARGET: ${{ inputs.cmake_target }} + run: | + cmake \ + --build . \ + --config "${BUILD_TYPE}" \ + --parallel "${BUILD_NPROC}" \ + --target "${CMAKE_TARGET}" + + - name: Upload rippled artifact (Linux) + if: ${{ runner.os == 'Linux' }} + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + env: + BUILD_DIR: ${{ inputs.build_dir }} + with: + name: rippled-${{ inputs.config_name }} + path: ${{ env.BUILD_DIR }}/rippled + retention-days: 3 + if-no-files-found: error + + - name: Check linking (Linux) + if: ${{ runner.os == 'Linux' }} + working-directory: ${{ inputs.build_dir }} + shell: bash + run: | + ldd ./rippled + if [ "$(ldd ./rippled | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then + echo 'The binary is statically linked.' + else + echo 'The binary is dynamically linked.' + exit 1 + fi + + - name: Verify presence of instrumentation (Linux) + if: ${{ runner.os == 'Linux' && env.ENABLED_VOIDSTAR == 'true' }} + working-directory: ${{ inputs.build_dir }} + shell: bash + run: | + ./rippled --version | grep libvoidstar + + - name: Run the separate tests + if: ${{ !inputs.build_only }} + working-directory: ${{ inputs.build_dir }} + # Windows locks some of the build files while running tests, and parallel jobs can collide + env: + BUILD_TYPE: ${{ inputs.build_type }} + PARALLELISM: ${{ runner.os == 'Windows' && '1' || steps.nproc.outputs.nproc }} + shell: bash + run: | + ctest \ + --output-on-failure \ + -C "${BUILD_TYPE}" \ + -j "${PARALLELISM}" + + - name: Prepare coverage report + if: ${{ !inputs.build_only && env.ENABLED_COVERAGE == 'true' }} + working-directory: ${{ inputs.build_dir }} + env: + BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} + BUILD_TYPE: ${{ inputs.build_type }} + shell: bash + run: | + cmake \ + --build . \ + --config "${BUILD_TYPE}" \ + --parallel "${BUILD_NPROC}" \ + --target coverage + + - name: Upload coverage report + if: ${{ github.repository_owner == 'XRPLF' && !inputs.build_only && env.ENABLED_COVERAGE == 'true' }} + uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 + with: + disable_search: true + disable_telem: true + fail_ci_if_error: true + files: ${{ inputs.build_dir }}/coverage.xml + 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 }} + 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/.github/workflows/reusable-build.yml b/.github/workflows/reusable-build.yml deleted file mode 100644 index a9d5fd7f7c..0000000000 --- a/.github/workflows/reusable-build.yml +++ /dev/null @@ -1,154 +0,0 @@ -name: Build rippled - -on: - workflow_call: - inputs: - build_dir: - description: "The directory where to build." - required: true - type: string - build_type: - description: 'The build type to use ("Debug", "Release").' - required: true - type: string - cmake_args: - description: "Additional arguments to pass to CMake." - required: true - type: string - cmake_target: - description: "The CMake target to build." - required: true - type: string - - runs_on: - description: Runner to run the job on as a JSON string - required: true - type: string - image: - description: "The image to run in (leave empty to run natively)" - required: true - type: string - - config_name: - description: "The name of the configuration." - required: true - type: string - - nproc_subtract: - description: "The number of processors to subtract when calculating parallelism." - required: true - type: number - - secrets: - CODECOV_TOKEN: - description: "The Codecov token to use for uploading coverage reports." - required: true - -defaults: - run: - shell: bash - -jobs: - build: - name: Build ${{ inputs.config_name }} - runs-on: ${{ fromJSON(inputs.runs_on) }} - container: ${{ inputs.image != '' && inputs.image || null }} - timeout-minutes: 60 - steps: - - name: Cleanup workspace - if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e - - - name: Checkout repository - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 - - - name: Prepare runner - uses: XRPLF/actions/.github/actions/prepare-runner@99685816bb60a95a66852f212f382580e180df3a - with: - disable_ccache: false - - - name: Print build environment - uses: ./.github/actions/print-env - - - name: Get number of processors - uses: XRPLF/actions/.github/actions/get-nproc@046b1620f6bfd6cd0985dc82c3df02786801fe0a - id: nproc - with: - subtract: ${{ inputs.nproc_subtract }} - - - name: Setup Conan - uses: ./.github/actions/setup-conan - - - name: Build dependencies - uses: ./.github/actions/build-deps - with: - build_dir: ${{ inputs.build_dir }} - build_nproc: ${{ steps.nproc.outputs.nproc }} - build_type: ${{ inputs.build_type }} - # Set the verbosity to "quiet" for Windows to avoid an excessive - # amount of logs. For other OSes, the "verbose" logs are more useful. - log_verbosity: ${{ runner.os == 'Windows' && 'quiet' || 'verbose' }} - - - name: Configure CMake - shell: bash - working-directory: ${{ inputs.build_dir }} - env: - BUILD_TYPE: ${{ inputs.build_type }} - CMAKE_ARGS: ${{ inputs.cmake_args }} - run: | - cmake \ - -G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \ - -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ - ${CMAKE_ARGS} \ - .. - - - name: Build the binary - shell: bash - working-directory: ${{ inputs.build_dir }} - env: - BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} - BUILD_TYPE: ${{ inputs.build_type }} - CMAKE_TARGET: ${{ inputs.cmake_target }} - run: | - cmake \ - --build . \ - --config "${BUILD_TYPE}" \ - --parallel ${BUILD_NPROC} \ - --target "${CMAKE_TARGET}" - - - name: Put built binaries in one location - shell: bash - working-directory: ${{ inputs.build_dir }} - env: - BUILD_TYPE_DIR: ${{ runner.os == 'Windows' && inputs.build_type || '' }} - CMAKE_TARGET: ${{ inputs.cmake_target }} - run: | - mkdir -p ./binaries/doctest/ - - cp ./${BUILD_TYPE_DIR}/rippled* ./binaries/ - if [ "${CMAKE_TARGET}" != 'coverage' ]; then - cp ./src/tests/libxrpl/${BUILD_TYPE_DIR}/xrpl.test.* ./binaries/doctest/ - fi - - - name: Upload rippled artifact - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - env: - BUILD_DIR: ${{ inputs.build_dir }} - with: - name: rippled-${{ inputs.config_name }} - path: ${{ env.BUILD_DIR }}/binaries/ - retention-days: 3 - if-no-files-found: error - - - name: Upload coverage report - if: ${{ github.repository_owner == 'XRPLF' && inputs.cmake_target == 'coverage' }} - uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 - with: - disable_search: true - disable_telem: true - fail_ci_if_error: true - files: ${{ inputs.build_dir }}/coverage.xml - plugins: noop - token: ${{ secrets.CODECOV_TOKEN }} - verbose: true diff --git a/.github/workflows/reusable-test.yml b/.github/workflows/reusable-test.yml deleted file mode 100644 index 8d4a4a8d33..0000000000 --- a/.github/workflows/reusable-test.yml +++ /dev/null @@ -1,111 +0,0 @@ -name: Test rippled - -on: - workflow_call: - inputs: - verify_voidstar: - description: "Whether to verify the presence of voidstar instrumentation." - required: true - type: boolean - run_tests: - description: "Whether to run unit tests" - required: true - type: boolean - - runs_on: - description: Runner to run the job on as a JSON string - required: true - type: string - image: - description: "The image to run in (leave empty to run natively)" - required: true - type: string - - config_name: - description: "The name of the configuration." - required: true - type: string - - nproc_subtract: - description: "The number of processors to subtract when calculating parallelism." - required: true - type: number - -jobs: - test: - name: Test ${{ inputs.config_name }} - runs-on: ${{ fromJSON(inputs.runs_on) }} - container: ${{ inputs.image != '' && inputs.image || null }} - timeout-minutes: 30 - steps: - - name: Cleanup workspace - if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e - - - name: Get number of processors - uses: XRPLF/actions/.github/actions/get-nproc@046b1620f6bfd6cd0985dc82c3df02786801fe0a - id: nproc - with: - subtract: ${{ inputs.nproc_subtract }} - - - name: Download rippled artifact - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 - with: - name: rippled-${{ inputs.config_name }} - - - name: Make binary executable (Linux and macOS) - shell: bash - if: ${{ runner.os == 'Linux' || runner.os == 'macOS' }} - run: | - chmod +x ./rippled - - - name: Check linking (Linux) - if: ${{ runner.os == 'Linux' }} - shell: bash - run: | - ldd ./rippled - if [ "$(ldd ./rippled | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then - echo 'The binary is statically linked.' - else - echo 'The binary is dynamically linked.' - exit 1 - fi - - - name: Verifying presence of instrumentation - if: ${{ inputs.verify_voidstar }} - shell: bash - run: | - ./rippled --version | grep libvoidstar - - - name: Run the embedded tests - if: ${{ inputs.run_tests }} - shell: bash - env: - BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} - run: | - ./rippled --unittest --unittest-jobs ${BUILD_NPROC} - - - name: Run the separate tests - if: ${{ inputs.run_tests }} - env: - EXT: ${{ runner.os == 'Windows' && '.exe' || '' }} - shell: bash - run: | - for test_file in ./doctest/*${EXT}; do - echo "Executing $test_file" - chmod +x "$test_file" - if [[ "${{ runner.os }}" == "Windows" && "$test_file" == "./doctest/xrpl.test.net.exe" ]]; then - echo "Skipping $test_file on Windows" - else - "$test_file" - fi - done - - - name: Debug failure (Linux) - if: ${{ failure() && runner.os == 'Linux' && inputs.run_tests }} - shell: bash - run: | - echo "IPv4 local port range:" - cat /proc/sys/net/ipv4/ip_local_port_range - echo "Netstat:" - netstat -an diff --git a/cmake/XrplAddTest.cmake b/cmake/XrplAddTest.cmake index a1dd3847bd..191a25c467 100644 --- a/cmake/XrplAddTest.cmake +++ b/cmake/XrplAddTest.cmake @@ -22,20 +22,4 @@ function(xrpl_add_test name) UNITY_BUILD_BATCH_SIZE 0) # Adjust as needed add_test(NAME ${target} COMMAND ${target}) - set_tests_properties( - ${target} PROPERTIES - FIXTURES_REQUIRED ${target}_fixture - ) - - add_test( - NAME ${target}.build - COMMAND - ${CMAKE_COMMAND} - --build ${CMAKE_BINARY_DIR} - --config $ - --target ${target} - ) - set_tests_properties(${target}.build PROPERTIES - FIXTURES_SETUP ${target}_fixture - ) endfunction() diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index 5bae91c929..880fdb2948 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/src/tests/libxrpl/CMakeLists.txt @@ -14,5 +14,9 @@ xrpl_add_test(crypto) target_link_libraries(xrpl.test.crypto PRIVATE xrpl.imports.test) xrpl_add_test(json) target_link_libraries(xrpl.test.json PRIVATE xrpl.imports.test) -xrpl_add_test(net) -target_link_libraries(xrpl.test.net PRIVATE xrpl.imports.test) + +# 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) +endif() From 12c629a1d2e676848ae185306ab2d28660cfc74f Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 10 Nov 2025 16:03:10 +0000 Subject: [PATCH 06/19] refactor: Retire CheckCashMakesTrustLine amendment (#5974) Amendments activated for more than 2 years can be retired. This change retires the CheckCashMakesTrustLine amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/Check_test.cpp | 105 +++++--------------- src/xrpld/app/tx/detail/CashCheck.cpp | 38 ++----- 3 files changed, 35 insertions(+), 110 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 6ac33f3e78..53da9eb38d 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -63,7 +63,6 @@ XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(TicketBatch, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYes) @@ -119,6 +118,7 @@ XRPL_RETIRE(fixRmSmallIncreasedQOffers) XRPL_RETIRE(fixSTAmountCanonicalize) XRPL_RETIRE(fixTakerDryOfferRemoval) XRPL_RETIRE(fixTrustLinesToSelf) +XRPL_RETIRE(CheckCashMakesTrustLine) XRPL_RETIRE(CryptoConditions) XRPL_RETIRE(Escrow) XRPL_RETIRE(EnforceInvariants) diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index 9fec61da7b..a9534bf5f9 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -679,9 +679,6 @@ class Check_test : public beast::unit_test::suite using namespace test::jtx; - bool const cashCheckMakesTrustLine = - features[featureCheckCashMakesTrustLine]; - Account const gw{"gateway"}; Account const alice{"alice"}; Account const bob{"bob"}; @@ -714,26 +711,10 @@ class Check_test : public beast::unit_test::suite // and fails because he hasn't got a trust line for USD. env(pay(gw, alice, USD(0.5))); env.close(); - if (!cashCheckMakesTrustLine) - { - // If cashing a check automatically creates a trustline then - // this returns tesSUCCESS and the check is removed from the - // ledger which would mess up later tests. - env(check::cash(bob, chkId1, USD(10)), ter(tecNO_LINE)); - env.close(); - } // bob sets up the trust line, but not at a high enough limit. env(trust(bob, USD(9.5))); env.close(); - if (!cashCheckMakesTrustLine) - { - // If cashing a check is allowed to exceed the trust line - // limit then this returns tesSUCCESS and the check is - // removed from the ledger which would mess up later tests. - env(check::cash(bob, chkId1, USD(10)), ter(tecPATH_PARTIAL)); - env.close(); - } // bob sets the trust line limit high enough but asks for more // than the check's SendMax. @@ -808,34 +789,31 @@ class Check_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, alice) == 2); BEAST_EXPECT(ownerCount(env, bob) == 1); - if (cashCheckMakesTrustLine) - { - // Automatic trust lines are enabled. But one aspect of - // automatic trust lines is that they allow the account - // cashing a check to exceed their trust line limit. Show - // that at work. - // - // bob's trust line limit is currently USD(10.5). Show that - // a payment to bob cannot exceed that trust line, but cashing - // a check can. + // Automatic trust lines are enabled. But one aspect of + // automatic trust lines is that they allow the account + // cashing a check to exceed their trust line limit. Show + // that at work. + // + // bob's trust line limit is currently USD(10.5). Show that + // a payment to bob cannot exceed that trust line, but cashing + // a check can. - // Payment of 20 USD fails. - env(pay(gw, bob, USD(20)), ter(tecPATH_PARTIAL)); - env.close(); + // Payment of 20 USD fails. + env(pay(gw, bob, USD(20)), ter(tecPATH_PARTIAL)); + env.close(); - uint256 const chkId20{getCheckIndex(gw, env.seq(gw))}; - env(check::create(gw, bob, USD(20))); - env.close(); + uint256 const chkId20{getCheckIndex(gw, env.seq(gw))}; + env(check::create(gw, bob, USD(20))); + env.close(); - // However cashing a check for 20 USD succeeds. - env(check::cash(bob, chkId20, USD(20))); - env.close(); - env.require(balance(bob, USD(30))); + // However cashing a check for 20 USD succeeds. + env(check::cash(bob, chkId20, USD(20))); + env.close(); + env.require(balance(bob, USD(30))); - // Clean up this most recent experiment so the rest of the - // tests work. - env(pay(bob, gw, USD(20))); - } + // Clean up this most recent experiment so the rest of the + // tests work. + env(pay(bob, gw, USD(20))); // ... so bob cancels alice's remaining check. env(check::cancel(bob, chkId3)); @@ -950,8 +928,7 @@ class Check_test : public beast::unit_test::suite env(check::create(alice, bob, USD(7))); env.close(); - env(check::cash(bob, chkId, USD(7)), - ter(cashCheckMakesTrustLine ? tecNO_AUTH : tecNO_LINE)); + env(check::cash(bob, chkId, USD(7)), ter(tecNO_AUTH)); env.close(); // Now give bob a trustline for USD. bob still can't cash the @@ -966,17 +943,6 @@ class Check_test : public beast::unit_test::suite env(trust(gw, bob["USD"](1)), txflags(tfSetfAuth)); env.close(); - // bob tries to cash the check again but fails because his trust - // limit is too low. - if (!cashCheckMakesTrustLine) - { - // If cashing a check is allowed to exceed the trust line - // limit then this returns tesSUCCESS and the check is - // removed from the ledger which would mess up later tests. - env(check::cash(bob, chkId, USD(7)), ter(tecPATH_PARTIAL)); - env.close(); - } - // Two possible outcomes here depending on whether cashing a // check can build a trust line: // o If it can't build a trust line, then since bob set his @@ -985,7 +951,7 @@ class Check_test : public beast::unit_test::suite // o If it can build a trust line, then the check is allowed to // exceed the trust limit and bob gets the full transfer. env(check::cash(bob, chkId, check::DeliverMin(USD(4)))); - STAmount const bobGot = cashCheckMakesTrustLine ? USD(7) : USD(5); + STAmount const bobGot = USD(7); verifyDeliveredAmount(env, bobGot); env.require(balance(alice, USD(8) - bobGot)); env.require(balance(bob, bobGot)); @@ -1368,23 +1334,6 @@ class Check_test : public beast::unit_test::suite env(pay(gw, alice, USD(20))); env.close(); - // Before bob gets a trustline, have him try to cash a check. - // Should fail. - { - uint256 const chkId{getCheckIndex(alice, env.seq(alice))}; - env(check::create(alice, bob, USD(20))); - env.close(); - - if (!features[featureCheckCashMakesTrustLine]) - { - // If cashing a check automatically creates a trustline then - // this returns tesSUCCESS and the check is removed from the - // ledger which would mess up later tests. - env(check::cash(bob, chkId, USD(20)), ter(tecNO_LINE)); - env.close(); - } - } - // Now set up bob's trustline. env(trust(bob, USD(20))); env.close(); @@ -1984,10 +1933,6 @@ class Check_test : public beast::unit_test::suite { // Explore automatic trust line creation when a check is cashed. // - // This capability is enabled by the featureCheckCashMakesTrustLine - // amendment. So this test executes only when that amendment is - // active. - assert(features[featureCheckCashMakesTrustLine]); testcase("Trust Line Creation"); @@ -2688,11 +2633,9 @@ public: { using namespace test::jtx; auto const sa = testable_amendments(); - testWithFeats(sa - featureCheckCashMakesTrustLine); testWithFeats(sa - disallowIncoming); testWithFeats(sa); - - testTrustLineCreation(sa); // Test with featureCheckCashMakesTrustLine + testTrustLineCreation(sa); } }; diff --git a/src/xrpld/app/tx/detail/CashCheck.cpp b/src/xrpld/app/tx/detail/CashCheck.cpp index a62dd51a63..4010aa0714 100644 --- a/src/xrpld/app/tx/detail/CashCheck.cpp +++ b/src/xrpld/app/tx/detail/CashCheck.cpp @@ -156,17 +156,6 @@ CashCheck::preclaim(PreclaimContext const& ctx) // An issuer can always accept their own currency. if (!value.native() && (value.getIssuer() != dstId)) { - auto const sleTrustLine = - ctx.view.read(keylet::line(dstId, issuerId, currency)); - - if (!sleTrustLine && - !ctx.view.rules().enabled(featureCheckCashMakesTrustLine)) - { - JLOG(ctx.j.warn()) - << "Cannot cash check for IOU without trustline."; - return tecNO_LINE; - } - auto const sleIssuer = ctx.view.read(keylet::account(issuerId)); if (!sleIssuer) { @@ -178,6 +167,9 @@ CashCheck::preclaim(PreclaimContext const& ctx) if (sleIssuer->at(sfFlags) & lsfRequireAuth) { + auto const sleTrustLine = + ctx.view.read(keylet::line(dstId, issuerId, currency)); + if (!sleTrustLine) { // We can only create a trust line if the issuer does not @@ -325,10 +317,7 @@ CashCheck::doApply() Keylet const trustLineKey = keylet::line(truster, trustLineIssue); bool const destLow = issuer > account_; - bool const checkCashMakesTrustLine = - psb.rules().enabled(featureCheckCashMakesTrustLine); - - if (checkCashMakesTrustLine && !psb.exists(trustLineKey)) + if (!psb.exists(trustLineKey)) { // 1. Can the check casher meet the reserve for the trust line? // 2. Create trust line between destination (this) account @@ -401,14 +390,11 @@ CashCheck::doApply() sleTrustLine->at(tweakedLimit) = savedLimit; }); - if (checkCashMakesTrustLine) - { - // Set the trust line limit to the highest possible value - // while flow runs. - STAmount const bigAmount( - trustLineIssue, STAmount::cMaxValue, STAmount::cMaxOffset); - sleTrustLine->at(tweakedLimit) = bigAmount; - } + // Set the trust line limit to the highest possible value + // while flow runs. + STAmount const bigAmount( + trustLineIssue, STAmount::cMaxValue, STAmount::cMaxOffset); + sleTrustLine->at(tweakedLimit) = bigAmount; // Let flow() do the heavy lifting on a check for an IOU. auto const result = flow( @@ -441,15 +427,11 @@ CashCheck::doApply() << "flow did not produce DeliverMin."; return tecPATH_PARTIAL; } - if (!checkCashMakesTrustLine) - // Set the delivered_amount metadata. - ctx_.deliver(result.actualAmountOut); } // Set the delivered amount metadata in all cases, not just // for DeliverMin. - if (checkCashMakesTrustLine) - ctx_.deliver(result.actualAmountOut); + ctx_.deliver(result.actualAmountOut); sleCheck = psb.peek(keylet::check(ctx_.tx[sfCheckID])); } From 3968efb5f1ed28ac8fa21ed1d9f312ee97eeaef3 Mon Sep 17 00:00:00 2001 From: Bart Date: Mon, 10 Nov 2025 11:33:54 -0500 Subject: [PATCH 07/19] chore: Make CMake improvements (#6010) This change removes unused definitions from the CMake files, moves variable definitions from `XrplSanity` to `XrplSettings` where they better belong, and updates the minimum GCC and Clang versions to match what we actually minimally support. --- cmake/CMakeFuncs.cmake | 18 ----------------- cmake/XrplConfig.cmake | 4 +--- cmake/XrplCore.cmake | 5 +---- cmake/XrplSanity.cmake | 30 +++++----------------------- cmake/XrplSettings.cmake | 43 +++++++++++++++++++++++++--------------- 5 files changed, 34 insertions(+), 66 deletions(-) diff --git a/cmake/CMakeFuncs.cmake b/cmake/CMakeFuncs.cmake index a4c66a120d..e5b2a451f4 100644 --- a/cmake/CMakeFuncs.cmake +++ b/cmake/CMakeFuncs.cmake @@ -1,21 +1,3 @@ -macro(group_sources_in source_dir curdir) - file(GLOB children RELATIVE ${source_dir}/${curdir} - ${source_dir}/${curdir}/*) - foreach (child ${children}) - if (IS_DIRECTORY ${source_dir}/${curdir}/${child}) - group_sources_in(${source_dir} ${curdir}/${child}) - else() - string(REPLACE "/" "\\" groupname ${curdir}) - source_group(${groupname} FILES - ${source_dir}/${curdir}/${child}) - endif() - endforeach() -endmacro() - -macro(group_sources curdir) - group_sources_in(${PROJECT_SOURCE_DIR} ${curdir}) -endmacro() - macro (exclude_from_default target_) set_target_properties (${target_} PROPERTIES EXCLUDE_FROM_ALL ON) set_target_properties (${target_} PROPERTIES EXCLUDE_FROM_DEFAULT_BUILD ON) diff --git a/cmake/XrplConfig.cmake b/cmake/XrplConfig.cmake index d13abee4ed..8a739d48a3 100644 --- a/cmake/XrplConfig.cmake +++ b/cmake/XrplConfig.cmake @@ -12,7 +12,7 @@ if (static OR MSVC) else () set (Boost_USE_STATIC_RUNTIME OFF) endif () -find_dependency (Boost 1.70 +find_dependency (Boost COMPONENTS chrono container @@ -52,5 +52,3 @@ if (TARGET ZLIB::ZLIB) set_target_properties(OpenSSL::Crypto PROPERTIES INTERFACE_LINK_LIBRARIES ZLIB::ZLIB) endif () - -include ("${CMAKE_CURRENT_LIST_DIR}/XrplTargets.cmake") diff --git a/cmake/XrplCore.cmake b/cmake/XrplCore.cmake index 164fc8523f..40b5535fc7 100644 --- a/cmake/XrplCore.cmake +++ b/cmake/XrplCore.cmake @@ -73,10 +73,7 @@ include(target_link_modules) # Level 01 add_module(xrpl beast) -target_link_libraries(xrpl.libxrpl.beast PUBLIC - xrpl.imports.main - xrpl.libpb -) +target_link_libraries(xrpl.libxrpl.beast PUBLIC xrpl.imports.main) # Level 02 add_module(xrpl basics) diff --git a/cmake/XrplSanity.cmake b/cmake/XrplSanity.cmake index 5239e5ba48..db983da73d 100644 --- a/cmake/XrplSanity.cmake +++ b/cmake/XrplSanity.cmake @@ -1,5 +1,5 @@ #[===================================================================[ - convenience variables and sanity checks + sanity checks #]===================================================================] get_property(is_multiconfig GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) @@ -16,39 +16,19 @@ if (NOT is_multiconfig) endif () endif () -get_directory_property(has_parent PARENT_DIRECTORY) -if (has_parent) - set (is_root_project OFF) -else () - set (is_root_project ON) -endif () - if ("${CMAKE_CXX_COMPILER_ID}" MATCHES ".*Clang") # both Clang and AppleClang set (is_clang TRUE) if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" AND - CMAKE_CXX_COMPILER_VERSION VERSION_LESS 8.0) - message (FATAL_ERROR "This project requires clang 8 or later") + CMAKE_CXX_COMPILER_VERSION VERSION_LESS 16.0) + message (FATAL_ERROR "This project requires clang 16 or later") endif () - # TODO min AppleClang version check ? elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") set (is_gcc TRUE) - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 8.0) - message (FATAL_ERROR "This project requires GCC 8 or later") + if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 12.0) + message (FATAL_ERROR "This project requires GCC 12 or later") endif () endif () -if (CMAKE_SYSTEM_NAME STREQUAL "Linux") - set (is_linux TRUE) -else () - set (is_linux FALSE) -endif () - -if ("$ENV{CI}" STREQUAL "true" OR "$ENV{CONTINUOUS_INTEGRATION}" STREQUAL "true") - set (is_ci TRUE) -else () - set (is_ci FALSE) -endif () - # check for in-source build and fail if ("${CMAKE_CURRENT_SOURCE_DIR}" STREQUAL "${CMAKE_BINARY_DIR}") message (FATAL_ERROR "Builds (in-source) are not allowed in " diff --git a/cmake/XrplSettings.cmake b/cmake/XrplSettings.cmake index 73534094aa..be339c135e 100644 --- a/cmake/XrplSettings.cmake +++ b/cmake/XrplSettings.cmake @@ -1,10 +1,25 @@ #[===================================================================[ - declare user options/settings + declare options and variables #]===================================================================] -include(ProcessorCount) +if(CMAKE_SYSTEM_NAME STREQUAL "Linux") + set (is_linux TRUE) +else() + set(is_linux FALSE) +endif() -ProcessorCount(PROCESSOR_COUNT) +if("$ENV{CI}" STREQUAL "true" OR "$ENV{CONTINUOUS_INTEGRATION}" STREQUAL "true") + set(is_ci TRUE) +else() + set(is_ci FALSE) +endif() + +get_directory_property(has_parent PARENT_DIRECTORY) +if(has_parent) + set(is_root_project OFF) +else() + set(is_root_project ON) +endif() option(assert "Enables asserts, even in release builds" OFF) @@ -25,10 +40,15 @@ if(unity) endif() set(CMAKE_UNITY_BUILD ON CACHE BOOL "Do a unity build") endif() + if(is_clang AND is_linux) option(voidstar "Enable Antithesis instrumentation." OFF) endif() + if(is_gcc OR is_clang) + include(ProcessorCount) + ProcessorCount(PROCESSOR_COUNT) + option(coverage "Generates coverage info." OFF) option(profile "Add profiling flags" OFF) set(coverage_test_parallelism "${PROCESSOR_COUNT}" CACHE STRING @@ -48,6 +68,7 @@ else() set(coverage OFF CACHE BOOL "gcc/clang only" FORCE) set(wextra OFF CACHE BOOL "gcc/clang only" FORCE) endif() + if(is_linux) option(BUILD_SHARED_LIBS "build shared xrpl libraries" OFF) option(static "link protobuf, openssl, libc++, and boost statically" ON) @@ -64,11 +85,13 @@ else() set(use_gold OFF CACHE BOOL "gold linker, linux only" FORCE) set(use_mold OFF CACHE BOOL "mold linker, linux only" FORCE) endif() + if(is_clang) option(use_lld "enables detection of lld linker" ON) else() set(use_lld OFF CACHE BOOL "try lld linker, clang only" FORCE) endif() + option(jemalloc "Enables jemalloc for heap profiling" OFF) option(werr "treat warnings as errors" OFF) option(local_protobuf @@ -102,16 +125,6 @@ if(san) message(FATAL_ERROR "${san} sanitizer does not seem to be supported by your compiler") endif() endif() -set(container_label "" CACHE STRING "tag to use for package building containers") -option(packages_only - "ONLY generate package building targets. This is special use-case and almost \ - certainly not what you want. Use with caution as you won't be able to build \ - any compiled targets locally." OFF) -option(have_package_container - "Sometimes you already have the tagged container you want to use for package \ - building and you don't want docker to rebuild it. This flag will detach the \ - dependency of the package build from the container build. It's an advanced \ - use case and most likely you should not be touching this flag." OFF) # the remaining options are obscure and rarely used option(beast_no_unit_test_inline @@ -125,15 +138,13 @@ option(boost_show_deprecated "Allow boost to fail on deprecated usage. Only useful if you're trying\ to find deprecated calls." OFF) -option(beast_hashers - "Use local implementations for sha/ripemd hashes (experimental, not recommended)" - OFF) if(WIN32) option(beast_disable_autolink "Disables autolinking of system libraries on WIN32" OFF) else() set(beast_disable_autolink OFF CACHE BOOL "WIN32 only" FORCE) endif() + if(coverage) message(STATUS "coverage build requested - forcing Debug build") set(CMAKE_BUILD_TYPE Debug CACHE STRING "build type" FORCE) From 3b810c305ab64172b3e1020b310bcbaa68b3376c Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 10 Nov 2025 17:33:20 +0000 Subject: [PATCH 08/19] fix: JSON parsing of negative `STNumber` and `STAmount` (#5990) This change fixes JSON parsing of negative `int` input in `STNumber` and `STAmount`. The conversion of JSON to `STNumber` or `STAmount` may trigger a condition where we negate smallest possible `int` value, which is undefined behaviour. We use a temporary storage as `int64_t` to avoid this bug. Note that this only affects RPC, because we do not parse JSON in the protocol layer, and hence no amendment is needed. --- include/xrpl/json/json_value.h | 11 +- src/libxrpl/json/json_value.cpp | 66 ++++++++- src/libxrpl/protocol/STAmount.cpp | 2 +- src/libxrpl/protocol/STNumber.cpp | 2 +- src/test/app/ValidatorSite_test.cpp | 4 +- src/test/protocol/STAmount_test.cpp | 212 ++++++++++++++++++++++++++++ src/test/protocol/STNumber_test.cpp | 25 ++++ src/tests/libxrpl/json/Value.cpp | 193 ++++++++++++++++++------- 8 files changed, 454 insertions(+), 61 deletions(-) diff --git a/include/xrpl/json/json_value.h b/include/xrpl/json/json_value.h index 878122c461..6b460ecd3b 100644 --- a/include/xrpl/json/json_value.h +++ b/include/xrpl/json/json_value.h @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -139,9 +140,9 @@ public: using ArrayIndex = UInt; static Value const null; - static Int const minInt; - static Int const maxInt; - static UInt const maxUInt; + static constexpr Int minInt = std::numeric_limits::min(); + static constexpr Int maxInt = std::numeric_limits::max(); + static constexpr UInt maxUInt = std::numeric_limits::max(); private: class CZString @@ -244,6 +245,10 @@ public: bool asBool() const; + /** Correct absolute value from int or unsigned int */ + UInt + asAbsUInt() const; + // TODO: What is the "empty()" method this docstring mentions? /** isNull() tests to see if this field is null. Don't use this method to test for emptiness: use empty(). */ diff --git a/src/libxrpl/json/json_value.cpp b/src/libxrpl/json/json_value.cpp index f283ba3f25..a9478f51ab 100644 --- a/src/libxrpl/json/json_value.cpp +++ b/src/libxrpl/json/json_value.cpp @@ -14,9 +14,6 @@ namespace Json { Value const Value::null; -Int const Value::minInt = Int(~(UInt(-1) / 2)); -Int const Value::maxInt = Int(UInt(-1) / 2); -UInt const Value::maxUInt = UInt(-1); class DefaultValueAllocator : public ValueAllocator { @@ -550,6 +547,69 @@ Value::asInt() const return 0; // unreachable; } +UInt +Value::asAbsUInt() const +{ + switch (type_) + { + case nullValue: + return 0; + + case intValue: { + // Doing this conversion through int64 avoids overflow error for + // value_.int_ = -1 * 2^31 i.e. numeric_limits::min(). + if (value_.int_ < 0) + return static_cast(value_.int_) * -1; + return value_.int_; + } + + case uintValue: + return value_.uint_; + + case realValue: { + if (value_.real_ < 0) + { + JSON_ASSERT_MESSAGE( + -1 * value_.real_ <= maxUInt, + "Real out of unsigned integer range"); + return UInt(-1 * value_.real_); + } + JSON_ASSERT_MESSAGE( + value_.real_ <= maxUInt, "Real out of unsigned integer range"); + return UInt(value_.real_); + } + + case booleanValue: + return value_.bool_ ? 1 : 0; + + case stringValue: { + char const* const str{value_.string_ ? value_.string_ : ""}; + auto const temp = beast::lexicalCastThrow(str); + if (temp < 0) + { + JSON_ASSERT_MESSAGE( + -1 * temp <= maxUInt, + "String out of unsigned integer range"); + return -1 * temp; + } + JSON_ASSERT_MESSAGE( + temp <= maxUInt, "String out of unsigned integer range"); + return temp; + } + + case arrayValue: + case objectValue: + JSON_ASSERT_MESSAGE(false, "Type is not convertible to int"); + + // LCOV_EXCL_START + default: + UNREACHABLE("Json::Value::asAbsInt : invalid type"); + // LCOV_EXCL_STOP + } + + return 0; // unreachable; +} + Value::UInt Value::asUInt() const { diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 4aac551003..d659ee0d05 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -1087,7 +1087,7 @@ amountFromJson(SField const& name, Json::Value const& v) } else { - parts.mantissa = -value.asInt(); + parts.mantissa = value.asAbsUInt(); parts.negative = true; } } diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index 889dd9ee68..c353f6b795 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -169,7 +169,7 @@ numberFromJson(SField const& field, Json::Value const& value) } else { - parts.mantissa = -value.asInt(); + parts.mantissa = value.asAbsUInt(); parts.negative = true; } } diff --git a/src/test/app/ValidatorSite_test.cpp b/src/test/app/ValidatorSite_test.cpp index e94ca6a009..be8ae8013f 100644 --- a/src/test/app/ValidatorSite_test.cpp +++ b/src/test/app/ValidatorSite_test.cpp @@ -572,7 +572,7 @@ public: false, true, 1, - std::chrono::seconds{Json::Value::maxInt + 1}}}); + std::chrono::seconds{Json::Value::minInt}}}); // force an out-of-range validUntil value on the future list // The first list is accepted. The second fails. The parser // returns the "best" result, so this looks like a success. @@ -608,7 +608,7 @@ public: false, true, 1, - std::chrono::seconds{Json::Value::maxInt + 1}, + std::chrono::seconds{Json::Value::minInt}, std::chrono::seconds{Json::Value::maxInt - 6000}}}); // verify refresh intervals are properly clamped testFetchList( diff --git a/src/test/protocol/STAmount_test.cpp b/src/test/protocol/STAmount_test.cpp index 28e4fe6aac..694e351303 100644 --- a/src/test/protocol/STAmount_test.cpp +++ b/src/test/protocol/STAmount_test.cpp @@ -3,6 +3,7 @@ #include #include #include +#include namespace ripple { @@ -585,6 +586,216 @@ public: #endif } + void + testParseJson() + { + static_assert(!std::is_convertible_v); + + { + STAmount const stnum{sfNumber}; + BEAST_EXPECT(stnum.getSType() == STI_AMOUNT); + BEAST_EXPECT(stnum.getText() == "0"); + BEAST_EXPECT(stnum.isDefault() == true); + BEAST_EXPECT(stnum.value() == Number{0}); + } + + { + BEAST_EXPECT( + amountFromJson(sfNumber, Json::Value(42)) == XRPAmount(42)); + BEAST_EXPECT( + amountFromJson(sfNumber, Json::Value(-42)) == XRPAmount(-42)); + + BEAST_EXPECT( + amountFromJson(sfNumber, Json::UInt(42)) == XRPAmount(42)); + + BEAST_EXPECT(amountFromJson(sfNumber, "-123") == XRPAmount(-123)); + + BEAST_EXPECT(amountFromJson(sfNumber, "123") == XRPAmount(123)); + BEAST_EXPECT(amountFromJson(sfNumber, "-123") == XRPAmount(-123)); + + BEAST_EXPECT(amountFromJson(sfNumber, "3.14e2") == XRPAmount(314)); + BEAST_EXPECT( + amountFromJson(sfNumber, "-3.14e2") == XRPAmount(-314)); + + BEAST_EXPECT(amountFromJson(sfNumber, "0") == XRPAmount(0)); + BEAST_EXPECT(amountFromJson(sfNumber, "-0") == XRPAmount(0)); + + constexpr auto imin = std::numeric_limits::min(); + BEAST_EXPECT(amountFromJson(sfNumber, imin) == XRPAmount(imin)); + BEAST_EXPECT( + amountFromJson(sfNumber, std::to_string(imin)) == + XRPAmount(imin)); + + constexpr auto imax = std::numeric_limits::max(); + BEAST_EXPECT(amountFromJson(sfNumber, imax) == XRPAmount(imax)); + BEAST_EXPECT( + amountFromJson(sfNumber, std::to_string(imax)) == + XRPAmount(imax)); + + constexpr auto umax = std::numeric_limits::max(); + BEAST_EXPECT(amountFromJson(sfNumber, umax) == XRPAmount(umax)); + BEAST_EXPECT( + amountFromJson(sfNumber, std::to_string(umax)) == + XRPAmount(umax)); + + // XRP does not handle fractional part + try + { + auto _ = amountFromJson(sfNumber, "0.0"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = + "XRP and MPT must be specified as integral amount."; + BEAST_EXPECT(e.what() == expected); + } + + // XRP does not handle fractional part + try + { + auto _ = amountFromJson(sfNumber, "1000e-2"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = + "XRP and MPT must be specified as integral amount."; + BEAST_EXPECT(e.what() == expected); + } + + // Obvious non-numbers tested here + try + { + auto _ = amountFromJson(sfNumber, ""); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + + try + { + auto _ = amountFromJson(sfNumber, "e"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'e' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + + try + { + auto _ = amountFromJson(sfNumber, "1e"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'1e' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + + try + { + auto _ = amountFromJson(sfNumber, "e2"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'e2' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + + try + { + auto _ = amountFromJson(sfNumber, Json::Value()); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = + "XRP may not be specified with a null Json value"; + BEAST_EXPECT(e.what() == expected); + } + + try + { + auto _ = amountFromJson( + sfNumber, + "123456789012345678901234567890123456789012345678901234" + "5678" + "901234567890123456789012345678901234567890123456789012" + "3456" + "78901234567890123456789012345678901234567890"); + BEAST_EXPECT(false); + } + catch (std::bad_cast const& e) + { + BEAST_EXPECT(true); + } + + // We do not handle leading zeros + try + { + auto _ = amountFromJson(sfNumber, "001"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'001' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + + try + { + auto _ = amountFromJson(sfNumber, "000.0"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'000.0' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + + // We do not handle dangling dot + try + { + auto _ = amountFromJson(sfNumber, ".1"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'.1' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + + try + { + auto _ = amountFromJson(sfNumber, "1."); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'1.' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + + try + { + auto _ = amountFromJson(sfNumber, "1.e3"); + BEAST_EXPECT(false); + } + catch (std::runtime_error const& e) + { + std::string const expected = "'1.e3' is not a number"; + BEAST_EXPECT(e.what() == expected); + } + } + } + void testConvertXRP() { @@ -1022,6 +1233,7 @@ public: testArithmetic(); testUnderflow(); testRounding(); + testParseJson(); testConvertXRP(); testConvertIOU(); testCanAddXRP(); diff --git a/src/test/protocol/STNumber_test.cpp b/src/test/protocol/STNumber_test.cpp index 927bc674e0..af455c6592 100644 --- a/src/test/protocol/STNumber_test.cpp +++ b/src/test/protocol/STNumber_test.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -126,6 +127,30 @@ struct STNumber_test : public beast::unit_test::suite BEAST_EXPECT( numberFromJson(sfNumber, "-0.000e6") == STNumber(sfNumber, 0)); + constexpr auto imin = std::numeric_limits::min(); + BEAST_EXPECT( + numberFromJson(sfNumber, imin) == + STNumber(sfNumber, Number(imin, 0))); + BEAST_EXPECT( + numberFromJson(sfNumber, std::to_string(imin)) == + STNumber(sfNumber, Number(imin, 0))); + + constexpr auto imax = std::numeric_limits::max(); + BEAST_EXPECT( + numberFromJson(sfNumber, imax) == + STNumber(sfNumber, Number(imax, 0))); + BEAST_EXPECT( + numberFromJson(sfNumber, std::to_string(imax)) == + STNumber(sfNumber, Number(imax, 0))); + + constexpr auto umax = std::numeric_limits::max(); + BEAST_EXPECT( + numberFromJson(sfNumber, umax) == + STNumber(sfNumber, Number(umax, 0))); + BEAST_EXPECT( + numberFromJson(sfNumber, std::to_string(umax)) == + STNumber(sfNumber, Number(umax, 0))); + // Obvious non-numbers tested here try { diff --git a/src/tests/libxrpl/json/Value.cpp b/src/tests/libxrpl/json/Value.cpp index b73e8aadc9..df777c98fc 100644 --- a/src/tests/libxrpl/json/Value.cpp +++ b/src/tests/libxrpl/json/Value.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -15,6 +16,14 @@ namespace ripple { TEST_SUITE_BEGIN("json_value"); +TEST_CASE("limits") +{ + using namespace Json; + static_assert(Value::minInt == Int(~(UInt(-1) / 2))); + static_assert(Value::maxInt == Int(UInt(-1) / 2)); + static_assert(Value::maxUInt == UInt(-1)); +} + TEST_CASE("construct and compare Json::StaticString") { static constexpr char sample[]{"Contents of a Json::StaticString"}; @@ -582,8 +591,6 @@ TEST_CASE("bad json") TEST_CASE("edge cases") { - std::string json; - std::uint32_t max_uint = std::numeric_limits::max(); std::int32_t max_int = std::numeric_limits::max(); std::int32_t min_int = std::numeric_limits::min(); @@ -592,71 +599,145 @@ TEST_CASE("edge cases") std::int32_t a_large_int = max_int - 1978; std::int32_t a_small_int = min_int + 1978; - json = "{\"max_uint\":" + std::to_string(max_uint); - json += ",\"max_int\":" + std::to_string(max_int); - json += ",\"min_int\":" + std::to_string(min_int); - json += ",\"a_uint\":" + std::to_string(a_uint); - json += ",\"a_large_int\":" + std::to_string(a_large_int); - json += ",\"a_small_int\":" + std::to_string(a_small_int); - json += "}"; + { + std::string json = "{\"max_uint\":" + std::to_string(max_uint); + json += ",\"max_int\":" + std::to_string(max_int); + json += ",\"min_int\":" + std::to_string(min_int); + json += ",\"a_uint\":" + std::to_string(a_uint); + json += ",\"a_large_int\":" + std::to_string(a_large_int); + json += ",\"a_small_int\":" + std::to_string(a_small_int); + json += "}"; - Json::Value j1; - Json::Reader r1; + Json::Value j1; + Json::Reader r1; - CHECK(r1.parse(json, j1)); - CHECK(j1["max_uint"].asUInt() == max_uint); - CHECK(j1["max_int"].asInt() == max_int); - CHECK(j1["min_int"].asInt() == min_int); - CHECK(j1["a_uint"].asUInt() == a_uint); - CHECK(j1["a_uint"] > a_large_int); - CHECK(j1["a_uint"] > a_small_int); - CHECK(j1["a_large_int"].asInt() == a_large_int); - CHECK(j1["a_large_int"].asUInt() == a_large_int); - CHECK(j1["a_large_int"] < a_uint); - CHECK(j1["a_small_int"].asInt() == a_small_int); - CHECK(j1["a_small_int"] < a_uint); + CHECK(r1.parse(json, j1)); + CHECK(j1["max_uint"].asUInt() == max_uint); + CHECK(j1["max_uint"].asAbsUInt() == max_uint); + CHECK(j1["max_int"].asInt() == max_int); + CHECK(j1["max_int"].asAbsUInt() == max_int); + CHECK(j1["min_int"].asInt() == min_int); + CHECK( + j1["min_int"].asAbsUInt() == + static_cast(min_int) * -1); + CHECK(j1["a_uint"].asUInt() == a_uint); + CHECK(j1["a_uint"].asAbsUInt() == a_uint); + CHECK(j1["a_uint"] > a_large_int); + CHECK(j1["a_uint"] > a_small_int); + CHECK(j1["a_large_int"].asInt() == a_large_int); + CHECK(j1["a_large_int"].asAbsUInt() == a_large_int); + CHECK(j1["a_large_int"].asUInt() == a_large_int); + CHECK(j1["a_large_int"] < a_uint); + CHECK(j1["a_small_int"].asInt() == a_small_int); + CHECK( + j1["a_small_int"].asAbsUInt() == + static_cast(a_small_int) * -1); + CHECK(j1["a_small_int"] < a_uint); + } - json = "{\"overflow\":"; - json += std::to_string(std::uint64_t(max_uint) + 1); - json += "}"; + std::uint64_t overflow = std::uint64_t(max_uint) + 1; + { + std::string json = "{\"overflow\":"; + json += std::to_string(overflow); + json += "}"; - Json::Value j2; - Json::Reader r2; + Json::Value j2; + Json::Reader r2; - CHECK(!r2.parse(json, j2)); + CHECK(!r2.parse(json, j2)); + } - json = "{\"underflow\":"; - json += std::to_string(std::int64_t(min_int) - 1); - json += "}"; + std::int64_t underflow = std::int64_t(min_int) - 1; + { + std::string json = "{\"underflow\":"; + json += std::to_string(underflow); + json += "}"; - Json::Value j3; - Json::Reader r3; + Json::Value j3; + Json::Reader r3; - CHECK(!r3.parse(json, j3)); + CHECK(!r3.parse(json, j3)); + } - Json::Value intString{"4294967296"}; - CHECK_THROWS_AS(intString.asUInt(), beast::BadLexicalCast); + { + Json::Value intString{std::to_string(overflow)}; + CHECK_THROWS_AS(intString.asUInt(), beast::BadLexicalCast); + CHECK_THROWS_AS(intString.asAbsUInt(), Json::error); - intString = "4294967295"; - CHECK(intString.asUInt() == 4294967295u); + intString = "4294967295"; + CHECK(intString.asUInt() == 4294967295u); + CHECK(intString.asAbsUInt() == 4294967295u); - intString = "0"; - CHECK(intString.asUInt() == 0); + intString = "0"; + CHECK(intString.asUInt() == 0); + CHECK(intString.asAbsUInt() == 0); - intString = "-1"; - CHECK_THROWS_AS(intString.asUInt(), beast::BadLexicalCast); + intString = "-1"; + CHECK_THROWS_AS(intString.asUInt(), beast::BadLexicalCast); + CHECK(intString.asAbsUInt() == 1); - intString = "2147483648"; - CHECK_THROWS_AS(intString.asInt(), beast::BadLexicalCast); + intString = "-4294967295"; + CHECK(intString.asAbsUInt() == 4294967295); - intString = "2147483647"; - CHECK(intString.asInt() == 2147483647); + intString = "-4294967296"; + CHECK_THROWS_AS(intString.asAbsUInt(), Json::error); - intString = "-2147483648"; - CHECK(intString.asInt() == -2147483648LL); // MSVC wants the LL + intString = "2147483648"; + CHECK_THROWS_AS(intString.asInt(), beast::BadLexicalCast); + CHECK(intString.asAbsUInt() == 2147483648); - intString = "-2147483649"; - CHECK_THROWS_AS(intString.asInt(), beast::BadLexicalCast); + intString = "2147483647"; + CHECK(intString.asInt() == 2147483647); + CHECK(intString.asAbsUInt() == 2147483647); + + intString = "-2147483648"; + CHECK(intString.asInt() == -2147483648LL); // MSVC wants the LL + CHECK(intString.asAbsUInt() == 2147483648LL); + + intString = "-2147483649"; + CHECK_THROWS_AS(intString.asInt(), beast::BadLexicalCast); + CHECK(intString.asAbsUInt() == 2147483649); + } + + { + Json::Value intReal{4294967297.0}; + CHECK_THROWS_AS(intReal.asUInt(), Json::error); + CHECK_THROWS_AS(intReal.asAbsUInt(), Json::error); + + intReal = 4294967295.0; + CHECK(intReal.asUInt() == 4294967295u); + CHECK(intReal.asAbsUInt() == 4294967295u); + + intReal = 0.0; + CHECK(intReal.asUInt() == 0); + CHECK(intReal.asAbsUInt() == 0); + + intReal = -1.0; + CHECK_THROWS_AS(intReal.asUInt(), Json::error); + CHECK(intReal.asAbsUInt() == 1); + + intReal = -4294967295.0; + CHECK(intReal.asAbsUInt() == 4294967295); + + intReal = -4294967296.0; + CHECK_THROWS_AS(intReal.asAbsUInt(), Json::error); + + intReal = 2147483648.0; + CHECK_THROWS_AS(intReal.asInt(), Json::error); + CHECK(intReal.asAbsUInt() == 2147483648); + + intReal = 2147483647.0; + CHECK(intReal.asInt() == 2147483647); + CHECK(intReal.asAbsUInt() == 2147483647); + + intReal = -2147483648.0; + CHECK(intReal.asInt() == -2147483648LL); // MSVC wants the LL + CHECK(intReal.asAbsUInt() == 2147483648LL); + + intReal = -2147483649.0; + CHECK_THROWS_AS(intReal.asInt(), Json::error); + CHECK(intReal.asAbsUInt() == 2147483649); + } } TEST_CASE("copy") @@ -793,6 +874,7 @@ TEST_CASE("conversions") CHECK(val.asString() == ""); CHECK(val.asInt() == 0); CHECK(val.asUInt() == 0); + CHECK(val.asAbsUInt() == 0); CHECK(val.asDouble() == 0.0); CHECK(val.asBool() == false); @@ -813,6 +895,7 @@ TEST_CASE("conversions") CHECK(val.asString() == "-1234"); CHECK(val.asInt() == -1234); CHECK_THROWS_AS(val.asUInt(), Json::error); + CHECK(val.asAbsUInt() == 1234u); CHECK(val.asDouble() == -1234.0); CHECK(val.asBool() == true); @@ -833,6 +916,7 @@ TEST_CASE("conversions") CHECK(val.asString() == "1234"); CHECK(val.asInt() == 1234); CHECK(val.asUInt() == 1234u); + CHECK(val.asAbsUInt() == 1234u); CHECK(val.asDouble() == 1234.0); CHECK(val.asBool() == true); @@ -853,6 +937,7 @@ TEST_CASE("conversions") CHECK(std::regex_match(val.asString(), std::regex("^2\\.0*$"))); CHECK(val.asInt() == 2); CHECK(val.asUInt() == 2u); + CHECK(val.asAbsUInt() == 2u); CHECK(val.asDouble() == 2.0); CHECK(val.asBool() == true); @@ -873,6 +958,7 @@ TEST_CASE("conversions") CHECK(val.asString() == "54321"); CHECK(val.asInt() == 54321); CHECK(val.asUInt() == 54321u); + CHECK(val.asAbsUInt() == 54321); CHECK_THROWS_AS(val.asDouble(), Json::error); CHECK(val.asBool() == true); @@ -893,6 +979,7 @@ TEST_CASE("conversions") CHECK(val.asString() == ""); CHECK_THROWS_AS(val.asInt(), std::exception); CHECK_THROWS_AS(val.asUInt(), std::exception); + CHECK_THROWS_AS(val.asAbsUInt(), std::exception); CHECK_THROWS_AS(val.asDouble(), std::exception); CHECK(val.asBool() == false); @@ -913,6 +1000,7 @@ TEST_CASE("conversions") CHECK(val.asString() == "false"); CHECK(val.asInt() == 0); CHECK(val.asUInt() == 0); + CHECK(val.asAbsUInt() == 0); CHECK(val.asDouble() == 0.0); CHECK(val.asBool() == false); @@ -933,6 +1021,7 @@ TEST_CASE("conversions") CHECK(val.asString() == "true"); CHECK(val.asInt() == 1); CHECK(val.asUInt() == 1); + CHECK(val.asAbsUInt() == 1); CHECK(val.asDouble() == 1.0); CHECK(val.asBool() == true); @@ -953,6 +1042,7 @@ TEST_CASE("conversions") CHECK_THROWS_AS(val.asString(), Json::error); CHECK_THROWS_AS(val.asInt(), Json::error); CHECK_THROWS_AS(val.asUInt(), Json::error); + CHECK_THROWS_AS(val.asAbsUInt(), Json::error); CHECK_THROWS_AS(val.asDouble(), Json::error); CHECK(val.asBool() == false); // empty or not @@ -973,6 +1063,7 @@ TEST_CASE("conversions") CHECK_THROWS_AS(val.asString(), Json::error); CHECK_THROWS_AS(val.asInt(), Json::error); CHECK_THROWS_AS(val.asUInt(), Json::error); + CHECK_THROWS_AS(val.asAbsUInt(), Json::error); CHECK_THROWS_AS(val.asDouble(), Json::error); CHECK(val.asBool() == false); // empty or not From 098eadca0af3bdf543dd83c642ec8264cc05ba6a Mon Sep 17 00:00:00 2001 From: Bart Date: Mon, 10 Nov 2025 13:02:35 -0500 Subject: [PATCH 09/19] refactor: Update RocksDB, SQLite, Doctest (#6015) --- conan.lock | 15 +++++++++------ conanfile.py | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/conan.lock b/conan.lock index cb25777423..28bf67b1be 100644 --- a/conan.lock +++ b/conan.lock @@ -6,10 +6,10 @@ "sqlite3/3.49.1#8631739a4c9b93bd3d6b753bac548a63%1756234266.869", "soci/4.0.3#a9f8d773cd33e356b5879a4b0564f287%1756234262.318", "snappy/1.1.10#968fef506ff261592ec30c574d4a7809%1756234314.246", - "rocksdb/10.0.1#85537f46e538974d67da0c3977de48ac%1756234304.347", + "rocksdb/10.5.1#4a197eca381a3e5ae8adf8cffa5aacd0%1759820024.194", "re2/20230301#dfd6e2bf050eb90ddd8729cfb4c844a4%1756234257.976", "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614", - "openssl/3.5.4#a1d5835cc6ed5c5b8f3cd5b9b5d24205%1759746684.671", + "openssl/3.5.4#a1d5835cc6ed5c5b8f3cd5b9b5d24205%1760106486.594", "nudb/2.0.9#c62cfd501e57055a7e0d8ee3d5e5427d%1756234237.107", "lz4/1.10.0#59fc63cac7f10fbe8e05c7e62c2f3504%1756234228.999", "libiconv/1.17#1e65319e945f2d31941a9d28cc13c058%1756223727.64", @@ -17,7 +17,7 @@ "libarchive/3.8.1#5cf685686322e906cb42706ab7e099a8%1756234256.696", "jemalloc/5.3.0#e951da9cf599e956cebc117880d2d9f8%1729241615.244", "grpc/1.50.1#02291451d1e17200293a409410d1c4e1%1756234248.958", - "doctest/2.4.11#a4211dfc329a16ba9f280f9574025659%1756234220.819", + "doctest/2.4.12#eb9fb352fb2fdfc8abb17ec270945165%1749889324.069", "date/3.0.4#f74bbba5a08fa388256688743136cb6f%1756234217.493", "c-ares/1.34.5#b78b91e7cfb1f11ce777a285bbf169c6%1756234217.915", "bzip2/1.0.8#00b4a4658791c1f06914e087f0e792f5%1756234261.716", @@ -30,7 +30,7 @@ "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614", "nasm/2.16.01#31e26f2ee3c4346ecd347911bd126904%1756234232.901", "msys2/cci.latest#5b73b10144f73cc5bfe0572ed9be39e1%1751977009.857", - "m4/1.4.19#b38ced39a01e31fef5435bc634461fd2%1700758725.451", + "m4/1.4.19#f119296e5c4772b3bb7ab060ae8f417b%1760707875.678", "cmake/3.31.8#dde3bde00bb843687e55aea5afa0e220%1756234232.89", "b2/5.3.3#107c15377719889654eb9a162a673975%1756234226.28", "automake/1.16.5#b91b7c384c3deaa9d535be02da14d04f%1755524470.56", @@ -48,9 +48,12 @@ "boost/1.83.0": [ "boost/1.88.0" ], - "sqlite3/3.44.2": [ + "sqlite3/[>=3.44 <4]": [ "sqlite3/3.49.1" + ], + "lz4/[>=1.9.4 <2]": [ + "lz4/1.10.0#59fc63cac7f10fbe8e05c7e62c2f3504" ] }, "config_requires": [] -} \ No newline at end of file +} diff --git a/conanfile.py b/conanfile.py index 7f8ab24fbd..41ec5d35f3 100644 --- a/conanfile.py +++ b/conanfile.py @@ -33,7 +33,7 @@ class Xrpl(ConanFile): ] test_requires = [ - 'doctest/2.4.11', + 'doctest/2.4.12', ] tool_requires = [ @@ -114,7 +114,7 @@ class Xrpl(ConanFile): if self.options.jemalloc: self.requires('jemalloc/5.3.0') if self.options.rocksdb: - self.requires('rocksdb/10.0.1') + self.requires('rocksdb/10.5.1') self.requires('xxhash/0.8.3', **transitive_headers_opt) exports_sources = ( From 33309480d48b85fd233ee686ab05dde93aa8b52b Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Mon, 10 Nov 2025 21:23:40 +0000 Subject: [PATCH 10/19] ci: Update Conan to 2.22.2 (#6019) This updates the CI image hashes after following change: https://github.com/XRPLF/ci/pull/81. And, since we use latest Conan, we can have `conan.lock` with a newline at the end, and we don't need to exclude it from `pre-commit` hooks any longer. --- .github/scripts/strategy-matrix/linux.json | 48 +++++++++++----------- .pre-commit-config.yaml | 3 +- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 85a78c96dc..a8fe90e6ac 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -15,168 +15,168 @@ "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "15", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "16", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "17", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "18", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "19", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "20", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "rhel", "distro_version": "8", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "rhel", "distro_version": "8", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "rhel", "distro_version": "10", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "rhel", "distro_version": "10", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "ubuntu", "distro_version": "jammy", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "16", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "17", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "18", - "image_sha": "97ba375" + "image_sha": "e1782cd" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "19", - "image_sha": "97ba375" + "image_sha": "e1782cd" } ], "build_type": ["Debug", "Release"], diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 85568a8b2e..a032fee75e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,6 +34,5 @@ repos: exclude: | (?x)^( external/.*| - .github/scripts/levelization/results/.*\.txt| - conan\.lock + .github/scripts/levelization/results/.*\.txt )$ From 9b332d88c1197a8720d0610b1a67399a3b097377 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde Date: Tue, 11 Nov 2025 14:07:45 +0000 Subject: [PATCH 11/19] refactor: Retire PayChanRecipientOwnerDir amendment (#5946) Amendments activated for more than 2 years can be retired. This change retires the PayChanRecipientOwnerDir amendment. --- include/xrpl/protocol/detail/features.macro | 4 +- src/test/app/AccountDelete_test.cpp | 107 +--------- src/test/app/PayChan_test.cpp | 215 ++------------------ src/xrpld/app/tx/detail/PayChan.cpp | 4 +- 4 files changed, 28 insertions(+), 302 deletions(-) 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/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)); From 03704f712b12848d5711881d675b15cb61b6514e Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 11 Nov 2025 14:55:16 +0000 Subject: [PATCH 12/19] chore: Move running of unit tests out of coverage target (#6018) This change makes the progress of unit tests visible and also gives more flexibility when running them. --- .../workflows/reusable-build-test-config.yml | 36 +++++++++---------- BUILD.md | 19 ++++------ cmake/CodeCoverage.cmake | 30 ++++++++++------ cmake/XrplCov.cmake | 9 ++--- cmake/XrplSettings.cmake | 7 ---- src/tests/libxrpl/CMakeLists.txt | 9 +++++ 6 files changed, 59 insertions(+), 51 deletions(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 4a8bc71321..79d21a870a 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -172,6 +172,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 }} @@ -197,21 +215,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 }} - 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 f13204ea4c..39570edbd3 100644 --- a/BUILD.md +++ b/BUILD.md @@ -495,18 +495,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` @@ -515,11 +515,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/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() From ff18cfef9626aa1cd7f70b3f6d8e603ff4e2360e Mon Sep 17 00:00:00 2001 From: Jingchen Date: Tue, 11 Nov 2025 15:21:07 +0000 Subject: [PATCH 13/19] refactor: Retire DepositPreAuth and DepositAuth amendments (#5978) Amendments activated for more than 2 years can be retired. This change retires the fDepositPreAuth and DepositAuth amendments. --- include/xrpl/protocol/detail/features.macro | 4 +- .../xrpl/protocol/detail/transactions.macro | 2 +- src/test/app/AMMExtended_test.cpp | 14 +- src/test/app/Delegate_test.cpp | 1 - src/test/app/DepositAuth_test.cpp | 242 ++++++------------ src/test/app/Escrow_test.cpp | 10 - src/test/app/Offer_test.cpp | 23 +- src/test/app/PayChan_test.cpp | 23 -- src/test/app/TxQ_test.cpp | 22 +- src/test/rpc/AccountSet_test.cpp | 16 +- src/test/rpc/Feature_test.cpp | 3 +- src/xrpld/app/tx/detail/CreateOffer.cpp | 21 +- src/xrpld/app/tx/detail/DeleteAccount.cpp | 6 +- src/xrpld/app/tx/detail/Escrow.cpp | 17 +- src/xrpld/app/tx/detail/PayChan.cpp | 24 +- src/xrpld/app/tx/detail/Payment.cpp | 110 ++++---- src/xrpld/app/tx/detail/SetAccount.cpp | 19 +- 17 files changed, 168 insertions(+), 389 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index e282919311..076307ff36 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -69,9 +69,7 @@ XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYe XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(DeletableAccounts, 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) -XRPL_FEATURE(DepositAuth, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes) // The following amendments are obsolete, but must remain supported @@ -120,6 +118,8 @@ XRPL_RETIRE(fixTakerDryOfferRemoval) XRPL_RETIRE(fixTrustLinesToSelf) XRPL_RETIRE(CheckCashMakesTrustLine) XRPL_RETIRE(CryptoConditions) +XRPL_RETIRE(DepositAuth) +XRPL_RETIRE(DepositPreauth) XRPL_RETIRE(Escrow) XRPL_RETIRE(EnforceInvariants) XRPL_RETIRE(FeeEscalation) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 1ddc5831b4..fd651fb94e 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -268,7 +268,7 @@ TRANSACTION(ttCHECK_CANCEL, 18, CheckCancel, #endif TRANSACTION(ttDEPOSIT_PREAUTH, 19, DepositPreauth, Delegation::delegatable, - featureDepositPreauth, + uint256{}, noPriv, ({ {sfAuthorize, soeOPTIONAL}, diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 2cfd690183..6b09ea0ce5 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -3039,8 +3039,6 @@ private: using namespace jtx; Account const becky{"becky"}; - bool const supportsPreauth = {features[featureDepositPreauth]}; - // The initial implementation of DepositAuth had a bug where an // account with the DepositAuth flag set could not make a payment // to itself. That bug was fixed in the DepositPreauth amendment. @@ -3068,15 +3066,11 @@ private: env(fset(becky, asfDepositAuth)); env.close(); - // becky pays herself again. Whether it succeeds depends on - // whether featureDepositPreauth is enabled. - TER const expect{ - supportsPreauth ? TER{tesSUCCESS} : TER{tecNO_PERMISSION}}; - + // becky pays herself again. env(pay(becky, becky, USD(10)), path(~USD), sendmax(XRP(10)), - ter(expect)); + ter(tesSUCCESS)); env.close(); } @@ -3784,9 +3778,7 @@ private: void testDepositAuth() { - auto const supported{jtx::testable_amendments()}; - testPayment(supported - featureDepositPreauth); - testPayment(supported); + testPayment(jtx::testable_amendments()); testPayIOU(); } diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index bec720a556..c0fd4b1f3b 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -1707,7 +1707,6 @@ class Delegate_test : public beast::unit_test::suite {"CheckCreate", featureChecks}, {"CheckCash", featureChecks}, {"CheckCancel", featureChecks}, - {"DepositPreauth", featureDepositPreauth}, {"Clawback", featureClawback}, {"AMMClawback", featureAMMClawback}, {"AMMCreate", featureAMM}, diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index 8d12a31088..54d5dd6254 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -33,21 +33,6 @@ struct DepositAuth_test : public beast::unit_test::suite Account const alice{"alice"}; { - // featureDepositAuth is disabled. - Env env(*this, testable_amendments() - featureDepositAuth); - env.fund(XRP(10000), alice); - - // Note that, to support old behavior, invalid flags are ignored. - env(fset(alice, asfDepositAuth)); - env.close(); - BEAST_EXPECT(!hasDepositAuth(env, alice)); - - env(fclear(alice, asfDepositAuth)); - env.close(); - BEAST_EXPECT(!hasDepositAuth(env, alice)); - } - { - // featureDepositAuth is enabled. Env env(*this); env.fund(XRP(10000), alice); @@ -281,8 +266,6 @@ struct DepositAuth_test : public beast::unit_test::suite bool noRipplePrev, bool noRippleNext, bool withDepositAuth) { - assert(!withDepositAuth || features[featureDepositAuth]); - Env env(*this, features); env.fund(XRP(10000), gw1, alice, bob); @@ -305,8 +288,6 @@ struct DepositAuth_test : public beast::unit_test::suite bool noRipplePrev, bool noRippleNext, bool withDepositAuth) { - assert(!withDepositAuth || features[featureDepositAuth]); - Env env(*this, features); env.fund(XRP(10000), gw1, gw2, alice); @@ -333,30 +314,16 @@ struct DepositAuth_test : public beast::unit_test::suite auto const noRippleNext = i & 0x2; auto const withDepositAuth = i & 0x4; testIssuer( - testable_amendments() | featureDepositAuth, + testable_amendments(), noRipplePrev, noRippleNext, withDepositAuth); - if (!withDepositAuth) - testIssuer( - testable_amendments() - featureDepositAuth, - noRipplePrev, - noRippleNext, - withDepositAuth); - testNonIssuer( - testable_amendments() | featureDepositAuth, + testable_amendments(), noRipplePrev, noRippleNext, withDepositAuth); - - if (!withDepositAuth) - testNonIssuer( - testable_amendments() - featureDepositAuth, - noRipplePrev, - noRippleNext, - withDepositAuth); } } @@ -400,26 +367,6 @@ struct DepositPreauth_test : public beast::unit_test::suite Account const alice{"alice"}; Account const becky{"becky"}; { - // featureDepositPreauth is disabled. - Env env(*this, testable_amendments() - featureDepositPreauth); - env.fund(XRP(10000), alice, becky); - env.close(); - - // Should not be able to add a DepositPreauth to alice. - env(deposit::auth(alice, becky), ter(temDISABLED)); - env.close(); - env.require(owners(alice, 0)); - env.require(owners(becky, 0)); - - // Should not be able to remove a DepositPreauth from alice. - env(deposit::unauth(alice, becky), ter(temDISABLED)); - env.close(); - env.require(owners(alice, 0)); - env.require(owners(becky, 0)); - } - { - // featureDepositPreauth is enabled. The valid case is really - // simple: // o We should be able to add and remove an entry, and // o That entry should cost one reserve. // o The reserve should be returned when the entry is removed. @@ -602,8 +549,6 @@ struct DepositPreauth_test : public beast::unit_test::suite Account const gw{"gw"}; IOU const USD(gw["USD"]); - bool const supportsPreauth = {features[featureDepositPreauth]}; - { // The initial implementation of DepositAuth had a bug where an // account with the DepositAuth flag set could not make a payment @@ -632,15 +577,11 @@ struct DepositPreauth_test : public beast::unit_test::suite env(fset(becky, asfDepositAuth)); env.close(); - // becky pays herself again. Whether it succeeds depends on - // whether featureDepositPreauth is enabled. - TER const expect{ - supportsPreauth ? TER{tesSUCCESS} : TER{tecNO_PERMISSION}}; - + // becky pays herself again. env(pay(becky, becky, USD(10)), path(~USD), sendmax(XRP(10)), - ter(expect)); + ter(tesSUCCESS)); env.close(); { @@ -652,29 +593,17 @@ struct DepositPreauth_test : public beast::unit_test::suite bool const supportsCredentials = features[featureCredentials]; - TER const expectCredentials( - supportsCredentials ? TER(tesSUCCESS) : TER(temDISABLED)); - TER const expectPayment( - !supportsCredentials - ? TER(temDISABLED) - : (!supportsPreauth ? TER(tecNO_PERMISSION) - : TER(tesSUCCESS))); - TER const expectDP( - !supportsPreauth - ? TER(temDISABLED) - : (!supportsCredentials ? TER(temDISABLED) - : TER(tesSUCCESS))); + TER const expectTer( + !supportsCredentials ? TER(temDISABLED) : TER(tesSUCCESS)); env(deposit::authCredentials(becky, {{carol, credType}}), - ter(expectDP)); + ter(expectTer)); env.close(); // gw accept credentials - env(credentials::create(gw, carol, credType), - ter(expectCredentials)); + env(credentials::create(gw, carol, credType), ter(expectTer)); env.close(); - env(credentials::accept(gw, carol, credType), - ter(expectCredentials)); + env(credentials::accept(gw, carol, credType), ter(expectTer)); env.close(); auto jv = credentials::ledgerEntry(env, gw, carol, credType); @@ -685,115 +614,94 @@ struct DepositPreauth_test : public beast::unit_test::suite env(pay(gw, becky, USD(100)), credentials::ids({credIdx}), - ter(expectPayment)); + ter(expectTer)); env.close(); } - - { - using namespace std::chrono; - - if (!supportsPreauth) - { - auto const seq1 = env.seq(alice); - env(escrow::create(alice, becky, XRP(100)), - escrow::finish_time(env.now() + 1s)); - env.close(); - - // Failed as rule is disabled - env(escrow::finish(gw, alice, seq1), - fee(1500), - ter(tecNO_PERMISSION)); - env.close(); - } - } } - if (supportsPreauth) - { - // Make sure DepositPreauthorization works for payments. + // Make sure DepositPreauthorization works for payments. - Account const carol{"carol"}; + Account const carol{"carol"}; - Env env(*this, features); - env.fund(XRP(5000), alice, becky, carol, gw); - env.close(); + Env env(*this, features); + env.fund(XRP(5000), alice, becky, carol, gw); + env.close(); - env.trust(USD(1000), alice); - env.trust(USD(1000), becky); - env.trust(USD(1000), carol); - env.close(); + env.trust(USD(1000), alice); + env.trust(USD(1000), becky); + env.trust(USD(1000), carol); + env.close(); - env(pay(gw, alice, USD(1000))); - env.close(); + env(pay(gw, alice, USD(1000))); + env.close(); - // Make XRP and IOU payments from alice to becky. Should be fine. - env(pay(alice, becky, XRP(100))); - env(pay(alice, becky, USD(100))); - env.close(); + // Make XRP and IOU payments from alice to becky. Should be fine. + env(pay(alice, becky, XRP(100))); + env(pay(alice, becky, USD(100))); + env.close(); - // becky decides to require authorization for deposits. - env(fset(becky, asfDepositAuth)); - env.close(); + // becky decides to require authorization for deposits. + env(fset(becky, asfDepositAuth)); + env.close(); - // alice can no longer pay becky. - env(pay(alice, becky, XRP(100)), ter(tecNO_PERMISSION)); - env(pay(alice, becky, USD(100)), ter(tecNO_PERMISSION)); - env.close(); + // alice can no longer pay becky. + env(pay(alice, becky, XRP(100)), ter(tecNO_PERMISSION)); + env(pay(alice, becky, USD(100)), ter(tecNO_PERMISSION)); + env.close(); - // becky preauthorizes carol for deposit, which doesn't provide - // authorization for alice. - env(deposit::auth(becky, carol)); - env.close(); + // becky preauthorizes carol for deposit, which doesn't provide + // authorization for alice. + env(deposit::auth(becky, carol)); + env.close(); - // alice still can't pay becky. - env(pay(alice, becky, XRP(100)), ter(tecNO_PERMISSION)); - env(pay(alice, becky, USD(100)), ter(tecNO_PERMISSION)); - env.close(); + // alice still can't pay becky. + env(pay(alice, becky, XRP(100)), ter(tecNO_PERMISSION)); + env(pay(alice, becky, USD(100)), ter(tecNO_PERMISSION)); + env.close(); - // becky preauthorizes alice for deposit. - env(deposit::auth(becky, alice)); - env.close(); + // becky preauthorizes alice for deposit. + env(deposit::auth(becky, alice)); + env.close(); - // alice can now pay becky. - env(pay(alice, becky, XRP(100))); - env(pay(alice, becky, USD(100))); - env.close(); + // alice can now pay becky. + env(pay(alice, becky, XRP(100))); + env(pay(alice, becky, USD(100))); + env.close(); - // alice decides to require authorization for deposits. - env(fset(alice, asfDepositAuth)); - env.close(); + // alice decides to require authorization for deposits. + env(fset(alice, asfDepositAuth)); + env.close(); - // Even though alice is authorized to pay becky, becky is not - // authorized to pay alice. - env(pay(becky, alice, XRP(100)), ter(tecNO_PERMISSION)); - env(pay(becky, alice, USD(100)), ter(tecNO_PERMISSION)); - env.close(); + // Even though alice is authorized to pay becky, becky is not + // authorized to pay alice. + env(pay(becky, alice, XRP(100)), ter(tecNO_PERMISSION)); + env(pay(becky, alice, USD(100)), ter(tecNO_PERMISSION)); + env.close(); - // becky unauthorizes carol. Should have no impact on alice. - env(deposit::unauth(becky, carol)); - env.close(); + // becky unauthorizes carol. Should have no impact on alice. + env(deposit::unauth(becky, carol)); + env.close(); - env(pay(alice, becky, XRP(100))); - env(pay(alice, becky, USD(100))); - env.close(); + env(pay(alice, becky, XRP(100))); + env(pay(alice, becky, USD(100))); + env.close(); - // becky unauthorizes alice. alice now can't pay becky. - env(deposit::unauth(becky, alice)); - env.close(); + // becky unauthorizes alice. alice now can't pay becky. + env(deposit::unauth(becky, alice)); + env.close(); - env(pay(alice, becky, XRP(100)), ter(tecNO_PERMISSION)); - env(pay(alice, becky, USD(100)), ter(tecNO_PERMISSION)); - env.close(); + env(pay(alice, becky, XRP(100)), ter(tecNO_PERMISSION)); + env(pay(alice, becky, USD(100)), ter(tecNO_PERMISSION)); + env.close(); - // becky decides to remove authorization for deposits. Now - // alice can pay becky again. - env(fclear(becky, asfDepositAuth)); - env.close(); + // becky decides to remove authorization for deposits. Now + // alice can pay becky again. + env(fclear(becky, asfDepositAuth)); + env.close(); - env(pay(alice, becky, XRP(100))); - env(pay(alice, becky, USD(100))); - env.close(); - } + env(pay(alice, becky, XRP(100))); + env(pay(alice, becky, USD(100))); + env.close(); } void @@ -1545,8 +1453,6 @@ struct DepositPreauth_test : public beast::unit_test::suite testEnable(); testInvalid(); auto const supported{jtx::testable_amendments()}; - testPayment(supported - featureDepositPreauth - featureCredentials); - testPayment(supported - featureDepositPreauth); testPayment(supported - featureCredentials); testPayment(supported); testCredentialsPayment(); diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 22e81ccca4..e3b2340022 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -252,16 +252,6 @@ struct Escrow_test : public beast::unit_test::suite using namespace jtx; using namespace std::chrono; - { - // Respect the "asfDisallowXRP" account flag: - Env env(*this, features - featureDepositAuth); - - env.fund(XRP(5000), "bob", "george"); - env(fset("george", asfDisallowXRP)); - env(escrow::create("bob", "george", XRP(10)), - escrow::finish_time(env.now() + 1s), - ter(tecNO_TARGET)); - } { // Ignore the "asfDisallowXRP" account flag, which we should // have been doing before. diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index aff617fdba..2cbf2598e1 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -180,13 +180,10 @@ public: // an offer. Show that the attempt to remove the offer fails. env.require(offers(alice, 2)); - // featureDepositPreauths changes the return code on an expired Offer. - // Adapt to that. - bool const featPreauth{features[featureDepositPreauth]}; env(offer(alice, XRP(5), USD(2)), json(sfExpiration.fieldName, lastClose(env)), json(jss::OfferSequence, offer2Seq), - ter(featPreauth ? TER{tecEXPIRED} : TER{tesSUCCESS})); + ter(tecEXPIRED)); env.close(); env.require(offers(alice, 2)); @@ -1082,13 +1079,9 @@ public: offers(alice, 0), owners(alice, 1)); - // Place an offer that should have already expired. - // The DepositPreauth amendment changes the return code; adapt to that. - bool const featPreauth{features[featureDepositPreauth]}; - env(offer(alice, xrpOffer, usdOffer), json(sfExpiration.fieldName, lastClose(env)), - ter(featPreauth ? TER{tecEXPIRED} : TER{tesSUCCESS})); + ter(tecEXPIRED)); env.require( balance(alice, startBalance - f - f), @@ -4499,21 +4492,13 @@ public: env(fset(gw, asfRequireAuth)); env.close(); - // The test behaves differently with or without DepositPreauth. - bool const preauth = features[featureDepositPreauth]; - // Before DepositPreauth an account with lsfRequireAuth set could not // create an offer to buy their own currency. After DepositPreauth // they can. - env(offer(gw, gwUSD(40), XRP(4000)), - ter(preauth ? TER{tesSUCCESS} : TER{tecNO_LINE})); + env(offer(gw, gwUSD(40), XRP(4000)), ter(tesSUCCESS)); env.close(); - env.require(offers(gw, preauth ? 1 : 0)); - - if (!preauth) - // The rest of the test verifies DepositPreauth behavior. - return; + env.require(offers(gw, 1)); // Set up an authorized trust line and pay alice gwUSD 50. env(trust(gw, aliceUSD(100)), txflags(tfSetfAuth)); diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 1756b8fdbf..893d71bfa8 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -656,16 +656,6 @@ struct PayChan_test : public beast::unit_test::suite auto const alice = Account("alice"); auto const bob = Account("bob"); - { - // Create a channel where dst disallows XRP - Env env(*this, features - featureDepositAuth); - env.fund(XRP(10000), alice, bob); - env(fset(bob, asfDisallowXRP)); - auto const chan = channel(alice, bob, env.seq(alice)); - env(create(alice, bob, XRP(1000), 3600s, alice.pk()), - ter(tecNO_TARGET)); - BEAST_EXPECT(!channelExists(*env.current(), chan)); - } { // Create a channel where dst disallows XRP. Ignore that flag, // since it's just advisory. @@ -677,19 +667,6 @@ struct PayChan_test : public beast::unit_test::suite BEAST_EXPECT(channelExists(*env.current(), chan)); } - { - // Claim to a channel where dst disallows XRP - // (channel is created before disallow xrp is set) - Env env(*this, features - featureDepositAuth); - env.fund(XRP(10000), alice, bob); - auto const chan = channel(alice, bob, env.seq(alice)); - env(create(alice, bob, XRP(1000), 3600s, alice.pk())); - BEAST_EXPECT(channelExists(*env.current(), chan)); - - env(fset(bob, asfDisallowXRP)); - auto const reqBal = XRP(500).value(); - env(claim(alice, chan, reqBal, reqBal), ter(tecNO_TARGET)); - } { // Claim to a channel where dst disallows XRP (channel is // created before disallow xrp is set). Ignore that flag diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index d176744c5c..4b7f156738 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -4330,9 +4330,10 @@ public: Account const ellie("ellie"); Account const fiona("fiona"); + constexpr int ledgersInQueue = 10; auto cfg = makeConfig( {{"minimum_txn_in_ledger_standalone", "1"}, - {"ledgers_in_queue", "5"}, + {"ledgers_in_queue", std::to_string(ledgersInQueue)}, {"maximum_txn_per_account", "10"}}, {{"account_reserve", "1000"}, {"owner_reserve", "50"}}); @@ -4358,7 +4359,9 @@ public: env.close(); env.fund(XRP(10000), fiona); env.close(); - checkMetrics(*this, env, 0, 10, 0, 2); + + auto const metrics = env.app().getTxQ().getMetrics(*env.current()); + checkMetrics(*this, env, 0, ledgersInQueue * metrics.txPerLedger, 0, 2); // Close ledgers until the amendments show up. int i = 0; @@ -4370,7 +4373,12 @@ public: } auto expectedPerLedger = ripple::detail::numUpVotedAmendments() + 1; checkMetrics( - *this, env, 0, 5 * expectedPerLedger, 0, expectedPerLedger); + *this, + env, + 0, + ledgersInQueue * expectedPerLedger, + 0, + expectedPerLedger); // Now wait 2 weeks modulo 256 ledgers for the amendments to be // enabled. Speed the process by closing ledgers every 80 minutes, @@ -4389,7 +4397,7 @@ public: *this, env, 0, - 5 * expectedPerLedger, + ledgersInQueue * expectedPerLedger, expectedPerLedger + 1, expectedPerLedger); @@ -4435,12 +4443,12 @@ public: prepareFee(++multiplier), ter(terQUEUED)); } - std::size_t expectedInQueue = 60; + std::size_t expectedInQueue = multiplier; checkMetrics( *this, env, expectedInQueue, - 5 * expectedPerLedger, + ledgersInQueue * expectedPerLedger, expectedPerLedger + 1, expectedPerLedger); @@ -4467,7 +4475,7 @@ public: *this, env, expectedInQueue, - 5 * expectedPerLedger, + ledgersInQueue * expectedPerLedger, expectedInLedger, expectedPerLedger); { diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index 31fc345862..3df3606a03 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -35,8 +35,7 @@ public: using namespace test::jtx; Account const alice("alice"); - // Test without DepositAuth enabled initially. - Env env(*this, testable_amendments() - featureDepositAuth); + Env env(*this, testable_amendments()); env.fund(XRP(10000), noripple(alice)); // Give alice a regular key so she can legally set and clear @@ -116,19 +115,6 @@ public: } } }; - - // Test with featureDepositAuth disabled. - testFlags( - {asfRequireDest, - asfRequireAuth, - asfDisallowXRP, - asfGlobalFreeze, - asfDisableMaster, - asfDefaultRipple}); - - // Enable featureDepositAuth and retest. - env.enableFeature(featureDepositAuth); - env.close(); testFlags( {asfRequireDest, asfRequireAuth, diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 1a53f39f60..13d5fd3969 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -322,8 +322,7 @@ class Feature_test : public beast::unit_test::suite testcase("No Params, Some Enabled"); using namespace test::jtx; - Env env{ - *this, FeatureBitset(featureDepositAuth, featureDepositPreauth)}; + Env env{*this, FeatureBitset{}}; std::map const& votes = ripple::detail::supportedAmendments(); diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index a1179da81b..fe96436f2f 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -177,13 +177,7 @@ CreateOffer::preclaim(PreclaimContext const& ctx) { // Note that this will get checked again in applyGuts, but it saves // us a call to checkAcceptAsset and possible false negative. - // - // The return code change is attached to featureDepositPreauth as a - // convenience, as the change is not big enough to deserve its own - // amendment. - return ctx.view.rules().enabled(featureDepositPreauth) - ? TER{tecEXPIRED} - : TER{tesSUCCESS}; + return tecEXPIRED; } // Make sure that we are authorized to hold what the taker will pay us. @@ -235,10 +229,7 @@ CreateOffer::checkAcceptAsset( return (flags & tapRETRY) ? TER{terNO_ACCOUNT} : TER{tecNO_ISSUER}; } - // This code is attached to the DepositPreauth amendment as a matter of - // convenience. The change is not significant enough to deserve its - // own amendment. - if (view.rules().enabled(featureDepositPreauth) && (issue.account == id)) + if (issue.account == id) // An account can always accept its own issuance. return tesSUCCESS; @@ -599,13 +590,7 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) { // If the offer has expired, the transaction has successfully // done nothing, so short circuit from here. - // - // The return code change is attached to featureDepositPreauth as a - // convenience. The change is not big enough to deserve a fix code. - TER const ter{ - sb.rules().enabled(featureDepositPreauth) ? TER{tecEXPIRED} - : TER{tesSUCCESS}}; - return {ter, true}; + return {tecEXPIRED, true}; } bool const bOpenLedger = sb.open(); diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index ed72900248..0654c8dbce 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -232,8 +232,7 @@ DeleteAccount::preclaim(PreclaimContext const& ctx) if (!ctx.tx.isFieldPresent(sfCredentialIDs)) { // Check whether the destination account requires deposit authorization. - if (ctx.view.rules().enabled(featureDepositAuth) && - (sleDst->getFlags() & lsfDepositAuth)) + if (sleDst->getFlags() & lsfDepositAuth) { if (!ctx.view.exists(keylet::depositPreauth(dst, account))) return tecNO_PERMISSION; @@ -353,8 +352,7 @@ DeleteAccount::doApply() if (!src || !dst) return tefBAD_LEDGER; // LCOV_EXCL_LINE - if (ctx_.view().rules().enabled(featureDepositAuth) && - ctx_.tx.isFieldPresent(sfCredentialIDs)) + if (ctx_.tx.isFieldPresent(sfCredentialIDs)) { if (auto err = verifyDepositPreauth( ctx_.tx, ctx_.view(), account_, dstID, dst, ctx_.journal); diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index fb3e3c509e..5cf90809a2 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -459,12 +459,6 @@ EscrowCreate::doApply() if (((*sled)[sfFlags] & lsfRequireDestTag) && !ctx_.tx[~sfDestinationTag]) return tecDST_TAG_NEEDED; - - // Obeying the lsfDisallowXRP flag was a bug. Piggyback on - // featureDepositAuth to remove the bug. - if (!ctx_.view().rules().enabled(featureDepositAuth) && - ((*sled)[sfFlags] & lsfDisallowXRP)) - return tecNO_TARGET; } // Create escrow in ledger. Note that we we use the value from the @@ -1041,13 +1035,10 @@ EscrowFinish::doApply() if (!sled) return tecNO_DST; - if (ctx_.view().rules().enabled(featureDepositAuth)) - { - if (auto err = verifyDepositPreauth( - ctx_.tx, ctx_.view(), account_, destID, sled, ctx_.journal); - !isTesSuccess(err)) - return err; - } + if (auto err = verifyDepositPreauth( + ctx_.tx, ctx_.view(), account_, destID, sled, ctx_.journal); + !isTesSuccess(err)) + return err; AccountID const account = (*slep)[sfAccount]; diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index a6d9996b89..c3dc99e7fb 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -209,12 +209,6 @@ PayChanCreate::preclaim(PreclaimContext const& ctx) if ((flags & lsfRequireDestTag) && !ctx.tx[~sfDestinationTag]) return tecDST_TAG_NEEDED; - // Obeying the lsfDisallowXRP flag was a bug. Piggyback on - // featureDepositAuth to remove the bug. - if (!ctx.view.rules().enabled(featureDepositAuth) && - (flags & lsfDisallowXRP)) - return tecNO_TARGET; - // Pseudo-accounts cannot receive payment channels, other than native // to their underlying ledger object - implemented in their respective // transaction types. Note, this is not amendment-gated because all @@ -525,20 +519,10 @@ PayChanClaim::doApply() if (!sled) return tecNO_DST; - // Obeying the lsfDisallowXRP flag was a bug. Piggyback on - // featureDepositAuth to remove the bug. - bool const depositAuth{ctx_.view().rules().enabled(featureDepositAuth)}; - if (!depositAuth && - (txAccount == src && (sled->getFlags() & lsfDisallowXRP))) - return tecNO_TARGET; - - if (depositAuth) - { - if (auto err = verifyDepositPreauth( - ctx_.tx, ctx_.view(), txAccount, dst, sled, ctx_.journal); - !isTesSuccess(err)) - return err; - } + if (auto err = verifyDepositPreauth( + ctx_.tx, ctx_.view(), txAccount, dst, sled, ctx_.journal); + !isTesSuccess(err)) + return err; (*slep)[sfBalance] = ctx_.tx[sfBalance]; XRPAmount const reqDelta = reqBalance - chanBalance; diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 305d54438e..3aff4db5e1 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -417,43 +417,28 @@ Payment::doApply() view().update(sleDst); } - // Determine whether the destination requires deposit authorization. - bool const depositAuth = view().rules().enabled(featureDepositAuth); - bool const reqDepositAuth = - sleDst->getFlags() & lsfDepositAuth && depositAuth; - - bool const depositPreauth = view().rules().enabled(featureDepositPreauth); - bool const ripple = (hasPaths || sendMax || !dstAmount.native()) && !mptDirect; - // If the destination has lsfDepositAuth set, then only direct XRP - // payments (no intermediate steps) are allowed to the destination. - if (!depositPreauth && ripple && reqDepositAuth) - return tecNO_PERMISSION; - if (ripple) { // Ripple payment with at least one intermediate step and uses // transitive balances. - if (depositPreauth && depositAuth) - { - // If depositPreauth is enabled, then an account that requires - // authorization has two ways to get an IOU Payment in: - // 1. If Account == Destination, or - // 2. If Account is deposit preauthorized by destination. + // An account that requires authorization has two ways to get an + // IOU Payment in: + // 1. If Account == Destination, or + // 2. If Account is deposit preauthorized by destination. - if (auto err = verifyDepositPreauth( - ctx_.tx, - ctx_.view(), - account_, - dstAccountID, - sleDst, - ctx_.journal); - !isTesSuccess(err)) - return err; - } + if (auto err = verifyDepositPreauth( + ctx_.tx, + ctx_.view(), + account_, + dstAccountID, + sleDst, + ctx_.journal); + !isTesSuccess(err)) + return err; path::RippleCalc::Input rcInput; rcInput.partialPaymentAllowed = partialPaymentAllowed; @@ -630,43 +615,40 @@ Payment::doApply() // The source account does have enough money. Make sure the // source account has authority to deposit to the destination. - if (depositAuth) + // An account that requires authorization has three ways to get an XRP + // Payment in: + // 1. If Account == Destination, or + // 2. If Account is deposit preauthorized by destination, or + // 3. If the destination's XRP balance is + // a. less than or equal to the base reserve and + // b. the deposit amount is less than or equal to the base reserve, + // then we allow the deposit. + // + // Rule 3 is designed to keep an account from getting wedged + // in an unusable state if it sets the lsfDepositAuth flag and + // then consumes all of its XRP. Without the rule if an + // account with lsfDepositAuth set spent all of its XRP, it + // would be unable to acquire more XRP required to pay fees. + // + // We choose the base reserve as our bound because it is + // a small number that seldom changes but is always sufficient + // to get the account un-wedged. + + // Get the base reserve. + XRPAmount const dstReserve{view().fees().reserve}; + + if (dstAmount > dstReserve || + sleDst->getFieldAmount(sfBalance) > dstReserve) { - // If depositPreauth is enabled, then an account that requires - // authorization has three ways to get an XRP Payment in: - // 1. If Account == Destination, or - // 2. If Account is deposit preauthorized by destination, or - // 3. If the destination's XRP balance is - // a. less than or equal to the base reserve and - // b. the deposit amount is less than or equal to the base reserve, - // then we allow the deposit. - // - // Rule 3 is designed to keep an account from getting wedged - // in an unusable state if it sets the lsfDepositAuth flag and - // then consumes all of its XRP. Without the rule if an - // account with lsfDepositAuth set spent all of its XRP, it - // would be unable to acquire more XRP required to pay fees. - // - // We choose the base reserve as our bound because it is - // a small number that seldom changes but is always sufficient - // to get the account un-wedged. - - // Get the base reserve. - XRPAmount const dstReserve{view().fees().reserve}; - - if (dstAmount > dstReserve || - sleDst->getFieldAmount(sfBalance) > dstReserve) - { - if (auto err = verifyDepositPreauth( - ctx_.tx, - ctx_.view(), - account_, - dstAccountID, - sleDst, - ctx_.journal); - !isTesSuccess(err)) - return err; - } + if (auto err = verifyDepositPreauth( + ctx_.tx, + ctx_.view(), + account_, + dstAccountID, + sleDst, + ctx_.journal); + !isTesSuccess(err)) + return err; } // Do the arithmetic for the transfer and make the ledger change. diff --git a/src/xrpld/app/tx/detail/SetAccount.cpp b/src/xrpld/app/tx/detail/SetAccount.cpp index e966dba8f5..db513d9f05 100644 --- a/src/xrpld/app/tx/detail/SetAccount.cpp +++ b/src/xrpld/app/tx/detail/SetAccount.cpp @@ -463,18 +463,15 @@ SetAccount::doApply() // // DepositAuth // - if (view().rules().enabled(featureDepositAuth)) + if (uSetFlag == asfDepositAuth) { - if (uSetFlag == asfDepositAuth) - { - JLOG(j_.trace()) << "Set lsfDepositAuth."; - uFlagsOut |= lsfDepositAuth; - } - else if (uClearFlag == asfDepositAuth) - { - JLOG(j_.trace()) << "Clear lsfDepositAuth."; - uFlagsOut &= ~lsfDepositAuth; - } + JLOG(j_.trace()) << "Set lsfDepositAuth."; + uFlagsOut |= lsfDepositAuth; + } + else if (uClearFlag == asfDepositAuth) + { + JLOG(j_.trace()) << "Clear lsfDepositAuth."; + uFlagsOut &= ~lsfDepositAuth; } // From 9ffb434315cc616627da909bc98b74351ce1bb6c Mon Sep 17 00:00:00 2001 From: Jingchen Date: Tue, 11 Nov 2025 17:45:13 +0000 Subject: [PATCH 14/19] refactor: Add `XRPL_RETIRE_FIX` and `XRPL_RETIRE_FEATURE` macros (#6014) Rather than having a single `XRPL_RETIRE` macro that applies to both feature and fix amendments, this change replaces it by new `XRPL_RETIRE_FIX` and `XRPL_RETIRE_FEATURE` macros that avoids confusion between whether to prefix the amendment name with `feature` or `fix`. --- include/xrpl/protocol/Feature.h | 30 ++++--- include/xrpl/protocol/detail/features.macro | 92 +++++++++++---------- src/libxrpl/protocol/Feature.cpp | 25 ++++-- 3 files changed, 85 insertions(+), 62 deletions(-) diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index 42f2db41f2..193f0665dc 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -78,12 +78,15 @@ namespace detail { #undef XRPL_FEATURE #pragma push_macro("XRPL_FIX") #undef XRPL_FIX -#pragma push_macro("XRPL_RETIRE") -#undef XRPL_RETIRE +#pragma push_macro("XRPL_RETIRE_FEATURE") +#undef XRPL_RETIRE_FEATURE +#pragma push_macro("XRPL_RETIRE_FIX") +#undef XRPL_RETIRE_FIX #define XRPL_FEATURE(name, supported, vote) +1 #define XRPL_FIX(name, supported, vote) +1 -#define XRPL_RETIRE(name) +1 +#define XRPL_RETIRE_FEATURE(name) +1 +#define XRPL_RETIRE_FIX(name) +1 // This value SHOULD be equal to the number of amendments registered in // Feature.cpp. Because it's only used to reserve storage, and determine how @@ -94,8 +97,10 @@ static constexpr std::size_t numFeatures = #include ); -#undef XRPL_RETIRE -#pragma pop_macro("XRPL_RETIRE") +#undef XRPL_RETIRE_FEATURE +#pragma pop_macro("XRPL_RETIRE_FEATURE") +#undef XRPL_RETIRE_FIX +#pragma pop_macro("XRPL_RETIRE_FIX") #undef XRPL_FIX #pragma pop_macro("XRPL_FIX") #undef XRPL_FEATURE @@ -339,17 +344,22 @@ foreachFeature(FeatureBitset bs, F&& f) #undef XRPL_FEATURE #pragma push_macro("XRPL_FIX") #undef XRPL_FIX -#pragma push_macro("XRPL_RETIRE") -#undef XRPL_RETIRE +#pragma push_macro("XRPL_RETIRE_FEATURE") +#undef XRPL_RETIRE_FEATURE +#pragma push_macro("XRPL_RETIRE_FIX") +#undef XRPL_RETIRE_FIX #define XRPL_FEATURE(name, supported, vote) extern uint256 const feature##name; #define XRPL_FIX(name, supported, vote) extern uint256 const fix##name; -#define XRPL_RETIRE(name) +#define XRPL_RETIRE_FEATURE(name) +#define XRPL_RETIRE_FIX(name) #include -#undef XRPL_RETIRE -#pragma pop_macro("XRPL_RETIRE") +#undef XRPL_RETIRE_FEATURE +#pragma pop_macro("XRPL_RETIRE_FEATURE") +#undef XRPL_RETIRE_FIX +#pragma pop_macro("XRPL_RETIRE_FIX") #undef XRPL_FIX #pragma pop_macro("XRPL_FIX") #undef XRPL_FEATURE diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 076307ff36..f38793cb9e 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -4,8 +4,11 @@ #if !defined(XRPL_FIX) #error "undefined macro: XRPL_FIX" #endif -#if !defined(XRPL_RETIRE) -#error "undefined macro: XRPL_RETIRE" +#if !defined(XRPL_RETIRE_FEATURE) +#error "undefined macro: XRPL_RETIRE_FEATURE" +#endif +#if !defined(XRPL_RETIRE_FIX) +#error "undefined macro: XRPL_RETIRE_FIX" #endif // Add new amendments to the top of this list. @@ -90,45 +93,46 @@ XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete) // All known amendments and amendments that may appear in a validated ledger // must be registered either here or above with the "active" amendments // -// Please keep this list sorted alphabetically for convenience. -XRPL_RETIRE(fix1201) -XRPL_RETIRE(fix1368) -XRPL_RETIRE(fix1373) -XRPL_RETIRE(fix1512) -XRPL_RETIRE(fix1513) -XRPL_RETIRE(fix1515) -XRPL_RETIRE(fix1523) -XRPL_RETIRE(fix1528) -XRPL_RETIRE(fix1543) -XRPL_RETIRE(fix1571) -XRPL_RETIRE(fix1578) -XRPL_RETIRE(fix1623) -XRPL_RETIRE(fix1781) -XRPL_RETIRE(fixAmendmentMajorityCalc) -XRPL_RETIRE(fixCheckThreading) -XRPL_RETIRE(fixMasterKeyAsRegularKey) -XRPL_RETIRE(fixNonFungibleTokensV1_2) -XRPL_RETIRE(fixNFTokenRemint) -XRPL_RETIRE(fixPayChanRecipientOwnerDir) -XRPL_RETIRE(fixQualityUpperBound) -XRPL_RETIRE(fixReducedOffersV1) -XRPL_RETIRE(fixRmSmallIncreasedQOffers) -XRPL_RETIRE(fixSTAmountCanonicalize) -XRPL_RETIRE(fixTakerDryOfferRemoval) -XRPL_RETIRE(fixTrustLinesToSelf) -XRPL_RETIRE(CheckCashMakesTrustLine) -XRPL_RETIRE(CryptoConditions) -XRPL_RETIRE(DepositAuth) -XRPL_RETIRE(DepositPreauth) -XRPL_RETIRE(Escrow) -XRPL_RETIRE(EnforceInvariants) -XRPL_RETIRE(FeeEscalation) -XRPL_RETIRE(FlowCross) -XRPL_RETIRE(HardenedValidations) -XRPL_RETIRE(ImmediateOfferKilled) -XRPL_RETIRE(MultiSign) -XRPL_RETIRE(NonFungibleTokensV1_1) -XRPL_RETIRE(PayChan) -XRPL_RETIRE(SortedDirectories) -XRPL_RETIRE(TickSize) -XRPL_RETIRE(TrustSetAuth) +// Please keep both lists sorted alphabetically for convenience. +XRPL_RETIRE_FIX(1201) +XRPL_RETIRE_FIX(1368) +XRPL_RETIRE_FIX(1373) +XRPL_RETIRE_FIX(1512) +XRPL_RETIRE_FIX(1513) +XRPL_RETIRE_FIX(1515) +XRPL_RETIRE_FIX(1523) +XRPL_RETIRE_FIX(1528) +XRPL_RETIRE_FIX(1543) +XRPL_RETIRE_FIX(1571) +XRPL_RETIRE_FIX(1578) +XRPL_RETIRE_FIX(1623) +XRPL_RETIRE_FIX(1781) +XRPL_RETIRE_FIX(AmendmentMajorityCalc) +XRPL_RETIRE_FIX(CheckThreading) +XRPL_RETIRE_FIX(MasterKeyAsRegularKey) +XRPL_RETIRE_FIX(NonFungibleTokensV1_2) +XRPL_RETIRE_FIX(NFTokenRemint) +XRPL_RETIRE_FIX(PayChanRecipientOwnerDir) +XRPL_RETIRE_FIX(QualityUpperBound) +XRPL_RETIRE_FIX(ReducedOffersV1) +XRPL_RETIRE_FIX(RmSmallIncreasedQOffers) +XRPL_RETIRE_FIX(STAmountCanonicalize) +XRPL_RETIRE_FIX(TakerDryOfferRemoval) +XRPL_RETIRE_FIX(TrustLinesToSelf) + +XRPL_RETIRE_FEATURE(CheckCashMakesTrustLine) +XRPL_RETIRE_FEATURE(CryptoConditions) +XRPL_RETIRE_FEATURE(DepositAuth) +XRPL_RETIRE_FEATURE(DepositPreauth) +XRPL_RETIRE_FEATURE(Escrow) +XRPL_RETIRE_FEATURE(EnforceInvariants) +XRPL_RETIRE_FEATURE(FeeEscalation) +XRPL_RETIRE_FEATURE(FlowCross) +XRPL_RETIRE_FEATURE(HardenedValidations) +XRPL_RETIRE_FEATURE(ImmediateOfferKilled) +XRPL_RETIRE_FEATURE(MultiSign) +XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1) +XRPL_RETIRE_FEATURE(PayChan) +XRPL_RETIRE_FEATURE(SortedDirectories) +XRPL_RETIRE_FEATURE(TickSize) +XRPL_RETIRE_FEATURE(TrustSetAuth) diff --git a/src/libxrpl/protocol/Feature.cpp b/src/libxrpl/protocol/Feature.cpp index 8d09378fc7..10c42ccb8a 100644 --- a/src/libxrpl/protocol/Feature.cpp +++ b/src/libxrpl/protocol/Feature.cpp @@ -411,8 +411,10 @@ featureToName(uint256 const& f) #undef XRPL_FEATURE #pragma push_macro("XRPL_FIX") #undef XRPL_FIX -#pragma push_macro("XRPL_RETIRE") -#undef XRPL_RETIRE +#pragma push_macro("XRPL_RETIRE_FEATURE") +#undef XRPL_RETIRE_FEATURE +#pragma push_macro("XRPL_RETIRE_FIX") +#undef XRPL_RETIRE_FIX #define XRPL_FEATURE(name, supported, vote) \ uint256 const feature##name = registerFeature(#name, supported, vote); @@ -420,16 +422,23 @@ featureToName(uint256 const& f) uint256 const fix##name = registerFeature("fix" #name, supported, vote); // clang-format off -#define XRPL_RETIRE(name) \ - [[deprecated("The referenced amendment has been retired")]] \ - [[maybe_unused]] \ - uint256 const retired##name = retireFeature(#name); +#define XRPL_RETIRE_FEATURE(name) \ + [[deprecated("The referenced feature amendment has been retired")]] \ + [[maybe_unused]] \ + uint256 const retiredFeature##name = retireFeature(#name); + +#define XRPL_RETIRE_FIX(name) \ + [[deprecated("The referenced fix amendment has been retired")]] \ + [[maybe_unused]] \ + uint256 const retiredFix##name = retireFeature("fix" #name); // clang-format on #include -#undef XRPL_RETIRE -#pragma pop_macro("XRPL_RETIRE") +#undef XRPL_RETIRE_FEATURE +#pragma pop_macro("XRPL_RETIRE_FEATURE") +#undef XRPL_RETIRE_FIX +#pragma pop_macro("XRPL_RETIRE_FIX") #undef XRPL_FIX #pragma pop_macro("XRPL_FIX") #undef XRPL_FEATURE From c2a90b706fdd8ebce191fdd3885852ced888f4d9 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 11 Nov 2025 18:17:03 +0000 Subject: [PATCH 15/19] ci: Specify bash as the default shell in workflows (#6021) --- .github/workflows/reusable-build-test-config.yml | 12 ++++-------- .github/workflows/reusable-strategy-matrix.yml | 4 ++++ .github/workflows/upload-conan-deps.yml | 4 ++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 79d21a870a..7032fa5a43 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -55,6 +55,10 @@ on: description: "The Codecov token to use for uploading coverage reports." required: true +defaults: + run: + shell: bash + jobs: build-and-test: name: ${{ inputs.config_name }} @@ -100,7 +104,6 @@ jobs: log_verbosity: ${{ runner.os == 'Windows' && 'quiet' || 'verbose' }} - name: Configure CMake - shell: bash working-directory: ${{ inputs.build_dir }} env: BUILD_TYPE: ${{ inputs.build_type }} @@ -114,7 +117,6 @@ jobs: .. - name: Build the binary - shell: bash working-directory: ${{ inputs.build_dir }} env: BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} @@ -141,7 +143,6 @@ jobs: - name: Check linking (Linux) if: ${{ runner.os == 'Linux' }} working-directory: ${{ inputs.build_dir }} - shell: bash run: | ldd ./rippled if [ "$(ldd ./rippled | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then @@ -154,7 +155,6 @@ jobs: - name: Verify presence of instrumentation (Linux) if: ${{ runner.os == 'Linux' && env.ENABLED_VOIDSTAR == 'true' }} working-directory: ${{ inputs.build_dir }} - shell: bash run: | ./rippled --version | grep libvoidstar @@ -165,7 +165,6 @@ jobs: env: BUILD_TYPE: ${{ inputs.build_type }} PARALLELISM: ${{ runner.os == 'Windows' && '1' || steps.nproc.outputs.nproc }} - shell: bash run: | ctest \ --output-on-failure \ @@ -175,7 +174,6 @@ jobs: - 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: | @@ -183,7 +181,6 @@ jobs: - 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 @@ -196,7 +193,6 @@ jobs: env: BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} BUILD_TYPE: ${{ inputs.build_type }} - shell: bash run: | cmake \ --build . \ diff --git a/.github/workflows/reusable-strategy-matrix.yml b/.github/workflows/reusable-strategy-matrix.yml index 129f65938b..c975347307 100644 --- a/.github/workflows/reusable-strategy-matrix.yml +++ b/.github/workflows/reusable-strategy-matrix.yml @@ -18,6 +18,10 @@ on: description: "The generated strategy matrix." value: ${{ jobs.generate-matrix.outputs.matrix }} +defaults: + run: + shell: bash + jobs: generate-matrix: runs-on: ubuntu-latest diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index f6262a2f1f..357d756fa7 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -40,6 +40,10 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +defaults: + run: + shell: bash + jobs: # Generate the strategy matrix to be used by the following job. generate-matrix: From 2ebc2ca885ffa90fa6c7e8cf421dc0a12a34c3a8 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 11 Nov 2025 19:39:09 +0000 Subject: [PATCH 16/19] fix: floating point representation errors in vault (#5997) This change fixes floating point errors in conversion of shares to assets and other way, used in `VaultDeposit`, `VaultWithdraw` and `VaultClawback`. In the floating point calculations the division introduces a larger error than multiplication. If we do division first, then the error introduced will be increased by the multiplication that follows, which is therefore the wrong order to perform these two operations. This change flips the order of arithmetic operations, which minimizes the error. --- src/libxrpl/ledger/View.cpp | 8 ++-- src/test/app/Vault_test.cpp | 83 ++++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index 0175b099ea..069bd3a4d8 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -2885,7 +2885,7 @@ assetsToSharesDeposit( .truncate()}; Number const shareTotal = issuance->at(sfOutstandingAmount); - shares = (shareTotal * (assets / assetTotal)).truncate(); + shares = ((shareTotal * assets) / assetTotal).truncate(); return shares; } @@ -2914,7 +2914,7 @@ sharesToAssetsDeposit( false}; Number const shareTotal = issuance->at(sfOutstandingAmount); - assets = assetTotal * (shares / shareTotal); + assets = (assetTotal * shares) / shareTotal; return assets; } @@ -2940,7 +2940,7 @@ assetsToSharesWithdraw( if (assetTotal == 0) return shares; Number const shareTotal = issuance->at(sfOutstandingAmount); - Number result = shareTotal * (assets / assetTotal); + Number result = (shareTotal * assets) / assetTotal; if (truncate == TruncateShares::yes) result = result.truncate(); shares = result; @@ -2968,7 +2968,7 @@ sharesToAssetsWithdraw( if (assetTotal == 0) return assets; Number const shareTotal = issuance->at(sfOutstandingAmount); - assets = assetTotal * (shares / shareTotal); + assets = (assetTotal * shares) / shareTotal; return assets; } diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index ccbf0fed42..2ca525c036 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -2454,6 +2454,7 @@ class Vault_test : public beast::unit_test::suite struct CaseArgs { int initialXRP = 1000; + Number initialIOU = 200; double transferRate = 1.0; }; @@ -2481,7 +2482,7 @@ class Vault_test : public beast::unit_test::suite PrettyAsset const asset = issuer["IOU"]; env.trust(asset(1000), owner); env.trust(asset(1000), charlie); - env(pay(issuer, owner, asset(200))); + env(pay(issuer, owner, asset(args.initialIOU))); env(rate(issuer, args.transferRate)); env.close(); @@ -2859,6 +2860,86 @@ class Vault_test : public beast::unit_test::suite env(tx1); }); + testCase( + [&, this]( + Env& env, + Account const& owner, + Account const& issuer, + Account const& charlie, + auto const& vaultAccount, + Vault& vault, + PrettyAsset const& asset, + auto&&...) { + testcase("IOU calculation rounding"); + + auto [tx, keylet] = + vault.create({.owner = owner, .asset = asset}); + tx[sfScale] = 1; + env(tx); + env.close(); + + auto const startingOwnerBalance = env.balance(owner, asset); + BEAST_EXPECT( + (startingOwnerBalance.value() == + STAmount{asset, 11875, -2})); + + // This operation (first deposit 100, then 3.75 x 5) is known to + // have triggered calculation rounding errors in Number + // (addition and division), causing the last deposit to be + // blocked by Vault invariants. + env(vault.deposit( + {.depositor = owner, + .id = keylet.key, + .amount = asset(100)})); + + auto const tx1 = vault.deposit( + {.depositor = owner, + .id = keylet.key, + .amount = asset(Number(375, -2))}); + for (auto i = 0; i < 5; ++i) + { + env(tx1); + } + env.close(); + + { + STAmount const xfer{asset, 1185, -1}; + BEAST_EXPECT( + env.balance(owner, asset) == + startingOwnerBalance.value() - xfer); + BEAST_EXPECT( + env.balance(vaultAccount(keylet), asset) == xfer); + + auto const vault = env.le(keylet); + BEAST_EXPECT(vault->at(sfAssetsAvailable) == xfer); + BEAST_EXPECT(vault->at(sfAssetsTotal) == xfer); + } + + // Total vault balance should be 118.5 IOU. Withdraw and delete + // the vault to verify this exact amount was deposited and the + // owner has matching shares + env(vault.withdraw( + {.depositor = owner, + .id = keylet.key, + .amount = asset(Number(1000 + 37 * 5, -1))})); + + { + BEAST_EXPECT( + env.balance(owner, asset) == + startingOwnerBalance.value()); + BEAST_EXPECT( + env.balance(vaultAccount(keylet), asset) == + beast::zero); + auto const vault = env.le(keylet); + BEAST_EXPECT(vault->at(sfAssetsAvailable) == beast::zero); + BEAST_EXPECT(vault->at(sfAssetsTotal) == beast::zero); + } + + env(vault.del({.owner = owner, .id = keylet.key})); + env.close(); + }, + {.initialIOU = Number(11875, -2)}); + auto const [acctReserve, incReserve] = [this]() -> std::pair { Env env{*this, testable_amendments()}; return { From 5fc07e3979da38ff3e04221caecb19670f195a4b Mon Sep 17 00:00:00 2001 From: hustrust Date: Wed, 12 Nov 2025 21:23:45 +0800 Subject: [PATCH 17/19] docs: fix spelling in comments (#6002) --- src/libxrpl/basics/BasicConfig.cpp | 2 +- src/test/app/NFToken_test.cpp | 2 +- src/test/app/NetworkOPs_test.cpp | 2 +- src/test/app/PayChan_test.cpp | 4 ++-- src/test/csf/Peer.h | 2 +- src/test/csf/collectors.h | 2 +- src/test/csf/events.h | 2 +- src/test/csf/ledgers.h | 2 +- src/test/nodestore/TestBase.h | 2 +- src/test/nodestore/import_test.cpp | 2 +- src/test/overlay/tx_reduce_relay_test.cpp | 2 +- src/test/rpc/NoRippleCheck_test.cpp | 2 +- 12 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libxrpl/basics/BasicConfig.cpp b/src/libxrpl/basics/BasicConfig.cpp index 280c660794..63a001bcc3 100644 --- a/src/libxrpl/basics/BasicConfig.cpp +++ b/src/libxrpl/basics/BasicConfig.cpp @@ -30,7 +30,7 @@ Section::append(std::vector const& lines) // '=' static boost::regex const re1( "^" // start of line - "(?:\\s*)" // whitespace (optonal) + "(?:\\s*)" // whitespace (optional) "([a-zA-Z][_a-zA-Z0-9]*)" // "(?:\\s*)" // whitespace (optional) "(?:=)" // '=' diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 238042d388..a11446df71 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6808,7 +6808,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite mintAndCreateSellOffer(env, alice, XRP(0)); // Bob can accept the offer because the new NFT is stored in - // an existing NFTokenPage so no new reserve is requried + // an existing NFTokenPage so no new reserve is required env(token::acceptSellOffer(bob, sellOfferIndex)); env.close(); } diff --git a/src/test/app/NetworkOPs_test.cpp b/src/test/app/NetworkOPs_test.cpp index 166c3140d4..d9afe6d628 100644 --- a/src/test/app/NetworkOPs_test.cpp +++ b/src/test/app/NetworkOPs_test.cpp @@ -21,7 +21,7 @@ public: void testAllBadHeldTransactions() { - // All trasactions are already marked as SF_BAD, and we should be able + // All transactions are already marked as SF_BAD, and we should be able // to handle the case properly without an assertion failure testcase("No valid transactions in batch"); diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 893d71bfa8..4a1f1c14eb 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -1889,7 +1889,7 @@ struct PayChan_test : public beast::unit_test::suite BEAST_EXPECT(channelAmount(*env.current(), chan) == XRP(1000)); rmAccount(env, alice, carol, tecHAS_OBLIGATIONS); - // can only remove bob if the channel isn't in their owner direcotry + // can only remove bob if the channel isn't in their owner directory rmAccount(env, bob, carol, TER(tecHAS_OBLIGATIONS)); auto const feeDrops = env.current()->fees().base; @@ -1925,7 +1925,7 @@ struct PayChan_test : public beast::unit_test::suite // Owner closes, will close after settleDelay env(claim(alice, chan), txflags(tfClose)); env.close(); - // settle delay hasn't ellapsed. Channels should exist. + // settle delay hasn't elapsed. Channels should exist. BEAST_EXPECT(channelExists(*env.current(), chan)); auto const closeTime = env.current()->info().parentCloseTime; auto const minExpiration = closeTime + settleDelay; diff --git a/src/test/csf/Peer.h b/src/test/csf/Peer.h index 3a76e7b841..5f6f006b0f 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -404,7 +404,7 @@ struct Peer { minDuration = std::min(minDuration, link.data.delay); - // Send a messsage to neighbors to find the ledger + // Send a message to neighbors to find the ledger net.send( this, link.target, [to = link.target, from = this, ledgerID]() { if (auto it = to->ledgers.find(ledgerID); diff --git a/src/test/csf/collectors.h b/src/test/csf/collectors.h index c361701b4e..53494dad44 100644 --- a/src/test/csf/collectors.h +++ b/src/test/csf/collectors.h @@ -28,7 +28,7 @@ namespace csf { /** Group of collectors. Presents a group of collectors as a single collector which process an event - by calling each collector sequentially. This is analagous to CollectorRefs + by calling each collector sequentially. This is analogous to CollectorRefs in CollectorRef.h, but does *not* erase the type information of the combined collectors. */ diff --git a/src/test/csf/events.h b/src/test/csf/events.h index bf01de4eb2..b25dc4b339 100644 --- a/src/test/csf/events.h +++ b/src/test/csf/events.h @@ -13,7 +13,7 @@ namespace test { namespace csf { // Events are emitted by peers at a variety of points during the simulation. -// Each event is emitted by a particlar peer at a particular time. Collectors +// Each event is emitted by a particular peer at a particular time. Collectors // process these events, perhaps calculating statistics or storing events to // a log for post-processing. // diff --git a/src/test/csf/ledgers.h b/src/test/csf/ledgers.h index 26b416a2fb..80e431c1e2 100644 --- a/src/test/csf/ledgers.h +++ b/src/test/csf/ledgers.h @@ -35,7 +35,7 @@ namespace csf { Ledgers are immutable value types. All ledgers with the same sequence number, transactions, close time, etc. will have the same ledger ID. The - LedgerOracle class below manges ID assignments for a simulation and is the + LedgerOracle class below manages ID assignments for a simulation and is the only way to close and create a new ledger. Since the parent ledger ID is part of type, this also means ledgers with distinct histories will have distinct ids, even if they have the same set of transactions, sequence diff --git a/src/test/nodestore/TestBase.h b/src/test/nodestore/TestBase.h index c1401abc33..0675ad85d4 100644 --- a/src/test/nodestore/TestBase.h +++ b/src/test/nodestore/TestBase.h @@ -81,7 +81,7 @@ public: case 3: return hotUNKNOWN; } - // will never happen, but make static analysys tool happy. + // will never happen, but make static analysis tool happy. return hotUNKNOWN; }(); diff --git a/src/test/nodestore/import_test.cpp b/src/test/nodestore/import_test.cpp index 69ef50d453..30aaaa2ea9 100644 --- a/src/test/nodestore/import_test.cpp +++ b/src/test/nodestore/import_test.cpp @@ -235,7 +235,7 @@ parse_args(std::string const& s) // '=' static boost::regex const re1( "^" // start of line - "(?:\\s*)" // whitespace (optonal) + "(?:\\s*)" // whitespace (optional) "([a-zA-Z][_a-zA-Z0-9]*)" // "(?:\\s*)" // whitespace (optional) "(?:=)" // '=' diff --git a/src/test/overlay/tx_reduce_relay_test.cpp b/src/test/overlay/tx_reduce_relay_test.cpp index 3eafa08713..1ca9e7aac6 100644 --- a/src/test/overlay/tx_reduce_relay_test.cpp +++ b/src/test/overlay/tx_reduce_relay_test.cpp @@ -245,7 +245,7 @@ private: // (20+0.25*(60-20)-5=25), queue the rest, skip counts towards relayed // (60-25-5=30) testRelay("skip", true, 60, 0, 20, 25, 25, 30, skip); - // relay to minPeers + disabled + 25% of (nPeers - minPeers - disalbed) + // relay to minPeers + disabled + 25% of (nPeers - minPeers - disabled) // (20+10+0.25*(70-20-10)=40), queue the rest (30) testRelay("disabled", true, 70, 10, 20, 25, 40, 30); // relay to minPeers + disabled-not-in-skip + 25% of (nPeers - minPeers diff --git a/src/test/rpc/NoRippleCheck_test.cpp b/src/test/rpc/NoRippleCheck_test.cpp index 94c85a59ef..81ec097181 100644 --- a/src/test/rpc/NoRippleCheck_test.cpp +++ b/src/test/rpc/NoRippleCheck_test.cpp @@ -252,7 +252,7 @@ class NoRippleCheckLimits_test : public beast::unit_test::suite auto checkBalance = [&env]() { // this is endpoint drop prevention. Non admin ports will drop // requests if they are coming too fast, so we manipulate the - // resource manager here to reset the enpoint balance (for + // resource manager here to reset the endpoint balance (for // localhost) if we get too close to the drop limit. It would // be better if we could add this functionality to Env somehow // or otherwise disable endpoint charging for certain test From 9b53bd9871bf4e3ef8e19c58c827ef0a8361b4c2 Mon Sep 17 00:00:00 2001 From: Bart Date: Wed, 12 Nov 2025 09:30:45 -0500 Subject: [PATCH 18/19] ci: Clean workspace on Windows self-hosted runners (#6024) This change updates the `cleanup-workspace` action to its latest version, which added support for Windows. --- .github/workflows/reusable-build-test-config.yml | 6 +++--- .github/workflows/upload-conan-deps.yml | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 7032fa5a43..768897f665 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -69,9 +69,9 @@ jobs: ENABLED_VOIDSTAR: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }} ENABLED_COVERAGE: ${{ contains(inputs.cmake_args, '-Dcoverage=ON') }} steps: - - name: Cleanup workspace - if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e + - name: Cleanup workspace (macOS and Windows) + if: ${{ runner.os == 'macOS' || runner.os == 'Windows' }} + uses: XRPLF/actions/.github/actions/cleanup-workspace@01b244d2718865d427b499822fbd3f15e7197fcc - name: Checkout repository uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 357d756fa7..320396c899 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -62,9 +62,9 @@ jobs: runs-on: ${{ matrix.architecture.runner }} container: ${{ 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) || null }} steps: - - name: Cleanup workspace - if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e + - name: Cleanup workspace (macOS and Windows) + if: ${{ runner.os == 'macOS' || runner.os == 'Windows' }} + uses: XRPLF/actions/.github/actions/cleanup-workspace@01b244d2718865d427b499822fbd3f15e7197fcc - name: Checkout repository uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 From 8580a5795af647f4274d6210faaa3b96bbac7cd2 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 12 Nov 2025 18:55:49 +0000 Subject: [PATCH 19/19] chore: Set version 3.1.0-b0 (#5986) Technically b0 is not a release, so no "release" prefix here. It marks the point at which we moved the preceding release (3.0.0 in this case) from Beta to Release Candidate. --- src/libxrpl/protocol/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index af21ce5985..7690ae586b 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/src/libxrpl/protocol/BuildInfo.cpp @@ -17,7 +17,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "3.0.0-b1" +char const* const versionString = "3.1.0-b0" // clang-format on #if defined(DEBUG) || defined(SANITIZER)