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 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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";