diff --git a/.codecov.yml b/.codecov.yml index cd52e2604d..4268758e44 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -1,10 +1,32 @@ codecov: require_ci_to_pass: true + # The C++ and Rust uploads land minutes apart; without this gate Codecov + # publishes a near-zero total from whichever one arrives first. + notify: + after_n_builds: 2 + wait_for_ci: true comment: behavior: default layout: reach,diff,flags,tree,reach - show_carryforward_flags: false + show_carryforward_flags: true + after_n_builds: 2 + +# C++ and Rust coverage upload from independent workflows under the `cpp` and +# `rust` flags; carryforward keeps one language's total when only the other reran. +flag_management: + default_rules: + carryforward: true + individual_flags: + - name: cpp + carryforward: true + paths: + - include/ + - src/ + - name: rust + carryforward: true + paths: + - crates/ coverage: range: "70..85" diff --git a/.cspell.config.yaml b/.cspell.config.yaml index d0eedad94c..7b4a280c65 100644 --- a/.cspell.config.yaml +++ b/.cspell.config.yaml @@ -318,6 +318,7 @@ words: - summands - superpeer - superpeers + - Swatinem - takergets - takerpays - ters @@ -384,4 +385,5 @@ words: - xrplf - xxhash - xxhasher + - zstdio - CGNAT diff --git a/.github/actions/cargo-cache/action.yml b/.github/actions/cargo-cache/action.yml new file mode 100644 index 0000000000..f716d3e4a4 --- /dev/null +++ b/.github/actions/cargo-cache/action.yml @@ -0,0 +1,39 @@ +name: Use cargo artifacts cache +description: > + Cache the cargo build artifacts with rust-cache. Never caches ~/.cargo/bin: + when saving the cache, rust-cache deletes all binaries that were already + present there, which on persistent self-hosted runners wipes the tools + installed by prepare-runner. Harmless on ephemeral runners, but kept + consistent everywhere. + +inputs: + workspaces: + description: "Workspaces to cache, as 'workspace -> target' lines." + required: false + default: crates + key: + description: "Additional part of the cache key." + required: false + default: "" + cache-directories: + description: "Additional non-workspace directories to cache." + required: false + default: "" + save-if: + description: > + Condition for saving the cache after the job. Defaults to save only from develop branch + required: false + default: ${{ github.ref == 'refs/heads/develop' }} + +runs: + using: composite + + steps: + - name: Use cargo artifacts cache + uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + cache-bin: "false" + cache-directories: ${{ inputs.cache-directories }} + key: ${{ inputs.key }} + save-if: ${{ inputs.save-if }} + workspaces: ${{ inputs.workspaces }} diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 1ccbd61102..7361a3db63 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,6 +4,7 @@ updates: directories: - / - .github/actions/build-deps/ + - .github/actions/cargo-cache/ - .github/actions/release-info/ - .github/actions/set-compiler-env/ - .github/actions/setup-conan/ @@ -19,3 +20,19 @@ updates: github-actions: patterns: - "*" + + - package-ecosystem: cargo + directory: /crates + schedule: + interval: weekly + day: monday + time: "04:00" + timezone: Etc/GMT + commit-message: + prefix: "chore: [DEPENDABOT] " + target-branch: develop + open-pull-requests-limit: 10 + groups: + rust-dependencies: + patterns: + - "*" diff --git a/.github/scripts/rename/binary.sh b/.github/scripts/rename/binary.sh index 89d884538c..4a3e86675a 100755 --- a/.github/scripts/rename/binary.sh +++ b/.github/scripts/rename/binary.sh @@ -49,7 +49,7 @@ ${SED_COMMAND} -i -E 's@ripple/xrpld@XRPLF/rippled@g' BUILD.md ${SED_COMMAND} -i -E 's@XRPLF/xrpld@XRPLF/rippled@g' BUILD.md ${SED_COMMAND} -i -E 's@xrpld \(`xrpld`\)@xrpld@g' BUILD.md ${SED_COMMAND} -i -E 's@XRPLF/xrpld@XRPLF/rippled@g' CONTRIBUTING.md -${SED_COMMAND} -i -E 's@XRPLF/xrpld@XRPLF/rippled@g' docs/build/install.md +${SED_COMMAND} -i -E 's@XRPLF/xrpld@XRPLF/rippled@g' docs/install.md popd echo "Processing complete." diff --git a/.github/scripts/rename/docs.sh b/.github/scripts/rename/docs.sh index 9f080b06e5..9d7be209a3 100755 --- a/.github/scripts/rename/docs.sh +++ b/.github/scripts/rename/docs.sh @@ -77,8 +77,8 @@ ${SED_COMMAND} -i 's/Ripple integrators/XRPL developers/' README.md ${SED_COMMAND} -i 's/sanitizer-configuration-for-rippled/sanitizer-configuration-for-xrpld/' docs/build/sanitizers.md ${SED_COMMAND} -i 's/rippled/xrpld/g' .github/scripts/levelization/README.md ${SED_COMMAND} -i 's/rippled/xrpld/g' .github/scripts/strategy-matrix/generate.py -${SED_COMMAND} -i 's@/rippled@/xrpld@g' docs/build/install.md -${SED_COMMAND} -i 's@github.com/XRPLF/xrpld@github.com/XRPLF/rippled@g' docs/build/install.md +${SED_COMMAND} -i 's@/rippled@/xrpld@g' docs/install.md +${SED_COMMAND} -i 's@github.com/XRPLF/xrpld@github.com/XRPLF/rippled@g' docs/install.md ${SED_COMMAND} -i 's/rippled/xrpld/g' docs/Doxyfile ${SED_COMMAND} -i 's/ripple_basics/basics/' include/xrpl/basics/CountedObject.h ${SED_COMMAND} -i 's//tmp/cargo-audit-issue.md + + - name: Create issue + if: ${{ steps.audit.outcome != 'success' && github.event_name == 'schedule' }} + uses: XRPLF/actions/create-issue@2b8bc36af85b88bca0dd7bfac2e2dc05f94ad712 + with: + title: "cargo audit found vulnerabilities" + body_file: /tmp/cargo-audit-issue.md + labels: "Bug,Security" + + - name: Fail if advisories were found + if: ${{ steps.audit.outcome != 'success' }} + run: | + echo "cargo audit found advisories!" + cat /tmp/cargo-audit.txt + exit 1 diff --git a/.github/workflows/check-tools.yml b/.github/workflows/check-tools.yml index 99dddd7d96..1169140481 100644 --- a/.github/workflows/check-tools.yml +++ b/.github/workflows/check-tools.yml @@ -79,7 +79,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f + uses: XRPLF/actions/prepare-runner@7bf7ceca5932114abdd0d43493c3c30c5a654e13 with: enable_ccache: false diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index a8209ac16f..933c7b8a54 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -86,6 +86,7 @@ jobs: .github/workflows/reusable-check-autogen.yml .github/workflows/reusable-clang-tidy.yml .github/workflows/reusable-package.yml + .github/workflows/reusable-rust.yml .github/workflows/reusable-strategy-matrix.yml .github/workflows/reusable-test.yml .github/workflows/reusable-upload-recipe.yml @@ -97,6 +98,7 @@ jobs: cfg/** cmake/** conan/** + crates/** external/** include/** src/** @@ -173,6 +175,13 @@ jobs: secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + rust: + needs: should-run + if: ${{ needs.should-run.outputs.go == 'true' }} + uses: ./.github/workflows/reusable-rust.yml + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + package: needs: [should-run, build-test] # Packaging consumes the debian/rhel release binaries, which are only built @@ -216,6 +225,7 @@ jobs: - check-rename - clang-tidy - build-test + - rust - package - upload-recipe - notify-clio diff --git a/.github/workflows/on-tag.yml b/.github/workflows/on-tag.yml index 1c9fb414f2..d8a9a5113e 100644 --- a/.github/workflows/on-tag.yml +++ b/.github/workflows/on-tag.yml @@ -49,3 +49,4 @@ jobs: secrets: remote_username: ${{ secrets.NEXUS_REMOTE_USERNAME }} remote_password: ${{ secrets.NEXUS_REMOTE_PASSWORD }} + signing_key: ${{ secrets.NEXUS_PACKAGES_PRIVATE_KEY }} diff --git a/.github/workflows/on-trigger.yml b/.github/workflows/on-trigger.yml index 0d679318a9..2099f5f739 100644 --- a/.github/workflows/on-trigger.yml +++ b/.github/workflows/on-trigger.yml @@ -24,6 +24,7 @@ on: - ".github/workflows/reusable-check-autogen.yml" - ".github/workflows/reusable-clang-tidy.yml" - ".github/workflows/reusable-package.yml" + - ".github/workflows/reusable-rust.yml" - ".github/workflows/reusable-strategy-matrix.yml" - ".github/workflows/reusable-test.yml" - ".github/workflows/reusable-upload-recipe.yml" @@ -35,6 +36,7 @@ on: - "cfg/**" - "cmake/**" - "conan/**" + - "crates/**" - "external/**" - "include/**" - "src/**" @@ -101,6 +103,11 @@ jobs: secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + rust: + uses: ./.github/workflows/reusable-rust.yml + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + upload-recipe: needs: build-test # Only run when pushing to the develop branch. @@ -120,3 +127,4 @@ jobs: secrets: remote_username: ${{ secrets.NEXUS_REMOTE_USERNAME }} remote_password: ${{ secrets.NEXUS_REMOTE_PASSWORD }} + signing_key: ${{ secrets.NEXUS_PACKAGES_PRIVATE_KEY }} diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index ac5fe46722..905e910591 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -14,7 +14,7 @@ on: jobs: # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks. run-hooks: - uses: XRPLF/actions/.github/workflows/pre-commit.yml@3ba08d6ddf114092891d48491fc2e26c3ba15552 + uses: XRPLF/actions/.github/workflows/pre-commit.yml@f1952595d212e86169935135efc66294b4574131 with: runs_on: ubuntu-latest container: '{ "image": "ghcr.io/xrplf/xrpld/pre-commit:sha-f56b79f" }' diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index 6e973a251d..b8ca7751ab 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -47,7 +47,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f + uses: XRPLF/actions/prepare-runner@7bf7ceca5932114abdd0d43493c3c30c5a654e13 with: enable_ccache: false diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 94d0706e70..2846c3fb85 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -129,7 +129,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f + uses: XRPLF/actions/prepare-runner@7bf7ceca5932114abdd0d43493c3c30c5a654e13 with: enable_ccache: ${{ inputs.ccache_enabled }} @@ -162,6 +162,18 @@ jobs: with: compiler: ${{ inputs.compiler }} + - name: Use cargo artifacts cache + uses: ./.github/actions/cargo-cache + with: + cache-directories: ${{ env.BUILD_DIR }}/corrosion + key: ${{ inputs.config_name }} + # two workspaces here because build artifacts are located in 2 places: + # - crates/target when cargo is called directly + # - build/cargo when cargo is called by cmake + workspaces: | + crates + crates -> ${{ runner.os == 'Windows' && format('../{0}/x64/{1}/cargo', env.BUILD_DIR, inputs.build_type) || format('../{0}/cargo', env.BUILD_DIR) }} + # `setup-nix-env` already did this for the Nix toolchain. - name: Setup Conan if: ${{ inputs.toolchain != 'nix' }} @@ -357,6 +369,11 @@ jobs: LD_PRELOAD="$PRELOAD" ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee "${GITHUB_WORKSPACE}/unittest.log" + - name: Run Rust tests + if: ${{ !inputs.build_only }} + working-directory: crates + run: cargo nextest run --workspace --all-features --locked --no-tests=warn + # Smoke-run every benchmark module with a single repetition to confirm the # benchmarks still build and execute. This is a correctness check, not a # performance measurement, so there is nothing to gain from repeating it @@ -428,6 +445,7 @@ jobs: disable_telem: true fail_ci_if_error: true files: ${{ env.BUILD_DIR }}/coverage.xml + flags: cpp plugins: noop token: ${{ secrets.CODECOV_TOKEN }} verbose: true diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 2049b1ce55..ac21c83ea0 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -27,7 +27,7 @@ jobs: determine-files: permissions: contents: read - uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@d041ac9f1fa9f07a4ba335eb4c1c82233fb3fef6 + uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@70145243b905dc3e040a61d39c00e178cfb96f71 run-clang-tidy: name: Run clang tidy @@ -43,7 +43,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f + uses: XRPLF/actions/prepare-runner@7bf7ceca5932114abdd0d43493c3c30c5a654e13 with: enable_ccache: false @@ -59,6 +59,12 @@ jobs: with: compiler: ${{ env.COMPILER }} + - name: Use cargo artifacts cache + uses: ./.github/actions/cargo-cache + with: + cache-directories: ${{ env.BUILD_DIR }}/corrosion + workspaces: crates -> ../${{ env.BUILD_DIR }}/cargo + - name: Setup Conan uses: ./.github/actions/setup-conan @@ -80,13 +86,13 @@ jobs: -Dwerr=ON \ -Dxrpld=ON \ -Dverify_headers=ON \ + -Drust=ON \ .. - # clang-tidy needs headers generated from proto files - - name: Build libxrpl.libpb + - name: Build clang-tidy prerequisites working-directory: ${{ env.BUILD_DIR }} run: | - ninja -j ${{ steps.nproc.outputs.nproc }} xrpl.libpb + ninja -j ${{ steps.nproc.outputs.nproc }} tidy_prerequisites - name: Run clang tidy id: run_clang_tidy diff --git a/.github/workflows/reusable-package.yml b/.github/workflows/reusable-package.yml index 430072b627..cfae706ee1 100644 --- a/.github/workflows/reusable-package.yml +++ b/.github/workflows/reusable-package.yml @@ -29,6 +29,9 @@ on: remote_password: description: "The password or token for that Nexus account." required: false + signing_key: + description: "Armoured PGP private key used to sign the RPMs. Required when publishing." + required: false defaults: run: @@ -98,6 +101,14 @@ jobs: PKG_CHANNEL: ${{ steps.release_info.outputs.channel }} run: ./package/build_pkg.sh + # Before the upload, so the artifact and the published package are the + # same bytes. DEBs are not signed, so the key is never set on that job. + - name: Sign RPM + if: ${{ inputs.publish && matrix.distro == 'rhel' }} + env: + PKG_SIGNING_KEY: ${{ secrets.signing_key }} + run: ./package/sign_rpm.sh "${BUILD_DIR}" + - name: Upload package artifact uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: diff --git a/.github/workflows/reusable-rust.yml b/.github/workflows/reusable-rust.yml new file mode 100644 index 0000000000..83301f97ad --- /dev/null +++ b/.github/workflows/reusable-rust.yml @@ -0,0 +1,80 @@ +# Clippy, coverage and documentation for the Rust crates in crates/. Each runs +# as an independent job on a GitHub-hosted runner, but inside the same container +# image used to build the crates in the C++/Corrosion path, so the toolchain +# (and therefore the lints, coverage instrumentation and the cargo cache) matches +# what production builds use. +# +# Rust unit tests are deliberately NOT run here. They run as part of the C++ +# build (reusable-build-test-config.yml), which already compiles the crates on a +# self-hosted runner, so there is no need to provision a toolchain again. +name: Rust + +on: + workflow_call: + secrets: + CODECOV_TOKEN: + description: "The Codecov token to use for uploading coverage reports." + required: true + +defaults: + run: + shell: bash + working-directory: crates + +permissions: + contents: read + +jobs: + clippy: + runs-on: ubuntu-latest + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-a0074f8 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Use cargo artifacts cache + uses: ./.github/actions/cargo-cache + + - name: Run clippy + run: cargo clippy --workspace --all-targets --all-features --locked -- -D warnings + + coverage: + runs-on: ubuntu-latest + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-a0074f8 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Use cargo artifacts cache + uses: ./.github/actions/cargo-cache + + - name: Generate coverage report + run: cargo llvm-cov nextest --workspace --all-features --locked --no-tests=warn --lcov --output-path lcov.info + + - name: Upload coverage report + if: ${{ github.repository == 'XRPLF/rippled' }} + uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 + with: + disable_search: true + disable_telem: true + fail_ci_if_error: true + files: crates/lcov.info + flags: rust + plugins: noop + token: ${{ secrets.CODECOV_TOKEN }} + verbose: true + + doc: + runs-on: ubuntu-latest + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-a0074f8 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Use cargo artifacts cache + uses: ./.github/actions/cargo-cache + + - name: Build documentation + env: + RUSTDOCFLAGS: "-D warnings" + run: cargo doc --workspace --no-deps --all-features --locked diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 65a3f9c5b6..184f13cc5e 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -68,7 +68,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f + uses: XRPLF/actions/prepare-runner@7bf7ceca5932114abdd0d43493c3c30c5a654e13 with: enable_ccache: false diff --git a/.gitignore b/.gitignore index 13b59a7e2c..c5af8eb7b4 100644 --- a/.gitignore +++ b/.gitignore @@ -89,3 +89,6 @@ target/ # clangd cache /.cache + +# Rust build directory +crates/target diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d339cb29ed..e5e69759fd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -62,6 +62,15 @@ repos: types_or: [c++, c, proto] exclude: ^include/xrpl/protocol_autogen/(transactions|ledger_entries)/ + - repo: local + hooks: + - id: cargo-fmt + name: cargo fmt + entry: cargo fmt --manifest-path crates/Cargo.toml --all + language: system + types: [rust] + pass_filenames: false # rustfmt formats the whole workspace + - repo: https://github.com/BlankSpruce/gersemi-pre-commit rev: e98930bdc210d3387007f9252d8c1694ea7e410f # frozen: 0.27.7 hooks: diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index c853cfb07c..d521f9c024 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -54,6 +54,9 @@ This section contains changes targeting a future version. - `submit`: The `fail_hard` field now returns an error if the value is not a boolean. [#6529](https://github.com/XRPLF/rippled/pull/6529) - `subscribe`: The `taker` field in the `books` array now returns `actMalformed` instead of `badIssuer` if the value is not a valid account. [#6529](https://github.com/XRPLF/rippled/pull/6529) - Fixed a bug in `Forwarded` HTTP header parsing where the extracted IP address could be incorrect when no comma or semicolon delimiter follows the address. This could cause the server to misidentify a client's IP address when operating behind a reverse proxy. [#6529](https://github.com/XRPLF/rippled/pull/6529) +- `vault_info`: Errors now identify what the request got wrong instead of reporting every failure as the unregistered token `malformedRequest`, and the `error`, `error_code` and `error_message` fields now agree with each other. An invalid `vault_id` or `seq` returns `invalidParams`, an invalid `owner` returns `actMalformed`, and a request that mixes `vault_id` with `owner`/`seq` or supplies neither returns `invalidParams` with a message naming the accepted combinations. [#8015](https://github.com/XRPLF/rippled/pull/8015) +- `vault_info`: A well-formed all-zero `vault_id` now returns `entryNotFound` instead of being rejected as malformed, and `entryNotFound` responses now include `error_code` and `error_message`. Clients that request `ripplerpc` 3.0 or above therefore receive HTTP 400 with that error rather than HTTP 200. [#8015](https://github.com/XRPLF/rippled/pull/8015) +- `vault_info`: `vault_id` and `owner` must now be strings, matching how `ledger_entry` reads the same fields. An object or an array in either field previously produced an internal error, and a number was silently converted to its decimal text; `vault_id` now returns `invalidParams` and `owner` returns `actMalformed`. [#8015](https://github.com/XRPLF/rippled/pull/8015) - `gateway_balances`: The `account` and `ident` fields now return an `invalidParams` error if the value is not a string, instead of an `internal` error. [#7655](https://github.com/XRPLF/rippled/pull/7655) - `account_lines`: The `peer` field now returns an error if the value is not a string. [#7728](https://github.com/XRPLF/rippled/pull/7728) diff --git a/BUILD.md b/BUILD.md index ae2e69bb97..e98d204d0b 100644 --- a/BUILD.md +++ b/BUILD.md @@ -304,6 +304,7 @@ See [Sanitizers docs](./docs/build/sanitizers.md) for more details. | ---------------- | ------------- | ----------------------------------------------------------------------------- | | `assert` | OFF | Force enabling assertions. | | `coverage` | OFF | Prepare the coverage report. | +| `rust` | OFF | Build the Rust crates and the C++ code that depends on them. | | `tests` | OFF | Build tests. | | `unity` | OFF | Configure a unity build. | | `verify_headers` | ON | Make the `verify-headers` target available to compile each header on its own. | @@ -316,6 +317,30 @@ memory) since they concatenate sources into fewer translation units. Non-unity builds may be faster for incremental builds, and can be helpful for detecting `#include` omissions. +### Rust crates + +The Rust crates in `crates/` are only part of the build when `rust` is ON. With +`-Drust=OFF` (the default) the `crates` directory is not added to the build, no +cxxbridge bindings are generated, and the C++ tests that exercise the Rust +interop are not compiled — so no Rust toolchain is needed. CI builds always pass +`-Drust=ON`. + +With `-Drust=ON` you need one extra dependency: a Rust toolchain (`cargo`, +`rustc`) matching the channel pinned in +[`rust-toolchain.toml`](./rust-toolchain.toml), which compiles the crates and +generates the cxxbridge bindings. It is provided by the +[Nix development shell](./docs/build/nix.md), so `-Drust=ON` works there without +any extra setup; otherwise install it as described in +[Rust](./docs/build/environment.md#rust). + +The crates also have their own Rust unit tests. Those are run with `cargo` and +need only the Rust toolchain, independently of CMake and of the `rust` option +(CI runs them with `cargo nextest`): + +```bash +cargo test --manifest-path crates/Cargo.toml --workspace +``` + ### Verifying headers The regular build only compiles `.cpp` files, so a header is only ever checked diff --git a/CMakeLists.txt b/CMakeLists.txt index efe7396661..a324cecedc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -158,7 +158,13 @@ if(coverage) include(XrplCov) endif() +add_custom_target(tidy_prerequisites) + +if(rust) + add_subdirectory(crates) +endif() include(XrplCore) + include(XrplProtocolAutogen) include(XrplInstall) include(XrplValidatorKeys) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc385cf6ed..35309a9824 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -225,8 +225,9 @@ environment, so you don't need to install most of the individual tools yourself. The version of each hook sourced from an external repository (`clang-format`, `gersemi`, etc.) is pinned in that file, so running the hooks locally uses exactly the same versions as CI. A few `local` hooks — most notably -`clang-tidy` — run tools from your own environment; see -[Installing clang-tidy](#installing-clang-tidy) for how to get those. +`clang-tidy` and `cargo fmt` — run tools from your own environment; see +[Installing clang-tidy](#installing-clang-tidy) and +[Rust](./docs/build/environment.md#rust) for how to get those. To get started, install `pre-commit` and enable the git hook scripts: @@ -255,6 +256,7 @@ The hooks configured in this repository include, among others: - `clang-tidy` — C++ static analysis (see [Clang-tidy](#clang-tidy)); opt in with `TIDY=1` - `fix-include-style`, `fix-pragma-once`, `check-doxygen-style` — C++ hygiene - `gersemi` — CMake formatting +- `cargo fmt` — Rust formatting for the crates in `crates/` - `prettier`, `black`, `shfmt` — formatting for JavaScript/JSON/Markdown, Python, and shell - `cspell` — spell checking @@ -319,7 +321,11 @@ See the [environment setup guide](./docs/build/environment.md#clang-tidy) for ho ### Running clang-tidy locally -Before running clang-tidy, you must build the project to generate required files (particularly protobuf headers). Refer to [`BUILD.md`](./BUILD.md) for build instructions. +Before running clang-tidy, you must generate the files it depends on (protobuf headers, and, when the project is configured with `-Drust=ON`, the cxxbridge headers from the Rust crates). Configure the project as described in [`BUILD.md`](./BUILD.md), then build the `tidy_prerequisites` target, which generates all of them: + +```bash +cmake --build build --target tidy_prerequisites +``` #### Via pre-commit (recommended) diff --git a/README.md b/README.md index 88c7943ebb..a0d30ef68b 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ Here are some good places to start learning the source code: | `./docs` | Source documentation files and doxygen config. | | `./cfg` | Example configuration files. | | `./src` | Source code. | +| `./crates` | Rust source code. | Some of the directories under `src` are external repositories included using git-subtree. See those directories' README files for more details. diff --git a/cmake/XrplCompiler.cmake b/cmake/XrplCompiler.cmake index 2b46739d97..29c1dfe478 100644 --- a/cmake/XrplCompiler.cmake +++ b/cmake/XrplCompiler.cmake @@ -120,7 +120,10 @@ if(MSVC) _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS $<$,$>:_CRTDBG_MAP_ALLOC> ) - target_link_libraries(common INTERFACE -errorreport:none -machine:X64) + target_link_libraries( + common + INTERFACE -errorreport:none -machine:X64 -ignore:4099 + ) else() target_compile_options( common diff --git a/cmake/XrplCore.cmake b/cmake/XrplCore.cmake index a3e08145d5..f3951d4eac 100644 --- a/cmake/XrplCore.cmake +++ b/cmake/XrplCore.cmake @@ -51,6 +51,8 @@ target_compile_options( target_link_libraries(xrpl.libpb PUBLIC protobuf::libprotobuf gRPC::grpc++) +add_dependencies(tidy_prerequisites xrpl.libpb) + # TODO: Clean up the number of library targets later. add_library(xrpl.imports.main INTERFACE) diff --git a/cmake/XrplSettings.cmake b/cmake/XrplSettings.cmake index be9bf1fda2..58b902baa1 100644 --- a/cmake/XrplSettings.cmake +++ b/cmake/XrplSettings.cmake @@ -32,6 +32,11 @@ endif() option(benchmark "Build benchmarks" ON) +# When OFF, the crates directory is not added to the build at all: no Rust +# toolchain is required, no cxxbridge bindings are generated, and the C++ tests +# that consume those bindings are left out of the build tree. +option(rust "Build the Rust crates and the C++ code that depends on them" OFF) + # Enabled by default so every header is compiled on its own as the main file of # its own compile_commands.json entry - this is what lets clang-tidy (and clangd # and IDEs) analyse a header's own includes directly. The per-header objects are diff --git a/conan.lock b/conan.lock index 5b01ffbf76..176f0b27cb 100644 --- a/conan.lock +++ b/conan.lock @@ -23,6 +23,7 @@ "fast_float/8.2.10#f6f28d6bb22112078e7dbda611caf681%1782494504.298", "ed25519/2015.03#ae761bdc52730a843f0809bdf6c1b1f6%1782307148.15562", "date/3.0.4#862e11e80030356b53c2c38599ceb32b%1782392402.538492", + "corrosion/0.6.1#bfa292df0a957bc70a450ff316cd9435%1786119416.131296", "c-ares/1.34.6#545240bb1c40e2cacd4362d6b8967650%1782392402.681654", "bzip2/1.0.8#c470882369c2d95c5c77e970c0c7e321%1782392402.296732", "boost/1.91.0#ea540ca2133d831b560036aa24dece3c%1782392419.475605", diff --git a/conanfile.py b/conanfile.py index 2742405b6c..0683a3779f 100644 --- a/conanfile.py +++ b/conanfile.py @@ -28,6 +28,7 @@ class Xrpl(ConanFile): } requires = [ + "corrosion/0.6.1", "ed25519/2015.03", "fast_float/8.2.10", "grpc/1.81.1", diff --git a/crates/.cargo/config.toml b/crates/.cargo/config.toml new file mode 100644 index 0000000000..fc29aa80f7 --- /dev/null +++ b/crates/.cargo/config.toml @@ -0,0 +1,17 @@ +# The Rust static libraries are linked into C++ targets, so the runtime linkage +# here has to match what the C++ build uses (see cmake/XrplCompiler.cmake). +# +# macOS needs nothing: AppleClang cannot link libgcc/libc++ statically, so the +# C++ build skips those flags on Apple as well. + +# Both amd64 and arm64 Linux builds link libgcc statically. This only affects +# links that rustc itself drives (`cargo test` binaries and the like) — the +# `staticlib` crates consumed by CMake are archived, not linked, so rustc +# silently ignores link args for them. Keeping libgcc_s.so.1 off the xrpld link +# line is handled in crates/CMakeLists.txt instead. +[target.'cfg(target_os = "linux")'] +rustflags = ["-C", "link-args=-static-libgcc"] + +# Windows builds use the static MSVC runtime. +[target.'cfg(windows)'] +rustflags = ["-C", "target-feature=+crt-static"] diff --git a/crates/CMakeLists.txt b/crates/CMakeLists.txt new file mode 100644 index 0000000000..3f83045cdb --- /dev/null +++ b/crates/CMakeLists.txt @@ -0,0 +1,104 @@ +find_package(Corrosion REQUIRED) + +corrosion_import_crate(MANIFEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/Cargo.toml) + +# The generated C++ lands in the build tree, so put a .clang-tidy next to it to +# keep clang-tidy from analyzing code we don't own. +configure_file( + generated.clang-tidy + "${CMAKE_CURRENT_BINARY_DIR}/.clang-tidy" + COPYONLY +) + +add_custom_target(xrpl_crates) +add_dependencies(tidy_prerequisites xrpl_crates) + +# On macOS, ld warns `ignoring duplicate libraries` when linking a crate. +# Corrosion is the source of both duplicates it names: +# +# * The crate archive and its cxxbridge archive, because +# `corrosion_add_cxxbridge` makes the two depend on each other, and CMake +# repeats a static library cycle on the link line so single-pass linkers can +# resolve it. (LINK_INTERFACE_MULTIPLICITY can only raise that count.) +# * `-lSystem`, which Corrosion copies from rustc's `native-static-libs` even +# though the compiler driver always links libSystem. +# +# ld needs neither: it resolves the cycle from one copy of each archive and +# links libSystem once. So silence the warning rather than rewrite Corrosion's +# link interface, which the cycle is also part of. The option itself is old — +# Xcode 15 is only where the warning became the default — and the check below +# leaves it out on a linker that does not know it. +if(is_macos) + include(CheckLinkerFlag) + check_linker_flag( + CXX + -Wl,-no_warn_duplicate_libraries + have_no_warn_duplicate_libraries + ) +endif() + +function(_unlink_libgcc_s crate) + if(NOT (is_linux AND static)) + return() + endif() + + # Corrosion exposes a crate's staticlib as an imported `-static` + # target and puts the native libs in its INTERFACE_LINK_LIBRARIES. If either + # of those changes, warn instead of silently letting libgcc_s.so.1 return. + set(imported "${crate}-static") + if(NOT TARGET ${imported}) + message( + FATAL_ERROR + "Corrosion did not create the imported target '${imported}', so " + "libgcc_s cannot be removed from the link interface of '${crate}'. " + "xrpld will link libgcc_s.so.1 dynamically. Check where Corrosion " + "${CORROSION_VERSION} now records `native-static-libs`." + ) + return() + endif() + + get_target_property(libs ${imported} INTERFACE_LINK_LIBRARIES) + if(NOT "gcc_s" IN_LIST libs) + message( + WARNING + "'gcc_s' was not in the link interface of '${imported}' as " + "expected. If the Rust toolchain stopped reporting it this " + "workaround is obsolete and can be deleted; otherwise xrpld may " + "link libgcc_s.so.1 dynamically. Verify with: " + "objdump -p xrpld | grep NEEDED" + ) + return() + endif() + + list(REMOVE_ITEM libs gcc_s) + set_property(TARGET ${imported} PROPERTY INTERFACE_LINK_LIBRARIES ${libs}) +endfunction() + +function(add_xrpl_crate name) + cmake_parse_arguments(ARG "" "CRATE" "FILES" ${ARGN}) + _unlink_libgcc_s(${ARG_CRATE}) + # `cc` picks its runtime flag from `crt-static` alone, so it compiles a + # crate's C++ with `-MT`; Debug needs `-MTd` (to match cmake/XrplCompiler.cmake). + if(is_msvc) + corrosion_set_env_vars( + ${ARG_CRATE} + "$<$:CXXFLAGS=-MTd>" + ) + endif() + corrosion_add_cxxbridge(${name}_cxxbridge CRATE ${ARG_CRATE} FILES + ${ARG_FILES} + ) + # Generated cxxbridge headers don't exist at configure time; CMake 3.28+ + # validates INTERFACE_SOURCES on consuming targets. Clear it to skip the + # existence check — build-time ordering is enforced by the custom commands. + set_target_properties(${name}_cxxbridge PROPERTIES INTERFACE_SOURCES "") + if(have_no_warn_duplicate_libraries) + target_link_options( + ${name}_cxxbridge + INTERFACE -Wl,-no_warn_duplicate_libraries + ) + endif() + add_dependencies(xrpl_crates ${name}_cxxbridge) +endfunction() + +add_xrpl_crate(rs_hello_world CRATE rs_hello_world FILES lib.rs) diff --git a/crates/Cargo.lock b/crates/Cargo.lock new file mode 100644 index 0000000000..70247f8e19 --- /dev/null +++ b/crates/Cargo.lock @@ -0,0 +1,301 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "anstyle" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" + +[[package]] +name = "cc" +version = "1.2.61" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d16d90359e986641506914ba71350897565610e87ce0ad9e6f28569db3dd5c6d" +dependencies = [ + "find-msvc-tools", + "shlex", +] + +[[package]] +name = "clap" +version = "4.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ddb117e43bbf7dacf0a4190fef4d345b9bad68dfc649cb349e7d17d28428e51" +dependencies = [ + "clap_builder", +] + +[[package]] +name = "clap_builder" +version = "4.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "714a53001bf66416adb0e2ef5ac857140e7dc3a0c48fb28b2f10762fc4b5069f" +dependencies = [ + "anstyle", + "clap_lex", + "strsim", +] + +[[package]] +name = "clap_lex" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" + +[[package]] +name = "codespan-reporting" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af491d569909a7e4dee0ad7db7f5341fef5c614d5b8ec8cf765732aba3cff681" +dependencies = [ + "serde", + "termcolor", + "unicode-width", +] + +[[package]] +name = "cxx" +version = "1.0.199" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "824894a4a85dca76d4c95c2b9098c036f5a29f627b30c12780774f6654e60974" +dependencies = [ + "cc", + "cxx-build", + "cxxbridge-cmd", + "cxxbridge-flags", + "cxxbridge-macro", + "foldhash", + "link-cplusplus", +] + +[[package]] +name = "cxx-build" +version = "1.0.199" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1ae0b651ea5b0000b19513aef5a03f194d7e3486f2d9258b658da8677fe9036" +dependencies = [ + "cc", + "codespan-reporting", + "indexmap", + "proc-macro2", + "quote", + "scratch", + "syn 3.0.3", +] + +[[package]] +name = "cxxbridge-cmd" +version = "1.0.199" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb05f91d3fb8435d9bab6ac5ce6ac1868be774325fb7fb2a91be39393b21388e" +dependencies = [ + "clap", + "codespan-reporting", + "indexmap", + "proc-macro2", + "quote", + "syn 3.0.3", +] + +[[package]] +name = "cxxbridge-flags" +version = "1.0.199" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf293202e0e3e98495785745389e8d0755b217e66f19194a5c695c25e03282ef" + +[[package]] +name = "cxxbridge-macro" +version = "1.0.199" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca001d746947c7249ed9d332a10f7a59daedbafeb0ec68c5c18a7db7a93f6ccc" +dependencies = [ + "indexmap", + "proc-macro2", + "quote", + "syn 3.0.3", +] + +[[package]] +name = "equivalent" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" + +[[package]] +name = "find-msvc-tools" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" + +[[package]] +name = "foldhash" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" + +[[package]] +name = "hashbrown" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f467dd6dccf739c208452f8014c75c18bb8301b050ad1cfb27153803edb0f51" + +[[package]] +name = "indexmap" +version = "2.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d466e9454f08e4a911e14806c24e16fba1b4c121d1ea474396f396069cf949d9" +dependencies = [ + "equivalent", + "hashbrown", +] + +[[package]] +name = "link-cplusplus" +version = "1.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f78c730aaa7d0b9336a299029ea49f9ee53b0ed06e9202e8cb7db9bae7b8c82" +dependencies = [ + "cc", +] + +[[package]] +name = "proc-macro2" +version = "1.0.106" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41f2619966050689382d2b44f664f4bc593e129785a36d6ee376ddf37259b924" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rs-hello_world" +version = "0.1.0" +dependencies = [ + "cxx", +] + +[[package]] +name = "scratch" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d68f2ec51b097e4c1a75b681a8bec621909b5e91f15bb7b840c4f2f7b01148b2" + +[[package]] +name = "serde" +version = "1.0.228" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" +dependencies = [ + "serde_core", + "serde_derive", +] + +[[package]] +name = "serde_core" +version = "1.0.228" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.228" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + +[[package]] +name = "shlex" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" + +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + +[[package]] +name = "syn" +version = "2.0.117" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "termcolor" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +dependencies = [ + "winapi-util", +] + +[[package]] +name = "unicode-ident" +version = "1.0.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" + +[[package]] +name = "unicode-width" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" + +[[package]] +name = "winapi-util" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "windows-link" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" + +[[package]] +name = "windows-sys" +version = "0.61.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" +dependencies = [ + "windows-link", +] diff --git a/crates/Cargo.toml b/crates/Cargo.toml new file mode 100644 index 0000000000..0bb0e9c550 --- /dev/null +++ b/crates/Cargo.toml @@ -0,0 +1,15 @@ +[workspace] +members = ["hello_world"] +resolver = "3" + +[workspace.dependencies] +cxx = { version = "1.0.198", features = ["c++20"] } + +[workspace.package] +edition = "2024" + +[profile.release] +opt-level = 3 +overflow-checks = true +lto = true +debug = true diff --git a/crates/generated.clang-tidy b/crates/generated.clang-tidy new file mode 100644 index 0000000000..8e2202d44a --- /dev/null +++ b/crates/generated.clang-tidy @@ -0,0 +1,10 @@ +--- +# Neutralizes clang-tidy for the corrosion/cxxbridge-generated C++. Copied into +# the crates build directory by crates/CMakeLists.txt, next to the generated +# sources, so clang-tidy picks it up instead of the top-level configuration. +# +# One check is kept enabled to avoid clang-tidy's "no checks enabled" error. +Checks: "-*,google-readability-todo" +WarningsAsErrors: "" +HeaderFilterRegex: "" +InheritParentConfig: false diff --git a/crates/hello_world/Cargo.toml b/crates/hello_world/Cargo.toml new file mode 100644 index 0000000000..2e5a329c9a --- /dev/null +++ b/crates/hello_world/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "rs-hello_world" +version = "0.1.0" +edition.workspace = true + +[lib] +crate-type = ["staticlib"] + +[dependencies] +cxx.workspace = true diff --git a/crates/hello_world/src/lib.rs b/crates/hello_world/src/lib.rs new file mode 100644 index 0000000000..b1cb121fa0 --- /dev/null +++ b/crates/hello_world/src/lib.rs @@ -0,0 +1,10 @@ +#[cxx::bridge(namespace = "rs::hello_world")] +mod ffi { + extern "Rust" { + fn hello_world() -> String; + } +} + +pub fn hello_world() -> String { + "hello_world".to_string() +} diff --git a/docs/build/environment.md b/docs/build/environment.md index 5616f32f37..51580b12a5 100644 --- a/docs/build/environment.md +++ b/docs/build/environment.md @@ -46,6 +46,9 @@ Besides a compiler, building `xrpld` requires: On Linux and macOS, the [Nix development shell](./nix.md) provides all of them (see below). On Windows they have to be installed manually. +Building with `-Drust=ON` additionally requires a Rust toolchain, see +[Rust](#rust). A default build does not, so it is not in the table above. + Once they are in place, verify that everything is installed and runnable with: ```bash @@ -121,6 +124,25 @@ manually: - [Git for Windows](https://git-scm.com/download/win) - Python, Conan, and CMake, at the versions listed in [Required tools](#required-tools). +- a [Rust toolchain](https://rustup.rs) — only needed to build with + `-Drust=ON`, see [Rust](#rust) + +## Rust + +The repository contains a Rust workspace in [`crates/`](../../crates), whose +crates are exposed to C++ through [cxx](https://cxx.rs) bindings. It is **not** +part of a default build: the CMake `rust` option is OFF by default, and with it +off no Rust toolchain is needed. It is only required when configuring with +`-Drust=ON` (which is what CI does), see [Options](../../BUILD.md#options). + +The toolchain (`cargo`, `rustc`) is pinned to the channel in +[`rust-toolchain.toml`](../../rust-toolchain.toml) at the repository root. If +you install Rust with [rustup](https://rustup.rs), that file is picked up +automatically, and `cargo`/`rustc` in the repository will use the pinned +version. + +Everything else the Rust build needs on the CMake side comes from Conan along +with the rest of the dependencies, so there is nothing further to install. ## Clang-tidy diff --git a/docs/build/nix.md b/docs/build/nix.md index 4c082afb28..0b701b39f3 100644 --- a/docs/build/nix.md +++ b/docs/build/nix.md @@ -128,6 +128,12 @@ Coverage builds (`-Dcoverage=ON`) work in the `gcc` shell (and `gcc-plain` on Li each ships a `gcov` matching its compiler, since Nix's cc-wrapper does not expose one. The `clang` shells do not include `llvm-cov`, so use a `gcc` shell for coverage. +Builds of the Rust crates (`-Drust=ON`) also work out of the box: every shell +provides the Rust toolchain pinned in +[`rust-toolchain.toml`](../../rust-toolchain.toml) (see +[Rust](./environment.md#rust)), plus the `cargo-audit`, `cargo-llvm-cov` and +`cargo-nextest` plugins. + ## Conan configuration The shell runs [`conan/init.sh`](../../conan/init.sh) on entry, so diff --git a/docs/build/install.md b/docs/install-legacy.md similarity index 87% rename from docs/build/install.md rename to docs/install-legacy.md index d3ce1e9d87..0a6800f17f 100644 --- a/docs/build/install.md +++ b/docs/install-legacy.md @@ -1,3 +1,10 @@ +# Installing xrpld 3.3.0 and earlier + +> [!IMPORTANT] +> These instructions apply to xrpld 3.3.0 and earlier, published to +> repos.ripple.com. +> For later releases see [install.md](./install.md). + This document contains instructions for installing xrpld. The APT package manager is common on Debian-based Linux distributions like Ubuntu, @@ -52,7 +59,7 @@ The default [prefix][1] is typically `/usr/local` on Linux and macOS and 5. Add the appropriate XRPL repository for your operating system version: - echo "deb [signed-by=/usr/local/share/keyrings/ripple-key.gpg] https://repos.ripple.com/repos/xrpld-deb focal stable" | \ + echo "deb [signed-by=/usr/local/share/keyrings/ripple-key.gpg] https://repos.ripple.com/repos/rippled-deb focal stable" | \ sudo tee -a /etc/apt/sources.list.d/ripple.list The above example is appropriate for **Ubuntu 20.04 Focal Fossa**. For other operating systems, replace the word `focal` with one of the following: @@ -106,8 +113,8 @@ The default [prefix][1] is typically `/usr/local` on Linux and macOS and enabled=1 gpgcheck=0 repo_gpgcheck=1 - baseurl=https://repos.ripple.com/repos/xrpld-rpm/stable/ - gpgkey=https://repos.ripple.com/repos/xrpld-rpm/stable/repodata/repomd.xml.key + baseurl=https://repos.ripple.com/repos/rippled-rpm/stable/ + gpgkey=https://repos.ripple.com/repos/rippled-rpm/stable/repodata/repomd.xml.key REPOFILE _Unstable_ @@ -118,8 +125,8 @@ The default [prefix][1] is typically `/usr/local` on Linux and macOS and enabled=1 gpgcheck=0 repo_gpgcheck=1 - baseurl=https://repos.ripple.com/repos/xrpld-rpm/unstable/ - gpgkey=https://repos.ripple.com/repos/xrpld-rpm/unstable/repodata/repomd.xml.key + baseurl=https://repos.ripple.com/repos/rippled-rpm/unstable/ + gpgkey=https://repos.ripple.com/repos/rippled-rpm/unstable/repodata/repomd.xml.key REPOFILE _Nightly_ @@ -130,8 +137,8 @@ The default [prefix][1] is typically `/usr/local` on Linux and macOS and enabled=1 gpgcheck=0 repo_gpgcheck=1 - baseurl=https://repos.ripple.com/repos/xrpld-rpm/nightly/ - gpgkey=https://repos.ripple.com/repos/xrpld-rpm/nightly/repodata/repomd.xml.key + baseurl=https://repos.ripple.com/repos/rippled-rpm/nightly/ + gpgkey=https://repos.ripple.com/repos/rippled-rpm/nightly/repodata/repomd.xml.key REPOFILE 2. Fetch the latest repo updates: diff --git a/docs/install.md b/docs/install.md new file mode 100644 index 0000000000..a3e2fefa02 --- /dev/null +++ b/docs/install.md @@ -0,0 +1,144 @@ +# Installing xrpld + +> [!NOTE] +> These instructions apply to packages published from 2026-08-19 onwards. +> For xrpld 3.3.0 and earlier see [install-legacy.md](./install-legacy.md). + +`xrpld` is published as DEB and RPM packages for 64-bit x86 Linux. +Use APT on Debian-based distributions such as Debian and Ubuntu, +and YUM on Red Hat-based distributions such as RHEL, AlmaLinux, and Rocky Linux. +To build from source instead, see [BUILD.md](../BUILD.md). + +## Release channels + +Packages are published to four channels: + +- `stable` - the latest production release +- `unstable` - release candidates +- `experimental` - beta builds +- `develop` - every push to the [`develop` branch](https://github.com/XRPLF/rippled/tree/develop) + +See [Publishing packages](../package/README.md#publishing-packages) for how channels are produced. + +The instructions below use `stable`. +To follow another channel, replace `stable` with its name +wherever it appears in the repository configuration. + +> [!WARNING] +> Channels other than `stable` may be broken at any time. +> Do not use them for production servers. + +## Install the xrpld package + +### With the APT package manager + +1. Install utilities: + + ```bash + sudo apt update -y + sudo apt install -y apt-transport-https ca-certificates curl gnupg + ``` + +2. Add the XRPL Foundation package-signing key to your list of trusted keys: + + ```bash + sudo install -d -m 0755 /etc/apt/keyrings + sudo curl -fsS https://packages.xrplf.org/xrplf.asc -o /etc/apt/keyrings/xrplf.asc + ``` + +3. Check the fingerprint of the newly-added key: + + ```bash + gpg --show-keys /etc/apt/keyrings/xrplf.asc + ``` + + The output should be: + + ```text + pub rsa4096 2026-08-18 [SC] + B655416741221F780FBCFBC9AA84D41A11D29FA9 + uid XRPLF Packages + ``` + + In particular, make sure that the fingerprint matches. + +4. Add the repository, using the channel you picked in [Release channels](#release-channels): + + ```bash + echo "deb [signed-by=/etc/apt/keyrings/xrplf.asc] https://packages.xrplf.org/repository/deb-stable any main" | \ + sudo tee /etc/apt/sources.list.d/xrplf.list + ``` + +5. Fetch the repository: + + ```bash + sudo apt -y update + ``` + +6. Install the `xrpld` software package: + + ```bash + sudo apt -y install xrpld + ``` + +### With the YUM package manager + +1. Add the XRPL Foundation package-signing key: + + ```bash + sudo rpm --import https://packages.xrplf.org/xrplf.asc + ``` + +2. Add the repository, using the channel you picked in [Release channels](#release-channels): + + ```bash + cat << REPOFILE | sudo tee /etc/yum.repos.d/xrplf.repo + [xrplf-stable] + name=XRP Ledger Packages + enabled=1 + baseurl=https://packages.xrplf.org/repository/rpm-stable/ + gpgcheck=1 + repo_gpgcheck=1 + gpgkey=https://packages.xrplf.org/xrplf.asc + REPOFILE + ``` + + `gpgcheck=1` verifies each package against the key above. + `repo_gpgcheck=1` verifies the repository metadata, which the server signs with the same key. + +3. Install the `xrpld` package: + + ```bash + sudo yum install -y xrpld + ``` + +## The xrpld service + +Both package managers install a systemd unit and enable it, so `xrpld` starts on boot. +Check whether it is already running: + +```bash +systemctl status xrpld.service +``` + +The APT packages start it immediately as well; the YUM packages do not, so start it yourself: + +```bash +sudo systemctl start xrpld.service +``` + +### Optional: binding to privileged ports + +To serve incoming API requests on port 80 or 443, grant the service the capability to bind them. +You must also update the config file's port settings. + +```bash +sudo install -d -m 0755 /etc/systemd/system/xrpld.service.d +sudo tee /etc/systemd/system/xrpld.service.d/privileged-ports.conf >/dev/null <<'EOF' +[Service] +CapabilityBoundingSet=CAP_NET_BIND_SERVICE +AmbientCapabilities=CAP_NET_BIND_SERVICE +EOF +sudo systemctl daemon-reload +sudo systemctl restart xrpld.service +``` diff --git a/include/xrpl/ledger/View.h b/include/xrpl/ledger/View.h index f7fd5b5a8c..bb0817673c 100644 --- a/include/xrpl/ledger/View.h +++ b/include/xrpl/ledger/View.h @@ -24,6 +24,7 @@ #include #include #include +#include namespace xrpl { @@ -198,7 +199,10 @@ dirLink( * if withdrawing to self. * - If withdrawing to self, succeed. * - If not, checks if the receiver requires deposit authorization, and if - * the sender has it. + * the sender has it (account-based or credential-based). + * - Expects any credentials passed in to already exist in the ledger, and + * returns an internal error otherwise. Validate them beforehand with + * credentials::valid(). * - Checks that the receiver will not exceed the limit (IOU trustline limit * or MPT MaximumAmount). */ @@ -209,7 +213,8 @@ canWithdraw( AccountID const& to, SLE::const_ref toSle, STAmount const& amount, - bool hasDestinationTag); + bool hasDestinationTag, + std::optional> const& credentialIDs = std::nullopt); /** * Checks that can withdraw funds from an object to itself or a destination. @@ -222,7 +227,10 @@ canWithdraw( * if withdrawing to self. * - If withdrawing to self, succeed. * - If not, checks if the receiver requires deposit authorization, and if - * the sender has it. + * the sender has it (account-based or credential-based). + * - Expects any credentials passed in to already exist in the ledger, and + * returns an internal error otherwise. Validate them beforehand with + * credentials::valid(). * - Checks that the receiver will not exceed the limit (IOU trustline limit * or MPT MaximumAmount). */ @@ -232,20 +240,25 @@ canWithdraw( AccountID const& from, AccountID const& to, STAmount const& amount, - bool hasDestinationTag); + bool hasDestinationTag, + std::optional> const& credentialIDs = std::nullopt); /** * Checks that can withdraw funds from an object to itself or a destination. * * The receiver may be either the submitting account (sfAccount) or a different - * destination account (sfDestination). + * destination account (sfDestination). Credentials, if any, are taken from the + * transaction's sfCredentialIDs field. * * - Checks that the receiver account exists. * - If the receiver requires a destination tag, check that one exists, even * if withdrawing to self. * - If withdrawing to self, succeed. * - If not, checks if the receiver requires deposit authorization, and if - * the sender has it. + * the sender has it (account-based or credential-based). + * - Expects any credentials in sfCredentialIDs to already exist in the + * ledger, and returns an internal error otherwise. Validate them + * beforehand with credentials::valid(). * - Checks that the receiver will not exceed the limit (IOU trustline limit * or MPT MaximumAmount). */ diff --git a/include/xrpl/ledger/helpers/LendingHelpers.h b/include/xrpl/ledger/helpers/LendingHelpers.h index c69efff964..4aa89ea672 100644 --- a/include/xrpl/ledger/helpers/LendingHelpers.h +++ b/include/xrpl/ledger/helpers/LendingHelpers.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include // IWYU pragma: keep #include @@ -21,6 +22,7 @@ #include #include +#include #include #include @@ -58,6 +60,42 @@ canApplyToBrokerCover( bool checkLendingProtocolDependencies(Rules const& rules, STTx const& tx); +/** + * The accounts and asset that LoanManage::defaultLoan's fixCleanup3_4_0 + * freeze/lock exemption applies to. + * + * `defaultLoan` moves funds from the LoanBroker pseudo-account to the Vault + * pseudo-account via `accountSend`. Since neither is the vault asset's + * issuer, this is a third-party transfer that transits through the issuer in + * two hops (broker -> issuer, issuer -> vault; see + * `directSendNoLimitIOU`/`directSendNoLimitMPT`), so the exemption must cover + * both the issuer/broker and issuer/vault pairs, not a direct broker/vault + * pair. `asset` scopes it further to the vault's own currency/MPT issuance, + * so an unrelated one the same accounts happen to hold is still protected. + */ +struct LoanDefaultFreezeExemptAccounts +{ + AccountID issuer; + AccountID broker; + AccountID vault; + Asset asset; +}; + +/** + * Resolves the accounts and asset a LoanManage default transaction is + * exempt from freeze/lock for. + * + * @param view Ledger view used to resolve the Loan -> LoanBroker -> Vault + * chain. + * @param tx The transaction under invariant review. + * @return The exempt accounts and asset if `tx` is a `ttLOAN_MANAGE` + * transaction with the `tfLoanDefault` flag set, `fixCleanup3_4_0` is + * enabled, and the loan/broker/vault objects it references can all be + * resolved; `std::nullopt` otherwise. + */ +[[nodiscard]] std::optional +getLoanDefaultFreezeExemptAccounts(ReadView const& view, STTx const& tx); + static constexpr std::uint32_t kSecondsInYear = 365 * 24 * 60 * 60; Number diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index f8676d3b63..997f368638 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -921,6 +921,7 @@ TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw, {sfAmount, SoeRequired, SoeMptSupported}, {sfDestination, SoeOptional}, {sfDestinationTag, SoeOptional}, + {sfCredentialIDs, SoeOptional}, })) /** This transaction claws back tokens from a vault. */ @@ -1004,6 +1005,7 @@ TRANSACTION(ttLOAN_BROKER_COVER_WITHDRAW, 77, LoanBrokerCoverWithdraw, {sfAmount, SoeRequired, SoeMptSupported}, {sfDestination, SoeOptional}, {sfDestinationTag, SoeOptional}, + {sfCredentialIDs, SoeOptional}, })) /** This transaction claws back First Loss Capital from a Loan Broker to diff --git a/include/xrpl/protocol_autogen/transactions/LoanBrokerCoverWithdraw.h b/include/xrpl/protocol_autogen/transactions/LoanBrokerCoverWithdraw.h index 56a93acbb4..148db4292c 100644 --- a/include/xrpl/protocol_autogen/transactions/LoanBrokerCoverWithdraw.h +++ b/include/xrpl/protocol_autogen/transactions/LoanBrokerCoverWithdraw.h @@ -121,6 +121,32 @@ public: { return this->tx_->isFieldPresent(sfDestinationTag); } + + /** + * @brief Get sfCredentialIDs (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getCredentialIDs() const + { + if (hasCredentialIDs()) + { + return this->tx_->at(sfCredentialIDs); + } + return std::nullopt; + } + + /** + * @brief Check if sfCredentialIDs is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasCredentialIDs() const + { + return this->tx_->isFieldPresent(sfCredentialIDs); + } }; /** @@ -214,6 +240,17 @@ public: return *this; } + /** + * @brief Set sfCredentialIDs (SoeOptional) + * @return Reference to this builder for method chaining. + */ + LoanBrokerCoverWithdrawBuilder& + setCredentialIDs(std::decay_t const& value) + { + object_[sfCredentialIDs] = value; + return *this; + } + /** * @brief Build and return the LoanBrokerCoverWithdraw wrapper. * @param publicKey The public key for signing. diff --git a/include/xrpl/protocol_autogen/transactions/VaultWithdraw.h b/include/xrpl/protocol_autogen/transactions/VaultWithdraw.h index 3211524e1f..17208cd76c 100644 --- a/include/xrpl/protocol_autogen/transactions/VaultWithdraw.h +++ b/include/xrpl/protocol_autogen/transactions/VaultWithdraw.h @@ -121,6 +121,32 @@ public: { return this->tx_->isFieldPresent(sfDestinationTag); } + + /** + * @brief Get sfCredentialIDs (SoeOptional) + * @return The field value, or std::nullopt if not present. + */ + [[nodiscard]] + protocol_autogen::Optional + getCredentialIDs() const + { + if (hasCredentialIDs()) + { + return this->tx_->at(sfCredentialIDs); + } + return std::nullopt; + } + + /** + * @brief Check if sfCredentialIDs is present. + * @return True if the field is present, false otherwise. + */ + [[nodiscard]] + bool + hasCredentialIDs() const + { + return this->tx_->isFieldPresent(sfCredentialIDs); + } }; /** @@ -214,6 +240,17 @@ public: return *this; } + /** + * @brief Set sfCredentialIDs (SoeOptional) + * @return Reference to this builder for method chaining. + */ + VaultWithdrawBuilder& + setCredentialIDs(std::decay_t const& value) + { + object_[sfCredentialIDs] = value; + return *this; + } + /** * @brief Build and return the VaultWithdraw wrapper. * @param publicKey The public key for signing. diff --git a/include/xrpl/tx/ApplyContext.h b/include/xrpl/tx/ApplyContext.h index 472afdf624..e827e69f01 100644 --- a/include/xrpl/tx/ApplyContext.h +++ b/include/xrpl/tx/ApplyContext.h @@ -17,7 +17,6 @@ #include #include #include -#include namespace xrpl { @@ -130,16 +129,6 @@ public: view_->rawDestroyXRP(fee); } - /** - * Applies all invariant checkers one by one. - * - * @param result the result generated by processing this transaction. - * @param fee the fee charged for this transaction - * @return the result code that should be returned for this transaction. - */ - TER - checkInvariants(TER const result, XRPAmount const fee); - ApplyViewContext getApplyViewContext() { @@ -150,13 +139,6 @@ public: } private: - static TER - failInvariantCheck(TER const result); - - template - TER - checkInvariantsHelper(TER const result, XRPAmount const fee, std::index_sequence); - OpenView& base_; ApplyFlags flags_; std::optional view_; diff --git a/include/xrpl/tx/Transactor.h b/include/xrpl/tx/Transactor.h index a71285f70e..96ad7e00bc 100644 --- a/include/xrpl/tx/Transactor.h +++ b/include/xrpl/tx/Transactor.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -147,7 +148,7 @@ struct FeePayer FeePayerType type{FeePayerType::Account}; }; -class Transactor +class Transactor : public TxInvariantCheck { protected: ApplyContext& ctx_; @@ -158,7 +159,7 @@ protected: XRPAmount preFeeBalance_{}; // Balance before fees. public: - virtual ~Transactor() = default; + ~Transactor() override = default; Transactor(Transactor const&) = delete; Transactor& operator=(Transactor const&) = delete; @@ -183,20 +184,50 @@ public: return ctx_.view(); } + /** + * Which invariant layers to check. + * + * Full runs the protocol invariants plus the transaction-specific + * check. This is always the scope of the initial pass, even when the + * tentative TER is a tec: a bug or exploit could still mutate ledger + * state, so transaction-specific invariants must run for failed + * transactions too. + * + * ProtocolOnly runs only the protocol invariants and is used + * exclusively for the second invariant pass that follows a + * fee-claim reset — specifically, the reset that + * Transactor::operator() performs when the initial invariant pass + * returns tecINVARIANT_FAILED, rolling the transaction's effects back + * to a fee-claim-only state. In that reduced state the + * transaction-specific post-conditions no longer apply, but the + * protocol invariants must still hold against the fee claim itself. + * ProtocolOnly is not intended for other context discards (e.g. the + * reset used to handle tecOVERSIZE/tecKILLED/etc. in + * processPersistentChanges, or the ctx_.discard() done under + * TapFailHard); those paths do not re-run invariants at all. + */ + enum class InvariantScope { Full, ProtocolOnly }; + /** * Check all invariants for the current transaction. * - * Runs transaction-specific invariants first (visitInvariantEntry + - * finalizeInvariants), then protocol-level invariants. Both layers - * always run; the worst failure code is returned. + * Delegates to the free xrpl::checkInvariants runner. When @p scope is + * InvariantScope::Full, this transactor is passed so both layers + * share a single walk of the modified ledger entries. A failure in + * either layer fails the transaction the same way: tecINVARIANT_FAILED + * on the first pass, which the caller may respond to by rolling the + * transaction back to a fee-claim state and re-invoking this with + * InvariantScope::ProtocolOnly; a failure on that post-reset pass + * escalates to tefINVARIANT_FAILED. * * @param result the tentative TER from transaction processing. * @param fee the fee consumed by the transaction. + * @param scope which invariant layers to check. * * @return the final TER after all invariant checks. */ [[nodiscard]] TER - checkInvariants(TER result, XRPAmount fee); + checkInvariants(TER result, XRPAmount fee, InvariantScope scope); ///////////////////////////////////////////////////// /* @@ -538,20 +569,30 @@ private: preflightUniversal(PreflightContext const& ctx); /** - * Check transaction-specific invariants only. - * - * Walks every modified ledger entry via visitInvariantEntry, then - * calls finalizeInvariants on the derived transactor. Returns - * tecINVARIANT_FAILED if any transaction invariant is violated. - * - * @param result the tentative TER from transaction processing. - * @param fee the fee consumed by the transaction. - * - * @return the original result if all invariants pass, or - * tecINVARIANT_FAILED otherwise. + * Bridges the two-phase TxInvariantCheck interface to this transactor's + * visitInvariantEntry/finalizeInvariants hooks. Declared private (rather + * than protected, like the hooks they forward to) so that neither this + * transactor nor any subclass can call them directly through a + * Transactor& — only through the TxInvariantCheck& that the free + * xrpl::checkInvariants runner holds, which is where the two-phase + * ordering is enforced. */ - [[nodiscard]] TER - checkTransactionInvariants(TER result, XRPAmount fee); + void + visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) final + { + visitInvariantEntry(isDelete, before, after); + } + + [[nodiscard]] bool + finalize( + STTx const& tx, + TER result, + XRPAmount fee, + ReadView const& view, + beast::Journal const& j) final + { + return finalizeInvariants(tx, result, fee, view, j); + } }; inline bool diff --git a/include/xrpl/tx/invariants/FreezeInvariant.h b/include/xrpl/tx/invariants/FreezeInvariant.h index c66e002872..301e464daf 100644 --- a/include/xrpl/tx/invariants/FreezeInvariant.h +++ b/include/xrpl/tx/invariants/FreezeInvariant.h @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -11,6 +12,7 @@ #include #include +#include #include namespace xrpl { @@ -70,7 +72,8 @@ private: STTx const& tx, beast::Journal const& j, bool enforce, - bool fixOverrideFreeze); + bool fixOverrideFreeze, + std::optional const& loanDefaultAccounts); static bool validateFrozenState( @@ -80,7 +83,8 @@ private: beast::Journal const& j, bool enforce, bool globalFreeze, - bool fixOverrideFreeze); + bool fixOverrideFreeze, + std::optional const& loanDefaultAccounts); }; } // namespace xrpl diff --git a/include/xrpl/tx/invariants/InvariantRunner.h b/include/xrpl/tx/invariants/InvariantRunner.h new file mode 100644 index 0000000000..29a9dc09b2 --- /dev/null +++ b/include/xrpl/tx/invariants/InvariantRunner.h @@ -0,0 +1,140 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace xrpl { + +/** + * @brief Runtime interface for a transaction-specific invariant check. + * + * The free checkInvariants runner drives two layers of checks over a single + * walk of the modified ledger entries: + * + * - Protocol checks are the concrete types in InvariantChecks, held in a + * std::tuple and dispatched statically by a compile-time fold (no + * virtual calls). They are duck-typed against the two-phase contract + * described below; see InvariantChecker_PROTOTYPE in InvariantCheck.h. + * - The transaction-specific check is injected at runtime through this + * interface, so the runner can call it without depending on the concrete + * transactor type. Transactor implements this interface directly (see + * Transactor.h) so that the interface's access can stay narrower than + * Transactor's own public surface: calling through a TxInvariantCheck& + * (all the runner ever holds) is public, but calling through a + * Transactor& is not, since Transactor overrides these as private + * (forwarding to its own protected visitInvariantEntry/finalizeInvariants). + * + * Both layers honour the same two-phase protocol: + * + * Phase 1 — state collection (visitEntry). Called once for each ledger + * entry created, modified, or deleted by the transaction. Implementations + * accumulate whatever state they need to evaluate their post-conditions. + * Must not throw. + * + * Phase 2 — condition evaluation (finalize). Called once after every + * modified entry has been visited. Returns true if all post-conditions + * hold, false to fail the transaction. + * + * Rule: invariants must run regardless of transaction result. finalize + * MUST perform meaningful checks even when the transaction has failed + * (when result is not tesSUCCESS). A bug or exploit could cause a failed + * transaction to mutate ledger state in unexpected ways; invariants are the + * last line of defense. + * + * The typical pattern: an invariant that expects a domain-specific state + * change (e.g. a Vault being created) should expect that change only when + * the transaction succeeded. A failed VaultCreate must not have created a + * Vault. + * + * Rule: privilege-gated checks apply to failed transactions too. Failed + * transactions carry no privileges. Any privilege-gated assertion must + * therefore also be enforced for failed transactions. + */ +class TxInvariantCheck +{ +public: + virtual ~TxInvariantCheck() = default; + + /** + * @brief Called for each ledger entry modified by the transaction. + * + * @param isDelete true if the SLE is being deleted. + * @param before the entry's state before the transaction (nullptr for + * newly created entries). + * @param after the entry's state after the transaction. For deletions + * this is the SLE being erased; use @p isDelete rather than + * a null @p after to detect deletions. @p after is + * never null. + */ + virtual void + visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) = 0; + + /** + * @brief Called after all entries have been visited. + * + * @param tx the transaction being applied. + * @param result the tentative TER result of the transaction. + * @param fee the fee consumed by the transaction. + * @param view read-only view of the ledger after the transaction. + * @param j journal for logging invariant failures. + * @return true if all invariants hold; false to fail with + * tecINVARIANT_FAILED / tefINVARIANT_FAILED. + */ + [[nodiscard]] virtual bool + finalize( + STTx const& tx, + TER result, + XRPAmount fee, + ReadView const& view, + beast::Journal const& j) = 0; +}; + +/** + * @brief Run all protocol invariant checks plus the transaction-specific check + * in a single pass over the modified entries. + * + * Both layers share one walk of the modified-entry set: @p txCheck's + * visitEntry accumulates state on the same traversal that drives the + * protocol checkers, then both layers' finalize run on the complete state. + * + * Any failure (a finalize returning false or an exception anywhere in the + * check) returns failInvariantCheck(result). On the first pass that yields + * tecINVARIANT_FAILED, which the transactor treats as a signal to roll the + * transaction's effects back to a fee-claim-only state and re-run this + * runner against the reduced state (see Transactor::InvariantScope). If + * that second pass also fails, the result escalates to tefINVARIANT_FAILED, + * which excludes the transaction from the ledger entirely. + * + * The whole traversal — both layers' visitEntry calls and both layers' + * finalize calls — runs under a single try/catch. There is no per-layer + * isolation: an exception anywhere aborts the remaining traversal and + * finalize calls and fails the transaction. + * + * @param ctx the apply context for the current transaction. + * @param result the tentative TER from transaction processing. + * @param fee the fee consumed by the transaction. + * @param txCheck the transaction-specific invariant check. + * @return the final TER after all invariant checks. + */ +[[nodiscard]] TER +checkInvariants( + ApplyContext& ctx, + TER result, + XRPAmount fee, + std::optional> txCheck); + +[[nodiscard]] inline TER +checkInvariants(ApplyContext& ctx, TER result, XRPAmount fee) +{ + return checkInvariants(ctx, result, fee, std::nullopt); +} + +} // namespace xrpl diff --git a/include/xrpl/tx/transactors/vault/VaultWithdraw.h b/include/xrpl/tx/transactors/vault/VaultWithdraw.h index 22ad39d26d..b61af8b323 100644 --- a/include/xrpl/tx/transactors/vault/VaultWithdraw.h +++ b/include/xrpl/tx/transactors/vault/VaultWithdraw.h @@ -20,6 +20,9 @@ public: { } + static bool + checkExtraFeatures(PreflightContext const& ctx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/package/README.md b/package/README.md index 4899ee203e..54b1e57204 100644 --- a/package/README.md +++ b/package/README.md @@ -9,6 +9,7 @@ a build configured with `-Dvalidator_keys=ON`. ``` package/ build_pkg.sh Staging and build script (called by the CMake `package` target and CI) + sign_rpm.sh Signs the built RPMs (called by CI when publishing) publish_pkg.sh Uploads built packages to the XRPLF Nexus repositories (called by CI) rpm/ xrpld.spec RPM spec @@ -32,7 +33,7 @@ package manager (`apt-get` -> deb, `dnf`/`yum` -> rpm). | Package type | Image (`package_configs.[].image` in `linux.json`) | Tools required | | ------------ | ---------------------------------------------------------- | --------------------------------------------------- | -| RPM | `ghcr.io/xrplf/xrpld/packaging-rhel:sha-` | `rpmbuild` | +| RPM | `ghcr.io/xrplf/xrpld/packaging-rhel:sha-` | `rpmbuild`, `rpmsign` | | DEB | `ghcr.io/xrplf/xrpld/packaging-debian:sha-` | `dpkg-buildpackage`, debhelper with compat level 13 | To print the full packaging matrix (artifact names and images) for the current @@ -125,15 +126,15 @@ release defaults to 1 and is overridable with `-Dpkg_release=N`. Packages are published to the XRPLF repositories on Sonatype Nexus at `https://packages.xrplf.org`. The `release-info` action decides the channel from -the event, and `publish_pkg.sh` maps that channel to a repository pair: +the event, and `publish_pkg.sh` maps that channel to its repositories: -| Event | Version | Channel | DEB repository | RPM repository | -| ------------------------ | ----------------- | -------------- | ------------------ | ------------------ | -| tag | `X.Y.Z` | `stable` | `deb-stable` | `rpm-stable` | -| tag | `X.Y.Z-rcN` | `unstable` | `deb-unstable` | `rpm-unstable` | -| tag | `X.Y.Z-bN` | `experimental` | `deb-experimental` | `rpm-experimental` | -| push to `develop` | `xrpld --version` | `develop` | `deb-develop` | `rpm-develop` | -| tag, non-public codebase | _any_ | `private` | `deb-private` | `rpm-private` | +| Event | Version | Channel | DEB repository | RPM upload repository | +| ------------------------ | ----------------- | -------------- | ------------------ | ------------------------- | +| tag | `X.Y.Z` | `stable` | `deb-stable` | `rpm-stable-hosted` | +| tag | `X.Y.Z-rcN` | `unstable` | `deb-unstable` | `rpm-unstable-hosted` | +| tag | `X.Y.Z-bN` | `experimental` | `deb-experimental` | `rpm-experimental-hosted` | +| push to `develop` | `xrpld --version` | `develop` | `deb-develop` | `rpm-develop-hosted` | +| tag, non-public codebase | _any_ | `private` | `deb-private` | `rpm-private-hosted` | Only a tag names a channel — do not extend that to `develop`, where `BuildInfo.cpp`'s `versionString` moves through `-bN`, `-rcN` and even the final @@ -152,11 +153,17 @@ any `XRPLF` repository, `on-pr.yml` never. Both authenticate with the `NEXUS_REMOTE_USERNAME` / `NEXUS_REMOTE_PASSWORD` secrets already used for the Conan remote. -Nexus owns the repository metadata; nothing here signs or indexes anything. Worth -knowing: +Nexus owns the repository metadata; nothing here indexes anything. Worth knowing: -- Each apt-hosted repository needs a distribution and a PGP signing keypair - configured in Nexus, which rejects one created without a keypair. +- Each apt-hosted repository needs a distribution (ours use `any`) and a PGP + signing keypair configured in Nexus, which rejects one created without a + keypair. Nexus signs the apt metadata with it, never the packages. +- Hosted yum repositories cannot be signed by Nexus, so each `rpm--hosted` + repository sits behind a `rpm-` yum group repository whose metadata + Nexus signs. Uploads go to the hosted repository; clients point at the group + and verify the metadata with `repo_gpgcheck=1`. Nexus never signs the RPMs + themselves, so `sign_rpm.sh` signs them before they are uploaded, and clients + verify them with `gpgcheck=1`. - yum metadata is rebuilt asynchronously, so a successful publish is not immediately installable. - Each job uploads only what it built, and uploads are not transactional, so a @@ -212,8 +219,12 @@ fail early. Flags are for explicit invocation; environment variables are intended for CMake/CI integration. The CI workflow and the CMake `package` target both invoke `build_pkg.sh` with no flags; CMake supplies `SRC_DIR`, `BUILD_DIR`, and -`PKG_RELEASE` via env, while CI supplies `BUILD_DIR` and `PKG_RELEASE` via env -and lets the script use defaults for the rest. +`PKG_RELEASE` via env, while CI supplies `BUILD_DIR`, `PKG_RELEASE` and +`PKG_CHANNEL` via env and lets the script use defaults for the rest. + +Signing is not part of this script. `sign_rpm.sh` does it in a separate CI step +that only runs when publishing, so a published RPM is always signed and a local +build never needs a key. It resolves `SRC_DIR` and `BUILD_DIR` to absolute paths, then calls `stage_common()` to copy the `xrpld` and `validator-keys` binaries, config files, @@ -230,14 +241,9 @@ what catches a binary still linked against the Nix store's ELF loader (see 3. Runs `rpmbuild -bb`, passing the normalized package metadata version as the `pkg_version` RPM macro and `PKG_RELEASE` as the `pkg_release` RPM macro. The spec uses manual `install` commands to place files, disables `dwz`, and - writes uncompressed RPM payloads while generating debuginfo packages. + generates debuginfo packages. 4. Output: `rpmbuild/RPMS/x86_64/xrpld-*.rpm` -The uncompressed RPM payload setting is intentionally unconditional for -generated RPMs. It trades larger RPM artifacts for much shorter package -build/validation time, which keeps RPM package validation in the same rough time -class as Debian package validation. - RPM upgrades intentionally do not restart a running `xrpld` service. The spec uses `%systemd_postun`, matching Debian's `dh_installsystemd --no-stop-on-upgrade` behavior; operators pick up the new binary on the next diff --git a/package/publish_pkg.sh b/package/publish_pkg.sh index be36b531de..8ea9b189f4 100755 --- a/package/publish_pkg.sh +++ b/package/publish_pkg.sh @@ -7,10 +7,13 @@ set -euo pipefail # Usage: publish_pkg.sh [package-dir] # # channel release channel, selecting the 'deb-' and -# 'rpm-' repository pair +# 'rpm--hosted' repositories # package-dir searched recursively for *.deb, *.ddeb and *.rpm ('build' by # default) # +# RPMs are uploaded to the hosted repository, but yum clients install from the +# 'rpm-' group repository in front of it, which serves signed metadata. +# # NEXUS_USERNAME and NEXUS_PASSWORD are required. NEXUS_URL overrides the target # instance, and DRY_RUN=1 lists the uploads without performing them. @@ -24,7 +27,7 @@ if [[ -z "${channel}" ]]; then fi deb_repo="deb-${channel}" -rpm_repo="rpm-${channel}" +rpm_repo="rpm-${channel}-hosted" if [[ -z "${DRY_RUN:-}" ]]; then : "${NEXUS_USERNAME:?is required}" "${NEXUS_PASSWORD:?is required}" diff --git a/package/rpm/xrpld.spec b/package/rpm/xrpld.spec index 0e3ee2a968..23974c8900 100644 --- a/package/rpm/xrpld.spec +++ b/package/rpm/xrpld.spec @@ -19,8 +19,10 @@ BuildRequires: systemd-rpm-macros %undefine _debugsource_packages %debug_package -# Intentionally trade larger RPM artifacts for faster package validation. -%global _binary_payload w.ufdio +# Level 3 rather than the el9 default of 19: it shrinks the multi-gigabyte +# debuginfo package roughly fourfold in about a second, where 19 would spend +# minutes on it. +%global _binary_payload w3.zstdio %global _find_debuginfo_dwz_opts %{nil} %build_mtime_policy clamp_to_source_date_epoch diff --git a/package/shared/xrpld.service b/package/shared/xrpld.service index f54e47aa14..22e6359ef0 100644 --- a/package/shared/xrpld.service +++ b/package/shared/xrpld.service @@ -24,9 +24,5 @@ LogsDirectoryMode=0750 LimitNOFILE=65536 SystemCallArchitectures=native -# Uncomment both lines to allow xrpld to bind to privileged ports (<1024) -#CapabilityBoundingSet=CAP_NET_BIND_SERVICE -#AmbientCapabilities=CAP_NET_BIND_SERVICE - [Install] WantedBy=multi-user.target diff --git a/package/sign_rpm.sh b/package/sign_rpm.sh new file mode 100755 index 0000000000..250e806dd7 --- /dev/null +++ b/package/sign_rpm.sh @@ -0,0 +1,67 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Sign the RPMs built by build_pkg.sh. Nexus signs the yum repository metadata +# (via the 'rpm-' group repository), but never the packages themselves, +# so they carry their own signature. Clients verify the packages with gpgcheck=1 +# and the metadata with repo_gpgcheck=1. +# +# Usage: sign_rpm.sh [package-dir] +# +# package-dir searched recursively for *.rpm ('build' by default) +# +# PKG_SIGNING_KEY must hold an armoured PGP private key. It has no flag, to keep +# the key out of the process list. +# +# The DEBs are deliberately not signed: embedded DEB signatures exist (debsigs), +# but apt does not verify them by default and trusts the repository metadata, +# which Nexus signs, instead. + +pkg_dir="${1:-build}" + +mapfile -d '' rpms < <(find "${pkg_dir}" -type f -name '*.rpm' -print0) + +# Signing nothing would otherwise look like a successful signing. +if [[ ${#rpms[@]} -eq 0 ]]; then + echo "sign_rpm.sh: no RPMs found in ${pkg_dir}." >&2 + exit 1 +fi + +: "${PKG_SIGNING_KEY:?is required}" + +# Global, and expanded by the trap when it fires: the keyring holds an +# unencrypted private key, so it must go even if signing fails. +signing_home="$(mktemp -d)" +trap 'rm -rf "${signing_home}"' EXIT +export GNUPGHOME="${signing_home}" + +printf '%s' "${PKG_SIGNING_KEY}" | gpg --batch --quiet --import + +# Exactly one secret key, so that picking the first below is not a guess between +# several. +secrets="$(gpg --list-secret-keys --with-colons | grep -c '^sec:' || true)" +if [[ "${secrets}" -ne 1 ]]; then + echo "sign_rpm.sh: PKG_SIGNING_KEY must hold exactly one secret key, found ${secrets}." >&2 + exit 1 +fi + +key="$(gpg --list-secret-keys --with-colons | awk -F: '/^fpr:/ { print $10; exit }')" +echo "Signing ${#rpms[@]} RPM(s) with ${key}." + +# Loopback pinentry: the key is unattended, so there is no tty to prompt on. +rpmsign \ + --define "_gpg_name ${key}" \ + --define "_gpg_sign_cmd_extra_args --pinentry-mode loopback --batch --yes" \ + --addsign "${rpms[@]}" + +# rpmsign can exit 0 having attached nothing, and an unsigned package is only +# rejected later, on the installing machine. Both header tags are checked +# because an RSA signature lands in RSAHEADER and a DSA or EdDSA one in +# DSAHEADER. +for pkg in "${rpms[@]}"; do + signature="$(rpm --query --queryformat '%{RSAHEADER:pgpsig}%{DSAHEADER:pgpsig}' --package "${pkg}")" + if [[ "${signature}" == "(none)(none)" ]]; then + echo "sign_rpm.sh: ${pkg} is unsigned after rpmsign." >&2 + exit 1 + fi +done diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index e01ae2e492..0cd082ff47 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -35,6 +35,7 @@ #include #include #include +#include namespace xrpl { @@ -467,7 +468,8 @@ canWithdraw( AccountID const& to, SLE::const_ref toSle, STAmount const& amount, - bool hasDestinationTag) + bool hasDestinationTag, + std::optional> const& credentialIDs) { if (auto const ret = checkDestinationAndTag(toSle, hasDestinationTag)) return ret; @@ -478,7 +480,28 @@ canWithdraw( if (toSle->isFlag(lsfDepositAuth)) { if (!view.exists(keylet::depositPreauth(to, from))) - return tecNO_PERMISSION; + { + if (credentialIDs.has_value()) + { + STVector256 const credIDs{*credentialIDs}; + + // Callers must have validated these in preclaim, so a missing + // credential here is an invariant violation. + for (auto const& h : credIDs) + { + if (!view.exists(keylet::credential(h))) + return tecINTERNAL; // LCOV_EXCL_LINE + } + + if (auto const ret = credentials::authorizedDepositPreauth(view, credIDs, to); + !isTesSuccess(ret)) + return ret; + } + else + { + return tecNO_PERMISSION; + } + } } return withdrawToDestExceedsLimit(view, from, to, amount); @@ -490,11 +513,12 @@ canWithdraw( AccountID const& from, AccountID const& to, STAmount const& amount, - bool hasDestinationTag) + bool hasDestinationTag, + std::optional> const& credentialIDs) { auto const toSle = view.read(keylet::account(to)); - return canWithdraw(view, from, to, toSle, amount, hasDestinationTag); + return canWithdraw(view, from, to, toSle, amount, hasDestinationTag, credentialIDs); } [[nodiscard]] TER @@ -503,7 +527,8 @@ canWithdraw(ReadView const& view, STTx const& tx) auto const from = tx[sfAccount]; auto const to = tx[~sfDestination].value_or(from); - return canWithdraw(view, from, to, tx[sfAmount], tx.isFieldPresent(sfDestinationTag)); + return canWithdraw( + view, from, to, tx[sfAmount], tx.isFieldPresent(sfDestinationTag), tx[~sfCredentialIDs]); } TER diff --git a/src/libxrpl/ledger/helpers/LendingHelpers.cpp b/src/libxrpl/ledger/helpers/LendingHelpers.cpp index 89b03a03a7..cf1bd4915f 100644 --- a/src/libxrpl/ledger/helpers/LendingHelpers.cpp +++ b/src/libxrpl/ledger/helpers/LendingHelpers.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -20,12 +21,15 @@ #include #include #include +#include +#include #include #include #include #include #include +#include #include #include @@ -77,6 +81,40 @@ checkLendingProtocolDependencies(Rules const& rules, STTx const& tx) return true; } +std::optional +getLoanDefaultFreezeExemptAccounts(ReadView const& view, STTx const& tx) +{ + if (tx.getTxnType() != ttLOAN_MANAGE || !tx.isFlag(tfLoanDefault) || + !view.rules().enabled(fixCleanup3_4_0)) + return std::nullopt; + + // Unlike the broker/vault lookups below, the submitter picks the LoanID, + // so a nonexistent Loan is an ordinary (if unusual) input, not a + // structural impossibility -- exercised directly in LendingHelpers_test. + auto const loanSle = view.read(keylet::loan(tx[sfLoanID])); + if (!loanSle) + return std::nullopt; + + // A Loan can't outlive its LoanBroker (LoanBrokerDelete's preclaim + // rejects deletion while DebtTotal != 0), and a LoanBroker can't outlive + // its Vault (VaultDelete's preclaim has the equivalent guard) -- so these + // two lookups are structurally guaranteed to succeed here. + auto const brokerSle = view.read(keylet::loanBroker(loanSle->at(sfLoanBrokerID))); + if (!brokerSle) + return std::nullopt; // LCOV_EXCL_LINE + + auto const vaultSle = view.read(keylet::vault(brokerSle->at(sfVaultID))); + if (!vaultSle) + return std::nullopt; // LCOV_EXCL_LINE + + Asset const vaultAsset = vaultSle->at(sfAsset); + return LoanDefaultFreezeExemptAccounts{ + .issuer = vaultAsset.getIssuer(), + .broker = brokerSle->at(sfAccount), + .vault = vaultSle->at(sfAccount), + .asset = vaultAsset}; +} + LoanPaymentParts& LoanPaymentParts::operator+=(LoanPaymentParts const& other) { diff --git a/src/libxrpl/tx/ApplyContext.cpp b/src/libxrpl/tx/ApplyContext.cpp index 5e5ab90441..50f46fceef 100644 --- a/src/libxrpl/tx/ApplyContext.cpp +++ b/src/libxrpl/tx/ApplyContext.cpp @@ -1,27 +1,19 @@ #include -#include #include #include #include #include -#include #include #include #include #include #include #include -#include -#include -#include #include -#include #include #include -#include -#include namespace xrpl { @@ -75,75 +67,4 @@ ApplyContext::visit( view_->visit(base_, func); // NOLINT(bugprone-unchecked-optional-access) } -TER -ApplyContext::failInvariantCheck(TER const result) -{ - // If we already failed invariant checks before and we are now attempting to - // only charge a fee, and even that fails the invariant checks something is - // very wrong. We switch to tefINVARIANT_FAILED, which does NOT get included - // in a ledger. - - return (result == tecINVARIANT_FAILED || result == tefINVARIANT_FAILED) - ? TER{tefINVARIANT_FAILED} - : TER{tecINVARIANT_FAILED}; -} - -template -TER -ApplyContext::checkInvariantsHelper( - TER const result, - XRPAmount const fee, - std::index_sequence) -{ - try - { - auto checkers = getInvariantChecks(); - - // call each check's per-entry method - visit( - [&checkers]( - uint256 const& index, bool isDelete, SLE::const_ref before, SLE::const_ref after) { - (..., std::get(checkers).visitEntry(isDelete, before, after)); - }); - - // Note: do not replace this logic with a `...&&` fold expression. - // The fold expression will only run until the first check fails (it - // short-circuits). While the logic is still correct, the log - // message won't be. Every failed invariant should write to the log, - // not just the first one. - std::array const finalizers{{std::get(checkers).finalize( - tx, result, fee, *view_, journal)...}}; // NOLINT(bugprone-unchecked-optional-access) - - // call each check's finalizer to see that it passes - if (!std::ranges::all_of(finalizers, [](auto const& b) { return b; })) - { - JLOG(journal.fatal()) << "Transaction has failed one or more global invariants: " - << to_string(tx.getJson(JsonOptions::Values::None)); - - return failInvariantCheck(result); - } - } - catch (std::exception const& ex) - { - JLOG(journal.fatal()) << "Transaction caused an exception in a global invariant" - << ", ex: " << ex.what() - << ", tx: " << to_string(tx.getJson(JsonOptions::Values::None)); - - return failInvariantCheck(result); - } - - return result; -} - -TER -ApplyContext::checkInvariants(TER const result, XRPAmount const fee) -{ - XRPL_ASSERT( - isTesSuccess(result) || isTecClaim(result), - "xrpl::ApplyContext::checkInvariants : is tesSUCCESS or tecCLAIM"); - - return checkInvariantsHelper( - result, fee, std::make_index_sequence>{}); -} - } // namespace xrpl diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 304f4d0f43..63092cc128 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -41,11 +41,11 @@ #include #include #include +#include #include #include #include -#include #include #include #include @@ -1546,53 +1546,12 @@ Transactor::processPersistentChanges(TER result, XRPAmount fee) } [[nodiscard]] TER -Transactor::checkTransactionInvariants(TER result, XRPAmount fee) +Transactor::checkInvariants(TER result, XRPAmount fee, InvariantScope scope) { - try - { - // Phase 1: visit modified entries - ctx_.visit( - [this](uint256 const&, bool isDelete, SLE::const_ref before, SLE::const_ref after) { - this->visitInvariantEntry(isDelete, before, after); - }); + if (scope == InvariantScope::Full) + return xrpl::checkInvariants(ctx_, result, fee, *this); - // Phase 2: finalize - if (!this->finalizeInvariants(ctx_.tx, result, fee, ctx_.view(), ctx_.journal)) - { - JLOG(ctx_.journal.fatal()) << // - "Transaction has failed one or more transaction invariants, tx: " << // - to_string(ctx_.tx.getJson(JsonOptions::Values::None)); - return tecINVARIANT_FAILED; - } - } - catch (std::exception const& ex) - { - JLOG(ctx_.journal.fatal()) << // - "Exception while checking transaction invariants: " << // - ex.what() << // - ", tx: " << // - to_string(ctx_.tx.getJson(JsonOptions::Values::None)); - - return tecINVARIANT_FAILED; - } - - return result; -} - -[[nodiscard]] TER -Transactor::checkInvariants(TER result, XRPAmount fee) -{ - /* - * DISABLED for 3.2.0 — Must be re-introduced for 3.3.0 - * - * Transaction invariants are disabled due to a performance regression: - * the two-pass design (transaction-specific invariants + protocol invariants) - * iterates over modified ledger entries twice per transaction. - * - * Until resolved, only protocol invariants are checked (delegated to ctx_). - * This is safe because all transaction invariants in 3.2.0 are no-ops. - */ - return ctx_.checkInvariants(result, fee); + return xrpl::checkInvariants(ctx_, result, fee); } //------------------------------------------------------------------------------ @@ -1680,24 +1639,29 @@ Transactor::operator()() if (!canApply) return logger(result, canApply); - // Check invariants: if `tecINVARIANT_FAILED` is not returned, we can - // proceed to apply the tx - result = checkInvariants(result, fee); + // First invariant pass: both protocol and transaction-specific + // checks run against the transaction's tentative outcome. If it + // does not return tecINVARIANT_FAILED, we can proceed to apply the + // tx. + result = checkInvariants(result, fee, InvariantScope::Full); if (result == tecINVARIANT_FAILED) { - // Reset to fee-claim only + // Fee-claim reset: roll the transaction's effects back so that + // only the fee deduction remains. This is the reset referenced + // by InvariantScope::ProtocolOnly. auto const resetResult = reset(fee); if (!isTesSuccess(resetResult.first)) result = resetResult.first; fee = resetResult.second; - // Check invariants again to ensure the fee claiming doesn't violate - // invariants. After reset, only protocol invariants are re-checked. - // Transaction invariants are not meaningful here — the transaction's - // effects have been rolled back. + // Re-check invariants against the post-reset (fee-claim only) + // state. The transaction's effects are gone, so the + // transaction-specific invariants no longer apply and only the + // protocol invariants are re-run. A failure here escalates to + // tefINVARIANT_FAILED and excludes the tx from the ledger. if (isTesSuccess(result) || isTecClaim(result)) - result = ctx_.checkInvariants(result, fee); + result = checkInvariants(result, fee, InvariantScope::ProtocolOnly); } // We ran through the invariant checker, which can, in some cases, diff --git a/src/libxrpl/tx/invariants/FreezeInvariant.cpp b/src/libxrpl/tx/invariants/FreezeInvariant.cpp index c4340b9aec..d6039eabd8 100644 --- a/src/libxrpl/tx/invariants/FreezeInvariant.cpp +++ b/src/libxrpl/tx/invariants/FreezeInvariant.cpp @@ -4,7 +4,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -17,6 +19,7 @@ #include #include +#include #include namespace xrpl { @@ -75,6 +78,20 @@ TransfersNotFrozen::finalize( [[maybe_unused]] bool const enforce = view.rules().enabled(featureDeepFreeze); bool const fixOverrideFreeze = view.rules().enabled(fixCleanup3_4_0); + /* + * XLS-0066: a broker must be able to default an already-late loan + * regardless of the vault asset's freeze state. LoanManage::defaultLoan + * moves First-Loss Capital from the broker to the vault pseudo-account via + * accountSend, which transits through the issuer in two hops (see + * getLoanDefaultFreezeExemptAccounts), so a frozen issuer would otherwise + * trip this invariant on either hop. Gated behind fixCleanup3_4_0, and + * scoped to exactly the issuer/broker and issuer/vault lines involved for + * the vault's own currency, so ledgers without the amendment (or an + * unrelated frozen currency/line touched by the same transaction) keep + * the current (blocking) behavior. + */ + auto const loanDefaultAccounts = getLoanDefaultFreezeExemptAccounts(view, tx); + return std::ranges::all_of(balanceChanges_, [&](auto const& entry) { auto const& [issue, changes] = entry; auto const issuerSle = findIssuer(issue.account, view); @@ -91,7 +108,8 @@ TransfersNotFrozen::finalize( return !enforce; } - return validateIssuerChanges(issuerSle, changes, tx, j, enforce, fixOverrideFreeze); + return validateIssuerChanges( + issuerSle, changes, tx, j, enforce, fixOverrideFreeze, loanDefaultAccounts); }); } @@ -201,7 +219,8 @@ TransfersNotFrozen::validateIssuerChanges( STTx const& tx, beast::Journal const& j, bool enforce, - bool fixOverrideFreeze) + bool fixOverrideFreeze, + std::optional const& loanDefaultAccounts) { if (!issuer) { @@ -227,7 +246,15 @@ TransfersNotFrozen::validateIssuerChanges( { bool const high = change.line->at(sfLowLimit).getIssuer() == issuer->at(sfAccount); - if (!validateFrozenState(change, high, tx, j, enforce, globalFreeze, fixOverrideFreeze)) + if (!validateFrozenState( + change, + high, + tx, + j, + enforce, + globalFreeze, + fixOverrideFreeze, + loanDefaultAccounts)) { return false; } @@ -244,7 +271,8 @@ TransfersNotFrozen::validateFrozenState( beast::Journal const& j, bool enforce, bool globalFreeze, - bool fixOverrideFreeze) + bool fixOverrideFreeze, + std::optional const& loanDefaultAccounts) { bool const freeze = change.balanceChangeSign < 0 && change.line->isFlag(high ? lsfLowFreeze : lsfHighFreeze); @@ -269,6 +297,33 @@ TransfersNotFrozen::validateFrozenState( return true; } + // XLS-0066: LoanManage::defaultLoan's transfer is exempt from freeze (see + // finalize()). Since neither the broker nor vault pseudo-account is the + // asset's issuer, accountSend routes it as two hops through the issuer + // (broker -> issuer, issuer -> vault), so both the issuer/broker and + // issuer/vault lines are exempt -- but only for the vault's own currency, + // so an unrelated frozen line (a different currency, or one touched by + // the same transaction for some other reason) is still caught. + if (loanDefaultAccounts && loanDefaultAccounts->asset.holds() && + loanDefaultAccounts->asset.get().currency == + change.line->at(sfBalance).get().currency) + { + AccountID const lowAcct = change.line->at(sfLowLimit).getIssuer(); + AccountID const highAcct = change.line->at(sfHighLimit).getIssuer(); + auto const& accts = *loanDefaultAccounts; + auto const isPair = [&](AccountID const& a, AccountID const& b) { + return (lowAcct == a && highAcct == b) || (lowAcct == b && highAcct == a); + }; + if (isPair(accts.issuer, accts.broker) || isPair(accts.issuer, accts.vault)) + { + JLOG(j.debug()) << "Invariant check allowing funds to be moved " + << (change.balanceChangeSign > 0 ? "to" : "from") + << " a frozen trustline for LoanManage default " + << tx.getTransactionID(); + return true; + } + } + JLOG(j.fatal()) << "Invariant failed: Attempting to move frozen funds for " << tx.getTransactionID(); // The comment above starting with "assert(enforce)" explains this assert. diff --git a/src/libxrpl/tx/invariants/InvariantRunner.cpp b/src/libxrpl/tx/invariants/InvariantRunner.cpp new file mode 100644 index 0000000000..55bff2d693 --- /dev/null +++ b/src/libxrpl/tx/invariants/InvariantRunner.cpp @@ -0,0 +1,110 @@ +#include + +#include +#include +#include +#include +#include +#include // IWYU pragma: keep +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl { + +namespace { + +TER +failInvariantCheck(TER const result) +{ + return (result == tecINVARIANT_FAILED || result == tefINVARIANT_FAILED) + ? TER{tefINVARIANT_FAILED} + : TER{tecINVARIANT_FAILED}; +} + +template +TER +checkInvariantsHelper( + ApplyContext& ctx, + TER const result, + XRPAmount const fee, + std::optional> txCheck, + std::index_sequence) +{ + bool allOk = true; + + try + { + auto checkers = getInvariantChecks(); + + ctx.visit([&](uint256 const&, bool isDelete, SLE::const_ref before, SLE::const_ref after) { + if (txCheck) + txCheck->get().visitEntry(isDelete, before, after); + (..., std::get(checkers).visitEntry(isDelete, before, after)); + }); + + if (txCheck) + { + if (!txCheck->get().finalize(ctx.tx, result, fee, ctx.view(), ctx.journal)) + { + JLOG(ctx.journal.fatal()) + << "Transaction has failed one or more transaction invariants: " + << to_string(ctx.tx.getJson(JsonOptions::Values::None)); + allOk = false; + } + } + + // Note: do not replace this logic with a `...&&` fold expression. + // The fold expression will only run until the first check fails (it + // short-circuits). While the logic is still correct, the log + // message won't be. Every failed invariant should write to the log, + // not just the first one. + std::array const finalizers{ + {std::get(checkers).finalize(ctx.tx, result, fee, ctx.view(), ctx.journal)...}}; + + if (!std::all_of(finalizers.cbegin(), finalizers.cend(), [](auto const& b) { return b; })) + { + JLOG(ctx.journal.fatal()) << "Transaction has failed one or more global invariants: " + << to_string(ctx.tx.getJson(JsonOptions::Values::None)); + allOk = false; + } + } + catch (std::exception const& ex) + { + JLOG(ctx.journal.fatal()) << "Transaction caused an exception during invariant checks" + << ", ex: " << ex.what() << ", tx: " + << to_string(ctx.tx.getJson(JsonOptions::Values::None)); + return failInvariantCheck(result); + } + + return allOk ? result : failInvariantCheck(result); +} + +} // namespace + +TER +checkInvariants( + ApplyContext& ctx, + TER const result, + XRPAmount const fee, + std::optional> txCheck) +{ + XRPL_ASSERT( + isTesSuccess(result) || isTecClaim(result), + "xrpl::checkInvariants : is tesSUCCESS or tecCLAIM"); + + return checkInvariantsHelper( + ctx, result, fee, txCheck, std::make_index_sequence>{}); +} + +} // namespace xrpl diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index 3be71428e4..58d8050f18 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -848,6 +849,14 @@ ValidMPTTransfer::finalize( if (hasPrivilege(tx, OverrideFreeze)) return true; + // XLS-0066: a broker must be able to default an already-late loan + // regardless of the vault asset's lock state. Gated behind + // fixCleanup3_4_0, and scoped below to exactly the broker/vault + // pseudo-accounts and the vault's own MPT issuance -- see + // FreezeInvariant.cpp's TransfersNotFrozen::finalize for the IOU-side + // equivalent and rationale. + auto const loanDefaultAccounts = getLoanDefaultFreezeExemptAccounts(view, tx); + // DEX transactions (AMM[Create,Deposit], cross-currency payments, offer creates) are // subject to the MPTCanTrade flag in addition to the standard transfer rules. // A payment is only DEX if it is a cross-currency payment. @@ -889,6 +898,13 @@ ValidMPTTransfer::finalize( auto const canTrade = sleIssuance->isFlag(lsfMPTCanTrade); auto const reqAuth = sleIssuance->isFlag(lsfMPTRequireAuth); + // This issuance is the LoanManage default's own vault asset, so the + // broker/vault freeze exemption applies to it -- an unrelated MPT + // issuance the same accounts happen to hold is still caught. + bool const isLoanDefaultAsset = loanDefaultAccounts && + loanDefaultAccounts->asset.holds() && + loanDefaultAccounts->asset.get().getMptID() == mptID; + for (auto const& [account, value] : values) { // Classify each account as a sender or receiver based on whether their MPTAmount @@ -907,8 +923,15 @@ ValidMPTTransfer::finalize( // Check once: if any involved account is frozen, the whole issuance transfer is // considered frozen. Only need to check for frozen if there is a transfer of funds. + // + // The LoanManage default exemption only waives the frozen check, and only for + // the specific broker/vault pseudo-accounts identified above -- authorization is + // still enforced for them, and both checks still apply to every other account. + bool const exemptFromFreeze = isLoanDefaultAsset && loanDefaultAccounts && + (account == loanDefaultAccounts->broker || + account == loanDefaultAccounts->vault); if (!invalidTransfer && - (isFrozen(view, account, *sleIssuance) || + ((!exemptFromFreeze && isFrozen(view, account, *sleIssuance)) || !isAuthorized(view, mptID, account, reqAuth))) { invalidTransfer = true; diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp index 498f3c99eb..e914596599 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -25,7 +26,11 @@ namespace xrpl { bool LoanBrokerCoverWithdraw::checkExtraFeatures(PreflightContext const& ctx) { - return checkLendingProtocolDependencies(ctx.rules, ctx.tx); + if (!checkLendingProtocolDependencies(ctx.rules, ctx.tx)) + return false; + + return !ctx.tx.isFieldPresent(sfCredentialIDs) || + (ctx.rules.enabled(featureCredentials) && ctx.rules.enabled(fixCleanup3_4_0)); } NotTEC @@ -49,6 +54,9 @@ LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx) } } + if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err)) + return err; + return tesSUCCESS; } @@ -109,6 +117,12 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) if (auto const ret = canTransfer(ctx.view, vaultAsset, pseudoAccountID, dstAcct, waive)) return ret; + // Validate credentials (if any) before canWithdraw, since canWithdraw may + // call credentials::authorizedDepositPreauth which assumes credentials + // already exist. + if (auto const err = credentials::valid(ctx.tx, ctx.view, account, ctx.j); !isTesSuccess(err)) + return err; + // Withdrawal to a 3rd party destination account is essentially a transfer. // Enforce all the usual asset transfer checks. AuthType authType = AuthType::WeakAuth; diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 7e32e720d6..40689572a0 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -27,6 +28,13 @@ namespace xrpl { +bool +VaultWithdraw::checkExtraFeatures(PreflightContext const& ctx) +{ + return !ctx.tx.isFieldPresent(sfCredentialIDs) || + (ctx.rules.enabled(featureCredentials) && ctx.rules.enabled(fixCleanup3_4_0)); +} + static WaiveUnrealizedLoss shouldWaiveWithdrawal(ReadView const& view, AccountID const& account, SLE::const_ref issuance) { @@ -59,6 +67,9 @@ VaultWithdraw::preflight(PreflightContext const& ctx) } } + if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err)) + return err; + return tesSUCCESS; } @@ -113,6 +124,12 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } + // Validate credentials (if any) before canWithdraw, since canWithdraw may + // call credentials::authorizedDepositPreauth which assumes credentials + // already exist. + if (auto const err = credentials::valid(ctx.tx, ctx.view, account, ctx.j); !isTesSuccess(err)) + return err; + if (fix313Enabled && amount.asset() == vaultShare) { // Post-fixCleanup3_1_3: if the user specified shares, convert @@ -144,7 +161,8 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) account, dstAcct, *maybeAssets, - ctx.tx.isFieldPresent(sfDestinationTag))) + ctx.tx.isFieldPresent(sfDestinationTag), + ctx.tx[~sfCredentialIDs])) return ret; } catch (std::overflow_error const&) diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 70eaadbe17..ced2dea9bb 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -56,6 +57,7 @@ #include #include #include +#include #include #include @@ -68,6 +70,7 @@ #include #include #include +#include #include #include #include @@ -217,7 +220,8 @@ class Invariants_test : public beast::unit_test::Suite TER terActual = tesSUCCESS; for (TER const& terExpect : ters) { - terActual = transactor->checkInvariants(terActual, fee); + terActual = + transactor->checkInvariants(terActual, fee, Transactor::InvariantScope::Full); expect( terExpect == terActual, "expected: " + transToken(terExpect) + " got: " + transToken(terActual), @@ -6379,12 +6383,137 @@ class Invariants_test : public beast::unit_test::Suite auto transactor = makeTransactor(ac); if (!BEAST_EXPECT(transactor)) return; - TER const result = transactor->checkInvariants(tesSUCCESS, XRPAmount{}); + TER const result = transactor->checkInvariants( + tesSUCCESS, XRPAmount{}, Transactor::InvariantScope::Full); BEAST_EXPECT(result == tecINVARIANT_FAILED); BEAST_EXPECT(sink.messages().str().contains("is missing pseudo-account field")); } } + void + testTxCheckException() + { + testcase << "txCheck exception"; + using namespace jtx; + + // A TxInvariantCheck that throws from the requested hook, so we can + // exercise checkInvariantsHelper's catch block via the + // transaction-specific layer (as opposed to the protocol layer, + // which testObjectHasPseudoAccount's last case already covers via a + // real Transactor's finalizeInvariants). + enum class ThrowFrom { VisitEntry, Finalize }; + + struct ThrowingTxInvariantCheck : TxInvariantCheck + { + ThrowFrom const throwFrom; + + explicit ThrowingTxInvariantCheck(ThrowFrom throwFrom) : throwFrom(throwFrom) + { + } + + void + visitEntry(bool, SLE::const_ref, SLE::const_ref) override + { + if (throwFrom == ThrowFrom::VisitEntry) + throw std::runtime_error("test-injected visitEntry exception"); + } + + [[nodiscard]] bool + finalize(STTx const&, TER, XRPAmount, ReadView const&, beast::Journal const&) override + { + if (throwFrom == ThrowFrom::Finalize) + throw std::runtime_error("test-injected finalize exception"); + return true; + } + }; + + for (auto const throwFrom : {ThrowFrom::VisitEntry, ThrowFrom::Finalize}) + { + Env env{*this}; + Account const alice{"alice"}; + env.fund(XRP(1000), alice); + env.close(); + + OpenView ov{*env.current()}; + STTx const tx{ttACCOUNT_SET, [](STObject&) {}}; + test::StreamSink sink{beast::Severity::Warning}; + beast::Journal const jlog{sink}; + ApplyContext ac{ + env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog}; + CurrentTransactionRulesGuard const rulesGuard(ov.rules()); + + // visitEntry only runs for entries the transaction touched, so + // make a modification for the traversal to report. + auto sle = ac.view().peek(keylet::account(alice.id())); + if (!BEAST_EXPECT(sle)) + return; + sle->at(sfSequence) = sle->at(sfSequence) + 1; + ac.view().update(sle); + + ThrowingTxInvariantCheck throwing{throwFrom}; + TER terActual = tesSUCCESS; + for (TER const& terExpect : {TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)}) + { + terActual = checkInvariants(ac, terActual, XRPAmount{}, throwing); + BEAST_EXPECT(terExpect == terActual); + BEAST_EXPECT(sink.messages().str().contains( + "Transaction caused an exception during invariant checks")); + } + } + } + + void + testTxCheckFinalizeFalse() + { + testcase << "txCheck finalize returns false"; + using namespace jtx; + + // A TxInvariantCheck whose finalize returns false, so we can exercise + // the "Transaction has failed one or more transaction invariants" + // log path in checkInvariantsHelper independently of any real + // transactor. This is the transaction-layer analogue of the + // protocol-layer coverage in testObjectHasPseudoAccount / others. + struct FailingTxInvariantCheck : TxInvariantCheck + { + void + visitEntry(bool, SLE::const_ref, SLE::const_ref) override + { + } + + [[nodiscard]] bool + finalize(STTx const&, TER, XRPAmount, ReadView const&, beast::Journal const&) override + { + return false; + } + }; + + Env env{*this}; + Account const alice{"alice"}; + env.fund(XRP(1000), alice); + env.close(); + + OpenView ov{*env.current()}; + STTx const tx{ttACCOUNT_SET, [](STObject&) {}}; + test::StreamSink sink{beast::Severity::Warning}; + beast::Journal const jlog{sink}; + ApplyContext ac{env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog}; + CurrentTransactionRulesGuard const rulesGuard(ov.rules()); + + FailingTxInvariantCheck failing; + TER terActual = tesSUCCESS; + for (TER const& terExpect : {TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)}) + { + terActual = checkInvariants(ac, terActual, XRPAmount{}, failing); + BEAST_EXPECT(terExpect == terActual); + BEAST_EXPECT(sink.messages().str().contains( + "Transaction has failed one or more transaction invariants")); + // The protocol-layer log must not appear: only the tx-layer + // finalize failed here. + BEAST_EXPECT(!sink.messages().str().contains( + "Transaction has failed one or more global invariants")); + } + } + void testConfidentialMPTTransfer() { @@ -6670,6 +6799,8 @@ public: testAMM(); testObjectHasPseudoAccount(); testSponsorship(); + testTxCheckException(); + testTxCheckFinalizeFalse(); } }; diff --git a/src/test/app/NFTokenBurn_test.cpp b/src/test/app/NFTokenBurn_test.cpp index 52565432a9..ae1d557bb9 100644 --- a/src/test/app/NFTokenBurn_test.cpp +++ b/src/test/app/NFTokenBurn_test.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -794,7 +795,7 @@ class NFTokenBurn_test : public beast::unit_test::Suite TER terActual = tesSUCCESS; for (TER const& terExpect : {TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)}) { - terActual = ac.checkInvariants(terActual, XRPAmount{}); + terActual = xrpl::checkInvariants(ac, terActual, XRPAmount{}); BEAST_EXPECT(terExpect == terActual); BEAST_EXPECT(sink.messages().str().starts_with("Invariant failed:")); // uncomment to log the invariant failure message @@ -830,7 +831,7 @@ class NFTokenBurn_test : public beast::unit_test::Suite TER terActual = tesSUCCESS; for (TER const& terExpect : {TER(tecINVARIANT_FAILED), TER(tefINVARIANT_FAILED)}) { - terActual = ac.checkInvariants(terActual, XRPAmount{}); + terActual = xrpl::checkInvariants(ac, terActual, XRPAmount{}); BEAST_EXPECT(terExpect == terActual); BEAST_EXPECT(sink.messages().str().starts_with("Invariant failed:")); // uncomment to log the invariant failure message diff --git a/src/test/app/lending/LendingHelpers_test.cpp b/src/test/app/lending/LendingHelpers_test.cpp index 909b617980..32c49feb02 100644 --- a/src/test/app/lending/LendingHelpers_test.cpp +++ b/src/test/app/lending/LendingHelpers_test.cpp @@ -2,18 +2,27 @@ // DO NOT REMOVE #include #include +#include #include +#include +#include +#include +#include #include #include #include #include +#include +#include #include #include #include #include #include +#include #include +#include #include #include @@ -1871,6 +1880,93 @@ public: } } + // Targeted unit test for getLoanDefaultFreezeExemptAccounts(): builds a real + // (XRP, so no trust lines needed) Vault/LoanBroker/Loan chain, then calls + // the function directly against hand-picked, unsubmitted transactions + // (via env.jt(), which never touches the ledger) to exercise every early + // return and the success path precisely. + void + testLoanDefaultFreezeExemptAccounts() + { + using namespace jtx; + using namespace loan; + + testcase("getLoanDefaultFreezeExemptAccounts"); + + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + Env env{*this}; + Vault const vault{env}; + env.fund(XRP(10'000), lender, borrower); + env.close(); + + auto [vaultTx, vaultKeylet] = vault.create({.owner = lender, .asset = xrpIssue()}); + env(vaultTx); + env.close(); + env(vault.deposit({.depositor = lender, .id = vaultKeylet.key, .amount = XRP(1'000)})); + env.close(); + + auto const brokerKeylet = + keylet::loanBroker(lender.id(), SeqProxy::rawSequence(env.seq(lender))); + env(loan_broker::set(lender, vaultKeylet.key)); + env.close(); + + env(set(borrower, brokerKeylet.key, Number{200'000}), + Sig(sfCounterpartySignature, lender), + Fee(env.current()->fees().base * 2)); + env.close(); + + auto const loanKeylet = keylet::loan(brokerKeylet.key, SeqProxy::rawSequence(1)); + + // Not a LoanManage transaction at all. + { + auto const jt = env.jt(jtx::pay(lender, borrower, XRP(1))); + BEAST_EXPECT(!getLoanDefaultFreezeExemptAccounts(*env.current(), *jt.stx)); + } + + // LoanManage, but not the tfLoanDefault flag. + { + auto const jt = env.jt(manage(lender, loanKeylet.key, tfLoanImpair)); + BEAST_EXPECT(!getLoanDefaultFreezeExemptAccounts(*env.current(), *jt.stx)); + } + + // tfLoanDefault, but fixCleanup3_4_0 is disabled. + { + env.disableFeature(fixCleanup3_4_0); + auto const jt = env.jt(manage(lender, loanKeylet.key, tfLoanDefault)); + BEAST_EXPECT(!getLoanDefaultFreezeExemptAccounts(*env.current(), *jt.stx)); + env.enableFeature(fixCleanup3_4_0); + } + + // tfLoanDefault, amendment enabled, but the referenced Loan doesn't + // exist (reusing the broker's own ID as a bogus LoanID, same trick + // testInvalidLoanManage-style tests use elsewhere in this suite). + { + auto const jt = env.jt(manage(lender, brokerKeylet.key, tfLoanDefault)); + BEAST_EXPECT(!getLoanDefaultFreezeExemptAccounts(*env.current(), *jt.stx)); + } + + // tfLoanDefault, amendment enabled, Loan/LoanBroker/Vault all exist: + // resolves the issuer, broker, vault accounts, and the vault's asset. + { + auto const jt = env.jt(manage(lender, loanKeylet.key, tfLoanDefault)); + auto const result = getLoanDefaultFreezeExemptAccounts(*env.current(), *jt.stx); + auto const brokerSle = env.le(brokerKeylet); + auto const vaultSle = env.le(vaultKeylet); + BEAST_EXPECT(result); + BEAST_EXPECT(brokerSle); + BEAST_EXPECT(vaultSle); + if (result && brokerSle && vaultSle) + { + BEAST_EXPECT(result->issuer == vaultSle->at(sfAsset).getIssuer()); + BEAST_EXPECT(result->broker == brokerSle->at(sfAccount)); + BEAST_EXPECT(result->vault == vaultSle->at(sfAccount)); + BEAST_EXPECT(result->asset == vaultSle->at(sfAsset)); + } + } + } + void run() override { @@ -1906,6 +2002,8 @@ public: testLoanOriginationExceedsVaultMaximumDispatcher(); testLoanVaultExposureDispatcher(); testLoanPaymentDeltasDispatcher(); + + testLoanDefaultFreezeExemptAccounts(); } }; diff --git a/src/test/app/lending/LoanBroker_test.cpp b/src/test/app/lending/LoanBroker_test.cpp index 31fead08eb..437a0cea99 100644 --- a/src/test/app/lending/LoanBroker_test.cpp +++ b/src/test/app/lending/LoanBroker_test.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -2534,6 +2535,132 @@ class LoanBroker_test : public beast::unit_test::Suite testRIPD4274MPT(); } + void + testCoverWithdrawCredentialDepositPreauth(FeatureBitset features) + { + testcase( + std::string{"CoverWithdraw with credential-based deposit preauth "} + + (features[fixCleanup3_4_0] ? "post-fix" : "pre-fix")); + using namespace jtx; + using namespace std::chrono_literals; + + bool const fixEnabled = features[fixCleanup3_4_0]; + + Env env(*this, features); + + Account const broker{"broker"}; + Account const dest{"dest"}; + Account const credIssuer{"credIssuer"}; + char const credType[] = "abcde"; + + env.fund(XRP(10'000), broker, dest, credIssuer); + env(fset(dest, asfDepositAuth)); + env.close(); + + PrettyAsset const asset{xrpIssue(), 1'000'000}; + + Vault const vault(env); + auto const [vaultTx, vaultKeylet] = vault.create({.owner = broker, .asset = asset}); + env(vaultTx); + env.close(); + + env(vault.deposit({.depositor = broker, .id = vaultKeylet.key, .amount = asset(1'000)})); + env.close(); + + auto const brokerKeylet = + keylet::loanBroker(broker.id(), SeqProxy::rawSequence(env.seq(broker))); + env(loan_broker::set(broker, vaultKeylet.key)); + env.close(); + + env(loan_broker::coverDeposit(broker, brokerKeylet.key, asset(500))); + env.close(); + + auto coverWithdrawToDest = [&]() { + return loan_broker::coverWithdraw(broker, brokerKeylet.key, asset(10)); + }; + + // Without any preauth, coverWithdraw to dest fails + env(coverWithdrawToDest(), loan_broker::kDestination(dest), Ter{tecNO_PERMISSION}); + env.close(); + + // Issue and accept a credential for the broker (with expiration) + auto jv = credentials::create(broker, credIssuer, credType); + std::uint32_t const expiration = + env.current()->header().parentCloseTime.time_since_epoch().count() + 100; + jv[sfExpiration.jsonName] = expiration; + env(jv); + env(credentials::accept(broker, credIssuer, credType)); + env.close(); + + auto const credKeylet = credentials::keylet(broker, credIssuer, credType); + auto const credIdx = + credentials::ledgerEntry(env, broker, credIssuer, credType)[jss::result][jss::index] + .asString(); + + // dest authorizes deposits from holders of credentials issued by credIssuer + env(deposit::authCredentials(dest, {{.issuer = credIssuer, .credType = credType}})); + env.close(); + + // Without supplying credentials, still fails + env(coverWithdrawToDest(), loan_broker::kDestination(dest), Ter{tecNO_PERMISSION}); + env.close(); + + if (!fixEnabled) + { + // Pre-fix: sfCredentialIDs in LoanBrokerCoverWithdraw is disabled + env(coverWithdrawToDest(), + loan_broker::kDestination(dest), + credentials::Ids({credIdx}), + Ter{temDISABLED}); + env.close(); + return; + } + + // With credentials, succeeds + env(coverWithdrawToDest(), loan_broker::kDestination(dest), credentials::Ids({credIdx})); + env.close(); + + // Bad credential id is rejected + std::string const invalidIdx = + "0E0B04ED60588A758B67E21FBBE95AC5A63598BA951761DC0EC9C08D7E01E034"; + env(coverWithdrawToDest(), + loan_broker::kDestination(dest), + credentials::Ids({invalidIdx}), + Ter{tecBAD_CREDENTIALS}); + env.close(); + + // Malformed credential array (duplicates) is rejected by checkFields + env(coverWithdrawToDest(), + loan_broker::kDestination(dest), + credentials::Ids({credIdx, credIdx}), + Ter{temMALFORMED}); + env.close(); + + // Valid credential not authorized by dest hits authorizedDepositPreauth error path + char const credType2[] = "fghij"; + env(credentials::create(broker, credIssuer, credType2)); + env(credentials::accept(broker, credIssuer, credType2)); + env.close(); + auto const credIdx2 = + credentials::ledgerEntry(env, broker, credIssuer, credType2)[jss::result][jss::index] + .asString(); + env(coverWithdrawToDest(), + loan_broker::kDestination(dest), + credentials::Ids({credIdx2}), + Ter{tecNO_PERMISSION}); + env.close(); + + // Advance time past expiration: credentials yield tecEXPIRED and are deleted + env.close(150s); + BEAST_EXPECT(env.le(credKeylet)); + env(coverWithdrawToDest(), + loan_broker::kDestination(dest), + credentials::Ids({credIdx}), + Ter{tecEXPIRED}); + env.close(); + BEAST_EXPECT(!env.le(credKeylet)); + } + // Exercises canApplyToBrokerCover (fixCleanup3_2_0): a deposit, withdraw, // or clawback whose amount rounds to zero at sfCoverAvailable's precision // scale must be rejected with tecPRECISION_LOSS once the amendment is on, @@ -2894,6 +3021,9 @@ public: testRIPD4274(); + testCoverWithdrawCredentialDepositPreauth(all_ - fixCleanup3_4_0); + testCoverWithdrawCredentialDepositPreauth(all_); + testLoanBrokerDeleteLockedMPT(all_); testLoanBrokerDeleteLockedMPT(all_ - fixCleanup3_2_0); diff --git a/src/test/app/lending/LoanCoverFreezeAuth_test.cpp b/src/test/app/lending/LoanCoverFreezeAuth_test.cpp index a9b3542c4e..b0c43190c5 100644 --- a/src/test/app/lending/LoanCoverFreezeAuth_test.cpp +++ b/src/test/app/lending/LoanCoverFreezeAuth_test.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -13,6 +14,7 @@ #include #include +#include #include #include #include @@ -368,6 +370,186 @@ private: }; } + void + testLoanDefaultBypassesFreeze() + { + testcase("LoanManage: default bypasses asset freeze"); + using namespace jtx; + using namespace loan; + Account const lender{"lender"}; + Account const issuer{"issuer"}; + Account const borrower{"borrower"}; + auto const iou = issuer["IOU"]; + + Env env(*this); + env.fund(XRP(1'000), lender, issuer, borrower); + env(trust(lender, iou(10'000'000))); + env(pay(issuer, lender, iou(5'000'000))); + BrokerInfo const brokerInfo{createVaultAndBroker(env, issuer["IOU"], lender)}; + + auto const loanSetFee = Fee(env.current()->fees().base * 2); + STAmount const debtMaximumRequest = brokerInfo.asset(1'000).value(); + + env(set(borrower, brokerInfo.brokerID, debtMaximumRequest), + Sig(sfCounterpartySignature, lender), + loanSetFee); + env.close(); + + auto const loanKeylet = keylet::loan(brokerInfo.brokerID, SeqProxy::rawSequence(1)); + + using tp = NetClock::time_point; + using d = NetClock::duration; + + // Get past the grace period so the loan is defaultable. + if (auto loan = env.le(loanKeylet); BEAST_EXPECT(loan)) + { + env.close(tp{d{loan->at(sfNextPaymentDueDate) + loan->at(sfGracePeriod) + 1}}); + } + + // Global freeze trips the post-apply TransfersNotFrozen invariant. + env(fset(issuer, asfGlobalFreeze)); + env.close(); + + // Pre-fixCleanup3_4_0, the invariant blocks the default. + env.disableFeature(fixCleanup3_4_0); + env(manage(lender, loanKeylet.key, tfLoanDefault), Ter(tecINVARIANT_FAILED)); + env.close(); + + // Per XLS-0066, a default must succeed despite the freeze. + env.enableFeature(fixCleanup3_4_0); + env(manage(lender, loanKeylet.key, tfLoanDefault), Ter(tesSUCCESS)); + } + + // A default must bypass an MPT global lock the same way it bypasses IOU + // freeze, including when the loan was already impaired beforehand + // (a different defaultLoan() accounting branch than the un-impaired + // path exercised above) and after an ordinary LoanPay was correctly + // blocked by the same lock. + void + testLoanDefaultBypassesMptLockAfterImpair() + { + testcase("LoanManage: default bypasses MPT lock after impairment"); + using namespace jtx; + using namespace loan; + + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + Env env(*this); + env.fund(XRP(1'000'000), issuer, lender, borrower); + env.close(); + + MPTTester mptt( + {.env = env, + .issuer = issuer, + .holders = {lender, borrower}, + .flags = tfMPTCanTransfer | tfMPTCanLock}); + PrettyAsset const asset = mptt.issuanceID(); + env(pay(issuer, lender, asset(10'000'000))); + env.close(); + + BrokerInfo const brokerInfo{createVaultAndBroker(env, asset, lender)}; + + auto const loanSetFee = Fee(env.current()->fees().base * 2); + STAmount const debtMaximumRequest = brokerInfo.asset(1'000).value(); + env(set(borrower, brokerInfo.brokerID, debtMaximumRequest), + Sig(sfCounterpartySignature, lender), + loanSetFee); + env.close(); + + auto const loanKeylet = keylet::loan(brokerInfo.brokerID, SeqProxy::rawSequence(1)); + + // Realize a loss via impairment before locking. + env(manage(lender, loanKeylet.key, tfLoanImpair)); + env.close(); + + // Issuer applies a global lock. + mptt.set({.account = issuer, .flags = tfMPTLock}); + env.close(); + + // An ordinary payment is correctly blocked by the lock. + env(pay(borrower, loanKeylet.key, debtMaximumRequest), Ter(tecLOCKED)); + env.close(); + + using tp = NetClock::time_point; + using d = NetClock::duration; + if (auto loan = env.le(loanKeylet); BEAST_EXPECT(loan)) + { + env.close(tp{d{loan->at(sfNextPaymentDueDate) + loan->at(sfGracePeriod) + 1}}); + } + + // Pre-fixCleanup3_4_0 the ValidMPTTransfer invariant blocks the + // default, mirroring the IOU path above. + env.disableFeature(fixCleanup3_4_0); + env(manage(lender, loanKeylet.key, tfLoanDefault), Ter(tecINVARIANT_FAILED)); + env.close(); + + // The default itself must succeed despite the lock. + env.enableFeature(fixCleanup3_4_0); + env(manage(lender, loanKeylet.key, tfLoanDefault), Ter(tesSUCCESS)); + } + + // The exemption must hold for an individually deep-frozen trust line, not + // just a global freeze: deep freeze is what the original report ran into, + // and it takes a different path through validateFrozenState (the frozen + // flag comes off the line rather than off the issuer). + void + testLoanDefaultBypassesDeepFreeze() + { + testcase("LoanManage: default bypasses asset deep freeze"); + using namespace jtx; + using namespace loan; + Account const lender{"lender"}; + Account const issuer{"issuer"}; + Account const borrower{"borrower"}; + auto const iou = issuer["IOU"]; + + Env env(*this); + env.fund(XRP(1'000), lender, issuer, borrower); + env(trust(lender, iou(10'000'000))); + env(pay(issuer, lender, iou(5'000'000))); + BrokerInfo const brokerInfo{createVaultAndBroker(env, issuer["IOU"], lender)}; + + auto const loanSetFee = Fee(env.current()->fees().base * 2); + STAmount const debtMaximumRequest = brokerInfo.asset(1'000).value(); + + env(set(borrower, brokerInfo.brokerID, debtMaximumRequest), + Sig(sfCounterpartySignature, lender), + loanSetFee); + env.close(); + + auto const loanKeylet = keylet::loan(brokerInfo.brokerID, SeqProxy::rawSequence(1)); + + using tp = NetClock::time_point; + using d = NetClock::duration; + + // Get past the grace period so the loan is defaultable. + if (auto loan = env.le(loanKeylet); BEAST_EXPECT(loan)) + { + env.close(tp{d{loan->at(sfNextPaymentDueDate) + loan->at(sfGracePeriod) + 1}}); + } + + // The default moves First-Loss Capital off the broker pseudo-account, + // so that is the line to freeze. + auto const brokerSle = env.le(brokerInfo.brokerKeylet()); + if (!BEAST_EXPECT(brokerSle)) + return; + Account const brokerPseudo{"brokerPseudo", brokerSle->at(sfAccount)}; + + env(trust(issuer, brokerPseudo["IOU"](0), tfSetFreeze | tfSetDeepFreeze)); + env.close(); + + // Pre-fixCleanup3_4_0, the invariant blocks the default. + env.disableFeature(fixCleanup3_4_0); + env(manage(lender, loanKeylet.key, tfLoanDefault), Ter(tecINVARIANT_FAILED)); + env.close(); + + // Per XLS-0066, a default must succeed despite the deep freeze. + env.enableFeature(fixCleanup3_4_0); + env(manage(lender, loanKeylet.key, tfLoanDefault), Ter(tesSUCCESS)); + } + void testLoanPayBrokerOwnerMissingTrustline(FeatureBitset features) { @@ -694,6 +876,9 @@ private: runAmendmentIndependent() { testServiceFeeOnBrokerDeepFreeze(); + testLoanDefaultBypassesFreeze(); + testLoanDefaultBypassesDeepFreeze(); + testLoanDefaultBypassesMptLockAfterImpair(); } // Tests run under each entry in amendmentCombinations(). diff --git a/src/test/app/vault/VaultDomain_test.cpp b/src/test/app/vault/VaultDomain_test.cpp index db8943921b..5af0842962 100644 --- a/src/test/app/vault/VaultDomain_test.cpp +++ b/src/test/app/vault/VaultDomain_test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -18,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -570,6 +572,112 @@ private: } } + void + testWithdrawCredentialDepositPreauth(FeatureBitset features) + { + testcase( + "withdraw with credential-based deposit preauth " + + std::string{features[fixCleanup3_4_0] ? "post-fix" : "pre-fix"}); + using namespace test::jtx; + using namespace std::chrono_literals; + + bool const fixEnabled = features[fixCleanup3_4_0]; + + Env env{*this, features}; + + Account const owner{"owner"}; + Account const depositor{"depositor"}; + Account const dest{"dest"}; + Account const credIssuer{"credIssuer"}; + char const credType[] = "abcde"; + + env.fund(XRP(1000), owner, depositor, dest, credIssuer); + env(fset(dest, asfDepositAuth)); + env.close(); + + PrettyAsset const asset{xrpIssue(), 1'000'000}; + Vault vault{env}; + auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); + env(tx); + env.close(); + + env(vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)})); + env.close(); + + auto withdrawToDest = [&]() { + auto wtx = + vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(10)}); + wtx[sfDestination] = dest.human(); + return wtx; + }; + + // Without any preauth, withdraw to dest fails + env(withdrawToDest(), Ter{tecNO_PERMISSION}); + env.close(); + + // Issue and accept a credential for the depositor (with expiration) + auto jv = credentials::create(depositor, credIssuer, credType); + std::uint32_t const expiration = + env.current()->header().parentCloseTime.time_since_epoch().count() + 100; + jv[sfExpiration.jsonName] = expiration; + env(jv); + env(credentials::accept(depositor, credIssuer, credType)); + env.close(); + + auto const credKeylet = credentials::keylet(depositor, credIssuer, credType); + auto const credIdx = + credentials::ledgerEntry(env, depositor, credIssuer, credType)[jss::result][jss::index] + .asString(); + + // dest authorizes deposits from holders of credentials issued by credIssuer + env(deposit::authCredentials(dest, {{.issuer = credIssuer, .credType = credType}})); + env.close(); + + // Withdraw without supplying credentials still fails + env(withdrawToDest(), Ter{tecNO_PERMISSION}); + env.close(); + + if (!fixEnabled) + { + // Pre-fix: sfCredentialIDs in VaultWithdraw is rejected as disabled + env(withdrawToDest(), credentials::Ids({credIdx}), Ter{temDISABLED}); + env.close(); + return; + } + + // Withdraw with credentials succeeds + env(withdrawToDest(), credentials::Ids({credIdx})); + env.close(); + + // Bad credential id is rejected + std::string const invalidIdx = + "0E0B04ED60588A758B67E21FBBE95AC5A63598BA951761DC0EC9C08D7E01E034"; + env(withdrawToDest(), credentials::Ids({invalidIdx}), Ter{tecBAD_CREDENTIALS}); + env.close(); + + // Malformed credential array (duplicates) is rejected by checkFields + env(withdrawToDest(), credentials::Ids({credIdx, credIdx}), Ter{temMALFORMED}); + env.close(); + + // Valid credential not authorized by dest hits authorizedDepositPreauth error path + char const credType2[] = "fghij"; + env(credentials::create(depositor, credIssuer, credType2)); + env(credentials::accept(depositor, credIssuer, credType2)); + env.close(); + auto const credIdx2 = + credentials::ledgerEntry(env, depositor, credIssuer, credType2)[jss::result][jss::index] + .asString(); + env(withdrawToDest(), credentials::Ids({credIdx2}), Ter{tecNO_PERMISSION}); + env.close(); + + // Advance time past expiration: credentials yield tecEXPIRED and are deleted + env.close(150s); + BEAST_EXPECT(env.le(credKeylet)); + env(withdrawToDest(), credentials::Ids({credIdx}), Ter{tecEXPIRED}); + env.close(); + BEAST_EXPECT(!env.le(credKeylet)); + } + public: void run() override @@ -578,6 +686,8 @@ public: testDomainLossAfterAcquisition(); testDomainCheckBuyerSideOffer(); testWithDomainChecXRP(); + testWithdrawCredentialDepositPreauth(all_ - fixCleanup3_4_0); + testWithdrawCredentialDepositPreauth(all_); } }; diff --git a/src/test/app/vault/VaultRPC_test.cpp b/src/test/app/vault/VaultRPC_test.cpp index 2ac092b5a7..dbceb1cb9c 100644 --- a/src/test/app/vault/VaultRPC_test.cpp +++ b/src/test/app/vault/VaultRPC_test.cpp @@ -9,11 +9,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -122,6 +124,22 @@ private: } }; + // An error response must carry a registered token together with the matching code and + // message, so that clients dispatching on either of them reach the same conclusion. + auto const checkError = [this]( + json::Value const& result, + std::string const& token, + ErrorCodeI const code, + std::string const& message) { + BEAST_EXPECT(result[jss::error].asString() == token); + BEAST_EXPECT(result[jss::error_code].asInt() == code); + BEAST_EXPECT(result[jss::error_message].asString() == message); + }; + + std::string const badSeqMessage = "Invalid field 'seq', not a positive 32-bit integer."; + std::string const badFieldsMessage = + "Must specify either 'vault_id' or both 'owner' and 'seq'."; + { testcase("RPC ledger_entry selected by key"); json::Value jvParams; @@ -276,16 +294,57 @@ private: jvParams[jss::ledger_index] = jss::validated; jvParams[jss::vault_id] = "foobar"; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError( + jv[jss::result], + "invalidParams", + RpcInvalidParams, + "Invalid field 'vault_id', not hex string."); } { - testcase("RPC vault_info json invalid index"); + testcase("RPC vault_info json numeric vault_id"); json::Value jvParams; jvParams[jss::ledger_index] = jss::validated; jvParams[jss::vault_id] = 0; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError( + jv[jss::result], + "invalidParams", + RpcInvalidParams, + "Invalid field 'vault_id', not hex string."); + } + + { + testcase("RPC vault_info json object vault_id"); + json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::vault_id] = json::Value(json::ValueType::Object); + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + checkError( + jv[jss::result], + "invalidParams", + RpcInvalidParams, + "Invalid field 'vault_id', not hex string."); + } + + { + // An all-zero key is a well-formed request for a vault that cannot exist, not a + // malformed one. parseHex accepts both the padded form and the short "0". + testcase("RPC vault_info json all zero vault_id"); + json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::vault_id] = strHex(uint256(beast::kZero)); + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); + } + + { + testcase("RPC vault_info json short zero vault_id"); + json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::vault_id] = "0"; + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); } { @@ -308,7 +367,7 @@ private: jvParams[jss::owner] = owner.human(); jvParams[jss::seq] = "foobar"; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -318,7 +377,7 @@ private: jvParams[jss::owner] = owner.human(); jvParams[jss::seq] = 0; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -328,7 +387,7 @@ private: jvParams[jss::owner] = owner.human(); jvParams[jss::seq] = -1; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -338,7 +397,7 @@ private: jvParams[jss::owner] = owner.human(); jvParams[jss::seq] = 1e20; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -348,7 +407,7 @@ private: jvParams[jss::owner] = owner.human(); jvParams[jss::seq] = true; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -358,7 +417,25 @@ private: jvParams[jss::owner] = "foobar"; jvParams[jss::seq] = sequence; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError( + jv[jss::result], + "actMalformed", + RpcActMalformed, + "Invalid field 'owner', not AccountID."); + } + + { + testcase("RPC vault_info json array owner"); + json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::owner] = json::Value(json::ValueType::Array); + jvParams[jss::seq] = sequence; + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + checkError( + jv[jss::result], + "actMalformed", + RpcActMalformed, + "Invalid field 'owner', not AccountID."); } { @@ -367,7 +444,7 @@ private: jvParams[jss::ledger_index] = jss::validated; jvParams[jss::owner] = owner.human(); auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -376,7 +453,7 @@ private: jvParams[jss::ledger_index] = jss::validated; jvParams[jss::seq] = sequence; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -386,7 +463,7 @@ private: jvParams[jss::vault_id] = strHex(keylet.key); jvParams[jss::seq] = sequence; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -396,7 +473,7 @@ private: jvParams[jss::vault_id] = strHex(keylet.key); jvParams[jss::owner] = owner.human(); auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -409,7 +486,7 @@ private: jvParams[jss::seq] = sequence; jvParams[jss::owner] = owner.human(); auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -417,7 +494,7 @@ private: json::Value jvParams; jvParams[jss::ledger_index] = jss::validated; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -427,15 +504,15 @@ private: } { - testcase("RPC vault_info command line invalid index"); + testcase("RPC vault_info command line zero index"); json::Value jv = env.rpc("vault_info", "0", "validated"); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); } { - testcase("RPC vault_info command line invalid index"); + testcase("RPC vault_info command line unknown index"); json::Value jv = env.rpc("vault_info", strHex(uint256(42)), "validated"); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "entryNotFound"); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); } { diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index 5e4cda243a..9cbfb8ca10 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/src/tests/libxrpl/CMakeLists.txt @@ -43,6 +43,9 @@ set(test_modules if(NOT WIN32) list(APPEND test_modules net) endif() +if(rust) + target_link_libraries(xrpl_tests PRIVATE rs_hello_world_cxxbridge) +endif() foreach(module IN LISTS test_modules) # Append the module's sources (${module}/*.cpp and ${module}.cpp, if any). @@ -52,6 +55,12 @@ foreach(module IN LISTS test_modules) "${CMAKE_CURRENT_SOURCE_DIR}/${module}/*.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/${module}.cpp" ) + if(NOT rust) + # Tests of the Rust interop include generated cxxbridge headers, which + # do not exist without the crates, so keep them out of the build tree + # entirely. They are named `Rust.cpp`. + list(FILTER sources EXCLUDE REGEX "/Rust[^/]*\\.cpp$") + endif() target_sources(xrpl_tests PRIVATE ${sources}) # Expose the module's private headers under their canonical include path. diff --git a/src/tests/libxrpl/basics/RustInterop.cpp b/src/tests/libxrpl/basics/RustInterop.cpp new file mode 100644 index 0000000000..8a6ad8a4ed --- /dev/null +++ b/src/tests/libxrpl/basics/RustInterop.cpp @@ -0,0 +1,9 @@ +#include +#include + +#include + +TEST(RustInteropTest, hello_world) +{ + EXPECT_EQ(std::string(rs::hello_world::hello_world()), "hello_world"); +} diff --git a/src/tests/libxrpl/protocol_autogen/transactions/LoanBrokerCoverWithdrawTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/LoanBrokerCoverWithdrawTests.cpp index 5b0a8c9146..043ab0a252 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/LoanBrokerCoverWithdrawTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/LoanBrokerCoverWithdrawTests.cpp @@ -33,6 +33,7 @@ TEST(TransactionsLoanBrokerCoverWithdrawTests, BuilderSettersRoundTrip) auto const amountValue = canonical_AMOUNT(); auto const destinationValue = canonical_ACCOUNT(); auto const destinationTagValue = canonical_UINT32(); + auto const credentialIDsValue = canonical_VECTOR256(); LoanBrokerCoverWithdrawBuilder builder{ accountValue, @@ -45,6 +46,7 @@ TEST(TransactionsLoanBrokerCoverWithdrawTests, BuilderSettersRoundTrip) // Set optional fields builder.setDestination(destinationValue); builder.setDestinationTag(destinationTagValue); + builder.setCredentialIDs(credentialIDsValue); auto tx = builder.build(publicKey, secretKey); @@ -90,6 +92,14 @@ TEST(TransactionsLoanBrokerCoverWithdrawTests, BuilderSettersRoundTrip) EXPECT_TRUE(tx.hasDestinationTag()); } + { + auto const& expected = credentialIDsValue; + auto const actualOpt = tx.getCredentialIDs(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfCredentialIDs should be present"; + expectEqualField(expected, *actualOpt, "sfCredentialIDs"); + EXPECT_TRUE(tx.hasCredentialIDs()); + } + } // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, @@ -110,6 +120,7 @@ TEST(TransactionsLoanBrokerCoverWithdrawTests, BuilderFromStTxRoundTrip) auto const amountValue = canonical_AMOUNT(); auto const destinationValue = canonical_ACCOUNT(); auto const destinationTagValue = canonical_UINT32(); + auto const credentialIDsValue = canonical_VECTOR256(); // Build an initial transaction LoanBrokerCoverWithdrawBuilder initialBuilder{ @@ -122,6 +133,7 @@ TEST(TransactionsLoanBrokerCoverWithdrawTests, BuilderFromStTxRoundTrip) initialBuilder.setDestination(destinationValue); initialBuilder.setDestinationTag(destinationTagValue); + initialBuilder.setCredentialIDs(credentialIDsValue); auto initialTx = initialBuilder.build(publicKey, secretKey); @@ -166,6 +178,13 @@ TEST(TransactionsLoanBrokerCoverWithdrawTests, BuilderFromStTxRoundTrip) expectEqualField(expected, *actualOpt, "sfDestinationTag"); } + { + auto const& expected = credentialIDsValue; + auto const actualOpt = rebuiltTx.getCredentialIDs(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfCredentialIDs should be present"; + expectEqualField(expected, *actualOpt, "sfCredentialIDs"); + } + } // 3) Verify wrapper throws when constructed from wrong transaction type. @@ -229,6 +248,8 @@ TEST(TransactionsLoanBrokerCoverWithdrawTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(tx.getDestination().has_value()); EXPECT_FALSE(tx.hasDestinationTag()); EXPECT_FALSE(tx.getDestinationTag().has_value()); + EXPECT_FALSE(tx.hasCredentialIDs()); + EXPECT_FALSE(tx.getCredentialIDs().has_value()); } } diff --git a/src/tests/libxrpl/protocol_autogen/transactions/VaultWithdrawTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/VaultWithdrawTests.cpp index 4067a6551d..518957d47b 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/VaultWithdrawTests.cpp +++ b/src/tests/libxrpl/protocol_autogen/transactions/VaultWithdrawTests.cpp @@ -33,6 +33,7 @@ TEST(TransactionsVaultWithdrawTests, BuilderSettersRoundTrip) auto const amountValue = canonical_AMOUNT(); auto const destinationValue = canonical_ACCOUNT(); auto const destinationTagValue = canonical_UINT32(); + auto const credentialIDsValue = canonical_VECTOR256(); VaultWithdrawBuilder builder{ accountValue, @@ -45,6 +46,7 @@ TEST(TransactionsVaultWithdrawTests, BuilderSettersRoundTrip) // Set optional fields builder.setDestination(destinationValue); builder.setDestinationTag(destinationTagValue); + builder.setCredentialIDs(credentialIDsValue); auto tx = builder.build(publicKey, secretKey); @@ -90,6 +92,14 @@ TEST(TransactionsVaultWithdrawTests, BuilderSettersRoundTrip) EXPECT_TRUE(tx.hasDestinationTag()); } + { + auto const& expected = credentialIDsValue; + auto const actualOpt = tx.getCredentialIDs(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfCredentialIDs should be present"; + expectEqualField(expected, *actualOpt, "sfCredentialIDs"); + EXPECT_TRUE(tx.hasCredentialIDs()); + } + } // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, @@ -110,6 +120,7 @@ TEST(TransactionsVaultWithdrawTests, BuilderFromStTxRoundTrip) auto const amountValue = canonical_AMOUNT(); auto const destinationValue = canonical_ACCOUNT(); auto const destinationTagValue = canonical_UINT32(); + auto const credentialIDsValue = canonical_VECTOR256(); // Build an initial transaction VaultWithdrawBuilder initialBuilder{ @@ -122,6 +133,7 @@ TEST(TransactionsVaultWithdrawTests, BuilderFromStTxRoundTrip) initialBuilder.setDestination(destinationValue); initialBuilder.setDestinationTag(destinationTagValue); + initialBuilder.setCredentialIDs(credentialIDsValue); auto initialTx = initialBuilder.build(publicKey, secretKey); @@ -166,6 +178,13 @@ TEST(TransactionsVaultWithdrawTests, BuilderFromStTxRoundTrip) expectEqualField(expected, *actualOpt, "sfDestinationTag"); } + { + auto const& expected = credentialIDsValue; + auto const actualOpt = rebuiltTx.getCredentialIDs(); + ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfCredentialIDs should be present"; + expectEqualField(expected, *actualOpt, "sfCredentialIDs"); + } + } // 3) Verify wrapper throws when constructed from wrong transaction type. @@ -229,6 +248,8 @@ TEST(TransactionsVaultWithdrawTests, OptionalFieldsReturnNullopt) EXPECT_FALSE(tx.getDestination().has_value()); EXPECT_FALSE(tx.hasDestinationTag()); EXPECT_FALSE(tx.getDestinationTag().has_value()); + EXPECT_FALSE(tx.hasCredentialIDs()); + EXPECT_FALSE(tx.getCredentialIDs().has_value()); } } diff --git a/src/xrpld/rpc/handlers/VaultInfo.cpp b/src/xrpld/rpc/handlers/VaultInfo.cpp index c216192ab3..0aa5334bd2 100644 --- a/src/xrpld/rpc/handlers/VaultInfo.cpp +++ b/src/xrpld/rpc/handlers/VaultInfo.cpp @@ -26,36 +26,48 @@ parseVault(json::Value const& params, json::Value& jvResult) uint256 uNodeIndex = beast::kZero; if (hasVaultId && !hasOwner && !hasSeq) { - if (!uNodeIndex.parseHex(params[jss::vault_id].asString())) + // asString() throws on an object or an array, so the type comes first. + auto const& vaultId = params[jss::vault_id]; + if (!vaultId.isString() || !uNodeIndex.parseHex(vaultId.asString())) { - rpc::injectError(RpcInvalidParams, jvResult); + rpc::injectError( + RpcInvalidParams, rpc::expectedFieldMessage(jss::vault_id, "hex string"), jvResult); return std::nullopt; } // else uNodeIndex holds the value we need } else if (!hasVaultId && hasOwner && hasSeq) { - auto const id = parseBase58(params[jss::owner].asString()); + auto const& owner = params[jss::owner]; + auto const id = owner.isString() ? parseBase58(owner.asString()) + : std::optional{}; if (!id) { - rpc::injectError(RpcActMalformed, jvResult); - return std::nullopt; - } - if (!(params[jss::seq].isInt() || params[jss::seq].isUInt()) || - params[jss::seq].asDouble() <= 0.0 || - params[jss::seq].asDouble() > double(json::Value::kMaxUInt)) - { - rpc::injectError(RpcInvalidParams, jvResult); + rpc::injectError( + RpcActMalformed, rpc::expectedFieldMessage(jss::owner, "AccountID"), jvResult); return std::nullopt; } - auto const seq = SeqProxy::rawSequence(params[jss::seq].asUInt()); + // Int and UInt are both 32 bits wide, so the type check is the only upper bound needed. + auto const& seqField = params[jss::seq]; + if (!(seqField.isInt() || seqField.isUInt()) || seqField.asDouble() <= 0.0) + { + rpc::injectError( + RpcInvalidParams, + rpc::expectedFieldMessage(jss::seq, "a positive 32-bit integer"), + jvResult); + return std::nullopt; + } + + auto const seq = SeqProxy::rawSequence(seqField.asUInt()); uNodeIndex = keylet::vault(*id, seq).key; } else { - // Invalid combination of fields vault_id/owner/seq - rpc::injectError(RpcInvalidParams, jvResult); + rpc::injectError( + RpcInvalidParams, + "Must specify either 'vault_id' or both 'owner' and 'seq'.", + jvResult); return std::nullopt; } @@ -71,20 +83,25 @@ doVaultInfo(rpc::JsonContext& context) if (!lpLedger) return jvResult; - auto const uNodeIndex = parseVault(context.params, jvResult).value_or(beast::kZero); - if (uNodeIndex == beast::kZero) + // No key means the request could not be turned into one, and parseVault has already said why. + auto const uNodeIndex = parseVault(context.params, jvResult); + if (!uNodeIndex) + return jvResult; + + // A zero key names an entry that cannot exist, and the ledger refuses to be asked for one. + if (*uNodeIndex == beast::kZero) { - jvResult[jss::error] = "malformedRequest"; + rpc::injectError(RpcEntryNotFound, jvResult); return jvResult; } - auto const sleVault = lpLedger->read(keylet::vault(uNodeIndex)); + auto const sleVault = lpLedger->read(keylet::vault(*uNodeIndex)); auto const sleIssuance = sleVault == nullptr // ? nullptr : lpLedger->read(keylet::mptokenIssuance(sleVault->at(sfShareMPTID))); if (!sleVault || !sleIssuance) { - jvResult[jss::error] = "entryNotFound"; + rpc::injectError(RpcEntryNotFound, jvResult); return jvResult; }