From e79673cf40cd4bbdb29e4f56fa2c3fd9e80edea4 Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Thu, 5 Feb 2026 11:23:44 +0100 Subject: [PATCH 01/12] fix typo in LendingHelpers unit-test (#6215) --- src/test/app/LendingHelpers_test.cpp | 132 ++++++++++++--------------- 1 file changed, 60 insertions(+), 72 deletions(-) diff --git a/src/test/app/LendingHelpers_test.cpp b/src/test/app/LendingHelpers_test.cpp index ee829550e1..58e4c5aaa4 100644 --- a/src/test/app/LendingHelpers_test.cpp +++ b/src/test/app/LendingHelpers_test.cpp @@ -592,20 +592,18 @@ class LendingHelpers_test : public beast::unit_test::suite auto const periodicRate = loanPeriodicRate(loanInterestRate, paymentInterval); Number const overpaymentAmount{50}; - ExtendedPaymentComponents const overpaymentComponents = computeOverpaymentComponents( + auto const overpaymentComponents = computeOverpaymentComponents( asset, loanScale, overpaymentAmount, TenthBips32(0), TenthBips32(0), managementFeeRate); - auto const loanProperites = computeLoanProperties( + auto const loanProperties = computeLoanProperties( asset, loanPrincipal, loanInterestRate, paymentInterval, paymentsRemaining, managementFeeRate, loanScale); - Number const periodicPayment = loanProperites.periodicPayment; - auto const ret = tryOverpayment( asset, loanScale, overpaymentComponents, - loanProperites.loanState, - periodicPayment, + loanProperties.loanState, + loanProperties.periodicPayment, periodicRate, paymentsRemaining, managementFeeRate, @@ -636,20 +634,20 @@ class LendingHelpers_test : public beast::unit_test::suite // =========== VALIDATE STATE CHANGES =========== BEAST_EXPECTS( - loanProperites.loanState.interestDue - newState.interestDue == 0, + loanProperties.loanState.interestDue - newState.interestDue == 0, " interest change mismatch: expected 0, got " + - to_string(loanProperites.loanState.interestDue - newState.interestDue)); + to_string(loanProperties.loanState.interestDue - newState.interestDue)); BEAST_EXPECTS( - loanProperites.loanState.managementFeeDue - newState.managementFeeDue == 0, + loanProperties.loanState.managementFeeDue - newState.managementFeeDue == 0, " management fee change mismatch: expected 0, got " + - to_string(loanProperites.loanState.managementFeeDue - newState.managementFeeDue)); + to_string(loanProperties.loanState.managementFeeDue - newState.managementFeeDue)); BEAST_EXPECTS( actualPaymentParts.principalPaid == - loanProperites.loanState.principalOutstanding - newState.principalOutstanding, + loanProperties.loanState.principalOutstanding - newState.principalOutstanding, " principalPaid mismatch: expected " + - to_string(loanProperites.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + + to_string(loanProperties.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + to_string(actualPaymentParts.principalPaid)); } @@ -672,7 +670,7 @@ class LendingHelpers_test : public beast::unit_test::suite std::uint32_t const paymentsRemaining = 10; auto const periodicRate = loanPeriodicRate(loanInterestRate, paymentInterval); - ExtendedPaymentComponents const overpaymentComponents = computeOverpaymentComponents( + auto const overpaymentComponents = computeOverpaymentComponents( asset, loanScale, Number{50, 0}, @@ -680,17 +678,15 @@ class LendingHelpers_test : public beast::unit_test::suite TenthBips32(10'000), // 10% overpayment fee managementFeeRate); - auto const loanProperites = computeLoanProperties( + auto const loanProperties = computeLoanProperties( asset, loanPrincipal, loanInterestRate, paymentInterval, paymentsRemaining, managementFeeRate, loanScale); - Number const periodicPayment = loanProperites.periodicPayment; - auto const ret = tryOverpayment( asset, loanScale, overpaymentComponents, - loanProperites.loanState, - periodicPayment, + loanProperties.loanState, + loanProperties.periodicPayment, periodicRate, paymentsRemaining, managementFeeRate, @@ -721,21 +717,21 @@ class LendingHelpers_test : public beast::unit_test::suite // =========== VALIDATE STATE CHANGES =========== // With no Loan interest, interest outstanding should not change BEAST_EXPECTS( - loanProperites.loanState.interestDue - newState.interestDue == 0, + loanProperties.loanState.interestDue - newState.interestDue == 0, " interest change mismatch: expected 0, got " + - to_string(loanProperites.loanState.interestDue - newState.interestDue)); + to_string(loanProperties.loanState.interestDue - newState.interestDue)); // With no Loan management fee, management fee due should not change BEAST_EXPECTS( - loanProperites.loanState.managementFeeDue - newState.managementFeeDue == 0, + loanProperties.loanState.managementFeeDue - newState.managementFeeDue == 0, " management fee change mismatch: expected 0, got " + - to_string(loanProperites.loanState.managementFeeDue - newState.managementFeeDue)); + to_string(loanProperties.loanState.managementFeeDue - newState.managementFeeDue)); BEAST_EXPECTS( actualPaymentParts.principalPaid == - loanProperites.loanState.principalOutstanding - newState.principalOutstanding, + loanProperties.loanState.principalOutstanding - newState.principalOutstanding, " principalPaid mismatch: expected " + - to_string(loanProperites.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + + to_string(loanProperties.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + to_string(actualPaymentParts.principalPaid)); } @@ -758,7 +754,7 @@ class LendingHelpers_test : public beast::unit_test::suite std::uint32_t const paymentsRemaining = 10; auto const periodicRate = loanPeriodicRate(loanInterestRate, paymentInterval); - ExtendedPaymentComponents const overpaymentComponents = computeOverpaymentComponents( + auto const overpaymentComponents = computeOverpaymentComponents( asset, loanScale, Number{50, 0}, @@ -766,17 +762,15 @@ class LendingHelpers_test : public beast::unit_test::suite TenthBips32(0), // 0% overpayment fee managementFeeRate); - auto const loanProperites = computeLoanProperties( + auto const loanProperties = computeLoanProperties( asset, loanPrincipal, loanInterestRate, paymentInterval, paymentsRemaining, managementFeeRate, loanScale); - Number const periodicPayment = loanProperites.periodicPayment; - auto const ret = tryOverpayment( asset, loanScale, overpaymentComponents, - loanProperites.loanState, - periodicPayment, + loanProperties.loanState, + loanProperties.periodicPayment, periodicRate, paymentsRemaining, managementFeeRate, @@ -812,22 +806,22 @@ class LendingHelpers_test : public beast::unit_test::suite // =========== VALIDATE STATE CHANGES =========== BEAST_EXPECTS( actualPaymentParts.principalPaid == - loanProperites.loanState.principalOutstanding - newState.principalOutstanding, + loanProperties.loanState.principalOutstanding - newState.principalOutstanding, " principalPaid mismatch: expected " + - to_string(loanProperites.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + + to_string(loanProperties.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + to_string(actualPaymentParts.principalPaid)); BEAST_EXPECTS( - actualPaymentParts.valueChange == newState.interestDue - loanProperites.loanState.interestDue, + actualPaymentParts.valueChange == newState.interestDue - loanProperties.loanState.interestDue, " valueChange mismatch: expected " + - to_string(newState.interestDue - loanProperites.loanState.interestDue) + ", got " + + to_string(newState.interestDue - loanProperties.loanState.interestDue) + ", got " + to_string(actualPaymentParts.valueChange)); // With no Loan management fee, management fee due should not change BEAST_EXPECTS( - loanProperites.loanState.managementFeeDue - newState.managementFeeDue == 0, + loanProperties.loanState.managementFeeDue - newState.managementFeeDue == 0, " management fee change mismatch: expected 0, got " + - to_string(loanProperites.loanState.managementFeeDue - newState.managementFeeDue)); + to_string(loanProperties.loanState.managementFeeDue - newState.managementFeeDue)); } void @@ -849,7 +843,7 @@ class LendingHelpers_test : public beast::unit_test::suite std::uint32_t const paymentsRemaining = 10; auto const periodicRate = loanPeriodicRate(loanInterestRate, paymentInterval); - ExtendedPaymentComponents const overpaymentComponents = computeOverpaymentComponents( + auto const overpaymentComponents = computeOverpaymentComponents( asset, loanScale, Number{50, 0}, @@ -857,17 +851,15 @@ class LendingHelpers_test : public beast::unit_test::suite TenthBips32(0), // 0% overpayment fee managementFeeRate); - auto const loanProperites = computeLoanProperties( + auto const loanProperties = computeLoanProperties( asset, loanPrincipal, loanInterestRate, paymentInterval, paymentsRemaining, managementFeeRate, loanScale); - Number const periodicPayment = loanProperites.periodicPayment; - auto const ret = tryOverpayment( asset, loanScale, overpaymentComponents, - loanProperites.loanState, - periodicPayment, + loanProperties.loanState, + loanProperties.periodicPayment, periodicRate, paymentsRemaining, managementFeeRate, @@ -904,26 +896,26 @@ class LendingHelpers_test : public beast::unit_test::suite // =========== VALIDATE STATE CHANGES =========== BEAST_EXPECTS( actualPaymentParts.principalPaid == - loanProperites.loanState.principalOutstanding - newState.principalOutstanding, + loanProperties.loanState.principalOutstanding - newState.principalOutstanding, " principalPaid mismatch: expected " + - to_string(loanProperites.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + + to_string(loanProperties.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + to_string(actualPaymentParts.principalPaid)); // The change in interest is equal to the value change sans the // overpayment interest BEAST_EXPECTS( actualPaymentParts.valueChange - actualPaymentParts.interestPaid == - newState.interestDue - loanProperites.loanState.interestDue, + newState.interestDue - loanProperties.loanState.interestDue, " valueChange mismatch: expected " + to_string( - newState.interestDue - loanProperites.loanState.interestDue + actualPaymentParts.interestPaid) + + newState.interestDue - loanProperties.loanState.interestDue + actualPaymentParts.interestPaid) + ", got " + to_string(actualPaymentParts.valueChange)); // With no Loan management fee, management fee due should not change BEAST_EXPECTS( - loanProperites.loanState.managementFeeDue - newState.managementFeeDue == 0, + loanProperties.loanState.managementFeeDue - newState.managementFeeDue == 0, " management fee change mismatch: expected 0, got " + - to_string(loanProperites.loanState.managementFeeDue - newState.managementFeeDue)); + to_string(loanProperties.loanState.managementFeeDue - newState.managementFeeDue)); } void @@ -947,7 +939,7 @@ class LendingHelpers_test : public beast::unit_test::suite std::uint32_t const paymentsRemaining = 10; auto const periodicRate = loanPeriodicRate(loanInterestRate, paymentInterval); - ExtendedPaymentComponents const overpaymentComponents = computeOverpaymentComponents( + auto const overpaymentComponents = computeOverpaymentComponents( asset, loanScale, Number{50, 0}, @@ -955,17 +947,15 @@ class LendingHelpers_test : public beast::unit_test::suite TenthBips32(0), // 0% overpayment fee managementFeeRate); - auto const loanProperites = computeLoanProperties( + auto const loanProperties = computeLoanProperties( asset, loanPrincipal, loanInterestRate, paymentInterval, paymentsRemaining, managementFeeRate, loanScale); - Number const periodicPayment = loanProperites.periodicPayment; - auto const ret = tryOverpayment( asset, loanScale, overpaymentComponents, - loanProperites.loanState, - periodicPayment, + loanProperties.loanState, + loanProperties.periodicPayment, periodicRate, paymentsRemaining, managementFeeRate, @@ -1004,23 +994,23 @@ class LendingHelpers_test : public beast::unit_test::suite // =========== VALIDATE STATE CHANGES =========== BEAST_EXPECTS( actualPaymentParts.principalPaid == - loanProperites.loanState.principalOutstanding - newState.principalOutstanding, + loanProperties.loanState.principalOutstanding - newState.principalOutstanding, " principalPaid mismatch: expected " + - to_string(loanProperites.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + + to_string(loanProperties.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + to_string(actualPaymentParts.principalPaid)); // Note that the management fee value change is not captured, as this // value is not needed to correctly update the Vault state. BEAST_EXPECTS( - (newState.managementFeeDue - loanProperites.loanState.managementFeeDue == Number{-20592, -5}), + (newState.managementFeeDue - loanProperties.loanState.managementFeeDue == Number{-20592, -5}), " management fee change mismatch: expected " + to_string(Number{-20592, -5}) + ", got " + - to_string(newState.managementFeeDue - loanProperites.loanState.managementFeeDue)); + to_string(newState.managementFeeDue - loanProperties.loanState.managementFeeDue)); BEAST_EXPECTS( actualPaymentParts.valueChange - actualPaymentParts.interestPaid == - newState.interestDue - loanProperites.loanState.interestDue, + newState.interestDue - loanProperties.loanState.interestDue, " valueChange mismatch: expected " + - to_string(newState.interestDue - loanProperites.loanState.interestDue) + ", got " + + to_string(newState.interestDue - loanProperties.loanState.interestDue) + ", got " + to_string(actualPaymentParts.valueChange - actualPaymentParts.interestPaid)); } @@ -1043,7 +1033,7 @@ class LendingHelpers_test : public beast::unit_test::suite std::uint32_t const paymentsRemaining = 10; auto const periodicRate = loanPeriodicRate(loanInterestRate, paymentInterval); - ExtendedPaymentComponents const overpaymentComponents = computeOverpaymentComponents( + auto const overpaymentComponents = computeOverpaymentComponents( asset, loanScale, Number{50, 0}, @@ -1051,17 +1041,15 @@ class LendingHelpers_test : public beast::unit_test::suite TenthBips32(10'000), // 10% overpayment fee managementFeeRate); - auto const loanProperites = computeLoanProperties( + auto const loanProperties = computeLoanProperties( asset, loanPrincipal, loanInterestRate, paymentInterval, paymentsRemaining, managementFeeRate, loanScale); - Number const periodicPayment = loanProperites.periodicPayment; - auto const ret = tryOverpayment( asset, loanScale, overpaymentComponents, - loanProperites.loanState, - periodicPayment, + loanProperties.loanState, + loanProperties.periodicPayment, periodicRate, paymentsRemaining, managementFeeRate, @@ -1101,23 +1089,23 @@ class LendingHelpers_test : public beast::unit_test::suite BEAST_EXPECTS( actualPaymentParts.principalPaid == - loanProperites.loanState.principalOutstanding - newState.principalOutstanding, + loanProperties.loanState.principalOutstanding - newState.principalOutstanding, " principalPaid mismatch: expected " + - to_string(loanProperites.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + + to_string(loanProperties.loanState.principalOutstanding - newState.principalOutstanding) + ", got " + to_string(actualPaymentParts.principalPaid)); // Note that the management fee value change is not captured, as this // value is not needed to correctly update the Vault state. BEAST_EXPECTS( - (newState.managementFeeDue - loanProperites.loanState.managementFeeDue == Number{-18304, -5}), + (newState.managementFeeDue - loanProperties.loanState.managementFeeDue == Number{-18304, -5}), " management fee change mismatch: expected " + to_string(Number{-18304, -5}) + ", got " + - to_string(newState.managementFeeDue - loanProperites.loanState.managementFeeDue)); + to_string(newState.managementFeeDue - loanProperties.loanState.managementFeeDue)); BEAST_EXPECTS( actualPaymentParts.valueChange - actualPaymentParts.interestPaid == - newState.interestDue - loanProperites.loanState.interestDue, + newState.interestDue - loanProperties.loanState.interestDue, " valueChange mismatch: expected " + - to_string(newState.interestDue - loanProperites.loanState.interestDue) + ", got " + + to_string(newState.interestDue - loanProperties.loanState.interestDue) + ", got " + to_string(actualPaymentParts.valueChange - actualPaymentParts.interestPaid)); } From 6006c281e22b8a907a3e3c993449ad42e598ed32 Mon Sep 17 00:00:00 2001 From: Niq Dudfield Date: Thu, 5 Feb 2026 22:40:27 +0700 Subject: [PATCH 02/12] fix: Increment sequence when accepting new manifests (#6059) The `ManifestCache::applyManifest` function was returning early without incrementing `seq_`. `OverlayImpl `uses this sequence to identify/invalidate a cached `TMManifests` message, which is exchanged with peers on connection. Depending on network size, startup sequencing, and topology, this can cause syncing issues. This change therefore increments `seq_` when a new manifest is accepted. --- src/test/app/Manifest_test.cpp | 5 +++++ src/xrpld/app/misc/detail/Manifest.cpp | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/test/app/Manifest_test.cpp b/src/test/app/Manifest_test.cpp index f5004a3200..598949f662 100644 --- a/src/test/app/Manifest_test.cpp +++ b/src/test/app/Manifest_test.cpp @@ -827,8 +827,13 @@ public: // applyManifest should accept new manifests with // higher sequence numbers + auto const seq0 = cache.sequence(); BEAST_EXPECT(cache.applyManifest(clone(s_a0)) == ManifestDisposition::accepted); + BEAST_EXPECT(cache.sequence() > seq0); + + auto const seq1 = cache.sequence(); BEAST_EXPECT(cache.applyManifest(clone(s_a0)) == ManifestDisposition::stale); + BEAST_EXPECT(cache.sequence() == seq1); BEAST_EXPECT(cache.applyManifest(clone(s_a1)) == ManifestDisposition::accepted); BEAST_EXPECT(cache.applyManifest(clone(s_a1)) == ManifestDisposition::stale); diff --git a/src/xrpld/app/misc/detail/Manifest.cpp b/src/xrpld/app/misc/detail/Manifest.cpp index c226407231..952814656b 100644 --- a/src/xrpld/app/misc/detail/Manifest.cpp +++ b/src/xrpld/app/misc/detail/Manifest.cpp @@ -459,6 +459,10 @@ ManifestCache::applyManifest(Manifest m) auto masterKey = m.masterKey; map_.emplace(std::move(masterKey), std::move(m)); + + // Something has changed. Keep track of it. + seq_++; + return ManifestDisposition::accepted; } From 0a626d95f435f603b89ba5bf95fe201c71088e7a Mon Sep 17 00:00:00 2001 From: Bart Date: Thu, 5 Feb 2026 11:45:57 -0500 Subject: [PATCH 03/12] refactor: Update secp256k1 to 0.7.1 (#6331) The latest secp256k1 release, 0.7.1, contains bug fixes that we may benefit from, see https://github.com/bitcoin-core/secp256k1/blob/master/CHANGELOG.md. --- conan.lock | 2 +- conanfile.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conan.lock b/conan.lock index 3b4d835564..900d3526e1 100644 --- a/conan.lock +++ b/conan.lock @@ -6,7 +6,7 @@ "sqlite3/3.49.1#8631739a4c9b93bd3d6b753bac548a63%1765850149.926", "soci/4.0.3#a9f8d773cd33e356b5879a4b0564f287%1765850149.46", "snappy/1.1.10#968fef506ff261592ec30c574d4a7809%1765850147.878", - "secp256k1/0.7.0#0fda78daa3b864deb8a2fbc083398356%1770226294.524", + "secp256k1/0.7.1#3a61e95e220062ef32c48d019e9c81f7%1770306721.686", "rocksdb/10.5.1#4a197eca381a3e5ae8adf8cffa5aacd0%1765850186.86", "re2/20230301#ca3b241baec15bd31ea9187150e0b333%1765850148.103", "protobuf/6.32.1#f481fd276fc23a33b85a3ed1e898b693%1765850161.038", diff --git a/conanfile.py b/conanfile.py index 4fb2deeec8..85406bf570 100644 --- a/conanfile.py +++ b/conanfile.py @@ -32,7 +32,7 @@ class Xrpl(ConanFile): "libarchive/3.8.1", "nudb/2.0.9", "openssl/3.5.5", - "secp256k1/0.7.0", + "secp256k1/0.7.1", "soci/4.0.3", "zlib/1.3.1", ] From 25d7c2c4ec7580db602145c258ff49463a58f14a Mon Sep 17 00:00:00 2001 From: Bart Date: Fri, 6 Feb 2026 09:12:45 -0500 Subject: [PATCH 04/12] chore: Restore unity builds (#6328) In certain cases, such as when modifying headers used by many compilation units, performing a unity build is slower than when performing a regular build with `ccache` enabled. There is also a benefit to a unity build in that it can detect things such as macro redefinitions within the group of files that are compiled together as a unit. This change therefore restores the ability to perform unity builds. However, instead of running every configuration with and without unity enabled, it is now only enabled for a single configuration to maintain lower computational use. As part of restoring the code, it became clear that currently two configurations have coverage enabled, since the check doesn't focus specifically on Debian Bookworm so it also applies to Debian Trixie. This has been fixed too in this change. --- .github/scripts/strategy-matrix/generate.py | 17 +++++++++++++++-- BUILD.md | 7 +++++++ cmake/XrplCore.cmake | 5 +++++ cmake/XrplSettings.cmake | 8 ++++++++ conanfile.py | 3 +++ src/libxrpl/net/RegisterSSLCerts.cpp | 3 +-- 6 files changed, 39 insertions(+), 4 deletions(-) diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index 75847b4d9d..27eb60c005 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -196,11 +196,22 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: # Enable code coverage for Debian Bookworm using GCC 15 in Debug on # linux/amd64 if ( - f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-15" + f"{os['distro_name']}-{os['distro_version']}" == "debian-bookworm" + and f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-15" and build_type == "Debug" and architecture["platform"] == "linux/amd64" ): - cmake_args = f"-Dcoverage=ON -Dcoverage_format=xml -DCODE_COVERAGE_VERBOSE=ON -DCMAKE_C_FLAGS=-O0 -DCMAKE_CXX_FLAGS=-O0 {cmake_args}" + cmake_args = f"{cmake_args} -Dcoverage=ON -Dcoverage_format=xml -DCODE_COVERAGE_VERBOSE=ON -DCMAKE_C_FLAGS=-O0 -DCMAKE_CXX_FLAGS=-O0" + + # Enable unity build for Ubuntu Jammy using GCC 12 in Debug on + # linux/amd64. + if ( + f"{os['distro_name']}-{os['distro_version']}" == "ubuntu-jammy" + and f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-12" + and build_type == "Debug" + and architecture["platform"] == "linux/amd64" + ): + cmake_args = f"{cmake_args} -Dunity=ON" # Generate a unique name for the configuration, e.g. macos-arm64-debug # or debian-bookworm-gcc-12-amd64-release. @@ -217,6 +228,8 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: config_name += f"-{build_type.lower()}" if "-Dcoverage=ON" in cmake_args: config_name += "-coverage" + if "-Dunity=ON" in cmake_args: + config_name += "-unity" # Add the configuration to the list, with the most unique fields first, # so that they are easier to identify in the GitHub Actions UI, as long diff --git a/BUILD.md b/BUILD.md index 35668aeabd..4d01700a61 100644 --- a/BUILD.md +++ b/BUILD.md @@ -575,10 +575,16 @@ See [Sanitizers docs](./docs/build/sanitizers.md) for more details. | `assert` | OFF | Enable assertions. | | `coverage` | OFF | Prepare the coverage report. | | `tests` | OFF | Build tests. | +| `unity` | OFF | Configure a unity build. | | `xrpld` | OFF | Build the xrpld application, and not just the libxrpl library. | | `werr` | OFF | Treat compilation warnings as errors | | `wextra` | OFF | Enable additional compilation warnings | +[Unity builds][5] may be faster for the first build (at the cost of much more +memory) since they concatenate sources into fewer translation units. Non-unity +builds may be faster for incremental builds, and can be helpful for detecting +`#include` omissions. + ## Troubleshooting ### Conan @@ -645,6 +651,7 @@ If you want to experiment with a new package, follow these steps: [1]: https://github.com/conan-io/conan-center-index/issues/13168 [2]: https://en.cppreference.com/w/cpp/compiler_support/20 [3]: https://docs.conan.io/en/latest/getting_started.html +[5]: https://en.wikipedia.org/wiki/Unity_build [6]: https://github.com/boostorg/beast/issues/2648 [7]: https://github.com/boostorg/beast/issues/2661 [gcovr]: https://gcovr.com/en/stable/getting-started.html diff --git a/cmake/XrplCore.cmake b/cmake/XrplCore.cmake index afefa8457d..57d0e83348 100644 --- a/cmake/XrplCore.cmake +++ b/cmake/XrplCore.cmake @@ -4,7 +4,12 @@ include(target_protobuf_sources) +# Protocol buffers cannot participate in a unity build, +# because all the generated sources +# define a bunch of `static const` variables with the same names, +# so we just build them as a separate library. add_library(xrpl.libpb) +set_target_properties(xrpl.libpb PROPERTIES UNITY_BUILD OFF) target_protobuf_sources(xrpl.libpb xrpl/proto LANGUAGE cpp IMPORT_DIRS include/xrpl/proto PROTOS include/xrpl/proto/xrpl.proto) diff --git a/cmake/XrplSettings.cmake b/cmake/XrplSettings.cmake index 0957e41918..6d332be19d 100644 --- a/cmake/XrplSettings.cmake +++ b/cmake/XrplSettings.cmake @@ -30,6 +30,14 @@ if (tests) endif () endif () +option(unity "Creates a build using UNITY support in cmake." OFF) +if (unity) + if (NOT is_ci) + set(CMAKE_UNITY_BUILD_BATCH_SIZE 15 CACHE STRING "") + endif () + set(CMAKE_UNITY_BUILD ON CACHE BOOL "Do a unity build") +endif () + if (is_clang AND is_linux) option(voidstar "Enable Antithesis instrumentation." OFF) endif () diff --git a/conanfile.py b/conanfile.py index 85406bf570..7300b537d9 100644 --- a/conanfile.py +++ b/conanfile.py @@ -23,6 +23,7 @@ class Xrpl(ConanFile): "shared": [True, False], "static": [True, False], "tests": [True, False], + "unity": [True, False], "xrpld": [True, False], } @@ -54,6 +55,7 @@ class Xrpl(ConanFile): "shared": False, "static": True, "tests": False, + "unity": False, "xrpld": False, "date/*:header_only": True, "ed25519/*:shared": False, @@ -166,6 +168,7 @@ class Xrpl(ConanFile): tc.variables["rocksdb"] = self.options.rocksdb tc.variables["BUILD_SHARED_LIBS"] = self.options.shared tc.variables["static"] = self.options.static + tc.variables["unity"] = self.options.unity tc.variables["xrpld"] = self.options.xrpld tc.generate() diff --git a/src/libxrpl/net/RegisterSSLCerts.cpp b/src/libxrpl/net/RegisterSSLCerts.cpp index 1b97a17e5c..a15472969e 100644 --- a/src/libxrpl/net/RegisterSSLCerts.cpp +++ b/src/libxrpl/net/RegisterSSLCerts.cpp @@ -85,8 +85,7 @@ registerSSLCerts(boost::asio::ssl::context& ctx, boost::system::error_code& ec, // There is a very unpleasant interaction between and // openssl x509 types (namely the former has macros that stomp // on the latter), these undefs allow this TU to be safely used in -// unity builds without messing up subsequent TUs. Although we -// no longer use unity builds, leaving the undefs here does no harm. +// unity builds without messing up subsequent TUs. #if BOOST_OS_WINDOWS #undef X509_NAME #undef X509_EXTENSIONS From 677758b1cc9d8afc190582a75160425096708f54 Mon Sep 17 00:00:00 2001 From: Bart Date: Fri, 6 Feb 2026 09:42:35 -0500 Subject: [PATCH 05/12] perf: Remove unnecessary caches (#5439) This change removes the cache in `DatabaseNodeImp` and simplifies the caching logic in `SHAMapStoreImp`. As NuDB and RocksDB internally already use caches, additional caches in the code are not very valuable or may even be unnecessary, as also confirmed during preliminary performance analyses. --- cfg/xrpld-example.cfg | 20 +-- include/xrpl/nodestore/Database.h | 4 - .../xrpl/nodestore/detail/DatabaseNodeImp.h | 32 ---- .../nodestore/detail/DatabaseRotatingImp.h | 3 - src/libxrpl/nodestore/DatabaseNodeImp.cpp | 148 +++++------------- src/libxrpl/nodestore/DatabaseRotatingImp.cpp | 6 - src/test/app/SHAMapStore_test.cpp | 18 +-- src/xrpld/app/main/Application.cpp | 4 - src/xrpld/app/misc/SHAMapStoreImp.cpp | 21 +-- src/xrpld/app/misc/SHAMapStoreImp.h | 2 - 10 files changed, 47 insertions(+), 211 deletions(-) diff --git a/cfg/xrpld-example.cfg b/cfg/xrpld-example.cfg index 93fab4a9ba..995d4e65ff 100644 --- a/cfg/xrpld-example.cfg +++ b/cfg/xrpld-example.cfg @@ -940,23 +940,7 @@ # # path Location to store the database # -# Optional keys -# -# cache_size Size of cache for database records. Default is 16384. -# Setting this value to 0 will use the default value. -# -# cache_age Length of time in minutes to keep database records -# cached. Default is 5 minutes. Setting this value to -# 0 will use the default value. -# -# Note: if neither cache_size nor cache_age is -# specified, the cache for database records will not -# be created. If only one of cache_size or cache_age -# is specified, the cache will be created using the -# default value for the unspecified parameter. -# -# Note: the cache will not be created if online_delete -# is specified. +# Optional keys for NuDB and RocksDB: # # fast_load Boolean. If set, load the last persisted ledger # from disk upon process start before syncing to @@ -964,8 +948,6 @@ # if sufficient IOPS capacity is available. # Default 0. # -# Optional keys for NuDB or RocksDB: -# # earliest_seq The default is 32570 to match the XRP ledger # network's earliest allowed sequence. Alternate # networks may set this value. Minimum value of 1. diff --git a/include/xrpl/nodestore/Database.h b/include/xrpl/nodestore/Database.h index d3dafff85d..d1f5b1bda2 100644 --- a/include/xrpl/nodestore/Database.h +++ b/include/xrpl/nodestore/Database.h @@ -133,10 +133,6 @@ public: std::uint32_t ledgerSeq, std::function const&)>&& callback); - /** Remove expired entries from the positive and negative caches. */ - virtual void - sweep() = 0; - /** Gather statistics pertaining to read and write activities. * * @param obj Json object reference into which to place counters. diff --git a/include/xrpl/nodestore/detail/DatabaseNodeImp.h b/include/xrpl/nodestore/detail/DatabaseNodeImp.h index dd6b8f9ddd..94859c4d22 100644 --- a/include/xrpl/nodestore/detail/DatabaseNodeImp.h +++ b/include/xrpl/nodestore/detail/DatabaseNodeImp.h @@ -23,32 +23,6 @@ public: beast::Journal j) : Database(scheduler, readThreads, config, j), backend_(std::move(backend)) { - std::optional cacheSize, cacheAge; - - if (config.exists("cache_size")) - { - cacheSize = get(config, "cache_size"); - if (cacheSize.value() < 0) - { - Throw("Specified negative value for cache_size"); - } - } - - if (config.exists("cache_age")) - { - cacheAge = get(config, "cache_age"); - if (cacheAge.value() < 0) - { - Throw("Specified negative value for cache_age"); - } - } - - if (cacheSize != 0 || cacheAge != 0) - { - cache_ = std::make_shared>( - "DatabaseNodeImp", cacheSize.value_or(0), std::chrono::minutes(cacheAge.value_or(0)), stopwatch(), j); - } - XRPL_ASSERT( backend_, "xrpl::NodeStore::DatabaseNodeImp::DatabaseNodeImp : non-null " @@ -103,13 +77,7 @@ public: std::uint32_t ledgerSeq, std::function const&)>&& callback) override; - void - sweep() override; - private: - // Cache for database objects. This cache is not always initialized. Check - // for null before using. - std::shared_ptr> cache_; // Persistent key/value storage std::shared_ptr backend_; diff --git a/include/xrpl/nodestore/detail/DatabaseRotatingImp.h b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h index e49c195d67..1a378c54c7 100644 --- a/include/xrpl/nodestore/detail/DatabaseRotatingImp.h +++ b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h @@ -55,9 +55,6 @@ public: void sync() override; - void - sweep() override; - private: std::shared_ptr writableBackend_; std::shared_ptr archiveBackend_; diff --git a/src/libxrpl/nodestore/DatabaseNodeImp.cpp b/src/libxrpl/nodestore/DatabaseNodeImp.cpp index f49867514c..11152a2027 100644 --- a/src/libxrpl/nodestore/DatabaseNodeImp.cpp +++ b/src/libxrpl/nodestore/DatabaseNodeImp.cpp @@ -10,11 +10,6 @@ DatabaseNodeImp::store(NodeObjectType type, Blob&& data, uint256 const& hash, st auto obj = NodeObject::createObject(type, std::move(data), hash); backend_->store(obj); - if (cache_) - { - // After the store, replace a negative cache entry if there is one - cache_->canonicalize(hash, obj, [](std::shared_ptr const& n) { return n->getType() == hotDUMMY; }); - } } void @@ -23,77 +18,36 @@ DatabaseNodeImp::asyncFetch( std::uint32_t ledgerSeq, std::function const&)>&& callback) { - if (cache_) - { - std::shared_ptr obj = cache_->fetch(hash); - if (obj) - { - callback(obj->getType() == hotDUMMY ? nullptr : obj); - return; - } - } Database::asyncFetch(hash, ledgerSeq, std::move(callback)); } -void -DatabaseNodeImp::sweep() -{ - if (cache_) - cache_->sweep(); -} - std::shared_ptr DatabaseNodeImp::fetchNodeObject(uint256 const& hash, std::uint32_t, FetchReport& fetchReport, bool duplicate) { - std::shared_ptr nodeObject = cache_ ? cache_->fetch(hash) : nullptr; + std::shared_ptr nodeObject = nullptr; + Status status; - if (!nodeObject) + try { - JLOG(j_.trace()) << "fetchNodeObject " << hash << ": record not " << (cache_ ? "cached" : "found"); - - Status status; - - try - { - status = backend_->fetch(hash.data(), &nodeObject); - } - catch (std::exception const& e) - { - JLOG(j_.fatal()) << "fetchNodeObject " << hash << ": Exception fetching from backend: " << e.what(); - Rethrow(); - } - - switch (status) - { - case ok: - if (cache_) - { - if (nodeObject) - cache_->canonicalize_replace_client(hash, nodeObject); - else - { - auto notFound = NodeObject::createObject(hotDUMMY, {}, hash); - cache_->canonicalize_replace_client(hash, notFound); - if (notFound->getType() != hotDUMMY) - nodeObject = notFound; - } - } - break; - case notFound: - break; - case dataCorrupt: - JLOG(j_.fatal()) << "fetchNodeObject " << hash << ": nodestore data is corrupted"; - break; - default: - JLOG(j_.warn()) << "fetchNodeObject " << hash << ": backend returns unknown result " << status; - break; - } + status = backend_->fetch(hash.data(), &nodeObject); } - else + catch (std::exception const& e) { - JLOG(j_.trace()) << "fetchNodeObject " << hash << ": record found in cache"; - if (nodeObject->getType() == hotDUMMY) - nodeObject.reset(); + JLOG(j_.fatal()) << "fetchNodeObject " << hash << ": Exception fetching from backend: " << e.what(); + Rethrow(); + } + + switch (status) + { + case ok: + case notFound: + break; + case dataCorrupt: + JLOG(j_.fatal()) << "fetchNodeObject " << hash << ": nodestore data is corrupted"; + break; + default: + JLOG(j_.warn()) << "fetchNodeObject " << hash << ": backend returns unknown result " << status; + break; } if (nodeObject) @@ -105,66 +59,36 @@ DatabaseNodeImp::fetchNodeObject(uint256 const& hash, std::uint32_t, FetchReport std::vector> DatabaseNodeImp::fetchBatch(std::vector const& hashes) { - std::vector> results{hashes.size()}; using namespace std::chrono; auto const before = steady_clock::now(); - std::unordered_map indexMap; - std::vector cacheMisses; - uint64_t hits = 0; - uint64_t fetches = 0; + + std::vector batch{}; + batch.reserve(hashes.size()); for (size_t i = 0; i < hashes.size(); ++i) { auto const& hash = hashes[i]; - // See if the object already exists in the cache - auto nObj = cache_ ? cache_->fetch(hash) : nullptr; - ++fetches; - if (!nObj) - { - // Try the database - indexMap[&hash] = i; - cacheMisses.push_back(&hash); - } - else - { - results[i] = nObj->getType() == hotDUMMY ? nullptr : nObj; - // It was in the cache. - ++hits; - } + batch.push_back(&hash); } - JLOG(j_.debug()) << "fetchBatch - cache hits = " << (hashes.size() - cacheMisses.size()) - << " - cache misses = " << cacheMisses.size(); - auto dbResults = backend_->fetchBatch(cacheMisses).first; - - for (size_t i = 0; i < dbResults.size(); ++i) + // Get the node objects that match the hashes from the backend. To protect + // against the backends returning fewer or more results than expected, the + // container is resized to the number of hashes. + auto results = backend_->fetchBatch(batch).first; + XRPL_ASSERT( + results.size() == hashes.size() || results.empty(), + "number of output objects either matches number of input hashes or is empty"); + results.resize(hashes.size()); + for (size_t i = 0; i < results.size(); ++i) { - auto nObj = std::move(dbResults[i]); - size_t index = indexMap[cacheMisses[i]]; - auto const& hash = hashes[index]; - - if (nObj) - { - // Ensure all threads get the same object - if (cache_) - cache_->canonicalize_replace_client(hash, nObj); - } - else + if (!results[i]) { JLOG(j_.error()) << "fetchBatch - " - << "record not found in db or cache. hash = " << strHex(hash); - if (cache_) - { - auto notFound = NodeObject::createObject(hotDUMMY, {}, hash); - cache_->canonicalize_replace_client(hash, notFound); - if (notFound->getType() != hotDUMMY) - nObj = std::move(notFound); - } + << "record not found in db. hash = " << strHex(hashes[i]); } - results[index] = std::move(nObj); } auto fetchDurationUs = std::chrono::duration_cast(steady_clock::now() - before).count(); - updateFetchMetrics(fetches, hits, fetchDurationUs); + updateFetchMetrics(hashes.size(), 0, fetchDurationUs); return results; } diff --git a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp index 2e72dca969..f25b9d068e 100644 --- a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp +++ b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp @@ -93,12 +93,6 @@ DatabaseRotatingImp::store(NodeObjectType type, Blob&& data, uint256 const& hash storeStats(1, nObj->getData().size()); } -void -DatabaseRotatingImp::sweep() -{ - // nothing to do -} - std::shared_ptr DatabaseRotatingImp::fetchNodeObject(uint256 const& hash, std::uint32_t, FetchReport& fetchReport, bool duplicate) { diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index 91845709c5..7951da26d1 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -490,19 +490,8 @@ public: Env env(*this, envconfig(onlineDelete)); ///////////////////////////////////////////////////////////// - // Create the backend. Normally, SHAMapStoreImp handles all these - // details - auto nscfg = env.app().config().section(ConfigSection::nodeDatabase()); - - // Provide default values: - if (!nscfg.exists("cache_size")) - nscfg.set( - "cache_size", std::to_string(env.app().config().getValueFor(SizedItem::treeCacheSize, std::nullopt))); - - if (!nscfg.exists("cache_age")) - nscfg.set( - "cache_age", std::to_string(env.app().config().getValueFor(SizedItem::treeCacheAge, std::nullopt))); - + // Create NodeStore with two backends to allow online deletion of data. + // Normally, SHAMapStoreImp handles all these details. NodeStoreScheduler scheduler(env.app().getJobQueue()); std::string const writableDb = "write"; @@ -510,9 +499,8 @@ public: auto writableBackend = makeBackendRotating(env, scheduler, writableDb); auto archiveBackend = makeBackendRotating(env, scheduler, archiveDb); - // Create NodeStore with two backends to allow online deletion of - // data constexpr int readThreads = 4; + auto nscfg = env.app().config().section(ConfigSection::nodeDatabase()); auto dbr = std::make_unique( scheduler, readThreads, diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index 3045097e4a..2c0d3c2b82 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -908,10 +908,6 @@ public: JLOG(m_journal.debug()) << "MasterTransaction sweep. Size before: " << oldMasterTxSize << "; size after: " << masterTxCache.size(); } - { - // Does not appear to have an associated cache. - getNodeStore().sweep(); - } { std::size_t const oldLedgerMasterCacheSize = getLedgerMaster().getFetchPackCacheSize(); diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 0ed3b8a3fc..dbdd682ef8 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -130,14 +130,6 @@ std::unique_ptr SHAMapStoreImp::makeNodeStore(int readThreads) { auto nscfg = app_.config().section(ConfigSection::nodeDatabase()); - - // Provide default values: - if (!nscfg.exists("cache_size")) - nscfg.set("cache_size", std::to_string(app_.config().getValueFor(SizedItem::treeCacheSize, std::nullopt))); - - if (!nscfg.exists("cache_age")) - nscfg.set("cache_age", std::to_string(app_.config().getValueFor(SizedItem::treeCacheAge, std::nullopt))); - std::unique_ptr db; if (deleteInterval_) @@ -226,8 +218,6 @@ SHAMapStoreImp::run() LedgerIndex lastRotated = state_db_.getState().lastRotated; netOPs_ = &app_.getOPs(); ledgerMaster_ = &app_.getLedgerMaster(); - fullBelowCache_ = &(*app_.getNodeFamily().getFullBelowCache()); - treeNodeCache_ = &(*app_.getNodeFamily().getTreeNodeCache()); if (advisoryDelete_) canDelete_ = state_db_.getCanDelete(); @@ -490,16 +480,19 @@ void SHAMapStoreImp::clearCaches(LedgerIndex validatedSeq) { ledgerMaster_->clearLedgerCachePrior(validatedSeq); - fullBelowCache_->clear(); + // Also clear the FullBelowCache so its generation counter is bumped. + // This prevents stale "full below" markers from persisting across + // backend rotation/online deletion and interfering with SHAMap sync. + app_.getNodeFamily().getFullBelowCache()->clear(); } void SHAMapStoreImp::freshenCaches() { - if (freshenCache(*treeNodeCache_)) - return; - if (freshenCache(app_.getMasterTransaction().getCache())) + if (freshenCache(*app_.getNodeFamily().getTreeNodeCache())) return; + + freshenCache(app_.getMasterTransaction().getCache()); } void diff --git a/src/xrpld/app/misc/SHAMapStoreImp.h b/src/xrpld/app/misc/SHAMapStoreImp.h index a0475e80d6..b046a78979 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.h +++ b/src/xrpld/app/misc/SHAMapStoreImp.h @@ -93,8 +93,6 @@ private: // as of run() or before NetworkOPs* netOPs_ = nullptr; LedgerMaster* ledgerMaster_ = nullptr; - FullBelowCache* fullBelowCache_ = nullptr; - TreeNodeCache* treeNodeCache_ = nullptr; static constexpr auto nodeStoreName_ = "NodeStore"; From 2305bc98a4c51550504060460d14aa43046379a1 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Fri, 6 Feb 2026 16:39:23 +0000 Subject: [PATCH 06/12] chore: Remove CODEOWNERS (#6337) --- .github/CODEOWNERS | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS deleted file mode 100644 index bc4fe2febd..0000000000 --- a/.github/CODEOWNERS +++ /dev/null @@ -1,8 +0,0 @@ -# Allow anyone to review any change by default. -* - -# Require the rpc-reviewers team to review changes to the rpc code. -include/xrpl/protocol/ @xrplf/rpc-reviewers -src/libxrpl/protocol/ @xrplf/rpc-reviewers -src/xrpld/rpc/ @xrplf/rpc-reviewers -src/xrpld/app/misc/ @xrplf/rpc-reviewers From f5208fc85020eb96de6a2b625ccb7f53c744d07a Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 6 Feb 2026 13:37:01 -0500 Subject: [PATCH 07/12] test: Add file and line location to Env (#6276) This change uses `std::source_location` to output the file and line location of the call that triggered a failed transaction. --- src/test/app/LedgerReplay_test.cpp | 3 +- src/test/jtx/Env.h | 62 +++++++++++++++++++++++++----- src/test/jtx/Env_ss.h | 21 +++++++--- src/test/jtx/impl/Env.cpp | 33 ++++++++-------- src/test/rpc/Subscribe_test.cpp | 3 +- 5 files changed, 88 insertions(+), 34 deletions(-) diff --git a/src/test/app/LedgerReplay_test.cpp b/src/test/app/LedgerReplay_test.cpp index 8bf1a8f668..229cad21c9 100644 --- a/src/test/app/LedgerReplay_test.cpp +++ b/src/test/app/LedgerReplay_test.cpp @@ -491,8 +491,7 @@ struct LedgerServer for (int i = 0; i < newTxes; ++i) { updateIdx(); - env.apply( - pay(accounts[fromIdx], + env(pay(accounts[fromIdx], accounts[toIdx], jtx::drops(ledgerMaster.getClosedLedger()->fees().base) + jtx::XRP(param.txAmount)), jtx::seq(jtx::autofill), diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index ef6240d48a..7d7dcd2cb8 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -42,6 +43,27 @@ namespace xrpl { namespace test { namespace jtx { +/** Wrapper that captures std::source_location when implicitly constructed. + This solves the problem of combining std::source_location with variadic + templates. The std::source_location default argument is evaluated at the + call site when the wrapper is constructed via implicit conversion. + + This is a template struct that holds the value directly, allowing implicit + conversion without template argument deduction issues via CTAD. +*/ +template +struct WithSourceLocation +{ + T value; + std::source_location loc; + + // Non-explicit constructor allows implicit conversion. + // The default argument for loc is evaluated at the call site. + WithSourceLocation(T v, std::source_location l = std::source_location::current()) : value(std::move(v)), loc(l) + { + } +}; + /** Designate accounts as no-ripple in Env::fund */ template std::array @@ -522,35 +544,57 @@ public: This calls postconditions. */ virtual void - submit(JTx const& jt); + submit(JTx const& jt, std::source_location const& loc = std::source_location::current()); /** Use the submit RPC command with a provided JTx object. This calls postconditions. */ void - sign_and_submit(JTx const& jt, Json::Value params = Json::nullValue); + sign_and_submit( + JTx const& jt, + Json::Value params = Json::nullValue, + std::source_location const& loc = std::source_location::current()); /** Check expected postconditions of JTx submission. */ void - postconditions(JTx const& jt, ParsedResult const& parsed, Json::Value const& jr = Json::Value()); + postconditions( + JTx const& jt, + ParsedResult const& parsed, + Json::Value const& jr = Json::Value(), + std::source_location const& loc = std::source_location::current()); /** Apply funclets and submit. */ /** @{ */ - template + template Env& - apply(JsonValue&& jv, FN const&... fN) + apply(WithSourceLocation jv, FN const&... fN) { - submit(jt(std::forward(jv), fN...)); + submit(jt(std::move(jv.value), fN...), jv.loc); return *this; } - template + template Env& - operator()(JsonValue&& jv, FN const&... fN) + apply(WithSourceLocation jv, FN const&... fN) { - return apply(std::forward(jv), fN...); + submit(jt(std::move(jv.value), fN...), jv.loc); + return *this; + } + + template + Env& + operator()(WithSourceLocation jv, FN const&... fN) + { + return apply(std::move(jv), fN...); + } + + template + Env& + operator()(WithSourceLocation jv, FN const&... fN) + { + return apply(std::move(jv), fN...); } /** @} */ diff --git a/src/test/jtx/Env_ss.h b/src/test/jtx/Env_ss.h index 78146a04a4..9c178a1c13 100644 --- a/src/test/jtx/Env_ss.h +++ b/src/test/jtx/Env_ss.h @@ -23,19 +23,20 @@ private: SignSubmitRunner& operator=(SignSubmitRunner&&) = delete; - SignSubmitRunner(Env& env, JTx&& jt) : env_(env), jt_(jt) + SignSubmitRunner(Env& env, JTx&& jt, std::source_location loc) : env_(env), jt_(jt), loc_(loc) { } void operator()(Json::Value const& params = Json::nullValue) { - env_.sign_and_submit(jt_, params); + env_.sign_and_submit(jt_, params, loc_); } private: Env& env_; JTx const jt_; + std::source_location const loc_; }; public: @@ -47,12 +48,20 @@ public: { } - template + template SignSubmitRunner - operator()(JsonValue&& jv, FN const&... fN) + operator()(WithSourceLocation jv, FN const&... fN) { - auto jtx = env_.jt(std::forward(jv), fN...); - return SignSubmitRunner(env_, std::move(jtx)); + auto jtx = env_.jt(std::move(jv.value), fN...); + return SignSubmitRunner(env_, std::move(jtx), jv.loc); + } + + template + SignSubmitRunner + operator()(WithSourceLocation jv, FN const&... fN) + { + auto jtx = env_.jt(std::move(jv.value), fN...); + return SignSubmitRunner(env_, std::move(jtx), jv.loc); } }; diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 9971da05c4..d8bcec84ee 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -27,6 +27,7 @@ #include #include +#include namespace xrpl { namespace test { @@ -330,7 +331,7 @@ Env::parseResult(Json::Value const& jr) } void -Env::submit(JTx const& jt) +Env::submit(JTx const& jt, std::source_location const& loc) { ParsedResult parsedResult; auto const jr = [&]() { @@ -356,11 +357,11 @@ Env::submit(JTx const& jt) return Json::Value(); } }(); - return postconditions(jt, parsedResult, jr); + return postconditions(jt, parsedResult, jr, loc); } void -Env::sign_and_submit(JTx const& jt, Json::Value params) +Env::sign_and_submit(JTx const& jt, Json::Value params, std::source_location const& loc) { auto const account = lookup(jt.jv[jss::Account].asString()); auto const& passphrase = account.name(); @@ -393,25 +394,26 @@ Env::sign_and_submit(JTx const& jt, Json::Value params) test.expect(parsedResult.ter, "ter uninitialized!"); ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED); - return postconditions(jt, parsedResult, jr); + return postconditions(jt, parsedResult, jr, loc); } void -Env::postconditions(JTx const& jt, ParsedResult const& parsed, Json::Value const& jr) +Env::postconditions(JTx const& jt, ParsedResult const& parsed, Json::Value const& jr, std::source_location const& loc) { auto const line = jt.testLine ? " (" + to_string(*jt.testLine) + ")" : ""; - bool bad = !test.expect(parsed.ter, "apply: No ter result!" + line); + auto const locStr = std::string("(") + loc.file_name() + ":" + to_string(loc.line()) + ")"; + bool bad = !test.expect(parsed.ter, "apply " + locStr + ": No ter result!" + line); bad = (jt.ter && parsed.ter && !test.expect( *parsed.ter == *jt.ter, - "apply: Got " + transToken(*parsed.ter) + " (" + transHuman(*parsed.ter) + "); Expected " + + "apply " + locStr + ": Got " + transToken(*parsed.ter) + " (" + transHuman(*parsed.ter) + "); Expected " + transToken(*jt.ter) + " (" + transHuman(*jt.ter) + ")" + line)); using namespace std::string_literals; bad = (jt.rpcCode && !test.expect( parsed.rpcCode == jt.rpcCode->first && parsed.rpcMessage == jt.rpcCode->second, - "apply: Got RPC result "s + + "apply " + locStr + ": Got RPC result "s + (parsed.rpcCode ? RPC::get_error_info(*parsed.rpcCode).token.c_str() : "NO RESULT") + " (" + parsed.rpcMessage + "); Expected " + RPC::get_error_info(jt.rpcCode->first).token.c_str() + " (" + jt.rpcCode->second + ")" + line)) || @@ -419,13 +421,14 @@ Env::postconditions(JTx const& jt, ParsedResult const& parsed, Json::Value const // If we have an rpcCode (just checked), then the rpcException check is // optional - the 'error' field may not be defined, but if it is, it must // match rpcError. - bad = (jt.rpcException && - !test.expect( - (jt.rpcCode && parsed.rpcError.empty()) || - (parsed.rpcError == jt.rpcException->first && - (!jt.rpcException->second || parsed.rpcException == *jt.rpcException->second)), - "apply: Got RPC result "s + parsed.rpcError + " (" + parsed.rpcException + "); Expected " + - jt.rpcException->first + " (" + jt.rpcException->second.value_or("n/a") + ")" + line)) || + bad = + (jt.rpcException && + !test.expect( + (jt.rpcCode && parsed.rpcError.empty()) || + (parsed.rpcError == jt.rpcException->first && + (!jt.rpcException->second || parsed.rpcException == *jt.rpcException->second)), + "apply " + locStr + ": Got RPC result "s + parsed.rpcError + " (" + parsed.rpcException + "); Expected " + + jt.rpcException->first + " (" + jt.rpcException->second.value_or("n/a") + ")" + line)) || bad; if (bad) { diff --git a/src/test/rpc/Subscribe_test.cpp b/src/test/rpc/Subscribe_test.cpp index 4f83f81da5..cce45fb4ef 100644 --- a/src/test/rpc/Subscribe_test.cpp +++ b/src/test/rpc/Subscribe_test.cpp @@ -835,8 +835,7 @@ public: { auto& from = (i % 2 == 0) ? a : b; auto& to = (i % 2 == 0) ? b : a; - env.apply( - pay(from, to, jtx::XRP(numXRP)), + env(pay(from, to, jtx::XRP(numXRP)), jtx::seq(jtx::autofill), jtx::fee(jtx::autofill), jtx::sig(jtx::autofill)); From bf4674f42b0515f0fe81d4b523c98d68fd7269b3 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 6 Feb 2026 15:30:22 -0500 Subject: [PATCH 08/12] refactor: Fix spelling issues in tests (#6199) This change removes the `src/tests` exception from the `cspell` config and fixes all the issues that arise as a result. No functionality/test change. --- .../cspell.config.yaml => cspell.config.yaml | 35 +++++--- include/xrpl/proto/xrpl.proto | 6 +- src/test/app/AMMCalc_test.cpp | 30 +++---- src/test/app/AMMExtended_test.cpp | 8 +- src/test/app/AMM_test.cpp | 32 +++---- src/test/app/Batch_test.cpp | 2 +- src/test/app/CrossingLimits_test.cpp | 22 ++--- src/test/app/DepositAuth_test.cpp | 6 +- src/test/app/EscrowToken_test.cpp | 12 +-- src/test/app/FixNFTokenPageLinks_test.cpp | 2 +- src/test/app/LPTokenTransfer_test.cpp | 2 +- src/test/app/LoanBroker_test.cpp | 2 +- src/test/app/Loan_test.cpp | 1 + src/test/app/MPToken_test.cpp | 2 +- src/test/app/NFToken_test.cpp | 18 ++-- src/test/app/Offer_test.cpp | 86 +++++++++---------- src/test/app/PayStrand_test.cpp | 6 +- src/test/app/ReducedOffer_test.cpp | 36 ++++---- src/test/app/SHAMapStore_test.cpp | 6 +- src/test/app/TrustAndBalance_test.cpp | 10 +-- src/test/app/Vault_test.cpp | 2 +- src/test/app/XChain_test.cpp | 5 +- src/test/core/Config_test.cpp | 36 ++++---- src/test/jtx/TestHelpers.h | 2 +- src/test/jtx/impl/AMM.cpp | 42 ++++----- src/test/jtx/impl/TestHelpers.cpp | 2 +- src/test/jtx/impl/mpt.cpp | 28 +++--- src/test/jtx/mpt.h | 8 +- src/test/ledger/Directory_test.cpp | 2 +- src/test/ledger/View_test.cpp | 10 +-- src/test/overlay/compression_test.cpp | 10 +-- src/test/protocol/STAmount_test.cpp | 4 +- src/test/rpc/AccountLines_test.cpp | 2 +- src/test/rpc/AccountObjects_test.cpp | 4 +- src/test/rpc/Handler_test.cpp | 1 + src/test/rpc/LedgerEntry_test.cpp | 4 +- src/test/rpc/NoRipple_test.cpp | 4 +- src/test/rpc/Transaction_test.cpp | 4 +- 38 files changed, 256 insertions(+), 238 deletions(-) rename .config/cspell.config.yaml => cspell.config.yaml (79%) diff --git a/.config/cspell.config.yaml b/cspell.config.yaml similarity index 79% rename from .config/cspell.config.yaml rename to cspell.config.yaml index a9c621f567..b2f4a33769 100644 --- a/.config/cspell.config.yaml +++ b/cspell.config.yaml @@ -1,14 +1,13 @@ ignorePaths: - build/** - src/libxrpl/crypto - - src/test/** # Will be removed in the future - CMakeUserPresets.json - Doxyfile - docs/**/*.puml - cmake/** - LICENSE.md language: en -allowCompoundWords: true +allowCompoundWords: true # TODO (#6334) ignoreRandomStrings: true minWordLength: 5 dictionaries: @@ -16,20 +15,29 @@ dictionaries: - en_US - en_GB ignoreRegExpList: - - /[rs][1-9A-HJ-NP-Za-km-z]{25,34}/g # addresses and seeds - - /(XRPL|BEAST)_[A-Z_0-9]+_H_INCLUDED+/g # include guards - - /(XRPL|BEAST)_[A-Z_0-9]+_H+/g # include guards + - /\b[rs][1-9A-HJ-NP-Za-km-z]{25,34}/g # addresses and seeds + - /\bC[A-Z0-9]{15}/g # CTIDs + - /\b(XRPL|BEAST)_[A-Z_0-9]+_H_INCLUDED+/g # include guards + - /\b(XRPL|BEAST)_[A-Z_0-9]+_H+/g # include guards - /::[a-z:_]+/g # things from other namespaces - - /lib[a-z]+/g # libraries - - /[0-9]{4}-[0-9]{2}-[0-9]{2}[,:][A-Za-zÀ-ÖØ-öø-ÿ.\s]+/g # copyright dates - - /[0-9]{4}[,:]?\s*[A-Za-zÀ-ÖØ-öø-ÿ.\s]+/g # copyright years + - /\blib[a-z]+/g # libraries + - /\b[0-9]{4}-[0-9]{2}-[0-9]{2}[,:][A-Za-zÀ-ÖØ-öø-ÿ.\s]+/g # copyright dates + - /\b[0-9]{4}[,:]?\s*[A-Za-zÀ-ÖØ-öø-ÿ.\s]+/g # copyright years - /\[[A-Za-z0-9-]+\]\(https:\/\/github.com\/[A-Za-z0-9-]+\)/g # Github usernames - /-[DWw][a-zA-Z0-9_-]+=/g # compile flags - /[\['"`]-[DWw][a-zA-Z0-9_-]+['"`\]]/g # compile flags + - ABCDEFGHIJKLMNOPQRSTUVWXYZ + - ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz +overrides: + - filename: "**/*_test.cpp" # all test files + ignoreRegExpList: + - /"[^"]*"/g # double-quoted strings + - /'[^']*'/g # single-quoted strings + - /`[^`]*`/g # backtick strings suggestWords: - xprl->xrpl - - xprld->xrpld - - unsynched->unsynced + - xprld->xrpld # cspell: disable-line not sure what this problem is.... + - unsynched->unsynced # cspell: disable-line not sure what this problem is.... - synched->synced - synch->sync words: @@ -51,6 +59,7 @@ words: - Britto - Btrfs - canonicality + - changespq - checkme - choco - chrono @@ -106,12 +115,14 @@ words: - inequation - insuf - insuff + - invasively - iou - ious - isrdc - itype - jemalloc - jlog + - jtnofill - keylet - keylets - keyvadb @@ -138,6 +149,7 @@ words: - Metafuncton - misprediction - mptbalance + - MPTDEX - mptflags - mptid - mptissuance @@ -147,6 +159,7 @@ words: - mptokenissuance - mptokens - mpts + - mtgox - multisig - multisign - multisigned @@ -174,6 +187,7 @@ words: - perminute - permissioned - pointee + - populator - preauth - preauthorization - preauthorize @@ -182,6 +196,7 @@ words: - protobuf - protos - ptrs + - pushd - pyenv - qalloc - queuable diff --git a/include/xrpl/proto/xrpl.proto b/include/xrpl/proto/xrpl.proto index 613ef70a1f..0af7deb35d 100644 --- a/include/xrpl/proto/xrpl.proto +++ b/include/xrpl/proto/xrpl.proto @@ -86,9 +86,9 @@ message TMPublicKey { // you must first combine coins from one address to another. enum TransactionStatus { - tsNEW = 1; // origin node did/could not validate - tsCURRENT = 2; // scheduled to go in this ledger - tsCOMMITED = 3; // in a closed ledger + tsNEW = 1; // origin node did/could not validate + tsCURRENT = 2; // scheduled to go in this ledger + tsCOMMITTED = 3; // in a closed ledger tsREJECT_CONFLICT = 4; tsREJECT_INVALID = 5; tsREJECT_FUNDS = 6; diff --git a/src/test/app/AMMCalc_test.cpp b/src/test/app/AMMCalc_test.cpp index 7d735c2575..bc50f02d3d 100644 --- a/src/test/app/AMMCalc_test.cpp +++ b/src/test/app/AMMCalc_test.cpp @@ -23,8 +23,8 @@ class AMMCalc_test : public beast::unit_test::suite { using token_iter = boost::sregex_token_iterator; using steps = std::vector>; - using trates = std::map; - using swapargs = std::tuple; + using transfer_rates = std::map; + using swapargs = std::tuple; jtx::Account const gw{jtx::Account("gw")}; token_iter const end_; @@ -101,10 +101,10 @@ class AMMCalc_test : public beast::unit_test::suite return {{{*a1, *a2}, amm}}; } - std::optional + std::optional getTransferRate(token_iter& p) { - trates rates{}; + transfer_rates rates{}; if (p == end_) return rates; std::string str = *p; @@ -115,8 +115,8 @@ class AMMCalc_test : public beast::unit_test::suite { if (auto const rate = getRate(p++)) { - auto const [currency, trate, delimited] = *rate; - rates[currency] = trate; + auto const [currency, transferRate, delimited] = *rate; + rates[currency] = transferRate; if (delimited) break; } @@ -180,13 +180,13 @@ class AMMCalc_test : public beast::unit_test::suite auto const vp = std::get(args); STAmount sout = std::get(args); auto const fee = std::get(args); - auto const rates = std::get(args); + auto const rates = std::get(args); STAmount resultOut = sout; STAmount resultIn{}; STAmount sin{}; int limitingStep = vp.size(); STAmount limitStepOut{}; - auto trate = [&](auto const& amt) { + auto transferRate = [&](auto const& amt) { auto const currency = to_string(amt.issue().currency); return rates.find(currency) != rates.end() ? rates.at(currency) : QUALITY_ONE; }; @@ -194,7 +194,7 @@ class AMMCalc_test : public beast::unit_test::suite sin = sout; for (auto it = vp.rbegin(); it != vp.rend(); ++it) { - sout = mulratio(sin, trate(sin), QUALITY_ONE, true); + sout = mulratio(sin, transferRate(sin), QUALITY_ONE, true); auto const [amts, amm] = *it; // assume no amm limit if (amm) @@ -221,7 +221,7 @@ class AMMCalc_test : public beast::unit_test::suite for (int i = limitingStep + 1; i < vp.size(); ++i) { auto const [amts, amm] = vp[i]; - sin = mulratio(sin, QUALITY_ONE, trate(sin), false); + sin = mulratio(sin, QUALITY_ONE, transferRate(sin), false); if (amm) { sout = swapAssetIn(amts, sin, fee); @@ -243,13 +243,13 @@ class AMMCalc_test : public beast::unit_test::suite auto const vp = std::get(args); STAmount sin = std::get(args); auto const fee = std::get(args); - auto const rates = std::get(args); + auto const rates = std::get(args); STAmount resultIn = sin; STAmount resultOut{}; STAmount sout{}; int limitingStep = 0; STAmount limitStepIn{}; - auto trate = [&](auto const& amt) { + auto transferRate = [&](auto const& amt) { auto const currency = to_string(amt.issue().currency); return rates.find(currency) != rates.end() ? rates.at(currency) : QUALITY_ONE; }; @@ -257,7 +257,7 @@ class AMMCalc_test : public beast::unit_test::suite for (auto it = vp.begin(); it != vp.end(); ++it) { auto const [amts, amm] = *it; - sin = mulratio(sin, QUALITY_ONE, trate(sin), + sin = mulratio(sin, QUALITY_ONE, transferRate(sin), false); // out of the next step // assume no amm limit if (amm) @@ -283,7 +283,7 @@ class AMMCalc_test : public beast::unit_test::suite // swap out if limiting step for (int i = limitingStep - 1; i >= 0; --i) { - sout = mulratio(sin, trate(sin), QUALITY_ONE, false); + sout = mulratio(sin, transferRate(sin), QUALITY_ONE, false); auto const [amts, amm] = vp[i]; if (amm) { @@ -296,7 +296,7 @@ class AMMCalc_test : public beast::unit_test::suite } resultIn = sin; } - resultOut = mulratio(resultOut, QUALITY_ONE, trate(resultOut), true); + resultOut = mulratio(resultOut, QUALITY_ONE, transferRate(resultOut), true); std::cout << "in: " << toString(resultIn) << " out: " << toString(resultOut) << std::endl; } diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 3cc48787d5..4eecb05905 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -1701,8 +1701,8 @@ private: // This removes no ripple for carol, // different from the original test fund(env, gw, {carol}, XRP(10'000), {}, Fund::Acct); - auto const AMMXRPPool = env.current()->fees().increment * 2; - env.fund(reserve(env, 5) + ammCrtFee(env) + AMMXRPPool, bob); + auto const ammXrpPool = env.current()->fees().increment * 2; + env.fund(reserve(env, 5) + ammCrtFee(env) + ammXrpPool, bob); env.close(); env.trust(USD(1'000), alice, bob, carol); env.trust(EUR(1'000), alice, bob, carol); @@ -1718,7 +1718,7 @@ private: // tecPATH_DRY, but the entire path should not be marked as dry. // This is the second error case to test (when flowV1 is used). env(offer(bob, EUR(50), XRP(50))); - AMM ammBob(env, bob, AMMXRPPool, USD(150)); + AMM ammBob(env, bob, ammXrpPool, USD(150)); env(pay(alice, carol, USD(1'000'000)), path(~XRP, ~USD), @@ -3248,7 +3248,7 @@ private: Path const p = [&] { Path result; - result.push_back(allpe(gw, BobUSD)); + result.push_back(allPathElements(gw, BobUSD)); result.push_back(cpe(EUR.currency)); return result; }(); diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 14b97f30f1..8a60e5f667 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -975,7 +975,7 @@ private: .tokens = IOUAmount{1, -10}, .asset1In = STAmount{USD, 1, -15}, .err = ter(tecAMM_INVALID_TOKENS)}); }); - // Single deposit with eprice, tokens rounded to 0 + // Single deposit with EPrice, tokens rounded to 0 testAMM([&](AMM& amm, Env& env) { amm.deposit(DepositArg{ .asset1In = STAmount{USD, 1, -15}, .maxEP = STAmount{USD, 1, -1}, .err = ter(tecAMM_INVALID_TOKENS)}); @@ -4204,8 +4204,8 @@ private: Account const chris("chris"); Account const simon("simon"); Account const ben("ben"); - Account const nataly("nataly"); - fund(env, gw, {bob, ed, paul, dan, chris, simon, ben, nataly}, {USD(1'500'000)}, Fund::Acct); + Account const natalie("natalie"); + fund(env, gw, {bob, ed, paul, dan, chris, simon, ben, natalie}, {USD(1'500'000)}, Fund::Acct); for (int i = 0; i < 10; ++i) { ammAlice.deposit(ben, STAmount{USD, 1, -10}); @@ -4224,8 +4224,8 @@ private: ammAlice.withdrawAll(ed, USD(0)); ammAlice.deposit(paul, USD(100'000)); ammAlice.withdrawAll(paul, USD(0)); - ammAlice.deposit(nataly, USD(1'000'000)); - ammAlice.withdrawAll(nataly, USD(0)); + ammAlice.deposit(natalie, USD(1'000'000)); + ammAlice.withdrawAll(natalie, USD(0)); } // Due to round off some accounts have a tiny gain, while // other have a tiny loss. The last account to withdraw @@ -4251,11 +4251,11 @@ private: BEAST_EXPECT(expectHolding(env, ed, USD(1'500'000))); BEAST_EXPECT(expectHolding(env, paul, USD(1'500'000))); if (!features[fixAMMv1_1] && !features[fixAMMv1_3]) - BEAST_EXPECT(expectHolding(env, nataly, STAmount{USD, UINT64_C(1'500'000'000000002), -9})); + BEAST_EXPECT(expectHolding(env, natalie, STAmount{USD, UINT64_C(1'500'000'000000002), -9})); else if (features[fixAMMv1_1] && !features[fixAMMv1_3]) - BEAST_EXPECT(expectHolding(env, nataly, STAmount{USD, UINT64_C(1'500'000'000000005), -9})); + BEAST_EXPECT(expectHolding(env, natalie, STAmount{USD, UINT64_C(1'500'000'000000005), -9})); else - BEAST_EXPECT(expectHolding(env, nataly, USD(1'500'000))); + BEAST_EXPECT(expectHolding(env, natalie, USD(1'500'000))); ammAlice.withdrawAll(alice); BEAST_EXPECT(!ammAlice.ammExists()); if (!features[fixAMMv1_1]) @@ -4264,7 +4264,7 @@ private: BEAST_EXPECT(expectHolding(env, alice, STAmount{USD, UINT64_C(30'000'0000000003), -10})); else BEAST_EXPECT(expectHolding(env, alice, USD(30'000))); - // alice XRP balance is 30,000initial - 50 ammcreate fee - + // alice XRP balance is 30,000 initial - 50 AMMCreate fee - // 10drops fee BEAST_EXPECT( accountBalance(env, alice) == std::to_string(29950000000 - env.current()->fees().base.drops())); @@ -4284,8 +4284,8 @@ private: Account const chris("chris"); Account const simon("simon"); Account const ben("ben"); - Account const nataly("nataly"); - fund(env, gw, {bob, ed, paul, dan, chris, simon, ben, nataly}, XRP(2'000'000), {}, Fund::Acct); + Account const natalie("natalie"); + fund(env, gw, {bob, ed, paul, dan, chris, simon, ben, natalie}, XRP(2'000'000), {}, Fund::Acct); for (int i = 0; i < 10; ++i) { ammAlice.deposit(ben, XRPAmount{1}); @@ -4304,8 +4304,8 @@ private: ammAlice.withdrawAll(ed, XRP(0)); ammAlice.deposit(paul, XRP(100'000)); ammAlice.withdrawAll(paul, XRP(0)); - ammAlice.deposit(nataly, XRP(1'000'000)); - ammAlice.withdrawAll(nataly, XRP(0)); + ammAlice.deposit(natalie, XRP(1'000'000)); + ammAlice.withdrawAll(natalie, XRP(0)); } auto const baseFee = env.current()->fees().base.drops(); if (!features[fixAMMv1_3]) @@ -4325,8 +4325,8 @@ private: BEAST_EXPECT(accountBalance(env, carol) == std::to_string(30'000'000'000 - 20 * baseFee)); BEAST_EXPECT(accountBalance(env, ed) == xrpBalance); BEAST_EXPECT(accountBalance(env, paul) == xrpBalance); - BEAST_EXPECT(accountBalance(env, nataly) == xrpBalance); - // 30,000 initial - 50 ammcreate fee - 10drops withdraw fee + BEAST_EXPECT(accountBalance(env, natalie) == xrpBalance); + // 30,000 initial - 50 AMMCreate fee - 10drops withdraw fee BEAST_EXPECT(accountBalance(env, alice) == std::to_string(29'950'000'000 - baseFee)); } else @@ -4346,7 +4346,7 @@ private: BEAST_EXPECT(accountBalance(env, carol) == std::to_string(30'000'000'000 - 20 * baseFee - 10)); BEAST_EXPECT(accountBalance(env, ed) == (xrpBalance + drops(2)).getText()); BEAST_EXPECT(accountBalance(env, paul) == (xrpBalance + drops(3)).getText()); - BEAST_EXPECT(accountBalance(env, nataly) == (xrpBalance + drops(5)).getText()); + BEAST_EXPECT(accountBalance(env, natalie) == (xrpBalance + drops(5)).getText()); BEAST_EXPECT(accountBalance(env, alice) == std::to_string(29'950'000'000 - baseFee + 80)); } }, diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index 035a1f8a6f..72f3677e3b 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -3618,7 +3618,7 @@ class Batch_test : public beast::unit_test::suite // another transaction is part of the batch, the batch might fail // because the sequence is out of order. This is because the canonical // order of transactions is determined by the account first. So in this - // case, alice's batch comes after bobs self submitted transaction even + // case, alice's batch comes after bob's self submitted transaction even // though the payment was submitted after the batch. using namespace test::jtx; diff --git a/src/test/app/CrossingLimits_test.cpp b/src/test/app/CrossingLimits_test.cpp index e1974adee1..0451822492 100644 --- a/src/test/app/CrossingLimits_test.cpp +++ b/src/test/app/CrossingLimits_test.cpp @@ -64,15 +64,15 @@ public: int const maxConsumed = 1000; env.fund(XRP(100000000), gw, "alice", "bob", "carol"); - int const bobsOfferCount = maxConsumed + 150; - env.trust(USD(bobsOfferCount), "bob"); - env(pay(gw, "bob", USD(bobsOfferCount))); + int const bobOfferCount = maxConsumed + 150; + env.trust(USD(bobOfferCount), "bob"); + env(pay(gw, "bob", USD(bobOfferCount))); env.close(); - n_offers(env, bobsOfferCount, "bob", XRP(1), USD(1)); + n_offers(env, bobOfferCount, "bob", XRP(1), USD(1)); // Alice offers to buy Bob's offers. However she hits the offer // crossing limit, so she can't buy them all at once. - env(offer("alice", USD(bobsOfferCount), XRP(bobsOfferCount))); + env(offer("alice", USD(bobOfferCount), XRP(bobOfferCount))); env.close(); env.require(balance("alice", USD(maxConsumed))); env.require(balance("bob", USD(150))); @@ -103,19 +103,19 @@ public: // The payment engine allows 1000 offers to cross. int const maxConsumed = 1000; - int const evitasOfferCount{maxConsumed + 49}; + int const evitaOfferCount{maxConsumed + 49}; env.trust(USD(1000), "alice"); env(pay(gw, "alice", USD(1000))); env.trust(USD(1000), "carol"); env(pay(gw, "carol", USD(1))); - env.trust(USD(evitasOfferCount + 1), "evita"); - env(pay(gw, "evita", USD(evitasOfferCount + 1))); + env.trust(USD(evitaOfferCount + 1), "evita"); + env(pay(gw, "evita", USD(evitaOfferCount + 1))); // The payment engine has a limit of 1000 funded or unfunded offers. int const carolsOfferCount{700}; n_offers(env, 400, "alice", XRP(1), USD(1)); n_offers(env, carolsOfferCount, "carol", XRP(1), USD(1)); - n_offers(env, evitasOfferCount, "evita", XRP(1), USD(1)); + n_offers(env, evitaOfferCount, "evita", XRP(1), USD(1)); // Bob offers to buy 1000 XRP for 1000 USD. He takes all 400 USD from // Alice's offers, 1 USD from Carol's and then removes 599 of Carol's @@ -126,8 +126,8 @@ public: env.require(owners("alice", 1)); env.require(balance("carol", USD(0))); env.require(owners("carol", carolsOfferCount - 599)); - env.require(balance("evita", USD(evitasOfferCount + 1))); - env.require(owners("evita", evitasOfferCount + 1)); + env.require(balance("evita", USD(evitaOfferCount + 1))); + env.require(owners("evita", evitaOfferCount + 1)); // Dan offers to buy maxConsumed + 50 XRP USD. He removes all of // Carol's remaining offers as unfunded, then takes diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index 535abc6af8..0cd7e7449c 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -1237,17 +1237,17 @@ struct DepositPreauth_test : public beast::unit_test::suite auto const dp = ledgerEntryDepositPreauth(env, stock, credentials); auto const& authCred(dp[jss::result][jss::node]["AuthorizeCredentials"]); BEAST_EXPECT(authCred.isArray() && authCred.size() == credentials.size()); - std::vector> readedCreds; + std::vector> readCreds; for (auto const& o : authCred) { auto const& c(o[jss::Credential]); auto issuer = c[jss::Issuer].asString(); if (BEAST_EXPECT(pubKey2Acc.contains(issuer))) - readedCreds.emplace_back(pubKey2Acc.at(issuer), c["CredentialType"].asString()); + readCreds.emplace_back(pubKey2Acc.at(issuer), c["CredentialType"].asString()); } - BEAST_EXPECT(std::ranges::is_sorted(readedCreds)); + BEAST_EXPECT(std::ranges::is_sorted(readCreds)); env(deposit::unauthCredentials(stock, credentials)); env.close(); diff --git a/src/test/app/EscrowToken_test.cpp b/src/test/app/EscrowToken_test.cpp index b5088f247a..1f3b3bab70 100644 --- a/src/test/app/EscrowToken_test.cpp +++ b/src/test/app/EscrowToken_test.cpp @@ -752,7 +752,7 @@ struct EscrowToken_test : public beast::unit_test::suite env.trust(USD(1), bob); env.close(); - // alice cannot finish because bobs limit is too low + // alice cannot finish because bob's limit is too low env(escrow::finish(alice, alice, seq1), escrow::condition(escrow::cb1), escrow::fulfillment(escrow::fb1), @@ -789,7 +789,7 @@ struct EscrowToken_test : public beast::unit_test::suite env.trust(USD(1), bob); env.close(); - // bob can finish even if bobs limit is too low + // bob can finish even if bob's limit is too low auto const bobPreLimit = env.limit(bob, USD); env(escrow::finish(bob, alice, seq1), @@ -799,7 +799,7 @@ struct EscrowToken_test : public beast::unit_test::suite ter(tesSUCCESS)); env.close(); - // bobs limit is not changed + // bob's limit is not changed BEAST_EXPECT(env.limit(bob, USD) == bobPreLimit); } } @@ -1606,7 +1606,7 @@ struct EscrowToken_test : public beast::unit_test::suite fee(baseFee * 150)); env.close(); auto const postBobLimit = env.limit(bob, USD); - // bobs limit is NOT changed + // bob's limit is NOT changed BEAST_EXPECT(postBobLimit == preBobLimit); } } @@ -1912,7 +1912,7 @@ struct EscrowToken_test : public beast::unit_test::suite } } void - testIOUINSF(FeatureBitset features) + testIOUInsufficientFunds(FeatureBitset features) { testcase("IOU Insufficient Funds"); using namespace test::jtx; @@ -3671,7 +3671,7 @@ struct EscrowToken_test : public beast::unit_test::suite testIOULimitAmount(features); testIOURequireAuth(features); testIOUFreeze(features); - testIOUINSF(features); + testIOUInsufficientFunds(features); testIOUPrecisionLoss(features); } diff --git a/src/test/app/FixNFTokenPageLinks_test.cpp b/src/test/app/FixNFTokenPageLinks_test.cpp index 8a3b51bbf1..88f368e9cf 100644 --- a/src/test/app/FixNFTokenPageLinks_test.cpp +++ b/src/test/app/FixNFTokenPageLinks_test.cpp @@ -442,7 +442,7 @@ class FixNFTokenPageLinks_test : public beast::unit_test::suite env(ledgerStateFix::nftPageLinks(daria, alice), fee(linkFixFee)); env.close(); - // alices's last page should now be present and include no links. + // alice's last page should now be present and include no links. { auto aliceLastNFTokenPage = env.le(keylet::nftpage_max(alice)); if (!BEAST_EXPECT(aliceLastNFTokenPage)) diff --git a/src/test/app/LPTokenTransfer_test.cpp b/src/test/app/LPTokenTransfer_test.cpp index f9cfc4d5ce..055b72ebe6 100644 --- a/src/test/app/LPTokenTransfer_test.cpp +++ b/src/test/app/LPTokenTransfer_test.cpp @@ -355,7 +355,7 @@ class LPTokenTransfer_test : public jtx::AMMTest env(token::acceptSellOffer(carol, sellOfferIndex)); env.close(); - // gateway freezes bobs's USD + // gateway freezes bob's USD env(trust(gw, bob["USD"](0), tfSetFreeze)); env.close(); diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 139350a881..58052e3fb1 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -905,7 +905,7 @@ class LoanBroker_test : public beast::unit_test::suite if (asset.holds()) { - // preclaim: AllowTrustLineClaback is not set + // preclaim: AllowTrustLineClawback is not set env(coverClawback(issuer), loanBrokerID(brokerKeylet.key), amount(vaultInfo.asset(2)), diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index f02ceddb69..5deb1f179c 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -10,6 +10,7 @@ #include #include +// cspell: words LOANTODO #include diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index f8d736c257..965dd86475 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -670,7 +670,7 @@ class MPToken_test : public beast::unit_test::suite mptAlice.set({.account = alice, .holder = bob, .flags = tfMPTLock}); if (!features[featureSingleAssetVault]) { - // Delete bobs' mptoken even though it is locked + // Delete bob's mptoken even though it is locked mptAlice.authorize({.account = bob, .flags = tfMPTUnauthorize}); mptAlice.set({.account = alice, .holder = bob, .flags = tfMPTUnlock, .err = tecOBJECT_NOT_FOUND}); diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 2ab2e2a94c..1b9ce82d4a 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -1383,28 +1383,28 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // Set flagOnlyXRP and offers using IOUs are rejected. { - uint256 const nftOnlyXRPID{token::getNextID(env, alice, 0u, tfOnlyXRP | tfTransferable)}; + uint256 const nftOnlyXrpID{token::getNextID(env, alice, 0u, tfOnlyXRP | tfTransferable)}; env(token::mint(alice, 0u), txflags(tfOnlyXRP | tfTransferable)); env.close(); BEAST_EXPECT(ownerCount(env, alice) == 2); - env(token::createOffer(alice, nftOnlyXRPID, gwAUD(50)), txflags(tfSellNFToken), ter(temBAD_AMOUNT)); + env(token::createOffer(alice, nftOnlyXrpID, gwAUD(50)), txflags(tfSellNFToken), ter(temBAD_AMOUNT)); env.close(); BEAST_EXPECT(ownerCount(env, alice) == 2); BEAST_EXPECT(ownerCount(env, buyer) == 1); - env(token::createOffer(buyer, nftOnlyXRPID, gwAUD(50)), token::owner(alice), ter(temBAD_AMOUNT)); + env(token::createOffer(buyer, nftOnlyXrpID, gwAUD(50)), token::owner(alice), ter(temBAD_AMOUNT)); env.close(); BEAST_EXPECT(ownerCount(env, buyer) == 1); // However offers for XRP are okay. BEAST_EXPECT(ownerCount(env, alice) == 2); - env(token::createOffer(alice, nftOnlyXRPID, XRP(60)), txflags(tfSellNFToken)); + env(token::createOffer(alice, nftOnlyXrpID, XRP(60)), txflags(tfSellNFToken)); env.close(); BEAST_EXPECT(ownerCount(env, alice) == 3); BEAST_EXPECT(ownerCount(env, buyer) == 1); - env(token::createOffer(buyer, nftOnlyXRPID, XRP(60)), token::owner(alice)); + env(token::createOffer(buyer, nftOnlyXrpID, XRP(60)), token::owner(alice)); env.close(); BEAST_EXPECT(ownerCount(env, buyer) == 2); } @@ -5155,8 +5155,8 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // bob now has a buy offer and a sell offer on the books. A broker // spots this and swoops in to make a profit. BEAST_EXPECT(nftCount(env, bob) == 1); - auto const bobsPriorBalance = env.balance(bob); - auto const brokersPriorBalance = env.balance(broker); + auto const bobPriorBalance = env.balance(bob); + auto const brokerPriorBalance = env.balance(broker); env(token::brokerOffers(broker, bobBuyOfferIndex, bobSellOfferIndex), token::brokerFee(XRP(1)), ter(tecCANT_ACCEPT_OWN_NFTOKEN_OFFER)); @@ -5165,8 +5165,8 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // A tec result was returned, so no state should change other // than the broker burning their transaction fee. BEAST_EXPECT(nftCount(env, bob) == 1); - BEAST_EXPECT(env.balance(bob) == bobsPriorBalance); - BEAST_EXPECT(env.balance(broker) == brokersPriorBalance - baseFee); + BEAST_EXPECT(env.balance(bob) == bobPriorBalance); + BEAST_EXPECT(env.balance(broker) == brokerPriorBalance - baseFee); } void diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 09d7e41141..4847b7edb0 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -1878,14 +1878,14 @@ public: BEAST_EXPECT(jrr[jss::node][sfBalance.fieldName][jss::value] == "49.96666666666667"); jrr = ledgerEntryState(env, bob, gw, "USD"); - Json::Value const bobsUSD = jrr[jss::node][sfBalance.fieldName][jss::value]; + Json::Value const bobUSD = jrr[jss::node][sfBalance.fieldName][jss::value]; if (!NumberSwitchOver) { - BEAST_EXPECT(bobsUSD == "-0.966500000033334"); + BEAST_EXPECT(bobUSD == "-0.966500000033334"); } else { - BEAST_EXPECT(bobsUSD == "-0.9665000000333333"); + BEAST_EXPECT(bobUSD == "-0.9665000000333333"); } } } @@ -2265,8 +2265,8 @@ public: // The scenario: // o alice has USD but wants XRP. // o bob has XRP but wants USD. - auto const alicesXRP = env.balance(alice); - auto const bobsXRP = env.balance(bob); + auto const aliceXRP = env.balance(alice); + auto const bobXRP = env.balance(bob); env(offer(alice, xrpOffer, usdOffer)); env.close(); @@ -2276,8 +2276,8 @@ public: env.require( balance(alice, USD(0)), balance(bob, usdOffer), - balance(alice, alicesXRP + xrpOffer - fee), - balance(bob, bobsXRP - xrpOffer - fee), + balance(alice, aliceXRP + xrpOffer - fee), + balance(bob, bobXRP - xrpOffer - fee), offers(alice, 0), offers(bob, 0)); @@ -2293,13 +2293,13 @@ public: env.require(offers(alice, 0)); verifyDefaultTrustline(env, bob, USD(1)); { - auto const bobsOffers = offersOnAccount(env, bob); - BEAST_EXPECT(bobsOffers.size() == 1); - auto const& bobsOffer = *(bobsOffers.front()); + auto const bobOffers = offersOnAccount(env, bob); + BEAST_EXPECT(bobOffers.size() == 1); + auto const& bobOffer = *(bobOffers.front()); - BEAST_EXPECT(bobsOffer[sfLedgerEntryType] == ltOFFER); - BEAST_EXPECT(bobsOffer[sfTakerGets] == USD(1)); - BEAST_EXPECT(bobsOffer[sfTakerPays] == XRP(1)); + BEAST_EXPECT(bobOffer[sfLedgerEntryType] == ltOFFER); + BEAST_EXPECT(bobOffer[sfTakerGets] == USD(1)); + BEAST_EXPECT(bobOffer[sfTakerPays] == XRP(1)); } } @@ -2375,13 +2375,13 @@ public: env.require(balance(bob, EUR(999))); { - auto bobsOffers = offersOnAccount(env, bob); - if (BEAST_EXPECT(bobsOffers.size() == 1)) + auto bobOffers = offersOnAccount(env, bob); + if (BEAST_EXPECT(bobOffers.size() == 1)) { - auto const& bobsOffer = *(bobsOffers.front()); + auto const& bobOffer = *(bobOffers.front()); - BEAST_EXPECT(bobsOffer[sfTakerGets] == USD(1)); - BEAST_EXPECT(bobsOffer[sfTakerPays] == EUR(1)); + BEAST_EXPECT(bobOffer[sfTakerGets] == USD(1)); + BEAST_EXPECT(bobOffer[sfTakerPays] == EUR(1)); } } @@ -2470,22 +2470,22 @@ public: verifyDefaultTrustline(env, bob, EUR(400)); verifyDefaultTrustline(env, carol, USD(400)); { - auto const alicesOffers = offersOnAccount(env, alice); - BEAST_EXPECT(alicesOffers.size() == 1); - auto const& alicesOffer = *(alicesOffers.front()); + auto const aliceOffers = offersOnAccount(env, alice); + BEAST_EXPECT(aliceOffers.size() == 1); + auto const& aliceOffer = *(aliceOffers.front()); - BEAST_EXPECT(alicesOffer[sfLedgerEntryType] == ltOFFER); - BEAST_EXPECT(alicesOffer[sfTakerGets] == USD(600)); - BEAST_EXPECT(alicesOffer[sfTakerPays] == XRP(600)); + BEAST_EXPECT(aliceOffer[sfLedgerEntryType] == ltOFFER); + BEAST_EXPECT(aliceOffer[sfTakerGets] == USD(600)); + BEAST_EXPECT(aliceOffer[sfTakerPays] == XRP(600)); } { - auto const bobsOffers = offersOnAccount(env, bob); - BEAST_EXPECT(bobsOffers.size() == 1); - auto const& bobsOffer = *(bobsOffers.front()); + auto const bobOffers = offersOnAccount(env, bob); + BEAST_EXPECT(bobOffers.size() == 1); + auto const& bobOffer = *(bobOffers.front()); - BEAST_EXPECT(bobsOffer[sfLedgerEntryType] == ltOFFER); - BEAST_EXPECT(bobsOffer[sfTakerGets] == XRP(600)); - BEAST_EXPECT(bobsOffer[sfTakerPays] == EUR(600)); + BEAST_EXPECT(bobOffer[sfLedgerEntryType] == ltOFFER); + BEAST_EXPECT(bobOffer[sfTakerGets] == XRP(600)); + BEAST_EXPECT(bobOffer[sfTakerPays] == EUR(600)); } // carol makes an offer that exactly consumes alice and bob's offers. @@ -2503,15 +2503,15 @@ public: verifyDefaultTrustline(env, carol, USD(1000)); // In pre-flow code alice's offer is left empty in the ledger. - auto const alicesOffers = offersOnAccount(env, alice); - if (alicesOffers.size() != 0) + auto const aliceOffers = offersOnAccount(env, alice); + if (aliceOffers.size() != 0) { - BEAST_EXPECT(alicesOffers.size() == 1); - auto const& alicesOffer = *(alicesOffers.front()); + BEAST_EXPECT(aliceOffers.size() == 1); + auto const& aliceOffer = *(aliceOffers.front()); - BEAST_EXPECT(alicesOffer[sfLedgerEntryType] == ltOFFER); - BEAST_EXPECT(alicesOffer[sfTakerGets] == USD(0)); - BEAST_EXPECT(alicesOffer[sfTakerPays] == XRP(0)); + BEAST_EXPECT(aliceOffer[sfLedgerEntryType] == ltOFFER); + BEAST_EXPECT(aliceOffer[sfTakerGets] == USD(0)); + BEAST_EXPECT(aliceOffer[sfTakerPays] == XRP(0)); } } @@ -3679,16 +3679,16 @@ public: // Place alice's tiny offer in the book first. Let's see what happens // when a reasonable offer crosses it. - STAmount const alicesCnyOffer{ + STAmount const aliceCnyOffer{ CNY.issue(), std::uint64_t(4926000000000000), -23}; - env(offer(alice, alicesCnyOffer, drops(1), tfPassive)); + env(offer(alice, aliceCnyOffer, drops(1), tfPassive)); env.close(); // bob places an ordinary offer - STAmount const bobsCnyStartBalance{ + STAmount const bobCnyStartBalance{ CNY.issue(), std::uint64_t(3767479960090235), -15}; - env(pay(gw, bob, bobsCnyStartBalance)); + env(pay(gw, bob, bobCnyStartBalance)); env.close(); env(offer( @@ -3697,9 +3697,9 @@ public: STAmount{CNY.issue(), std::uint64_t(1000000000000000), -20})); env.close(); - env.require(balance(alice, alicesCnyOffer)); + env.require(balance(alice, aliceCnyOffer)); env.require(balance(alice, startXrpBalance - fee - drops(1))); - env.require(balance(bob, bobsCnyStartBalance - alicesCnyOffer)); + env.require(balance(bob, bobCnyStartBalance - aliceCnyOffer)); env.require(balance(bob, startXrpBalance - (fee * 2) + drops(1))); } diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 2f3b36eae5..9b534d9284 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -73,11 +73,11 @@ equal(std::unique_ptr const& s1, DirectStepInfo const& dsi) } bool -equal(std::unique_ptr const& s1, XRPEndpointStepInfo const& xrpsi) +equal(std::unique_ptr const& s1, XRPEndpointStepInfo const& xrpStepInfo) { if (!s1) return false; - return test::xrpEndpointStepEqual(*s1, xrpsi.acc); + return test::xrpEndpointStepEqual(*s1, xrpStepInfo.acc); } bool @@ -970,7 +970,7 @@ struct PayStrand_test : public beast::unit_test::suite Path const p = [&] { Path result; - result.push_back(allpe(gw, bob["USD"])); + result.push_back(allPathElements(gw, bob["USD"])); result.push_back(cpe(EUR.currency)); return result; }(); diff --git a/src/test/app/ReducedOffer_test.cpp b/src/test/app/ReducedOffer_test.cpp index aa1473d2ee..b967096eb8 100644 --- a/src/test/app/ReducedOffer_test.cpp +++ b/src/test/app/ReducedOffer_test.cpp @@ -79,8 +79,8 @@ public: STAmount const initialRate = Quality(newOffer).rate(); std::uint32_t const bobOfferSeq = env.seq(bob); STAmount const bobInitialBalance = env.balance(bob); - STAmount const bobsFee = env.current()->fees().base; - env(offer(bob, newOffer.in, newOffer.out, tfSell), fee(bobsFee)); + STAmount const bobFee = env.current()->fees().base; + env(offer(bob, newOffer.in, newOffer.out, tfSell), fee(bobFee)); env.close(); STAmount const bobFinalBalance = env.balance(bob); @@ -100,7 +100,7 @@ public: amountFromJson(sfTakerGets, bobOffer[jss::node][sfTakerGets.jsonName]); STAmount const reducedTakerPays = amountFromJson(sfTakerPays, bobOffer[jss::node][sfTakerPays.jsonName]); - STAmount const bobGot = env.balance(bob) + bobsFee - bobInitialBalance; + STAmount const bobGot = env.balance(bob) + bobFee - bobInitialBalance; BEAST_EXPECT(reducedTakerPays < newOffer.in); BEAST_EXPECT(reducedTakerGets < newOffer.out); STAmount const inLedgerRate = Quality(Amounts{reducedTakerPays, reducedTakerGets}).rate(); @@ -141,7 +141,7 @@ public: }; // bob's offer (the new offer) is the same every time: - Amounts const bobsOffer{STAmount(XRP(1)), STAmount(USD.issue(), 1, 0)}; + Amounts const bobOffer{STAmount(XRP(1)), STAmount(USD.issue(), 1, 0)}; // alice's offer has a slightly smaller TakerPays with each // iteration. This should mean that the size of the offer bob @@ -151,10 +151,10 @@ public: mantissaReduce += 20'000'000ull) { STAmount aliceUSD{ - bobsOffer.out.issue(), bobsOffer.out.mantissa() - mantissaReduce, bobsOffer.out.exponent()}; - STAmount aliceXRP{bobsOffer.in.issue(), bobsOffer.in.mantissa() - 1}; - Amounts alicesOffer{aliceUSD, aliceXRP}; - blockedCount += exerciseOfferPair(alicesOffer, bobsOffer); + bobOffer.out.issue(), bobOffer.out.mantissa() - mantissaReduce, bobOffer.out.exponent()}; + STAmount aliceXRP{bobOffer.in.issue(), bobOffer.in.mantissa() - 1}; + Amounts aliceOffer{aliceUSD, aliceXRP}; + blockedCount += exerciseOfferPair(aliceOffer, bobOffer); } // None of the test cases should produce a potentially blocking @@ -279,9 +279,9 @@ public: STAmount bobUSD{ aliceOffer.out.issue(), aliceOffer.out.mantissa() - mantissaReduce, aliceOffer.out.exponent()}; STAmount bobXRP{aliceOffer.in.issue(), aliceOffer.in.mantissa() - 1}; - Amounts bobsOffer{bobUSD, bobXRP}; + Amounts bobOffer{bobUSD, bobXRP}; - blockedCount += exerciseOfferPair(aliceOffer, bobsOffer); + blockedCount += exerciseOfferPair(aliceOffer, bobOffer); } // None of the test cases should produce a potentially blocking @@ -333,7 +333,7 @@ public: // then we use that as evidence that bob's offer blocked the // order book. { - bool const bobsOfferGone = !offerInLedger(env, bob, bobOfferSeq); + bool const bobOfferGone = !offerInLedger(env, bob, bobOfferSeq); STAmount const aliceBalanceUSD = env.balance(alice, USD); // Sanity check the ledger if alice got USD. @@ -341,11 +341,11 @@ public: { BEAST_EXPECT(aliceBalanceUSD == initialBobUSD); BEAST_EXPECT(env.balance(bob, USD) == USD(0)); - BEAST_EXPECT(bobsOfferGone); + BEAST_EXPECT(bobOfferGone); } // Track occurrences of order book blocking. - if (!bobsOfferGone && aliceBalanceUSD.signum() == 0) + if (!bobOfferGone && aliceBalanceUSD.signum() == 0) { ++blockedOrderBookCount; } @@ -423,13 +423,13 @@ public: // Examine the aftermath of alice's offer. { - bool const bobsOfferGone = !offerInLedger(env, bob, bobOfferSeq); + bool const bobOfferGone = !offerInLedger(env, bob, bobOfferSeq); STAmount aliceBalanceUSD = env.balance(alice, USD); #if 0 std::cout - << "bobs initial: " << initialBobUSD + << "bob initial: " << initialBobUSD << "; alice final: " << aliceBalanceUSD - << "; bobs offer: " << bobsOfferJson.toStyledString() + << "; bob offer: " << bobOfferJson.toStyledString() << std::endl; #endif // Sanity check the ledger if alice got USD. @@ -437,11 +437,11 @@ public: { BEAST_EXPECT(aliceBalanceUSD == initialBobUSD); BEAST_EXPECT(env.balance(bob, USD) == USD(0)); - BEAST_EXPECT(bobsOfferGone); + BEAST_EXPECT(bobOfferGone); } // Track occurrences of order book blocking. - if (!bobsOfferGone && aliceBalanceUSD.signum() == 0) + if (!bobOfferGone && aliceBalanceUSD.signum() == 0) { ++blockedOrderBookCount; } diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index 7951da26d1..c671d6fc27 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -44,10 +44,10 @@ class SHAMapStore_test : public beast::unit_test::suite auto const seq = json[jss::result][jss::ledger_index].asUInt(); - std::optional oinfo = env.app().getRelationalDatabase().getLedgerInfoByIndex(seq); - if (!oinfo) + std::optional outInfo = env.app().getRelationalDatabase().getLedgerInfoByIndex(seq); + if (!outInfo) return false; - LedgerHeader const& info = oinfo.value(); + LedgerHeader const& info = outInfo.value(); std::string const outHash = to_string(info.hash); LedgerIndex const outSeq = info.seq; diff --git a/src/test/app/TrustAndBalance_test.cpp b/src/test/app/TrustAndBalance_test.cpp index b695430c7f..aa06ccbdf1 100644 --- a/src/test/app/TrustAndBalance_test.cpp +++ b/src/test/app/TrustAndBalance_test.cpp @@ -383,20 +383,20 @@ class TrustAndBalance_test : public beast::unit_test::suite jvs[jss::streams].append("transactions"); BEAST_EXPECT(wsc->invoke("subscribe", jvs)[jss::status] == "success"); - char const* invoiceid = "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89"; + char const* invoiceId = "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89"; Json::Value jv; - auto tx = env.jt(pay(env.master, alice, XRP(10000)), json(sfInvoiceID.fieldName, invoiceid)); + auto tx = env.jt(pay(env.master, alice, XRP(10000)), json(sfInvoiceID.fieldName, invoiceId)); jv[jss::tx_blob] = strHex(tx.stx->getSerializer().slice()); auto jrr = wsc->invoke("submit", jv)[jss::result]; BEAST_EXPECT(jrr[jss::status] == "success"); - BEAST_EXPECT(jrr[jss::tx_json][sfInvoiceID.fieldName] == invoiceid); + BEAST_EXPECT(jrr[jss::tx_json][sfInvoiceID.fieldName] == invoiceId); env.close(); using namespace std::chrono_literals; - BEAST_EXPECT(wsc->findMsg(2s, [invoiceid](auto const& jval) { + BEAST_EXPECT(wsc->findMsg(2s, [invoiceId](auto const& jval) { auto const& t = jval[jss::transaction]; - return t[jss::TransactionType] == jss::Payment && t[sfInvoiceID.fieldName] == invoiceid; + return t[jss::TransactionType] == jss::Payment && t[sfInvoiceID.fieldName] == invoiceId; })); BEAST_EXPECT(wsc->invoke("unsubscribe", jv)[jss::status] == "success"); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index e39f665711..5d31c674c0 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -4578,7 +4578,7 @@ class Vault_test : public beast::unit_test::suite { testcase("VaultClawback (asset) - " + prefix + " issuer XRP clawback fails"); auto [vault, vaultKeylet] = setupVault(asset, owner, depositor, issuer); - // If the asset is XRP, clawback with amount fails as malfored + // If the asset is XRP, clawback with amount fails as malformed // when asset is specified. env(vault.clawback({ .issuer = issuer, diff --git a/src/test/app/XChain_test.cpp b/src/test/app/XChain_test.cpp index e074e03d97..2921e10ae4 100644 --- a/src/test/app/XChain_test.cpp +++ b/src/test/app/XChain_test.cpp @@ -2149,7 +2149,7 @@ struct XChain_test : public beast::unit_test::suite, public jtx::XChainBridgeObj } void - testXChainAddAccountCreateNonBatchAttestation() + testXChainAddAccountCreateNonBatchAttestation() // cspell: disable-line { using namespace jtx; @@ -3417,7 +3417,7 @@ struct XChain_test : public beast::unit_test::suite, public jtx::XChainBridgeObj testXChainCommit(); testXChainAddAttestation(); testXChainAddClaimNonBatchAttestation(); - testXChainAddAccountCreateNonBatchAttestation(); + testXChainAddAccountCreateNonBatchAttestation(); // cspell: disable-line testXChainClaim(); testXChainCreateAccount(); testFeeDipsIntoReserve(); @@ -3535,6 +3535,7 @@ private: do { callback_called = false; + // cspell: ignore attns for (size_t i = 0; i < signers_attns.size(); ++i) { for (auto& [bridge, claims] : signers_attns[i]) diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index 1035be6fc0..d5f3bb556c 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -634,8 +634,8 @@ nHBu9PTL9dn2GuZtdW4U2WzBwffyX9qsQCd9CNU4Z5YG3PQfViM8 Config c; std::string toLoad(R"xrpldConfig( [validator_list_sites] -xrplvalidators.com -trustthesevalidators.gov +xrpl-validators.com +trust-these-validators.gov [validator_list_keys] 021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566 @@ -645,8 +645,8 @@ trustthesevalidators.gov )xrpldConfig"); c.loadFromString(toLoad); BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 2); - BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values()[0] == "xrplvalidators.com"); - BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values()[1] == "trustthesevalidators.gov"); + BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values()[0] == "xrpl-validators.com"); + BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values()[1] == "trust-these-validators.gov"); BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 1); BEAST_EXPECT( c.section(SECTION_VALIDATOR_LIST_KEYS).values()[0] == @@ -661,8 +661,8 @@ trustthesevalidators.gov Config c; std::string toLoad(R"xrpldConfig( [validator_list_sites] -xrplvalidators.com -trustthesevalidators.gov +xrpl-validators.com +trust-these-validators.gov [validator_list_keys] 021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566 @@ -672,8 +672,8 @@ trustthesevalidators.gov )xrpldConfig"); c.loadFromString(toLoad); BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 2); - BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values()[0] == "xrplvalidators.com"); - BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values()[1] == "trustthesevalidators.gov"); + BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values()[0] == "xrpl-validators.com"); + BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_SITES).values()[1] == "trust-these-validators.gov"); BEAST_EXPECT(c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 1); BEAST_EXPECT( c.section(SECTION_VALIDATOR_LIST_KEYS).values()[0] == @@ -689,8 +689,8 @@ trustthesevalidators.gov Config c; std::string toLoad(R"xrpldConfig( [validator_list_sites] -xrplvalidators.com -trustthesevalidators.gov +xrpl-validators.com +trust-these-validators.gov [validator_list_keys] 021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566 @@ -718,8 +718,8 @@ trustthesevalidators.gov Config c; std::string toLoad(R"xrpldConfig( [validator_list_sites] -xrplvalidators.com -trustthesevalidators.gov +xrpl-validators.com +trust-these-validators.gov [validator_list_keys] 021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566 @@ -747,8 +747,8 @@ value = 2 Config c; std::string toLoad(R"xrpldConfig( [validator_list_sites] -xrplvalidators.com -trustthesevalidators.gov +xrpl-validators.com +trust-these-validators.gov [validator_list_keys] 021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566 @@ -774,8 +774,8 @@ trustthesevalidators.gov Config c; std::string toLoad(R"xrpldConfig( [validator_list_sites] -xrplvalidators.com -trustthesevalidators.gov +xrpl-validators.com +trust-these-validators.gov )xrpldConfig"); std::string error; auto const expectedError = "[validator_list_keys] config section is missing"; @@ -887,8 +887,8 @@ nHB1X37qrniVugfQcuBTAjswphC1drx7QjFFojJPZwKHHnt8kU7v nHUkAWDR4cB8AgPg7VXMX6et8xRTQb2KJfgv1aBEXozwrawRKgMB [validator_list_sites] -xrplvalidators.com -trustthesevalidators.gov +xrpl-validators.com +trust-these-validators.gov [validator_list_keys] 021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566 diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index aea9edeb93..c0e54900f6 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -535,7 +535,7 @@ cpe(Currency const& c); // All path element STPathElement -allpe(AccountID const& a, Issue const& iss); +allPathElements(AccountID const& a, Issue const& iss); /***************************************************************/ /* Check */ diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index 4d27ddf740..f920858ea5 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -253,12 +253,12 @@ AMM::expectAmmRpcInfo( } bool -AMM::expectAmmInfo(STAmount const& asset1, STAmount const& asset2, IOUAmount const& balance, Json::Value const& jvres) +AMM::expectAmmInfo(STAmount const& asset1, STAmount const& asset2, IOUAmount const& balance, Json::Value const& jvRes) const { - if (!jvres.isMember(jss::amm)) + if (!jvRes.isMember(jss::amm)) return false; - auto const& jv = jvres[jss::amm]; + auto const& jv = jvRes[jss::amm]; if (!jv.isMember(jss::amount) || !jv.isMember(jss::amount2) || !jv.isMember(jss::lp_token)) return false; STAmount asset1Info; @@ -360,26 +360,26 @@ AMM::deposit( maxEP->setJson(jv[jss::EPrice]); if (tfee) jv[jss::TradingFee] = *tfee; - std::uint32_t jvflags = 0; + std::uint32_t jvFlags = 0; if (flags) - jvflags = *flags; + jvFlags = *flags; // If including asset1In and asset2In or tokens as // deposit min amounts then must set the flags // explicitly instead of relying on this logic. - if (!(jvflags & tfDepositSubTx)) + if (!(jvFlags & tfDepositSubTx)) { if (tokens && !asset1In) - jvflags |= tfLPToken; + jvFlags |= tfLPToken; else if (tokens && asset1In) - jvflags |= tfOneAssetLPToken; + jvFlags |= tfOneAssetLPToken; else if (asset1In && asset2In) - jvflags |= tfTwoAsset; + jvFlags |= tfTwoAsset; else if (maxEP && asset1In) - jvflags |= tfLimitLPToken; + jvFlags |= tfLimitLPToken; else if (asset1In) - jvflags |= tfSingleAsset; + jvFlags |= tfSingleAsset; } - jv[jss::Flags] = jvflags; + jv[jss::Flags] = jvFlags; return deposit(account, jv, assets, seq, ter); } @@ -465,23 +465,23 @@ AMM::withdraw( STAmount const saMaxEP{*maxEP, lptIssue_}; saMaxEP.setJson(jv[jss::EPrice]); } - std::uint32_t jvflags = 0; + std::uint32_t jvFlags = 0; if (flags) - jvflags = *flags; - if (!(jvflags & tfWithdrawSubTx)) + jvFlags = *flags; + if (!(jvFlags & tfWithdrawSubTx)) { if (tokens && !asset1Out) - jvflags |= tfLPToken; + jvFlags |= tfLPToken; else if (asset1Out && asset2Out) - jvflags |= tfTwoAsset; + jvFlags |= tfTwoAsset; else if (tokens && asset1Out) - jvflags |= tfOneAssetLPToken; + jvFlags |= tfOneAssetLPToken; else if (asset1Out && maxEP) - jvflags |= tfLimitLPToken; + jvFlags |= tfLimitLPToken; else if (asset1Out) - jvflags |= tfSingleAsset; + jvFlags |= tfSingleAsset; } - jv[jss::Flags] = jvflags; + jv[jss::Flags] = jvFlags; return withdraw(account, jv, seq, assets, ter); } diff --git a/src/test/jtx/impl/TestHelpers.cpp b/src/test/jtx/impl/TestHelpers.cpp index 7155c8fd0a..d89dec456b 100644 --- a/src/test/jtx/impl/TestHelpers.cpp +++ b/src/test/jtx/impl/TestHelpers.cpp @@ -304,7 +304,7 @@ cpe(Currency const& c) // All path element STPathElement -allpe(AccountID const& a, Issue const& iss) +allPathElements(AccountID const& a, Issue const& iss) { return STPathElement( STPathElement::typeAccount | STPathElement::typeCurrency | STPathElement::typeIssuer, diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 90378fae90..f505bff740 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -99,10 +99,10 @@ MPTTester::operator MPT() const } Json::Value -MPTTester::createjv(MPTCreate const& arg) +MPTTester::createJV(MPTCreate const& arg) { if (!arg.issuer) - Throw("MPTTester::createjv: issuer is not set"); + Throw("MPTTester::createJV: issuer is not set"); Json::Value jv; jv[sfAccount] = arg.issuer->human(); if (arg.assetScale) @@ -128,7 +128,7 @@ MPTTester::create(MPTCreate const& arg) if (id_) Throw("MPT can't be reused"); id_ = makeMptID(env_.seq(issuer_), issuer_); - Json::Value jv = createjv( + Json::Value jv = createJV( {.issuer = issuer_, .maxAmt = arg.maxAmt, .assetScale = arg.assetScale, @@ -179,11 +179,11 @@ MPTTester::create(MPTCreate const& arg) } Json::Value -MPTTester::destroyjv(MPTDestroy const& arg) +MPTTester::destroyJV(MPTDestroy const& arg) { Json::Value jv; if (!arg.issuer || !arg.id) - Throw("MPTTester::destroyjv: issuer/id is not set"); + Throw("MPTTester::destroyJV: issuer/id is not set"); jv[sfAccount] = arg.issuer->human(); jv[sfMPTokenIssuanceID] = to_string(*arg.id); jv[sfTransactionType] = jss::MPTokenIssuanceDestroy; @@ -196,7 +196,7 @@ MPTTester::destroy(MPTDestroy const& arg) { if (!arg.id && !id_) Throw("MPT has not been created"); - Json::Value jv = destroyjv({.issuer = arg.issuer ? arg.issuer : issuer_, .id = arg.id ? arg.id : id_}); + Json::Value jv = destroyJV({.issuer = arg.issuer ? arg.issuer : issuer_, .id = arg.id ? arg.id : id_}); submit(arg, jv); } @@ -210,11 +210,11 @@ MPTTester::holder(std::string const& holder_) const } Json::Value -MPTTester::authorizejv(MPTAuthorize const& arg) +MPTTester::authorizeJV(MPTAuthorize const& arg) { Json::Value jv; if (!arg.account || !arg.id) - Throw("MPTTester::authorizejv: issuer/id is not set"); + Throw("MPTTester::authorizeJV: account/id is not set"); jv[sfAccount] = arg.account->human(); jv[sfMPTokenIssuanceID] = to_string(*arg.id); if (arg.holder) @@ -229,7 +229,7 @@ MPTTester::authorize(MPTAuthorize const& arg) { if (!arg.id && !id_) Throw("MPT has not been created"); - Json::Value jv = authorizejv({ + Json::Value jv = authorizeJV({ .account = arg.account ? arg.account : issuer_, .holder = arg.holder, .id = arg.id ? arg.id : id_, @@ -289,11 +289,11 @@ MPTTester::authorizeHolders(Holders const& holders) } Json::Value -MPTTester::setjv(MPTSet const& arg) +MPTTester::setJV(MPTSet const& arg) { Json::Value jv; if (!arg.account || !arg.id) - Throw("MPTTester::setjv: issuer/id is not set"); + Throw("MPTTester::setJV: account and/or id is not set"); jv[sfAccount] = arg.account->human(); jv[sfMPTokenIssuanceID] = to_string(*arg.id); if (arg.holder) @@ -328,7 +328,7 @@ MPTTester::set(MPTSet const& arg) { if (!arg.id && !id_) Throw("MPT has not been created"); - Json::Value jv = setjv( + Json::Value jv = setJV( {.account = arg.account ? arg.account : issuer_, .holder = arg.holder, .id = arg.id ? arg.id : id_, @@ -476,7 +476,7 @@ MPTTester::pay( Throw("MPT has not been created"); auto const srcAmt = getBalance(src); auto const destAmt = getBalance(dest); - auto const outstnAmt = getBalance(issuer_); + auto const outstandingAmt = getBalance(issuer_); if (credentials) env_(jtx::pay(src, dest, mpt(amount)), ter(err.value_or(tesSUCCESS)), credentials::ids(*credentials)); @@ -505,7 +505,7 @@ MPTTester::pay( env_.require(mptbalance(*this, src, srcAmt - actual)); env_.require(mptbalance(*this, dest, destAmt + amount)); // Outstanding amount is reduced by the transfer fee if any - env_.require(mptbalance(*this, issuer_, outstnAmt - (actual - amount))); + env_.require(mptbalance(*this, issuer_, outstandingAmt - (actual - amount))); } } diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 792d43056f..510248240d 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -177,19 +177,19 @@ public: create(MPTCreate const& arg = MPTCreate{}); static Json::Value - createjv(MPTCreate const& arg = MPTCreate{}); + createJV(MPTCreate const& arg = MPTCreate{}); void destroy(MPTDestroy const& arg = MPTDestroy{}); static Json::Value - destroyjv(MPTDestroy const& arg = MPTDestroy{}); + destroyJV(MPTDestroy const& arg = MPTDestroy{}); void authorize(MPTAuthorize const& arg = MPTAuthorize{}); static Json::Value - authorizejv(MPTAuthorize const& arg = MPTAuthorize{}); + authorizeJV(MPTAuthorize const& arg = MPTAuthorize{}); void authorizeHolders(Holders const& holders); @@ -198,7 +198,7 @@ public: set(MPTSet const& set = {}); static Json::Value - setjv(MPTSet const& set = {}); + setJV(MPTSet const& set = {}); [[nodiscard]] bool checkDomainID(std::optional expected) const; diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index 16d319671c..0774390490 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -164,7 +164,7 @@ struct Directory_test : public beast::unit_test::suite return c; }(); - // First, Alices creates a lot of trustlines, and then + // First, Alice creates a lot of trustlines, and then // deletes them in a different order: { auto cl = currencies; diff --git a/src/test/ledger/View_test.cpp b/src/test/ledger/View_test.cpp index b2cc8d7057..33813e135e 100644 --- a/src/test/ledger/View_test.cpp +++ b/src/test/ledger/View_test.cpp @@ -982,8 +982,8 @@ class GetAmendments_test : public beast::unit_test::suite BEAST_EXPECT(majorities.size() >= 2); // None of the amendments should be enabled yet. - auto enableds = getEnabledAmendments(*env.closed()); - BEAST_EXPECT(enableds.empty()); + auto enabledAmendments = getEnabledAmendments(*env.closed()); + BEAST_EXPECT(enabledAmendments.empty()); // Now wait 2 weeks modulo 256 ledgers for the amendments to be // enabled. Speed the process by closing ledgers every 80 minutes, @@ -992,12 +992,12 @@ class GetAmendments_test : public beast::unit_test::suite { using namespace std::chrono_literals; env.close(80min); - enableds = getEnabledAmendments(*env.closed()); - if (!enableds.empty()) + enabledAmendments = getEnabledAmendments(*env.closed()); + if (!enabledAmendments.empty()) break; } BEAST_EXPECT(i == 255); - BEAST_EXPECT(enableds.size() >= 2); + BEAST_EXPECT(enabledAmendments.size() >= 2); } void diff --git a/src/test/overlay/compression_test.cpp b/src/test/overlay/compression_test.cpp index 6dffe4e5c9..19876809d2 100644 --- a/src/test/overlay/compression_test.cpp +++ b/src/test/overlay/compression_test.cpp @@ -172,12 +172,12 @@ public: std::string usdTxBlob = ""; auto wsc = makeWSClient(env.app().config()); { - Json::Value jrequestUsd; - jrequestUsd[jss::secret] = toBase58(generateSeed("bob")); - jrequestUsd[jss::tx_json] = pay("bob", "alice", bob["USD"](fund / 2)); - Json::Value jreply_usd = wsc->invoke("sign", jrequestUsd); + Json::Value requestUSD; + requestUSD[jss::secret] = toBase58(generateSeed("bob")); + requestUSD[jss::tx_json] = pay("bob", "alice", bob["USD"](fund / 2)); + Json::Value replyUSD = wsc->invoke("sign", requestUSD); - usdTxBlob = toBinary(jreply_usd[jss::result][jss::tx_blob].asString()); + usdTxBlob = toBinary(replyUSD[jss::result][jss::tx_blob].asString()); } auto transaction = std::make_shared(); diff --git a/src/test/protocol/STAmount_test.cpp b/src/test/protocol/STAmount_test.cpp index 10e159dd8d..98bcc7b1bc 100644 --- a/src/test/protocol/STAmount_test.cpp +++ b/src/test/protocol/STAmount_test.cpp @@ -441,9 +441,9 @@ public: STAmount smallValue(noIssue(), (STAmount::cMinValue + STAmount::cMaxValue) / 2, STAmount::cMinOffset + 1); STAmount zeroSt(noIssue(), 0); - STAmount smallXsmall = multiply(smallValue, smallValue, noIssue()); + STAmount smallXSmall = multiply(smallValue, smallValue, noIssue()); - BEAST_EXPECT(smallXsmall == beast::zero); + BEAST_EXPECT(smallXSmall == beast::zero); STAmount bigDsmall = divide(smallValue, bigValue, noIssue()); diff --git a/src/test/rpc/AccountLines_test.cpp b/src/test/rpc/AccountLines_test.cpp index 337da27ea0..1f5f190cfa 100644 --- a/src/test/rpc/AccountLines_test.cpp +++ b/src/test/rpc/AccountLines_test.cpp @@ -380,7 +380,7 @@ public: BEAST_EXPECT(aliceLines2[jss::result][jss::lines].size() == 1); BEAST_EXPECT(!aliceLines2[jss::result].isMember(jss::marker)); - // Get account lines for beckys account, using alices SignerList as a + // Get account lines for becky's account, using alice's SignerList as a // marker. This should cause an error. Json::Value beckyLinesParams; beckyLinesParams[jss::account] = becky.human(); diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index e9c877b421..0dec2bde7f 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -17,7 +17,7 @@ namespace xrpl { namespace test { -static char const* bobs_account_objects[] = { +static char const* bob_account_objects[] = { R"json({ "Account" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", "BookDirectory" : "50AD0A9E54D2B381288D535EB724E4275FFBF41580D28A925D038D7EA4C68000", @@ -254,7 +254,7 @@ public: Json::Value bobj[4]; for (int i = 0; i < 4; ++i) - Json::Reader{}.parse(bobs_account_objects[i], bobj[i]); + Json::Reader{}.parse(bob_account_objects[i], bobj[i]); // test 'unstepped' // i.e. request account objects without explicit limit/marker paging diff --git a/src/test/rpc/Handler_test.cpp b/src/test/rpc/Handler_test.cpp index 189949d92c..7aeb059f56 100644 --- a/src/test/rpc/Handler_test.cpp +++ b/src/test/rpc/Handler_test.cpp @@ -8,6 +8,7 @@ #include #include #include +// cspell: words stdev namespace xrpl::test { diff --git a/src/test/rpc/LedgerEntry_test.cpp b/src/test/rpc/LedgerEntry_test.cpp index a7ae547272..aa959629b1 100644 --- a/src/test/rpc/LedgerEntry_test.cpp +++ b/src/test/rpc/LedgerEntry_test.cpp @@ -501,7 +501,7 @@ class LedgerEntry_test : public beast::unit_test::suite accountRootIndex = jrr[jss::index].asString(); } { - constexpr char alicesAcctRootBinary[]{ + constexpr char aliceAcctRootBinary[]{ "1100612200800000240000000425000000032D00000000559CE54C3B934E4" "73A995B477E92EC229F99CED5B62BF4D2ACE4DC42719103AE2F6240000002" "540BE4008114AE123A8556F3CF91154711376AFB0F894F832B3D"}; @@ -513,7 +513,7 @@ class LedgerEntry_test : public beast::unit_test::suite jvParams[jss::ledger_hash] = ledgerHash; Json::Value const jrr = env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result]; BEAST_EXPECT(jrr.isMember(jss::node_binary)); - BEAST_EXPECT(jrr[jss::node_binary] == alicesAcctRootBinary); + BEAST_EXPECT(jrr[jss::node_binary] == aliceAcctRootBinary); } { // Request alice's account root using the index. diff --git a/src/test/rpc/NoRipple_test.cpp b/src/test/rpc/NoRipple_test.cpp index 07b0c95e92..9cfc69ccd5 100644 --- a/src/test/rpc/NoRipple_test.cpp +++ b/src/test/rpc/NoRipple_test.cpp @@ -38,8 +38,8 @@ public: // Check no-ripple flag on sender 'gateway' Json::Value lines{env.rpc("json", "account_lines", to_string(account_gw))}; - auto const& gline0 = lines[jss::result][jss::lines][0u]; - BEAST_EXPECT(gline0[jss::no_ripple].asBool() == SetOrClear); + auto const& gwLine0 = lines[jss::result][jss::lines][0u]; + BEAST_EXPECT(gwLine0[jss::no_ripple].asBool() == SetOrClear); // Check no-ripple peer flag on destination 'alice' lines = env.rpc("json", "account_lines", to_string(account_alice)); diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index 0e212afb21..3b289d73ca 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/src/test/rpc/Transaction_test.cpp @@ -512,7 +512,7 @@ class Transaction_test : public beast::unit_test::suite } void - testCTIDRPC(FeatureBitset features) + testRPCsForCTID(FeatureBitset features) { testcase("CTID RPC"); @@ -804,7 +804,7 @@ public: testRangeRequest(features); testRangeCTIDRequest(features); testCTIDValidation(features); - testCTIDRPC(features); + testRPCsForCTID(features); forAllApiVersions(std::bind_front(&Transaction_test::testRequest, this, features)); } }; From db2734cbc96329f64c23ea68fadc2fc898696510 Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Fri, 6 Feb 2026 21:33:42 +0000 Subject: [PATCH 09/12] refactor: Change main thread name to `xrpld-main` (#6336) This change builds on the thread-renaming PR (#6212), by renaming the main thread name to reduce ambiguity in performance monitoring tools. --- src/xrpld/app/main/Main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xrpld/app/main/Main.cpp b/src/xrpld/app/main/Main.cpp index 031daeb5e9..aaf7af95ab 100644 --- a/src/xrpld/app/main/Main.cpp +++ b/src/xrpld/app/main/Main.cpp @@ -323,7 +323,7 @@ run(int argc, char** argv) { using namespace std; - beast::setCurrentThreadName("main"); + beast::setCurrentThreadName("xrpld-main"); po::variables_map vm; From 65f9cf80c083f95fe0436195e4dc24cad8be07c4 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 9 Feb 2026 12:13:39 -0500 Subject: [PATCH 10/12] add readme to src/xrpld/app/wasm (#6340) * add readme to src/xrpl/app/wasm * important block * respond to copilot --- src/xrpld/app/wasm/README.md | 189 +++++++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 src/xrpld/app/wasm/README.md diff --git a/src/xrpld/app/wasm/README.md b/src/xrpld/app/wasm/README.md new file mode 100644 index 0000000000..3275011b49 --- /dev/null +++ b/src/xrpld/app/wasm/README.md @@ -0,0 +1,189 @@ +# WASM Module for Programmable Escrows + +This module provides WebAssembly (WASM) execution capabilities for programmable +escrows on the XRP Ledger. When an escrow is finished, the WASM code runs to +determine whether the escrow conditions are met, enabling custom programmable +logic for escrow release conditions. + +For the full specification, see +[XLS-0102: WASM VM](https://xls.xrpl.org/xls/XLS-0102-wasm-vm.html). + +## Architecture + +The module follows a layered architecture: + +``` +┌─────────────────────────────────────────────────────────────┐ +│ WasmEngine (WasmVM.h) │ +│ runEscrowWasm(), preflightEscrowWasm() │ +│ Host function registration │ +├─────────────────────────────────────────────────────────────┤ +│ WasmiEngine (WasmiVM.h) │ +│ Low-level wasmi interpreter integration │ +├─────────────────────────────────────────────────────────────┤ +│ HostFuncWrapper │ HostFuncImpl │ +│ C-style WASM bridges │ C++ implementations │ +├─────────────────────────────────────────────────────────────┤ +│ HostFunc (Interface) │ +│ Abstract base class for host functions │ +└─────────────────────────────────────────────────────────────┘ +``` + +### Key Components + +- **`WasmVM.h` / `detail/WasmVM.cpp`** - High-level facade providing: + - `WasmEngine` singleton that wraps the underlying WASM interpreter + - `runEscrowWasm()` - Execute WASM code for escrow finish + - `preflightEscrowWasm()` - Validate WASM code during preflight + - `createWasmImport()` - Register all host functions + +- **`WasmiVM.h` / `detail/WasmiVM.cpp`** - Low-level integration with the + [wasmi](https://github.com/wasmi-labs/wasmi) WebAssembly interpreter: + - `WasmiEngine` - Manages WASM modules, instances, and execution + - Memory management and gas metering + - Function invocation and result handling + +- **`HostFunc.h`** - Abstract `HostFunctions` base class defining the interface + for all callable host functions. Each method returns + `Expected`. + +- **`HostFuncImpl.h` / `detail/HostFuncImpl*.cpp`** - Concrete + `WasmHostFunctionsImpl` class that implements host functions with access to + `ApplyContext` for ledger state queries. Implementation split across files: + - `HostFuncImpl.cpp` - Core utilities (updateData, checkSignature, etc.) + - `HostFuncImplFloat.cpp` - Float/number arithmetic operations + - `HostFuncImplGetter.cpp` - Field access (transaction, ledger objects) + - `HostFuncImplKeylet.cpp` - Keylet construction functions + - `HostFuncImplLedgerHeader.cpp` - Ledger header info access + - `HostFuncImplNFT.cpp` - NFT-related queries + - `HostFuncImplTrace.cpp` - Debugging/tracing functions + +- **`HostFuncWrapper.h` / `detail/HostFuncWrapper.cpp`** - C-style wrapper + functions that bridge WASM calls to C++ `HostFunctions` methods. Each host + function has: + - A `_proto` type alias defining the function signature + - A `_wrap` function that extracts parameters and calls the implementation + +- **`ParamsHelper.h`** - Utilities for WASM parameter handling: + - `WASM_IMPORT_FUNC` / `WASM_IMPORT_FUNC2` macros for registration + - `wasmParams()` helper for building parameter vectors + - Type conversion between WASM and C++ types + +## Host Functions + +Host functions allow WASM code to interact with the XRP Ledger. They are +organized into categories: + +- **Ledger Information** - Access ledger sequence, timestamps, hashes, fees +- **Transaction & Ledger Object Access** - Read fields from the transaction + and ledger objects (including the current escrow object) +- **Keylet Construction** - Build keylets to look up various ledger object types +- **Cryptography** - Signature verification and hashing +- **Float Arithmetic** - Mathematical operations for amount calculations +- **NFT Operations** - Query NFT properties +- **Tracing/Debugging** - Log messages for debugging + +For the complete list of available host functions, their WASM names, and gas +costs, see the [XLS-0102 specification](https://xls.xrpl.org/xls/XLS-0102-wasm-vm.html) +or `detail/WasmVM.cpp` where they are registered via `WASM_IMPORT_FUNC2` macros. +For method signatures, see `HostFunc.h`. + +## Gas Model + +Each host function has an associated gas cost. The gas cost is specified when +registering the function in `detail/WasmVM.cpp`: + +```cpp +WASM_IMPORT_FUNC2(i, getLedgerSqn, "get_ledger_sqn", hfs, 60); +// ^^ gas cost +``` + +WASM execution is metered, and if the gas limit is exceeded, execution fails. + +## Entry Point + +The WASM module must export a function with the name defined by +`ESCROW_FUNCTION_NAME` (currently `"finish"`). This function: + +- Takes no parameters (or parameters passed via host function calls) +- Returns an `int32_t`: + - `1` (or positive): Escrow conditions are met, allow finish + - `0` (or negative): Escrow conditions are not met, reject finish + +## Adding a New Host Function + +To add a new host function, follow these steps: + +### 1. Add to HostFunc.h (Base Class) + +Add a virtual method declaration with a default implementation that returns an +error: + +```cpp +virtual Expected +myNewFunction(ParamType1 param1, ParamType2 param2) +{ + return Unexpected(HostFunctionError::INTERNAL); +} +``` + +### 2. Add to HostFuncImpl.h (Declaration) + +Add the method override declaration in `WasmHostFunctionsImpl`: + +```cpp +Expected +myNewFunction(ParamType1 param1, ParamType2 param2) override; +``` + +### 3. Implement in detail/HostFuncImpl\*.cpp + +Add the implementation in the appropriate file: + +```cpp +Expected +WasmHostFunctionsImpl::myNewFunction(ParamType1 param1, ParamType2 param2) +{ + // Implementation using ctx (ApplyContext) for ledger access + return result; +} +``` + +### 4. Add Wrapper to HostFuncWrapper.h + +Add the prototype and wrapper declaration: + +```cpp +using myNewFunction_proto = int32_t(uint8_t const*, int32_t, ...); +wasm_trap_t* +myNewFunction_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* results); +``` + +### 5. Implement Wrapper in detail/HostFuncWrapper.cpp + +Implement the C-style wrapper that bridges WASM to C++: + +```cpp +wasm_trap_t* +myNewFunction_wrap(void* env, wasm_val_vec_t const* params, wasm_val_vec_t* results) +{ + // Extract parameters from params + // Call hfs->myNewFunction(...) + // Set results and return +} +``` + +### 6. Register in WasmVM.cpp + +Add the function registration in `setCommonHostFunctions()` or +`createWasmImport()`: + +```cpp +WASM_IMPORT_FUNC2(i, myNewFunction, "my_new_function", hfs, 100); +// ^^ WASM name ^^ gas cost +``` + +> [!IMPORTANT] +> New host functions MUST be amendment-gated in `WasmVM.cpp`. +> Wrap the registration in an amendment check to ensure the function is only +> available after the corresponding amendment is enabled on the network. From e11f6190b74599737f9f554da3b141f269e43803 Mon Sep 17 00:00:00 2001 From: Olek <115580134+oleks-rip@users.noreply.github.com> Date: Tue, 10 Feb 2026 14:02:53 -0500 Subject: [PATCH 11/12] fix: Update invariant checks for Permissioned Domains (#6134) --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/Invariants_test.cpp | 636 ++++++++++++++------ src/test/app/PermissionedDomains_test.cpp | 32 +- src/test/jtx/impl/balance.cpp | 24 +- src/xrpld/app/tx/detail/InvariantCheck.cpp | 102 +++- src/xrpld/app/tx/detail/InvariantCheck.h | 6 +- 6 files changed, 555 insertions(+), 247 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index d8498ffa2f..961bc6e44c 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,7 +15,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. - +XRPL_FIX (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(LendingProtocol, Supported::yes, VoteBehavior::DefaultNo) diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index b3fa027753..7558c951b0 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -36,6 +36,12 @@ class Invariants_test : public beast::unit_test::suite // changes that will cause the check to fail. using Precheck = std::function; + static FeatureBitset + defaultAmendments() + { + return xrpl::test::jtx::testable_amendments() | featureInvariantsV1_1 | featureSingleAssetVault; + } + /** Run a specific test case to put the ledger into a state that will be * detected by an invariant. Simulates the actions of a transaction that * would violate an invariant. @@ -62,10 +68,23 @@ class Invariants_test : public beast::unit_test::suite std::initializer_list ters = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, Preclose const& preclose = {}, TxAccount setTxAccount = TxAccount::None) + { + return doInvariantCheck( + test::jtx::Env(*this, defaultAmendments()), expect_logs, precheck, fee, tx, ters, preclose, setTxAccount); + } + + void + doInvariantCheck( + test::jtx::Env&& env, + std::vector const& expect_logs, + Precheck const& precheck, + XRPAmount fee = XRPAmount{}, + STTx tx = STTx{ttACCOUNT_SET, [](STObject&) {}}, + std::initializer_list ters = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}, + Preclose const& preclose = {}, + TxAccount setTxAccount = TxAccount::None) { using namespace test::jtx; - FeatureBitset amendments = testable_amendments() | featureInvariantsV1_1 | featureSingleAssetVault; - Env env{*this, amendments}; Account const A1{"A1"}; Account const A2{"A2"}; @@ -74,11 +93,28 @@ class Invariants_test : public beast::unit_test::suite BEAST_EXPECT(preclose(A1, A2, env)); env.close(); + if (setTxAccount != TxAccount::None) + tx.setAccountID(sfAccount, setTxAccount == TxAccount::A1 ? A1.id() : A2.id()); + + return doInvariantCheck(std::move(env), A1, A2, expect_logs, precheck, fee, tx, ters); + } + + void + doInvariantCheck( + test::jtx::Env&& env, + test::jtx::Account const& A1, + test::jtx::Account const& A2, + std::vector const& expect_logs, + Precheck const& precheck, + XRPAmount fee = XRPAmount{}, + STTx tx = STTx{ttACCOUNT_SET, [](STObject&) {}}, + std::initializer_list ters = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}) + { + using namespace test::jtx; + OpenView ov{*env.current()}; test::StreamSink sink{beast::severities::kWarning}; beast::Journal jlog{sink}; - if (setTxAccount != TxAccount::None) - tx.setAccountID(sfAccount, setTxAccount == TxAccount::A1 ? A1.id() : A2.id()); ApplyContext ac{env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, tapNONE, jlog}; BEAST_EXPECT(precheck(A1, A2, ac)); @@ -91,19 +127,21 @@ class Invariants_test : public beast::unit_test::suite for (TER const& terExpect : ters) { terActual = ac.checkInvariants(terActual, fee); - BEAST_EXPECT(terExpect == terActual); + BEAST_EXPECTS(terExpect == terActual, std::to_string(TERtoInt(terActual))); auto const messages = sink.messages().str(); - BEAST_EXPECT( - messages.starts_with("Invariant failed:") || messages.starts_with("Transaction caused an exception")); + + if (terActual != tesSUCCESS) + { + BEAST_EXPECTS( + messages.starts_with("Invariant failed:") || + messages.starts_with("Transaction caused an exception"), + messages); + } + // std::cerr << messages << '\n'; for (auto const& m : expect_logs) { - if (messages.find(m) == std::string::npos) - { - // uncomment if you want to log the invariant failure - // std::cerr << " --> " << m << std::endl; - fail(); - } + BEAST_EXPECTS(messages.find(m) != std::string::npos, m); } } } @@ -1119,86 +1157,80 @@ class Invariants_test : public beast::unit_test::suite }); } - void + static std::shared_ptr createPermissionedDomain( ApplyContext& ac, - std::shared_ptr& sle, test::jtx::Account const& A1, - test::jtx::Account const& A2) + test::jtx::Account const& A2, + std::uint32_t numCreds = 2, + std::uint32_t seq = 10) { - sle->setAccountID(sfOwner, A1); - sle->setFieldU32(sfSequence, 10); + Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), seq); + auto sle = std::make_shared(pdKeylet); - STArray credentials(sfAcceptedCredentials, 2); - for (std::size_t n = 0; n < 2; ++n) + sle->setAccountID(sfOwner, A1); + sle->setFieldU32(sfSequence, seq); + + if (numCreds) { - auto cred = STObject::makeInnerObject(sfCredential); - cred.setAccountID(sfIssuer, A2); - auto credType = "cred_type" + std::to_string(n); - cred.setFieldVL(sfCredentialType, Slice(credType.c_str(), credType.size())); - credentials.push_back(std::move(cred)); + // This array is sorted naturally, but if you willing to change this + // behavior don't forget to use credentials::makeSorted + STArray credentials(sfAcceptedCredentials, numCreds); + for (std::size_t n = 0; n < numCreds; ++n) + { + auto cred = STObject::makeInnerObject(sfCredential); + cred.setAccountID(sfIssuer, A2); + auto credType = "cred_type" + std::to_string(n); + cred.setFieldVL(sfCredentialType, Slice(credType.c_str(), credType.size())); + credentials.push_back(std::move(cred)); + } + sle->setFieldArray(sfAcceptedCredentials, credentials); } - sle->setFieldArray(sfAcceptedCredentials, credentials); + ac.view().insert(sle); + return sle; }; void - testPermissionedDomainInvariants() + testPermissionedDomainInvariants(FeatureBitset features) { using namespace test::jtx; - testcase << "PermissionedDomain"; - doInvariantCheck( - {{"permissioned domain with no rules."}}, - [](Account const& A1, Account const&, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - slePd->setAccountID(sfOwner, A1); - slePd->setFieldU32(sfSequence, 10); + bool const fixPDEnabled = features[fixPermissionedDomainInvariant]; + std::initializer_list badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED}; + std::initializer_list failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; - ac.view().insert(slePd); - return true; + testcase << "PermissionedDomain" + std::string(fixPDEnabled ? " fix" : ""); + + doInvariantCheck( + Env(*this, features), + {{"permissioned domain with no rules."}}, + [](Account const& A1, Account const& A2, ApplyContext& ac) { + return createPermissionedDomain(ac, A1, A2, 0).get(); }, XRPAmount{}, - STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? failTers : badTers); testcase << "PermissionedDomain 2"; auto constexpr tooBig = maxPermissionedDomainCredentialsArraySize + 1; doInvariantCheck( + Env(*this, features), {{"permissioned domain bad credentials size " + std::to_string(tooBig)}}, [](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - slePd->setAccountID(sfOwner, A1); - slePd->setFieldU32(sfSequence, 10); - - STArray credentials(sfAcceptedCredentials, tooBig); - for (std::size_t n = 0; n < tooBig; ++n) - { - auto cred = STObject::makeInnerObject(sfCredential); - cred.setAccountID(sfIssuer, A2); - auto credType = std::string("cred_type") + std::to_string(n); - cred.setFieldVL(sfCredentialType, Slice(credType.c_str(), credType.size())); - credentials.push_back(std::move(cred)); - } - slePd->setFieldArray(sfAcceptedCredentials, credentials); - ac.view().insert(slePd); - return true; + return !!createPermissionedDomain(ac, A1, A2, tooBig); }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixPDEnabled ? failTers : badTers); testcase << "PermissionedDomain 3"; doInvariantCheck( + Env(*this, features), {{"permissioned domain credentials aren't sorted"}}, [](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - slePd->setAccountID(sfOwner, A1); - slePd->setFieldU32(sfSequence, 10); + auto slePd = createPermissionedDomain(ac, A1, A2, 0); STArray credentials(sfAcceptedCredentials, 2); for (std::size_t n = 0; n < 2; ++n) @@ -1210,21 +1242,19 @@ class Invariants_test : public beast::unit_test::suite credentials.push_back(std::move(cred)); } slePd->setFieldArray(sfAcceptedCredentials, credentials); - ac.view().insert(slePd); + ac.view().update(slePd); return true; }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + fixPDEnabled ? failTers : badTers); testcase << "PermissionedDomain 4"; doInvariantCheck( + Env(*this, features), {{"permissioned domain credentials aren't unique"}}, [](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - slePd->setAccountID(sfOwner, A1); - slePd->setFieldU32(sfSequence, 10); + auto slePd = createPermissionedDomain(ac, A1, A2, 0); STArray credentials(sfAcceptedCredentials, 2); for (std::size_t n = 0; n < 2; ++n) @@ -1235,22 +1265,20 @@ class Invariants_test : public beast::unit_test::suite credentials.push_back(std::move(cred)); } slePd->setFieldArray(sfAcceptedCredentials, credentials); - ac.view().insert(slePd); + ac.view().update(slePd); return true; }, XRPAmount{}, - STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 1"; doInvariantCheck( + Env(*this, features), {{"permissioned domain with no rules."}}, [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - // create PD - createPermissionedDomain(ac, slePd, A1, A2); + auto slePd = createPermissionedDomain(ac, A1, A2); // update PD with empty rules { @@ -1262,18 +1290,16 @@ class Invariants_test : public beast::unit_test::suite return true; }, XRPAmount{}, - STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 2"; doInvariantCheck( + Env(*this, features), {{"permissioned domain bad credentials size " + std::to_string(tooBig)}}, [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - // create PD - createPermissionedDomain(ac, slePd, A1, A2); + auto slePd = createPermissionedDomain(ac, A1, A2); // update PD { @@ -1295,18 +1321,16 @@ class Invariants_test : public beast::unit_test::suite return true; }, XRPAmount{}, - STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 3"; doInvariantCheck( + Env(*this, features), {{"permissioned domain credentials aren't sorted"}}, [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - // create PD - createPermissionedDomain(ac, slePd, A1, A2); + auto slePd = createPermissionedDomain(ac, A1, A2); // update PD { @@ -1327,18 +1351,16 @@ class Invariants_test : public beast::unit_test::suite return true; }, XRPAmount{}, - STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 4"; doInvariantCheck( + Env(*this, features), {{"permissioned domain credentials aren't unique"}}, [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - // create PD - createPermissionedDomain(ac, slePd, A1, A2); + auto slePd = createPermissionedDomain(ac, A1, A2); // update PD { @@ -1357,8 +1379,155 @@ class Invariants_test : public beast::unit_test::suite return true; }, XRPAmount{}, - STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? failTers : badTers); + + std::initializer_list goodTers = {tesSUCCESS, tesSUCCESS}; + + std::vector badMoreThan1{{"transaction affected more than 1 permissioned domain entry."}}; + std::vector emptyV; + std::vector badNoDomains{{"no domain objects affected by"}}; + std::vector badNotDeleted{{"domain object modified, but not deleted by "}}; + std::vector badDeleted{{"domain object deleted by"}}; + std::vector badTx{{"domain object(s) affected by an unauthorized transaction."}}; + + { + testcase << "PermissionedDomain set 2 domains "; + doInvariantCheck( + Env(*this, features), + fixPDEnabled ? badMoreThan1 : emptyV, + [](Account const& A1, Account const& A2, ApplyContext& ac) { + createPermissionedDomain(ac, A1, A2); + createPermissionedDomain(ac, A1, A2, 2, 11); + return true; + }, + XRPAmount{}, + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? failTers : goodTers); + } + + { + testcase << "PermissionedDomain del 2 domains"; + + Env env1(*this, features); + + Account const A1{"A1"}; + Account const A2{"A2"}; + env1.fund(XRP(1000), A1, A2); + env1.close(); + + [[maybe_unused]] auto [seq1, pd1] = createPermissionedDomainEnv(env1, A1, A2); + [[maybe_unused]] auto [seq2, pd2] = createPermissionedDomainEnv(env1, A1, A2); + env1.close(); + + doInvariantCheck( + std::move(env1), + A1, + A2, + fixPDEnabled ? badMoreThan1 : emptyV, + [&pd1, &pd2](Account const&, Account const&, ApplyContext& ac) { + auto sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1}); + auto sle2 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd2}); + ac.view().erase(sle1); + ac.view().erase(sle2); + return true; + }, + XRPAmount{}, + STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, + fixPDEnabled ? failTers : goodTers); + } + + { + testcase << "PermissionedDomain set 0 domains "; + doInvariantCheck( + Env(*this, features), + fixPDEnabled ? badNoDomains : emptyV, + [](Account const&, Account const&, ApplyContext&) { return true; }, + XRPAmount{}, + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? badTers : goodTers); + } + + { + testcase << "PermissionedDomain del 0 domains"; + + Env env1(*this, features); + + Account const A1{"A1"}; + Account const A2{"A2"}; + env1.fund(XRP(1000), A1, A2); + env1.close(); + + [[maybe_unused]] auto [seq1, pd1] = createPermissionedDomainEnv(env1, A1, A2); + [[maybe_unused]] auto [seq2, pd2] = createPermissionedDomainEnv(env1, A1, A2); + env1.close(); + + doInvariantCheck( + Env(*this, features), + A1, + A2, + fixPDEnabled ? badNoDomains : emptyV, + [](Account const&, Account const&, ApplyContext&) { return true; }, + XRPAmount{}, + STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, + fixPDEnabled ? badTers : goodTers); + } + + { + testcase << "PermissionedDomain set, delete domain"; + + Env env1(*this, features); + + Account const A1{"A1"}; + Account const A2{"A2"}; + env1.fund(XRP(1000), A1, A2); + env1.close(); + + [[maybe_unused]] auto [seq1, pd1] = createPermissionedDomainEnv(env1, A1, A2); + env1.close(); + + doInvariantCheck( + std::move(env1), + A1, + A2, + fixPDEnabled ? badDeleted : emptyV, + [&pd1](Account const&, Account const&, ApplyContext& ac) { + auto sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1}); + ac.view().erase(sle1); + return true; + }, + XRPAmount{}, + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, + fixPDEnabled ? failTers : goodTers); + } + + { + testcase << "PermissionedDomain del, create domain "; + doInvariantCheck( + Env(*this, features), + fixPDEnabled ? badNotDeleted : emptyV, + [](Account const& A1, Account const& A2, ApplyContext& ac) { + createPermissionedDomain(ac, A1, A2); + return true; + }, + XRPAmount{}, + STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, + fixPDEnabled ? failTers : goodTers); + } + + { + testcase << "PermissionedDomain invalid tx"; + + doInvariantCheck( + fixPDEnabled ? badTx : emptyV, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + createPermissionedDomain(ac, A1, A2); + return true; + }, + XRPAmount{}, + STTx{ttPAYMENT, [](STObject&) {}}, + failTers); + } } void @@ -1478,13 +1647,43 @@ class Invariants_test : public beast::unit_test::suite }); } - void - testPermissionedDEX() + static std::pair + createPermissionedDomainEnv( + test::jtx::Env& env, + test::jtx::Account const& A1, + test::jtx::Account const& A2, + std::uint32_t numCreds = 2) { using namespace test::jtx; - testcase << "PermissionedDEX"; + + pdomain::Credentials credentials; + + for (std::size_t n = 0; n < numCreds; ++n) + { + auto credType = "cred_type" + std::to_string(n); + credentials.push_back({A2, credType}); + } + + std::uint32_t const seq = env.seq(A1); + env(pdomain::setTx(A1, credentials)); + uint256 key = pdomain::getNewDomain(env.meta()); + + // std::cout << "PD, acc: " << A1.id() << ", seq: " << seq << ", k: " << + // key << std::endl; + return {seq, key}; + } + + void + testPermissionedDEX(FeatureBitset features) + { + using namespace test::jtx; + + bool const fixPDEnabled = features[fixPermissionedDomainInvariant]; + + testcase << "PermissionedDEX" + std::string(fixPDEnabled ? " fix" : ""); doInvariantCheck( + Env(*this, features), {{"domain doesn't exist"}}, [](Account const& A1, Account const&, ApplyContext& ac) { Keylet const offerKey = keylet::offer(A1.id(), 10); @@ -1511,12 +1710,9 @@ class Invariants_test : public beast::unit_test::suite // missing domain ID in offer object doInvariantCheck( + Env(*this, features), {{"hybrid offer is malformed"}}, [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - createPermissionedDomain(ac, slePd, A1, A2); - Keylet const offerKey = keylet::offer(A2.id(), 10); auto sleOffer = std::make_shared(offerKey); sleOffer->setAccountID(sfAccount, A2); @@ -1531,116 +1727,154 @@ class Invariants_test : public beast::unit_test::suite return true; }, XRPAmount{}, - STTx{ttOFFER_CREATE, [&](STObject& tx) {}}, + STTx{ttOFFER_CREATE, [&](STObject&) {}}, {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); // more than one entry in sfAdditionalBooks - doInvariantCheck( - {{"hybrid offer is malformed"}}, - [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - createPermissionedDomain(ac, slePd, A1, A2); + { + Env env1(*this, features); - Keylet const offerKey = keylet::offer(A2.id(), 10); - auto sleOffer = std::make_shared(offerKey); - sleOffer->setAccountID(sfAccount, A2); - sleOffer->setFieldAmount(sfTakerPays, A1["USD"](10)); - sleOffer->setFieldAmount(sfTakerGets, XRP(1)); - sleOffer->setFlag(lsfHybrid); - sleOffer->setFieldH256(sfDomainID, pdKeylet.key); + Account const A1{"A1"}; + Account const A2{"A2"}; + env1.fund(XRP(1000), A1, A2); + env1.close(); - STArray bookArr; - bookArr.push_back(STObject::makeInnerObject(sfBook)); - bookArr.push_back(STObject::makeInnerObject(sfBook)); - sleOffer->setFieldArray(sfAdditionalBooks, bookArr); - ac.view().insert(sleOffer); - return true; - }, - XRPAmount{}, - STTx{ttOFFER_CREATE, [&](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + [[maybe_unused]] auto [seq1, pd1] = createPermissionedDomainEnv(env1, A1, A2); + env1.close(); + + doInvariantCheck( + std::move(env1), + A1, + A2, + {{"hybrid offer is malformed"}}, + [&pd1](Account const& A1, Account const& A2, ApplyContext& ac) { + Keylet const offerKey = keylet::offer(A2.id(), 10); + auto sleOffer = std::make_shared(offerKey); + sleOffer->setAccountID(sfAccount, A2); + sleOffer->setFieldAmount(sfTakerPays, A1["USD"](10)); + sleOffer->setFieldAmount(sfTakerGets, XRP(1)); + sleOffer->setFlag(lsfHybrid); + sleOffer->setFieldH256(sfDomainID, pd1); + + STArray bookArr; + bookArr.push_back(STObject::makeInnerObject(sfBook)); + bookArr.push_back(STObject::makeInnerObject(sfBook)); + sleOffer->setFieldArray(sfAdditionalBooks, bookArr); + ac.view().insert(sleOffer); + return true; + }, + XRPAmount{}, + STTx{ttOFFER_CREATE, [&](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + } // hybrid offer missing sfAdditionalBooks - doInvariantCheck( - {{"hybrid offer is malformed"}}, - [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - createPermissionedDomain(ac, slePd, A1, A2); + { + Env env1(*this, features); - Keylet const offerKey = keylet::offer(A2.id(), 10); - auto sleOffer = std::make_shared(offerKey); - sleOffer->setAccountID(sfAccount, A2); - sleOffer->setFieldAmount(sfTakerPays, A1["USD"](10)); - sleOffer->setFieldAmount(sfTakerGets, XRP(1)); - sleOffer->setFlag(lsfHybrid); - sleOffer->setFieldH256(sfDomainID, pdKeylet.key); - ac.view().insert(sleOffer); - return true; - }, - XRPAmount{}, - STTx{ttOFFER_CREATE, [&](STObject& tx) {}}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + Account const A1{"A1"}; + Account const A2{"A2"}; + env1.fund(XRP(1000), A1, A2); + env1.close(); - doInvariantCheck( - {{"transaction consumed wrong domains"}}, - [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - createPermissionedDomain(ac, slePd, A1, A2); + [[maybe_unused]] auto [seq1, pd1] = createPermissionedDomainEnv(env1, A1, A2); + env1.close(); - Keylet const badDomainKeylet = keylet::permissionedDomain(A1.id(), 20); - auto sleBadPd = std::make_shared(badDomainKeylet); - createPermissionedDomain(ac, sleBadPd, A1, A2); + doInvariantCheck( + std::move(env1), + A1, + A2, + {{"hybrid offer is malformed"}}, + [&pd1](Account const& A1, Account const& A2, ApplyContext& ac) { + Keylet const offerKey = keylet::offer(A2.id(), 10); + auto sleOffer = std::make_shared(offerKey); + sleOffer->setAccountID(sfAccount, A2); + sleOffer->setFieldAmount(sfTakerPays, A1["USD"](10)); + sleOffer->setFieldAmount(sfTakerGets, XRP(1)); + sleOffer->setFlag(lsfHybrid); + sleOffer->setFieldH256(sfDomainID, pd1); + ac.view().insert(sleOffer); + return true; + }, + XRPAmount{}, + STTx{ttOFFER_CREATE, [&](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + } - Keylet const offerKey = keylet::offer(A2.id(), 10); - auto sleOffer = std::make_shared(offerKey); - sleOffer->setAccountID(sfAccount, A2); - sleOffer->setFieldAmount(sfTakerPays, A1["USD"](10)); - sleOffer->setFieldAmount(sfTakerGets, XRP(1)); - sleOffer->setFieldH256(sfDomainID, pdKeylet.key); - ac.view().insert(sleOffer); - return true; - }, - XRPAmount{}, - STTx{ - ttOFFER_CREATE, - [&](STObject& tx) { - Account const A1{"A1"}; - Keylet const badDomainKey = keylet::permissionedDomain(A1.id(), 20); - tx.setFieldH256(sfDomainID, badDomainKey.key); - tx.setFieldAmount(sfTakerPays, A1["USD"](10)); - tx.setFieldAmount(sfTakerGets, XRP(1)); - }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + { + Env env1(*this, features); - doInvariantCheck( - {{"domain transaction affected regular offers"}}, - [&](Account const& A1, Account const& A2, ApplyContext& ac) { - Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); - auto slePd = std::make_shared(pdKeylet); - createPermissionedDomain(ac, slePd, A1, A2); + Account const A1{"A1"}; + Account const A2{"A2"}; + env1.fund(XRP(1000), A1, A2); + env1.close(); - Keylet const offerKey = keylet::offer(A2.id(), 10); - auto sleOffer = std::make_shared(offerKey); - sleOffer->setAccountID(sfAccount, A2); - sleOffer->setFieldAmount(sfTakerPays, A1["USD"](10)); - sleOffer->setFieldAmount(sfTakerGets, XRP(1)); - ac.view().insert(sleOffer); - return true; - }, - XRPAmount{}, - STTx{ - ttOFFER_CREATE, - [&](STObject& tx) { - Account const A1{"A1"}; - Keylet const domainKey = keylet::permissionedDomain(A1.id(), 10); - tx.setFieldH256(sfDomainID, domainKey.key); - tx.setFieldAmount(sfTakerPays, A1["USD"](10)); - tx.setFieldAmount(sfTakerGets, XRP(1)); - }}, - {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + [[maybe_unused]] auto [seq1, pd1] = createPermissionedDomainEnv(env1, A1, A2); + [[maybe_unused]] auto [seq2, pd2] = createPermissionedDomainEnv(env1, A1, A2); + env1.close(); + + doInvariantCheck( + std::move(env1), + A1, + A2, + {{"transaction consumed wrong domains"}}, + [&pd1](Account const& A1, Account const& A2, ApplyContext& ac) { + Keylet const offerKey = keylet::offer(A2.id(), 10); + auto sleOffer = std::make_shared(offerKey); + sleOffer->setAccountID(sfAccount, A2); + sleOffer->setFieldAmount(sfTakerPays, A1["USD"](10)); + sleOffer->setFieldAmount(sfTakerGets, XRP(1)); + sleOffer->setFieldH256(sfDomainID, pd1); + ac.view().insert(sleOffer); + return true; + }, + XRPAmount{}, + STTx{ + ttOFFER_CREATE, + [&pd2, &A1](STObject& tx) { + tx.setFieldH256(sfDomainID, pd2); + tx.setFieldAmount(sfTakerPays, A1["USD"](10)); + tx.setFieldAmount(sfTakerGets, XRP(1)); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + } + + { + Env env1(*this, features); + + Account const A1{"A1"}; + Account const A2{"A2"}; + env1.fund(XRP(1000), A1, A2); + env1.close(); + + [[maybe_unused]] auto [seq1, pd1] = createPermissionedDomainEnv(env1, A1, A2); + env1.close(); + + doInvariantCheck( + std::move(env1), + A1, + A2, + {{"domain transaction affected regular offers"}}, + [&](Account const& A1, Account const& A2, ApplyContext& ac) { + Keylet const offerKey = keylet::offer(A2.id(), 10); + auto sleOffer = std::make_shared(offerKey); + sleOffer->setAccountID(sfAccount, A2); + sleOffer->setFieldAmount(sfTakerPays, A1["USD"](10)); + sleOffer->setFieldAmount(sfTakerGets, XRP(1)); + ac.view().insert(sleOffer); + return true; + }, + XRPAmount{}, + STTx{ + ttOFFER_CREATE, + [&](STObject& tx) { + Account const A1{"A1"}; + tx.setFieldH256(sfDomainID, pd1); + tx.setFieldAmount(sfTakerPays, A1["USD"](10)); + tx.setFieldAmount(sfTakerGets, XRP(1)); + }}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + } } Keylet @@ -3490,8 +3724,10 @@ public: testNoZeroEscrow(); testValidNewAccountRoot(); testNFTokenPageInvariants(); - testPermissionedDomainInvariants(); - testPermissionedDEX(); + testPermissionedDomainInvariants(defaultAmendments() | fixPermissionedDomainInvariant); + testPermissionedDomainInvariants(defaultAmendments() - fixPermissionedDomainInvariant); + testPermissionedDEX(defaultAmendments() | fixPermissionedDomainInvariant); + testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant); testNoModifiedUnmodifiableFields(); testValidPseudoAccounts(); testValidLoanBroker(); diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index 1a1fe9a9eb..8485202144 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -38,13 +38,17 @@ class PermissionedDomains_test : public beast::unit_test::suite testable_amendments() // | featurePermissionedDomains | featureCredentials}; + FeatureBitset withFix_{ + testable_amendments() // + | featurePermissionedDomains | featureCredentials}; + // Verify that each tx type can execute if the feature is enabled. void - testEnabled() + testEnabled(FeatureBitset features) { testcase("Enabled"); Account const alice("alice"); - Env env(*this, withFeature_); + Env env(*this, features); env.fund(XRP(1000), alice); pdomain::Credentials credentials{{alice, "first credential"}}; env(pdomain::setTx(alice, credentials)); @@ -237,10 +241,10 @@ class PermissionedDomains_test : public beast::unit_test::suite // Test PermissionedDomainSet void - testSet() + testSet(FeatureBitset features) { testcase("Set"); - Env env(*this, withFeature_); + Env env(*this, features); env.set_parse_failure_expected(true); int const accNum = 12; @@ -395,10 +399,10 @@ class PermissionedDomains_test : public beast::unit_test::suite // Test PermissionedDomainDelete void - testDelete() + testDelete(FeatureBitset features) { testcase("Delete"); - Env env(*this, withFeature_); + Env env(*this, features); Account const alice("alice"); env.fund(XRP(1000), alice); @@ -448,14 +452,14 @@ class PermissionedDomains_test : public beast::unit_test::suite } void - testAccountReserve() + testAccountReserve(FeatureBitset features) { // Verify that the reserve behaves as expected for creating. testcase("Account Reserve"); using namespace test::jtx; - Env env(*this, withFeature_); + Env env(*this, features); Account const alice("alice"); // Fund alice enough to exist, but not enough to meet @@ -500,12 +504,16 @@ public: void run() override { - testEnabled(); + testEnabled(withFeature_); + testEnabled(withFix_); testCredentialsDisabled(); testDisabled(); - testSet(); - testDelete(); - testAccountReserve(); + testSet(withFeature_); + testSet(withFix_); + testDelete(withFeature_); + testDelete(withFix_); + testAccountReserve(withFeature_); + testAccountReserve(withFix_); } }; diff --git a/src/test/jtx/impl/balance.cpp b/src/test/jtx/impl/balance.cpp index 9d6f5c6124..1ebeeed437 100644 --- a/src/test/jtx/impl/balance.cpp +++ b/src/test/jtx/impl/balance.cpp @@ -4,6 +4,10 @@ namespace xrpl { namespace test { namespace jtx { +#define TEST_EXPECT(cond) env.test.expect(cond, __FILE__, __LINE__) +#define TEST_EXPECTS(cond, reason) \ + ((cond) ? (env.test.pass(), true) : (env.test.fail((reason), __FILE__, __LINE__), false)) + void doBalance(Env& env, AccountID const& account, bool none, STAmount const& value, Issue const& issue) { @@ -12,11 +16,13 @@ doBalance(Env& env, AccountID const& account, bool none, STAmount const& value, auto const sle = env.le(keylet::account(account)); if (none) { - env.test.expect(!sle); + TEST_EXPECT(!sle); } - else if (env.test.expect(sle)) + else if (TEST_EXPECT(sle)) { - env.test.expect(sle->getFieldAmount(sfBalance) == value); + TEST_EXPECTS( + sle->getFieldAmount(sfBalance) == value, + sle->getFieldAmount(sfBalance).getText() + " / " + value.getText()); } } else @@ -24,15 +30,15 @@ doBalance(Env& env, AccountID const& account, bool none, STAmount const& value, auto const sle = env.le(keylet::line(account, issue)); if (none) { - env.test.expect(!sle); + TEST_EXPECT(!sle); } - else if (env.test.expect(sle)) + else if (TEST_EXPECT(sle)) { auto amount = sle->getFieldAmount(sfBalance); amount.setIssuer(issue.account); if (account > issue.account) amount.negate(); - env.test.expect(amount == value); + TEST_EXPECTS(amount == value, amount.getText()); } } } @@ -43,12 +49,12 @@ doBalance(Env& env, AccountID const& account, bool none, STAmount const& value, auto const sle = env.le(keylet::mptoken(mptIssue.getMptID(), account)); if (none) { - env.test.expect(!sle); + TEST_EXPECT(!sle); } - else if (env.test.expect(sle)) + else if (TEST_EXPECT(sle)) { STAmount const amount{mptIssue, sle->getFieldU64(sfMPTAmount)}; - env.test.expect(amount == value); + TEST_EXPECT(amount == value); } } diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index fe47449c36..24a37270ce 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1507,7 +1507,7 @@ ValidMPTIssuance::finalize( void ValidPermissionedDomain::visitEntry( - bool, + bool isDel, std::shared_ptr const& before, std::shared_ptr const& after) { @@ -1516,39 +1516,29 @@ ValidPermissionedDomain::visitEntry( if (after && after->getType() != ltPERMISSIONED_DOMAIN) return; - auto check = [](SleStatus& sleStatus, std::shared_ptr const& sle) { + auto check = [isDel](std::vector& sleStatus, std::shared_ptr const& sle) { auto const& credentials = sle->getFieldArray(sfAcceptedCredentials); - sleStatus.credentialsSize_ = credentials.size(); auto const sorted = credentials::makeSorted(credentials); - sleStatus.isUnique_ = !sorted.empty(); + + SleStatus ss{credentials.size(), false, !sorted.empty(), isDel}; // If array have duplicates then all the other checks are invalid - sleStatus.isSorted_ = false; - - if (sleStatus.isUnique_) + if (ss.isUnique_) { unsigned i = 0; for (auto const& cred : sorted) { auto const& credTx = credentials[i++]; - sleStatus.isSorted_ = (cred.first == credTx[sfIssuer]) && (cred.second == credTx[sfCredentialType]); - if (!sleStatus.isSorted_) + ss.isSorted_ = (cred.first == credTx[sfIssuer]) && (cred.second == credTx[sfCredentialType]); + if (!ss.isSorted_) break; } } + sleStatus.emplace_back(std::move(ss)); }; - if (before) - { - sleStatus_[0] = SleStatus(); - check(*sleStatus_[0], after); - } - if (after) - { - sleStatus_[1] = SleStatus(); - check(*sleStatus_[1], after); - } + check(sleStatus_, after); } bool @@ -1559,9 +1549,6 @@ ValidPermissionedDomain::finalize( ReadView const& view, beast::Journal const& j) { - if (tx.getTxnType() != ttPERMISSIONED_DOMAIN_SET || result != tesSUCCESS) - return true; - auto check = [](SleStatus const& sleStatus, beast::Journal const& j) { if (!sleStatus.credentialsSize_) { @@ -1595,7 +1582,76 @@ ValidPermissionedDomain::finalize( return true; }; - return (sleStatus_[0] ? check(*sleStatus_[0], j) : true) && (sleStatus_[1] ? check(*sleStatus_[1], j) : true); + if (view.rules().enabled(fixPermissionedDomainInvariant)) + { + // No permissioned domains should be affected if the transaction failed + if (result != tesSUCCESS) + // If nothing changed, all is good. If there were changes, that's + // bad. + return sleStatus_.empty(); + + if (sleStatus_.size() > 1) + { + JLOG(j.fatal()) << "Invariant failed: transaction affected more " + "than 1 permissioned domain entry."; + return false; + } + + switch (tx.getTxnType()) + { + case ttPERMISSIONED_DOMAIN_SET: { + if (sleStatus_.empty()) + { + JLOG(j.fatal()) << "Invariant failed: no domain objects affected by " + "PermissionedDomainSet"; + return false; + } + + auto const& sleStatus = sleStatus_[0]; + if (sleStatus.isDelete_) + { + JLOG(j.fatal()) << "Invariant failed: domain object " + "deleted by PermissionedDomainSet"; + return false; + } + return check(sleStatus, j); + } + case ttPERMISSIONED_DOMAIN_DELETE: { + if (sleStatus_.empty()) + { + JLOG(j.fatal()) << "Invariant failed: no domain objects affected by " + "PermissionedDomainDelete"; + return false; + } + + if (!sleStatus_[0].isDelete_) + { + JLOG(j.fatal()) << "Invariant failed: domain object " + "modified, but not deleted by " + "PermissionedDomainDelete"; + return false; + } + return true; + } + default: { + if (!sleStatus_.empty()) + { + JLOG(j.fatal()) << "Invariant failed: " << sleStatus_.size() + << " domain object(s) affected by an " + "unauthorized transaction. " + << tx.getTxnType(); + return false; + } + return true; + } + } + } + else + { + if (tx.getTxnType() != ttPERMISSIONED_DOMAIN_SET || result != tesSUCCESS || sleStatus_.empty()) + return true; + return check(sleStatus_[0], j); + } } //------------------------------------------------------------------------------ diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h index 9a20e372ba..7e9a62d9e2 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.h +++ b/src/xrpld/app/tx/detail/InvariantCheck.h @@ -449,9 +449,11 @@ class ValidPermissionedDomain struct SleStatus { std::size_t credentialsSize_{0}; - bool isSorted_ = false, isUnique_ = false; + bool isSorted_ = false; + bool isUnique_ = false; + bool isDelete_ = false; }; - std::optional sleStatus_[2]; + std::vector sleStatus_; public: void From 77673663cad4ddca4a921df8a32d008fa7f37869 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 10 Feb 2026 17:42:41 -0500 Subject: [PATCH 12/12] fix cspell issues in tests (#6348) --- cspell.config.yaml | 3 + src/test/app/Wasm_test.cpp | 6 +- .../wasm_fixtures/codecov_tests/src/lib.rs | 56 +++++++++---------- src/test/app/wasm_fixtures/copyFixtures.py | 1 + 4 files changed, 35 insertions(+), 31 deletions(-) diff --git a/cspell.config.yaml b/cspell.config.yaml index a4f5e52cf8..d03eda58f9 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -6,6 +6,8 @@ ignorePaths: - docs/**/*.puml - cmake/** - LICENSE.md + - src/test/app/wasm_fixtures/**/*.wat + - src/test/app/wasm_fixtures/*.c language: en allowCompoundWords: true # TODO (#6334) ignoreRandomStrings: true @@ -59,6 +61,7 @@ words: - Britto - Btrfs - canonicality + - cdylib - changespq - checkme - choco diff --git a/src/test/app/Wasm_test.cpp b/src/test/app/Wasm_test.cpp index c4ca26250c..1802cd9d44 100644 --- a/src/test/app/Wasm_test.cpp +++ b/src/test/app/Wasm_test.cpp @@ -229,8 +229,8 @@ struct Wasm_test : public beast::unit_test::suite Bytes outb; outb.resize(1024); - auto const minsz = std::min(static_cast(512), static_cast(b58WasmHex.size())); - auto const s = std::string_view(b58WasmHex.c_str(), minsz); + auto const minSize = std::min(static_cast(512), static_cast(b58WasmHex.size())); + auto const s = std::string_view(b58WasmHex.c_str(), minSize); auto const re = engine.run(b58Wasm, "b58enco", wasmParams(outb, s)); @@ -802,7 +802,7 @@ struct Wasm_test : public beast::unit_test::suite std::shared_ptr hfs(new TestHostFunctions(env, 0)); auto imports = createWasmImport(*hfs); - { // Calls float_from_uint with bad aligment. + { // Calls float_from_uint with bad alignment. // Can be checked through codecov auto& engine = WasmEngine::instance(); diff --git a/src/test/app/wasm_fixtures/codecov_tests/src/lib.rs b/src/test/app/wasm_fixtures/codecov_tests/src/lib.rs index 5e58d13ecd..59ff5bc164 100644 --- a/src/test/app/wasm_fixtures/codecov_tests/src/lib.rs +++ b/src/test/app/wasm_fixtures/codecov_tests/src/lib.rs @@ -564,13 +564,13 @@ pub extern "C" fn finish() -> i32 { "amm_keylet_len_wrong_non_xrp_currency_len", ) }); - let xrpissue: &[u8] = &[0; 40]; // 40 bytes + let xrp_issue: &[u8] = &[0; 40]; // 40 bytes with_buffer::<32, _, _>(|ptr, len| { check_result( unsafe { host::amm_keylet( - xrpissue.as_ptr(), - xrpissue.len(), + xrp_issue.as_ptr(), + xrp_issue.len(), asset1_bytes.as_ptr(), asset1_bytes.len(), ptr, @@ -1265,7 +1265,7 @@ pub extern "C" fn finish() -> i32 { check_result( unsafe { host::account_keylet(locator.as_ptr(), locator.len(), ptr, len) }, error_codes::INVALID_PARAMS, - "account_keylet_wrong_size_accountid", + "account_keylet_wrong_size_account_id", ) }); let seq: i32 = 1; @@ -1283,7 +1283,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "check_keylet_wrong_size_accountid", + "check_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1301,7 +1301,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "credential_keylet_wrong_size_accountid1", + "credential_keylet_wrong_size_account_id1", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1319,7 +1319,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "credential_keylet_wrong_size_accountid2", + "credential_keylet_wrong_size_account_id2", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1335,7 +1335,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "delegate_keylet_wrong_size_accountid1", + "delegate_keylet_wrong_size_account_id1", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1351,7 +1351,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "delegate_keylet_wrong_size_accountid2", + "delegate_keylet_wrong_size_account_id2", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1367,7 +1367,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "deposit_preauth_keylet_wrong_size_accountid1", + "deposit_preauth_keylet_wrong_size_account_id1", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1383,14 +1383,14 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "deposit_preauth_keylet_wrong_size_accountid2", + "deposit_preauth_keylet_wrong_size_account_id2", ) }); with_buffer::<2, _, _>(|ptr, len| { check_result( unsafe { host::did_keylet(locator.as_ptr(), locator.len(), ptr, len) }, error_codes::INVALID_PARAMS, - "did_keylet_wrong_size_accountid", + "did_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1406,7 +1406,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "escrow_keylet_wrong_size_accountid", + "escrow_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1424,7 +1424,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "line_keylet_wrong_size_accountid1", + "line_keylet_wrong_size_account_id1", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1442,7 +1442,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "line_keylet_wrong_size_accountid2", + "line_keylet_wrong_size_account_id2", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1458,7 +1458,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "mpt_issuance_keylet_wrong_size_accountid", + "mpt_issuance_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1474,7 +1474,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "mptoken_keylet_wrong_size_accountid", + "mptoken_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1490,7 +1490,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "nft_offer_keylet_wrong_size_accountid", + "nft_offer_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1506,7 +1506,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "offer_keylet_wrong_size_accountid", + "offer_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1522,7 +1522,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "oracle_keylet_wrong_size_accountid", + "oracle_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1540,7 +1540,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "paychan_keylet_wrong_size_accountid1", + "paychan_keylet_wrong_size_account_id1", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1558,7 +1558,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "paychan_keylet_wrong_size_accountid2", + "paychan_keylet_wrong_size_account_id2", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1574,14 +1574,14 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "permissioned_domain_keylet_wrong_size_accountid", + "permissioned_domain_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { check_result( unsafe { host::signers_keylet(locator.as_ptr(), locator.len(), ptr, len) }, error_codes::INVALID_PARAMS, - "signers_keylet_wrong_size_accountid", + "signers_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1597,7 +1597,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "ticket_keylet_wrong_size_accountid", + "ticket_keylet_wrong_size_account_id", ) }); with_buffer::<2, _, _>(|ptr, len| { @@ -1613,7 +1613,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "vault_keylet_wrong_size_accountid", + "vault_keylet_wrong_size_account_id", ) }); let uint256: &[u8] = b"00000000000000000000000000000001"; @@ -1630,7 +1630,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "get_nft_wrong_size_accountid", + "get_nft_wrong_size_account_id", ) }); check_result( @@ -1643,7 +1643,7 @@ pub extern "C" fn finish() -> i32 { ) }, error_codes::INVALID_PARAMS, - "trace_account_wrong_size_accountid", + "trace_account_wrong_size_account_id", ); // invalid Currency was already tested above diff --git a/src/test/app/wasm_fixtures/copyFixtures.py b/src/test/app/wasm_fixtures/copyFixtures.py index 2c0abc560d..3d9faddbed 100644 --- a/src/test/app/wasm_fixtures/copyFixtures.py +++ b/src/test/app/wasm_fixtures/copyFixtures.py @@ -1,3 +1,4 @@ +# cspell: disable import os import sys import subprocess