From f1bb4ded21ca2f9e430b9a661fbe630c4410109b Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 4 Jun 2026 20:05:53 -0400 Subject: [PATCH 01/12] clang-tidy: template param names, const correctness, braces --- include/xrpl/basics/Number.h | 10 +++++----- src/test/basics/Number_test.cpp | 6 +++++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index ffb991a41d..8043a850a8 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -54,11 +54,11 @@ namespace detail { constexpr std::size_t kUint64Digits = 20; constexpr std::size_t kUint128Digits = 39; -template -consteval std::array +template +consteval std::array buildPowersOfTen() { - std::array result{}; + std::array result{}; T power = 1; std::size_t exponent = 0; @@ -78,8 +78,8 @@ buildPowersOfTen() } // namespace detail -template -constexpr std::array kPowerOfTenImpl = detail::buildPowersOfTen(); +template +constexpr std::array kPowerOfTenImpl = detail::buildPowersOfTen(); constexpr auto kPowerOfTen = kPowerOfTenImpl; diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index a68dcd3286..24cf5d6967 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -1698,7 +1698,7 @@ public: BigInt const exactProduct = BigInt(kAValue) * BigInt(kBValue); // What Number actually stored. - BigInt storedValue = toBigInt(product); + BigInt const storedValue = toBigInt(product); BigInt const signedDifference = storedValue - exactProduct; @@ -1930,9 +1930,13 @@ public: BEAST_EXPECT(toBigInt(a) == BigInt{"100000000000000000000"}); if (scale != MantissaRange::MantissaScale::Small) + { BEAST_EXPECT(toBigInt(b) == BigInt{"-1000000000000000001"}); + } else + { BEAST_EXPECT(toBigInt(b) == BigInt{"-1000000000000000000"}); + } Number sum; { From 2111bb4b9593a5c4f4a43ad1a27cde310159bdf5 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Fri, 5 Jun 2026 15:11:47 +0100 Subject: [PATCH 02/12] ci: Update clang-tidy to nix-based v22 (#7412) --- .clang-tidy | 2 +- .github/workflows/reusable-clang-tidy.yml | 7 +- include/xrpl/nodestore/detail/varint.h | 2 +- src/libxrpl/basics/Log.cpp | 3 +- src/libxrpl/basics/Number.cpp | 8 +- src/libxrpl/beast/core/CurrentThreadName.cpp | 1 - src/libxrpl/server/Port.cpp | 4 +- src/test/app/AccountDelete_test.cpp | 2 +- src/test/app/Batch_test.cpp | 972 +++++++++++++++--- src/test/app/Credentials_test.cpp | 4 +- src/test/app/CrossingLimitsMPT_test.cpp | 4 +- src/test/app/CrossingLimits_test.cpp | 4 +- src/test/app/DepositAuth_test.cpp | 87 +- src/test/app/Escrow_test.cpp | 4 +- src/test/app/LedgerReplay_test.cpp | 2 +- src/test/app/PayStrand_test.cpp | 28 +- src/test/app/PermissionedDEX_test.cpp | 6 +- src/test/app/PermissionedDomains_test.cpp | 98 +- src/test/app/TxQ_test.cpp | 2 +- src/test/app/ValidatorList_test.cpp | 87 +- src/test/app/ValidatorSite_test.cpp | 391 ++++--- src/test/basics/PerfLog_test.cpp | 2 +- .../beast/aged_associative_container_test.cpp | 6 +- src/test/core/Config_test.cpp | 16 +- src/test/jtx/impl/WSClient.cpp | 2 +- src/test/jtx/impl/permissioned_dex.cpp | 2 +- src/test/jtx/impl/permissioned_domains.cpp | 4 +- src/test/nodestore/import_test.cpp | 6 +- src/test/rpc/AccountObjects_test.cpp | 4 +- src/test/rpc/DepositAuthorized_test.cpp | 4 +- src/test/rpc/LedgerEntry_test.cpp | 49 +- src/xrpld/app/misc/detail/ValidatorList.cpp | 2 +- .../peerfinder/detail/PeerfinderConfig.cpp | 3 +- src/xrpld/rpc/detail/Pathfinder.cpp | 84 +- src/xrpld/rpc/detail/ServerHandler.cpp | 18 +- 35 files changed, 1392 insertions(+), 528 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index b23d7ccbff..2d72eae701 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -154,7 +154,7 @@ Checks: "-*, " # --- # readability-inconsistent-declaration-parameter-name, # in this codebase this check will break a lot of arg names -# readability-static-accessed-through-instance, # this check is probably unnecessary. it makes the code less readable +# readability-static-accessed-through-instance, # this check is probably unnecessary. It makes the code less readable # --- CheckOptions: diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 8be1db5fb2..cfbd8af963 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -36,7 +36,7 @@ jobs: needs: [determine-files] if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }} runs-on: ["self-hosted", "Linux", "X64", "heavy"] - container: "ghcr.io/xrplf/ci/debian-trixie:clang-21-sha-53033a2" + container: "ghcr.io/xrplf/xrpld/nix-debian:sha-8abe82e" permissions: contents: read issues: write @@ -56,6 +56,11 @@ jobs: uses: XRPLF/actions/get-nproc@cf0433aa74563aead044a1e395610c96d65a37cf id: nproc + - name: Set compiler environment + uses: ./.github/actions/set-compiler-env + with: + compiler: clang + - name: Setup Conan uses: ./.github/actions/setup-conan diff --git a/include/xrpl/nodestore/detail/varint.h b/include/xrpl/nodestore/detail/varint.h index e6b78fcf08..0c49274d70 100644 --- a/include/xrpl/nodestore/detail/varint.h +++ b/include/xrpl/nodestore/detail/varint.h @@ -25,7 +25,7 @@ struct varint_traits { explicit varint_traits() = default; - static constexpr std::size_t kMax = (8 * sizeof(T) + 6) / 7; + static constexpr std::size_t kMax = ((8 * sizeof(T)) + 6) / 7; }; // Returns: Number of bytes consumed or 0 on error, diff --git a/src/libxrpl/basics/Log.cpp b/src/libxrpl/basics/Log.cpp index 1079f91280..d1e54a515f 100644 --- a/src/libxrpl/basics/Log.cpp +++ b/src/libxrpl/basics/Log.cpp @@ -61,7 +61,8 @@ Logs::File::open(boost::filesystem::path const& path) bool wasOpened = false; // VFALCO TODO Make this work with Unicode file paths - std::unique_ptr stream(new std::ofstream(path.c_str(), std::fstream::app)); + std::unique_ptr stream = + std::make_unique(path.c_str(), std::fstream::app); if (stream->good()) { diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 275d82d8c9..23e913bbdc 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -1241,9 +1241,11 @@ root(Number f, unsigned d) } // Quadratic least squares curve fit of f^(1/d) in the range [0, 1] - auto const D = (((6 * di + 11) * di + 6) * di) + 1; // NOLINT(readability-identifier-naming) - auto const a0 = 3 * di * ((2 * di - 3) * di + 1); - auto const a1 = 24 * di * (2 * di - 1); + + // NOLINTNEXTLINE(readability-identifier-naming) + auto const D = (((((6 * di) + 11) * di) + 6) * di) + 1; + auto const a0 = 3 * di * ((((2 * di) - 3) * di) + 1); + auto const a1 = 24 * di * ((2 * di) - 1); auto const a2 = -30 * (di - 1) * di; Number r = ((Number{a2} * f + Number{a1}) * f + Number{a0}) / Number{D}; if (neg) diff --git a/src/libxrpl/beast/core/CurrentThreadName.cpp b/src/libxrpl/beast/core/CurrentThreadName.cpp index 52d9063179..628fec5b7a 100644 --- a/src/libxrpl/beast/core/CurrentThreadName.cpp +++ b/src/libxrpl/beast/core/CurrentThreadName.cpp @@ -71,7 +71,6 @@ setCurrentThreadNameImpl(std::string_view name) #if BOOST_OS_LINUX #include -#include #include // IWYU pragma: keep namespace beast::detail { diff --git a/src/libxrpl/server/Port.cpp b/src/libxrpl/server/Port.cpp index 00c10b2b55..b3fd7a1526 100644 --- a/src/libxrpl/server/Port.cpp +++ b/src/libxrpl/server/Port.cpp @@ -26,8 +26,8 @@ namespace xrpl { bool Port::secure() const { - return protocol.count("peer") > 0 || protocol.count("https") > 0 || protocol.count("wss") > 0 || - protocol.count("wss2") > 0; + return protocol.contains("peer") || protocol.contains("https") || protocol.contains("wss") || + protocol.contains("wss2"); } std::string diff --git a/src/test/app/AccountDelete_test.cpp b/src/test/app/AccountDelete_test.cpp index 951f99919b..65ff9ed839 100644 --- a/src/test/app/AccountDelete_test.cpp +++ b/src/test/app/AccountDelete_test.cpp @@ -813,7 +813,7 @@ public: env.close(); // alice create DepositPreauth Object - env(deposit::authCredentials(alice, {{carol, credType}})); + env(deposit::authCredentials(alice, {{.issuer = carol, .credType = credType}})); env.close(); // becky attempts to delete her account, but alice won't take her diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index 8755fe9f9c..791bb5a4d6 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -1017,7 +1017,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -1059,7 +1063,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -1101,7 +1109,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -1143,7 +1155,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -1185,7 +1201,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -1520,9 +1540,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1552,7 +1584,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); @@ -1581,7 +1617,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); @@ -1610,7 +1650,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); @@ -1662,10 +1706,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tecUNFUNDED_PAYMENT", txIDs[0], batchID}, - {2, "Payment", "tecUNFUNDED_PAYMENT", txIDs[1], batchID}, - {3, "Payment", "tecUNFUNDED_PAYMENT", txIDs[2], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[2], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1695,9 +1755,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tecUNFUNDED_PAYMENT", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1727,8 +1799,16 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1758,8 +1838,16 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1789,8 +1877,16 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1826,11 +1922,31 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "OfferCreate", "tecKILLED", txIDs[0], batchID}, - {2, "OfferCreate", "tecKILLED", txIDs[1], batchID}, - {3, "OfferCreate", "tecKILLED", txIDs[2], batchID}, - {4, "Payment", "tesSUCCESS", txIDs[3], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "OfferCreate", + .result = "tecKILLED", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "OfferCreate", + .result = "tecKILLED", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "OfferCreate", + .result = "tecKILLED", + .txHash = txIDs[2], + .batchID = batchID}, + {.index = 4, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[3], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1878,8 +1994,16 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tecUNFUNDED_PAYMENT", txIDs[0], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[0], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1909,11 +2033,31 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "Payment", "tesSUCCESS", txIDs[2], batchID}, - {4, "Payment", "tesSUCCESS", txIDs[3], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[2], + .batchID = batchID}, + {.index = 4, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[3], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1944,10 +2088,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "Payment", "tecUNFUNDED_PAYMENT", txIDs[2], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[2], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -1978,9 +2138,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2011,9 +2183,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2044,10 +2228,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "OfferCreate", "tecKILLED", txIDs[2], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "OfferCreate", + .result = "tecKILLED", + .txHash = txIDs[2], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2095,11 +2295,31 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tecUNFUNDED_PAYMENT", txIDs[1], batchID}, - {3, "Payment", "tecUNFUNDED_PAYMENT", txIDs[2], batchID}, - {4, "Payment", "tesSUCCESS", txIDs[3], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[2], + .batchID = batchID}, + {.index = 4, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[3], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2130,11 +2350,31 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "Payment", "tecUNFUNDED_PAYMENT", txIDs[2], batchID}, - {4, "Payment", "tesSUCCESS", txIDs[3], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tecUNFUNDED_PAYMENT", + .txHash = txIDs[2], + .batchID = batchID}, + {.index = 4, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[3], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2165,10 +2405,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "Payment", "tesSUCCESS", txIDs[3], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[3], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2199,10 +2455,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "Payment", "tesSUCCESS", txIDs[3], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[3], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2232,10 +2504,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "OfferCreate", "tecKILLED", txIDs[2], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "OfferCreate", + .result = "tecKILLED", + .txHash = txIDs[2], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2454,9 +2742,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "AccountSet", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "AccountSet", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2503,9 +2803,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "AccountSet", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "AccountSet", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2558,9 +2870,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "AccountDelete", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "AccountDelete", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2601,10 +2925,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "AccountDelete", "tecHAS_OBLIGATIONS", txIDs[1], batchID}, - {3, "Payment", "tesSUCCESS", txIDs[2], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "AccountDelete", + .result = "tecHAS_OBLIGATIONS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[2], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2642,7 +2982,11 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); @@ -2876,9 +3220,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "CheckCreate", "tesSUCCESS", txIDs[0], batchID}, - {2, "CheckCash", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "CheckCreate", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "CheckCash", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2922,9 +3278,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "CheckCreate", "tecDST_TAG_NEEDED", txIDs[0], batchID}, - {2, "CheckCash", "tecNO_ENTRY", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "CheckCreate", + .result = "tecDST_TAG_NEEDED", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "CheckCash", + .result = "tecNO_ENTRY", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -2987,10 +3355,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "TicketCreate", "tesSUCCESS", txIDs[0], batchID}, - {2, "CheckCreate", "tesSUCCESS", txIDs[1], batchID}, - {3, "CheckCash", "tesSUCCESS", txIDs[2], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "TicketCreate", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "CheckCreate", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "CheckCash", + .result = "tesSUCCESS", + .txHash = txIDs[2], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -3047,9 +3431,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "CheckCreate", "tesSUCCESS", txIDs[0], batchID}, - {2, "CheckCash", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "CheckCreate", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "CheckCash", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -3099,9 +3495,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -3147,9 +3555,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -3196,9 +3616,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -3257,9 +3689,21 @@ class Batch_test : public beast::unit_test::Suite { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -3268,7 +3712,11 @@ class Batch_test : public beast::unit_test::Suite { // next ledger contains noop txn std::vector const testCases = { - {0, "AccountSet", "tesSUCCESS", noopTxnID, std::nullopt}, + {.index = 0, + .txType = "AccountSet", + .result = "tesSUCCESS", + .txHash = noopTxnID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -3301,9 +3749,21 @@ class Batch_test : public beast::unit_test::Suite { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -3340,9 +3800,21 @@ class Batch_test : public beast::unit_test::Suite { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -3382,10 +3854,26 @@ class Batch_test : public beast::unit_test::Suite { std::vector const testCases = { - {0, "AccountSet", "tesSUCCESS", noopTxnID, std::nullopt}, - {1, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {2, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {3, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "AccountSet", + .result = "tesSUCCESS", + .txHash = noopTxnID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -3442,9 +3930,21 @@ class Batch_test : public beast::unit_test::Suite { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -3489,9 +3989,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -3552,10 +4064,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "CheckCreate", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "CheckCash", "tesSUCCESS", objTxnID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "CheckCreate", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "CheckCash", + .result = "tesSUCCESS", + .txHash = objTxnID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -3601,10 +4129,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "CheckCreate", "tesSUCCESS", objTxnID, std::nullopt}, - {1, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {2, "CheckCash", "tesSUCCESS", txIDs[0], batchID}, - {3, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "CheckCreate", + .result = "tesSUCCESS", + .txHash = objTxnID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 2, + .txType = "CheckCash", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -3646,10 +4190,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); { std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "CheckCreate", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, - {3, "CheckCash", "tesSUCCESS", objTxnID, std::nullopt}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "CheckCreate", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, + {.index = 3, + .txType = "CheckCash", + .result = "tesSUCCESS", + .txHash = objTxnID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -3742,10 +4302,26 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Payment", "tesSUCCESS", payTxn1ID, std::nullopt}, - {1, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {2, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {3, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = payTxn1ID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 3, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -3753,7 +4329,11 @@ class Batch_test : public beast::unit_test::Suite { // next ledger includes the payment txn std::vector const testCases = { - {0, "Payment", "tesSUCCESS", payTxn2ID, std::nullopt}, + {.index = 0, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = payTxn2ID, + .batchID = std::nullopt}, }; validateClosedLedger(env, testCases); } @@ -3965,9 +4545,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -4014,9 +4606,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "Payment", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -4064,9 +4668,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "AccountSet", "tesSUCCESS", txIDs[0], batchID}, - {2, "Payment", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "AccountSet", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "Payment", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); @@ -4126,9 +4742,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "MPTokenIssuanceSet", "tesSUCCESS", txIDs[0], batchID}, - {2, "MPTokenIssuanceSet", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "MPTokenIssuanceSet", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "MPTokenIssuanceSet", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -4167,9 +4795,21 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "TrustSet", "tesSUCCESS", txIDs[0], batchID}, - {2, "TrustSet", "tesSUCCESS", txIDs[1], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "TrustSet", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, + {.index = 2, + .txType = "TrustSet", + .result = "tesSUCCESS", + .txHash = txIDs[1], + .batchID = batchID}, }; validateClosedLedger(env, testCases); } @@ -4207,8 +4847,16 @@ class Batch_test : public beast::unit_test::Suite env.close(); std::vector const testCases = { - {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, - {1, "TrustSet", "tesSUCCESS", txIDs[0], batchID}, + {.index = 0, + .txType = "Batch", + .result = "tesSUCCESS", + .txHash = batchID, + .batchID = std::nullopt}, + {.index = 1, + .txType = "TrustSet", + .result = "tesSUCCESS", + .txHash = txIDs[0], + .batchID = batchID}, // jv2 fails with terNO_DELEGATE_PERMISSION. }; validateClosedLedger(env, testCases); diff --git a/src/test/app/Credentials_test.cpp b/src/test/app/Credentials_test.cpp index 9416ca222f..456a53bc01 100644 --- a/src/test/app/Credentials_test.cpp +++ b/src/test/app/Credentials_test.cpp @@ -1078,7 +1078,7 @@ struct Credentials_test : public beast::unit_test::Suite } // Create DepositPreauth - env(deposit::authCredentials(becky, {{subject, credType}})); + env(deposit::authCredentials(becky, {{.issuer = subject, .credType = credType}})); env.close(); // env(); auto jtx = env.jt(pay(subject, becky, XRP(100)), credentials::Ids({credIdx})); @@ -1087,7 +1087,7 @@ struct Credentials_test : public beast::unit_test::Suite auto const stx = std::make_shared(*jtx.stx); // Create PermissionedDomain - env(pdomain::setTx(becky, {{issuer, credType}})); + env(pdomain::setTx(becky, {{.issuer = issuer, .credType = credType}})); env.close(); auto const objects = pdomain::getObjects(becky, env); if (!BEAST_EXPECT(!objects.empty())) diff --git a/src/test/app/CrossingLimitsMPT_test.cpp b/src/test/app/CrossingLimitsMPT_test.cpp index 4a016f31dc..8bd0c767f7 100644 --- a/src/test/app/CrossingLimitsMPT_test.cpp +++ b/src/test/app/CrossingLimitsMPT_test.cpp @@ -270,7 +270,7 @@ public: env.require(Balance(alice, usd(2'503))); env.require(Balance(alice, eur(1'100))); - auto const numAOffers = 2'000 + 100 + 1'000 + 1 - (2 * 100 + 2 * 199 + 1 + 1); + auto const numAOffers = 2'000 + 100 + 1'000 + 1 - ((2 * 100) + (2 * 199) + 1 + 1); env.require(offers(alice, numAOffers)); env.require(Owners(alice, numAOffers + 2)); @@ -358,7 +358,7 @@ public: env.require(Balance(alice, usd(2'494))); env.require(Balance(alice, eur(1'100))); auto const numAOffers = - 1 + 2'000 + 100 + 1'000 + 1 - (1 + 2 * 100 + 2 * 199 + 1 + 1); + 1 + 2'000 + 100 + 1'000 + 1 - (1 + (2 * 100) + (2 * 199) + 1 + 1); env.require(offers(alice, numAOffers)); env.require(Owners(alice, numAOffers + 2)); diff --git a/src/test/app/CrossingLimits_test.cpp b/src/test/app/CrossingLimits_test.cpp index 3cf8f50990..c48892f04e 100644 --- a/src/test/app/CrossingLimits_test.cpp +++ b/src/test/app/CrossingLimits_test.cpp @@ -258,7 +258,7 @@ public: env.require(Balance(alice, usd(2503))); env.require(Balance(alice, eur(1100))); - auto const numAOffers = 2000 + 100 + 1000 + 1 - (2 * 100 + 2 * 199 + 1 + 1); + auto const numAOffers = 2000 + 100 + 1000 + 1 - ((2 * 100) + (2 * 199) + 1 + 1); env.require(offers(alice, numAOffers)); env.require(Owners(alice, numAOffers + 2)); @@ -329,7 +329,7 @@ public: env.require(Balance(alice, usd(2494))); env.require(Balance(alice, eur(1100))); - auto const numAOffers = 1 + 2000 + 100 + 1000 + 1 - (1 + 2 * 100 + 2 * 199 + 1 + 1); + auto const numAOffers = 1 + 2000 + 100 + 1000 + 1 - (1 + (2 * 100) + (2 * 199) + 1 + 1); env.require(offers(alice, numAOffers)); env.require(Owners(alice, numAOffers + 2)); diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index 98597a175f..3496e67b54 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -614,7 +614,8 @@ struct DepositPreauth_test : public beast::unit_test::Suite TER const expectTer(!supportsCredentials ? TER(temDISABLED) : TER(tesSUCCESS)); - env(deposit::authCredentials(becky, {{carol, credType}}), Ter(expectTer)); + env(deposit::authCredentials(becky, {{.issuer = carol, .credType = credType}}), + Ter(expectTer)); env.close(); // gw accept credentials @@ -744,7 +745,8 @@ struct DepositPreauth_test : public beast::unit_test::Suite env.close(); // Setup DepositPreauth object failed - amendent is not supported - env(deposit::authCredentials(bob, {{issuer, credType}}), Ter(temDISABLED)); + env(deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}}), + Ter(temDISABLED)); env.close(); // But can create old DepositPreauth @@ -782,10 +784,11 @@ struct DepositPreauth_test : public beast::unit_test::Suite // Bob will accept payments from accounts with credentials signed // by 'issuer' - env(deposit::authCredentials(bob, {{issuer, credType}})); + env(deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}})); env.close(); - auto const jDP = ledgerEntryDepositPreauth(env, bob, {{issuer, credType}}); + auto const jDP = + ledgerEntryDepositPreauth(env, bob, {{.issuer = issuer, .credType = credType}}); BEAST_EXPECT( jDP.isObject() && jDP.isMember(jss::result) && !jDP[jss::result].isMember(jss::error) && jDP[jss::result].isMember(jss::node) && @@ -858,11 +861,14 @@ struct DepositPreauth_test : public beast::unit_test::Suite } // Bob setup DepositPreauth object, duplicates is not allowed - env(deposit::authCredentials(bob, {{issuer, credType}, {issuer, credType}}), + env(deposit::authCredentials( + bob, + {{.issuer = issuer, .credType = credType}, + {.issuer = issuer, .credType = credType}}), Ter(temMALFORMED)); // Bob setup DepositPreauth object - env(deposit::authCredentials(bob, {{issuer, credType}})); + env(deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}})); env.close(); { @@ -928,35 +934,37 @@ struct DepositPreauth_test : public beast::unit_test::Suite { // both included [AuthorizeCredentials UnauthorizeCredentials] - auto jv = deposit::authCredentials(bob, {{issuer, credType}}); + auto jv = deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}}); jv[sfUnauthorizeCredentials.jsonName] = json::ValueType::Array; env(jv, Ter(temMALFORMED)); } { // both included [Unauthorize, AuthorizeCredentials] - auto jv = deposit::authCredentials(bob, {{issuer, credType}}); + auto jv = deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}}); jv[sfUnauthorize.jsonName] = issuer.human(); env(jv, Ter(temMALFORMED)); } { // both included [Authorize, AuthorizeCredentials] - auto jv = deposit::authCredentials(bob, {{issuer, credType}}); + auto jv = deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}}); jv[sfAuthorize.jsonName] = issuer.human(); env(jv, Ter(temMALFORMED)); } { // both included [Unauthorize, UnauthorizeCredentials] - auto jv = deposit::unauthCredentials(bob, {{issuer, credType}}); + auto jv = + deposit::unauthCredentials(bob, {{.issuer = issuer, .credType = credType}}); jv[sfUnauthorize.jsonName] = issuer.human(); env(jv, Ter(temMALFORMED)); } { // both included [Authorize, UnauthorizeCredentials] - auto jv = deposit::unauthCredentials(bob, {{issuer, credType}}); + auto jv = + deposit::unauthCredentials(bob, {{.issuer = issuer, .credType = credType}}); jv[sfAuthorize.jsonName] = issuer.human(); env(jv, Ter(temMALFORMED)); } @@ -983,7 +991,7 @@ struct DepositPreauth_test : public beast::unit_test::Suite { // empty credential type - auto jv = deposit::authCredentials(bob, {{issuer, {}}}); + auto jv = deposit::authCredentials(bob, {{.issuer = issuer, .credType = {}}}); env(jv, Ter(temMALFORMED)); } @@ -993,14 +1001,23 @@ struct DepositPreauth_test : public beast::unit_test::Suite i("i"); auto const& z = credType; auto jv = deposit::authCredentials( - bob, {{a, z}, {b, z}, {c, z}, {d, z}, {e, z}, {f, z}, {g, z}, {h, z}, {i, z}}); + bob, + {{.issuer = a, .credType = z}, + {.issuer = b, .credType = z}, + {.issuer = c, .credType = z}, + {.issuer = d, .credType = z}, + {.issuer = e, .credType = z}, + {.issuer = f, .credType = z}, + {.issuer = g, .credType = z}, + {.issuer = h, .credType = z}, + {.issuer = i, .credType = z}}); env(jv, Ter(temARRAY_TOO_LARGE)); } { // Can't create with non-existing issuer Account const rick{"rick"}; - auto jv = deposit::authCredentials(bob, {{rick, credType}}); + auto jv = deposit::authCredentials(bob, {{.issuer = rick, .credType = credType}}); env(jv, Ter(tecNO_ISSUER)); env.close(); } @@ -1010,21 +1027,24 @@ struct DepositPreauth_test : public beast::unit_test::Suite Account const john{"john"}; env.fund(env.current()->fees().accountReserve(0), john); env.close(); - auto jv = deposit::authCredentials(john, {{issuer, credType}}); + auto jv = + deposit::authCredentials(john, {{.issuer = issuer, .credType = credType}}); env(jv, Ter(tecINSUFFICIENT_RESERVE)); } { // NO deposit object exists - env(deposit::unauthCredentials(bob, {{issuer, credType}}), Ter(tecNO_ENTRY)); + env(deposit::unauthCredentials(bob, {{.issuer = issuer, .credType = credType}}), + Ter(tecNO_ENTRY)); } // Create DepositPreauth object { - env(deposit::authCredentials(bob, {{issuer, credType}})); + env(deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}})); env.close(); - auto const jDP = ledgerEntryDepositPreauth(env, bob, {{issuer, credType}}); + auto const jDP = + ledgerEntryDepositPreauth(env, bob, {{.issuer = issuer, .credType = credType}}); BEAST_EXPECT( jDP.isObject() && jDP.isMember(jss::result) && !jDP[jss::result].isMember(jss::error) && @@ -1045,14 +1065,16 @@ struct DepositPreauth_test : public beast::unit_test::Suite } // can't create duplicate - env(deposit::authCredentials(bob, {{issuer, credType}}), Ter(tecDUPLICATE)); + env(deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}}), + Ter(tecDUPLICATE)); } // Delete DepositPreauth object { - env(deposit::unauthCredentials(bob, {{issuer, credType}})); + env(deposit::unauthCredentials(bob, {{.issuer = issuer, .credType = credType}})); env.close(); - auto const jDP = ledgerEntryDepositPreauth(env, bob, {{issuer, credType}}); + auto const jDP = + ledgerEntryDepositPreauth(env, bob, {{.issuer = issuer, .credType = credType}}); BEAST_EXPECT( jDP.isObject() && jDP.isMember(jss::result) && jDP[jss::result].isMember(jss::error) && @@ -1119,7 +1141,10 @@ struct DepositPreauth_test : public beast::unit_test::Suite env(fset(bob, asfDepositAuth)); env.close(); // Bob setup DepositPreauth object - env(deposit::authCredentials(bob, {{issuer, credType}, {issuer, credType2}})); + env(deposit::authCredentials( + bob, + {{.issuer = issuer, .credType = credType}, + {.issuer = issuer, .credType = credType2}})); env.close(); { @@ -1228,7 +1253,7 @@ struct DepositPreauth_test : public beast::unit_test::Suite env(fset(bob, asfDepositAuth)); env.close(); // Bob setup DepositPreauth object - env(deposit::authCredentials(bob, {{issuer, credType}})); + env(deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}})); env.close(); auto const seq = env.seq(alice); @@ -1286,14 +1311,14 @@ struct DepositPreauth_test : public beast::unit_test::Suite env.fund(XRP(5000), stock, alice, bob); std::vector credentials = { - {"a", "a"}, - {"b", "b"}, - {"c", "c"}, - {"d", "d"}, - {"e", "e"}, - {"f", "f"}, - {"g", "g"}, - {"h", "h"}}; + {.issuer = "a", .credType = "a"}, + {.issuer = "b", .credType = "b"}, + {.issuer = "c", .credType = "c"}, + {.issuer = "d", .credType = "d"}, + {.issuer = "e", .credType = "e"}, + {.issuer = "f", .credType = "f"}, + {.issuer = "g", .credType = "g"}, + {.issuer = "h", .credType = "h"}}; for (auto const& c : credentials) env.fund(XRP(5000), c.issuer); diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 3e76524cf1..5623bc4443 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -1544,7 +1544,7 @@ struct Escrow_test : public beast::unit_test::Suite credentials::Ids({credIdx}), Ter(tecNO_PERMISSION)); - env(deposit::authCredentials(bob, {{zelda, credType}})); + env(deposit::authCredentials(bob, {{.issuer = zelda, .credType = credType}})); env.close(); // Success @@ -1601,7 +1601,7 @@ struct Escrow_test : public beast::unit_test::Suite // Bob require pre-authorization env(fset(bob, asfDepositAuth)); env.close(); - env(deposit::authCredentials(bob, {{zelda, credType}})); + env(deposit::authCredentials(bob, {{.issuer = zelda, .credType = credType}})); env.close(); // Use any valid credentials if account == dst diff --git a/src/test/app/LedgerReplay_test.cpp b/src/test/app/LedgerReplay_test.cpp index 1978d04fe1..810d93e6e1 100644 --- a/src/test/app/LedgerReplay_test.cpp +++ b/src/test/app/LedgerReplay_test.cpp @@ -549,7 +549,7 @@ struct LedgerServer while (senders.contains(fromIdx)) fromIdx = (fromIdx + 1) % fundedAccounts; senders.insert(fromIdx); - toIdx = (toIdx + r * 2) % fundedAccounts; + toIdx = (toIdx + (r * 2)) % fundedAccounts; if (toIdx == fromIdx) toIdx = (toIdx + 1) % fundedAccounts; }; diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 67a37833b2..471c641f36 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -632,7 +632,13 @@ struct PayStrand_test : public beast::unit_test::Suite // Insert implied account test( - env, usd, std::nullopt, STPath(), tesSUCCESS, D{alice, gw, usdC}, D{gw, bob, usdC}); + env, + usd, + std::nullopt, + STPath(), + tesSUCCESS, + D{.src = alice, .dst = gw, .currency = usdC}, + D{.src = gw, .dst = bob, .currency = usdC}); env.trust(eur(1000), alice, bob); // Insert implied offer @@ -642,9 +648,9 @@ struct PayStrand_test : public beast::unit_test::Suite usd, STPath(), tesSUCCESS, - D{alice, gw, usdC}, + D{.src = alice, .dst = gw, .currency = usdC}, B{usd, eur, std::nullopt}, - D{gw, bob, eurC}); + D{.src = gw, .dst = bob, .currency = eurC}); // Path with explicit offer test( @@ -653,9 +659,9 @@ struct PayStrand_test : public beast::unit_test::Suite usd, STPath({ipe(eur)}), tesSUCCESS, - D{alice, gw, usdC}, + D{.src = alice, .dst = gw, .currency = usdC}, B{usd, eur, std::nullopt}, - D{gw, bob, eurC}); + D{.src = gw, .dst = bob, .currency = eurC}); // Path with offer that changes issuer only env.trust(carol["USD"](1000), bob); @@ -665,9 +671,9 @@ struct PayStrand_test : public beast::unit_test::Suite usd, STPath({iape(carol)}), tesSUCCESS, - D{alice, gw, usdC}, + D{.src = alice, .dst = gw, .currency = usdC}, B{usd, carol["USD"], std::nullopt}, - D{carol, bob, usdC}); + D{.src = carol, .dst = bob, .currency = usdC}); // Path with XRP src currency test( @@ -678,7 +684,7 @@ struct PayStrand_test : public beast::unit_test::Suite tesSUCCESS, XRPS{alice}, B{XRP, usd, std::nullopt}, - D{gw, bob, usdC}); + D{.src = gw, .dst = bob, .currency = usdC}); // Path with XRP dst currency. test( @@ -688,7 +694,7 @@ struct PayStrand_test : public beast::unit_test::Suite STPath({STPathElement{ STPathElement::TypeCurrency, xrpAccount(), xrpCurrency(), xrpAccount()}}), tesSUCCESS, - D{alice, gw, usdC}, + D{.src = alice, .dst = gw, .currency = usdC}, B{usd, XRP, std::nullopt}, XRPS{bob}); @@ -699,10 +705,10 @@ struct PayStrand_test : public beast::unit_test::Suite usd, STPath({cpe(xrpCurrency())}), tesSUCCESS, - D{alice, gw, usdC}, + D{.src = alice, .dst = gw, .currency = usdC}, B{usd, XRP, std::nullopt}, B{XRP, eur, std::nullopt}, - D{gw, bob, eurC}); + D{.src = gw, .dst = bob, .currency = eurC}); // XRP -> XRP transaction can't include a path test(env, XRP, std::nullopt, STPath({ape(carol)}), temBAD_PATH); diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index a88cbaa868..d534f20248 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -704,7 +704,8 @@ class PermissionedDEX_test : public beast::unit_test::Suite env.close(); auto const badCredType = "badCred"; - pdomain::Credentials const credentials{{badDomainOwner, badCredType}}; + pdomain::Credentials const credentials{ + {.issuer = badDomainOwner, .credType = badCredType}}; env(pdomain::setTx(badDomainOwner, credentials)); auto objects = pdomain::getObjects(badDomainOwner, env); @@ -1222,7 +1223,8 @@ class PermissionedDEX_test : public beast::unit_test::Suite env.close(); auto const badCredType = "badCred"; - pdomain::Credentials const credentials{{badDomainOwner, badCredType}}; + pdomain::Credentials const credentials{ + {.issuer = badDomainOwner, .credType = badCredType}}; env(pdomain::setTx(badDomainOwner, credentials)); auto objects = pdomain::getObjects(badDomainOwner, env); diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index 0857a4bdef..f2d7bce152 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -62,7 +62,7 @@ class PermissionedDomains_test : public beast::unit_test::Suite Account const alice("alice"); Env env(*this, features); env.fund(XRP(1000), alice); - pdomain::Credentials const credentials{{alice, "first credential"}}; + pdomain::Credentials const credentials{{.issuer = alice, .credType = "first credential"}}; env(pdomain::setTx(alice, credentials)); BEAST_EXPECT(env.ownerCount(alice) == 1); auto objects = pdomain::getObjects(alice, env); @@ -84,7 +84,7 @@ class PermissionedDomains_test : public beast::unit_test::Suite Account const alice("alice"); Env env(*this, amendments); env.fund(XRP(1000), alice); - pdomain::Credentials const credentials{{alice, "first credential"}}; + pdomain::Credentials const credentials{{.issuer = alice, .credType = "first credential"}}; env(pdomain::setTx(alice, credentials), Ter(temDISABLED)); } @@ -96,7 +96,7 @@ class PermissionedDomains_test : public beast::unit_test::Suite Account const alice("alice"); Env env(*this, testableAmendments() - featurePermissionedDomains); env.fund(XRP(1000), alice); - pdomain::Credentials const credentials{{alice, "first credential"}}; + pdomain::Credentials const credentials{{.issuer = alice, .credType = "first credential"}}; env(pdomain::setTx(alice, credentials), Ter(temDISABLED)); env(pdomain::deleteTx(alice, uint256(75)), Ter(temDISABLED)); } @@ -124,40 +124,40 @@ class PermissionedDomains_test : public beast::unit_test::Suite // Test 11 credentials. pdomain::Credentials const credentials11{ - {alice2, "credential1"}, - {alice3, "credential2"}, - {alice4, "credential3"}, - {alice5, "credential4"}, - {alice6, "credential5"}, - {alice7, "credential6"}, - {alice8, "credential7"}, - {alice9, "credential8"}, - {alice10, "credential9"}, - {alice11, "credential10"}, - {alice12, "credential11"}}; + {.issuer = alice2, .credType = "credential1"}, + {.issuer = alice3, .credType = "credential2"}, + {.issuer = alice4, .credType = "credential3"}, + {.issuer = alice5, .credType = "credential4"}, + {.issuer = alice6, .credType = "credential5"}, + {.issuer = alice7, .credType = "credential6"}, + {.issuer = alice8, .credType = "credential7"}, + {.issuer = alice9, .credType = "credential8"}, + {.issuer = alice10, .credType = "credential9"}, + {.issuer = alice11, .credType = "credential10"}, + {.issuer = alice12, .credType = "credential11"}}; BEAST_EXPECT(credentials11.size() == kMaxPermissionedDomainCredentialsArraySize + 1); env(pdomain::setTx(account, credentials11, domain), Ter(temARRAY_TOO_LARGE)); // Test credentials including non-existent issuer. Account const nobody("nobody"); pdomain::Credentials const credentialsNon{ - {alice2, "credential1"}, - {alice3, "credential2"}, - {alice4, "credential3"}, - {nobody, "credential4"}, - {alice5, "credential5"}, - {alice6, "credential6"}, - {alice7, "credential7"}}; + {.issuer = alice2, .credType = "credential1"}, + {.issuer = alice3, .credType = "credential2"}, + {.issuer = alice4, .credType = "credential3"}, + {.issuer = nobody, .credType = "credential4"}, + {.issuer = alice5, .credType = "credential5"}, + {.issuer = alice6, .credType = "credential6"}, + {.issuer = alice7, .credType = "credential7"}}; env(pdomain::setTx(account, credentialsNon, domain), Ter(tecNO_ISSUER)); // Test bad fee env(pdomain::setTx(account, credentials11, domain), Fee(1, true), Ter(temBAD_FEE)); pdomain::Credentials const credentials4{ - {alice2, "credential1"}, - {alice3, "credential2"}, - {alice4, "credential3"}, - {alice5, "credential4"}, + {.issuer = alice2, .credType = "credential1"}, + {.issuer = alice3, .credType = "credential2"}, + {.issuer = alice4, .credType = "credential3"}, + {.issuer = alice5, .credType = "credential4"}, }; auto txJsonMutable = pdomain::setTx(account, credentials4, domain); auto const credentialOrig = txJsonMutable["AcceptedCredentials"][2u]; @@ -192,11 +192,11 @@ class PermissionedDomains_test : public beast::unit_test::Suite // permissioned domains, so transactions should return errors { pdomain::Credentials const credentialsDup{ - {alice7, "credential6"}, - {alice2, "credential1"}, - {alice3, "credential2"}, - {alice2, "credential1"}, - {alice5, "credential4"}, + {.issuer = alice7, .credType = "credential6"}, + {.issuer = alice2, .credType = "credential1"}, + {.issuer = alice3, .credType = "credential2"}, + {.issuer = alice2, .credType = "credential1"}, + {.issuer = alice5, .credType = "credential4"}, }; std::unordered_map human2Acc; @@ -230,11 +230,11 @@ class PermissionedDomains_test : public beast::unit_test::Suite // sort correctly. { pdomain::Credentials const credentialsSame{ - {alice2, "credential3"}, - {alice3, "credential2"}, - {alice2, "credential9"}, - {alice5, "credential4"}, - {alice2, "credential6"}, + {.issuer = alice2, .credType = "credential3"}, + {.issuer = alice3, .credType = "credential2"}, + {.issuer = alice2, .credType = "credential9"}, + {.issuer = alice5, .credType = "credential4"}, + {.issuer = alice2, .credType = "credential6"}, }; std::unordered_map human2Acc; for (auto const& c : credentialsSame) @@ -290,7 +290,7 @@ class PermissionedDomains_test : public beast::unit_test::Suite env.fund(XRP(1000), alice[i]); // Create new from existing account with a single credential. - pdomain::Credentials const credentials1{{alice[2], "credential1"}}; + pdomain::Credentials const credentials1{{.issuer = alice[2], .credType = "credential1"}}; { env(pdomain::setTx(alice[0], credentials1)); BEAST_EXPECT(env.ownerCount(alice[0]) == 1); @@ -314,7 +314,7 @@ class PermissionedDomains_test : public beast::unit_test::Suite "89"; static_assert(kLongCredentialType.size() == kMaxCredentialTypeLength); pdomain::Credentials const longCredentials{ - {alice[1], std::string(kLongCredentialType)}}; + {.issuer = alice[1], .credType = std::string(kLongCredentialType)}}; env(pdomain::setTx(alice[0], longCredentials)); @@ -345,16 +345,16 @@ class PermissionedDomains_test : public beast::unit_test::Suite // Create new from existing account with 10 credentials. // Last credential describe domain owner itself pdomain::Credentials const credentials10{ - {alice[2], "credential1"}, - {alice[3], "credential2"}, - {alice[4], "credential3"}, - {alice[5], "credential4"}, - {alice[6], "credential5"}, - {alice[7], "credential6"}, - {alice[8], "credential7"}, - {alice[9], "credential8"}, - {alice[10], "credential9"}, - {alice[0], "credential10"}, + {.issuer = alice[2], .credType = "credential1"}, + {.issuer = alice[3], .credType = "credential2"}, + {.issuer = alice[4], .credType = "credential3"}, + {.issuer = alice[5], .credType = "credential4"}, + {.issuer = alice[6], .credType = "credential5"}, + {.issuer = alice[7], .credType = "credential6"}, + {.issuer = alice[8], .credType = "credential7"}, + {.issuer = alice[9], .credType = "credential8"}, + {.issuer = alice[10], .credType = "credential9"}, + {.issuer = alice[0], .credType = "credential10"}, }; uint256 domain2; { @@ -434,7 +434,7 @@ class PermissionedDomains_test : public beast::unit_test::Suite env.fund(XRP(1000), alice); auto const setFee(drops(env.current()->fees().increment)); - pdomain::Credentials const credentials{{alice, "first credential"}}; + pdomain::Credentials const credentials{{.issuer = alice, .credType = "first credential"}}; env(pdomain::setTx(alice, credentials)); env.close(); @@ -498,7 +498,7 @@ class PermissionedDomains_test : public beast::unit_test::Suite BEAST_EXPECT(env.ownerCount(alice) == 0); // alice does not have enough XRP to cover the reserve. - pdomain::Credentials const credentials{{alice, "first credential"}}; + pdomain::Credentials const credentials{{.issuer = alice, .credType = "first credential"}}; env(pdomain::setTx(alice, credentials), Ter(tecINSUFFICIENT_RESERVE)); BEAST_EXPECT(env.ownerCount(alice) == 0); BEAST_EXPECT(pdomain::getObjects(alice, env).empty()); diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 8333fce3b3..dc28388de4 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -1228,7 +1228,7 @@ public: // Try to replace a middle item in the queue // with enough fee to bankrupt bob and make the // later transactions unable to pay their fees - std::int64_t bobFee = env.le(bob)->getFieldAmount(sfBalance).xrp().drops() - (9 * 10 - 1); + std::int64_t bobFee = env.le(bob)->getFieldAmount(sfBalance).xrp().drops() - ((9 * 10) - 1); env(noop(bob), Seq(bobSeq + 5), Fee(bobFee), Ter(telCAN_NOT_QUEUE_BALANCE)); checkMetrics(*this, env, 10, 12, 7, 6); diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 20a3557db5..80483446a2 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -632,7 +632,11 @@ private: checkResult( trustedKeys->applyLists( - manifest1, version, {{expiredblob, expiredSig, {}}, {blob2, sig2, {}}}, siteUri), + manifest1, + version, + {{.blob = expiredblob, .signature = expiredSig, .manifest = {}}, + {.blob = blob2, .signature = sig2, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Expired, ListDisposition::Accepted); @@ -665,7 +669,11 @@ private: checkResult( trustedKeys->applyLists( - manifest1, version2, {{blob7, sig7, {}}, {blob8, sig8, {}}}, siteUri), + manifest1, + version2, + {{.blob = blob7, .signature = sig7, .manifest = {}}, + {.blob = blob8, .signature = sig8, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Pending, ListDisposition::Pending); @@ -697,7 +705,11 @@ private: checkResult( trustedKeys->applyLists( - manifest1, version, {{blob6a, sig6a, {}}, {blob6, sig6, {}}}, siteUri), + manifest1, + version, + {{.blob = blob6a, .signature = sig6a, .manifest = {}}, + {.blob = blob6, .signature = sig6, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Pending, ListDisposition::Pending); @@ -709,7 +721,11 @@ private: checkResult( trustedKeys->applyLists( - manifest1, version, {{blob7, sig7, {}}, {blob6, sig6, {}}}, siteUri), + manifest1, + version, + {{.blob = blob7, .signature = sig7, .manifest = {}}, + {.blob = blob6, .signature = sig6, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::KnownSequence, ListDisposition::KnownSequence); @@ -720,7 +736,12 @@ private: // try empty or mangled manifest checkResult( - trustedKeys->applyLists("", version, {{blob7, sig7, {}}, {blob6, sig6, {}}}, siteUri), + trustedKeys->applyLists( + "", + version, + {{.blob = blob7, .signature = sig7, .manifest = {}}, + {.blob = blob6, .signature = sig6, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Invalid, ListDisposition::Invalid); @@ -729,7 +750,8 @@ private: trustedKeys->applyLists( base64Encode("not a manifest"), version, - {{blob7, sig7, {}}, {blob6, sig6, {}}}, + {{.blob = blob7, .signature = sig7, .manifest = {}}, + {.blob = blob6, .signature = sig6, .manifest = {}}}, siteUri), publisherPublic, ListDisposition::Invalid, @@ -740,7 +762,11 @@ private: randomMasterKey(), publisherSecret, pubSigningKeys1.first, pubSigningKeys1.second, 1)); checkResult( - trustedKeys->applyLists(untrustedManifest, version, {{blob2, sig2, {}}}, siteUri), + trustedKeys->applyLists( + untrustedManifest, + version, + {{.blob = blob2, .signature = sig2, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Untrusted, ListDisposition::Untrusted); @@ -748,7 +774,11 @@ private: // do not use list with unhandled version auto const badVersion = 666; checkResult( - trustedKeys->applyLists(manifest1, badVersion, {{blob2, sig2, {}}}, siteUri), + trustedKeys->applyLists( + manifest1, + badVersion, + {{.blob = blob2, .signature = sig2, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::UnsupportedVersion, ListDisposition::UnsupportedVersion); @@ -759,7 +789,8 @@ private: auto const sig3 = signList(blob3, pubSigningKeys1); checkResult( - trustedKeys->applyLists(manifest1, version, {{blob3, sig3, {}}}, siteUri), + trustedKeys->applyLists( + manifest1, version, {{.blob = blob3, .signature = sig3, .manifest = {}}}, siteUri), publisherPublic, ListDisposition::Accepted, ListDisposition::Accepted); @@ -780,7 +811,11 @@ private: // do not re-apply lists with past or current sequence numbers checkResult( trustedKeys->applyLists( - manifest1, version, {{blob2, sig2, {}}, {blob3, sig3, {}}}, siteUri), + manifest1, + version, + {{.blob = blob2, .signature = sig2, .manifest = {}}, + {.blob = blob3, .signature = sig3, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Stale, ListDisposition::SameSequence); @@ -799,7 +834,9 @@ private: trustedKeys->applyLists( manifest2, version, - {{blob2, sig2, manifest1}, {blob3, sig3, manifest1}, {blob4, sig4, {}}}, + {{.blob = blob2, .signature = sig2, .manifest = manifest1}, + {.blob = blob3, .signature = sig3, .manifest = manifest1}, + {.blob = blob4, .signature = sig4, .manifest = {}}}, siteUri), publisherPublic, ListDisposition::Stale, @@ -820,7 +857,11 @@ private: auto const blob5 = makeList(lists.at(5), sequence5, validUntil.time_since_epoch().count()); auto const badSig = signList(blob5, pubSigningKeys1); checkResult( - trustedKeys->applyLists(manifest1, version, {{blob5, badSig, {}}}, siteUri), + trustedKeys->applyLists( + manifest1, + version, + {{.blob = blob5, .signature = badSig, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Invalid, ListDisposition::Invalid); @@ -833,7 +874,11 @@ private: // Reprocess the pending list, but the signature is no longer valid checkResult( trustedKeys->applyLists( - manifest1, version, {{blob7, sig7, {}}, {blob8, sig8, {}}}, siteUri), + manifest1, + version, + {{.blob = blob7, .signature = sig7, .manifest = {}}, + {.blob = blob8, .signature = sig8, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Invalid, ListDisposition::Invalid); @@ -884,7 +929,11 @@ private: checkResult( trustedKeys->applyLists( - manifest2, version, {{blob8, sig8, manifest1}, {blob8, sig82, {}}}, siteUri), + manifest2, + version, + {{.blob = blob8, .signature = sig8, .manifest = manifest1}, + {.blob = blob8, .signature = sig82, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Invalid, ListDisposition::SameSequence); @@ -903,7 +952,11 @@ private: auto const sig9 = signList(blob9, signingKeysMax); checkResult( - trustedKeys->applyLists(maxManifest, version, {{blob9, sig9, {}}}, siteUri), + trustedKeys->applyLists( + maxManifest, + version, + {{.blob = blob9, .signature = sig9, .manifest = {}}}, + siteUri), publisherPublic, ListDisposition::Untrusted, ListDisposition::Untrusted); @@ -1900,7 +1953,9 @@ private: return PreparedList{ .publisherPublic = publisherPublic, .manifest = manifest, - .blobs = {{blob1, sig1, {}}, {blob2, sig2, {}}}, + .blobs = + {{.blob = blob1, .signature = sig1, .manifest = {}}, + {.blob = blob2, .signature = sig2, .manifest = {}}}, .version = version, .expirations = {expiration1, expiration2}}; }; diff --git a/src/test/app/ValidatorSite_test.cpp b/src/test/app/ValidatorSite_test.cpp index f7f805faa2..e3e3ec27df 100644 --- a/src/test/app/ValidatorSite_test.cpp +++ b/src/test/app/ValidatorSite_test.cpp @@ -380,226 +380,333 @@ public: for (auto ssl : {true, false}) { // fetch single site - testFetchList(good, {{"/validators", "", ssl}}); - testFetchList(good, {{"/validators2", "", ssl}}); + testFetchList(good, {{.path = "/validators", .msg = "", .ssl = ssl}}); + testFetchList(good, {{.path = "/validators2", .msg = "", .ssl = ssl}}); // fetch multiple sites - testFetchList(good, {{"/validators", "", ssl}, {"/validators", "", ssl}}); - testFetchList(good, {{"/validators", "", ssl}, {"/validators2", "", ssl}}); - testFetchList(good, {{"/validators2", "", ssl}, {"/validators", "", ssl}}); - testFetchList(good, {{"/validators2", "", ssl}, {"/validators2", "", ssl}}); + testFetchList( + good, + {{.path = "/validators", .msg = "", .ssl = ssl}, + {.path = "/validators", .msg = "", .ssl = ssl}}); + testFetchList( + good, + {{.path = "/validators", .msg = "", .ssl = ssl}, + {.path = "/validators2", .msg = "", .ssl = ssl}}); + testFetchList( + good, + {{.path = "/validators2", .msg = "", .ssl = ssl}, + {.path = "/validators", .msg = "", .ssl = ssl}}); + testFetchList( + good, + {{.path = "/validators2", .msg = "", .ssl = ssl}, + {.path = "/validators2", .msg = "", .ssl = ssl}}); // fetch single site with single redirects - testFetchList(good, {{"/redirect_once/301", "", ssl}}); - testFetchList(good, {{"/redirect_once/302", "", ssl}}); - testFetchList(good, {{"/redirect_once/307", "", ssl}}); - testFetchList(good, {{"/redirect_once/308", "", ssl}}); + testFetchList(good, {{.path = "/redirect_once/301", .msg = "", .ssl = ssl}}); + testFetchList(good, {{.path = "/redirect_once/302", .msg = "", .ssl = ssl}}); + testFetchList(good, {{.path = "/redirect_once/307", .msg = "", .ssl = ssl}}); + testFetchList(good, {{.path = "/redirect_once/308", .msg = "", .ssl = ssl}}); // one redirect, one not - testFetchList(good, {{"/validators", "", ssl}, {"/redirect_once/302", "", ssl}}); - testFetchList(good, {{"/validators2", "", ssl}, {"/redirect_once/302", "", ssl}}); + testFetchList( + good, + {{.path = "/validators", .msg = "", .ssl = ssl}, + {.path = "/redirect_once/302", .msg = "", .ssl = ssl}}); + testFetchList( + good, + {{.path = "/validators2", .msg = "", .ssl = ssl}, + {.path = "/redirect_once/302", .msg = "", .ssl = ssl}}); // UNLs with a "gap" between validUntil of one and validFrom of the // next testFetchList( good, - {{"/validators2", - "", - ssl, - false, - false, - 1, - detail::kDefaultExpires, - std::chrono::seconds{-90}}}); + {{.path = "/validators2", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = detail::kDefaultExpires, + .effectiveOverlap = std::chrono::seconds{-90}}}); // fetch single site with unending redirect (fails to load) testFetchList( - good, {{"/redirect_forever/301", "Exceeded max redirects", ssl, true, true}}); + good, + {{.path = "/redirect_forever/301", + .msg = "Exceeded max redirects", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // two that redirect forever testFetchList( good, - {{"/redirect_forever/307", "Exceeded max redirects", ssl, true, true}, - {"/redirect_forever/308", "Exceeded max redirects", ssl, true, true}}); + {{.path = "/redirect_forever/307", + .msg = "Exceeded max redirects", + .ssl = ssl, + .failFetch = true, + .failApply = true}, + {.path = "/redirect_forever/308", + .msg = "Exceeded max redirects", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // one unending redirect, one not testFetchList( good, - {{"/validators", "", ssl}, - {"/redirect_forever/302", "Exceeded max redirects", ssl, true, true}}); + {{.path = "/validators", .msg = "", .ssl = ssl}, + {.path = "/redirect_forever/302", + .msg = "Exceeded max redirects", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // one unending redirect, one not testFetchList( good, - {{"/validators2", "", ssl}, - {"/redirect_forever/302", "Exceeded max redirects", ssl, true, true}}); + {{.path = "/validators2", .msg = "", .ssl = ssl}, + {.path = "/redirect_forever/302", + .msg = "Exceeded max redirects", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // invalid redir Location testFetchList( good, - {{"/redirect_to/ftp://invalid-url/302", - "Invalid redirect location", - ssl, - true, - true}}); + {{.path = "/redirect_to/ftp://invalid-url/302", + .msg = "Invalid redirect location", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); testFetchList( good, - {{"/redirect_to/file://invalid-url/302", - "Invalid redirect location", - ssl, - true, - true}}); + {{.path = "/redirect_to/file://invalid-url/302", + .msg = "Invalid redirect location", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // invalid json testFetchList( - good, {{"/validators/bad", "Unable to parse JSON response", ssl, true, true}}); + good, + {{.path = "/validators/bad", + .msg = "Unable to parse JSON response", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); testFetchList( - good, {{"/validators2/bad", "Unable to parse JSON response", ssl, true, true}}); + good, + {{.path = "/validators2/bad", + .msg = "Unable to parse JSON response", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // error status returned - testFetchList(good, {{"/bad-resource", "returned bad status", ssl, true, true}}); + testFetchList( + good, + {{.path = "/bad-resource", + .msg = "returned bad status", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // location field missing testFetchList( good, - {{"/redirect_nolo/308", "returned a redirect with no Location", ssl, true, true}}); + {{.path = "/redirect_nolo/308", + .msg = "returned a redirect with no Location", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // json fields missing testFetchList( good, - {{"/validators/missing", "Missing fields in JSON response", ssl, true, true}}); + {{.path = "/validators/missing", + .msg = "Missing fields in JSON response", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); testFetchList( good, - {{"/validators2/missing", "Missing fields in JSON response", ssl, true, true}}); + {{.path = "/validators2/missing", + .msg = "Missing fields in JSON response", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // timeout - testFetchList(good, {{"/sleep/13", "took too long", ssl, true, true}}); + testFetchList( + good, + {{.path = "/sleep/13", + .msg = "took too long", + .ssl = ssl, + .failFetch = true, + .failApply = true}}); // bad manifest format using known versions // * Retrieves a v1 formatted list claiming version 2 - testFetchList(good, {{"/validators", "Missing fields", ssl, true, true, 2}}); + testFetchList( + good, + {{.path = "/validators", + .msg = "Missing fields", + .ssl = ssl, + .failFetch = true, + .failApply = true, + .serverVersion = 2}}); // * Retrieves a v2 formatted list claiming version 1 - testFetchList(good, {{"/validators2", "Missing fields", ssl, true, true, 0}}); + testFetchList( + good, + {{.path = "/validators2", + .msg = "Missing fields", + .ssl = ssl, + .failFetch = true, + .failApply = true, + .serverVersion = 0}}); // bad manifest version // Because versions other than 1 are treated as v2, the v1 // list won't have the blobs_v2 fields, and thus will claim to have // missing fields - testFetchList(good, {{"/validators", "Missing fields", ssl, true, true, 4}}); - testFetchList(good, {{"/validators2", "1 unsupported version", ssl, false, true, 4}}); + testFetchList( + good, + {{.path = "/validators", + .msg = "Missing fields", + .ssl = ssl, + .failFetch = true, + .failApply = true, + .serverVersion = 4}}); + testFetchList( + good, + {{.path = "/validators2", + .msg = "1 unsupported version", + .ssl = ssl, + .failFetch = false, + .failApply = true, + .serverVersion = 4}}); using namespace std::chrono_literals; // get expired validator list testFetchList( good, - {{"/validators", "Applied 1 expired validator list(s)", ssl, false, false, 1, 0s}}); + {{.path = "/validators", + .msg = "Applied 1 expired validator list(s)", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = 0s}}); testFetchList( good, - {{"/validators2", - "Applied 1 expired validator list(s)", - ssl, - false, - false, - 1, - 0s, - -1s}}); + {{.path = "/validators2", + .msg = "Applied 1 expired validator list(s)", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = 0s, + .effectiveOverlap = -1s}}); // force an out-of-range validUntil value testFetchList( good, - {{"/validators", - "1 invalid validator list(s)", - ssl, - false, - true, - 1, - std::chrono::seconds{json::Value::kMinInt}}}); + {{.path = "/validators", + .msg = "1 invalid validator list(s)", + .ssl = ssl, + .failFetch = false, + .failApply = true, + .serverVersion = 1, + .expiresFromNow = std::chrono::seconds{json::Value::kMinInt}}}); // force an out-of-range validUntil value on the future list // The first list is accepted. The second fails. The parser // returns the "best" result, so this looks like a success. testFetchList( good, - {{"/validators2", - "", - ssl, - false, - false, - 1, - std::chrono::seconds{json::Value::kMaxInt - 300}, - 299s}}); + {{.path = "/validators2", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = std::chrono::seconds{json::Value::kMaxInt - 300}, + .effectiveOverlap = 299s}}); // force an out-of-range validFrom value // The first list is accepted. The second fails. The parser // returns the "best" result, so this looks like a success. testFetchList( good, - {{"/validators2", - "", - ssl, - false, - false, - 1, - std::chrono::seconds{json::Value::kMaxInt - 300}, - 301s}}); + {{.path = "/validators2", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = std::chrono::seconds{json::Value::kMaxInt - 300}, + .effectiveOverlap = 301s}}); // force an out-of-range validUntil value on _both_ lists testFetchList( good, - {{"/validators2", - "2 invalid validator list(s)", - ssl, - false, - true, - 1, - std::chrono::seconds{json::Value::kMinInt}, - std::chrono::seconds{json::Value::kMaxInt - 6000}}}); + {{.path = "/validators2", + .msg = "2 invalid validator list(s)", + .ssl = ssl, + .failFetch = false, + .failApply = true, + .serverVersion = 1, + .expiresFromNow = std::chrono::seconds{json::Value::kMinInt}, + .effectiveOverlap = std::chrono::seconds{json::Value::kMaxInt - 6000}}}); // verify refresh intervals are properly clamped testFetchList( good, - {{"/validators/refresh/0", - "", - ssl, - false, - false, - 1, - detail::kDefaultExpires, - detail::kDefaultEffectiveOverlap, - 1}}); // minimum of 1 minute + {{.path = "/validators/refresh/0", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = detail::kDefaultExpires, + .effectiveOverlap = detail::kDefaultEffectiveOverlap, + .expectedRefreshMin = 1}}); // minimum of 1 minute testFetchList( good, - {{"/validators2/refresh/0", - "", - ssl, - false, - false, - 1, - detail::kDefaultExpires, - detail::kDefaultEffectiveOverlap, - 1}}); // minimum of 1 minute + {{.path = "/validators2/refresh/0", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = detail::kDefaultExpires, + .effectiveOverlap = detail::kDefaultEffectiveOverlap, + .expectedRefreshMin = 1}}); // minimum of 1 minute testFetchList( good, - {{"/validators/refresh/10", - "", - ssl, - false, - false, - 1, - detail::kDefaultExpires, - detail::kDefaultEffectiveOverlap, - 10}}); // 10 minutes is fine + {{.path = "/validators/refresh/10", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = detail::kDefaultExpires, + .effectiveOverlap = detail::kDefaultEffectiveOverlap, + .expectedRefreshMin = 10}}); // 10 minutes is fine testFetchList( good, - {{"/validators2/refresh/10", - "", - ssl, - false, - false, - 1, - detail::kDefaultExpires, - detail::kDefaultEffectiveOverlap, - 10}}); // 10 minutes is fine + {{.path = "/validators2/refresh/10", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = detail::kDefaultExpires, + .effectiveOverlap = detail::kDefaultEffectiveOverlap, + .expectedRefreshMin = 10}}); // 10 minutes is fine testFetchList( good, - {{"/validators/refresh/2000", - "", - ssl, - false, - false, - 1, - detail::kDefaultExpires, - detail::kDefaultEffectiveOverlap, - 60 * 24}}); // max of 24 hours + {{.path = "/validators/refresh/2000", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = detail::kDefaultExpires, + .effectiveOverlap = detail::kDefaultEffectiveOverlap, + .expectedRefreshMin = 60 * 24}}); // max of 24 hours testFetchList( good, - {{"/validators2/refresh/2000", - "", - ssl, - false, - false, - 1, - detail::kDefaultExpires, - detail::kDefaultEffectiveOverlap, - 60 * 24}}); // max of 24 hours + {{.path = "/validators2/refresh/2000", + .msg = "", + .ssl = ssl, + .failFetch = false, + .failApply = false, + .serverVersion = 1, + .expiresFromNow = detail::kDefaultExpires, + .effectiveOverlap = detail::kDefaultEffectiveOverlap, + .expectedRefreshMin = 60 * 24}}); // max of 24 hours } using namespace boost::filesystem; for (auto const& file : directory_iterator(good.subdir())) diff --git a/src/test/basics/PerfLog_test.cpp b/src/test/basics/PerfLog_test.cpp index c370c241c5..41b5f81f5d 100644 --- a/src/test/basics/PerfLog_test.cpp +++ b/src/test/basics/PerfLog_test.cpp @@ -619,7 +619,7 @@ public: // Total queued duration is triangle number of (i + 1). BEAST_EXPECT( - jsonToUInt64(total[jss::queued_duration_us]) == (((i * i) + 3 * i + 2) / 2)); + jsonToUInt64(total[jss::queued_duration_us]) == (((i * i) + (3 * i) + 2) / 2)); BEAST_EXPECT(total[jss::running_duration_us] == "0"); } diff --git a/src/test/beast/aged_associative_container_test.cpp b/src/test/beast/aged_associative_container_test.cpp index f2ce72b584..d7f74aaa7d 100644 --- a/src/test/beast/aged_associative_container_test.cpp +++ b/src/test/beast/aged_associative_container_test.cpp @@ -414,11 +414,11 @@ public: // unordered template - std::enable_if_t::type::is_unordered::value> + std::enable_if_t::is_unordered::value> checkUnorderedContentsRefRef(C&& c, Values const& v); template - std::enable_if_t::type::is_unordered::value> + std::enable_if_t::is_unordered::value> checkUnorderedContentsRefRef(C&&, Values const&) { } @@ -641,7 +641,7 @@ AgedAssociativeContainerTestBase::checkMapContents(Container& c, Values const& v // unordered template -std::enable_if_t::type::is_unordered::value> +std::enable_if_t::is_unordered::value> AgedAssociativeContainerTestBase::checkUnorderedContentsRefRef(C&& c, Values const& v) { using Cont = std::remove_reference_t; diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index ce6774827e..92f59fe644 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -1457,14 +1457,14 @@ r.ripple.com:51235 }; std::vector const units = { - {"seconds", 1, 15 * 60, false}, - {"minutes", 60, 14, false}, - {"minutes", 60, 15, true}, - {"hours", 3600, 10, true}, - {"days", 86400, 10, true}, - {"weeks", 604800, 2, true}, - {"months", 2592000, 1, false}, - {"years", 31536000, 1, false}}; + {.unit = "seconds", .numSeconds = 1, .configVal = 15 * 60, .shouldPass = false}, + {.unit = "minutes", .numSeconds = 60, .configVal = 14, .shouldPass = false}, + {.unit = "minutes", .numSeconds = 60, .configVal = 15, .shouldPass = true}, + {.unit = "hours", .numSeconds = 3600, .configVal = 10, .shouldPass = true}, + {.unit = "days", .numSeconds = 86400, .configVal = 10, .shouldPass = true}, + {.unit = "weeks", .numSeconds = 604800, .configVal = 2, .shouldPass = true}, + {.unit = "months", .numSeconds = 2592000, .configVal = 1, .shouldPass = false}, + {.unit = "years", .numSeconds = 31536000, .configVal = 1, .shouldPass = false}}; std::string space; for (auto& [unit, sec, val, shouldPass] : units) diff --git a/src/test/jtx/impl/WSClient.cpp b/src/test/jtx/impl/WSClient.cpp index 551fd1404b..6d069523d2 100644 --- a/src/test/jtx/impl/WSClient.cpp +++ b/src/test/jtx/impl/WSClient.cpp @@ -70,7 +70,7 @@ class WSClientImpl : public WSClient continue; ParsedPort pp; parsePort(pp, cfg[name], log); - if (pp.protocol.count(ps) == 0) + if (!pp.protocol.contains(ps)) continue; using namespace boost::asio::ip; if (pp.ip && pp.ip->is_unspecified()) diff --git a/src/test/jtx/impl/permissioned_dex.cpp b/src/test/jtx/impl/permissioned_dex.cpp index 5c059e9e80..a6b24d7ac6 100644 --- a/src/test/jtx/impl/permissioned_dex.cpp +++ b/src/test/jtx/impl/permissioned_dex.cpp @@ -26,7 +26,7 @@ setupDomain( env.fund(XRP(100000), domainOwner); env.close(); - pdomain::Credentials const credentials{{domainOwner, credType}}; + pdomain::Credentials const credentials{{.issuer = domainOwner, .credType = credType}}; env(pdomain::setTx(domainOwner, credentials)); auto const objects = pdomain::getObjects(domainOwner, env); diff --git a/src/test/jtx/impl/permissioned_domains.cpp b/src/test/jtx/impl/permissioned_domains.cpp index 690451c7d8..385008be43 100644 --- a/src/test/jtx/impl/permissioned_domains.cpp +++ b/src/test/jtx/impl/permissioned_domains.cpp @@ -130,7 +130,9 @@ credentialsFromJson( auto const& credentialType = obj["CredentialType"]; // NOLINTNEXTLINE(bugprone-unchecked-optional-access): used only in tests auto blob = strUnHex(credentialType.asString()).value(); - ret.push_back({human2Acc.at(issuer.asString()), std::string(blob.begin(), blob.end())}); + ret.push_back( + {.issuer = human2Acc.at(issuer.asString()), + .credType = std::string(blob.begin(), blob.end())}); } return ret; } diff --git a/src/test/nodestore/import_test.cpp b/src/test/nodestore/import_test.cpp index a80b5ccc93..de99edd655 100644 --- a/src/test/nodestore/import_test.cpp +++ b/src/test/nodestore/import_test.cpp @@ -297,17 +297,17 @@ public: auto const args = parseArgs(arg()); bool usage = args.empty(); - if (!usage && args.find("from") == args.end()) + if (!usage && !args.contains("from")) { log << "Missing parameter: from"; usage = true; } - if (!usage && args.find("to") == args.end()) + if (!usage && !args.contains("to")) { log << "Missing parameter: to"; usage = true; } - if (!usage && args.find("buffer") == args.end()) + if (!usage && !args.contains("buffer")) { log << "Missing parameter: buffer"; usage = true; diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 4307b7ab7f..31b20b37d4 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -692,11 +692,11 @@ public: { std::string const credentialType1 = "credential1"; - Account issuer("issuer"); + Account const issuer("issuer"); env.fund(XRP(5000), issuer); // gw creates an PermissionedDomain. - env(pdomain::setTx(gw, {{issuer, credentialType1}})); + env(pdomain::setTx(gw, {{.issuer = issuer, .credType = credentialType1}})); env.close(); // Find the PermissionedDomain. diff --git a/src/test/rpc/DepositAuthorized_test.cpp b/src/test/rpc/DepositAuthorized_test.cpp index 89053557a7..e6720602c9 100644 --- a/src/test/rpc/DepositAuthorized_test.cpp +++ b/src/test/rpc/DepositAuthorized_test.cpp @@ -324,7 +324,7 @@ public: env.close(); // becky authorize any account recognized by carol to make a payment - env(deposit::authCredentials(becky, {{carol, credType}})); + env(deposit::authCredentials(becky, {{.issuer = carol, .credType = credType}})); env.close(); { @@ -507,7 +507,7 @@ public: env.close(); // becky authorize any account recognized by carol to make a payment - env(deposit::authCredentials(becky, {{carol, credType2}})); + env(deposit::authCredentials(becky, {{.issuer = carol, .credType = credType2}})); env.close(); { diff --git a/src/test/rpc/LedgerEntry_test.cpp b/src/test/rpc/LedgerEntry_test.cpp index d231a2d4a0..dd9eb1c119 100644 --- a/src/test/rpc/LedgerEntry_test.cpp +++ b/src/test/rpc/LedgerEntry_test.cpp @@ -784,8 +784,8 @@ class LedgerEntry_test : public beast::unit_test::Suite env, jss::amm, { - {jss::asset, "malformedRequest"}, - {jss::asset2, "malformedRequest"}, + {.fieldName = jss::asset, .malformedErrorMsg = "malformedRequest"}, + {.fieldName = jss::asset2, .malformedErrorMsg = "malformedRequest"}, }); }; auto getIOU = [&](Env& env) -> PrettyAsset { return alice["USD"]; }; @@ -900,9 +900,9 @@ class LedgerEntry_test : public beast::unit_test::Suite env, jss::credential, { - {jss::subject, "malformedRequest"}, - {jss::issuer, "malformedRequest"}, - {jss::credential_type, "malformedRequest"}, + {.fieldName = jss::subject, .malformedErrorMsg = "malformedRequest"}, + {.fieldName = jss::issuer, .malformedErrorMsg = "malformedRequest"}, + {.fieldName = jss::credential_type, .malformedErrorMsg = "malformedRequest"}, }); } } @@ -954,8 +954,8 @@ class LedgerEntry_test : public beast::unit_test::Suite env, jss::delegate, { - {jss::account, "malformedAddress"}, - {jss::authorize, "malformedAddress"}, + {.fieldName = jss::account, .malformedErrorMsg = "malformedAddress"}, + {.fieldName = jss::authorize, .malformedErrorMsg = "malformedAddress"}, }); } } @@ -1011,8 +1011,10 @@ class LedgerEntry_test : public beast::unit_test::Suite env, jss::deposit_preauth, { - {jss::owner, "malformedOwner"}, - {jss::authorized, "malformedAuthorized", false}, + {.fieldName = jss::owner, .malformedErrorMsg = "malformedOwner"}, + {.fieldName = jss::authorized, + .malformedErrorMsg = "malformedAuthorized", + .required = false}, }); } } @@ -1037,7 +1039,7 @@ class LedgerEntry_test : public beast::unit_test::Suite // Setup Bob with DepositAuth env(fset(bob, asfDepositAuth)); env.close(); - env(deposit::authCredentials(bob, {{issuer, credType}})); + env(deposit::authCredentials(bob, {{.issuer = issuer, .credType = credType}})); env.close(); } @@ -1458,7 +1460,10 @@ class LedgerEntry_test : public beast::unit_test::Suite { // Malformed escrow fields runLedgerEntryTest( - env, jss::escrow, {{jss::owner, "malformedOwner"}, {jss::seq, "malformedSeq"}}); + env, + jss::escrow, + {{.fieldName = jss::owner, .malformedErrorMsg = "malformedOwner"}, + {.fieldName = jss::seq, .malformedErrorMsg = "malformedSeq"}}); } } @@ -1667,7 +1672,8 @@ class LedgerEntry_test : public beast::unit_test::Suite runLedgerEntryTest( env, jss::offer, - {{jss::account, "malformedAddress"}, {jss::seq, "malformedRequest"}}); + {{.fieldName = jss::account, .malformedErrorMsg = "malformedAddress"}, + {.fieldName = jss::seq, .malformedErrorMsg = "malformedRequest"}}); } } @@ -1774,8 +1780,8 @@ class LedgerEntry_test : public beast::unit_test::Suite env, fieldName, { - {jss::accounts, "malformedRequest"}, - {jss::currency, "malformedCurrency"}, + {.fieldName = jss::accounts, .malformedErrorMsg = "malformedRequest"}, + {.fieldName = jss::currency, .malformedErrorMsg = "malformedCurrency"}, }); } { @@ -1955,8 +1961,8 @@ class LedgerEntry_test : public beast::unit_test::Suite env, jss::ticket, { - {jss::account, "malformedAddress"}, - {jss::ticket_seq, "malformedRequest"}, + {.fieldName = jss::account, .malformedErrorMsg = "malformedAddress"}, + {.fieldName = jss::ticket_seq, .malformedErrorMsg = "malformedRequest"}, }); } } @@ -2034,8 +2040,9 @@ class LedgerEntry_test : public beast::unit_test::Suite env, jss::oracle, { - {jss::account, "malformedAccount"}, - {jss::oracle_document_id, "malformedDocumentID"}, + {.fieldName = jss::account, .malformedErrorMsg = "malformedAccount"}, + {.fieldName = jss::oracle_document_id, + .malformedErrorMsg = "malformedDocumentID"}, }); } } @@ -2172,7 +2179,7 @@ class LedgerEntry_test : public beast::unit_test::Suite env.close(); auto const seq = env.seq(alice); - env(pdomain::setTx(alice, {{alice, "first credential"}})); + env(pdomain::setTx(alice, {{.issuer = alice, .credType = "first credential"}})); env.close(); auto const objects = pdomain::getObjects(alice, env); if (!BEAST_EXPECT(objects.size() == 1)) @@ -2221,8 +2228,8 @@ class LedgerEntry_test : public beast::unit_test::Suite env, jss::permissioned_domain, { - {jss::account, "malformedAddress"}, - {jss::seq, "malformedRequest"}, + {.fieldName = jss::account, .malformedErrorMsg = "malformedAddress"}, + {.fieldName = jss::seq, .malformedErrorMsg = "malformedRequest"}, }); } } diff --git a/src/xrpld/app/misc/detail/ValidatorList.cpp b/src/xrpld/app/misc/detail/ValidatorList.cpp index 57b65814e1..0981c32050 100644 --- a/src/xrpld/app/misc/detail/ValidatorList.cpp +++ b/src/xrpld/app/misc/detail/ValidatorList.cpp @@ -455,7 +455,7 @@ ValidatorList::parseBlobs(std::uint32_t version, json::Value const& body) std::vector ValidatorList::parseBlobs(protocol::TMValidatorList const& body) { - return {{body.blob(), body.signature(), {}}}; + return {{.blob = body.blob(), .signature = body.signature(), .manifest = {}}}; } // static diff --git a/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp b/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp index fcf30fa4f4..5d276dc9c5 100644 --- a/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp +++ b/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp @@ -18,7 +18,8 @@ Config::Config() : outPeers(calcOutPeers()) std::size_t Config::calcOutPeers() const { - return std::max((maxPeers * Tuning::kOutPercent + 50) / 100, std::size_t(Tuning::kMinOutCount)); + return std::max( + ((maxPeers * Tuning::kOutPercent) + 50) / 100, std::size_t(Tuning::kMinOutCount)); } void diff --git a/src/xrpld/rpc/detail/Pathfinder.cpp b/src/xrpld/rpc/detail/Pathfinder.cpp index daa50cfb07..25da86ef8f 100644 --- a/src/xrpld/rpc/detail/Pathfinder.cpp +++ b/src/xrpld/rpc/detail/Pathfinder.cpp @@ -568,7 +568,11 @@ Pathfinder::rankPaths( JLOG(j_.debug()) << "findPaths: quality: " << uQuality << ": " << currentPath.getJson(JsonOptions::Values::None); - rankedPaths.push_back({uQuality, currentPath.size(), liquidity, i}); + rankedPaths.push_back( + {.quality = uQuality, + .length = currentPath.size(), + .liquidity = liquidity, + .index = i}); } } } @@ -1373,7 +1377,7 @@ fillPaths(Pathfinder::PaymentType type, PathCostList const& costs) auto& list = gPathTable[type]; XRPL_ASSERT(list.empty(), "xrpl::fillPaths : empty paths"); for (auto& cost : costs) - list.push_back({cost.cost, makePath(cost.path)}); + list.push_back({.searchLevel = cost.cost, .type = makePath(cost.path)}); } } // namespace @@ -1396,58 +1400,58 @@ Pathfinder::initPathTable() fillPaths( PaymentType::XrpToNonXrp, - {{1, "sfd"}, // source -> book -> gateway - {3, "sfad"}, // source -> book -> account -> destination - {5, "sfaad"}, // source -> book -> account -> account -> destination - {6, "sbfd"}, // source -> book -> book -> destination - {8, "sbafd"}, // source -> book -> account -> book -> destination - {9, "sbfad"}, // source -> book -> book -> account -> destination - {10, "sbafad"}}); + {{.cost = 1, .path = "sfd"}, // source -> book -> gateway + {.cost = 3, .path = "sfad"}, // source -> book -> account -> destination + {.cost = 5, .path = "sfaad"}, // source -> book -> account -> account -> destination + {.cost = 6, .path = "sbfd"}, // source -> book -> book -> destination + {.cost = 8, .path = "sbafd"}, // source -> book -> account -> book -> destination + {.cost = 9, .path = "sbfad"}, // source -> book -> book -> account -> destination + {.cost = 10, .path = "sbafad"}}); fillPaths( PaymentType::NonXrpToXrp, - {{1, "sxd"}, // gateway buys XRP - {2, "saxd"}, // source -> gateway -> book(XRP) -> dest - {6, "saaxd"}, - {7, "sbxd"}, - {8, "sabxd"}, - {9, "sabaxd"}}); + {{.cost = 1, .path = "sxd"}, // gateway buys XRP + {.cost = 2, .path = "saxd"}, // source -> gateway -> book(XRP) -> dest + {.cost = 6, .path = "saaxd"}, + {.cost = 7, .path = "sbxd"}, + {.cost = 8, .path = "sabxd"}, + {.cost = 9, .path = "sabaxd"}}); // non-XRP to non-XRP (same currency) fillPaths( PaymentType::NonXrpToSame, { - {1, "sad"}, // source -> gateway -> destination - {1, "sfd"}, // source -> book -> destination - {4, "safd"}, // source -> gateway -> book -> destination - {4, "sfad"}, - {5, "saad"}, - {5, "sbfd"}, - {6, "sxfad"}, - {6, "safad"}, - {6, "saxfd"}, // source -> gateway -> book to XRP -> book -> - // destination - {6, "saxfad"}, - {6, "sabfd"}, // source -> gateway -> book -> book -> destination - {7, "saaad"}, + {.cost = 1, .path = "sad"}, // source -> gateway -> destination + {.cost = 1, .path = "sfd"}, // source -> book -> destination + {.cost = 4, .path = "safd"}, // source -> gateway -> book -> destination + {.cost = 4, .path = "sfad"}, + {.cost = 5, .path = "saad"}, + {.cost = 5, .path = "sbfd"}, + {.cost = 6, .path = "sxfad"}, + {.cost = 6, .path = "safad"}, + {.cost = 6, .path = "saxfd"}, // source -> gateway -> book to XRP -> book -> + // destination + {.cost = 6, .path = "saxfad"}, + {.cost = 6, .path = "sabfd"}, // source -> gateway -> book -> book -> destination + {.cost = 7, .path = "saaad"}, }); // non-XRP to non-XRP (different currency) fillPaths( PaymentType::NonXrpToNonXrp, { - {1, "sfad"}, - {1, "safd"}, - {3, "safad"}, - {4, "sxfd"}, - {5, "saxfd"}, - {5, "sxfad"}, - {5, "sbfd"}, - {6, "saxfad"}, - {6, "sabfd"}, - {7, "saafd"}, - {8, "saafad"}, - {9, "safaad"}, + {.cost = 1, .path = "sfad"}, + {.cost = 1, .path = "safd"}, + {.cost = 3, .path = "safad"}, + {.cost = 4, .path = "sxfd"}, + {.cost = 5, .path = "saxfd"}, + {.cost = 5, .path = "sxfad"}, + {.cost = 5, .path = "sbfd"}, + {.cost = 6, .path = "saxfad"}, + {.cost = 6, .path = "sabfd"}, + {.cost = 7, .path = "saafd"}, + {.cost = 8, .path = "saafad"}, + {.cost = 9, .path = "safaad"}, }); /* cspell: enable */ } diff --git a/src/xrpld/rpc/detail/ServerHandler.cpp b/src/xrpld/rpc/detail/ServerHandler.cpp index 0bdece3ec3..c73e474e18 100644 --- a/src/xrpld/rpc/detail/ServerHandler.cpp +++ b/src/xrpld/rpc/detail/ServerHandler.cpp @@ -165,10 +165,10 @@ ServerHandler::setup(Setup const& setup, beast::Journal journal) port.port = endpointPort; if ((setup_.client.port == 0u) && - (port.protocol.count("http") > 0 || port.protocol.count("https") > 0)) + (port.protocol.contains("http") || port.protocol.contains("https"))) setup_.client.port = endpointPort; - if ((setup_.overlay.port() == 0u) && (port.protocol.count("peer") > 0)) + if ((setup_.overlay.port() == 0u) && (port.protocol.contains("peer"))) setup_.overlay.port(endpointPort); } } @@ -217,7 +217,7 @@ ServerHandler::onHandoff( using namespace boost::beast; auto const& p{session.port().protocol}; bool const isWs{ - p.count("ws") > 0 || p.count("ws2") > 0 || p.count("wss") > 0 || p.count("wss2") > 0}; + p.contains("ws") || p.contains("ws2") || p.contains("wss") || p.contains("wss2")}; if (websocket::is_upgrade(request)) { @@ -251,7 +251,7 @@ ServerHandler::onHandoff( return handoff; } - if (bundle && p.count("peer") > 0) + if (bundle && p.contains("peer")) return app_.getOverlay().onHandoff(std::move(bundle), std::move(request), remoteAddress); if (isWs && isStatusRequest(request)) @@ -301,7 +301,7 @@ void ServerHandler::onRequest(Session& session) { // Make sure RPC is enabled on the port - if (session.port().protocol.count("http") == 0 && session.port().protocol.count("https") == 0) + if (!session.port().protocol.contains("http") && !session.port().protocol.contains("https")) { httpReply(403, "Forbidden", makeOutput(session), app_.getJournal("RPC")); session.close(true); @@ -1180,7 +1180,7 @@ parsePorts(Config const& config, std::ostream& log) else { auto const count = std::count_if(result.cbegin(), result.cend(), [](Port const& p) { - return p.protocol.count("peer") != 0; + return p.protocol.contains("peer"); }); if (count > 1) @@ -1203,12 +1203,12 @@ setupClient(ServerHandler::Setup& setup) decltype(setup.ports)::const_iterator iter; for (iter = setup.ports.cbegin(); iter != setup.ports.cend(); ++iter) { - if (iter->protocol.count("http") > 0 || iter->protocol.count("https") > 0) + if (iter->protocol.contains("http") || iter->protocol.contains("https")) break; } if (iter == setup.ports.cend()) return; - setup.client.secure = iter->protocol.count("https") > 0; + setup.client.secure = iter->protocol.contains("https"); if (beast::IP::isUnspecified(iter->ip)) { // VFALCO HACK! to make localhost work @@ -1230,7 +1230,7 @@ static void setupOverlay(ServerHandler::Setup& setup) { auto const iter = std::ranges::find_if( - setup.ports, [](Port const& port) { return port.protocol.count("peer") != 0; }); + setup.ports, [](Port const& port) { return port.protocol.contains("peer"); }); if (iter == setup.ports.cend()) { setup.overlay = {}; From 6571f75d399e3e237fbcb1ddfe23adbe3d98286c Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Fri, 5 Jun 2026 15:36:05 +0100 Subject: [PATCH 03/12] ci: Use multiple directories in dependabot config (#7413) --- .github/dependabot.yml | 40 ++++++---------------------------------- 1 file changed, 6 insertions(+), 34 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 0e6b840fe7..da7a30dc77 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,40 +1,12 @@ version: 2 updates: - package-ecosystem: github-actions - directory: / - schedule: - interval: weekly - day: monday - time: "04:00" - timezone: Etc/GMT - commit-message: - prefix: "ci: [DEPENDABOT] " - target-branch: develop - - - package-ecosystem: github-actions - directory: .github/actions/build-deps/ - schedule: - interval: weekly - day: monday - time: "04:00" - timezone: Etc/GMT - commit-message: - prefix: "ci: [DEPENDABOT] " - target-branch: develop - - - package-ecosystem: github-actions - directory: .github/actions/generate-version/ - schedule: - interval: weekly - day: monday - time: "04:00" - timezone: Etc/GMT - commit-message: - prefix: "ci: [DEPENDABOT] " - target-branch: develop - - - package-ecosystem: github-actions - directory: .github/actions/setup-conan/ + directories: + - / + - .github/actions/build-deps/ + - .github/actions/generate-version/ + - .github/actions/set-compiler-env/ + - .github/actions/setup-conan/ schedule: interval: weekly day: monday From 74c66d0944f9da3dd94ac670e7b6c5524f15af65 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Fri, 5 Jun 2026 12:06:41 -0400 Subject: [PATCH 04/12] refactor: Construct Number::Guard from MantissaRange or relevant fields - Simplifies the function signatures in Guard, because it doesn't need to have those values passed in constantly. - Also simplifies some of the functions because they don't need to store values just to pass them to Guard functions. --- include/xrpl/basics/Number.h | 14 ++- src/libxrpl/basics/Number.cpp | 194 ++++++++++++++-------------------- 2 files changed, 86 insertions(+), 122 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 8043a850a8..dd74208485 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -147,7 +147,7 @@ struct MantissaRange final int const log{getExponent(scale)}; rep const min{getMin(scale, log)}; rep const max{(min * 10) - 1}; - CuspRoundingFix const cuspRoundingFixEnabled{isCuspFixEnabled(scale)}; + CuspRoundingFix const cuspRoundingFix{isCuspFixEnabled(scale)}; static MantissaRange const& getMantissaRange(MantissaScale scale); @@ -549,9 +549,15 @@ private: // changing the values inside the range. static thread_local std::reference_wrapper kRange; + class Guard; + void normalize(MantissaRange const& range); + // Guard has the fields that we need, as well as MantissaRange, so if we have a guard, use that + void + normalize(Guard const& guard); + /** Normalize Number components to an arbitrary range. * * min/maxMantissa are parameters because this function is used by both @@ -566,7 +572,7 @@ private: int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); + MantissaRange::CuspRoundingFix cuspRoundingFix); template friend void @@ -576,7 +582,7 @@ private: int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + MantissaRange::CuspRoundingFix cuspRoundingFix, bool dropped); [[nodiscard]] bool @@ -594,8 +600,6 @@ private: // UB, and can vary across compilers. static internalrep externalToInternal(rep mantissa); - - class Guard; }; constexpr Number::Number(bool negative, internalrep mantissa, int exponent, Unchecked) noexcept diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 8e3887e475..7717495cbe 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -65,7 +65,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 15); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max < Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Disabled); } { [[maybe_unused]] @@ -76,7 +76,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Disabled); } { [[maybe_unused]] @@ -87,7 +87,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Enabled); } return map; }(); @@ -171,7 +171,21 @@ class Number::Guard std::uint8_t sbit_ : 1 {0}; // the sign of the guard digits public: - explicit Guard() = default; + internalrep const minMantissa_; + internalrep const maxMantissa_; + MantissaRange::CuspRoundingFix const cuspRoundingFix_; + + explicit Guard( + internalrep const& minMantissa, + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFix) + : minMantissa_(minMantissa), maxMantissa_(maxMantissa), cuspRoundingFix_(cuspRoundingFix) + { + } + + explicit Guard(MantissaRange const& range) : Guard(range.min, range.max, range.cuspRoundingFix) + { + } // set & test the sign bit void @@ -221,19 +235,12 @@ public: // Modify the result to the correctly rounded value template void - doRoundUp( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, - std::string location); + doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location); // Modify the result to the correctly rounded value template void - doRoundDown(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); + doRoundDown(bool& negative, T& mantissa, int& exponent); // Modify the result to the correctly rounded value void @@ -245,7 +252,7 @@ private: template void - bringIntoRange(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); + bringIntoRange(bool& negative, T& mantissa, int& exponent); }; inline void @@ -359,15 +366,11 @@ Number::Guard::round() const noexcept template void -Number::Guard::bringIntoRange( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa) +Number::Guard::bringIntoRange(bool& negative, T& mantissa, int& exponent) { // Bring mantissa back into the minMantissa / maxMantissa range AFTER // rounding - if (mantissa < minMantissa) + if (mantissa < minMantissa_) { mantissa *= 10; --exponent; @@ -384,22 +387,15 @@ Number::Guard::bringIntoRange( template void -Number::Guard::doRoundUp( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, - std::string location) +Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location) { auto r = round(); if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { - auto const safeToIncrement = [&maxMantissa](auto const& mantissa) { - return mantissa < maxMantissa && mantissa < kMaxRep; + auto const safeToIncrement = [this](auto const& mantissa) { + return mantissa < maxMantissa_ && mantissa < kMaxRep; }; - if (cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFix_ == MantissaRange::CuspRoundingFix::Enabled) { // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". @@ -420,14 +416,7 @@ Number::Guard::doRoundUp( safeToIncrement(mantissa), "xrpl::Number::Guard::doRoundUp", "can't recurse more than once"); - doRoundUp( - negative, - mantissa, - exponent, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - location); + doRoundUp(negative, mantissa, exponent, location); return; } } @@ -438,7 +427,7 @@ Number::Guard::doRoundUp( ++mantissa; // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". - if (mantissa > maxMantissa || mantissa > kMaxRep) + if (mantissa > maxMantissa_ || mantissa > kMaxRep) { // Don't use doDropDigit here mantissa /= 10; @@ -446,30 +435,26 @@ Number::Guard::doRoundUp( } } } - bringIntoRange(negative, mantissa, exponent, minMantissa); + bringIntoRange(negative, mantissa, exponent); if (exponent > kMaxExponent) Throw(std::string(location)); } template void -Number::Guard::doRoundDown( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa) +Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) { auto r = round(); if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { --mantissa; - if (mantissa < minMantissa) + if (mantissa < minMantissa_) { mantissa *= 10; --exponent; } } - bringIntoRange(negative, mantissa, exponent, minMantissa); + bringIntoRange(negative, mantissa, exponent); } // Modify the result to the correctly rounded value @@ -536,7 +521,7 @@ doNormalize( int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + MantissaRange::CuspRoundingFix cuspRoundingFix, bool dropped) { static constexpr auto kMinExponent = Number::kMinExponent; @@ -559,7 +544,7 @@ doNormalize( m *= 10; --exponent; } - Guard g; + Guard g(minMantissa, maxMantissa, cuspRoundingFix); if (negative) g.setNegative(); if (dropped) @@ -604,14 +589,7 @@ doNormalize( XRPL_ASSERT_PARTS(m <= kMaxRep, "xrpl::doNormalize", "intermediate mantissa fits in int64"); mantissa = m; - g.doRoundUp( - negative, - mantissa, - exponent, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::normalize 2"); + g.doRoundUp(negative, mantissa, exponent, "Number::normalize 2"); XRPL_ASSERT_PARTS( mantissa >= minMantissa && mantissa <= maxMantissa, "xrpl::doNormalize", @@ -626,13 +604,12 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); // LCOV_EXCL_STOP } @@ -644,13 +621,12 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); // LCOV_EXCL_STOP } @@ -662,16 +638,27 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); } void Number::normalize(MantissaRange const& range) { - normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFixEnabled); + normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFix); +} + +void +Number::normalize(Guard const& guard) +{ + normalize( + negative_, + mantissa_, + exponent_, + guard.minMantissa_, + guard.maxMantissa_, + guard.cuspRoundingFix_); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -725,19 +712,20 @@ Number::operator+=(Number const& y) bool const yn = y.negative_; uint128_t ym = y.mantissa_; auto ye = y.exponent_; - Guard g; + Guard g(kRange); - auto const& range = kRange.get(); + auto const& minMantissa = g.minMantissa_; + auto const& maxMantissa = g.maxMantissa_; + auto const cuspRoundingFix = g.cuspRoundingFix_; // Bring the exponents of both values into agreement, so the mantissas are on the same scale // and can be added directly together // expandM / expandE: First try to expand the mantissa and bring the exponent down // shringM / shrinkE: Then shrink the mantissa and bring the exponent up, if necessary - auto const adjust = [&g, &range]( - uint128_t& expandM, int& expandE, uint128_t& shrinkM, int& shrinkE) { + auto const adjust = [&g](uint128_t& expandM, int& expandE, uint128_t& shrinkM, int& shrinkE) { constexpr uint128_t kSafeLimit = kPowerOfTenImpl[37]; - if (range.cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (g.cuspRoundingFix_ == MantissaRange::CuspRoundingFix::Enabled) { while (shrinkE < expandE && shrinkM % 10 == 0) { @@ -774,14 +762,10 @@ Number::operator+=(Number const& y) adjust(xm, xe, ym, ye); } - auto const& minMantissa = range.min; - auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; - if (xn == yn) { xm += ym; - if (range.cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) { while (xm > maxMantissa || xm > kMaxRep) { @@ -795,14 +779,7 @@ Number::operator+=(Number const& y) g.doDropDigit(xm, xe); } } - g.doRoundUp( - xn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::addition overflow"); + g.doRoundUp(xn, xm, xe, "Number::addition overflow"); } else { @@ -822,32 +799,25 @@ Number::operator+=(Number const& y) xm -= g.pop(); --xe; } - g.doRoundDown(xn, xm, xe, minMantissa); - if (range.cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled && xm != 0) + g.doRoundDown(xn, xm, xe); + if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled && xm != 0) { - // make a new guard - Guard g; + // this will be going away + Guard g(kRange); if (xn) g.setNegative(); while (xm > maxMantissa || xm > kMaxRep) { g.doDropDigit(xm, xe); } - g.doRoundUp( - xn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::addition overflow"); + g.doRoundUp(xn, xm, xe, "Number::addition overflow"); } } negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; - normalize(range); + normalize(g); return *this; } @@ -881,14 +851,11 @@ Number::operator*=(Number const& y) auto ze = xe + ye; auto zs = xs * ys; bool zn = (zs == -1); - Guard g; + Guard g(kRange); if (zn) g.setNegative(); - auto const& range = kRange.get(); - auto const& minMantissa = range.min; - auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + auto const& maxMantissa = g.maxMantissa_; while (zm > maxMantissa || zm > kMaxRep) { @@ -897,19 +864,12 @@ Number::operator*=(Number const& y) xm = static_cast(zm); xe = ze; - g.doRoundUp( - zn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::multiplication overflow : exponent is " + std::to_string(xe)); + g.doRoundUp(zn, xm, xe, "Number::multiplication overflow : exponent is " + std::to_string(xe)); negative_ = zn; mantissa_ = xm; exponent_ = xe; - normalize(range); + normalize(g); return *this; } @@ -945,7 +905,7 @@ Number::operator/=(Number const& y) auto const& range = kRange.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + auto const cuspRoundingFix = range.cuspRoundingFix; // Division operates on two large integers (16-digit for small // mantissas, 19-digit for large) using integer math. If the values @@ -1077,14 +1037,14 @@ Number::operator/=(Number const& y) // rounding fix is enabled, flag if there is still // a remainder from stage 2. bool const useTrailingRemainder = - cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled; + cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled; if (useTrailingRemainder) { dropped = partialNumerator % dm != 0; } } } - doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled, dropped); + doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFix, dropped); negative_ = zp; mantissa_ = static_cast(zm); exponent_ = ze; @@ -1098,7 +1058,7 @@ operator rep() const { rep drops = mantissa(); int offset = exponent(); - Guard g; + Guard g(kRange); if (drops != 0) { if (negative_) From 63ffdc39dc6bc1565b7e6437ed6686721c9379c2 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Fri, 5 Jun 2026 18:05:19 +0100 Subject: [PATCH 05/12] ci: Refactor build-related nix / docker / workflows (#7408) --- .github/scripts/strategy-matrix/linux.json | 2 +- .github/workflows/build-nix-image.yml | 109 ------------------ .github/workflows/build-nix-images.yml | 56 +++++++++ .github/workflows/build-packaging-images.yml | 48 ++++++++ .github/workflows/publish-docs.yml | 18 +-- .../workflows/reusable-build-docker-image.yml | 2 +- .../reusable-build-merge-docker-images.yml | 89 ++++++++++++++ .github/workflows/reusable-upload-recipe.yml | 2 +- .../nix.Dockerfile => nix/docker/Dockerfile | 12 +- {docker => nix/docker}/check-tools.sh | 0 .../docker}/install-sanitizer-libs.sh | 0 {docker => nix/docker}/loader-path.sh | 0 .../docker}/test_files/compile-cpp-sources.sh | 0 .../docker}/test_files/cpp_sources/asan.cpp | 0 .../test_files/cpp_sources/regular.cpp | 0 .../docker}/test_files/cpp_sources/tsan.cpp | 0 .../docker}/test_files/cpp_sources/ubsan.cpp | 0 .../docker}/test_files/run-test-binaries.sh | 0 package/Dockerfile | 14 +++ package/README.md | 13 +-- package/install-packaging-tools.sh | 68 +++++++++++ 21 files changed, 295 insertions(+), 138 deletions(-) delete mode 100644 .github/workflows/build-nix-image.yml create mode 100644 .github/workflows/build-nix-images.yml create mode 100644 .github/workflows/build-packaging-images.yml create mode 100644 .github/workflows/reusable-build-merge-docker-images.yml rename docker/nix.Dockerfile => nix/docker/Dockerfile (90%) rename {docker => nix/docker}/check-tools.sh (100%) rename {docker => nix/docker}/install-sanitizer-libs.sh (100%) rename {docker => nix/docker}/loader-path.sh (100%) rename {docker => nix/docker}/test_files/compile-cpp-sources.sh (100%) rename {docker => nix/docker}/test_files/cpp_sources/asan.cpp (100%) rename {docker => nix/docker}/test_files/cpp_sources/regular.cpp (100%) rename {docker => nix/docker}/test_files/cpp_sources/tsan.cpp (100%) rename {docker => nix/docker}/test_files/cpp_sources/ubsan.cpp (100%) rename {docker => nix/docker}/test_files/run-test-binaries.sh (100%) create mode 100644 package/Dockerfile create mode 100755 package/install-packaging-tools.sh diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 3070b8d9f4..7da48a6a25 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -1,5 +1,5 @@ { - "image_tag": "sha-6c54342", + "image_tag": "sha-8abe82e", "configs": { "ubuntu": [ { diff --git a/.github/workflows/build-nix-image.yml b/.github/workflows/build-nix-image.yml deleted file mode 100644 index bae4cfd437..0000000000 --- a/.github/workflows/build-nix-image.yml +++ /dev/null @@ -1,109 +0,0 @@ -name: Build Nix Docker image - -on: - push: - branches: - - develop - paths: - - ".github/workflows/build-nix-image.yml" - - ".github/workflows/reusable-build-docker-image.yml" - - "docker/**" - - "flake.nix" - - "flake.lock" - - "nix/**" - pull_request: - paths: - - ".github/workflows/build-nix-image.yml" - - ".github/workflows/reusable-build-docker-image.yml" - - "docker/**" - - "flake.nix" - - "flake.lock" - - "nix/**" - workflow_dispatch: - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -defaults: - run: - shell: bash - -jobs: - build: - name: Build ${{ matrix.distro.name }} (${{ matrix.target.platform }}) - permissions: - contents: read - packages: write - strategy: - fail-fast: false - matrix: - # The base images are the oldest supported version of each distro - # that we want to build images for. - distro: - - name: nixos - base_image: nixos/nix:latest - - name: ubuntu - base_image: ubuntu:20.04 - - name: rhel - base_image: registry.access.redhat.com/ubi9/ubi:latest - - name: debian - base_image: debian:bookworm - target: - - platform: linux/amd64 - runner: ubuntu-latest - - platform: linux/arm64 - runner: ubuntu-24.04-arm - uses: ./.github/workflows/reusable-build-docker-image.yml - with: - image_name: ghcr.io/xrplf/xrpld/nix-${{ matrix.distro.name }} - dockerfile: docker/nix.Dockerfile - base_image: ${{ matrix.distro.base_image }} - platform: ${{ matrix.target.platform }} - runner: ${{ matrix.target.runner }} - push: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} - - merge: - name: Merge ${{ matrix.distro }} manifest - needs: build - if: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} - runs-on: ubuntu-latest - permissions: - contents: read - packages: write - strategy: - fail-fast: false - matrix: - distro: [nixos, ubuntu, rhel, debian] - env: - IMAGE_NAME: ghcr.io/xrplf/xrpld/nix-${{ matrix.distro }} - - steps: - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0 - - - name: Docker metadata - id: meta - uses: docker/metadata-action@80c7e94dd9b9319bd5eb7a0e0fe9291e23a2a2e9 # v6.1.0 - with: - images: ${{ env.IMAGE_NAME }} - tags: | - type=sha,prefix=sha-,format=short - type=raw,value=latest - - - name: Login to GitHub Container Registry - uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0 - with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Create multi-arch manifests - run: | - for tag in $(jq -cr '.tags[]' <<<"$DOCKER_METADATA_OUTPUT_JSON"); do - docker buildx imagetools create -t "$tag" "${tag}-amd64" "${tag}-arm64" - done - - - name: Inspect image - run: | - docker buildx imagetools inspect "${IMAGE_NAME}:${{ steps.meta.outputs.version }}" diff --git a/.github/workflows/build-nix-images.yml b/.github/workflows/build-nix-images.yml new file mode 100644 index 0000000000..dc02f84e0f --- /dev/null +++ b/.github/workflows/build-nix-images.yml @@ -0,0 +1,56 @@ +name: Build Nix Docker images + +on: + push: + branches: + - develop + paths: + - ".github/workflows/build-nix-images.yml" + - ".github/workflows/reusable-build-docker-image.yml" + - ".github/workflows/reusable-build-merge-docker-images.yml" + - "flake.nix" + - "flake.lock" + - "nix/**" + pull_request: + paths: + - ".github/workflows/build-nix-images.yml" + - ".github/workflows/reusable-build-docker-image.yml" + - ".github/workflows/reusable-build-merge-docker-images.yml" + - "flake.nix" + - "flake.lock" + - "nix/**" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +defaults: + run: + shell: bash + +jobs: + build-merge: + name: Build and push nix-${{ matrix.distro.name }} + permissions: + contents: read + packages: write + strategy: + fail-fast: false + matrix: + # The base images are the oldest supported version of each distro + # that we want to build images for. + distro: + - name: nixos + base_image: nixos/nix:latest + - name: ubuntu + base_image: ubuntu:20.04 + - name: debian + base_image: debian:bookworm + - name: rhel + base_image: registry.access.redhat.com/ubi9/ubi:latest + uses: ./.github/workflows/reusable-build-merge-docker-images.yml + with: + image_name: ghcr.io/xrplf/xrpld/nix-${{ matrix.distro.name }} + dockerfile: nix/docker/Dockerfile + base_image: ${{ matrix.distro.base_image }} diff --git a/.github/workflows/build-packaging-images.yml b/.github/workflows/build-packaging-images.yml new file mode 100644 index 0000000000..a11a16f298 --- /dev/null +++ b/.github/workflows/build-packaging-images.yml @@ -0,0 +1,48 @@ +name: Build packaging Docker images + +on: + push: + branches: + - develop + paths: + - ".github/workflows/build-packaging-images.yml" + - ".github/workflows/reusable-build-docker-image.yml" + - ".github/workflows/reusable-build-merge-docker-images.yml" + - "package/Dockerfile" + - "package/install-packaging-tools.sh" + pull_request: + paths: + - ".github/workflows/build-packaging-images.yml" + - ".github/workflows/reusable-build-docker-image.yml" + - ".github/workflows/reusable-build-merge-docker-images.yml" + - "package/Dockerfile" + - "package/install-packaging-tools.sh" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +defaults: + run: + shell: bash + +jobs: + build-merge: + name: Build and push packaging-${{ matrix.distro.name }} + permissions: + contents: read + packages: write + strategy: + fail-fast: false + matrix: + distro: + - name: debian + base_image: debian:bookworm + - name: rhel + base_image: registry.access.redhat.com/ubi9/ubi:latest + uses: ./.github/workflows/reusable-build-merge-docker-images.yml + with: + image_name: ghcr.io/xrplf/xrpld/packaging-${{ matrix.distro.name }} + dockerfile: package/Dockerfile + base_image: ${{ matrix.distro.base_image }} diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index d619be5543..bbe6ec4592 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -41,7 +41,7 @@ env: jobs: build: runs-on: ubuntu-latest - container: ghcr.io/xrplf/ci/tools-rippled-documentation:sha-a8c7be1 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e steps: - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -57,19 +57,11 @@ jobs: with: subtract: ${{ env.NPROC_SUBTRACT }} - - name: Check configuration - run: | - echo 'Checking path.' - echo ${PATH} | tr ':' '\n' + - name: Print build environment + uses: XRPLF/actions/print-build-env@59dec886e4afb05a1724443af08baccbc045b574 - echo 'Checking environment variables.' - env | sort - - echo 'Checking CMake version.' - cmake --version - - echo 'Checking Doxygen version.' - doxygen --version + - name: Check Doxygen version + run: doxygen --version - name: Build documentation env: diff --git a/.github/workflows/reusable-build-docker-image.yml b/.github/workflows/reusable-build-docker-image.yml index c3795e56fa..5830ef9ece 100644 --- a/.github/workflows/reusable-build-docker-image.yml +++ b/.github/workflows/reusable-build-docker-image.yml @@ -38,7 +38,7 @@ defaults: jobs: build: - name: Build (${{ inputs.platform }}) + name: Build ${{ inputs.platform }} runs-on: ${{ inputs.runner }} permissions: contents: read diff --git a/.github/workflows/reusable-build-merge-docker-images.yml b/.github/workflows/reusable-build-merge-docker-images.yml new file mode 100644 index 0000000000..98deb6ea3f --- /dev/null +++ b/.github/workflows/reusable-build-merge-docker-images.yml @@ -0,0 +1,89 @@ +name: Reusable build and merge Docker image (multi-arch) + +on: + workflow_call: + inputs: + image_name: + description: "Full image name without tag (e.g. 'ghcr.io/xrplf/xrpld/nix-ubuntu')" + required: true + type: string + dockerfile: + description: "Path to the Dockerfile, relative to the repository root" + required: true + type: string + base_image: + description: "Value passed to the Dockerfile as the BASE_IMAGE build arg" + required: true + type: string + +defaults: + run: + shell: bash + +jobs: + build: + name: Build ${{ inputs.image_name }} + permissions: + contents: read + packages: write + + strategy: + fail-fast: false + matrix: + target: + - platform: linux/amd64 + runner: ubuntu-latest + - platform: linux/arm64 + runner: ubuntu-24.04-arm + + uses: ./.github/workflows/reusable-build-docker-image.yml + with: + image_name: ${{ inputs.image_name }} + dockerfile: ${{ inputs.dockerfile }} + base_image: ${{ inputs.base_image }} + platform: ${{ matrix.target.platform }} + runner: ${{ matrix.target.runner }} + push: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} + + merge: + name: Merge ${{ inputs.image_name }} + needs: build + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + + steps: + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0 + + - name: Docker metadata + id: meta + uses: docker/metadata-action@80c7e94dd9b9319bd5eb7a0e0fe9291e23a2a2e9 # v6.1.0 + with: + images: ${{ inputs.image_name }} + tags: | + type=sha,prefix=sha-,format=short + type=raw,value=latest + + - name: Login to GitHub Container Registry + uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0 + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Create multi-arch manifests + if: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} + run: | + for tag in $(jq -cr '.tags[]' <<<"$DOCKER_METADATA_OUTPUT_JSON"); do + docker buildx imagetools create -t "$tag" "${tag}-amd64" "${tag}-arm64" + done + + - name: Inspect image + if: ${{ github.repository == 'XRPLF/rippled' && github.event_name == 'push' }} + env: + IMAGE_NAME: ${{ inputs.image_name }} + IMAGE_VERSION: ${{ steps.meta.outputs.version }} + run: | + docker buildx imagetools inspect "${IMAGE_NAME}:${IMAGE_VERSION}" diff --git a/.github/workflows/reusable-upload-recipe.yml b/.github/workflows/reusable-upload-recipe.yml index d3fe0f356b..b7ec5f9ef7 100644 --- a/.github/workflows/reusable-upload-recipe.yml +++ b/.github/workflows/reusable-upload-recipe.yml @@ -40,7 +40,7 @@ defaults: jobs: upload: runs-on: ubuntu-latest - container: ghcr.io/xrplf/ci/ubuntu-noble:gcc-13-sha-5dd7158 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e steps: - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 diff --git a/docker/nix.Dockerfile b/nix/docker/Dockerfile similarity index 90% rename from docker/nix.Dockerfile rename to nix/docker/Dockerfile index 6248708417..e6df48e18c 100644 --- a/docker/nix.Dockerfile +++ b/nix/docker/Dockerfile @@ -56,7 +56,7 @@ ENV GIT_SSL_CAINFO="/nix/ci-env/etc/ssl/certs/ca-bundle.crt" # Externally-built dynamically-linked ELF binaries hard-code the loader path # (e.g. /lib64/ld-linux-x86-64.so.2) in their PT_INTERP header. Install it # from the Nix store when the base image doesn't already provide one. -COPY docker/loader-path.sh /tmp/loader-path.sh +COPY nix/docker/loader-path.sh /tmp/loader-path.sh RUN < deb, `dnf`/`yum` -> rpm). The image tag is composed as -`ghcr.io/xrplf/ci/{distro}-{version}:{compiler}-{cver}-sha-{image_sha}` — +`ghcr.io/xrplf/xrpld/packaging-:sha-` — the same scheme used by `reusable-build-test.yml`. Bump `image_sha` in `linux.json` and both CI and local builds pick up the new image with no workflow edits. | Package type | Image (derived from `linux.json`) | Tool required | | ------------ | ---------------------------------------------------- | --------------------------------------------------------------- | -| RPM | `ghcr.io/xrplf/ci/rhel-9:gcc-12-sha-` | `rpmbuild` | -| DEB | `ghcr.io/xrplf/ci/ubuntu-jammy:gcc-12-sha-` | `dpkg-buildpackage`, `debhelper (>= 13)`, `dh-sequence-systemd` | +| RPM | `ghcr.io/xrplf/xrpld/packaging-rhel:sha-` | `rpmbuild` | +| DEB | `ghcr.io/xrplf/xrpld/packaging-debian:sha-` | `dpkg-buildpackage`, `debhelper (>= 13)`, `dh-sequence-systemd` | To print the exact image tags for the current `linux.json`: ```bash -./.github/scripts/strategy-matrix/generate.py --packaging --config=.github/scripts/strategy-matrix/linux.json +./.github/scripts/strategy-matrix/generate.py --packaging ``` ## Building packages diff --git a/package/install-packaging-tools.sh b/package/install-packaging-tools.sh new file mode 100755 index 0000000000..a26159a204 --- /dev/null +++ b/package/install-packaging-tools.sh @@ -0,0 +1,68 @@ +#!/bin/bash + +set -euo pipefail + +if [ ! -f /etc/os-release ]; then + echo "ERROR: /etc/os-release not found; cannot detect OS" >&2 + exit 1 +fi + +# shellcheck source=/dev/null +. /etc/os-release + +echo "Detected OS: ${ID} ${VERSION_ID:-}" + +case "${ID}" in + ubuntu | debian | rhel | centos | rocky | almalinux) + echo "Supported OS detected: ${ID}" + ;; + *) + echo "ERROR: unsupported OS '${ID}'. Supported: debian, ubuntu, rhel-family" >&2 + exit 1 + ;; +esac + +function install() { + case "${ID}" in + debian | ubuntu) + apt-get update -y + apt-get install -y --no-install-recommends \ + ca-certificates \ + debhelper \ + debhelper-compat \ + dpkg-dev \ + git + ;; + + rhel | centos | rocky | almalinux) + dnf install -y --setopt=install_weak_deps=False \ + git \ + rpm-build \ + redhat-rpm-config \ + systemd-rpm-macros + ;; + esac +} + +function postinstall() { + # Don't clear cache in non-CI environments + if [ -z "${CI:-}" ]; then + echo "Not running in CI environment; skipping cache cleanup" + return + fi + + case "${ID}" in + debian | ubuntu) + apt-get clean + rm -rf /var/lib/apt/lists/* + ;; + + rhel | centos | rocky | almalinux) + dnf clean -y all + rm -rf /var/cache/dnf/* + ;; + esac +} + +install +postinstall From fc57dab78bd7dc4a9a022c16fd557b18f7a5e571 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 5 Jun 2026 17:17:47 +0000 Subject: [PATCH 06/12] ci: [DEPENDABOT] bump actions/checkout from 6.0.2 to 6.0.3 (#7414) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/check-pr-description.yml | 2 +- .github/workflows/on-pr.yml | 2 +- .github/workflows/publish-docs.yml | 2 +- .github/workflows/reusable-build-docker-image.yml | 2 +- .github/workflows/reusable-build-test-config.yml | 2 +- .github/workflows/reusable-check-levelization.yml | 2 +- .github/workflows/reusable-check-rename.yml | 2 +- .github/workflows/reusable-clang-tidy.yml | 2 +- .github/workflows/reusable-package.yml | 6 +++--- .github/workflows/reusable-strategy-matrix.yml | 2 +- .github/workflows/reusable-upload-recipe.yml | 2 +- .github/workflows/upload-conan-deps.yml | 2 +- 12 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/check-pr-description.yml b/.github/workflows/check-pr-description.yml index ff28220171..a60b83738a 100644 --- a/.github/workflows/check-pr-description.yml +++ b/.github/workflows/check-pr-description.yml @@ -23,7 +23,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Write PR body to file env: diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index db3c8667e5..4b2edeb93d 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -33,7 +33,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Determine changed files # This step checks whether any files have changed that should # cause the next jobs to run. We do it this way rather than diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index bbe6ec4592..35f33b6446 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -44,7 +44,7 @@ jobs: container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Prepare runner uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab diff --git a/.github/workflows/reusable-build-docker-image.yml b/.github/workflows/reusable-build-docker-image.yml index 5830ef9ece..253563c6a5 100644 --- a/.github/workflows/reusable-build-docker-image.yml +++ b/.github/workflows/reusable-build-docker-image.yml @@ -46,7 +46,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Determine arch id: vars diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index e1154f74be..6f0750e3f7 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -110,7 +110,7 @@ jobs: uses: XRPLF/actions/cleanup-workspace@c7d9ce5ebb03c752a354889ecd870cadfc2b1cd4 - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Prepare runner uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab diff --git a/.github/workflows/reusable-check-levelization.yml b/.github/workflows/reusable-check-levelization.yml index b5d57a177a..813c0e1e36 100644 --- a/.github/workflows/reusable-check-levelization.yml +++ b/.github/workflows/reusable-check-levelization.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Check levelization run: python .github/scripts/levelization/generate.py - name: Check for differences diff --git a/.github/workflows/reusable-check-rename.yml b/.github/workflows/reusable-check-rename.yml index 7aa5b80594..5002cc7f40 100644 --- a/.github/workflows/reusable-check-rename.yml +++ b/.github/workflows/reusable-check-rename.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Check definitions run: .github/scripts/rename/definitions.sh . - name: Check copyright notices diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index cfbd8af963..31e06d05eb 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -42,7 +42,7 @@ jobs: issues: write steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Prepare runner uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab diff --git a/.github/workflows/reusable-package.yml b/.github/workflows/reusable-package.yml index 670c01733e..890277d184 100644 --- a/.github/workflows/reusable-package.yml +++ b/.github/workflows/reusable-package.yml @@ -27,7 +27,7 @@ jobs: matrix: ${{ steps.generate.outputs.matrix }} steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 @@ -45,7 +45,7 @@ jobs: version: ${{ steps.version.outputs.version }} steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: sparse-checkout: | .github/actions/generate-version @@ -94,7 +94,7 @@ jobs: systemd-rpm-macros - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Download pre-built binary uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 diff --git a/.github/workflows/reusable-strategy-matrix.yml b/.github/workflows/reusable-strategy-matrix.yml index 16a2b4e336..ea134b43b2 100644 --- a/.github/workflows/reusable-strategy-matrix.yml +++ b/.github/workflows/reusable-strategy-matrix.yml @@ -23,7 +23,7 @@ jobs: matrix: ${{ steps.generate.outputs.matrix }} steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 diff --git a/.github/workflows/reusable-upload-recipe.yml b/.github/workflows/reusable-upload-recipe.yml index b7ec5f9ef7..6e1ea943ca 100644 --- a/.github/workflows/reusable-upload-recipe.yml +++ b/.github/workflows/reusable-upload-recipe.yml @@ -43,7 +43,7 @@ jobs: container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e steps: - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Generate build version number id: version diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 87465b4d3d..6310c90899 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -64,7 +64,7 @@ jobs: uses: XRPLF/actions/cleanup-workspace@c7d9ce5ebb03c752a354889ecd870cadfc2b1cd4 - name: Checkout repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Prepare runner uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab From 949887feb9f32b49829e9c29712697f567b23916 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Fri, 5 Jun 2026 20:24:32 +0100 Subject: [PATCH 07/12] build: Create single test binary xrpl_tests (#7327) --- .gersemi/definitions.cmake | 3 - .github/scripts/rename/cmake.sh | 4 - .../workflows/reusable-build-test-config.yml | 21 ++--- cmake/XrplAddTest.cmake | 22 ----- cmake/XrplCov.cmake | 2 +- src/tests/libxrpl/CMakeLists.txt | 89 ++++++++++--------- src/tests/libxrpl/crypto/main.cpp | 8 -- src/tests/libxrpl/json/main.cpp | 8 -- src/tests/libxrpl/{basics => }/main.cpp | 0 src/tests/libxrpl/net/main.cpp | 8 -- src/tests/libxrpl/tx/main.cpp | 8 -- 11 files changed, 59 insertions(+), 114 deletions(-) delete mode 100644 cmake/XrplAddTest.cmake delete mode 100644 src/tests/libxrpl/crypto/main.cpp delete mode 100644 src/tests/libxrpl/json/main.cpp rename src/tests/libxrpl/{basics => }/main.cpp (100%) delete mode 100644 src/tests/libxrpl/net/main.cpp delete mode 100644 src/tests/libxrpl/tx/main.cpp diff --git a/.gersemi/definitions.cmake b/.gersemi/definitions.cmake index a16e330ffa..245f827f90 100644 --- a/.gersemi/definitions.cmake +++ b/.gersemi/definitions.cmake @@ -11,9 +11,6 @@ endfunction() function(create_symbolic_link target link) endfunction() -function(xrpl_add_test name) -endfunction() - macro(exclude_from_default target_) endmacro() diff --git a/.github/scripts/rename/cmake.sh b/.github/scripts/rename/cmake.sh index 28bf777fed..3539f563e0 100755 --- a/.github/scripts/rename/cmake.sh +++ b/.github/scripts/rename/cmake.sh @@ -43,9 +43,6 @@ pushd "${DIRECTORY}" # Rename the files. find cmake -type f -name 'Rippled*.cmake' -exec bash -c 'mv "${1}" "${1/Rippled/Xrpl}"' - {} \; find cmake -type f -name 'Ripple*.cmake' -exec bash -c 'mv "${1}" "${1/Ripple/Xrpl}"' - {} \; -if [ -e cmake/xrpl_add_test.cmake ]; then - mv cmake/xrpl_add_test.cmake cmake/XrplAddTest.cmake -fi if [ -e include/xrpl/proto/ripple.proto ]; then mv include/xrpl/proto/ripple.proto include/xrpl/proto/xrpl.proto fi @@ -60,7 +57,6 @@ find cmake -type f -name '*.cmake' | while read -r FILE; do done ${SED_COMMAND} -i -E 's/Rippled?/Xrpl/g' CMakeLists.txt ${SED_COMMAND} -i 's/ripple/xrpl/g' CMakeLists.txt -${SED_COMMAND} -i 's/include(xrpl_add_test)/include(XrplAddTest)/' src/tests/libxrpl/CMakeLists.txt ${SED_COMMAND} -i 's/ripple.pb.h/xrpl.pb.h/' include/xrpl/protocol/messages.h ${SED_COMMAND} -i 's/ripple.pb.h/xrpl.pb.h/' BUILD.md ${SED_COMMAND} -i 's/ripple.pb.h/xrpl.pb.h/' BUILD.md diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 6f0750e3f7..c215540b2e 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -236,6 +236,15 @@ jobs: retention-days: 3 if-no-files-found: error + - name: Upload the test binary (Linux) + if: ${{ github.event.repository.visibility == 'public' && runner.os == 'Linux' }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: xrpl_tests-${{ inputs.config_name }} + path: ${{ env.BUILD_DIR }}/xrpl_tests + retention-days: 3 + if-no-files-found: error + - name: Export server definitions if: ${{ runner.os != 'Windows' && !inputs.build_only && env.VOIDSTAR_ENABLED != 'true' }} working-directory: ${{ env.BUILD_DIR }} @@ -286,16 +295,8 @@ jobs: - name: Run the separate tests if: ${{ !inputs.build_only }} - working-directory: ${{ env.BUILD_DIR }} - # Windows locks some of the build files while running tests, and parallel jobs can collide - env: - BUILD_TYPE: ${{ inputs.build_type }} - PARALLELISM: ${{ runner.os == 'Windows' && '1' || steps.nproc.outputs.nproc }} - run: | - ctest \ - --output-on-failure \ - -C "${BUILD_TYPE}" \ - -j "${PARALLELISM}" + working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }} + run: ./xrpl_tests - name: Run the embedded tests if: ${{ !inputs.build_only }} diff --git a/cmake/XrplAddTest.cmake b/cmake/XrplAddTest.cmake deleted file mode 100644 index 2f1209e03c..0000000000 --- a/cmake/XrplAddTest.cmake +++ /dev/null @@ -1,22 +0,0 @@ -include(isolate_headers) - -function(xrpl_add_test name) - set(target ${PROJECT_NAME}.test.${name}) - - file( - GLOB_RECURSE sources - CONFIGURE_DEPENDS - "${CMAKE_CURRENT_SOURCE_DIR}/${name}/*.cpp" - "${CMAKE_CURRENT_SOURCE_DIR}/${name}.cpp" - ) - add_executable(${target} ${ARGN} ${sources}) - - isolate_headers( - ${target} - "${CMAKE_SOURCE_DIR}" - "${CMAKE_SOURCE_DIR}/tests/${name}" - PRIVATE - ) - - add_test(NAME ${target} COMMAND ${target}) -endfunction() diff --git a/cmake/XrplCov.cmake b/cmake/XrplCov.cmake index d81d7e689f..86ba534a88 100644 --- a/cmake/XrplCov.cmake +++ b/cmake/XrplCov.cmake @@ -47,7 +47,7 @@ setup_target_for_coverage_gcovr( "include/xrpl/beast/test" "include/xrpl/beast/unit_test" "${CMAKE_BINARY_DIR}/pb-xrpl.libpb" - DEPENDENCIES xrpld xrpl.tests + DEPENDENCIES xrpld xrpl_tests ) add_code_coverage_to_target(opts INTERFACE) diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index ee07698519..60288e5f20 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/src/tests/libxrpl/CMakeLists.txt @@ -1,51 +1,56 @@ -include(XrplAddTest) +include(GoogleTest) +include(isolate_headers) # Test requirements. find_package(GTest REQUIRED) -# Custom target for all tests defined in this file -add_custom_target(xrpl.tests) - -# Test helpers -add_library(xrpl.helpers.test STATIC) -target_sources( - xrpl.helpers.test - PRIVATE helpers/Account.cpp helpers/TestSink.cpp helpers/TxTest.cpp +# Single combined gtest binary built from the shared test helpers and all test +# modules below. +add_executable( + xrpl_tests + main.cpp + helpers/Account.cpp + helpers/TestSink.cpp + helpers/TxTest.cpp ) -target_include_directories(xrpl.helpers.test PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) -target_link_libraries(xrpl.helpers.test PUBLIC xrpl.libxrpl gtest::gtest) - -# Common library dependencies for the rest of the tests. -add_library(xrpl.imports.test INTERFACE) -target_link_libraries( - xrpl.imports.test - INTERFACE gtest::gtest xrpl.libxrpl xrpl.helpers.test +set_target_properties( + xrpl_tests + PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}" ) +# Lets test sources include the shared helpers as . +target_include_directories(xrpl_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) +target_link_libraries(xrpl_tests PRIVATE GTest::gtest xrpl.libxrpl) -# One test for each module. -xrpl_add_test(basics) -target_link_libraries(xrpl.test.basics PRIVATE xrpl.imports.test) -add_dependencies(xrpl.tests xrpl.test.basics) - -xrpl_add_test(crypto) -target_link_libraries(xrpl.test.crypto PRIVATE xrpl.imports.test) -add_dependencies(xrpl.tests xrpl.test.crypto) - -xrpl_add_test(json) -target_link_libraries(xrpl.test.json PRIVATE xrpl.imports.test) -add_dependencies(xrpl.tests xrpl.test.json) - -xrpl_add_test(tx) -target_link_libraries(xrpl.test.tx PRIVATE xrpl.imports.test) -add_dependencies(xrpl.tests xrpl.test.tx) - -xrpl_add_test(protocol_autogen) -target_link_libraries(xrpl.test.protocol_autogen PRIVATE xrpl.imports.test) -add_dependencies(xrpl.tests xrpl.test.protocol_autogen) - -# Network unit tests are currently not supported on Windows +# One source subdirectory per module. Network unit tests are currently not +# supported on Windows. +set(test_modules + basics + crypto + json + tx + protocol_autogen +) if(NOT WIN32) - xrpl_add_test(net) - target_link_libraries(xrpl.test.net PRIVATE xrpl.imports.test) - add_dependencies(xrpl.tests xrpl.test.net) + list(APPEND test_modules net) endif() + +foreach(module IN LISTS test_modules) + # Append the module's sources (${module}/*.cpp and ${module}.cpp, if any). + file( + GLOB_RECURSE sources + CONFIGURE_DEPENDS + "${CMAKE_CURRENT_SOURCE_DIR}/${module}/*.cpp" + "${CMAKE_CURRENT_SOURCE_DIR}/${module}.cpp" + ) + target_sources(xrpl_tests PRIVATE ${sources}) + + # Expose the module's private headers under their canonical include path. + isolate_headers( + xrpl_tests + "${CMAKE_SOURCE_DIR}" + "${CMAKE_SOURCE_DIR}/tests/${module}" + PRIVATE + ) +endforeach() + +gtest_discover_tests(xrpl_tests) diff --git a/src/tests/libxrpl/crypto/main.cpp b/src/tests/libxrpl/crypto/main.cpp deleted file mode 100644 index 5142bbe08a..0000000000 --- a/src/tests/libxrpl/crypto/main.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include - -int -main(int argc, char** argv) -{ - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} diff --git a/src/tests/libxrpl/json/main.cpp b/src/tests/libxrpl/json/main.cpp deleted file mode 100644 index 5142bbe08a..0000000000 --- a/src/tests/libxrpl/json/main.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include - -int -main(int argc, char** argv) -{ - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} diff --git a/src/tests/libxrpl/basics/main.cpp b/src/tests/libxrpl/main.cpp similarity index 100% rename from src/tests/libxrpl/basics/main.cpp rename to src/tests/libxrpl/main.cpp diff --git a/src/tests/libxrpl/net/main.cpp b/src/tests/libxrpl/net/main.cpp deleted file mode 100644 index 5142bbe08a..0000000000 --- a/src/tests/libxrpl/net/main.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include - -int -main(int argc, char** argv) -{ - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} diff --git a/src/tests/libxrpl/tx/main.cpp b/src/tests/libxrpl/tx/main.cpp deleted file mode 100644 index 5142bbe08a..0000000000 --- a/src/tests/libxrpl/tx/main.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include - -int -main(int argc, char** argv) -{ - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} From 012c67a7eb8ed2ec3f12e770ebaec32fa8282404 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Fri, 5 Jun 2026 18:02:45 -0400 Subject: [PATCH 08/12] Rework subtraction rounding (again) for more accuracy - Go back to the old method of computing the mantissa, but when post processing, expand the mantissa to slightly larger than maxMantissa, then in doRoundDown, if the result is not exact, subtract one. Finally, let doNormalize figure out the rounding of the result. --- include/xrpl/basics/Number.h | 20 ++++ src/libxrpl/basics/Number.cpp | 133 +++++++++++++--------- src/test/basics/Number_test.cpp | 190 +++++++++++++++++++++++++------- 3 files changed, 254 insertions(+), 89 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index dd74208485..61b63fe234 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -877,6 +877,26 @@ to_string(MantissaRange::MantissaScale const& scale) } } +inline std::string +to_string(Number::RoundingMode const& round) +{ + switch (round) + { + enum class RoundingMode { ToNearest, TowardsZero, Downward, Upward }; + + case Number::RoundingMode::ToNearest: + return "ToNearest"; + case Number::RoundingMode::TowardsZero: + return "TowardsZero"; + case Number::RoundingMode::Downward: + return "Downward"; + case Number::RoundingMode::Upward: + return "Upward"; + default: + throw std::runtime_error("Bad rounding mode"); + } +} + class SaveNumberRoundMode { Number::RoundingMode mode_; diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 7717495cbe..71feed822d 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -208,6 +208,10 @@ public: unsigned pop() noexcept; + // if true, there are no more digits to recover with pop() + bool + empty() const noexcept; + /** Drop a digit from the mantissa, and increment the exponent, storing the dropped digit in * this Guard. * @@ -221,8 +225,17 @@ public: doDropDigit(T& mantissa, int& exponent) noexcept; enum class Round { + // The result is exact. No rounding is needed. + Exact = -2, + // Round down. Since we use integer math, that usually means no change is needed. + // Exceptions are for when the result is between kMaxRap and kMaxRepUp (round to kMaxRep), + // or after subtraction where _any_ remainder will modify the result. The latter is what + // distinguishes Exact from Down. Down = -1, + // The result was exactly half-way between two integers. This will round to whichever of + // the two is even. Even = 0, + // Round up. Always adds 1 (or subtracts 1 in some cases if cuspRoundingFix is not enabled) Up = 1, }; @@ -302,6 +315,13 @@ Number::Guard::pop() noexcept return d; } +// if true, there are no more digits to recover with pop() +inline bool +Number::Guard::empty() const noexcept +{ + return digits_ == 0 && !xbit_; +} + template void Number::Guard::doDropDigit(T& mantissa, int& exponent) noexcept @@ -332,6 +352,12 @@ Number::Guard::round() const noexcept { auto mode = Number::getround(); + if (cuspRoundingFix_ != MantissaRange::CuspRoundingFix::Disabled && empty()) + { + // No remainder + return Round::Exact; + } + if (mode == RoundingMode::TowardsZero) return Round::Down; @@ -445,13 +471,31 @@ void Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) { auto r = round(); - if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) + if (cuspRoundingFix_ != MantissaRange::CuspRoundingFix::Disabled) { - --mantissa; - if (mantissa < minMantissa_) + // If there was any remainder, subtract 1 from the result, and pad with 9s. + // Example with 4 digit mantissas: + // 1000 - 0.0000000001 = 999.9999999999 + // In operator+=, the result will be: 1000, with Guard holding (0, xbit=true) + // * Rounding away from zero, the result should be 1000 + // * Rounding towards zero, the result should be 999.9 + // * Rounding to nearest, the result should be 999.9 + // The most accurate result is always 999.9 + if (r != Round::Exact) { - mantissa *= 10; - --exponent; + --mantissa; + } + } + else + { + if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) + { + --mantissa; + if (mantissa < minMantissa_) + { + mantissa *= 10; + --exponent; + } } } bringIntoRange(negative, mantissa, exponent); @@ -720,46 +764,24 @@ Number::operator+=(Number const& y) // Bring the exponents of both values into agreement, so the mantissas are on the same scale // and can be added directly together - // expandM / expandE: First try to expand the mantissa and bring the exponent down - // shringM / shrinkE: Then shrink the mantissa and bring the exponent up, if necessary - auto const adjust = [&g](uint128_t& expandM, int& expandE, uint128_t& shrinkM, int& shrinkE) { - constexpr uint128_t kSafeLimit = kPowerOfTenImpl[37]; - - if (g.cuspRoundingFix_ == MantissaRange::CuspRoundingFix::Enabled) - { - while (shrinkE < expandE && shrinkM % 10 == 0) - { - g.doDropDigit(shrinkM, shrinkE); - } - - // We've got 128 bits of mantissa to work with here. Don't throw away data unless we - // have to - while (shrinkE < expandE && expandE > kMinExponent && expandM < kSafeLimit) - { - expandM *= 10; - --expandE; - } - } - - while (shrinkE < expandE) - { - g.doDropDigit(shrinkM, shrinkE); - } - }; - + // Then shrink the mantissa and bring the exponent up of the value with the lower exponent if (xe < ye) { if (xn) g.setNegative(); - - adjust(ym, ye, xm, xe); + do + { + g.doDropDigit(xm, xe); + } while (xe < ye); } else if (xe > ye) { if (yn) g.setNegative(); - - adjust(xm, xe, ym, ye); + do + { + g.doDropDigit(ym, ye); + } while (xe > ye); } if (xn == yn) @@ -793,31 +815,38 @@ Number::operator+=(Number const& y) xe = ye; xn = yn; } - while (xm < minMantissa && xm * 10 <= kMaxRep) + if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) { - xm *= 10; - xm -= g.pop(); - --xe; - } - g.doRoundDown(xn, xm, xe); - if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled && xm != 0) - { - // this will be going away - Guard g(kRange); - if (xn) - g.setNegative(); - while (xm > maxMantissa || xm > kMaxRep) + // Grow xm/xe and pull digits out of the Guard until it's just past the range, so that + // normalize will have enough information to make an accurate rounding decision, but + // stop if the Guard empties out. Note that if the xbit is set, the Guard will never be + // empty. + while (xm <= maxMantissa && !g.empty()) { - g.doDropDigit(xm, xe); + xm *= 10; + xm -= g.pop(); + --xe; } - g.doRoundUp(xn, xm, xe, "Number::addition overflow"); } + else + { + // Grow xm/xe and pull digits out of the Guard until it's back in range. + while (xm < minMantissa && xm * 10 <= kMaxRep) + { + xm *= 10; + xm -= g.pop(); + --xe; + } + } + // Round down, based on whether there is any data left in the Guard (depending on + // cuspRoundingFix) + g.doRoundDown(xn, xm, xe); } + doNormalize(xn, xm, xe, minMantissa, maxMantissa, cuspRoundingFix, false); negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; - normalize(g); return *this; } diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 24cf5d6967..9e928f4e9f 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -43,12 +43,17 @@ class Number_test : public beast::unit_test::Suite return out; } - static BigInt + BigInt toBigInt(Number const& n) { BigInt v = n.mantissa(); for (int i = 0; i < n.exponent(); ++i) v *= 10; + for (int i = 0; i > n.exponent(); --i) + { + BEAST_EXPECT(v % 10 == 0); + v /= 10; + } return v; } @@ -1923,49 +1928,160 @@ public: } { - testcase << "operator+ TowardsZero rounds away from zero " << to_string(scale); + testcase << "subtraction rounding " << to_string(scale); - Number const a{1LL, 20}; - Number const b{-1'000'000'000'000'000'001LL}; - - BEAST_EXPECT(toBigInt(a) == BigInt{"100000000000000000000"}); - if (scale != MantissaRange::MantissaScale::Small) - { - BEAST_EXPECT(toBigInt(b) == BigInt{"-1000000000000000001"}); - } - else - { - BEAST_EXPECT(toBigInt(b) == BigInt{"-1000000000000000000"}); - } - - Number sum; - { - NumberRoundModeGuard const roundGuard{Number::RoundingMode::TowardsZero}; - sum = a + b; - } - - BigInt const exact = toBigInt(a) + toBigInt(b); - BigInt const stored = toBigInt(sum); - BigInt const diff = stored - exact; - - log << "\n a = " << a << "\n b = " << b - << "\n exact a + b = " << exact.str() << "\n TowardsZero = " << stored.str() - << "\n difference = " << diff.str() << "\n"; - log.flush(); + auto const exp = Number::mantissaLog(); + Number const a{1LL, exp + 2}; + Number const b{-(Number{1, exp} + 1)}; if (scale == MantissaRange::MantissaScale::Small) { - BEAST_EXPECT(stored == exact); - } - else if (scale == MantissaRange::MantissaScale::LargeLegacy) - { - BEAST_EXPECT(stored > exact); + BEAST_EXPECT(toBigInt(a) == BigInt{"100000000000000000"}); + BEAST_EXPECT(toBigInt(b) == BigInt{"-1000000000000001"}); } else { - BEAST_EXPECT(stored < exact); - BEAST_EXPECT(diff < 0); - BEAST_EXPECT(-diff < pow10(sum.exponent())); + BEAST_EXPECT(toBigInt(a) == BigInt{"100000000000000000000"}); + BEAST_EXPECT(toBigInt(b) == BigInt{"-1000000000000000001"}); + } + + auto construct = [&a, &b, this](Number::RoundingMode r) { + NumberRoundModeGuard const roundGuard{r}; + auto const sum = a + b; + BigInt const stored = toBigInt(sum); + return std::make_pair(r, std::make_pair(stored, sum)); + }; + + auto const bigA = toBigInt(a); + auto const bigB = toBigInt(b); + BigInt const exact = bigA + bigB; + + auto const sums = [&]() { + std::map> sums; + sums.emplace(construct(Number::RoundingMode::TowardsZero)); + sums.emplace(construct(Number::RoundingMode::Upward)); + sums.emplace(construct(Number::RoundingMode::Downward)); + sums.emplace(construct(Number::RoundingMode::ToNearest)); + return sums; + }(); + + log << "\n a = " << a << " (" << fmt(bigA) << ")\n b = " << b + << " (" << fmt(bigB) << ")\n exact a + b = " << fmt(exact) << "\n"; + for (auto const& [r, sum] : sums) + { + auto const diff = sum.first - exact; + auto const rLabel = to_string(r); + log << std::string(15 - rLabel.length(), ' ') << rLabel << " = " << fmt(sum.first) + << "\n difference = " << fmt(diff) << "\n"; + } + log.flush(); + + switch (scale) + { + case MantissaRange::MantissaScale::Small: + case MantissaRange::MantissaScale::LargeLegacy: { + // Without the fix, all the results but one round up + BEAST_EXPECT(sums.at(Number::RoundingMode::TowardsZero).first > exact); + BEAST_EXPECT(sums.at(Number::RoundingMode::Upward).first > exact); + BEAST_EXPECT(sums.at(Number::RoundingMode::ToNearest).first > exact); + // Downward works because the Guard sign is negative, and Downward returns Up + // instead of Down if negative and there's a remainder, whereas TowardsZero + // always returns Down. + BEAST_EXPECT(sums.at(Number::RoundingMode::Downward).first < exact); + break; + } + default: { + for (auto const& [r, sum] : sums) + { + auto const epsilon = pow10(sum.second.exponent()); + BEAST_EXPECT(epsilon == 100); + auto diff = sum.first - exact; + switch (r) + { + case Number::RoundingMode::Upward: + case Number::RoundingMode::ToNearest: + BEAST_EXPECT(sum.first > exact); + BEAST_EXPECT(diff < epsilon); + break; + default: + BEAST_EXPECT(sum.first < exact); + BEAST_EXPECT(-diff < epsilon); + } + } + } + } + } + { + auto const offset = 30; + testcase << "subtraction rounding offset of " << offset << " " << to_string(scale); + + auto const exp = Number::mantissaLog(); + Number const a{1LL, exp + offset}; + Number const b{-1}; + + auto construct = [&a, &b, this](Number::RoundingMode r) { + NumberRoundModeGuard const roundGuard{r}; + auto const sum = a + b; + BigInt const stored = toBigInt(sum); + return std::make_pair(r, std::make_pair(stored, sum)); + }; + + auto const bigA = toBigInt(a); + auto const bigB = toBigInt(b); + BigInt const exact = bigA + bigB; + + auto const sums = [&]() { + std::map> sums; + sums.emplace(construct(Number::RoundingMode::TowardsZero)); + sums.emplace(construct(Number::RoundingMode::Upward)); + sums.emplace(construct(Number::RoundingMode::Downward)); + sums.emplace(construct(Number::RoundingMode::ToNearest)); + return sums; + }(); + + log << "\n a = " << a << " (" << fmt(bigA) << ")\n b = " << b + << " (" << fmt(bigB) << ")\n exact a + b = " << fmt(exact) << "\n"; + for (auto const& [r, sum] : sums) + { + auto const diff = sum.first - exact; + auto const rLabel = to_string(r); + log << std::string(15 - rLabel.length(), ' ') << rLabel << " = " << fmt(sum.first) + << "\n difference = " << fmt(diff) << "\n"; + } + log.flush(); + + switch (scale) + { + case MantissaRange::MantissaScale::Small: + case MantissaRange::MantissaScale::LargeLegacy: { + // Without the fix, all the results but one round up + BEAST_EXPECT(sums.at(Number::RoundingMode::TowardsZero).first > exact); + BEAST_EXPECT(sums.at(Number::RoundingMode::Upward).first > exact); + BEAST_EXPECT(sums.at(Number::RoundingMode::ToNearest).first > exact); + // Downward works because the Guard sign is negative, and Downward returns Up + // instead of Down if negative and there's a remainder, whereas TowardsZero + // always returns Down. + BEAST_EXPECT(sums.at(Number::RoundingMode::Downward).first < exact); + break; + } + default: { + for (auto const& [r, sum] : sums) + { + auto const epsilon = pow10(sum.second.exponent()); + auto diff = sum.first - exact; + switch (r) + { + case Number::RoundingMode::Upward: + case Number::RoundingMode::ToNearest: + BEAST_EXPECT(sum.first > exact); + BEAST_EXPECT(diff < epsilon); + break; + default: + BEAST_EXPECT(sum.first < exact); + BEAST_EXPECT(-diff < epsilon); + } + } + } } } } From 961ac6671ecad8136b81ed51ddafc988120e28d3 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Sat, 6 Jun 2026 13:09:52 -0400 Subject: [PATCH 09/12] Improve comment descriptions --- src/libxrpl/basics/Number.cpp | 49 ++++++++++++----------------------- 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 71feed822d..69e48650ee 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -208,7 +208,7 @@ public: unsigned pop() noexcept; - // if true, there are no more digits to recover with pop() + // if true, there are no digits in the guard, including dropped digits (xbit_) bool empty() const noexcept; @@ -225,15 +225,14 @@ public: doDropDigit(T& mantissa, int& exponent) noexcept; enum class Round { - // The result is exact. No rounding is needed. + // The result is exact. No rounding is needed. Only used if cuspRoundingFix is enabled. Exact = -2, // Round down. Since we use integer math, that usually means no change is needed. // Exceptions are for when the result is between kMaxRap and kMaxRepUp (round to kMaxRep), // or after subtraction where _any_ remainder will modify the result. The latter is what // distinguishes Exact from Down. Down = -1, - // The result was exactly half-way between two integers. This will round to whichever of - // the two is even. + // The result was exactly half-way between two integers. This will round to even. Even = 0, // Round up. Always adds 1 (or subtracts 1 in some cases if cuspRoundingFix is not enabled) Up = 1, @@ -315,7 +314,6 @@ Number::Guard::pop() noexcept return d; } -// if true, there are no more digits to recover with pop() inline bool Number::Guard::empty() const noexcept { @@ -473,14 +471,8 @@ Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) auto r = round(); if (cuspRoundingFix_ != MantissaRange::CuspRoundingFix::Disabled) { - // If there was any remainder, subtract 1 from the result, and pad with 9s. - // Example with 4 digit mantissas: - // 1000 - 0.0000000001 = 999.9999999999 - // In operator+=, the result will be: 1000, with Guard holding (0, xbit=true) - // * Rounding away from zero, the result should be 1000 - // * Rounding towards zero, the result should be 999.9 - // * Rounding to nearest, the result should be 999.9 - // The most accurate result is always 999.9 + // If there was any remainder, subtract 1 from the result. This is sufficient to get the + // best rounding. if (r != Round::Exact) { --mantissa; @@ -763,8 +755,9 @@ Number::operator+=(Number const& y) auto const cuspRoundingFix = g.cuspRoundingFix_; // Bring the exponents of both values into agreement, so the mantissas are on the same scale - // and can be added directly together - // Then shrink the mantissa and bring the exponent up of the value with the lower exponent + // and can be added directly together. + // Shrink the mantissa and bring the exponent up of the value with the lower exponent. Store any + // dropped digits in the Guard. if (xe < ye) { if (xn) @@ -787,19 +780,9 @@ Number::operator+=(Number const& y) if (xn == yn) { xm += ym; - if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) + if (xm > maxMantissa || xm > kMaxRep) { - while (xm > maxMantissa || xm > kMaxRep) - { - g.doDropDigit(xm, xe); - } - } - else - { - if (xm > maxMantissa || xm > kMaxRep) - { - g.doDropDigit(xm, xe); - } + g.doDropDigit(xm, xe); } g.doRoundUp(xn, xm, xe, "Number::addition overflow"); } @@ -817,11 +800,13 @@ Number::operator+=(Number const& y) } if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) { - // Grow xm/xe and pull digits out of the Guard until it's just past the range, so that - // normalize will have enough information to make an accurate rounding decision, but - // stop if the Guard empties out. Note that if the xbit is set, the Guard will never be - // empty. - while (xm <= maxMantissa && !g.empty()) + // Grow xm/xe and pull digits out of the Guard until it's a little bit larger than + // maxMantissa, so that normalize will have enough information to make an accurate + // rounding decision, but stop if the Guard empties out, because no rounding will be + // necessary. (Normalize will pad it back into range.) Note that if any digits were lost + // (xbit), the Guard will never be empty, so xm will get big. + auto const upperLimit = static_cast(minMantissa) * 1000; + while (xm < upperLimit && !g.empty()) { xm *= 10; xm -= g.pop(); From 3a5c85067a0251dc4c60c4bf8eb5ddccbdaf851a Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Sat, 6 Jun 2026 14:33:31 -0400 Subject: [PATCH 10/12] Include rounding in failed unit tests --- src/test/basics/Number_test.cpp | 53 +++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 9e928f4e9f..aa57af01b3 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -1981,13 +1981,17 @@ public: case MantissaRange::MantissaScale::Small: case MantissaRange::MantissaScale::LargeLegacy: { // Without the fix, all the results but one round up - BEAST_EXPECT(sums.at(Number::RoundingMode::TowardsZero).first > exact); - BEAST_EXPECT(sums.at(Number::RoundingMode::Upward).first > exact); - BEAST_EXPECT(sums.at(Number::RoundingMode::ToNearest).first > exact); - // Downward works because the Guard sign is negative, and Downward returns Up - // instead of Down if negative and there's a remainder, whereas TowardsZero - // always returns Down. - BEAST_EXPECT(sums.at(Number::RoundingMode::Downward).first < exact); + for (auto const& [r, sum] : sums) { + if (r == Number::RoundingMode::Downward) { + // Downward works because the Guard sign is negative, and Downward returns Up + // instead of Down if negative and there's a remainder, whereas TowardsZero + // always returns Down. + BEAST_EXPECTS(sums.at(Number::RoundingMode::Downward).first < exact, to_string(r)); + } + else { + BEAST_EXPECTS(sums.at(r).first > exact, to_string(r)); + } + } break; } default: { @@ -2000,12 +2004,12 @@ public: { case Number::RoundingMode::Upward: case Number::RoundingMode::ToNearest: - BEAST_EXPECT(sum.first > exact); - BEAST_EXPECT(diff < epsilon); + BEAST_EXPECTS(sum.first > exact, to_string(r)); + BEAST_EXPECTS(diff < epsilon, to_string(r)); break; default: - BEAST_EXPECT(sum.first < exact); - BEAST_EXPECT(-diff < epsilon); + BEAST_EXPECTS(sum.first < exact, to_string(r)); + BEAST_EXPECTS(-diff < epsilon, to_string(r)); } } } @@ -2054,14 +2058,17 @@ public: { case MantissaRange::MantissaScale::Small: case MantissaRange::MantissaScale::LargeLegacy: { - // Without the fix, all the results but one round up - BEAST_EXPECT(sums.at(Number::RoundingMode::TowardsZero).first > exact); - BEAST_EXPECT(sums.at(Number::RoundingMode::Upward).first > exact); - BEAST_EXPECT(sums.at(Number::RoundingMode::ToNearest).first > exact); - // Downward works because the Guard sign is negative, and Downward returns Up - // instead of Down if negative and there's a remainder, whereas TowardsZero - // always returns Down. - BEAST_EXPECT(sums.at(Number::RoundingMode::Downward).first < exact); + for (auto const& [r, sum] : sums) { + if (r == Number::RoundingMode::Downward) { + // Downward works because the Guard sign is negative, and Downward returns Up + // instead of Down if negative and there's a remainder, whereas TowardsZero + // always returns Down. + BEAST_EXPECTS(sums.at(Number::RoundingMode::Downward).first < exact, to_string(r)); + } + else { + BEAST_EXPECTS(sums.at(r).first > exact, to_string(r)); + } + } break; } default: { @@ -2073,12 +2080,12 @@ public: { case Number::RoundingMode::Upward: case Number::RoundingMode::ToNearest: - BEAST_EXPECT(sum.first > exact); - BEAST_EXPECT(diff < epsilon); + BEAST_EXPECTS(sum.first > exact, to_string(r)); + BEAST_EXPECTS(diff < epsilon, to_string(r)); break; default: - BEAST_EXPECT(sum.first < exact); - BEAST_EXPECT(-diff < epsilon); + BEAST_EXPECTS(sum.first < exact, to_string(r)); + BEAST_EXPECTS(-diff < epsilon, to_string(r)); } } } From 8743be8eae2a28b05f3683d3f0164dd50737305e Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Sat, 6 Jun 2026 14:34:31 -0400 Subject: [PATCH 11/12] Rollback Number class changes; show the fix works without side effects --- include/xrpl/basics/Number.h | 38 ++--- src/libxrpl/basics/Number.cpp | 285 +++++++++++++++------------------- 2 files changed, 138 insertions(+), 185 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 61b63fe234..0b4caaa722 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -51,43 +51,37 @@ namespace detail { * compile time. Doing it at runtime would be pretty wasteful and * inefficient. */ -constexpr std::size_t kUint64Digits = 20; -constexpr std::size_t kUint128Digits = 39; - -template -consteval std::array +constexpr std::size_t kInt64Digits = 20; +consteval std::array buildPowersOfTen() { - std::array result{}; + std::array result{}; - T power = 1; + std::uint64_t power = 1; std::size_t exponent = 0; // end the loop early so it doesn't overflow; for (; exponent < result.size() - 1; ++exponent, power *= 10) { result[exponent] = power; - if (power > std::numeric_limits::max() / 10) + if (power > std::numeric_limits::max() / 10) throw std::logic_error("Power of 10 table is too big"); } result[exponent] = power; - if (power < std::numeric_limits::max() / 10) - throw std::logic_error("Power of 10 table is not big enough for the given type"); + if (power < std::numeric_limits::max() / 10) + throw std::logic_error("Power of 10 table is not big enough for the uint64_t type"); return result; } } // namespace detail -template -constexpr std::array kPowerOfTenImpl = detail::buildPowersOfTen(); - -constexpr auto kPowerOfTen = kPowerOfTenImpl; +constexpr std::array kPowerOfTen = detail::buildPowersOfTen(); static_assert(kPowerOfTen[0] == 1); static_assert(kPowerOfTen[1] == 10); static_assert(kPowerOfTen[10] == 10'000'000'000); static_assert( - isPowerOfTen(kPowerOfTen.back()) && *logTen(kPowerOfTen.back()) == detail::kUint64Digits - 1); + isPowerOfTen(kPowerOfTen.back()) && *logTen(kPowerOfTen.back()) == detail::kInt64Digits - 1); /** MantissaRange defines a range for the mantissa of a normalized Number. * @@ -147,7 +141,7 @@ struct MantissaRange final int const log{getExponent(scale)}; rep const min{getMin(scale, log)}; rep const max{(min * 10) - 1}; - CuspRoundingFix const cuspRoundingFix{isCuspFixEnabled(scale)}; + CuspRoundingFix const cuspRoundingFixEnabled{isCuspFixEnabled(scale)}; static MantissaRange const& getMantissaRange(MantissaScale scale); @@ -549,15 +543,9 @@ private: // changing the values inside the range. static thread_local std::reference_wrapper kRange; - class Guard; - void normalize(MantissaRange const& range); - // Guard has the fields that we need, as well as MantissaRange, so if we have a guard, use that - void - normalize(Guard const& guard); - /** Normalize Number components to an arbitrary range. * * min/maxMantissa are parameters because this function is used by both @@ -572,7 +560,7 @@ private: int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFix); + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); template friend void @@ -582,7 +570,7 @@ private: int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFix, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, bool dropped); [[nodiscard]] bool @@ -600,6 +588,8 @@ private: // UB, and can vary across compilers. static internalrep externalToInternal(rep mantissa); + + class Guard; }; constexpr Number::Number(bool negative, internalrep mantissa, int exponent, Unchecked) noexcept diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index d43eebc314..23e913bbdc 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -65,7 +65,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 15); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max < Number::kMaxRep); - static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Disabled); + static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); } { [[maybe_unused]] @@ -76,7 +76,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Disabled); + static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); } { [[maybe_unused]] @@ -87,7 +87,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Enabled); + static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); } return map; }(); @@ -171,21 +171,7 @@ class Number::Guard std::uint8_t sbit_ : 1 {0}; // the sign of the guard digits public: - internalrep const minMantissa_; - internalrep const maxMantissa_; - MantissaRange::CuspRoundingFix const cuspRoundingFix_; - - explicit Guard( - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFix) - : minMantissa_(minMantissa), maxMantissa_(maxMantissa), cuspRoundingFix_(cuspRoundingFix) - { - } - - explicit Guard(MantissaRange const& range) : Guard(range.min, range.max, range.cuspRoundingFix) - { - } + explicit Guard() = default; // set & test the sign bit void @@ -208,10 +194,6 @@ public: unsigned pop() noexcept; - // if true, there are no digits in the guard, including dropped digits (xbit_) - bool - empty() const noexcept; - /** Drop a digit from the mantissa, and increment the exponent, storing the dropped digit in * this Guard. * @@ -224,35 +206,28 @@ public: void doDropDigit(T& mantissa, int& exponent) noexcept; - enum class Round { - // The result is exact. No rounding is needed. Only used if cuspRoundingFix is enabled. - Exact = -2, - // Round down. Since we use integer math, that usually means no change is needed. - // Exceptions are for when the result is between kMaxRap and kMaxRepUp (round to kMaxRep), - // or after subtraction where _any_ remainder will modify the result. The latter is what - // distinguishes Exact from Down. - Down = -1, - // The result was exactly half-way between two integers. This will round to even. - Even = 0, - // Round up. Always adds 1 (or subtracts 1 in some cases if cuspRoundingFix is not enabled) - Up = 1, - }; - // Indicate round direction: 1 is up, -1 is down, 0 is even // This enables the client to round towards nearest, and on // tie, round towards even. - [[nodiscard]] Round + [[nodiscard]] int round() const noexcept; // Modify the result to the correctly rounded value template void - doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location); + doRoundUp( + bool& negative, + T& mantissa, + int& exponent, + internalrep const& minMantissa, + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + std::string location); // Modify the result to the correctly rounded value template void - doRoundDown(bool& negative, T& mantissa, int& exponent); + doRoundDown(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); // Modify the result to the correctly rounded value void @@ -264,7 +239,7 @@ private: template void - bringIntoRange(bool& negative, T& mantissa, int& exponent); + bringIntoRange(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); }; inline void @@ -314,12 +289,6 @@ Number::Guard::pop() noexcept return d; } -inline bool -Number::Guard::empty() const noexcept -{ - return digits_ == 0 && !xbit_; -} - template void Number::Guard::doDropDigit(T& mantissa, int& exponent) noexcept @@ -345,56 +314,54 @@ Number::Guard::doDropDigit(uint128_t& mantissa, int& exponent) noexce // -1 if Guard is less than half // 0 if Guard is exactly half // 1 if Guard is greater than half -Number::Guard::Round +int Number::Guard::round() const noexcept { auto mode = Number::getround(); - if (cuspRoundingFix_ != MantissaRange::CuspRoundingFix::Disabled && empty()) - { - // No remainder - return Round::Exact; - } - if (mode == RoundingMode::TowardsZero) - return Round::Down; + return -1; if (mode == RoundingMode::Downward) { if (sbit_) { if (digits_ > 0 || xbit_) - return Round::Up; + return 1; } - return Round::Down; + return -1; } if (mode == RoundingMode::Upward) { if (sbit_) - return Round::Down; + return -1; if (digits_ > 0 || xbit_) - return Round::Up; - return Round::Down; + return 1; + return -1; } // assume round to nearest if mode is not one of the predefined values if (digits_ > 0x5000'0000'0000'0000) - return Round::Up; + return 1; if (digits_ < 0x5000'0000'0000'0000) - return Round::Down; + return -1; if (xbit_) - return Round::Up; - return Round::Even; + return 1; + return 0; } template void -Number::Guard::bringIntoRange(bool& negative, T& mantissa, int& exponent) +Number::Guard::bringIntoRange( + bool& negative, + T& mantissa, + int& exponent, + internalrep const& minMantissa) { // Bring mantissa back into the minMantissa / maxMantissa range AFTER // rounding - if (mantissa < minMantissa_) + if (mantissa < minMantissa) { mantissa *= 10; --exponent; @@ -411,15 +378,22 @@ Number::Guard::bringIntoRange(bool& negative, T& mantissa, int& exponent) template void -Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location) +Number::Guard::doRoundUp( + bool& negative, + T& mantissa, + int& exponent, + internalrep const& minMantissa, + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + std::string location) { auto r = round(); - if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) + if (r == 1 || (r == 0 && (mantissa & 1) == 1)) { - auto const safeToIncrement = [this](auto const& mantissa) { - return mantissa < maxMantissa_ && mantissa < kMaxRep; + auto const safeToIncrement = [&maxMantissa](auto const& mantissa) { + return mantissa < maxMantissa && mantissa < kMaxRep; }; - if (cuspRoundingFix_ == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) { // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". @@ -440,7 +414,14 @@ Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string safeToIncrement(mantissa), "xrpl::Number::Guard::doRoundUp", "can't recurse more than once"); - doRoundUp(negative, mantissa, exponent, location); + doRoundUp( + negative, + mantissa, + exponent, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + location); return; } } @@ -451,7 +432,7 @@ Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string ++mantissa; // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". - if (mantissa > maxMantissa_ || mantissa > kMaxRep) + if (mantissa > maxMantissa || mantissa > kMaxRep) { // Don't use doDropDigit here mantissa /= 10; @@ -459,38 +440,30 @@ Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string } } } - bringIntoRange(negative, mantissa, exponent); + bringIntoRange(negative, mantissa, exponent, minMantissa); if (exponent > kMaxExponent) Throw(std::string(location)); } template void -Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) +Number::Guard::doRoundDown( + bool& negative, + T& mantissa, + int& exponent, + internalrep const& minMantissa) { auto r = round(); - if (cuspRoundingFix_ != MantissaRange::CuspRoundingFix::Disabled) + if (r == 1 || (r == 0 && (mantissa & 1) == 1)) { - // If there was any remainder, subtract 1 from the result. This is sufficient to get the - // best rounding. - if (r != Round::Exact) + --mantissa; + if (mantissa < minMantissa) { - --mantissa; + mantissa *= 10; + --exponent; } } - else - { - if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) - { - --mantissa; - if (mantissa < minMantissa_) - { - mantissa *= 10; - --exponent; - } - } - } - bringIntoRange(negative, mantissa, exponent); + bringIntoRange(negative, mantissa, exponent, minMantissa); } // Modify the result to the correctly rounded value @@ -498,7 +471,7 @@ void Number::Guard::doRound(rep& drops, std::string location) const { auto r = round(); - if (r == Round::Up || (r == Round::Even && (drops & 1) == 1)) + if (r == 1 || (r == 0 && (drops & 1) == 1)) { if (drops >= kMaxRep) { @@ -557,7 +530,7 @@ doNormalize( int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFix, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, bool dropped) { static constexpr auto kMinExponent = Number::kMinExponent; @@ -580,7 +553,7 @@ doNormalize( m *= 10; --exponent; } - Guard g(minMantissa, maxMantissa, cuspRoundingFix); + Guard g; if (negative) g.setNegative(); if (dropped) @@ -625,7 +598,14 @@ doNormalize( XRPL_ASSERT_PARTS(m <= kMaxRep, "xrpl::doNormalize", "intermediate mantissa fits in int64"); mantissa = m; - g.doRoundUp(negative, mantissa, exponent, "Number::normalize 2"); + g.doRoundUp( + negative, + mantissa, + exponent, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + "Number::normalize 2"); XRPL_ASSERT_PARTS( mantissa >= minMantissa && mantissa <= maxMantissa, "xrpl::doNormalize", @@ -640,12 +620,13 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFix) + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); + doNormalize( + negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); // LCOV_EXCL_STOP } @@ -657,12 +638,13 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFix) + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); + doNormalize( + negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); // LCOV_EXCL_STOP } @@ -674,27 +656,16 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFix) + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); + doNormalize( + negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); } void Number::normalize(MantissaRange const& range) { - normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFix); -} - -void -Number::normalize(Guard const& guard) -{ - normalize( - negative_, - mantissa_, - exponent_, - guard.minMantissa_, - guard.maxMantissa_, - guard.cuspRoundingFix_); + normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFixEnabled); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -748,16 +719,7 @@ Number::operator+=(Number const& y) bool const yn = y.negative_; uint128_t ym = y.mantissa_; auto ye = y.exponent_; - Guard g(kRange); - - auto const& minMantissa = g.minMantissa_; - auto const& maxMantissa = g.maxMantissa_; - auto const cuspRoundingFix = g.cuspRoundingFix_; - - // Bring the exponents of both values into agreement, so the mantissas are on the same scale - // and can be added directly together. - // Shrink the mantissa and bring the exponent up of the value with the lower exponent. Store any - // dropped digits in the Guard. + Guard g; if (xe < ye) { if (xn) @@ -777,6 +739,11 @@ Number::operator+=(Number const& y) } while (xe > ye); } + auto const& range = kRange.get(); + auto const& minMantissa = range.min; + auto const& maxMantissa = range.max; + auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + if (xn == yn) { xm += ym; @@ -784,7 +751,14 @@ Number::operator+=(Number const& y) { g.doDropDigit(xm, xe); } - g.doRoundUp(xn, xm, xe, "Number::addition overflow"); + g.doRoundUp( + xn, + xm, + xe, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + "Number::addition overflow"); } else { @@ -798,40 +772,19 @@ Number::operator+=(Number const& y) xe = ye; xn = yn; } - if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) + while (xm < minMantissa && xm * 10 <= kMaxRep) { - // Grow xm/xe and pull digits out of the Guard until it's a little bit larger than - // maxMantissa, so that normalize will have enough information to make an accurate - // rounding decision, but stop if the Guard empties out, because no rounding will be - // necessary. (Normalize will pad it back into range.) Note that if any digits were lost - // (xbit), the Guard will never be empty, so xm will get big. - auto const upperLimit = static_cast(minMantissa) * 1000; - while (xm < upperLimit && !g.empty()) - { - xm *= 10; - xm -= g.pop(); - --xe; - } + xm *= 10; + xm -= g.pop(); + --xe; } - else - { - // Grow xm/xe and pull digits out of the Guard until it's back in range. - while (xm < minMantissa && xm * 10 <= kMaxRep) - { - xm *= 10; - xm -= g.pop(); - --xe; - } - } - // Round down, based on whether there is any data left in the Guard (depending on - // cuspRoundingFix) - g.doRoundDown(xn, xm, xe); + g.doRoundDown(xn, xm, xe, minMantissa); } - doNormalize(xn, xm, xe, minMantissa, maxMantissa, cuspRoundingFix, false); negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; + normalize(range); return *this; } @@ -865,11 +818,14 @@ Number::operator*=(Number const& y) auto ze = xe + ye; auto zs = xs * ys; bool zn = (zs == -1); - Guard g(kRange); + Guard g; if (zn) g.setNegative(); - auto const& maxMantissa = g.maxMantissa_; + auto const& range = kRange.get(); + auto const& minMantissa = range.min; + auto const& maxMantissa = range.max; + auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; while (zm > maxMantissa || zm > kMaxRep) { @@ -878,12 +834,19 @@ Number::operator*=(Number const& y) xm = static_cast(zm); xe = ze; - g.doRoundUp(zn, xm, xe, "Number::multiplication overflow : exponent is " + std::to_string(xe)); + g.doRoundUp( + zn, + xm, + xe, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + "Number::multiplication overflow : exponent is " + std::to_string(xe)); negative_ = zn; mantissa_ = xm; exponent_ = xe; - normalize(g); + normalize(range); return *this; } @@ -919,7 +882,7 @@ Number::operator/=(Number const& y) auto const& range = kRange.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; - auto const cuspRoundingFix = range.cuspRoundingFix; + auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; // Division operates on two large integers (16-digit for small // mantissas, 19-digit for large) using integer math. If the values @@ -1051,14 +1014,14 @@ Number::operator/=(Number const& y) // rounding fix is enabled, flag if there is still // a remainder from stage 2. bool const useTrailingRemainder = - cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled; + cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled; if (useTrailingRemainder) { dropped = partialNumerator % dm != 0; } } } - doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFix, dropped); + doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled, dropped); negative_ = zp; mantissa_ = static_cast(zm); exponent_ = ze; @@ -1072,7 +1035,7 @@ operator rep() const { rep drops = mantissa(); int offset = exponent(); - Guard g(kRange); + Guard g; if (drops != 0) { if (negative_) From c165af497e15c6ee72cdbc3ba88bc3565c052fcd Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Sat, 6 Jun 2026 14:35:35 -0400 Subject: [PATCH 12/12] Revert "Rollback Number class changes; show the fix works without side effects" This reverts commit 8743be8eae2a28b05f3683d3f0164dd50737305e. --- include/xrpl/basics/Number.h | 38 +++-- src/libxrpl/basics/Number.cpp | 285 +++++++++++++++++++--------------- 2 files changed, 185 insertions(+), 138 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 0b4caaa722..61b63fe234 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -51,37 +51,43 @@ namespace detail { * compile time. Doing it at runtime would be pretty wasteful and * inefficient. */ -constexpr std::size_t kInt64Digits = 20; -consteval std::array +constexpr std::size_t kUint64Digits = 20; +constexpr std::size_t kUint128Digits = 39; + +template +consteval std::array buildPowersOfTen() { - std::array result{}; + std::array result{}; - std::uint64_t power = 1; + T power = 1; std::size_t exponent = 0; // end the loop early so it doesn't overflow; for (; exponent < result.size() - 1; ++exponent, power *= 10) { result[exponent] = power; - if (power > std::numeric_limits::max() / 10) + if (power > std::numeric_limits::max() / 10) throw std::logic_error("Power of 10 table is too big"); } result[exponent] = power; - if (power < std::numeric_limits::max() / 10) - throw std::logic_error("Power of 10 table is not big enough for the uint64_t type"); + if (power < std::numeric_limits::max() / 10) + throw std::logic_error("Power of 10 table is not big enough for the given type"); return result; } } // namespace detail -constexpr std::array kPowerOfTen = detail::buildPowersOfTen(); +template +constexpr std::array kPowerOfTenImpl = detail::buildPowersOfTen(); + +constexpr auto kPowerOfTen = kPowerOfTenImpl; static_assert(kPowerOfTen[0] == 1); static_assert(kPowerOfTen[1] == 10); static_assert(kPowerOfTen[10] == 10'000'000'000); static_assert( - isPowerOfTen(kPowerOfTen.back()) && *logTen(kPowerOfTen.back()) == detail::kInt64Digits - 1); + isPowerOfTen(kPowerOfTen.back()) && *logTen(kPowerOfTen.back()) == detail::kUint64Digits - 1); /** MantissaRange defines a range for the mantissa of a normalized Number. * @@ -141,7 +147,7 @@ struct MantissaRange final int const log{getExponent(scale)}; rep const min{getMin(scale, log)}; rep const max{(min * 10) - 1}; - CuspRoundingFix const cuspRoundingFixEnabled{isCuspFixEnabled(scale)}; + CuspRoundingFix const cuspRoundingFix{isCuspFixEnabled(scale)}; static MantissaRange const& getMantissaRange(MantissaScale scale); @@ -543,9 +549,15 @@ private: // changing the values inside the range. static thread_local std::reference_wrapper kRange; + class Guard; + void normalize(MantissaRange const& range); + // Guard has the fields that we need, as well as MantissaRange, so if we have a guard, use that + void + normalize(Guard const& guard); + /** Normalize Number components to an arbitrary range. * * min/maxMantissa are parameters because this function is used by both @@ -560,7 +572,7 @@ private: int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); + MantissaRange::CuspRoundingFix cuspRoundingFix); template friend void @@ -570,7 +582,7 @@ private: int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + MantissaRange::CuspRoundingFix cuspRoundingFix, bool dropped); [[nodiscard]] bool @@ -588,8 +600,6 @@ private: // UB, and can vary across compilers. static internalrep externalToInternal(rep mantissa); - - class Guard; }; constexpr Number::Number(bool negative, internalrep mantissa, int exponent, Unchecked) noexcept diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 23e913bbdc..d43eebc314 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -65,7 +65,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 15); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max < Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Disabled); } { [[maybe_unused]] @@ -76,7 +76,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Disabled); } { [[maybe_unused]] @@ -87,7 +87,7 @@ MantissaRange::getRanges() static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Enabled); } return map; }(); @@ -171,7 +171,21 @@ class Number::Guard std::uint8_t sbit_ : 1 {0}; // the sign of the guard digits public: - explicit Guard() = default; + internalrep const minMantissa_; + internalrep const maxMantissa_; + MantissaRange::CuspRoundingFix const cuspRoundingFix_; + + explicit Guard( + internalrep const& minMantissa, + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFix) + : minMantissa_(minMantissa), maxMantissa_(maxMantissa), cuspRoundingFix_(cuspRoundingFix) + { + } + + explicit Guard(MantissaRange const& range) : Guard(range.min, range.max, range.cuspRoundingFix) + { + } // set & test the sign bit void @@ -194,6 +208,10 @@ public: unsigned pop() noexcept; + // if true, there are no digits in the guard, including dropped digits (xbit_) + bool + empty() const noexcept; + /** Drop a digit from the mantissa, and increment the exponent, storing the dropped digit in * this Guard. * @@ -206,28 +224,35 @@ public: void doDropDigit(T& mantissa, int& exponent) noexcept; + enum class Round { + // The result is exact. No rounding is needed. Only used if cuspRoundingFix is enabled. + Exact = -2, + // Round down. Since we use integer math, that usually means no change is needed. + // Exceptions are for when the result is between kMaxRap and kMaxRepUp (round to kMaxRep), + // or after subtraction where _any_ remainder will modify the result. The latter is what + // distinguishes Exact from Down. + Down = -1, + // The result was exactly half-way between two integers. This will round to even. + Even = 0, + // Round up. Always adds 1 (or subtracts 1 in some cases if cuspRoundingFix is not enabled) + Up = 1, + }; + // Indicate round direction: 1 is up, -1 is down, 0 is even // This enables the client to round towards nearest, and on // tie, round towards even. - [[nodiscard]] int + [[nodiscard]] Round round() const noexcept; // Modify the result to the correctly rounded value template void - doRoundUp( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, - std::string location); + doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location); // Modify the result to the correctly rounded value template void - doRoundDown(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); + doRoundDown(bool& negative, T& mantissa, int& exponent); // Modify the result to the correctly rounded value void @@ -239,7 +264,7 @@ private: template void - bringIntoRange(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); + bringIntoRange(bool& negative, T& mantissa, int& exponent); }; inline void @@ -289,6 +314,12 @@ Number::Guard::pop() noexcept return d; } +inline bool +Number::Guard::empty() const noexcept +{ + return digits_ == 0 && !xbit_; +} + template void Number::Guard::doDropDigit(T& mantissa, int& exponent) noexcept @@ -314,54 +345,56 @@ Number::Guard::doDropDigit(uint128_t& mantissa, int& exponent) noexce // -1 if Guard is less than half // 0 if Guard is exactly half // 1 if Guard is greater than half -int +Number::Guard::Round Number::Guard::round() const noexcept { auto mode = Number::getround(); + if (cuspRoundingFix_ != MantissaRange::CuspRoundingFix::Disabled && empty()) + { + // No remainder + return Round::Exact; + } + if (mode == RoundingMode::TowardsZero) - return -1; + return Round::Down; if (mode == RoundingMode::Downward) { if (sbit_) { if (digits_ > 0 || xbit_) - return 1; + return Round::Up; } - return -1; + return Round::Down; } if (mode == RoundingMode::Upward) { if (sbit_) - return -1; + return Round::Down; if (digits_ > 0 || xbit_) - return 1; - return -1; + return Round::Up; + return Round::Down; } // assume round to nearest if mode is not one of the predefined values if (digits_ > 0x5000'0000'0000'0000) - return 1; + return Round::Up; if (digits_ < 0x5000'0000'0000'0000) - return -1; + return Round::Down; if (xbit_) - return 1; - return 0; + return Round::Up; + return Round::Even; } template void -Number::Guard::bringIntoRange( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa) +Number::Guard::bringIntoRange(bool& negative, T& mantissa, int& exponent) { // Bring mantissa back into the minMantissa / maxMantissa range AFTER // rounding - if (mantissa < minMantissa) + if (mantissa < minMantissa_) { mantissa *= 10; --exponent; @@ -378,22 +411,15 @@ Number::Guard::bringIntoRange( template void -Number::Guard::doRoundUp( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, - std::string location) +Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location) { auto r = round(); - if (r == 1 || (r == 0 && (mantissa & 1) == 1)) + if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { - auto const safeToIncrement = [&maxMantissa](auto const& mantissa) { - return mantissa < maxMantissa && mantissa < kMaxRep; + auto const safeToIncrement = [this](auto const& mantissa) { + return mantissa < maxMantissa_ && mantissa < kMaxRep; }; - if (cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFix_ == MantissaRange::CuspRoundingFix::Enabled) { // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". @@ -414,14 +440,7 @@ Number::Guard::doRoundUp( safeToIncrement(mantissa), "xrpl::Number::Guard::doRoundUp", "can't recurse more than once"); - doRoundUp( - negative, - mantissa, - exponent, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - location); + doRoundUp(negative, mantissa, exponent, location); return; } } @@ -432,7 +451,7 @@ Number::Guard::doRoundUp( ++mantissa; // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". - if (mantissa > maxMantissa || mantissa > kMaxRep) + if (mantissa > maxMantissa_ || mantissa > kMaxRep) { // Don't use doDropDigit here mantissa /= 10; @@ -440,30 +459,38 @@ Number::Guard::doRoundUp( } } } - bringIntoRange(negative, mantissa, exponent, minMantissa); + bringIntoRange(negative, mantissa, exponent); if (exponent > kMaxExponent) Throw(std::string(location)); } template void -Number::Guard::doRoundDown( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa) +Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) { auto r = round(); - if (r == 1 || (r == 0 && (mantissa & 1) == 1)) + if (cuspRoundingFix_ != MantissaRange::CuspRoundingFix::Disabled) { - --mantissa; - if (mantissa < minMantissa) + // If there was any remainder, subtract 1 from the result. This is sufficient to get the + // best rounding. + if (r != Round::Exact) { - mantissa *= 10; - --exponent; + --mantissa; } } - bringIntoRange(negative, mantissa, exponent, minMantissa); + else + { + if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) + { + --mantissa; + if (mantissa < minMantissa_) + { + mantissa *= 10; + --exponent; + } + } + } + bringIntoRange(negative, mantissa, exponent); } // Modify the result to the correctly rounded value @@ -471,7 +498,7 @@ void Number::Guard::doRound(rep& drops, std::string location) const { auto r = round(); - if (r == 1 || (r == 0 && (drops & 1) == 1)) + if (r == Round::Up || (r == Round::Even && (drops & 1) == 1)) { if (drops >= kMaxRep) { @@ -530,7 +557,7 @@ doNormalize( int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + MantissaRange::CuspRoundingFix cuspRoundingFix, bool dropped) { static constexpr auto kMinExponent = Number::kMinExponent; @@ -553,7 +580,7 @@ doNormalize( m *= 10; --exponent; } - Guard g; + Guard g(minMantissa, maxMantissa, cuspRoundingFix); if (negative) g.setNegative(); if (dropped) @@ -598,14 +625,7 @@ doNormalize( XRPL_ASSERT_PARTS(m <= kMaxRep, "xrpl::doNormalize", "intermediate mantissa fits in int64"); mantissa = m; - g.doRoundUp( - negative, - mantissa, - exponent, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::normalize 2"); + g.doRoundUp(negative, mantissa, exponent, "Number::normalize 2"); XRPL_ASSERT_PARTS( mantissa >= minMantissa && mantissa <= maxMantissa, "xrpl::doNormalize", @@ -620,13 +640,12 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); // LCOV_EXCL_STOP } @@ -638,13 +657,12 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); // LCOV_EXCL_STOP } @@ -656,16 +674,27 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); } void Number::normalize(MantissaRange const& range) { - normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFixEnabled); + normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFix); +} + +void +Number::normalize(Guard const& guard) +{ + normalize( + negative_, + mantissa_, + exponent_, + guard.minMantissa_, + guard.maxMantissa_, + guard.cuspRoundingFix_); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -719,7 +748,16 @@ Number::operator+=(Number const& y) bool const yn = y.negative_; uint128_t ym = y.mantissa_; auto ye = y.exponent_; - Guard g; + Guard g(kRange); + + auto const& minMantissa = g.minMantissa_; + auto const& maxMantissa = g.maxMantissa_; + auto const cuspRoundingFix = g.cuspRoundingFix_; + + // Bring the exponents of both values into agreement, so the mantissas are on the same scale + // and can be added directly together. + // Shrink the mantissa and bring the exponent up of the value with the lower exponent. Store any + // dropped digits in the Guard. if (xe < ye) { if (xn) @@ -739,11 +777,6 @@ Number::operator+=(Number const& y) } while (xe > ye); } - auto const& range = kRange.get(); - auto const& minMantissa = range.min; - auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; - if (xn == yn) { xm += ym; @@ -751,14 +784,7 @@ Number::operator+=(Number const& y) { g.doDropDigit(xm, xe); } - g.doRoundUp( - xn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::addition overflow"); + g.doRoundUp(xn, xm, xe, "Number::addition overflow"); } else { @@ -772,19 +798,40 @@ Number::operator+=(Number const& y) xe = ye; xn = yn; } - while (xm < minMantissa && xm * 10 <= kMaxRep) + if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) { - xm *= 10; - xm -= g.pop(); - --xe; + // Grow xm/xe and pull digits out of the Guard until it's a little bit larger than + // maxMantissa, so that normalize will have enough information to make an accurate + // rounding decision, but stop if the Guard empties out, because no rounding will be + // necessary. (Normalize will pad it back into range.) Note that if any digits were lost + // (xbit), the Guard will never be empty, so xm will get big. + auto const upperLimit = static_cast(minMantissa) * 1000; + while (xm < upperLimit && !g.empty()) + { + xm *= 10; + xm -= g.pop(); + --xe; + } } - g.doRoundDown(xn, xm, xe, minMantissa); + else + { + // Grow xm/xe and pull digits out of the Guard until it's back in range. + while (xm < minMantissa && xm * 10 <= kMaxRep) + { + xm *= 10; + xm -= g.pop(); + --xe; + } + } + // Round down, based on whether there is any data left in the Guard (depending on + // cuspRoundingFix) + g.doRoundDown(xn, xm, xe); } + doNormalize(xn, xm, xe, minMantissa, maxMantissa, cuspRoundingFix, false); negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; - normalize(range); return *this; } @@ -818,14 +865,11 @@ Number::operator*=(Number const& y) auto ze = xe + ye; auto zs = xs * ys; bool zn = (zs == -1); - Guard g; + Guard g(kRange); if (zn) g.setNegative(); - auto const& range = kRange.get(); - auto const& minMantissa = range.min; - auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + auto const& maxMantissa = g.maxMantissa_; while (zm > maxMantissa || zm > kMaxRep) { @@ -834,19 +878,12 @@ Number::operator*=(Number const& y) xm = static_cast(zm); xe = ze; - g.doRoundUp( - zn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::multiplication overflow : exponent is " + std::to_string(xe)); + g.doRoundUp(zn, xm, xe, "Number::multiplication overflow : exponent is " + std::to_string(xe)); negative_ = zn; mantissa_ = xm; exponent_ = xe; - normalize(range); + normalize(g); return *this; } @@ -882,7 +919,7 @@ Number::operator/=(Number const& y) auto const& range = kRange.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + auto const cuspRoundingFix = range.cuspRoundingFix; // Division operates on two large integers (16-digit for small // mantissas, 19-digit for large) using integer math. If the values @@ -1014,14 +1051,14 @@ Number::operator/=(Number const& y) // rounding fix is enabled, flag if there is still // a remainder from stage 2. bool const useTrailingRemainder = - cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled; + cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled; if (useTrailingRemainder) { dropped = partialNumerator % dm != 0; } } } - doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled, dropped); + doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFix, dropped); negative_ = zp; mantissa_ = static_cast(zm); exponent_ = ze; @@ -1035,7 +1072,7 @@ operator rep() const { rep drops = mantissa(); int offset = exponent(); - Guard g; + Guard g(kRange); if (drops != 0) { if (negative_)