diff --git a/.cspell.config.yaml b/.cspell.config.yaml index 13da132b90..e220cd0249 100644 --- a/.cspell.config.yaml +++ b/.cspell.config.yaml @@ -262,6 +262,8 @@ words: - Rohrs - roundings - rustc + - rustfmt + - rustup - sahyadri - Satoshi - scons @@ -305,6 +307,7 @@ words: - takerpays - ters - TMEndpointv2 + - toolchain - tparam - trixie - tx diff --git a/.gersemi/definitions.cmake b/.gersemi/definitions.cmake index aa63076c8b..0932a72463 100644 --- a/.gersemi/definitions.cmake +++ b/.gersemi/definitions.cmake @@ -11,6 +11,9 @@ endfunction() function(create_symbolic_link target link) endfunction() +function(xrpl_add_benchmark name) +endfunction() + macro(exclude_from_default target_) endmacro() diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index b31e6ae961..3c9c514516 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -1,3 +1,6 @@ +benchmarks.libxrpl > xrpl.basics +benchmarks.libxrpl > xrpl.config +benchmarks.libxrpl > xrpl.nodestore libxrpl.basics > xrpl.basics libxrpl.conditions > xrpl.basics libxrpl.conditions > xrpl.conditions diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 03ac1c6334..7edbf96ef6 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -1,5 +1,5 @@ { - "image_tag": "sha-e29b523", + "image_tag": "sha-2e25435", "configs": { "ubuntu": [ { diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index bbf6f8c39e..442a202a44 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -25,18 +25,11 @@ on: - unlabeled concurrency: - # Use a per-ref group so a newer run (a push, or a change to a label below) - # supersedes the in-progress one for that ref. Label events we don't act on get - # their own unique group (per run id) instead, keeping them out of the shared - # group so real builds keep running. Keep this list in sync with `should-run`. - group: >- - ${{ github.workflow }}-${{ github.ref }}${{ - ((github.event.action == 'labeled' || github.event.action == 'unlabeled') - && github.event.label.name != 'Ready to merge' - && github.event.label.name != 'DraftRunCI' - && github.event.label.name != 'Full CI build') - && format('-{0}', github.run_id) || '' - }} + # A single per-ref group with cancel-in-progress means any newer run (a push + # or a label change) supersedes the in-progress one for that ref. Keeping + # exactly one authoritative run per ref ensures a fast do-nothing run can never + # mask a real build's checks. + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true defaults: @@ -44,21 +37,17 @@ defaults: shell: bash jobs: - # This job determines whether the rest of the workflow should run. It runs - # when the PR is not a draft (which should also cover merge-group) or has the - # 'DraftRunCI' or 'Full CI build' label. For label events it only runs when the - # label added or removed is one we act on ('Ready to merge', 'DraftRunCI' or - # 'Full CI build'), so unrelated label changes do not trigger a redundant run. + # This job determines whether the rest of the workflow should run at all, + # based on the current set of labels: it runs when the PR is not a draft + # (which should also cover merge-group) or has the 'DraftRunCI' or + # 'Full CI build' label. Whether a build then happens, and whether it is the + # minimal or full matrix, is decided further below and in the strategy matrix. should-run: if: >- ${{ - ((github.event.action != 'labeled' && github.event.action != 'unlabeled') - || github.event.label.name == 'Ready to merge' - || github.event.label.name == 'DraftRunCI' - || github.event.label.name == 'Full CI build') - && (!github.event.pull_request.draft - || contains(github.event.pull_request.labels.*.name, 'DraftRunCI') - || contains(github.event.pull_request.labels.*.name, 'Full CI build')) + !github.event.pull_request.draft + || contains(github.event.pull_request.labels.*.name, 'DraftRunCI') + || contains(github.event.pull_request.labels.*.name, 'Full CI build') }} runs-on: ubuntu-latest steps: diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index 1ac8d61655..90182e7cbb 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -41,13 +41,13 @@ env: jobs: build: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-e29b523 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-2e25435 steps: - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@64ec3cf3b152b4444638f470bbd6df7a7a30c81c + uses: XRPLF/actions/prepare-runner@ad188deb3dae79dc39816e16ddfdad1e06c6fab2 with: enable_ccache: false diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index b2327f67ea..d4135207fe 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -113,7 +113,7 @@ jobs: uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@64ec3cf3b152b4444638f470bbd6df7a7a30c81c + uses: XRPLF/actions/prepare-runner@ad188deb3dae79dc39816e16ddfdad1e06c6fab2 with: enable_ccache: ${{ inputs.ccache_enabled }} @@ -324,6 +324,23 @@ jobs: LD_PRELOAD="$PRELOAD" ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log + # 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 it is skipped for instrumented builds + # (sanitizers/coverage/voidstar), where it would be slow and meaningless, + # and on Windows, where the `install` target does not build them. + - name: Run the benchmarks + if: ${{ !inputs.build_only && runner.os != 'Windows' && env.SANITIZERS_ENABLED == 'false' && env.COVERAGE_ENABLED != 'true' && env.VOIDSTAR_ENABLED != 'true' }} + working-directory: ${{ env.BUILD_DIR }} + run: | + rc=0 + while IFS= read -r bench; do + echo "::group::${bench}" + "./${bench}" --benchmark_repetitions=1 || rc=1 + echo "::endgroup::" + done < <(find src/benchmarks -type f -perm -u+x -name 'xrpl.bench.*') + exit "${rc}" + - name: Show test failure summary if: ${{ failure() && !inputs.build_only }} env: diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index a6bbe669ce..4a10e4b0f7 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -34,7 +34,7 @@ jobs: needs: [determine-files] if: ${{ needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.need_full_run == 'true' }} runs-on: ["self-hosted", "Linux", "X64", "heavy"] - container: "ghcr.io/xrplf/xrpld/nix-debian:sha-e29b523" + container: "ghcr.io/xrplf/xrpld/nix-debian:sha-2e25435" permissions: contents: read issues: write @@ -43,7 +43,7 @@ jobs: uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@64ec3cf3b152b4444638f470bbd6df7a7a30c81c + uses: XRPLF/actions/prepare-runner@ad188deb3dae79dc39816e16ddfdad1e06c6fab2 with: enable_ccache: false diff --git a/.github/workflows/reusable-upload-recipe.yml b/.github/workflows/reusable-upload-recipe.yml index feeee0a621..07077163ed 100644 --- a/.github/workflows/reusable-upload-recipe.yml +++ b/.github/workflows/reusable-upload-recipe.yml @@ -40,7 +40,7 @@ defaults: jobs: upload: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-e29b523 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-2e25435 env: REMOTE_NAME: ${{ inputs.remote_name }} CONAN_LOGIN_USERNAME_XRPLF: ${{ secrets.remote_username }} diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 22ad36d98f..abc0867b15 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -68,7 +68,7 @@ jobs: uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@64ec3cf3b152b4444638f470bbd6df7a7a30c81c + uses: XRPLF/actions/prepare-runner@ad188deb3dae79dc39816e16ddfdad1e06c6fab2 with: enable_ccache: false diff --git a/CMakeLists.txt b/CMakeLists.txt index 1e8befcc8f..f2e8fb3ae5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -131,6 +131,10 @@ else() endif() target_link_libraries(xrpl_libs INTERFACE ${nudb}) +if(benchmark) + find_package(benchmark REQUIRED) +endif() + if(coverage) include(XrplCov) endif() @@ -145,3 +149,7 @@ if(tests) include(CTest) add_subdirectory(src/tests/libxrpl) endif() + +if(benchmark) + add_subdirectory(src/benchmarks/libxrpl) +endif() diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9929b2eb39..7632741e35 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -83,6 +83,7 @@ If you create new source files, they must be organized as follows: `src/libxrpl`. - All other non-test files must go under `src/xrpld`. - All test source files must go under `src/test`. +- All benchmark source files must go under `src/benchmarks`. The source must be formatted according to the style guide below. The easiest way to satisfy this is to install the [`pre-commit`](#pre-commit-hooks) hooks, diff --git a/cmake/XrplAddBenchmark.cmake b/cmake/XrplAddBenchmark.cmake new file mode 100644 index 0000000000..1dd875dd61 --- /dev/null +++ b/cmake/XrplAddBenchmark.cmake @@ -0,0 +1,36 @@ +include(isolate_headers) + +# Define a benchmark executable for the module `name`. +# +# This follows the same general pattern as other build helpers in this repo +# (e.g. `add_module`): create a target and isolate headers, but here the target +# is a benchmark executable and no `add_test(...)` is registered. +# +# `isolate_headers` exposes only `${CMAKE_CURRENT_SOURCE_DIR}/${name}` on the +# include path, rooted at `src`, so a benchmark's own headers are reached as +# `` and nothing else in the tree leaks in. +function(xrpl_add_benchmark name) + set(target ${PROJECT_NAME}.bench.${name}) + + file( + GLOB_RECURSE sources + CONFIGURE_DEPENDS + "${CMAKE_CURRENT_SOURCE_DIR}/${name}/*.cpp" + "${CMAKE_CURRENT_SOURCE_DIR}/${name}.cpp" + ) + add_executable(${target} ${ARGN} ${sources}) + + # Benchmark sources register cases through Google Benchmark's static + # registrars (anonymous-namespace lambdas). Merging several such files into + # one unity translation unit collides those internal-linkage entities, so + # keep benchmarks out of the unity build - mirroring xrpl.libpb in + # XrplCore.cmake. Each file compiles fine on its own. + set_target_properties(${target} PROPERTIES UNITY_BUILD OFF) + + isolate_headers( + ${target} + "${CMAKE_SOURCE_DIR}/src" + "${CMAKE_CURRENT_SOURCE_DIR}/${name}" + PRIVATE + ) +endfunction() diff --git a/cmake/XrplSettings.cmake b/cmake/XrplSettings.cmake index 757a596096..be9bf1fda2 100644 --- a/cmake/XrplSettings.cmake +++ b/cmake/XrplSettings.cmake @@ -30,6 +30,8 @@ if(tests) endif() endif() +option(benchmark "Build benchmarks" ON) + # 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 b6ddfa4e58..c6a4070c77 100644 --- a/conan.lock +++ b/conan.lock @@ -10,22 +10,23 @@ "rocksdb/10.5.1#4a197eca381a3e5ae8adf8cffa5aacd0%1782392413.075713", "re2/20251105#8579cfd0bda4daf0683f9e3898f964b4%1782392402.431897", "protobuf/6.33.5#ff253ead763bd8d9904a52979cd21e81%1782392410.233933", - "openssl/3.6.3#1163d4ddc603907084d08a6a0c6e580f%1782307150.583886", + "openssl/3.6.3#f806de8933e3bf6f01016c6a888cee2e%1783945160.863288", "nudb/2.0.9#11149c73f8f2baff9a0198fe25971fc7%1782392402.297166", - "mpt-crypto/0.4.0-rc2#a580f2f9ad0e795de696aa62d54fb9af%1782425834.488828", + "mpt-crypto/0.4.0-rc4#ffdba12f2332357f0d8b0ae944cfff52%1784138702.932355", "lz4/1.10.0#982d9b673900f665a1da109e09c17cab%1782392402.164188", "libiconv/1.17#9923bc6dc6f106646d6967e0039a5ada%1782392792.775744", "libbacktrace/cci.20210118#a7691bfccd8caaf66309df196790a5a1%1782392402.420732", "libarchive/3.8.7#c446109bd1f1d8ba7936c94189bc50e6%1782392403.066892", "jemalloc/5.3.1#1fc58d55316041f10fbc1e8a2eae632a%1776700028.228", "gtest/1.17.0#5224b3b3ff3b4ce1133cbdd27d53ee7d%1782392402.791979", - "grpc/1.81.1#5217e6ef0544c42b46f4af35d5e7f649%1782307148.845616", + "grpc/1.81.1#f729f6d75992d20f9c72828e9142d62f%1783945160.094135", "ed25519/2015.03#ae761bdc52730a843f0809bdf6c1b1f6%1782307148.15562", "date/3.0.4#862e11e80030356b53c2c38599ceb32b%1782392402.538492", "c-ares/1.34.6#545240bb1c40e2cacd4362d6b8967650%1782392402.681654", "bzip2/1.0.8#c470882369c2d95c5c77e970c0c7e321%1782392402.296732", "boost/1.91.0#ea540ca2133d831b560036aa24dece3c%1782392419.475605", - "abseil/20250127.0#bb0baf1f362bc4a725a24eddd419b8f7%1782307147.395833" + "benchmark/1.9.5#b885dc73ad67b40a55d45684d1c88ad1%1782736613.864841", + "abseil/20250127.0#9ef01c1451a8340f9022e46238c0fbb6%1783945159.651047" ], "build_requires": [ "zlib/1.3.2#1cb806da49011867778ffb6ac7190fcb%1782392402.122708", @@ -38,7 +39,7 @@ "b2/5.4.2#ffd6084a119587e70f11cd45d1a386e2%1782392402.624226", "automake/1.16.5#b91b7c384c3deaa9d535be02da14d04f%1755524470.56", "autoconf/2.71#51077f068e61700d65bb05541ea1e4b0%1731054366.86", - "abseil/20250127.0#bb0baf1f362bc4a725a24eddd419b8f7%1782307147.395833" + "abseil/20250127.0#9ef01c1451a8340f9022e46238c0fbb6%1783945159.651047" ], "python_requires": [], "overrides": { diff --git a/conanfile.py b/conanfile.py index db12dcb585..f883761f0e 100644 --- a/conanfile.py +++ b/conanfile.py @@ -15,6 +15,7 @@ class Xrpl(ConanFile): settings = "os", "compiler", "build_type", "arch" options = { "assertions": [True, False], + "benchmark": [True, False], "coverage": [True, False], "fPIC": [True, False], "jemalloc": [True, False], @@ -46,6 +47,7 @@ class Xrpl(ConanFile): default_options = { "assertions": False, + "benchmark": True, "coverage": False, "fPIC": True, "jemalloc": False, @@ -129,12 +131,14 @@ class Xrpl(ConanFile): self.options["boost"].without_cobalt = True def requirements(self): + if self.options.benchmark: + self.requires("benchmark/1.9.5") self.requires("boost/1.91.0", force=True, transitive_headers=True) self.requires("date/3.0.4", transitive_headers=True) if self.options.jemalloc: self.requires("jemalloc/5.3.1") self.requires("lz4/1.10.0", force=True) - self.requires("mpt-crypto/0.4.0-rc2", transitive_headers=True) + self.requires("mpt-crypto/0.4.0-rc4", transitive_headers=True) self.requires("protobuf/6.33.5", force=True) if self.options.rocksdb: self.requires("rocksdb/10.5.1") @@ -162,6 +166,7 @@ class Xrpl(ConanFile): def generate(self): tc = CMakeToolchain(self) tc.variables["tests"] = self.options.tests + tc.variables["benchmark"] = self.options.benchmark tc.variables["assert"] = self.options.assertions tc.variables["coverage"] = self.options.coverage tc.variables["jemalloc"] = self.options.jemalloc diff --git a/include/xrpl/ledger/helpers/LendingHelpers.h b/include/xrpl/ledger/helpers/LendingHelpers.h index e2605e9ab7..8e0d11cccb 100644 --- a/include/xrpl/ledger/helpers/LendingHelpers.h +++ b/include/xrpl/ledger/helpers/LendingHelpers.h @@ -263,10 +263,11 @@ constructLoanState( Number const& principalOutstanding, Number const& managementFeeOutstanding); -// Constructs a valid LoanState object from a Loan object, which always has -// rounded values +// Overload of constructLoanState() that reads the three tracked fields +// directly from a Loan ledger object, which always holds rounded values, +// rather than taking them as separate Number arguments. LoanState -constructRoundedLoanState(SLE::const_ref loan); +constructLoanState(SLE::const_ref loan); Number computeManagementFee( diff --git a/include/xrpl/protocol/STObject.h b/include/xrpl/protocol/STObject.h index a1cdff22e0..ad87d106c4 100644 --- a/include/xrpl/protocol/STObject.h +++ b/include/xrpl/protocol/STObject.h @@ -229,13 +229,6 @@ public: [[nodiscard]] AccountID getAccountID(SField const& field) const; - /** - * The account responsible for the authorization: the delegate when - * sfDelegate is present, otherwise the account. - */ - [[nodiscard]] AccountID - getInitiator() const; - [[nodiscard]] Blob getFieldVL(SField const& field) const; [[nodiscard]] STAmount const& diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index 989bd11c10..d329d42eee 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -142,9 +142,26 @@ public: TxnSql status, std::string const& escapedMetaData) const; - [[nodiscard]] std::vector const& + /** + * The IDs of the inner transactions of a Batch. + */ + [[nodiscard]] std::vector getBatchTransactionIDs() const; + /** + * The inner transactions of a Batch, built and validated at construction. + * Always seated for Batch STTx instances (construction throws if oversized). + */ + [[nodiscard]] std::vector> const& + getBatchTransactions() const; + + /** + * The account responsible for the authorization: the delegate when + * sfDelegate is present, otherwise the account. + */ + [[nodiscard]] AccountID + getInitiator() const; + [[nodiscard]] AccountID getFeePayerID() const; @@ -166,13 +183,16 @@ private: checkMultiSign(Rules const& rules, STObject const& sigObject) const; [[nodiscard]] std::expected - checkBatchSingleSign(STObject const& batchSigner) const; + checkBatchSingleSign(STObject const& batchSigner, std::vector const& txIds) const; [[nodiscard]] std::expected - checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const; + checkBatchMultiSign( + STObject const& batchSigner, + Rules const& rules, + std::vector const& txIds) const; void - buildBatchTxnIds(); + buildBatchTxns(); STBase* copy(std::size_t n, void* buf) const override; @@ -180,11 +200,11 @@ private: move(std::size_t n, void* buf) override; friend class detail::STVar; - std::optional> batchTxnIds_; + std::optional>> batchTxns_; }; bool -passesLocalChecks(STObject const& st, std::string&); +passesLocalChecks(STTx const& tx, std::string&); /** * Sterilize a transaction. diff --git a/include/xrpl/tx/transactors/system/Batch.h b/include/xrpl/tx/transactors/system/Batch.h index 6540005877..ceaa42e8e0 100644 --- a/include/xrpl/tx/transactors/system/Batch.h +++ b/include/xrpl/tx/transactors/system/Batch.h @@ -12,6 +12,7 @@ #include #include +#include namespace xrpl { @@ -39,6 +40,9 @@ public: static NotTEC checkSign(PreclaimContext const& ctx); + static TER + preclaim(PreclaimContext const& ctx); + TER doApply() override; @@ -76,6 +80,10 @@ private: // only be reached through Batch::checkSign. static NotTEC checkBatchSign(PreclaimContext const& ctx); + + // nullopt on overflow or oversized signer arrays. + static std::optional + calculateBaseFeeImpl(ReadView const& view, STTx const& tx); }; } // namespace xrpl diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 0000000000..dbc9e74c5d --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,8 @@ +# Rust toolchain pin for rustup-based CI runners and local development. +# rustup reads this file and installs the pinned toolchain (see the +# prepare-runner action in XRPLF/actions, which runs `rustup toolchain install`). +# NOTE: the Nix CI image and development shell ignore this file; its rustc comes from flake.lock. +[toolchain] +channel = "1.95" +components = ["rustfmt", "clippy"] +profile = "minimal" diff --git a/src/benchmarks/libxrpl/CMakeLists.txt b/src/benchmarks/libxrpl/CMakeLists.txt new file mode 100644 index 0000000000..ac751a0413 --- /dev/null +++ b/src/benchmarks/libxrpl/CMakeLists.txt @@ -0,0 +1,22 @@ +include(XrplAddBenchmark) + +# Benchmark requirements. +find_package(benchmark REQUIRED) + +# Custom target for all benchmarks defined in this file. +add_custom_target(xrpl.benchmarks) + +# Common library dependencies for every benchmark module. `benchmark_main` +# supplies a `main()` that parses the standard Google Benchmark CLI flags +# (`--benchmark_filter`, `--benchmark_format`, ...), so no per-module main.cpp +# is needed. +add_library(xrpl.imports.bench INTERFACE) +target_link_libraries( + xrpl.imports.bench + INTERFACE benchmark::benchmark_main xrpl.libxrpl +) + +# One benchmark executable for each module. +xrpl_add_benchmark(nodestore) +target_link_libraries(xrpl.bench.nodestore PRIVATE xrpl.imports.bench) +add_dependencies(xrpl.benchmarks xrpl.bench.nodestore) diff --git a/src/benchmarks/libxrpl/nodestore/Backend.cpp b/src/benchmarks/libxrpl/nodestore/Backend.cpp new file mode 100644 index 0000000000..7db0185053 --- /dev/null +++ b/src/benchmarks/libxrpl/nodestore/Backend.cpp @@ -0,0 +1,329 @@ +#include + +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl::NodeStore { +namespace { + +constexpr std::size_t kPoolSizes[] = {1000, 10000, 100000}; +constexpr int kThreadCounts[] = {1, 4, 8}; +constexpr std::size_t kBatchSize = 256; + +constexpr std::string_view kNamePrefix = "BM_Backend_"; +constexpr std::string_view kNameSeparator = "/"; + +struct RunState +{ + std::unique_ptr harness; + Batch present; // prefix-1 objects, eligible to be stored + Batch recent; // prefix-1 objects in the "future" key space + std::vector missing; // prefix-2 keys that are never stored + std::vector shuffle; // [0, poolSize) permutation for random-like access + std::size_t avgPayload = 0; // mean getData().size() over `present` + + void + release() + { + harness.reset(); + Batch{}.swap(present); + Batch{}.swap(recent); + std::vector{}.swap(missing); + std::vector{}.swap(shuffle); + } +}; + +struct SetupContext +{ + RunState& rs; + Backend& backend; + std::size_t poolSize; +}; + +struct IterateContext +{ + RunState& rs; + Backend& backend; + std::size_t index; + std::size_t poolSize; +}; + +struct Workload +{ + std::string_view name; + std::function setup; + std::function iterate; + bool reportBytes = false; // SetBytesProcessed from rs.avgPayload + bool clobber = true; // ClobberMemory after the loop (false for pure stores) + bool pinToPool = false; // pin iterations to one pool sweep instead of autotuning +}; + +// One store() per iteration. Iterations are pinned to one pool sweep (per +// thread) so the index never wraps past the pool - otherwise NuDB::doInsert +// swallows key_exists and the workload degenerates into duplicate-detection +// no-ops. +Workload const kInsert{ + .name = "Insert", + .setup = + [](SetupContext const& ctx) { + ctx.rs.present = makePool(1, ctx.poolSize); + ctx.rs.avgPayload = averagePayload(ctx.rs.present); + }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, backend, index, poolSize] = ctx; + backend.store(rs.present[index % poolSize]); + }, + .reportBytes = true, + .clobber = false, + .pinToPool = true, +}; + +// One fetch() of a present key (a hit) per iteration. +Workload const kFetch{ + .name = "Fetch", + .setup = + [](SetupContext const& ctx) { + ctx.rs.present = makePool(1, ctx.poolSize); + ctx.rs.avgPayload = averagePayload(ctx.rs.present); + prepopulate(ctx.backend, ctx.rs.present); + }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, backend, index, poolSize] = ctx; + std::shared_ptr result; + backend.fetch(rs.present[index % poolSize]->getHash(), &result); + benchmark::DoNotOptimize(result); + }, + .reportBytes = true, +}; + +// One fetch() of a never-stored key (a miss); the backend is left empty. +Workload const kMissing{ + .name = "Missing", + .setup = [](SetupContext const& ctx) { ctx.rs.missing = makeMissingKeys(ctx.poolSize); }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, backend, index, poolSize] = ctx; + std::shared_ptr result; + backend.fetch(rs.missing[index % poolSize], &result); + benchmark::DoNotOptimize(result); + }, +}; + +// 80% hits / 20% misses. The fetch index comes from a shuffle table so access +// is random-like without per-iteration RNG cost; sequential `index % poolSize` +// would be artificially cache-friendly to RocksDB's block cache. +Workload const kMixed{ + .name = "Mixed", + .setup = + [](SetupContext const& ctx) { + ctx.rs.present = makePool(1, ctx.poolSize); + ctx.rs.missing = makeMissingKeys(ctx.poolSize); + ctx.rs.shuffle = makeShuffle(ctx.poolSize, /*seed=*/1); + prepopulate(ctx.backend, ctx.rs.present); + }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, backend, index, poolSize] = ctx; + std::shared_ptr result; + auto const pick = rs.shuffle[index % poolSize]; + if (index % 5 == 0) + { + backend.fetch(rs.missing[pick], &result); + } + else + { + backend.fetch(rs.present[pick]->getHash(), &result); + } + benchmark::DoNotOptimize(result); + }, +}; + +// An xrpld-like cycle: a hit, a maybe-miss recent fetch, and a store. The +// recent fetch uses the shuffle table (not `slot`) so it doesn't fetch the item +// it's about to store this iteration - which would give an all-miss-then-hit +// step instead of a smooth ramp. The store walks sequentially so each recent +// object is stored once. +Workload const kWork{ + .name = "Work", + .setup = + [](SetupContext const& ctx) { + ctx.rs.present = makePool(1, ctx.poolSize); + ctx.rs.recent = makePool(1, ctx.poolSize, ctx.poolSize); + ctx.rs.shuffle = makeShuffle(ctx.poolSize, /*seed=*/2); + prepopulate(ctx.backend, ctx.rs.present); + }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, backend, index, poolSize] = ctx; + auto const slot = index % poolSize; + auto const pick = rs.shuffle[slot]; + + std::shared_ptr historical; + backend.fetch(rs.present[pick]->getHash(), &historical); + benchmark::DoNotOptimize(historical); + + std::shared_ptr recent; + backend.fetch(rs.recent[pick]->getHash(), &recent); + benchmark::DoNotOptimize(recent); + + backend.store(rs.recent[slot]); + }, + .clobber = true, + .pinToPool = true, +}; + +auto +makeRunner(Workload w, std::string cfg, std::shared_ptr rs) +{ + return [w = std::move(w), cfg = std::move(cfg), rs = std::move(rs)](benchmark::State& state) { + auto const poolSize = static_cast(state.range(0)); + if (state.thread_index() == 0) + { + rs->harness = std::make_unique(cfg); + w.setup( + SetupContext{.rs = *rs, .backend = *rs->harness->backend, .poolSize = poolSize}); + } + + std::size_t index = state.thread_index(); + for (auto _ : state) + { + w.iterate( + IterateContext{ + .rs = *rs, + .backend = *rs->harness->backend, + .index = index, + .poolSize = poolSize}); + index += state.threads(); + } + + if (w.clobber) + benchmark::ClobberMemory(); + + state.SetItemsProcessed(state.iterations()); + if (w.reportBytes) + state.SetBytesProcessed(static_cast(state.iterations() * rs->avgPayload)); + + if (state.thread_index() == 0) + rs->release(); + }; +} + +// Register workload `w` against backend `bc`, choosing the registration shape +// from `w.pinToPool`. +void +registerWorkload(BackendConfig const& bc, Workload const& w) +{ + std::string const cfg = bc.config; + std::string name{kNamePrefix}; + name += w.name; + name += kNameSeparator; + name += bc.name; + + if (!w.pinToPool) + { + auto rs = std::make_shared(); + auto* b = benchmark::RegisterBenchmark(name, makeRunner(w, cfg, rs)); + b->RangeMultiplier(10)->Range(kPoolSizes[0], kPoolSizes[std::size(kPoolSizes) - 1]); + b->Threads(1)->Threads(4)->Threads(8)->UseRealTime(); + + return; + } + + for (auto const poolSize : kPoolSizes) + { + for (auto const threads : kThreadCounts) + { + if (poolSize % static_cast(threads) != 0) + continue; + + auto rs = std::make_shared(); + benchmark::RegisterBenchmark(name, makeRunner(w, cfg, rs)) + ->Arg(poolSize) + ->Iterations(poolSize / static_cast(threads)) + ->Threads(threads) + ->UseRealTime(); + } + } +} + +// One storeBatch() of kBatchSize objects per iteration. Single-threaded: +// Backend::storeBatch must not run concurrently with itself or store(). +// Iterations are pinned to the batch count so the index never wraps into +// key_exists no-ops. Kept separate from Workload: batch slicing and the +// per-batch item/byte accounting don't fit the thread-axis mold. +void +registerStoreBatch(BackendConfig const& bc) +{ + std::string const cfg = bc.config; + std::string name{kNamePrefix}; + name += "StoreBatch"; + name += kNameSeparator; + name += bc.name; + for (auto const poolSize : kPoolSizes) + { + auto const numBatches = poolSize / kBatchSize; + if (numBatches == 0) + continue; + + auto rs = std::make_shared(); + benchmark::RegisterBenchmark( + name, + [rs, cfg](benchmark::State& state) { + auto const poolSize = static_cast(state.range(0)); + rs->harness = std::make_unique(cfg); + rs->present = makePool(1, poolSize); + rs->avgPayload = averagePayload(rs->present); + std::vector const batches = sliceBatches(rs->present, kBatchSize); + if (batches.empty()) + { + state.SkipWithError("pool smaller than one batch"); + return; + } + + std::size_t index = 0; + for (auto _ : state) + { + rs->harness->backend->storeBatch(batches[index % batches.size()]); + ++index; + } + + state.SetItemsProcessed(static_cast(state.iterations() * kBatchSize)); + state.SetBytesProcessed( + static_cast(state.iterations() * kBatchSize * rs->avgPayload)); + rs->release(); + }) + ->Arg(poolSize) + ->Iterations(numBatches); + } +} + +[[maybe_unused]] bool const kRegistered = [] { + auto const workloads = std::to_array({&kInsert, &kFetch, &kMissing, &kMixed, &kWork}); + for (auto const& bc : backendConfigs()) + { + for (auto const* w : workloads) + registerWorkload(bc, *w); + + registerStoreBatch(bc); + } + return true; +}(); + +} // namespace +} // namespace xrpl::NodeStore diff --git a/src/benchmarks/libxrpl/nodestore/Database.cpp b/src/benchmarks/libxrpl/nodestore/Database.cpp new file mode 100644 index 0000000000..2303075ab9 --- /dev/null +++ b/src/benchmarks/libxrpl/nodestore/Database.cpp @@ -0,0 +1,243 @@ +#include + +#include +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl::NodeStore { +namespace { + +// Number of distinct objects pre-generated per run. +constexpr std::size_t kDefaultPoolSize = 100000; + +// Async read threads the Database spawns. Unused by the synchronous fetch path +// these benchmarks take; kept fixed so runs are comparable. +constexpr int kReadThreads = 4; + +constexpr std::string_view kNamePrefix = "BM_Database_"; +constexpr std::string_view kNameSeparator = "/"; + +struct RunState +{ + std::unique_ptr harness; + Batch present; // prefix-1 objects, eligible to be stored + Batch recent; // prefix-1 objects in the "future" key space + std::vector missing; // prefix-2 keys that are never stored + std::vector shuffle; // [0, poolSize) permutation for random-like access + std::size_t avgPayload = 0; // mean getData().size() over `present` +}; + +struct SetupContext +{ + RunState& rs; + Database& db; + std::size_t poolSize; +}; + +struct IterateContext +{ + RunState& rs; + Database& db; + std::uint32_t seq; + std::size_t index; + std::size_t poolSize; +}; + +struct Workload +{ + std::string_view name; + std::function setup; + std::function iterate; + bool reportBytes = false; + bool pinIterations = false; +}; + +void +prepopulate(Database& db, Batch const& objects) +{ + auto const seq = db.earliestLedgerSeq(); + for (auto const& obj : objects) + { + Blob data(obj->getData()); + db.store(obj->getType(), std::move(data), obj->getHash(), seq); + } + db.sync(); +} + +// One store() per iteration; a fresh Blob copy is handed over each time. +Workload const kStore{ + .name = "Store", + .setup = + [](SetupContext const& ctx) { + ctx.rs.present = makePool(1, ctx.poolSize); + ctx.rs.avgPayload = averagePayload(ctx.rs.present); + }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, db, seq, index, poolSize] = ctx; + auto const& obj = rs.present[index % poolSize]; + Blob data(obj->getData()); + db.store(obj->getType(), std::move(data), obj->getHash(), seq); + }, + .reportBytes = true, + .pinIterations = true, +}; + +// One fetchNodeObject() of a stored key (a hit) per iteration. +Workload const kFetch{ + .name = "Fetch", + .setup = + [](SetupContext const& ctx) { + ctx.rs.present = makePool(1, ctx.poolSize); + ctx.rs.avgPayload = averagePayload(ctx.rs.present); + prepopulate(ctx.db, ctx.rs.present); + }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, db, seq, index, poolSize] = ctx; + auto obj = db.fetchNodeObject(rs.present[index % poolSize]->getHash(), seq); + benchmark::DoNotOptimize(obj); + }, + .reportBytes = true, +}; + +// One fetchNodeObject() of a never-stored key (a miss) per iteration. +Workload const kMissing{ + .name = "Missing", + .setup = [](SetupContext const& ctx) { ctx.rs.missing = makeMissingKeys(ctx.poolSize); }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, db, seq, index, poolSize] = ctx; + auto obj = db.fetchNodeObject(rs.missing[index % poolSize], seq); + benchmark::DoNotOptimize(obj); + }, +}; + +// 80% hits / 20% misses. The fetch index comes from a shuffle table so access +// is random-like without per-iteration RNG cost; sequential `index % poolSize` +// would be artificially cache-friendly. +Workload const kMixed{ + .name = "Mixed", + .setup = + [](SetupContext const& ctx) { + ctx.rs.present = makePool(1, ctx.poolSize); + ctx.rs.missing = makeMissingKeys(ctx.poolSize); + ctx.rs.shuffle = makeShuffle(ctx.poolSize, /*seed=*/1); + prepopulate(ctx.db, ctx.rs.present); + }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, db, seq, index, poolSize] = ctx; + auto const pick = rs.shuffle[index % poolSize]; + std::shared_ptr obj; + if (index % 5 == 0) + { + obj = db.fetchNodeObject(rs.missing[pick], seq); + } + else + { + obj = db.fetchNodeObject(rs.present[pick]->getHash(), seq); + } + benchmark::DoNotOptimize(obj); + }, +}; + +// An xrpld-like cycle: a hit, a maybe-miss recent fetch, and a store. The +// recent fetch uses the shuffle table (not `slot`) so it doesn't fetch the item +// it's about to store this iteration - which would give an all-miss-then-hit +// step instead of a smooth ramp. The store walks sequentially so each recent +// object is stored once. +Workload const kWork{ + .name = "Work", + .setup = + [](SetupContext const& ctx) { + ctx.rs.present = makePool(1, ctx.poolSize); + ctx.rs.recent = makePool(1, ctx.poolSize, ctx.poolSize); + ctx.rs.shuffle = makeShuffle(ctx.poolSize, /*seed=*/2); + prepopulate(ctx.db, ctx.rs.present); + }, + .iterate = + [](IterateContext const& ctx) { + auto& [rs, db, seq, index, poolSize] = ctx; + auto const slot = index % poolSize; + auto const pick = rs.shuffle[slot]; + + auto historical = db.fetchNodeObject(rs.present[pick]->getHash(), seq); + benchmark::DoNotOptimize(historical); + + auto recent = db.fetchNodeObject(rs.recent[pick]->getHash(), seq); + benchmark::DoNotOptimize(recent); + + auto const& obj = rs.recent[slot]; + Blob data(obj->getData()); + db.store(obj->getType(), std::move(data), obj->getHash(), seq); + }, + .pinIterations = true, +}; + +void +registerWorkload(BackendConfig const& bc, Workload const& w) +{ + auto rs = std::make_shared(); + std::string const cfg = bc.config; + std::string name{kNamePrefix}; + name += w.name; + name += kNameSeparator; + name += bc.name; + auto* b = benchmark::RegisterBenchmark(name, [rs, cfg, w](benchmark::State& state) { + auto const poolSize = static_cast(state.range(0)); + rs->harness = std::make_unique(cfg, kReadThreads); + auto& db = *rs->harness->db; + w.setup(SetupContext{.rs = *rs, .db = db, .poolSize = poolSize}); + auto const seq = db.earliestLedgerSeq(); + + std::size_t index = 0; + for (auto _ : state) + { + w.iterate( + IterateContext{ + .rs = *rs, .db = db, .seq = seq, .index = index, .poolSize = poolSize}); + ++index; + } + benchmark::ClobberMemory(); + + state.SetItemsProcessed(state.iterations()); + if (w.reportBytes) + { + state.SetBytesProcessed(static_cast(state.iterations() * rs->avgPayload)); + } + rs->harness.reset(); + }); + + b->Arg(kDefaultPoolSize); + + if (w.pinIterations) + b->Iterations(kDefaultPoolSize); +} + +[[maybe_unused]] bool const kRegistered = [] { + auto const workloads = std::to_array({&kStore, &kFetch, &kMissing, &kMixed, &kWork}); + for (auto const& bc : backendConfigs()) + { + for (auto const* w : workloads) + registerWorkload(bc, *w); + } + return true; +}(); + +} // namespace +} // namespace xrpl::NodeStore diff --git a/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h b/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h new file mode 100644 index 0000000000..fe6c2a350e --- /dev/null +++ b/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h @@ -0,0 +1,318 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// Shared helpers for the NodeStore benchmarks. +// +namespace xrpl::NodeStore { + +// Fill `bytes` of memory at `buffer` with random bits drawn from `g`. +template +inline void +rngcpy(void* buffer, std::size_t bytes, Generator& g) +{ + using result_type = typename Generator::result_type; + while (bytes >= sizeof(result_type)) + { + auto const v = g(); + std::memcpy(buffer, &v, sizeof(v)); + buffer = reinterpret_cast(buffer) + sizeof(v); + bytes -= sizeof(v); + } + + if (bytes > 0) + { + auto const v = g(); + std::memcpy(buffer, &v, bytes); + } +} + +/** + * @brief Deterministic generator of a reproducible sequence of random NodeObjects. + * + * Indexing is stable: `obj(n)` and `key(n)` always return the same value for a + * given `n`, regardless of call order, because the engine is reseeded from `n` + * on every call. + * + * Using different prefixes guarantees the two key spaces are disjoint for the fetch-miss + * workloads. + */ +class Sequence +{ +private: + static constexpr auto kMinSize = 250; + static constexpr auto kMaxSize = 1250; + + beast::xor_shift_engine gen_; + std::uint8_t prefix_; + std::discrete_distribution dType_; + std::uniform_int_distribution dSize_; + +public: + explicit Sequence(std::uint8_t prefix) + : prefix_(prefix) + // uniform distribution over hotLEDGER - hotTRANSACTION_NODE + // but exclude hotTRANSACTION = 2 (removed) + , dType_({1, 1, 0, 1, 1}) + , dSize_(kMinSize, kMaxSize) + { + } + + // Returns the n-th key. Used to generate keys that are never stored. + // The layout mirrors obj()'s: prefix at byte 0, RNG over the rest, so the + // two key spaces stay disjoint by construction (not by coincidence). + uint256 + key(std::size_t n) + { + gen_.seed(n + 1); + uint256 result; + auto const data = static_cast(&*result.begin()); + *data = prefix_; + rngcpy(data + 1, result.size() - 1, gen_); + return result; + } + + // Returns the n-th complete NodeObject. + std::shared_ptr + obj(std::size_t n) + { + gen_.seed(n + 1); + uint256 key; + auto const data = static_cast(&*key.begin()); + *data = prefix_; + rngcpy(data + 1, key.size() - 1, gen_); + Blob value(dSize_(gen_)); + rngcpy(&value[0], value.size(), gen_); + return NodeObject::createObject( + safeCast(dType_(gen_)), std::move(value), key); + } + + // Fills `b` with `size` consecutive NodeObjects starting at index `n`. + void + batch(std::size_t n, Batch& b, std::size_t size) + { + b.clear(); + b.reserve(size); + while ((size--) != 0u) + b.push_back(obj(n++)); + } +}; + +// Parse a comma-separated "key=value,key=value" string into a config Section. +inline Section +parseConfig(std::string const& s) +{ + Section section; + std::vector values; + boost::split(values, s, boost::algorithm::is_any_of(",")); + section.append(values); + return section; +} + +// Pre-generate `count` distinct objects from key space `prefix`, starting at +// sequence index `start`. +inline Batch +makePool(std::uint8_t prefix, std::size_t count, std::size_t start = 0) +{ + Sequence seq(prefix); + Batch pool; + pool.reserve(count); + for (std::size_t i = 0; i < count; ++i) + pool.push_back(seq.obj(start + i)); + return pool; +} + +// Pre-generate `count` keys disjoint from every `makePool(...)` object, for +// measuring fetches that miss. +inline std::vector +makeMissingKeys(std::size_t count) +{ + Sequence seq(2); + std::vector keys; + keys.reserve(count); + for (std::size_t i = 0; i < count; ++i) + keys.push_back(seq.key(i)); + return keys; +} + +// Mean payload size across a pool, used for SetBytesProcessed throughput. +inline std::size_t +averagePayload(Batch const& pool) +{ + if (pool.empty()) + return 0; + std::size_t total = 0; + for (auto const& obj : pool) + total += obj->getData().size(); + return total / pool.size(); +} + +// Store every object and flush, so a following fetch exercises the real read +// path rather than an in-memory write buffer. +// +// We chunk the write at kBatchWriteLimitSize because Types.h documents that as +// the maximum allowed batch size. NuDB happens to tolerate larger batches +// today, but the benchmark should not rely on that. +// +// sync() is a no-op for both NuDB and RocksDB at the moment (NuDB has a small +// internal burst buffer that the timed loop will warm up). That is a contract +// hint, not a guarantee; if either backend ever grows a real flush we get it +// here for free. +inline void +prepopulate(Backend& backend, Batch const& objects) +{ + for (std::size_t i = 0; i < objects.size(); i += kBatchWriteLimitSize) + { + auto const end = std::min(i + kBatchWriteLimitSize, objects.size()); + backend.storeBatch(Batch(objects.begin() + i, objects.begin() + end)); + } + backend.sync(); +} + +// A deterministic permutation of [0, size). Lets the timed loop visit the +// pre-generated pool in a random-like order with zero RNG cost per iteration - +// the Timing_test workloads it replaces used uniform_int_distribution per +// fetch, and a shuffle table reproduces that access pattern without paying for +// the distribution inside the timed region. +inline std::vector +makeShuffle(std::size_t size, std::uint64_t seed) +{ + std::vector v(size); + std::iota(v.begin(), v.end(), std::size_t{0}); + beast::xor_shift_engine gen(seed); + std::shuffle(v.begin(), v.end(), gen); + return v; +} + +// Partition a pool into fixed-size batches. Any trailing remainder shorter than +// `batchSize` is dropped, so every returned batch has exactly `batchSize`. +inline std::vector +sliceBatches(Batch const& pool, std::size_t batchSize) +{ + std::vector batches; + if (batchSize == 0) + return batches; + batches.reserve(pool.size() / batchSize); + for (std::size_t i = 0; i + batchSize <= pool.size(); i += batchSize) + batches.emplace_back(pool.begin() + i, pool.begin() + i + batchSize); + return batches; +} + +/** + * @brief RAII owner of a NodeStore Backend opened on a private temporary directory. + * + * Member declaration order matters: `tempDir` is declared first so it is + * destroyed last, after the backend has closed and released its files. + */ +struct BackendHarness +{ + beast::TempDir tempDir; + DummyScheduler scheduler; + beast::Journal journal{beast::Journal::getNullSink()}; + std::unique_ptr backend; + + explicit BackendHarness(std::string const& configString) + { + Section config = parseConfig(configString); + // A private, unique path per harness, so concurrent or repeated runs + // never share on-disk state. + config.set("path", tempDir.path()); + backend = + Manager::instance().makeBackend(config, megabytes(std::size_t{4}), scheduler, journal); + backend->setDeletePath(); + backend->open(); + } + + ~BackendHarness() + { + if (backend) + backend->close(); + } +}; + +/** + * RAII owner of a NodeStore Database - the application-facing wrapper around a + * Backend, which adds fetch/store accounting and the async read-thread pool. + */ +struct DatabaseHarness +{ + beast::TempDir tempDir; + DummyScheduler scheduler; + beast::Journal journal{beast::Journal::getNullSink()}; + std::unique_ptr db; + + DatabaseHarness(std::string const& configString, int readThreads) + { + Section config = parseConfig(configString); + config.set("path", tempDir.path()); + db = Manager::instance().makeDatabase( + megabytes(std::size_t{4}), scheduler, readThreads, config, journal); + } + + ~DatabaseHarness() + { + if (db) + db->stop(); + } +}; + +// A NodeStore backend to benchmark, named for the --benchmark_filter CLI flag. +struct BackendConfig +{ + char const* name; // short label, e.g. "nudb" + char const* config; // parseConfig() string, e.g. "type=nudb" +}; + +// The backends every workload is registered against. +// +// The in-memory backend is intentionally excluded. It keeps its table in a +// process-global map keyed by path, with no removal API, so building a fresh +// backend per run - as a microbenchmark must - would leak the whole dataset on +// every run. Timing_test, the suite this benchmark replaces, excluded it for +// the same reason. NuDB and RocksDB are the production backends worth timing. +// +// RocksDB is included only when it was compiled in (xrpl.libxrpl carries +// XRPL_ROCKSDB_AVAILABLE transitively). +inline std::vector const& +backendConfigs() +{ + static std::vector const kConfigs = { + {.name = "nudb", .config = "type=nudb"}, +#if XRPL_ROCKSDB_AVAILABLE + {.name = "rocksdb", + .config = "type=rocksdb,open_files=2000,filter_bits=12,cache_mb=256," + "file_size_mb=8,file_size_mult=2"}, +#endif + }; + return kConfigs; +} + +} // namespace xrpl::NodeStore diff --git a/src/libxrpl/ledger/helpers/LendingHelpers.cpp b/src/libxrpl/ledger/helpers/LendingHelpers.cpp index f7ec8a8bc3..e6c3d632c1 100644 --- a/src/libxrpl/ledger/helpers/LendingHelpers.cpp +++ b/src/libxrpl/ledger/helpers/LendingHelpers.cpp @@ -312,6 +312,25 @@ computeInterestAndFeeParts( return std::make_pair(interest - fee, fee); } +/* Rounds a raw (unrounded) interest amount to the loan's scale, then splits + * the rounded amount into net interest (to the vault) and management fee (to + * the broker). + * + * This is the common "round then split" step shared by late payment, full + * payment, and overpayment interest calculations. + */ +std::pair +roundAndSplitInterest( + Asset const& asset, + Number const& rawInterest, + TenthBips16 managementFeeRate, + std::int32_t loanScale, + Number::RoundingMode mode = Number::getround()) +{ + auto const interest = roundToAsset(asset, rawInterest, loanScale, mode); + return computeInterestAndFeeParts(asset, interest, managementFeeRate, loanScale); +} + /* Calculates penalty interest accrued on overdue payments. * Returns 0 if payment is not late. * @@ -387,22 +406,18 @@ loanAccruedInterest( * * This is the core function that updates the Loan ledger object fields based on * a computed payment. - - * The function is templated to work with both direct Number/uint32_t values - * (for testing/simulation) and ValueProxy types (for actual ledger updates). */ -template LoanPaymentParts -doPayment( - ExtendedPaymentComponents const& payment, - NumberProxy& totalValueOutstandingProxy, - NumberProxy& principalOutstandingProxy, - NumberProxy& managementFeeOutstandingProxy, - UInt32Proxy& paymentRemainingProxy, - UInt32Proxy& prevPaymentDateProxy, - UInt32OptionalProxy& nextDueDateProxy, - std::uint32_t paymentInterval) +doPayment(ExtendedPaymentComponents const& payment, SLE::ref loan) { + auto totalValueOutstandingProxy = loan->at(sfTotalValueOutstanding); + auto principalOutstandingProxy = loan->at(sfPrincipalOutstanding); + auto managementFeeOutstandingProxy = loan->at(sfManagementFeeOutstanding); + auto paymentRemainingProxy = loan->at(sfPaymentRemaining); + auto prevPaymentDateProxy = loan->at(sfPreviousPaymentDueDate); + auto nextDueDateProxy = loan->at(sfNextPaymentDueDate); + std::uint32_t const paymentInterval = loan->at(sfPaymentInterval); + XRPL_ASSERT_PARTS(nextDueDateProxy, "xrpl::detail::doPayment", "Next due date proxy set"); if (payment.specialCase == PaymentSpecialCase::Final) @@ -470,16 +485,12 @@ doPayment( // Principal can never exceed total value (principal is part of total value) XRPL_ASSERT_PARTS( - // Use an explicit cast because the template parameter can be - // ValueProxy or Number static_cast(principalOutstandingProxy) <= static_cast(totalValueOutstandingProxy), "xrpl::detail::doPayment", "principal does not exceed total"); XRPL_ASSERT_PARTS( - // Use an explicit cast because the template parameter can be - // ValueProxy or Number static_cast(managementFeeOutstandingProxy) >= beast::kZero, "xrpl::detail::doPayment", "fee outstanding stays valid"); @@ -717,22 +728,23 @@ tryOverpayment( * overpayment would leave the loan in an invalid state, we can reject it * gracefully without corrupting the ledger data. */ -template std::expected doOverpayment( Rules const& rules, Asset const& asset, std::int32_t loanScale, ExtendedPaymentComponents const& overpaymentComponents, - NumberProxy& totalValueOutstandingProxy, - NumberProxy& principalOutstandingProxy, - NumberProxy& managementFeeOutstandingProxy, - NumberProxy& periodicPaymentProxy, + SLE::ref loan, Number const& periodicRate, - std::uint32_t const paymentRemaining, TenthBips16 const managementFeeRate, beast::Journal j) { + auto totalValueOutstandingProxy = loan->at(sfTotalValueOutstanding); + auto principalOutstandingProxy = loan->at(sfPrincipalOutstanding); + auto managementFeeOutstandingProxy = loan->at(sfManagementFeeOutstanding); + auto periodicPaymentProxy = loan->at(sfPeriodicPayment); + auto const paymentsRemaining = loan->at(sfPaymentRemaining); + auto const loanState = constructLoanState( totalValueOutstandingProxy, principalOutstandingProxy, managementFeeOutstandingProxy); auto const periodicPayment = periodicPaymentProxy; @@ -744,7 +756,7 @@ doOverpayment( << ", interestPart: " << overpaymentComponents.trackedInterestPart() << ", untrackedInterest: " << overpaymentComponents.untrackedInterest << ", totalDue: " << overpaymentComponents.totalDue - << ", payments remaining :" << paymentRemaining; + << ", payments remaining :" << paymentsRemaining; // Attempt to re-amortize the loan with the overpayment applied. // This modifies the temporary copies, leaving the proxies unchanged. @@ -756,7 +768,7 @@ doOverpayment( loanState, periodicPayment, periodicRate, - paymentRemaining, + paymentsRemaining, managementFeeRate, j); if (!ret) @@ -864,16 +876,15 @@ std::expected computeLatePayment( Asset const& asset, ApplyView const& view, - Number const& principalOutstanding, - std::int32_t nextDueDate, + SLE::const_ref loan, ExtendedPaymentComponents const& periodic, - TenthBips32 lateInterestRate, - std::int32_t loanScale, - Number const& latePaymentFee, STAmount const& amount, TenthBips16 managementFeeRate, beast::Journal j) { + std::int32_t const nextDueDate = loan->at(sfNextPaymentDueDate); + std::int32_t const loanScale = loan->at(sfLoanScale); + // Check if the due date has passed. If not, reject the payment as // being too soon if (!hasExpired(view, nextDueDate)) @@ -881,15 +892,15 @@ computeLatePayment( // Calculate the penalty interest based on how long the payment is overdue. auto const latePaymentInterest = loanLatePaymentInterest( - principalOutstanding, lateInterestRate, view.parentCloseTime(), nextDueDate); + loan->at(sfPrincipalOutstanding), + TenthBips32{loan->at(sfLateInterestRate)}, + view.parentCloseTime(), + nextDueDate); // Round the late interest and split it between the vault (net interest) - // and the broker (management fee portion). This lambda ensures we - // round before splitting to maintain precision. - auto const [roundedLateInterest, roundedLateManagementFee] = [&]() { - auto const interest = roundToAsset(asset, latePaymentInterest, loanScale); - return computeInterestAndFeeParts(asset, interest, managementFeeRate, loanScale); - }(); + // and the broker (management fee portion). + auto const [roundedLateInterest, roundedLateManagementFee] = + roundAndSplitInterest(asset, latePaymentInterest, managementFeeRate, loanScale); XRPL_ASSERT(roundedLateInterest >= 0, "xrpl::detail::computeLatePayment : valid late interest"); XRPL_ASSERT_PARTS( @@ -908,7 +919,7 @@ computeLatePayment( // 1. Regular service fee (from periodic.untrackedManagementFee) // 2. Late payment fee (fixed penalty) // 3. Management fee portion of late interest - periodic.untrackedManagementFee + latePaymentFee + roundedLateManagementFee, + periodic.untrackedManagementFee + loan->at(sfLatePaymentFee) + roundedLateManagementFee, // Untracked interest includes: // 1. Any untracked interest from the regular payment (usually 0) @@ -958,22 +969,15 @@ std::expected computeFullPayment( Asset const& asset, ApplyView& view, - Number const& principalOutstanding, - Number const& managementFeeOutstanding, - Number const& periodicPayment, - std::uint32_t paymentRemaining, - std::uint32_t prevPaymentDate, - std::uint32_t const startDate, - std::uint32_t const paymentInterval, - TenthBips32 const closeInterestRate, - std::int32_t loanScale, - Number const& totalInterestOutstanding, + SLE::const_ref loan, Number const& periodicRate, - Number const& closePaymentFee, STAmount const& amount, TenthBips16 managementFeeRate, beast::Journal j) { + std::uint32_t const paymentRemaining = loan->at(sfPaymentRemaining); + std::int32_t const loanScale = loan->at(sfLoanScale); + // Full payment must be made before the final scheduled payment. if (paymentRemaining <= 1) { @@ -986,7 +990,7 @@ computeFullPayment( // This theoretical (unrounded) value is used to compute interest and // penalties accurately. Number const theoreticalPrincipalOutstanding = loanPrincipalFromPeriodicPayment( - view.rules(), periodicPayment, periodicRate, paymentRemaining); + view.rules(), loan->at(sfPeriodicPayment), periodicRate, paymentRemaining); // Full payment interest includes both accrued interest (time since last // payment) and prepayment penalty (for closing early). @@ -994,18 +998,21 @@ computeFullPayment( theoreticalPrincipalOutstanding, periodicRate, view.parentCloseTime(), - paymentInterval, - prevPaymentDate, - startDate, - closeInterestRate); + loan->at(sfPaymentInterval), + loan->at(sfPreviousPaymentDueDate), + loan->at(sfStartDate), + TenthBips32{loan->at(sfCloseInterestRate)}); - // Split the full payment interest into net interest (to vault) and - // management fee (to broker), applying proper rounding. - auto const [roundedFullInterest, roundedFullManagementFee] = [&]() { - auto const interest = - roundToAsset(asset, fullPaymentInterest, loanScale, Number::RoundingMode::Downward); - return computeInterestAndFeeParts(asset, interest, managementFeeRate, loanScale); - }(); + // Split the full payment interest into net interest (to vault) and management fee (to broker), + // applying proper rounding. + auto const [roundedFullInterest, roundedFullManagementFee] = roundAndSplitInterest( + asset, fullPaymentInterest, managementFeeRate, loanScale, Number::RoundingMode::Downward); + + LoanState const loanState = constructLoanState(loan); + Number const principalOutstanding = loanState.principalOutstanding; + Number const managementFeeOutstanding = loanState.managementFeeDue; + Number const totalInterestOutstanding = loanState.interestDue; + Number const closePaymentFee = roundToAsset(asset, loan->at(sfClosePaymentFee), loanScale); ExtendedPaymentComponents const full{ PaymentComponents{ @@ -1046,8 +1053,7 @@ computeFullPayment( "xrpl::detail::computeFullPayment", "total due is rounded"); - JLOG(j.trace()) << "computeFullPayment result: periodicPayment: " << periodicPayment - << ", periodicRate: " << periodicRate + JLOG(j.trace()) << "computeFullPayment result: periodicRate: " << periodicRate << ", paymentRemaining: " << paymentRemaining << ", theoreticalPrincipalOutstanding: " << theoreticalPrincipalOutstanding << ", fullPaymentInterest: " << fullPaymentInterest @@ -1298,6 +1304,34 @@ computePaymentComponents( }; } +/* Thin overload of computePaymentComponents() that unwraps the tracked + * fields directly from the Loan ledger object. `periodicRate` is derived + * rather than stored, and `managementFeeRate` comes from the LoanBroker, not + * the Loan, so both remain explicit parameters. Kept separate from the + * value-based overload above, which is exercised directly by unit tests + * against simulated (non-ledger) loan states. + */ +PaymentComponents +computePaymentComponents( + Rules const& rules, + Asset const& asset, + SLE::ref loan, + Number const& periodicRate, + TenthBips16 managementFeeRate) +{ + return computePaymentComponents( + rules, + asset, + loan->at(sfLoanScale), + loan->at(sfTotalValueOutstanding), + loan->at(sfPrincipalOutstanding), + loan->at(sfManagementFeeOutstanding), + loan->at(sfPeriodicPayment), + periodicRate, + loan->at(sfPaymentRemaining), + managementFeeRate); +} + /* Computes payment components for an overpayment scenario. * * An overpayment occurs when a borrower pays more than the scheduled periodic @@ -1342,11 +1376,12 @@ computeOverpaymentComponents( // This interest doesn't follow the normal amortization schedule - it's // a one-time charge for paying early. // Equation (20) and (21) from XLS-66 spec, Section A-2 Equation Glossary - auto const [roundedOverpaymentInterest, roundedOverpaymentManagementFee] = [&]() { - auto const interest = - roundToAsset(asset, tenthBipsOfValue(overpayment, overpaymentInterestRate), loanScale); - return detail::computeInterestAndFeeParts(asset, interest, managementFeeRate, loanScale); - }(); + auto const [roundedOverpaymentInterest, roundedOverpaymentManagementFee] = + roundAndSplitInterest( + asset, + tenthBipsOfValue(overpayment, overpaymentInterestRate), + managementFeeRate, + loanScale); auto const result = detail::ExtendedPaymentComponents{ // Build the payment components, after fees and penalty @@ -1373,6 +1408,265 @@ computeOverpaymentComponents( return result; } +/* Derives the two rate values every make*Payment() helper needs: the + * broker's management fee rate, and the loan's periodic (per-payment-period) + * interest rate. + */ +std::pair +loanRatesFor(SLE::const_ref loan, SLE::const_ref brokerSle) +{ + TenthBips16 const managementFeeRate{brokerSle->at(sfManagementFeeRate)}; + TenthBips32 const interestRate{loan->at(sfInterestRate)}; + Number const periodicRate = loanPeriodicRate(interestRate, loan->at(sfPaymentInterval)); + XRPL_ASSERT(interestRate == 0 || periodicRate > 0, "xrpl::detail::loanRatesFor : valid rate"); + return {managementFeeRate, periodicRate}; +} + +/* Handles a full (early payoff) payment. Implements the "full payment" + * branch of the make_payment function from the XLS-66 spec, Section + * 3.2.4.4. + */ +std::expected +makeFullPayment( + Asset const& asset, + ApplyView& view, + SLE::ref loan, + SLE::const_ref brokerSle, + STAmount const& amount, + beast::Journal j) +{ + auto const [managementFeeRate, periodicRate] = loanRatesFor(loan, brokerSle); + + auto const fullPaymentComponents = + computeFullPayment(asset, view, loan, periodicRate, amount, managementFeeRate, j); + + // computeFullPayment only ever fails with a genuine error TER (never + // tesSUCCESS), so there is no separate "no-op" outcome to handle here. + if (fullPaymentComponents.has_value()) + return doPayment(*fullPaymentComponents, loan); + return std::unexpected(fullPaymentComponents.error()); +} + +/* Handles a late payment (past due date, with the late-payment flag set). + * Implements the "late payment" branch of the make_payment function from + * the XLS-66 spec, Section 3.2.4.4. + */ +std::expected +makeLatePayment( + Asset const& asset, + ApplyView const& view, + SLE::ref loan, + SLE::const_ref brokerSle, + STAmount const& amount, + beast::Journal j) +{ + auto const [managementFeeRate, periodicRate] = loanRatesFor(loan, brokerSle); + + Number const serviceFee = loan->at(sfLoanServiceFee); + ExtendedPaymentComponents const periodic{ + computePaymentComponents(view.rules(), asset, loan, periodicRate, managementFeeRate), + serviceFee}; + XRPL_ASSERT_PARTS( + periodic.trackedPrincipalDelta >= 0, + "xrpl::detail::makeLatePayment", + "regular payment valid principal"); + + auto const latePaymentComponents = + computeLatePayment(asset, view, loan, periodic, amount, managementFeeRate, j); + + // computeLatePayment only ever fails with a genuine error TER (never + // tesSUCCESS), so there is no separate "no-op" outcome to handle here. + if (latePaymentComponents.has_value()) + return doPayment(*latePaymentComponents, loan); + return std::unexpected(latePaymentComponents.error()); +} + +/* Handles regular scheduled payments, including an optional overpayment tail. + * Implements the "regular" and "overpayment" branches of the make_payment + * function from the XLS-66 spec, Section 3.2.4.4. + */ +std::expected +makeRegularPayment( + Asset const& asset, + ApplyView const& view, + SLE::ref loan, + SLE::const_ref brokerSle, + STAmount const& amount, + LoanPaymentType const paymentType, + beast::Journal j) +{ + using namespace Lending; + + XRPL_ASSERT_PARTS( + paymentType == LoanPaymentType::Regular || paymentType == LoanPaymentType::Overpayment, + "xrpl::detail::makeRegularPayment", + "regular payment type"); + + auto const [managementFeeRate, periodicRate] = loanRatesFor(loan, brokerSle); + + std::int32_t const loanScale = loan->at(sfLoanScale); + Number const serviceFee = loan->at(sfLoanServiceFee); + + ExtendedPaymentComponents periodic{ + computePaymentComponents(view.rules(), asset, loan, periodicRate, managementFeeRate), + serviceFee}; + XRPL_ASSERT_PARTS( + periodic.trackedPrincipalDelta >= 0, + "xrpl::detail::makeRegularPayment", + "regular payment valid principal"); + + // Keep a running total of the actual parts paid + LoanPaymentParts totalParts; + Number totalPaid = kNumZero; + std::size_t numPayments = 0; + + // Cached here (rather than re-looking up loan->at(sfPaymentRemaining) at each use) since it's + // read multiple times below. It's a write-through proxy, so it still reflects doPayment's + // mutations each iteration. + auto paymentRemainingProxy = loan->at(sfPaymentRemaining); + + while ((amount >= (totalPaid + periodic.totalDue)) && paymentRemainingProxy > 0 && + numPayments < kLoanMaximumPaymentsPerTransaction) + { + // Try to make more payments + XRPL_ASSERT_PARTS( + periodic.trackedPrincipalDelta >= 0, + "xrpl::detail::makeRegularPayment", + "payment pays non-negative principal"); + + totalPaid += periodic.totalDue; + totalParts += doPayment(periodic, loan); + ++numPayments; + + XRPL_ASSERT_PARTS( + (periodic.specialCase == PaymentSpecialCase::Final) == (paymentRemainingProxy == 0), + "xrpl::detail::makeRegularPayment", + "final payment is the final payment"); + + // Don't compute the next payment if this was the last payment + if (periodic.specialCase == PaymentSpecialCase::Final) + break; + + periodic = ExtendedPaymentComponents{ + computePaymentComponents(view.rules(), asset, loan, periodicRate, managementFeeRate), + serviceFee}; + } + + if (numPayments == 0) + { + JLOG(j.warn()) << "Regular loan payment amount is insufficient. Due: " << periodic.totalDue + << ", paid: " << amount; + return std::unexpected(tecINSUFFICIENT_PAYMENT); + } + + XRPL_ASSERT_PARTS( + totalParts.principalPaid + totalParts.interestPaid + totalParts.feePaid == totalPaid, + "xrpl::detail::makeRegularPayment", + "payment parts add up"); + XRPL_ASSERT_PARTS( + totalParts.valueChange == 0, "xrpl::detail::makeRegularPayment", "no value change"); + + // ------------------------------------------------------------- + // overpayment handling + // + // If the "fixCleanup3_1_3" amendment is enabled, truncate "amount", + // at the loan scale. If the raw value is used, the overpayment + // amount could be meaningless dust. Trying to process such a small + // amount will, at best, waste time when all the result values round + // to zero. At worst, it can cause logical errors with tiny amounts + // of interest that don't add up correctly. + auto const roundedAmount = view.rules().enabled(fixCleanup3_1_3) + ? roundToAsset(asset, amount, loanScale, Number::RoundingMode::TowardsZero) + : amount; + + bool const overpaymentSupported = + paymentType == LoanPaymentType::Overpayment && loan->isFlag(lsfLoanOverpayment); + + bool const overpaymentAllowed = // + paymentRemainingProxy > 0 && // + totalPaid < roundedAmount && // + numPayments < kLoanMaximumPaymentsPerTransaction; + + if (overpaymentSupported && overpaymentAllowed) + { + TenthBips32 const overpaymentInterestRate{loan->at(sfOverpaymentInterestRate)}; + TenthBips32 const overpaymentFeeRate{loan->at(sfOverpaymentFee)}; + + // It shouldn't be possible for the overpayment to be greater than + // totalValueOutstanding, because that would have been processed as + // another normal payment. But cap it just in case. + Number const overpaymentRaw = + std::min(roundedAmount - totalPaid, *loan->at(sfTotalValueOutstanding)); + + bool const fixEnabled = view.rules().enabled(fixCleanup3_2_0); + Number const overpayment = fixEnabled + ? roundToAsset(asset, overpaymentRaw, loanScale, Number::RoundingMode::Downward) + : overpaymentRaw; + + // Post-amendment, the rounded overpayment can be zero; pre-amendment + // it's always positive given the surrounding guards. + if (!fixEnabled || overpayment > 0) + { + ExtendedPaymentComponents const overpaymentComponents = computeOverpaymentComponents( + view.rules(), + asset, + loanScale, + overpayment, + overpaymentInterestRate, + overpaymentFeeRate, + managementFeeRate); + + // Don't process an overpayment if the whole amount (or more!) + // gets eaten by fees and interest. + if (overpaymentComponents.trackedPrincipalDelta > 0) + { + XRPL_ASSERT_PARTS( + overpaymentComponents.untrackedInterest >= beast::kZero, + "xrpl::detail::makeRegularPayment", + "overpayment penalty did not reduce value of loan"); + if (auto const overResult = doOverpayment( + view.rules(), + asset, + loanScale, + overpaymentComponents, + loan, + periodicRate, + managementFeeRate, + j)) + { + totalParts += *overResult; + } + else if (overResult.error()) + { + // error() will be the TER returned if a payment is not + // made. It will only evaluate to true if it's unsuccessful. + // Otherwise, tesSUCCESS means nothing was done, so + // continue. + return std::unexpected(overResult.error()); + } + } + } + } + + // Check the final results are rounded, to double-check that the + // intermediate steps were rounded. + XRPL_ASSERT( + isRounded(asset, totalParts.principalPaid, loanScale) && + totalParts.principalPaid >= beast::kZero, + "xrpl::detail::makeRegularPayment : total principal paid is valid"); + XRPL_ASSERT( + isRounded(asset, totalParts.interestPaid, loanScale) && + totalParts.interestPaid >= beast::kZero, + "xrpl::detail::makeRegularPayment : total interest paid is valid"); + XRPL_ASSERT( + isRounded(asset, totalParts.valueChange, loanScale), + "xrpl::detail::makeRegularPayment : loan value change is valid"); + XRPL_ASSERT( + isRounded(asset, totalParts.feePaid, loanScale) && totalParts.feePaid >= beast::kZero, + "xrpl::detail::makeRegularPayment : fee paid is valid"); + return totalParts; +} + } // namespace detail detail::LoanStateDeltas @@ -1632,8 +1926,10 @@ constructLoanState( } LoanState -constructRoundedLoanState(SLE::const_ref loan) +constructLoanState(SLE::const_ref loan) { + XRPL_ASSERT(loan && loan->getType() == ltLOAN, "xrpl::constructLoanState : valid loan SLE"); + return constructLoanState( loan->at(sfTotalValueOutstanding), loan->at(sfPrincipalOutstanding), @@ -1790,12 +2086,7 @@ loanMakePayment( LoanPaymentType const paymentType, beast::Journal j) { - using namespace Lending; - - auto principalOutstandingProxy = loan->at(sfPrincipalOutstanding); - auto paymentRemainingProxy = loan->at(sfPaymentRemaining); - - if (paymentRemainingProxy == 0 || principalOutstandingProxy == 0) + if (loan->at(sfPaymentRemaining) == 0 || loan->at(sfPrincipalOutstanding) == 0) { // Loan complete this is already checked in LoanPay::preclaim() // LCOV_EXCL_START @@ -1804,9 +2095,6 @@ loanMakePayment( // LCOV_EXCL_STOP } - auto totalValueOutstandingProxy = loan->at(sfTotalValueOutstanding); - auto managementFeeOutstandingProxy = loan->at(sfManagementFeeOutstanding); - // Next payment due date must be set unless the loan is complete auto nextDueDateProxy = loan->at(sfNextPaymentDueDate); if (*nextDueDateProxy == 0) @@ -1815,26 +2103,8 @@ loanMakePayment( return std::unexpected(tecINTERNAL); } - std::int32_t const loanScale = loan->at(sfLoanScale); - - TenthBips32 const interestRate{loan->at(sfInterestRate)}; - - Number const serviceFee = loan->at(sfLoanServiceFee); - TenthBips16 const managementFeeRate{brokerSle->at(sfManagementFeeRate)}; - - Number const periodicPayment = loan->at(sfPeriodicPayment); - - auto prevPaymentDateProxy = loan->at(sfPreviousPaymentDueDate); - std::uint32_t const startDate = loan->at(sfStartDate); - - std::uint32_t const paymentInterval = loan->at(sfPaymentInterval); - - // Compute the periodic rate that will be used for calculations - // throughout - Number const periodicRate = loanPeriodicRate(interestRate, paymentInterval); - XRPL_ASSERT(interestRate == 0 || periodicRate > 0, "xrpl::loanMakePayment : valid rate"); - - XRPL_ASSERT(*totalValueOutstandingProxy > 0, "xrpl::loanMakePayment : valid total value"); + XRPL_ASSERT( + *loan->at(sfTotalValueOutstanding) > 0, "xrpl::loanMakePayment : valid total value"); view.update(loan); @@ -1844,311 +2114,29 @@ loanMakePayment( { // If the payment is late, and the late flag was not set, it's not // valid - JLOG(j.warn()) << "Loan payment is overdue. Use the tfLoanLatePayment " - "transaction " - "flag to make a late payment. Loan was created on " - << startDate << ", prev payment due date is " << prevPaymentDateProxy - << ", next payment due date is " << nextDueDateProxy << ", ledger time is " + JLOG(j.warn()) << "Loan payment is overdue. Use the tfLoanLatePayment transaction flag to " + "make a late payment. Loan was created on " + << loan->at(sfStartDate) << ", prev payment due date is " + << loan->at(sfPreviousPaymentDueDate) << ", next payment due date is " + << nextDueDateProxy << ", ledger time is " << view.parentCloseTime().time_since_epoch().count(); return std::unexpected(tecEXPIRED); } - // ------------------------------------------------------------- - // full payment handling - if (paymentType == LoanPaymentType::Full) + switch (paymentType) { - TenthBips32 const closeInterestRate{loan->at(sfCloseInterestRate)}; - Number const closePaymentFee = roundToAsset(asset, loan->at(sfClosePaymentFee), loanScale); - - LoanState const roundedLoanState = constructLoanState( - totalValueOutstandingProxy, principalOutstandingProxy, managementFeeOutstandingProxy); - - auto const fullPaymentComponents = detail::computeFullPayment( - asset, - view, - principalOutstandingProxy, - managementFeeOutstandingProxy, - periodicPayment, - paymentRemainingProxy, - prevPaymentDateProxy, - startDate, - paymentInterval, - closeInterestRate, - loanScale, - roundedLoanState.interestDue, - periodicRate, - closePaymentFee, - amount, - managementFeeRate, - j); - - if (fullPaymentComponents.has_value()) - { - return doPayment( - *fullPaymentComponents, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - paymentRemainingProxy, - prevPaymentDateProxy, - nextDueDateProxy, - paymentInterval); - } - - if (fullPaymentComponents.error()) - { - // error() will be the TER returned if a payment is not made. It - // will only evaluate to true if it's unsuccessful. Otherwise, - // tesSUCCESS means nothing was done, so continue. - return std::unexpected(fullPaymentComponents.error()); - } - - // LCOV_EXCL_START - UNREACHABLE("xrpl::loanMakePayment : invalid full payment result"); - JLOG(j.error()) << "Full payment computation failed unexpectedly."; - return std::unexpected(tecINTERNAL); - // LCOV_EXCL_STOP + case LoanPaymentType::Full: + return detail::makeFullPayment(asset, view, loan, brokerSle, amount, j); + case LoanPaymentType::Late: + return detail::makeLatePayment(asset, view, loan, brokerSle, amount, j); + case LoanPaymentType::Regular: + case LoanPaymentType::Overpayment: + return detail::makeRegularPayment(asset, view, loan, brokerSle, amount, paymentType, j); } - // ------------------------------------------------------------- - // compute the periodic payment info that will be needed whether the - // payment is late or regular - detail::ExtendedPaymentComponents periodic{ - detail::computePaymentComponents( - view.rules(), - asset, - loanScale, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - periodicPayment, - periodicRate, - paymentRemainingProxy, - managementFeeRate), - serviceFee}; - XRPL_ASSERT_PARTS( - periodic.trackedPrincipalDelta >= 0, - "xrpl::loanMakePayment", - "regular payment valid principal"); - - // ------------------------------------------------------------- - // late payment handling - if (paymentType == LoanPaymentType::Late) - { - TenthBips32 const lateInterestRate{loan->at(sfLateInterestRate)}; - Number const latePaymentFee = loan->at(sfLatePaymentFee); - - auto const latePaymentComponents = detail::computeLatePayment( - asset, - view, - principalOutstandingProxy, - nextDueDateProxy, - periodic, - lateInterestRate, - loanScale, - latePaymentFee, - amount, - managementFeeRate, - j); - - if (latePaymentComponents.has_value()) - { - return doPayment( - *latePaymentComponents, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - paymentRemainingProxy, - prevPaymentDateProxy, - nextDueDateProxy, - paymentInterval); - } - - if (latePaymentComponents.error()) - { - // error() will be the TER returned if a payment is not made. It - // will only evaluate to true if it's unsuccessful. - return std::unexpected(latePaymentComponents.error()); - } - - // LCOV_EXCL_START - UNREACHABLE("xrpl::loanMakePayment : invalid late payment result"); - JLOG(j.error()) << "Late payment computation failed unexpectedly."; - return std::unexpected(tecINTERNAL); - // LCOV_EXCL_STOP - } - - // ------------------------------------------------------------- - // regular periodic payment handling - - XRPL_ASSERT_PARTS( - paymentType == LoanPaymentType::Regular || paymentType == LoanPaymentType::Overpayment, - "xrpl::loanMakePayment", - "regular payment type"); - - // Keep a running total of the actual parts paid - LoanPaymentParts totalParts; - Number totalPaid; - std::size_t numPayments = 0; - - while ((amount >= (totalPaid + periodic.totalDue)) && paymentRemainingProxy > 0 && - numPayments < kLoanMaximumPaymentsPerTransaction) - { - // Try to make more payments - XRPL_ASSERT_PARTS( - periodic.trackedPrincipalDelta >= 0, - "xrpl::loanMakePayment", - "payment pays non-negative principal"); - - totalPaid += periodic.totalDue; - totalParts += detail::doPayment( - periodic, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - paymentRemainingProxy, - prevPaymentDateProxy, - nextDueDateProxy, - paymentInterval); - ++numPayments; - - XRPL_ASSERT_PARTS( - (periodic.specialCase == detail::PaymentSpecialCase::Final) == - (paymentRemainingProxy == 0), - "xrpl::loanMakePayment", - "final payment is the final payment"); - - // Don't compute the next payment if this was the last payment - if (periodic.specialCase == detail::PaymentSpecialCase::Final) - break; - - periodic = detail::ExtendedPaymentComponents{ - detail::computePaymentComponents( - view.rules(), - asset, - loanScale, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - periodicPayment, - periodicRate, - paymentRemainingProxy, - managementFeeRate), - serviceFee}; - } - - if (numPayments == 0) - { - JLOG(j.warn()) << "Regular loan payment amount is insufficient. Due: " << periodic.totalDue - << ", paid: " << amount; - return std::unexpected(tecINSUFFICIENT_PAYMENT); - } - - XRPL_ASSERT_PARTS( - totalParts.principalPaid + totalParts.interestPaid + totalParts.feePaid == totalPaid, - "xrpl::loanMakePayment", - "payment parts add up"); - XRPL_ASSERT_PARTS(totalParts.valueChange == 0, "xrpl::loanMakePayment", "no value change"); - - // ------------------------------------------------------------- - // overpayment handling - // - // If the "fixCleanup3_1_3" amendment is enabled, truncate "amount", - // at the loan scale. If the raw value is used, the overpayment - // amount could be meaningless dust. Trying to process such a small - // amount will, at best, waste time when all the result values round - // to zero. At worst, it can cause logical errors with tiny amounts - // of interest that don't add up correctly. - auto const roundedAmount = view.rules().enabled(fixCleanup3_1_3) - ? roundToAsset(asset, amount, loanScale, Number::RoundingMode::TowardsZero) - : amount; - if (paymentType == LoanPaymentType::Overpayment && loan->isFlag(lsfLoanOverpayment) && - paymentRemainingProxy > 0 && totalPaid < roundedAmount && - numPayments < kLoanMaximumPaymentsPerTransaction) - { - TenthBips32 const overpaymentInterestRate{loan->at(sfOverpaymentInterestRate)}; - TenthBips32 const overpaymentFeeRate{loan->at(sfOverpaymentFee)}; - - // It shouldn't be possible for the overpayment to be greater than - // totalValueOutstanding, because that would have been processed as - // another normal payment. But cap it just in case. - Number const overpaymentRaw = - std::min(roundedAmount - totalPaid, *totalValueOutstandingProxy); - - bool const fixEnabled = view.rules().enabled(fixCleanup3_2_0); - Number const overpayment = fixEnabled - ? roundToAsset(asset, overpaymentRaw, loanScale, Number::RoundingMode::Downward) - : overpaymentRaw; - - // Post-amendment, the rounded overpayment can be zero; pre-amendment - // it's always positive given the surrounding guards. - if (!fixEnabled || overpayment > 0) - { - detail::ExtendedPaymentComponents const overpaymentComponents = - detail::computeOverpaymentComponents( - view.rules(), - asset, - loanScale, - overpayment, - overpaymentInterestRate, - overpaymentFeeRate, - managementFeeRate); - - // Don't process an overpayment if the whole amount (or more!) - // gets eaten by fees and interest. - if (overpaymentComponents.trackedPrincipalDelta > 0) - { - XRPL_ASSERT_PARTS( - overpaymentComponents.untrackedInterest >= beast::kZero, - "xrpl::loanMakePayment", - "overpayment penalty did not reduce value of loan"); - // Can't just use `periodicPayment` here, because it might - // change - auto periodicPaymentProxy = loan->at(sfPeriodicPayment); - if (auto const overResult = detail::doOverpayment( - view.rules(), - asset, - loanScale, - overpaymentComponents, - totalValueOutstandingProxy, - principalOutstandingProxy, - managementFeeOutstandingProxy, - periodicPaymentProxy, - periodicRate, - paymentRemainingProxy, - managementFeeRate, - j)) - { - totalParts += *overResult; - } - else if (overResult.error()) - { - // error() will be the TER returned if a payment is not - // made. It will only evaluate to true if it's unsuccessful. - // Otherwise, tesSUCCESS means nothing was done, so - // continue. - return std::unexpected(overResult.error()); - } - } - } - } - - // Check the final results are rounded, to double-check that the - // intermediate steps were rounded. - XRPL_ASSERT( - isRounded(asset, totalParts.principalPaid, loanScale) && - totalParts.principalPaid >= beast::kZero, - "xrpl::loanMakePayment : total principal paid is valid"); - XRPL_ASSERT( - isRounded(asset, totalParts.interestPaid, loanScale) && - totalParts.interestPaid >= beast::kZero, - "xrpl::loanMakePayment : total interest paid is valid"); - XRPL_ASSERT( - isRounded(asset, totalParts.valueChange, loanScale), - "xrpl::loanMakePayment : loan value change is valid"); - XRPL_ASSERT( - isRounded(asset, totalParts.feePaid, loanScale) && totalParts.feePaid >= beast::kZero, - "xrpl::loanMakePayment : fee paid is valid"); - return totalParts; + // LCOV_EXCL_START + UNREACHABLE("xrpl::loanMakePayment : invalid payment type"); + return std::unexpected(tecINTERNAL); + // LCOV_EXCL_STOP } } // namespace xrpl diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index 793ddb18d2..6ac352f3e1 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/src/libxrpl/protocol/BuildInfo.cpp @@ -23,7 +23,7 @@ namespace { //------------------------------------------------------------------------------ // clang-format off // NOLINTNEXTLINE(readability-identifier-naming) -char const* const versionString = "3.3.0-b1" +char const* const versionString = "3.3.0-rc1" // clang-format on ; diff --git a/src/libxrpl/protocol/STObject.cpp b/src/libxrpl/protocol/STObject.cpp index ab9ae7d4d7..4b3ace2be3 100644 --- a/src/libxrpl/protocol/STObject.cpp +++ b/src/libxrpl/protocol/STObject.cpp @@ -633,20 +633,6 @@ STObject::getAccountID(SField const& field) const return getFieldByValue(field); } -AccountID -STObject::getInitiator() const -{ - // If sfDelegate is present, the delegate account is the initiator - // note: if a delegate is specified, its authorization to act on behalf of the account is - // enforced in `Transactor::invokeCheckPermission` - // cryptographic signature validity is checked separately (e.g., in `Transactor::checkSign`) - if (isFieldPresent(sfDelegate)) - return getAccountID(sfDelegate); - - // Default initiator - return getAccountID(sfAccount); -} - Blob STObject::getFieldVL(SField const& field) const { diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index be4ba2e8e5..17d7617590 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -72,7 +72,7 @@ STTx::STTx(STObject&& object) { applyTemplate(getTxFormat(txType_)->getSOTemplate()); // may throw tid_ = getHash(HashPrefix::TransactionId); - buildBatchTxnIds(); + buildBatchTxns(); } STTx::STTx(SerialIter& sit) : STObject(sfTransaction) @@ -89,7 +89,7 @@ STTx::STTx(SerialIter& sit) : STObject(sfTransaction) applyTemplate(getTxFormat(txType_)->getSOTemplate()); // May throw tid_ = getHash(HashPrefix::TransactionId); - buildBatchTxnIds(); + buildBatchTxns(); } STTx::STTx(TxType type, std::function assembler) : STObject(sfTransaction) @@ -110,7 +110,7 @@ STTx::STTx(TxType type, std::function assembler) : STObject(sfT logicError("Transaction type was mutated during assembly"); tid_ = getHash(HashPrefix::TransactionId); - buildBatchTxnIds(); + buildBatchTxns(); } STBase* @@ -279,12 +279,9 @@ STTx::checkSign(Rules const& rules) const return std::unexpected("Sponsor: " + ret.error()); } - // Verify the batch signer signatures here too, so they are cached with the - // rest of signature checking (checkValidity / SF_SIGGOOD) and stay out of - // the transaction engine. Gated on a batch (batchTxnIds_ seated) that - // actually carries signers; a batch whose inners are all from the outer - // account has no sfBatchSigners and needs no signer crypto. - if (batchTxnIds_ && isFieldPresent(sfBatchSigners)) + // Verify batch signer signatures here so they are cached with the rest + // of signature checking. + if (isFieldPresent(sfBatchSigners)) { if (auto const ret = checkBatchSign(rules); !ret) return ret; @@ -307,11 +304,28 @@ STTx::checkBatchSign(Rules const& rules) const if (!isFieldPresent(sfBatchSigners)) return std::unexpected("Missing BatchSigners field."); // LCOV_EXCL_LINE STArray const& signers{getFieldArray(sfBatchSigners)}; + // Bound signature verification to the protocol cap. This runs in + // checkValidity (via checkSign) at relay / submit time, BEFORE preflight + // and passesLocalChecks enforce the cap. Without this guard a malicious + // peer could put an oversized sfBatchSigners array in a 1 MB blob and + // force one signature verification per entry before any of those checks + // (or the fee charge) runs. + if (signers.size() > kMaxBatchSigners) + return std::unexpected("BatchSigners array exceeds max entries."); + // Defensive. + if (!batchTxns_) + { + // LCOV_EXCL_START + UNREACHABLE("STTx::checkBatchSign : batch transactions not built"); + return std::unexpected("Missing inner transactions."); + // LCOV_EXCL_STOP + } + auto const txIds = getBatchTransactionIDs(); for (auto const& signer : signers) { Blob const& signingPubKey = signer.getFieldVL(sfSigningPubKey); - auto const result = signingPubKey.empty() ? checkBatchMultiSign(signer, rules) - : checkBatchSingleSign(signer); + auto const result = signingPubKey.empty() ? checkBatchMultiSign(signer, rules, txIds) + : checkBatchSingleSign(signer, txIds); if (!result) return result; @@ -441,12 +455,11 @@ STTx::checkSingleSign(STObject const& sigObject) const } std::expected -STTx::checkBatchSingleSign(STObject const& batchSigner) const +STTx::checkBatchSingleSign(STObject const& batchSigner, std::vector const& txIds) const { XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::checkBatchSingleSign : batch transaction"); Serializer msg; - serializeBatch( - msg, getAccountID(sfAccount), getSeqValue(), getFlags(), getBatchTransactionIDs()); + serializeBatch(msg, getAccountID(sfAccount), getSeqValue(), getFlags(), txIds); finishMultiSigningData(batchSigner.getAccountID(sfAccount), msg); return singleSignHelper(batchSigner, msg.slice()); } @@ -529,7 +542,10 @@ multiSignHelper( } std::expected -STTx::checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const +STTx::checkBatchMultiSign( + STObject const& batchSigner, + Rules const& rules, + std::vector const& txIds) const { XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::checkBatchMultiSign : batch transaction"); // We can ease the computational load inside the loop a bit by @@ -537,8 +553,7 @@ STTx::checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const // with the stuff that stays constant from signature to signature. auto const batchSignerAccount = batchSigner.getAccountID(sfAccount); Serializer dataStart; - serializeBatch( - dataStart, getAccountID(sfAccount), getSeqValue(), getFlags(), getBatchTransactionIDs()); + serializeBatch(dataStart, getAccountID(sfAccount), getSeqValue(), getFlags(), txIds); dataStart.addBitString(batchSignerAccount); return multiSignHelper( batchSigner, @@ -577,38 +592,79 @@ STTx::checkMultiSign(Rules const& rules, STObject const& sigObject) const } void -STTx::buildBatchTxnIds() +STTx::buildBatchTxns() { // Precondition: the template must have been applied first, so the fields // (including sfRawTransactions) are canonical before the inner txns are // hashed. The constructors call this immediately after applying the // template; isFree() being false confirms a template is set. - XRPL_ASSERT(!isFree(), "STTx::buildBatchTxnIds : template applied"); - if (getTxnType() != ttBATCH || !isFieldPresent(sfRawTransactions)) + XRPL_ASSERT(!isFree(), "STTx::buildBatchTxns : template applied"); + if (getTxnType() != ttBATCH) return; + // A Batch always seats its inner transactions here, so every downstream + // consumer can rely on them. sfRawTransactions is required by the format + // (applyTemplate rejects a Batch without it); this guards a future change + // that made it optional. + if (!isFieldPresent(sfRawTransactions)) + { + // LCOV_EXCL_START + UNREACHABLE("STTx::buildBatchTxns : missing RawTransactions"); + Throw("Batch has no RawTransactions."); + // LCOV_EXCL_STOP + } auto const& raw = getFieldArray(sfRawTransactions); + if (raw.size() > kMaxBatchTxCount) + Throw("Batch has too many inner transactions."); - // Seated for any batch with raw transactions. The count is validated in - // preflight and at the relay boundary, so build every id here; this keeps - // the invariant batchTxnIds_->size() == rawTransactions.size(). - auto& ids = batchTxnIds_.emplace(); - ids.reserve(raw.size()); + // Build and validate each inner as an STTx once. A malformed inner throws; + // a nested batch is rejected before building it (a batch cannot contain a + // batch, and building one would recurse). + auto& txns = batchTxns_.emplace(); + txns.reserve(raw.size()); for (STObject const& rb : raw) - ids.push_back(rb.getHash(HashPrefix::TransactionId)); + { + if (rb.getFieldU16(sfTransactionType) == ttBATCH) + Throw("Batch inner transaction cannot be a Batch."); + + txns.push_back(std::make_shared(STObject{rb})); + } } -std::vector const& +std::vector STTx::getBatchTransactionIDs() const { - XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::getBatchTransactionIDs : batch transaction"); + auto const& txns = getBatchTransactions(); + std::vector ids; + ids.reserve(txns.size()); + for (auto const& stx : txns) + ids.push_back(stx->getTransactionID()); + return ids; +} + +std::vector> const& +STTx::getBatchTransactions() const +{ + XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::getBatchTransactions : batch transaction"); + XRPL_ASSERT(batchTxns_.has_value(), "STTx::getBatchTransactions : batch transactions built"); XRPL_ASSERT( - batchTxnIds_.has_value(), "STTx::getBatchTransactionIDs : batch transaction IDs built"); - XRPL_ASSERT( - batchTxnIds_->size() == getFieldArray(sfRawTransactions).size(), - "STTx::getBatchTransactionIDs : batch transaction IDs size mismatch"); - // NOLINTNEXTLINE(bugprone-unchecked-optional-access): guarded by assert above - return *batchTxnIds_; + batchTxns_->size() == getFieldArray(sfRawTransactions).size(), + "STTx::getBatchTransactions : batch transactions size mismatch"); + return *batchTxns_; +} + +AccountID +STTx::getInitiator() const +{ + // If sfDelegate is present, the delegate account is the initiator + // note: if a delegate is specified, its authorization to act on behalf of the account is + // enforced in `Transactor::invokeCheckPermission` + // cryptographic signature validity is checked separately (e.g., in `Transactor::checkSign`) + if (isFieldPresent(sfDelegate)) + return getAccountID(sfDelegate); + + // Default initiator + return getAccountID(sfAccount); } AccountID @@ -754,86 +810,62 @@ invalidMPTAmountInTx(STObject const& tx) } static bool -isBatchRawTransactionOkay(STObject const& st, std::string& reason) +isBatchRawTransactionOkay(STTx const& tx, std::string& reason) { - if (!st.isFieldPresent(sfRawTransactions)) + if (!tx.isFieldPresent(sfRawTransactions)) return true; // sfRawTransactions only appears on a Batch. passesLocalChecks runs on // unverified user and peer input, so reject (rather than assert) a non-batch // transaction that carries it. - if (st.getFieldU16(sfTransactionType) != ttBATCH) + if (tx.getTxnType() != ttBATCH) { reason = "Only Batch transactions may contain raw transactions."; return false; } - if (st.isFieldPresent(sfBatchSigners) && - st.getFieldArray(sfBatchSigners).size() > kMaxBatchSigners) + if (tx.isFieldPresent(sfBatchSigners) && + tx.getFieldArray(sfBatchSigners).size() > kMaxBatchSigners) { - reason = "Batch Signers array exceeds max entries."; + reason = "BatchSigners array exceeds max entries."; return false; } - auto const& rawTxns = st.getFieldArray(sfRawTransactions); - if (rawTxns.size() > kMaxBatchTxCount) + // Inner structure (type, template, no nesting, count) is validated when the + // batch STTx is constructed; here we only run each inner's local checks. + for (auto const& inner : tx.getBatchTransactions()) { - reason = "Raw Transactions array exceeds max entries."; - return false; - } - for (STObject raw : rawTxns) - { - try - { - auto const tt = safeCast(raw.getFieldU16(sfTransactionType)); - if (tt == ttBATCH) - { - reason = "Raw Transactions may not contain batch transactions."; - return false; - } - - raw.applyTemplate(getTxFormat(tt)->getSOTemplate()); - - // passesLocalChecks recurses back into isBatchRawTransactionOkay, - // but an inner can never be a batch (rejected above), so the - // recursion terminates at depth 1. - if (!passesLocalChecks(raw, reason)) - return false; - } - catch (std::exception const& e) - { - reason = e.what(); + if (!passesLocalChecks(*inner, reason)) return false; - } } return true; } bool -passesLocalChecks(STObject const& st, std::string& reason) +passesLocalChecks(STTx const& tx, std::string& reason) { - if (!isMemoOkay(st, reason)) + if (!isMemoOkay(tx, reason)) return false; - if (!isAccountFieldOkay(st)) + if (!isAccountFieldOkay(tx)) { reason = "An account field is invalid."; return false; } - if (isPseudoTx(st)) + if (isPseudoTx(tx)) { reason = "Cannot submit pseudo transactions."; return false; } - if (invalidMPTAmountInTx(st)) + if (invalidMPTAmountInTx(tx)) { reason = "Amount can not be MPT."; return false; } - if (!isBatchRawTransactionOkay(st, reason)) + if (!isBatchRawTransactionOkay(tx, reason)) return false; return true; diff --git a/src/libxrpl/tx/apply.cpp b/src/libxrpl/tx/apply.cpp index d85c4cfc40..f93b19a158 100644 --- a/src/libxrpl/tx/apply.cpp +++ b/src/libxrpl/tx/apply.cpp @@ -177,9 +177,9 @@ applyBatchTransactions( int applied = 0; - for (STObject rb : batchTxn.getFieldArray(sfRawTransactions)) + for (auto const& stx : batchTxn.getBatchTransactions()) { - auto const result = applyOneTransaction(STTx{std::move(rb)}); + auto const result = applyOneTransaction(*stx); XRPL_ASSERT( result.applied == (isTesSuccess(result.ter) || isTecClaim(result.ter)), "Outer Batch failure, inner transaction should not be applied"); diff --git a/src/libxrpl/tx/transactors/check/CheckCancel.cpp b/src/libxrpl/tx/transactors/check/CheckCancel.cpp index ed60817224..f4602f98c6 100644 --- a/src/libxrpl/tx/transactors/check/CheckCancel.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCancel.cpp @@ -1,10 +1,12 @@ #include #include +#include #include #include #include #include +#include #include #include #include @@ -19,6 +21,9 @@ namespace xrpl { NotTEC CheckCancel::preflight(PreflightContext const& ctx) { + if (ctx.rules.enabled(fixCleanup3_3_0) && ctx.tx[sfCheckID] == beast::kZero) + return temMALFORMED; + return tesSUCCESS; } diff --git a/src/libxrpl/tx/transactors/check/CheckCash.cpp b/src/libxrpl/tx/transactors/check/CheckCash.cpp index 4e810d94cf..a8c989f4df 100644 --- a/src/libxrpl/tx/transactors/check/CheckCash.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCash.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -51,6 +52,9 @@ CheckCash::checkExtraFeatures(xrpl::PreflightContext const& ctx) NotTEC CheckCash::preflight(PreflightContext const& ctx) { + if (ctx.rules.enabled(fixCleanup3_3_0) && ctx.tx[sfCheckID] == beast::kZero) + return temMALFORMED; + // Exactly one of Amount or DeliverMin must be present. auto const optAmount = ctx.tx[~sfAmount]; auto const optDeliverMin = ctx.tx[~sfDeliverMin]; diff --git a/src/libxrpl/tx/transactors/system/Batch.cpp b/src/libxrpl/tx/transactors/system/Batch.cpp index dee465cb88..ccb113e07b 100644 --- a/src/libxrpl/tx/transactors/system/Batch.cpp +++ b/src/libxrpl/tx/transactors/system/Batch.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -28,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -46,17 +46,11 @@ namespace xrpl { * * @param view The ledger view providing fee and state information. * @param tx The batch transaction to calculate the fee for. - * @return XRPAmount The total base fee required for the batch transaction. - * - * @throws std::overflow_error If any fee calculation would overflow the - * XRPAmount type. - * @throws std::length_error If the number of inner transactions or signers - * exceeds the allowed maximum. - * @throws std::invalid_argument If an inner transaction is itself a batch - * transaction. + * @return XRPAmount The total base fee required for the batch transaction, + * or std::nullopt on failure (overflow, oversized arrays). */ -XRPAmount -Batch::calculateBaseFee(ReadView const& view, STTx const& tx) +std::optional +Batch::calculateBaseFeeImpl(ReadView const& view, STTx const& tx) { XRPAmount const maxAmount{std::numeric_limits::max()}; @@ -67,49 +61,26 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) if (baseFee > maxAmount - view.fees().base) { JLOG(debugLog().error()) << "BatchTrace: Base fee overflow detected."; - return XRPAmount{kInitialXrp}; + return std::nullopt; } // LCOV_EXCL_STOP XRPAmount const batchBase = view.fees().base + baseFee; - // Calculate the Inner Txn Fees + // Calculate the Inner Txn Fees. Inners are built and validated (count, + // no nesting) at construction, so they are reused here directly. XRPAmount txnFees{0}; - if (tx.isFieldPresent(sfRawTransactions)) + for (auto const& stx : tx.getBatchTransactions()) { - auto const& txns = tx.getFieldArray(sfRawTransactions); - + auto const fee = xrpl::calculateBaseFee(view, *stx); // LCOV_EXCL_START - if (txns.size() > kMaxBatchTxCount) + if (txnFees > maxAmount - fee) { - JLOG(debugLog().error()) << "BatchTrace: Raw Transactions array exceeds max entries."; - return XRPAmount{kInitialXrp}; + JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in txnFees calculation."; + return std::nullopt; } // LCOV_EXCL_STOP - - for (STObject txn : txns) - { - STTx const stx = STTx{std::move(txn)}; - - // LCOV_EXCL_START - if (stx.getTxnType() == ttBATCH) - { - JLOG(debugLog().error()) << "BatchTrace: Inner Batch transaction found."; - return XRPAmount{kInitialXrp}; - } - // LCOV_EXCL_STOP - - auto const fee = xrpl::calculateBaseFee(view, stx); - // LCOV_EXCL_START - if (txnFees > maxAmount - fee) - { - JLOG(debugLog().error()) - << "BatchTrace: XRPAmount overflow in txnFees calculation."; - return XRPAmount{kInitialXrp}; - } - // LCOV_EXCL_STOP - txnFees += fee; - } + txnFees += fee; } // Calculate the Signers/BatchSigners Fees @@ -122,7 +93,7 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) if (signers.size() > kMaxBatchSigners) { JLOG(debugLog().error()) << "BatchTrace: Batch Signers array exceeds max entries."; - return XRPAmount{kInitialXrp}; + return std::nullopt; } // LCOV_EXCL_STOP @@ -140,7 +111,7 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) { JLOG(debugLog().error()) << "BatchTrace: Nested Signers array exceeds max entries."; - return kInitialXrp; + return std::nullopt; } // LCOV_EXCL_STOP signerCount += nestedSigners.size(); @@ -152,7 +123,7 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) if (signerCount > 0 && view.fees().base > maxAmount / signerCount) { JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in signerCount calculation."; - return XRPAmount{kInitialXrp}; + return std::nullopt; } // LCOV_EXCL_STOP @@ -162,13 +133,13 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) if (signerFees > maxAmount - txnFees) { JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in signerFees calculation."; - return XRPAmount{kInitialXrp}; + return std::nullopt; } XRPAmount const innerFees = txnFees + signerFees; if (innerFees > maxAmount - batchBase) { JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in total fee calculation."; - return XRPAmount{kInitialXrp}; + return std::nullopt; } // LCOV_EXCL_STOP @@ -176,6 +147,24 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx) return innerFees + batchBase; } +XRPAmount +Batch::calculateBaseFee(ReadView const& view, STTx const& tx) +{ + if (auto const fee = calculateBaseFeeImpl(view, tx)) + return *fee; + // The fee could not be computed, so return a placeholder the account can + // pay; preclaim rejects the transaction with tecINSUFF_FEE. + return view.fees().base; // LCOV_EXCL_LINE +} + +TER +Batch::preclaim(PreclaimContext const& ctx) +{ + if (!calculateBaseFeeImpl(ctx.view, ctx.tx)) + return tecINSUFF_FEE; // LCOV_EXCL_LINE + return tesSUCCESS; +} + std::uint32_t Batch::getFlagsMask(PreflightContext const& ctx) { @@ -246,13 +235,6 @@ Batch::preflight(PreflightContext const& ctx) return temARRAY_EMPTY; } - if (rawTxns.size() > kMaxBatchTxCount) - { - JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:" - << "txns array exceeds 8 entries."; - return temARRAY_TOO_LARGE; - } - if (ctx.tx.isFieldPresent(sfBatchSigners) && ctx.tx.getFieldArray(sfBatchSigners).size() > kMaxBatchSigners) { @@ -293,9 +275,9 @@ Batch::preflight(PreflightContext const& ctx) return tesSUCCESS; }; - for (STObject rb : rawTxns) + for (auto const& stxPtr : ctx.tx.getBatchTransactions()) { - STTx const stx = STTx{std::move(rb)}; + STTx const& stx = *stxPtr; auto const hash = stx.getTransactionID(); if (!uniqueHashes.emplace(hash).second) { @@ -306,14 +288,6 @@ Batch::preflight(PreflightContext const& ctx) } auto const txType = stx.getFieldU16(sfTransactionType); - if (txType == ttBATCH) - { - JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: " - << "batch cannot have an inner batch txn. " - << "txID: " << hash; - return temINVALID; - } - if (std::ranges::any_of( kDisabledTxTypes, [txType](auto const& disabled) { return txType == disabled; })) { @@ -434,15 +408,14 @@ Batch::preflightSigValidated(PreflightContext const& ctx) ctx.tx.getTxnType() == ttBATCH, "xrpl::Batch::preflightSigValidated : batch transaction"); auto const parentBatchId = ctx.tx.getTransactionID(); auto const outerAccount = ctx.tx.getAccountID(sfAccount); - auto const& rawTxns = ctx.tx.getFieldArray(sfRawTransactions); - // Accounts that must sign the batch: each inner authorizer and counterparty // (excluding the outer account), sorted and de-duplicated to match against // the ascending, unique batch signers. std::vector requiredSigners; requiredSigners.reserve(kMaxBatchSigners); - for (STObject const& rb : rawTxns) + for (auto const& stxPtr : ctx.tx.getBatchTransactions()) { + STTx const& rb = *stxPtr; // A delegated inner is signed by the delegate, not the account holder, // so the delegate is the required signer when present. AccountID const authorizer = rb.getInitiator(); diff --git a/src/libxrpl/tx/transactors/token/ConfidentialMPTClawback.cpp b/src/libxrpl/tx/transactors/token/ConfidentialMPTClawback.cpp index e0d1d28a8f..6366e99105 100644 --- a/src/libxrpl/tx/transactors/token/ConfidentialMPTClawback.cpp +++ b/src/libxrpl/tx/transactors/token/ConfidentialMPTClawback.cpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include @@ -22,9 +21,6 @@ namespace xrpl { NotTEC ConfidentialMPTClawback::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureConfidentialTransfer)) - return temDISABLED; - auto const account = ctx.tx[sfAccount]; // Only issuer can clawback diff --git a/src/libxrpl/tx/transactors/token/ConfidentialMPTConvert.cpp b/src/libxrpl/tx/transactors/token/ConfidentialMPTConvert.cpp index 44e2596325..454eb39ead 100644 --- a/src/libxrpl/tx/transactors/token/ConfidentialMPTConvert.cpp +++ b/src/libxrpl/tx/transactors/token/ConfidentialMPTConvert.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -26,9 +25,6 @@ namespace xrpl { NotTEC ConfidentialMPTConvert::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureConfidentialTransfer)) - return temDISABLED; - // issuer cannot convert if (MPTIssue(ctx.tx[sfMPTokenIssuanceID]).getIssuer() == ctx.tx[sfAccount]) return temMALFORMED; diff --git a/src/libxrpl/tx/transactors/token/ConfidentialMPTConvertBack.cpp b/src/libxrpl/tx/transactors/token/ConfidentialMPTConvertBack.cpp index d6fed78833..87f9e476d6 100644 --- a/src/libxrpl/tx/transactors/token/ConfidentialMPTConvertBack.cpp +++ b/src/libxrpl/tx/transactors/token/ConfidentialMPTConvertBack.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -25,9 +24,6 @@ namespace xrpl { NotTEC ConfidentialMPTConvertBack::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureConfidentialTransfer)) - return temDISABLED; - // issuer cannot convert back if (MPTIssue(ctx.tx[sfMPTokenIssuanceID]).getIssuer() == ctx.tx[sfAccount]) return temMALFORMED; diff --git a/src/libxrpl/tx/transactors/token/ConfidentialMPTMergeInbox.cpp b/src/libxrpl/tx/transactors/token/ConfidentialMPTMergeInbox.cpp index 02c759c521..0b98382a61 100644 --- a/src/libxrpl/tx/transactors/token/ConfidentialMPTMergeInbox.cpp +++ b/src/libxrpl/tx/transactors/token/ConfidentialMPTMergeInbox.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -24,9 +23,6 @@ namespace xrpl { NotTEC ConfidentialMPTMergeInbox::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureConfidentialTransfer)) - return temDISABLED; - // issuer cannot merge if (MPTIssue(ctx.tx[sfMPTokenIssuanceID]).getIssuer() == ctx.tx[sfAccount]) return temMALFORMED; diff --git a/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp b/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp index 302b7d239b..d121ec2634 100644 --- a/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp +++ b/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp @@ -31,9 +31,6 @@ ConfidentialMPTSend::checkExtraFeatures(PreflightContext const& ctx) NotTEC ConfidentialMPTSend::preflight(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureConfidentialTransfer)) - return temDISABLED; - auto const account = ctx.tx[sfAccount]; auto const issuer = MPTIssue(ctx.tx[sfMPTokenIssuanceID]).getIssuer(); diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index f9c156fe06..5085ad6172 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -56,7 +56,6 @@ #include #include #include -#include #include #include #include @@ -72,6 +71,7 @@ #include #include #include +#include #include #include #include @@ -313,8 +313,8 @@ class Batch_test : public beast::unit_test::Suite env.close(); } - // DEFENSIVE: temARRAY_TOO_LARGE: Batch: txns array exceeds 8 entries. - // ACTUAL: telENV_RPC_FAILED: isRawTransactionOkay() + // An oversized batch (more than kMaxBatchTxCount inners) fails STTx + // construction, so the transaction cannot be built. { auto const seq = env.seq(alice); auto const batchFee = batch::calcBatchFee(env, 0, 9); @@ -328,7 +328,7 @@ class Batch_test : public beast::unit_test::Suite batch::Inner(pay(alice, bob, XRP(1)), seq + 7), batch::Inner(pay(alice, bob, XRP(1)), seq + 8), batch::Inner(pay(alice, bob, XRP(1)), seq + 9), - Ter(telENV_RPC_FAILED)); + Ter(temMALFORMED)); env.close(); } @@ -345,15 +345,15 @@ class Batch_test : public beast::unit_test::Suite env.close(); } - // DEFENSIVE: temINVALID: Batch: batch cannot have inner batch txn. - // ACTUAL: telENV_RPC_FAILED: isRawTransactionOkay() + // A batch may not contain a batch: the nested inner fails STTx + // construction, so the transaction cannot be built. { auto const seq = env.seq(alice); auto const batchFee = batch::calcBatchFee(env, 0, 2); env(batch::outer(alice, seq, batchFee, tfAllOrNothing), batch::Inner(batch::outer(alice, seq, batchFee, tfAllOrNothing), seq), batch::Inner(pay(alice, bob, XRP(1)), seq + 2), - Ter(telENV_RPC_FAILED)); + Ter(temMALFORMED)); env.close(); } @@ -938,80 +938,41 @@ class Batch_test : public beast::unit_test::Suite env.fund(XRP(10000), alice, bob); + // An inner missing a required field can no longer be submitted: the + // outer STTx builds and validates each inner at construction, so + // building the batch (as signing does) throws. Returns true if the + // build fails. + auto batchCtorFails = [&](json::StaticString const& field) -> bool { + auto const batchFee = batch::calcBatchFee(env, 1, 2); + auto const seq = env.seq(alice); + auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1); + tx1.removeMember(field); + try + { + // Env::st swallows the construction failure and yields a null + // stx, so a malformed inner shows up as no transaction built. + auto const jt = env.jtnofill( + batch::outer(alice, seq, batchFee, tfAllOrNothing), + tx1, + batch::Inner(pay(alice, bob, XRP(10)), seq + 2)); + return jt.stx == nullptr; + } + catch (std::exception const&) + { + return true; + } + }; + // Invalid: sfTransactionType - { - auto const batchFee = batch::calcBatchFee(env, 1, 2); - auto const seq = env.seq(alice); - auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1); - tx1.removeMember(jss::TransactionType); - auto jt = env.jtnofill( - batch::outer(alice, seq, batchFee, tfAllOrNothing), - tx1, - batch::Inner(pay(alice, bob, XRP(10)), seq + 2)); - - env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED)); - env.close(); - } - + BEAST_EXPECT(batchCtorFails(jss::TransactionType)); // Invalid: sfAccount - { - auto const batchFee = batch::calcBatchFee(env, 1, 2); - auto const seq = env.seq(alice); - auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1); - tx1.removeMember(jss::Account); - auto jt = env.jtnofill( - batch::outer(alice, seq, batchFee, tfAllOrNothing), - tx1, - batch::Inner(pay(alice, bob, XRP(10)), seq + 2)); - - env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED)); - env.close(); - } - + BEAST_EXPECT(batchCtorFails(jss::Account)); // Invalid: sfSequence - { - auto const batchFee = batch::calcBatchFee(env, 1, 2); - auto const seq = env.seq(alice); - auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1); - tx1.removeMember(jss::Sequence); - auto jt = env.jtnofill( - batch::outer(alice, seq, batchFee, tfAllOrNothing), - tx1, - batch::Inner(pay(alice, bob, XRP(10)), seq + 2)); - - env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED)); - env.close(); - } - + BEAST_EXPECT(batchCtorFails(jss::Sequence)); // Invalid: sfFee - { - auto const batchFee = batch::calcBatchFee(env, 1, 2); - auto const seq = env.seq(alice); - auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1); - tx1.removeMember(jss::Fee); - auto jt = env.jtnofill( - batch::outer(alice, seq, batchFee, tfAllOrNothing), - tx1, - batch::Inner(pay(alice, bob, XRP(10)), seq + 2)); - - env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED)); - env.close(); - } - + BEAST_EXPECT(batchCtorFails(jss::Fee)); // Invalid: sfSigningPubKey - { - auto const batchFee = batch::calcBatchFee(env, 1, 2); - auto const seq = env.seq(alice); - auto tx1 = batch::Inner(pay(alice, bob, XRP(10)), seq + 1); - tx1.removeMember(jss::SigningPubKey); - auto jt = env.jtnofill( - batch::outer(alice, seq, batchFee, tfAllOrNothing), - tx1, - batch::Inner(pay(alice, bob, XRP(10)), seq + 2)); - - env(jt.jv, batch::Sig(bob), Ter(telENV_RPC_FAILED)); - env.close(); - } + BEAST_EXPECT(batchCtorFails(jss::SigningPubKey)); // Inner OfferCreate with MPT TakerPays. Valid under featureMPTokensV2. { @@ -1512,11 +1473,13 @@ class Batch_test : public beast::unit_test::Suite batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), - Ter(telENV_RPC_FAILED)); + Ter(temMALFORMED)); env.close(); } - // temARRAY_TOO_LARGE: Batch: txns array exceeds 8 entries. + // An oversized batch (more than kMaxBatchTxCount inners) fails STTx + // construction, so it never reaches apply or checkValidity - Env::st + // swallows the failure and yields a null stx. { Env env{*this, features}; @@ -1539,11 +1502,44 @@ class Batch_test : public beast::unit_test::Suite batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), batch::Inner(pay(alice, bob, XRP(1)), aliceSeq)); - env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal j) { - auto const result = xrpl::apply(env.app(), view, *jt.stx, TapNone, j); - BEAST_EXPECT(!result.applied && result.ter == temARRAY_TOO_LARGE); - return result.applied; - }); + BEAST_EXPECT(jt.stx == nullptr); + } + + // An oversized batch cannot slip through as validly signed even when it + // carries a BatchSigners array: the oversized inner array fails STTx + // construction before any signature is checked, so the batch can't be + // built at all (building it - as signing does - throws). + { + Env env{*this, features}; + + auto const alice = Account("alice"); + auto const bob = Account("bob"); + env.fund(XRP(10000), alice, bob); + env.close(); + + auto const aliceSeq = env.seq(alice); + auto const batchFee = batch::calcBatchFee(env, kMaxBatchSigners + 1, 9); + bool threw = false; + try + { + env.jtnofill( + batch::outer(alice, aliceSeq, batchFee, tfAllOrNothing), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Inner(pay(alice, bob, XRP(1)), aliceSeq), + batch::Sig(std::vector(kMaxBatchSigners + 1, bob))); + } + catch (std::exception const&) + { + threw = true; + } + BEAST_EXPECT(threw); } // Regression: the relay-boundary local check (isBatchRawTransactionOkay) @@ -1598,6 +1594,14 @@ class Batch_test : public beast::unit_test::Suite BEAST_EXPECT(!result.applied && result.ter == temARRAY_TOO_LARGE); return result.applied; }); + + // Regression (uncapped batch-signer verification): the relay + // boundary (checkValidity) rejects the oversized signers array via + // the checkBatchSign guard, BEFORE verifying a single signature. + auto const [valid, reason] = + xrpl::checkValidity(env.app().getHashRouter(), *jt.stx, env.current()->rules()); + BEAST_EXPECT(valid == xrpl::Validity::SigBad); + BEAST_EXPECT(reason == "BatchSigners array exceeds max entries."); } } @@ -5524,7 +5528,8 @@ class Batch_test : public beast::unit_test::Suite return Batch::calculateBaseFee(*env.current(), *jtx.stx); }; - // bad: Inner Batch transaction found + // bad: a batch may not contain a batch - the nested inner fails STTx + // construction, so the transaction cannot be built. { auto const seq = env.seq(alice); XRPAmount const batchFee = batch::calcBatchFee(env, 0, 2); @@ -5532,11 +5537,11 @@ class Batch_test : public beast::unit_test::Suite batch::outer(alice, seq, batchFee, tfAllOrNothing), batch::Inner(batch::outer(alice, seq, batchFee, tfAllOrNothing), seq), batch::Inner(pay(alice, bob, XRP(1)), seq + 2)); - XRPAmount const txBaseFee = getBaseFee(jtx); - BEAST_EXPECT(txBaseFee == XRPAmount(kInitialXrp)); + BEAST_EXPECT(jtx.stx == nullptr); } - // bad: Raw Transactions array exceeds max entries. + // bad: an oversized batch (more than kMaxBatchTxCount inners) fails + // STTx construction, so it cannot be built. { auto const seq = env.seq(alice); XRPAmount const batchFee = batch::calcBatchFee(env, 0, 2); @@ -5553,8 +5558,7 @@ class Batch_test : public beast::unit_test::Suite batch::Inner(pay(alice, bob, XRP(1)), seq + 8), batch::Inner(pay(alice, bob, XRP(1)), seq + 9)); - XRPAmount const txBaseFee = getBaseFee(jtx); - BEAST_EXPECT(txBaseFee == XRPAmount(kInitialXrp)); + BEAST_EXPECT(jtx.stx == nullptr); } // bad: Signers array exceeds max entries. @@ -5567,8 +5571,9 @@ class Batch_test : public beast::unit_test::Suite batch::Inner(pay(alice, bob, XRP(10)), seq + 1), batch::Inner(pay(alice, bob, XRP(5)), seq + 2), batch::Sig(std::vector(kMaxBatchSigners + 1, bob))); + // Failure paths fall back to the ledger base fee. XRPAmount const txBaseFee = getBaseFee(jtx); - BEAST_EXPECT(txBaseFee == XRPAmount(kInitialXrp)); + BEAST_EXPECT(txBaseFee == env.current()->fees().base); } // good: diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index b19a9f6894..840c06bd84 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -1257,6 +1257,13 @@ class Check_test : public beast::unit_test::Suite env.close(); } + // Can't run pre-amendment behavior due to assertion failure. + if (features[fixCleanup3_3_0]) + { + env(check::cash(bob, uint256{}, usd(20)), Ter(temMALFORMED)); + env.close(); + } + // alice creates her checks ahead of time. uint256 const chkIdU{getCheckIndex(alice, env.seq(alice))}; env(check::create(alice, bob, usd(20))); @@ -1704,6 +1711,13 @@ class Check_test : public beast::unit_test::Suite // Non-existent check. env(check::cancel(bob, getCheckIndex(alice, env.seq(alice))), Ter(tecNO_ENTRY)); env.close(); + + // Can't run pre-amendment behavior due to assertion failure. + if (features[fixCleanup3_3_0]) + { + env(check::cancel(bob, uint256{}), Ter(temMALFORMED)); + env.close(); + } } void diff --git a/src/test/app/LedgerMaster_test.cpp b/src/test/app/LedgerMaster_test.cpp index 738358eb90..ece25356fd 100644 --- a/src/test/app/LedgerMaster_test.cpp +++ b/src/test/app/LedgerMaster_test.cpp @@ -128,9 +128,7 @@ class LedgerMaster_test : public beast::unit_test::Suite auto const deleteInterval = 8; - Env env{*this, envconfig([](auto cfg) { - return onlineDelete(std::move(cfg), deleteInterval); - })}; + Env env{*this, envconfig(onlineDelete, deleteInterval)}; auto const alice = Account("alice"); env.fund(XRP(1000), alice); @@ -146,7 +144,7 @@ class LedgerMaster_test : public beast::unit_test::Suite BEAST_EXPECTS(lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers()); BEAST_EXPECTS(lastRotated == 3, to_string(lastRotated)); BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0); - BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0); + BEAST_EXPECT(minSeq + 1 > maxSeq - 1); BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2); BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2); BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2); diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 371fcae54f..231a3b405a 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -446,7 +446,7 @@ protected: env.test.BEAST_EXPECT(loan->at(sfPeriodicPayment) == periodicPayment); env.test.BEAST_EXPECT(loan->at(sfFlags) == flags); - auto const ls = constructRoundedLoanState(loan); + auto const ls = constructLoanState(loan); auto const interestRate = TenthBips32{loan->at(sfInterestRate)}; auto const paymentInterval = loan->at(sfPaymentInterval); @@ -1119,7 +1119,7 @@ protected: // No reason for this not to exist return; } - auto const current = constructRoundedLoanState(loanSle); + auto const current = constructLoanState(loanSle); auto const errors = nextTrueState - current; log << currencyLabel << " Loan balances: " << "\n\tAmount taken: " << paymentComponents.trackedValueDelta @@ -6056,7 +6056,7 @@ protected: auto const loanSle = env.le(loanKeylet); if (!BEAST_EXPECT(loanSle)) return; - auto const state = constructRoundedLoanState(loanSle); + auto const state = constructLoanState(loanSle); log << "Loan state:" << std::endl; log << " ValueOutstanding: " << state.valueOutstanding << std::endl; diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index ac810e88f9..6f96528820 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -643,7 +643,7 @@ public: BEAST_EXPECTS(maxSeq == 3, std::to_string(maxSeq)); BEAST_EXPECTS(lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers()); BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0); - BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0); + BEAST_EXPECT(minSeq + 1 > maxSeq - 1); BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2); BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2); BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2); @@ -674,10 +674,13 @@ public: { std::this_thread::sleep_for(100ms); } - // Even the slowest machines should be able to finalize deleteSeq within 4 + // Even the slowest machines should be able to finalize deleteSeq within 10 // loops (400ms). If this test ever actually fails feel free to lower this - // cutoff. - BEAST_EXPECTS(iterations > 25, to_string(iterations)); + // cutoff. The intent of this test is to flag if the loop takes a very long + // time, but still allow the rest of this function to finish. + BEAST_EXPECTS(iterations > 20, std::to_string(iterations)); + if (!BEAST_EXPECT(lm.haveLedger(deleteSeq))) + return; } lm.clearLedger(deleteSeq); diff --git a/src/test/nodestore/Timing_test.cpp b/src/test/nodestore/Timing_test.cpp deleted file mode 100644 index 1f282d9d7f..0000000000 --- a/src/test/nodestore/Timing_test.cpp +++ /dev/null @@ -1,729 +0,0 @@ -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#ifndef NODESTORE_TIMING_DO_VERIFY -#define NODESTORE_TIMING_DO_VERIFY 0 -#endif - -namespace xrpl::NodeStore { - -std::unique_ptr -makeBackend(Section const& config, Scheduler& scheduler, beast::Journal journal) -{ - return Manager::instance().makeBackend(config, megabytes(4), scheduler, journal); -} - -// Fill memory with random bits -template -static void -rngcpy(void* buffer, std::size_t bytes, Generator& g) -{ - using result_type = Generator::result_type; - while (bytes >= sizeof(result_type)) - { - auto const v = g(); - memcpy(buffer, &v, sizeof(v)); - buffer = reinterpret_cast(buffer) + sizeof(v); - bytes -= sizeof(v); - } - - if (bytes > 0) - { - auto const v = g(); - memcpy(buffer, &v, bytes); - } -} - -// Instance of node factory produces a deterministic sequence -// of random NodeObjects within the given -class Sequence -{ -private: - static constexpr auto kMinLedger = 1; - static constexpr auto kMaxLedger = 1000000; - static constexpr auto kMinSize = 250; - static constexpr auto kMaxSize = 1250; - - beast::xor_shift_engine gen_; - std::uint8_t prefix_; - std::discrete_distribution dType_; - std::uniform_int_distribution dSize_; - -public: - explicit Sequence(std::uint8_t prefix) - : prefix_(prefix) - // uniform distribution over hotLEDGER - hotTRANSACTION_NODE - // but exclude hotTRANSACTION = 2 (removed) - , dType_({1, 1, 0, 1, 1}) - , dSize_(kMinSize, kMaxSize) - { - } - - // Returns the n-th key - uint256 - key(std::size_t n) - { - gen_.seed(n + 1); - uint256 result; - rngcpy(&*result.begin(), result.size(), gen_); - return result; - } - - // Returns the n-th complete NodeObject - std::shared_ptr - obj(std::size_t n) - { - gen_.seed(n + 1); - uint256 key; - auto const data = static_cast(&*key.begin()); - *data = prefix_; - rngcpy(data + 1, key.size() - 1, gen_); - Blob value(dSize_(gen_)); - rngcpy(&value[0], value.size(), gen_); - return NodeObject::createObject( - safeCast(dType_(gen_)), std::move(value), key); - } - - // returns a batch of NodeObjects starting at n - void - batch(std::size_t n, Batch& b, std::size_t size) - { - b.clear(); - b.reserve(size); - while ((size--) != 0u) - b.emplace_back(obj(n++)); - } -}; - -//---------------------------------------------------------------------------------- - -class Timing_test : public beast::unit_test::Suite -{ -public: - static constexpr auto kMissingNodePercent = 20; // percent of fetches for missing nodes - - std::size_t const defaultRepeat = 3; -#ifndef NDEBUG - std::size_t const defaultItems = 10000; -#else - std::size_t const defaultItems = 100000; // release -#endif - - using clock_type = std::chrono::steady_clock; - using duration_type = std::chrono::milliseconds; - - struct Params - { - std::size_t items; - std::size_t threads; - }; - - static std::string - toString(Section const& config) - { - std::string s; - for (auto iter = config.begin(); iter != config.end(); ++iter) - s += (iter != config.begin() ? "," : "") + iter->first + "=" + iter->second; - return s; - } - - static std::string - toString(duration_type const& d) - { - std::stringstream ss; - ss << std::fixed << std::setprecision(3) << (d.count() / 1000.) << "s"; - return ss.str(); - } - - static Section - parse(std::string s) - { - Section section; - std::vector v; - boost::split(v, s, boost::algorithm::is_any_of(",")); - section.append(v); - return section; - } - - //-------------------------------------------------------------------------- - - // Workaround for GCC's parameter pack expansion in lambdas - // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47226 - template - class ParallelForLambda - { - private: - std::size_t const n_; - std::atomic& c_; - - public: - ParallelForLambda(std::size_t n, std::atomic& c) : n_(n), c_(c) - { - } - - template - void - operator()(Args&&... args) - { - Body body(args...); - for (;;) - { - auto const i = c_++; - if (i >= n_) - break; - body(i); - } - } - }; - - /* Execute parallel-for loop. - - Constructs `number_of_threads` instances of `Body` - with `args...` parameters and runs them on individual threads - with unique loop indexes in the range [0, n). - */ - template - void - parallelFor(std::size_t const n, std::size_t numberOfThreads, Args const&... args) - { - std::atomic c(0); - std::vector t; - t.reserve(numberOfThreads); - for (std::size_t id = 0; id < numberOfThreads; ++id) - t.emplace_back(*this, ParallelForLambda(n, c), args...); - for (auto& _ : t) - _.join(); - } - - template - void - parallelForId(std::size_t const n, std::size_t numberOfThreads, Args const&... args) - { - std::atomic c(0); - std::vector t; - t.reserve(numberOfThreads); - for (std::size_t id = 0; id < numberOfThreads; ++id) - t.emplace_back(*this, ParallelForLambda(n, c), id, args...); - for (auto& _ : t) - _.join(); - } - - //-------------------------------------------------------------------------- - - // Insert only - void - doInsert(Section const& config, Params const& params, beast::Journal journal) - { - DummyScheduler scheduler; - auto backend = makeBackend(config, scheduler, journal); - BEAST_EXPECT(backend != nullptr); - backend->open(); - - class Body - { - private: - Suite& suite_; - Backend& backend_; - Sequence seq_; - - public: - explicit Body(Suite& s, Backend& backend) : suite_(s), backend_(backend), seq_(1) - { - } - - void - operator()(std::size_t i) - { - try - { - backend_.store(seq_.obj(i)); - } - catch (std::exception const& e) - { - suite_.fail(e.what()); - } - } - }; - - try - { - parallelFor(params.items, params.threads, std::ref(*this), std::ref(*backend)); - } - catch (std::exception const&) - { -#if NODESTORE_TIMING_DO_VERIFY - backend->verify(); -#endif - rethrow(); - } - backend->close(); - } - - // Fetch existing keys - void - doFetch(Section const& config, Params const& params, beast::Journal journal) - { - DummyScheduler scheduler; - auto backend = makeBackend(config, scheduler, journal); - BEAST_EXPECT(backend != nullptr); - backend->open(); - - class Body - { - private: - Suite& suite_; - Backend& backend_; - Sequence seq1_; - beast::xor_shift_engine gen_; - std::uniform_int_distribution dist_; - - public: - Body(std::size_t id, Suite& s, Params const& params, Backend& backend) - : suite_(s), backend_(backend), seq1_(1), gen_(id + 1), dist_(0, params.items - 1) - { - } - - void - operator()(std::size_t i) - { - try - { - std::shared_ptr obj; - std::shared_ptr result; - obj = seq1_.obj(dist_(gen_)); - backend_.fetch(obj->getHash(), &result); - suite_.expect(result && isSame(result, obj)); - } - catch (std::exception const& e) - { - suite_.fail(e.what()); - } - } - }; - try - { - parallelForId( - params.items, - params.threads, - std::ref(*this), - std::ref(params), - std::ref(*backend)); - } - catch (std::exception const&) - { -#if NODESTORE_TIMING_DO_VERIFY - backend->verify(); -#endif - rethrow(); - } - backend->close(); - } - - // Perform lookups of non-existent keys - void - doMissing(Section const& config, Params const& params, beast::Journal journal) - { - DummyScheduler scheduler; - auto backend = makeBackend(config, scheduler, journal); - BEAST_EXPECT(backend != nullptr); - backend->open(); - - class Body - { - private: - Suite& suite_; - // Params const& params_; - Backend& backend_; - Sequence seq2_; - beast::xor_shift_engine gen_; - std::uniform_int_distribution dist_; - - public: - Body(std::size_t id, Suite& s, Params const& params, Backend& backend) - : suite_(s) - //, params_ (params) - , backend_(backend) - , seq2_(2) - , gen_(id + 1) - , dist_(0, params.items - 1) - { - } - - void - operator()(std::size_t i) - { - try - { - auto const hash = seq2_.key(i); - std::shared_ptr result; - backend_.fetch(hash, &result); - suite_.expect(!result); - } - catch (std::exception const& e) - { - suite_.fail(e.what()); - } - } - }; - - try - { - parallelForId( - params.items, - params.threads, - std::ref(*this), - std::ref(params), - std::ref(*backend)); - } - catch (std::exception const&) - { -#if NODESTORE_TIMING_DO_VERIFY - backend->verify(); -#endif - rethrow(); - } - backend->close(); - } - - // Fetch with present and missing keys - void - doMixed(Section const& config, Params const& params, beast::Journal journal) - { - DummyScheduler scheduler; - auto backend = makeBackend(config, scheduler, journal); - BEAST_EXPECT(backend != nullptr); - backend->open(); - - class Body - { - private: - Suite& suite_; - // Params const& params_; - Backend& backend_; - Sequence seq1_; - Sequence seq2_; - beast::xor_shift_engine gen_; - std::uniform_int_distribution rand_; - std::uniform_int_distribution dist_; - - public: - Body(std::size_t id, Suite& s, Params const& params, Backend& backend) - : suite_(s) - //, params_ (params) - , backend_(backend) - , seq1_(1) - , seq2_(2) - , gen_(id + 1) - , rand_(0, 99) - , dist_(0, params.items - 1) - { - } - - void - operator()(std::size_t i) - { - try - { - if (rand_(gen_) < kMissingNodePercent) - { - auto const hash = seq2_.key(dist_(gen_)); - std::shared_ptr result; - backend_.fetch(hash, &result); - suite_.expect(!result); - } - else - { - std::shared_ptr obj; - std::shared_ptr result; - obj = seq1_.obj(dist_(gen_)); - backend_.fetch(obj->getHash(), &result); - suite_.expect(result && isSame(result, obj)); - } - } - catch (std::exception const& e) - { - suite_.fail(e.what()); - } - } - }; - - try - { - parallelForId( - params.items, - params.threads, - std::ref(*this), - std::ref(params), - std::ref(*backend)); - } - catch (std::exception const&) - { -#if NODESTORE_TIMING_DO_VERIFY - backend->verify(); -#endif - rethrow(); - } - backend->close(); - } - - // Simulate an xrpld workload: - // Each thread randomly: - // inserts a new key - // fetches an old key - // fetches recent, possibly non existent data - void - doWork(Section const& config, Params const& params, beast::Journal journal) - { - DummyScheduler scheduler; - auto backend = makeBackend(config, scheduler, journal); - BEAST_EXPECT(backend != nullptr); - backend->setDeletePath(); - backend->open(); - - class Body - { - private: - Suite& suite_; - Params const& params_; - Backend& backend_; - Sequence seq1_; - beast::xor_shift_engine gen_; - std::uniform_int_distribution rand_; - std::uniform_int_distribution recent_; - std::uniform_int_distribution older_; - - public: - Body(std::size_t id, Suite& s, Params const& params, Backend& backend) - : suite_(s) - , params_(params) - , backend_(backend) - , seq1_(1) - , gen_(id + 1) - , rand_(0, 99) - , recent_(params.items, (params.items * 2) - 1) - , older_(0, params.items - 1) - { - } - - void - operator()(std::size_t i) - { - try - { - if (rand_(gen_) < 200) - { - // historical lookup - std::shared_ptr obj; - std::shared_ptr result; - auto const j = older_(gen_); - obj = seq1_.obj(j); - backend_.fetch(obj->getHash(), &result); - suite_.expect(result != nullptr); - suite_.expect(isSame(result, obj)); - } - - char p[2]; - p[0] = rand_(gen_) < 50 ? 0 : 1; - p[1] = 1 - p[0]; - for (char const op : p) - { - // NOLINTNEXTLINE(bugprone-switch-missing-default-case) - switch (op) - { - case 0: { - // fetch recent - std::shared_ptr obj; - std::shared_ptr result; - auto const j = recent_(gen_); - obj = seq1_.obj(j); - backend_.fetch(obj->getHash(), &result); - suite_.expect(!result || isSame(result, obj)); - break; - } - - case 1: { - // insert new - auto const j = i + params_.items; - backend_.store(seq1_.obj(j)); - break; - } - } - } - } - catch (std::exception const& e) - { - suite_.fail(e.what()); - } - } - }; - - try - { - parallelForId( - params.items, - params.threads, - std::ref(*this), - std::ref(params), - std::ref(*backend)); - } - catch (std::exception const&) - { -#if NODESTORE_TIMING_DO_VERIFY - backend->verify(); -#endif - rethrow(); - } - backend->close(); - } - - //-------------------------------------------------------------------------- - - using test_func = void (Timing_test::*)(Section const&, Params const&, beast::Journal); - using test_list = std::vector>; - - duration_type - doTest(test_func f, Section const& config, Params const& params, beast::Journal journal) - { - auto const start = clock_type::now(); - (this->*f)(config, params, journal); - return std::chrono::duration_cast(clock_type::now() - start); - } - - void - doTests( - std::size_t threads, - test_list const& tests, - std::vector const& configStrings) - { - using std::setw; - int w = 8; - for (auto const& test : tests) - { - w = std::max::size_type>(w, test.first.size()); - } - log << threads << " Thread" << (threads > 1 ? "s" : "") << ", " << defaultItems - << " Objects" << std::endl; - { - std::stringstream ss; - ss << std::left << setw(10) << "Backend" << std::right; - for (auto const& test : tests) - ss << " " << setw(w) << test.first; - log << ss.str() << std::endl; - } - - using beast::Severity; - test::SuiteJournal journal("Timing_test", *this); - - for (auto const& configString : configStrings) - { - Params params{}; - params.items = defaultItems; - params.threads = threads; - for (auto i = defaultRepeat; (i--) != 0u;) - { - beast::TempDir const tempDir; - Section config = parse(configString); - config.set(Keys::kPath, tempDir.path()); - std::stringstream ss; - ss << std::left << setw(10) << get(config, Keys::kType, std::string()) - << std::right; - for (auto const& test : tests) - { - ss << " " << setw(w) << toString(doTest(test.second, config, params, journal)); - } - ss << " " << toString(config); - log << ss.str() << std::endl; - } - } - } - - void - run() override - { - testcase("Timing", beast::unit_test::AbortT::AbortOnFail); - - /* Parameters: - - repeat Number of times to repeat each test - items Number of objects to create in the database - - */ - std::string const defaultArgs = - "type=nudb" -#if XRPL_ROCKSDB_AVAILABLE - ";type=rocksdb,open_files=2000,filter_bits=12,cache_mb=256," - "file_size_mb=8,file_size_mult=2" -#endif - ; - - test_list const tests = { - {"Insert", &Timing_test::doInsert}, - {"Fetch", &Timing_test::doFetch}, - {"Missing", &Timing_test::doMissing}, - {"Mixed", &Timing_test::doMixed}, - {"Work", &Timing_test::doWork}}; - - auto args = arg().empty() ? defaultArgs : arg(); - std::vector configStrings; - boost::split(configStrings, args, boost::algorithm::is_any_of(";")); - for (auto iter = configStrings.begin(); iter != configStrings.end();) - { - if (iter->empty()) - { - iter = configStrings.erase(iter); - } - else - { - ++iter; - } - } - - doTests(1, tests, configStrings); - doTests(4, tests, configStrings); - doTests(8, tests, configStrings); - // do_tests (16, tests, config_strings); - } -}; - -BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(Timing, nodestore, xrpl, 1); - -} // namespace xrpl::NodeStore diff --git a/src/test/protocol/STTx_test.cpp b/src/test/protocol/STTx_test.cpp index 45bd2729c4..777234be83 100644 --- a/src/test/protocol/STTx_test.cpp +++ b/src/test/protocol/STTx_test.cpp @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -50,15 +51,10 @@ public: run() override { testMalformedSerializedForm(); - - testcase("secp256k1 signatures"); testSTTx(KeyType::Secp256k1); - - testcase("ed25519 signatures"); testSTTx(KeyType::Ed25519); - - testcase("STObject constructor errors"); testObjectCtorErrors(); + testBatchInnerCtorErrors(); } void @@ -1328,6 +1324,8 @@ public: void testSTTx(KeyType keyType) { + testcase(std::string(to_string(keyType)) + " signatures"); + auto const keypair = randomKeyPair(keyType); STTx j(ttACCOUNT_SET, [&keypair](auto& obj) { @@ -1382,6 +1380,8 @@ public: void testObjectCtorErrors() { + testcase("STObject constructor errors"); + auto const kp1 = randomKeyPair(KeyType::Secp256k1); auto const id1 = calcAccountID(kp1.first); @@ -1463,6 +1463,75 @@ public: BEAST_EXPECT(got == "Field 'Fee' is required but missing."); } } + + void + testBatchInnerCtorErrors() + { + testcase("Batch inner transaction validation"); + + auto const kp1 = randomKeyPair(KeyType::Secp256k1); + auto const id1 = calcAccountID(kp1.first); + + auto const kp2 = randomKeyPair(KeyType::Secp256k1); + auto const id2 = calcAccountID(kp2.first); + + // A raw inner transaction object of the given transaction type. + auto makeInner = [&](std::uint16_t txType) { + STObject inner(sfRawTransaction); + inner.setFieldU16(sfTransactionType, txType); + inner.setAccountID(sfAccount, id1); + inner.setAccountID(sfDestination, id2); + inner.setFieldAmount(sfAmount, STAmount(10000000000ull)); + inner.setFieldAmount(sfFee, STAmount(0ull)); + inner.setFieldU32(sfSequence, 1); + inner.setFieldVL(sfSigningPubKey, Slice(kp1.first.data(), kp1.first.size())); + return inner; + }; + + // An outer Batch STObject wrapping the given inner. + auto makeBatch = [&](STObject inner) { + STArray rawTxns(sfRawTransactions); + rawTxns.push_back(std::move(inner)); + + STObject batch(sfGeneric); + batch.setFieldU16(sfTransactionType, ttBATCH); + batch.setAccountID(sfAccount, id1); + batch.setFieldAmount(sfFee, STAmount(20ull)); + batch.setFieldU32(sfSequence, 1); + batch.setFieldVL(sfSigningPubKey, Slice(kp1.first.data(), kp1.first.size())); + batch.setFieldArray(sfRawTransactions, rawTxns); + return batch; + }; + + { + // A batch whose inner is a well-formed transaction constructs. + std::string errorMsg; + try + { + STTx{makeBatch(makeInner(ttPAYMENT))}; + } + catch (std::exception const& err) + { + errorMsg = err.what(); + } + BEAST_EXPECT(errorMsg.empty()); + } + { + // A batch whose inner carries an unregistered transaction type is + // rejected at construction, rather than surviving as a raw STObject + // and throwing later from an unprotected fee-calculation path. + std::string errorMsg; + try + { + STTx{makeBatch(makeInner(60000))}; + } + catch (std::exception const& err) + { + errorMsg = err.what(); + } + BEAST_EXPECT(matches(errorMsg.c_str(), "Invalid transaction type 60000")); + } + } }; class InnerObjectFormatsSerializer_test : public beast::unit_test::Suite diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index ba6720281c..589d592a29 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -1586,6 +1586,14 @@ LedgerMaster::getCompleteLedgers() const std::size_t LedgerMaster::missingFromCompleteLedgerRange(LedgerIndex first, LedgerIndex last) const { + if (first > last) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::LedgerMaster::missingFromCompleteLedgerRange : invalid parameters"); + return 0; + // LCOV_EXCL_STOP + } + RangeSet const target{range(first, last)}; auto const missing = [&target, this] {