From 90e63803838e15f659d35590062baf59e7234b40 Mon Sep 17 00:00:00 2001 From: Bart Date: Fri, 18 Jul 2025 12:55:15 -0400 Subject: [PATCH 1/8] refactor: Update date, libarchive, nudb, openssl, sqlite3, xxhash packages (#5567) This PR updates several dependencies to their latest versions. Not all dependencies have been updated, as some need to be patched and some require additional code changes due to backward incompatibilities introduced by the version bump. --- .github/actions/dependencies/action.yml | 1 - .github/workflows/macos.yml | 1 - .github/workflows/nix.yml | 13 +++- .github/workflows/windows.yml | 1 - BUILD.md | 17 ----- conanfile.py | 16 ++--- external/nudb/conandata.yml | 10 --- external/nudb/conanfile.py | 72 ------------------- ...-0001-add-include-stdexcept-for-msvc.patch | 24 ------- 9 files changed, 19 insertions(+), 136 deletions(-) delete mode 100644 external/nudb/conandata.yml delete mode 100644 external/nudb/conanfile.py delete mode 100644 external/nudb/patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch diff --git a/.github/actions/dependencies/action.yml b/.github/actions/dependencies/action.yml index 731e3e862f..eeb8df105c 100644 --- a/.github/actions/dependencies/action.yml +++ b/.github/actions/dependencies/action.yml @@ -12,7 +12,6 @@ runs: conan export --version 1.1.10 external/snappy conan export --version 9.7.3 external/rocksdb conan export --version 4.0.3 external/soci - conan export --version 2.0.8 external/nudb - name: add Ripple Conan remote if: env.CONAN_URL != '' shell: bash diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 4c386ead62..13c817087a 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -96,7 +96,6 @@ jobs: conan export --version 1.1.10 external/snappy conan export --version 9.7.3 external/rocksdb conan export --version 4.0.3 external/soci - conan export --version 2.0.8 external/nudb - name: add Ripple Conan remote if: env.CONAN_URL != '' shell: bash diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index 409a1defc0..7543ade692 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -164,6 +164,16 @@ jobs: generator: Ninja configuration: ${{ matrix.configuration }} cmake-args: "-Dassert=TRUE -Dwerr=TRUE ${{ matrix.cmake-args }}" + - name: check linking + run: | + cd ${build_dir} + ldd ./rippled + if [ "$(ldd ./rippled | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then + echo 'The binary is statically linked.' + else + echo 'The binary is dynamically linked.' + exit 1 + fi - name: test run: | cd ${build_dir} @@ -220,6 +230,7 @@ jobs: cd ${build_dir} ./rippled --unittest --unittest-jobs $(nproc) ctest -j $(nproc) --output-on-failure + coverage: strategy: fail-fast: false @@ -296,7 +307,6 @@ jobs: attempt_limit: 5 attempt_delay: 210000 # in milliseconds - conan: needs: dependencies runs-on: [self-hosted, heavy] @@ -313,7 +323,6 @@ jobs: uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 with: name: ${{ env.platform }}-${{ env.compiler }}-${{ env.configuration }} - - name: extract cache run: | mkdir -p ${CONAN_HOME} diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 30ad32a89c..ae0302f2a7 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -92,7 +92,6 @@ jobs: conan export --version 1.1.10 external/snappy conan export --version 9.7.3 external/rocksdb conan export --version 4.0.3 external/soci - conan export --version 2.0.8 external/nudb - name: add Ripple Conan remote if: env.CONAN_URL != '' shell: bash diff --git a/BUILD.md b/BUILD.md index 1ba767cd88..fba238d2bc 100644 --- a/BUILD.md +++ b/BUILD.md @@ -167,8 +167,6 @@ It does not explicitly link the C++ standard library, which allows you to statically link it with GCC, if you want. ``` - # Conan 1.x - conan export external/snappy snappy/1.1.10@ # Conan 2.x conan export --version 1.1.10 external/snappy ``` @@ -177,8 +175,6 @@ Export our [Conan recipe for RocksDB](./external/rocksdb). It does not override paths to dependencies when building with Visual Studio. ``` - # Conan 1.x - conan export external/rocksdb rocksdb/9.7.3@ # Conan 2.x conan export --version 9.7.3 external/rocksdb ``` @@ -187,23 +183,10 @@ Export our [Conan recipe for SOCI](./external/soci). It patches their CMake to correctly import its dependencies. ``` - # Conan 1.x - conan export external/soci soci/4.0.3@ # Conan 2.x conan export --version 4.0.3 external/soci ``` -Export our [Conan recipe for NuDB](./external/nudb). -It fixes some source files to add missing `#include`s. - - - ``` - # Conan 1.x - conan export external/nudb nudb/2.0.8@ - # Conan 2.x - conan export --version 2.0.8 external/nudb - ``` - ### Build and Test 1. Create a build directory and move into it. diff --git a/conanfile.py b/conanfile.py index 8e964784f8..d79b47bc6f 100644 --- a/conanfile.py +++ b/conanfile.py @@ -25,9 +25,9 @@ class Xrpl(ConanFile): requires = [ 'grpc/1.50.1', - 'libarchive/3.7.6', - 'nudb/2.0.8', - 'openssl/1.1.1v', + 'libarchive/3.8.1', + 'nudb/2.0.9', + 'openssl/1.1.1w', 'soci/4.0.3', 'zlib/1.3.1', ] @@ -37,7 +37,7 @@ class Xrpl(ConanFile): ] tool_requires = [ - 'protobuf/3.21.9', + 'protobuf/3.21.12', ] default_options = { @@ -105,15 +105,15 @@ class Xrpl(ConanFile): # Conan 2 requires transitive headers to be specified transitive_headers_opt = {'transitive_headers': True} if conan_version.split('.')[0] == '2' else {} self.requires('boost/1.83.0', force=True, **transitive_headers_opt) - self.requires('date/3.0.3', **transitive_headers_opt) + self.requires('date/3.0.4', **transitive_headers_opt) self.requires('lz4/1.10.0', force=True) - self.requires('protobuf/3.21.9', force=True) - self.requires('sqlite3/3.47.0', force=True) + self.requires('protobuf/3.21.12', force=True) + self.requires('sqlite3/3.49.1', force=True) if self.options.jemalloc: self.requires('jemalloc/5.3.0') if self.options.rocksdb: self.requires('rocksdb/9.7.3') - self.requires('xxhash/0.8.2', **transitive_headers_opt) + self.requires('xxhash/0.8.3', **transitive_headers_opt) exports_sources = ( 'CMakeLists.txt', diff --git a/external/nudb/conandata.yml b/external/nudb/conandata.yml deleted file mode 100644 index 721129f88e..0000000000 --- a/external/nudb/conandata.yml +++ /dev/null @@ -1,10 +0,0 @@ -sources: - "2.0.8": - url: "https://github.com/CPPAlliance/NuDB/archive/2.0.8.tar.gz" - sha256: "9b71903d8ba111cd893ab064b9a8b6ac4124ed8bd6b4f67250205bc43c7f13a8" -patches: - "2.0.8": - - patch_file: "patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch" - patch_description: "Fix build for MSVC by including stdexcept" - patch_type: "portability" - patch_source: "https://github.com/cppalliance/NuDB/pull/100/files" diff --git a/external/nudb/conanfile.py b/external/nudb/conanfile.py deleted file mode 100644 index a046e2ba89..0000000000 --- a/external/nudb/conanfile.py +++ /dev/null @@ -1,72 +0,0 @@ -import os - -from conan import ConanFile -from conan.tools.build import check_min_cppstd -from conan.tools.files import apply_conandata_patches, copy, export_conandata_patches, get -from conan.tools.layout import basic_layout - -required_conan_version = ">=1.52.0" - - -class NudbConan(ConanFile): - name = "nudb" - description = "A fast key/value insert-only database for SSD drives in C++11" - license = "BSL-1.0" - url = "https://github.com/conan-io/conan-center-index" - homepage = "https://github.com/CPPAlliance/NuDB" - topics = ("header-only", "KVS", "insert-only") - - package_type = "header-library" - settings = "os", "arch", "compiler", "build_type" - no_copy_source = True - - @property - def _min_cppstd(self): - return 11 - - def export_sources(self): - export_conandata_patches(self) - - def layout(self): - basic_layout(self, src_folder="src") - - def requirements(self): - self.requires("boost/1.83.0") - - def package_id(self): - self.info.clear() - - def validate(self): - if self.settings.compiler.cppstd: - check_min_cppstd(self, self._min_cppstd) - - def source(self): - get(self, **self.conan_data["sources"][self.version], strip_root=True) - - def build(self): - apply_conandata_patches(self) - - def package(self): - copy(self, "LICENSE*", - dst=os.path.join(self.package_folder, "licenses"), - src=self.source_folder) - copy(self, "*", - dst=os.path.join(self.package_folder, "include"), - src=os.path.join(self.source_folder, "include")) - - def package_info(self): - self.cpp_info.bindirs = [] - self.cpp_info.libdirs = [] - - self.cpp_info.set_property("cmake_target_name", "NuDB") - self.cpp_info.set_property("cmake_target_aliases", ["NuDB::nudb"]) - self.cpp_info.set_property("cmake_find_mode", "both") - - self.cpp_info.components["core"].set_property("cmake_target_name", "nudb") - self.cpp_info.components["core"].names["cmake_find_package"] = "nudb" - self.cpp_info.components["core"].names["cmake_find_package_multi"] = "nudb" - self.cpp_info.components["core"].requires = ["boost::thread", "boost::system"] - - # TODO: to remove in conan v2 once cmake_find_package_* generators removed - self.cpp_info.names["cmake_find_package"] = "NuDB" - self.cpp_info.names["cmake_find_package_multi"] = "NuDB" diff --git a/external/nudb/patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch b/external/nudb/patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch deleted file mode 100644 index 2d5264f3ce..0000000000 --- a/external/nudb/patches/2.0.8-0001-add-include-stdexcept-for-msvc.patch +++ /dev/null @@ -1,24 +0,0 @@ -diff --git a/include/nudb/detail/stream.hpp b/include/nudb/detail/stream.hpp -index 6c07bf1..e0ce8ed 100644 ---- a/include/nudb/detail/stream.hpp -+++ b/include/nudb/detail/stream.hpp -@@ -14,6 +14,7 @@ - #include - #include - #include -+#include - - namespace nudb { - namespace detail { -diff --git a/include/nudb/impl/context.ipp b/include/nudb/impl/context.ipp -index beb7058..ffde0b3 100644 ---- a/include/nudb/impl/context.ipp -+++ b/include/nudb/impl/context.ipp -@@ -9,6 +9,7 @@ - #define NUDB_IMPL_CONTEXT_IPP - - #include -+#include - - namespace nudb { - From 1a40f18bddd4c1553f2655b4c816988c48d4f563 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Fri, 18 Jul 2025 10:58:46 -0700 Subject: [PATCH 2/8] Remove the type filter from "ledger" RPC command (#4934) This issue was reported on the Javascript client library: XRPLF/xrpl.js#2611 The type filter (Note: as of the latest version of rippled, type parameter is deprecated) does not work as expected. This PR removes the type filter from the ledger command. --- include/xrpl/protocol/ErrorCodes.h | 2 ++ src/test/rpc/LedgerRPC_test.cpp | 38 ++++++++++++++++---- src/xrpld/app/ledger/LedgerToJson.h | 6 ++-- src/xrpld/app/ledger/detail/LedgerToJson.cpp | 19 +++++----- src/xrpld/rpc/handlers/LedgerHandler.cpp | 4 --- src/xrpld/rpc/handlers/LedgerHandler.h | 18 ++++++++-- 6 files changed, 60 insertions(+), 27 deletions(-) diff --git a/include/xrpl/protocol/ErrorCodes.h b/include/xrpl/protocol/ErrorCodes.h index 9c9319ba42..f06b927566 100644 --- a/include/xrpl/protocol/ErrorCodes.h +++ b/include/xrpl/protocol/ErrorCodes.h @@ -169,6 +169,8 @@ enum warning_code_i { warnRPC_AMENDMENT_BLOCKED = 1002, warnRPC_EXPIRED_VALIDATOR_LIST = 1003, // unused = 1004 + warnRPC_FIELDS_DEPRECATED = 2004, // rippled needs to maintain + // compatibility with Clio on this code. }; //------------------------------------------------------------------------------ diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 5b26f43161..9ba9c9a655 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -711,6 +711,7 @@ class LedgerRPC_test : public beast::unit_test::suite env.close(); std::string index; + int hashesLedgerEntryIndex = -1; { Json::Value jvParams; jvParams[jss::ledger_index] = 3u; @@ -721,11 +722,27 @@ class LedgerRPC_test : public beast::unit_test::suite env.rpc("json", "ledger", to_string(jvParams))[jss::result]; BEAST_EXPECT(jrr[jss::ledger].isMember(jss::accountState)); BEAST_EXPECT(jrr[jss::ledger][jss::accountState].isArray()); - BEAST_EXPECT(jrr[jss::ledger][jss::accountState].size() == 1u); + + for (auto i = 0; i < jrr[jss::ledger][jss::accountState].size(); + i++) + if (jrr[jss::ledger][jss::accountState][i]["LedgerEntryType"] == + jss::LedgerHashes) + { + index = jrr[jss::ledger][jss::accountState][i]["index"] + .asString(); + hashesLedgerEntryIndex = i; + } + + for (auto const& object : jrr[jss::ledger][jss::accountState]) + if (object["LedgerEntryType"] == jss::LedgerHashes) + index = object["index"].asString(); + + // jss::type is a deprecated field BEAST_EXPECT( - jrr[jss::ledger][jss::accountState][0u]["LedgerEntryType"] == - jss::LedgerHashes); - index = jrr[jss::ledger][jss::accountState][0u]["index"].asString(); + jrr.isMember(jss::warnings) && jrr[jss::warnings].isArray() && + jrr[jss::warnings].size() == 1 && + jrr[jss::warnings][0u][jss::id].asInt() == + warnRPC_FIELDS_DEPRECATED); } { Json::Value jvParams; @@ -737,8 +754,17 @@ class LedgerRPC_test : public beast::unit_test::suite env.rpc("json", "ledger", to_string(jvParams))[jss::result]; BEAST_EXPECT(jrr[jss::ledger].isMember(jss::accountState)); BEAST_EXPECT(jrr[jss::ledger][jss::accountState].isArray()); - BEAST_EXPECT(jrr[jss::ledger][jss::accountState].size() == 1u); - BEAST_EXPECT(jrr[jss::ledger][jss::accountState][0u] == index); + BEAST_EXPECT( + hashesLedgerEntryIndex > 0 && + jrr[jss::ledger][jss::accountState][hashesLedgerEntryIndex] == + index); + + // jss::type is a deprecated field + BEAST_EXPECT( + jrr.isMember(jss::warnings) && jrr[jss::warnings].isArray() && + jrr[jss::warnings].size() == 1 && + jrr[jss::warnings][0u][jss::id].asInt() == + warnRPC_FIELDS_DEPRECATED); } } diff --git a/src/xrpld/app/ledger/LedgerToJson.h b/src/xrpld/app/ledger/LedgerToJson.h index 40be57fc9c..be017bca86 100644 --- a/src/xrpld/app/ledger/LedgerToJson.h +++ b/src/xrpld/app/ledger/LedgerToJson.h @@ -37,9 +37,8 @@ struct LedgerFill ReadView const& l, RPC::Context* ctx, int o = 0, - std::vector q = {}, - LedgerEntryType t = ltANY) - : ledger(l), options(o), txQueue(std::move(q)), type(t), context(ctx) + std::vector q = {}) + : ledger(l), options(o), txQueue(std::move(q)), context(ctx) { if (context) closeTime = context->ledgerMaster.getCloseTimeBySeq(ledger.seq()); @@ -58,7 +57,6 @@ struct LedgerFill ReadView const& ledger; int options; std::vector txQueue; - LedgerEntryType type; RPC::Context* context; std::optional closeTime; }; diff --git a/src/xrpld/app/ledger/detail/LedgerToJson.cpp b/src/xrpld/app/ledger/detail/LedgerToJson.cpp index 3e4f4b8f0a..0e6f81dfbc 100644 --- a/src/xrpld/app/ledger/detail/LedgerToJson.cpp +++ b/src/xrpld/app/ledger/detail/LedgerToJson.cpp @@ -268,19 +268,16 @@ fillJsonState(Object& json, LedgerFill const& fill) for (auto const& sle : ledger.sles) { - if (fill.type == ltANY || sle->getType() == fill.type) + if (binary) { - if (binary) - { - auto&& obj = appendObject(array); - obj[jss::hash] = to_string(sle->key()); - obj[jss::tx_blob] = serializeHex(*sle); - } - else if (expanded) - array.append(sle->getJson(JsonOptions::none)); - else - array.append(to_string(sle->key())); + auto&& obj = appendObject(array); + obj[jss::hash] = to_string(sle->key()); + obj[jss::tx_blob] = serializeHex(*sle); } + else if (expanded) + array.append(sle->getJson(JsonOptions::none)); + else + array.append(to_string(sle->key())); } } diff --git a/src/xrpld/rpc/handlers/LedgerHandler.cpp b/src/xrpld/rpc/handlers/LedgerHandler.cpp index 4015bb9fcc..8987f2d07e 100644 --- a/src/xrpld/rpc/handlers/LedgerHandler.cpp +++ b/src/xrpld/rpc/handlers/LedgerHandler.cpp @@ -54,10 +54,6 @@ LedgerHandler::check() bool const binary = params[jss::binary].asBool(); bool const owner_funds = params[jss::owner_funds].asBool(); bool const queue = params[jss::queue].asBool(); - auto type = chooseLedgerEntryType(params); - if (type.first) - return type.first; - type_ = type.second; options_ = (full ? LedgerFill::full : 0) | (expand ? LedgerFill::expand : 0) | diff --git a/src/xrpld/rpc/handlers/LedgerHandler.h b/src/xrpld/rpc/handlers/LedgerHandler.h index 0e47164ad3..a573589cbc 100644 --- a/src/xrpld/rpc/handlers/LedgerHandler.h +++ b/src/xrpld/rpc/handlers/LedgerHandler.h @@ -76,7 +76,6 @@ private: std::vector queueTxs_; Json::Value result_; int options_ = 0; - LedgerEntryType type_; }; //////////////////////////////////////////////////////////////////////////////// @@ -91,7 +90,7 @@ LedgerHandler::writeResult(Object& value) if (ledger_) { Json::copyFrom(value, result_); - addJson(value, {*ledger_, &context_, options_, queueTxs_, type_}); + addJson(value, {*ledger_, &context_, options_, queueTxs_}); } else { @@ -105,6 +104,21 @@ LedgerHandler::writeResult(Object& value) addJson(open, {*master.getCurrentLedger(), &context_, 0}); } } + + Json::Value warnings{Json::arrayValue}; + if (context_.params.isMember(jss::type)) + { + Json::Value& w = warnings.append(Json::objectValue); + w[jss::id] = warnRPC_FIELDS_DEPRECATED; + w[jss::message] = + "Some fields from your request are deprecated. Please check the " + "documentation at " + "https://xrpl.org/docs/references/http-websocket-apis/ " + "and update your request. Field `type` is deprecated."; + } + + if (warnings.size()) + value[jss::warnings] = std::move(warnings); } } // namespace RPC From 13353ae36de247396c162e50ce41b70748a43a62 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 21 Jul 2025 13:22:32 +0100 Subject: [PATCH 3/8] Fix macos runner (#5585) This change fixes the MacOS pipeline issue by limiting GitHub to choose the existing runners, ensuring the new experimental runners are excluded until they are ready. --- .github/workflows/macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 13c817087a..e9a4db3bed 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -40,7 +40,7 @@ jobs: - Ninja configuration: - Release - runs-on: [self-hosted, macOS] + runs-on: [self-hosted, macOS, mac-runner-m1] env: # The `build` action requires these variables. build_dir: .build From e95683a0fbdf102a92c104c4e4f407f599a96f3a Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Mon, 21 Jul 2025 15:14:22 +0200 Subject: [PATCH 4/8] refactor: Change boost::shared_mutex to std::shared_mutex (#5576) This change reverts the usage of boost::shared_mutex back to std::shared_mutex. The change was originally introduced as a workaround for a bug in glibc 2.28 and older versions, which could cause threads using std::shared_mutex to stall. This issue primarily affected Ubuntu 18.04 and earlier distributions, which we no longer support. --- src/xrpld/app/misc/ValidatorList.h | 2 +- src/xrpld/overlay/detail/PeerImp.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/xrpld/app/misc/ValidatorList.h b/src/xrpld/app/misc/ValidatorList.h index 4cb32282db..1f5d728824 100644 --- a/src/xrpld/app/misc/ValidatorList.h +++ b/src/xrpld/app/misc/ValidatorList.h @@ -226,7 +226,7 @@ class ValidatorList TimeKeeper& timeKeeper_; boost::filesystem::path const dataPath_; beast::Journal const j_; - boost::shared_mutex mutable mutex_; + std::shared_mutex mutable mutex_; using lock_guard = std::lock_guard; using shared_lock = std::shared_lock; diff --git a/src/xrpld/overlay/detail/PeerImp.h b/src/xrpld/overlay/detail/PeerImp.h index ecd3fc7f63..d5f8e4d179 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -98,7 +98,7 @@ private: // Node public key of peer. PublicKey const publicKey_; std::string name_; - boost::shared_mutex mutable nameMutex_; + std::shared_mutex mutable nameMutex_; // The indices of the smallest and largest ledgers this peer has available // @@ -214,7 +214,7 @@ private: total_bytes() const; private: - boost::shared_mutex mutable mutex_; + std::shared_mutex mutable mutex_; boost::circular_buffer rollingAvg_{30, 0ull}; clock_type::time_point intervalStart_{clock_type::now()}; std::uint64_t totalBytes_{0}; From 03e46cd02617b3d7ce32c1a6820f91e2a04b961c Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 21 Jul 2025 15:03:53 +0100 Subject: [PATCH 5/8] Remove `include(default)` from libxrpl profile (#5587) Remove `include(default)` from `conan/profiles/libxrpl`. This means that we will now rely on compiler workarounds stored elsewhere e.g. in global.conf. --- .github/workflows/macos.yml | 4 +++- .github/workflows/nix.yml | 8 ++++++-- .github/workflows/windows.yml | 5 +++-- conan/profiles/libxrpl | 4 ---- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index e9a4db3bed..8acd90eeff 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -24,6 +24,8 @@ env: CONAN_GLOBAL_CONF: | core.download:parallel={{os.cpu_count()}} core.upload:parallel={{os.cpu_count()}} + core:default_build_profile=libxrpl + core:default_profile=libxrpl tools.build:jobs={{ (os.cpu_count() * 4/5) | int }} tools.build:verbosity=verbose tools.compilation:verbosity=verbose @@ -87,7 +89,7 @@ jobs: clang --version - name: configure Conan run : | - echo "${CONAN_GLOBAL_CONF}" > global.conf + echo "${CONAN_GLOBAL_CONF}" >> $(conan config home)/global.conf conan config install conan/profiles/ -tf $(conan config home)/profiles/ conan profile show - name: export custom recipes diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index 7543ade692..8218dcc276 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -25,6 +25,8 @@ env: CONAN_GLOBAL_CONF: | core.download:parallel={{ os.cpu_count() }} core.upload:parallel={{ os.cpu_count() }} + core:default_build_profile=libxrpl + core:default_profile=libxrpl tools.build:jobs={{ (os.cpu_count() * 4/5) | int }} tools.build:verbosity=verbose tools.compilation:verbosity=verbose @@ -91,7 +93,8 @@ jobs: env | sort - name: configure Conan run: | - echo "${CONAN_GLOBAL_CONF}" >> ${CONAN_HOME}/global.conf + echo "${CONAN_GLOBAL_CONF}" >> $(conan config home)/global.conf + conan config install conan/profiles/ -tf $(conan config home)/profiles/ conan profile show - name: archive profile # Create this archive before dependencies are added to the local cache. @@ -379,7 +382,8 @@ jobs: - name: configure Conan run: | - echo "${CONAN_GLOBAL_CONF}" >> ${CONAN_HOME}/global.conf + echo "${CONAN_GLOBAL_CONF}" >> $(conan config home)/global.conf + conan config install conan/profiles/ -tf $(conan config home)/profiles/ conan profile show - name: build dependencies run: | diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index ae0302f2a7..254850f26a 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -27,6 +27,8 @@ env: CONAN_GLOBAL_CONF: | core.download:parallel={{os.cpu_count()}} core.upload:parallel={{os.cpu_count()}} + core:default_build_profile=libxrpl + core:default_profile=libxrpl tools.build:jobs=24 tools.build:verbosity=verbose tools.compilation:verbosity=verbose @@ -82,8 +84,7 @@ jobs: - name: configure Conan shell: bash run: | - echo "${CONAN_GLOBAL_CONF}" > global.conf - mv conan/profiles/libxrpl conan/profiles/default + echo "${CONAN_GLOBAL_CONF}" >> $(conan config home)/global.conf conan config install conan/profiles/ -tf $(conan config home)/profiles/ conan profile show - name: export custom recipes diff --git a/conan/profiles/libxrpl b/conan/profiles/libxrpl index 862244536b..b037b8c4a2 100644 --- a/conan/profiles/libxrpl +++ b/conan/profiles/libxrpl @@ -6,10 +6,6 @@ {% set compiler_version = detect_api.default_compiler_version(compiler, version) %} {% endif %} -{% if os == "Linux" %} -include(default) -{% endif %} - [settings] os={{ os }} arch={{ arch }} From 60909655d3bfdadfe86cd9620f5e2b6018a5bba0 Mon Sep 17 00:00:00 2001 From: Luc des Trois Maisons <3maisons@gmail.com> Date: Tue, 22 Jul 2025 11:42:43 -0400 Subject: [PATCH 6/8] Restructure beast::rngfill (#5563) The current implementation of rngfill is prone to false warnings from GCC about array bounds violations. Looking at the code, the implementation naively manipulates both the bytes count and the buffer pointer directly to ensure the trailing memcpy doesn't overrun the buffer. As expressed, there is a data dependency on both fields between loop iterations. Now, ideally, an optimizing compiler would realize that these dependencies were unnecessary and end up restructuring its intermediate representation into a functionally equivalent form with them absent. However, the point at which this occurs may be disjoint from when warning analyses are performed, potentially rendering them more difficult to determine precisely. In addition, it may also consume a portion of the budget the optimizer has allocated to attempting to improve a translation unit's performance. Given this is a function template which requires context-sensitive instantiation, this code would be more prone than most to being inlined, with a decrease in optimization budget corresponding to the effort the optimizer has already expended, having already optimized one or more calling functions. Thus, the scope for impacting the the ultimate quality of the code generated is elevated. For this change, we rearrange things so that the location and contents of each memcpy can be computed independently, relying on a simple loop iteration counter as the only changing input between iterations. --- include/xrpl/beast/utility/rngfill.h | 38 ++++++++++------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/include/xrpl/beast/utility/rngfill.h b/include/xrpl/beast/utility/rngfill.h index 2b5a9ba040..e1b47618ba 100644 --- a/include/xrpl/beast/utility/rngfill.h +++ b/include/xrpl/beast/utility/rngfill.h @@ -31,38 +31,28 @@ namespace beast { template void -rngfill(void* buffer, std::size_t bytes, Generator& g) +rngfill(void* const buffer, std::size_t const bytes, Generator& g) { using result_type = typename Generator::result_type; + constexpr std::size_t result_size = sizeof(result_type); - while (bytes >= sizeof(result_type)) + std::uint8_t* const buffer_start = static_cast(buffer); + std::size_t const complete_iterations = bytes / result_size; + std::size_t const bytes_remaining = bytes % result_size; + + for (std::size_t count = 0; count < complete_iterations; ++count) { - auto const v = g(); - std::memcpy(buffer, &v, sizeof(v)); - buffer = reinterpret_cast(buffer) + sizeof(v); - bytes -= sizeof(v); + result_type const v = g(); + std::size_t const offset = count * result_size; + std::memcpy(buffer_start + offset, &v, result_size); } - XRPL_ASSERT( - bytes < sizeof(result_type), "beast::rngfill(void*) : maximum bytes"); - -#ifdef __GNUC__ - // gcc 11.1 (falsely) warns about an array-bounds overflow in release mode. - // gcc 12.1 (also falsely) warns about an string overflow in release mode. -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Warray-bounds" -#pragma GCC diagnostic ignored "-Wstringop-overflow" -#endif - - if (bytes > 0) + if (bytes_remaining > 0) { - auto const v = g(); - std::memcpy(buffer, &v, bytes); + result_type const v = g(); + std::size_t const offset = complete_iterations * result_size; + std::memcpy(buffer_start + offset, &v, bytes_remaining); } - -#ifdef __GNUC__ -#pragma GCC diagnostic pop -#endif } template < From 7ff4f79d304dc2145f74718a9fcdf33578c32479 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 23 Jul 2025 11:44:18 +0100 Subject: [PATCH 7/8] Fix clang-format CI job (#5598) For jobs running in containers, $GITHUB_WORKSPACE and ${{ github.workspace }} might not be the same directory. The actions/checkout step is supposed to checkout into `$GITHUB_WORKSPACE` and then add it to safe.directory (see instructions at https://github.com/actions/checkout), but that's apparently not happening for some container images. We can't be sure what is actually happening, so we preemptively add both directories to `safe.directory`. See also the GitHub issue opened in 2022 that still has not been resolved https://github.com/actions/runner/issues/2058. --- .github/workflows/clang-format.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/clang-format.yml b/.github/workflows/clang-format.yml index 83752c4780..0d81f87791 100644 --- a/.github/workflows/clang-format.yml +++ b/.github/workflows/clang-format.yml @@ -11,6 +11,15 @@ jobs: runs-on: ubuntu-24.04 container: ghcr.io/xrplf/ci/tools-rippled-clang-format steps: + # For jobs running in containers, $GITHUB_WORKSPACE and ${{ github.workspace }} might not be the + # same directory. The actions/checkout step is *supposed* to checkout into $GITHUB_WORKSPACE and + # then add it to safe.directory (see instructions at https://github.com/actions/checkout) + # but that's apparently not happening for some container images. We can't be sure what is actually + # happening, so let's pre-emptively add both directories to safe.directory. There's a + # Github issue opened in 2022 and not resolved in 2025 https://github.com/actions/runner/issues/2058 ¯\_(ツ)_/¯ + - run: | + git config --global --add safe.directory $GITHUB_WORKSPACE + git config --global --add safe.directory ${{ github.workspace }} - uses: actions/checkout@v4 - name: Format first-party sources run: | From c233df720a240c6cd5b774ab4a557c272d51c7aa Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Wed, 23 Jul 2025 13:03:12 +0100 Subject: [PATCH 8/8] refactor: Makes HashRouter flags more type-safe (#5371) This change addresses the issue #5336: Refactor HashRouter flags to be more type-safe. * Switched numeric flags to enum type. * Updated unit tests --- src/test/app/Batch_test.cpp | 10 ++- src/test/app/HashRouter_test.cpp | 127 ++++++++++++++++++--------- src/xrpld/app/ledger/Ledger.cpp | 3 +- src/xrpld/app/misc/HashRouter.cpp | 14 +-- src/xrpld/app/misc/HashRouter.h | 82 ++++++++++++----- src/xrpld/app/misc/NetworkOPs.cpp | 21 +++-- src/xrpld/app/tx/detail/Escrow.cpp | 14 +-- src/xrpld/app/tx/detail/apply.cpp | 27 +++--- src/xrpld/overlay/detail/PeerImp.cpp | 25 +++--- src/xrpld/overlay/detail/PeerImp.h | 3 +- 10 files changed, 219 insertions(+), 107 deletions(-) diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index c8fcc4092b..92f286ca6a 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -3652,14 +3652,18 @@ class Batch_test : public beast::unit_test::suite { // Submit a tx with tfInnerBatchTxn uint256 const txBad = submitTx(tfInnerBatchTxn); - BEAST_EXPECT(env.app().getHashRouter().getFlags(txBad) == 0); + BEAST_EXPECT( + env.app().getHashRouter().getFlags(txBad) == + HashRouterFlags::UNDEFINED); } // Validate: NetworkOPs::processTransaction() { uint256 const txid = processTxn(tfInnerBatchTxn); - // HashRouter::getFlags() should return SF_BAD - BEAST_EXPECT(env.app().getHashRouter().getFlags(txid) == SF_BAD); + // HashRouter::getFlags() should return LedgerFlags::BAD + BEAST_EXPECT( + env.app().getHashRouter().getFlags(txid) == + HashRouterFlags::BAD); } } diff --git a/src/test/app/HashRouter_test.cpp b/src/test/app/HashRouter_test.cpp index 0737116f13..44170e152e 100644 --- a/src/test/app/HashRouter_test.cpp +++ b/src/test/app/HashRouter_test.cpp @@ -45,15 +45,19 @@ class HashRouter_test : public beast::unit_test::suite TestStopwatch stopwatch; HashRouter router(getSetup(2s, 1s), stopwatch); - uint256 const key1(1); - uint256 const key2(2); - uint256 const key3(3); + HashRouterFlags key1(HashRouterFlags::PRIVATE1); + HashRouterFlags key2(HashRouterFlags::PRIVATE2); + HashRouterFlags key3(HashRouterFlags::PRIVATE3); + + auto const ukey1 = uint256{static_cast(key1)}; + auto const ukey2 = uint256{static_cast(key2)}; + auto const ukey3 = uint256{static_cast(key3)}; // t=0 - router.setFlags(key1, 11111); - BEAST_EXPECT(router.getFlags(key1) == 11111); - router.setFlags(key2, 22222); - BEAST_EXPECT(router.getFlags(key2) == 22222); + router.setFlags(ukey1, HashRouterFlags::PRIVATE1); + BEAST_EXPECT(router.getFlags(ukey1) == HashRouterFlags::PRIVATE1); + router.setFlags(ukey2, HashRouterFlags::PRIVATE2); + BEAST_EXPECT(router.getFlags(ukey2) == HashRouterFlags::PRIVATE2); // key1 : 0 // key2 : 0 // key3: null @@ -62,7 +66,7 @@ class HashRouter_test : public beast::unit_test::suite // Because we are accessing key1 here, it // will NOT be expired for another two ticks - BEAST_EXPECT(router.getFlags(key1) == 11111); + BEAST_EXPECT(router.getFlags(ukey1) == HashRouterFlags::PRIVATE1); // key1 : 1 // key2 : 0 // key3 null @@ -70,9 +74,9 @@ class HashRouter_test : public beast::unit_test::suite ++stopwatch; // t=3 - router.setFlags(key3, 33333); // force expiration - BEAST_EXPECT(router.getFlags(key1) == 11111); - BEAST_EXPECT(router.getFlags(key2) == 0); + router.setFlags(ukey3, HashRouterFlags::PRIVATE3); // force expiration + BEAST_EXPECT(router.getFlags(ukey1) == HashRouterFlags::PRIVATE1); + BEAST_EXPECT(router.getFlags(ukey2) == HashRouterFlags::UNDEFINED); } void @@ -83,15 +87,21 @@ class HashRouter_test : public beast::unit_test::suite TestStopwatch stopwatch; HashRouter router(getSetup(2s, 1s), stopwatch); - uint256 const key1(1); - uint256 const key2(2); - uint256 const key3(3); - uint256 const key4(4); + HashRouterFlags key1(HashRouterFlags::PRIVATE1); + HashRouterFlags key2(HashRouterFlags::PRIVATE2); + HashRouterFlags key3(HashRouterFlags::PRIVATE3); + HashRouterFlags key4(HashRouterFlags::PRIVATE4); + + auto const ukey1 = uint256{static_cast(key1)}; + auto const ukey2 = uint256{static_cast(key2)}; + auto const ukey3 = uint256{static_cast(key3)}; + auto const ukey4 = uint256{static_cast(key4)}; + BEAST_EXPECT(key1 != key2 && key2 != key3 && key3 != key4); // t=0 - router.setFlags(key1, 12345); - BEAST_EXPECT(router.getFlags(key1) == 12345); + router.setFlags(ukey1, HashRouterFlags::BAD); + BEAST_EXPECT(router.getFlags(ukey1) == HashRouterFlags::BAD); // key1 : 0 // key2 : null // key3 : null @@ -103,26 +113,27 @@ class HashRouter_test : public beast::unit_test::suite // so key1 will be expired after the second // call to setFlags. // t=1 - router.setFlags(key2, 9999); - BEAST_EXPECT(router.getFlags(key1) == 12345); - BEAST_EXPECT(router.getFlags(key2) == 9999); + + router.setFlags(ukey2, HashRouterFlags::PRIVATE5); + BEAST_EXPECT(router.getFlags(ukey1) == HashRouterFlags::BAD); + BEAST_EXPECT(router.getFlags(ukey2) == HashRouterFlags::PRIVATE5); // key1 : 1 // key2 : 1 // key3 : null ++stopwatch; // t=2 - BEAST_EXPECT(router.getFlags(key2) == 9999); + BEAST_EXPECT(router.getFlags(ukey2) == HashRouterFlags::PRIVATE5); // key1 : 1 // key2 : 2 // key3 : null ++stopwatch; // t=3 - router.setFlags(key3, 2222); - BEAST_EXPECT(router.getFlags(key1) == 0); - BEAST_EXPECT(router.getFlags(key2) == 9999); - BEAST_EXPECT(router.getFlags(key3) == 2222); + router.setFlags(ukey3, HashRouterFlags::BAD); + BEAST_EXPECT(router.getFlags(ukey1) == HashRouterFlags::UNDEFINED); + BEAST_EXPECT(router.getFlags(ukey2) == HashRouterFlags::PRIVATE5); + BEAST_EXPECT(router.getFlags(ukey3) == HashRouterFlags::BAD); // key1 : 3 // key2 : 3 // key3 : 3 @@ -130,10 +141,10 @@ class HashRouter_test : public beast::unit_test::suite ++stopwatch; // t=4 // No insertion, no expiration - router.setFlags(key1, 7654); - BEAST_EXPECT(router.getFlags(key1) == 7654); - BEAST_EXPECT(router.getFlags(key2) == 9999); - BEAST_EXPECT(router.getFlags(key3) == 2222); + router.setFlags(ukey1, HashRouterFlags::SAVED); + BEAST_EXPECT(router.getFlags(ukey1) == HashRouterFlags::SAVED); + BEAST_EXPECT(router.getFlags(ukey2) == HashRouterFlags::PRIVATE5); + BEAST_EXPECT(router.getFlags(ukey3) == HashRouterFlags::BAD); // key1 : 4 // key2 : 4 // key3 : 4 @@ -142,11 +153,11 @@ class HashRouter_test : public beast::unit_test::suite ++stopwatch; // t=6 - router.setFlags(key4, 7890); - BEAST_EXPECT(router.getFlags(key1) == 0); - BEAST_EXPECT(router.getFlags(key2) == 0); - BEAST_EXPECT(router.getFlags(key3) == 0); - BEAST_EXPECT(router.getFlags(key4) == 7890); + router.setFlags(ukey4, HashRouterFlags::TRUSTED); + BEAST_EXPECT(router.getFlags(ukey1) == HashRouterFlags::UNDEFINED); + BEAST_EXPECT(router.getFlags(ukey2) == HashRouterFlags::UNDEFINED); + BEAST_EXPECT(router.getFlags(ukey3) == HashRouterFlags::UNDEFINED); + BEAST_EXPECT(router.getFlags(ukey4) == HashRouterFlags::TRUSTED); // key1 : 6 // key2 : 6 // key3 : 6 @@ -168,18 +179,18 @@ class HashRouter_test : public beast::unit_test::suite uint256 const key4(4); BEAST_EXPECT(key1 != key2 && key2 != key3 && key3 != key4); - int flags = 12345; // This value is ignored + HashRouterFlags flags(HashRouterFlags::BAD); // This value is ignored router.addSuppression(key1); BEAST_EXPECT(router.addSuppressionPeer(key2, 15)); BEAST_EXPECT(router.addSuppressionPeer(key3, 20, flags)); - BEAST_EXPECT(flags == 0); + BEAST_EXPECT(flags == HashRouterFlags::UNDEFINED); ++stopwatch; BEAST_EXPECT(!router.addSuppressionPeer(key1, 2)); BEAST_EXPECT(!router.addSuppressionPeer(key2, 3)); BEAST_EXPECT(!router.addSuppressionPeer(key3, 4, flags)); - BEAST_EXPECT(flags == 0); + BEAST_EXPECT(flags == HashRouterFlags::UNDEFINED); BEAST_EXPECT(router.addSuppressionPeer(key4, 5)); } @@ -192,9 +203,9 @@ class HashRouter_test : public beast::unit_test::suite HashRouter router(getSetup(2s, 1s), stopwatch); uint256 const key1(1); - BEAST_EXPECT(router.setFlags(key1, 10)); - BEAST_EXPECT(!router.setFlags(key1, 10)); - BEAST_EXPECT(router.setFlags(key1, 20)); + BEAST_EXPECT(router.setFlags(key1, HashRouterFlags::PRIVATE1)); + BEAST_EXPECT(!router.setFlags(key1, HashRouterFlags::PRIVATE1)); + BEAST_EXPECT(router.setFlags(key1, HashRouterFlags::PRIVATE2)); } void @@ -250,7 +261,7 @@ class HashRouter_test : public beast::unit_test::suite HashRouter router(getSetup(5s, 1s), stopwatch); uint256 const key(1); HashRouter::PeerShortID peer = 1; - int flags; + HashRouterFlags flags; BEAST_EXPECT(router.shouldProcess(key, peer, flags, 1s)); BEAST_EXPECT(!router.shouldProcess(key, peer, flags, 1s)); @@ -364,6 +375,39 @@ class HashRouter_test : public beast::unit_test::suite } } + void + testFlagsOps() + { + testcase("Bitwise Operations"); + + using HF = HashRouterFlags; + using UHF = std::underlying_type_t; + + HF f1 = HF::BAD; + HF f2 = HF::SAVED; + HF combined = f1 | f2; + + BEAST_EXPECT( + static_cast(combined) == + (static_cast(f1) | static_cast(f2))); + + HF temp = f1; + temp |= f2; + BEAST_EXPECT(temp == combined); + + HF intersect = combined & f1; + BEAST_EXPECT(intersect == f1); + + HF temp2 = combined; + temp2 &= f1; + BEAST_EXPECT(temp2 == f1); + + BEAST_EXPECT(any(f1)); + BEAST_EXPECT(any(f2)); + BEAST_EXPECT(any(combined)); + BEAST_EXPECT(!any(HF::UNDEFINED)); + } + public: void run() override @@ -375,6 +419,7 @@ public: testRelay(); testProcess(); testSetup(); + testFlagsOps(); } }; diff --git a/src/xrpld/app/ledger/Ledger.cpp b/src/xrpld/app/ledger/Ledger.cpp index 3cdf0ab1a7..6de4f2cbde 100644 --- a/src/xrpld/app/ledger/Ledger.cpp +++ b/src/xrpld/app/ledger/Ledger.cpp @@ -996,7 +996,8 @@ pendSaveValidated( bool isSynchronous, bool isCurrent) { - if (!app.getHashRouter().setFlags(ledger->info().hash, SF_SAVED)) + if (!app.getHashRouter().setFlags( + ledger->info().hash, HashRouterFlags::SAVED)) { // We have tried to save this ledger recently auto stream = app.journal("Ledger").debug(); diff --git a/src/xrpld/app/misc/HashRouter.cpp b/src/xrpld/app/misc/HashRouter.cpp index dc87b2bce1..b241d6a98a 100644 --- a/src/xrpld/app/misc/HashRouter.cpp +++ b/src/xrpld/app/misc/HashRouter.cpp @@ -65,7 +65,10 @@ HashRouter::addSuppressionPeerWithStatus(uint256 const& key, PeerShortID peer) } bool -HashRouter::addSuppressionPeer(uint256 const& key, PeerShortID peer, int& flags) +HashRouter::addSuppressionPeer( + uint256 const& key, + PeerShortID peer, + HashRouterFlags& flags) { std::lock_guard lock(mutex_); @@ -79,7 +82,7 @@ bool HashRouter::shouldProcess( uint256 const& key, PeerShortID peer, - int& flags, + HashRouterFlags& flags, std::chrono::seconds tx_interval) { std::lock_guard lock(mutex_); @@ -91,7 +94,7 @@ HashRouter::shouldProcess( return s.shouldProcess(suppressionMap_.clock().now(), tx_interval); } -int +HashRouterFlags HashRouter::getFlags(uint256 const& key) { std::lock_guard lock(mutex_); @@ -100,9 +103,10 @@ HashRouter::getFlags(uint256 const& key) } bool -HashRouter::setFlags(uint256 const& key, int flags) +HashRouter::setFlags(uint256 const& key, HashRouterFlags flags) { - XRPL_ASSERT(flags, "ripple::HashRouter::setFlags : valid input"); + XRPL_ASSERT( + static_cast(flags), "ripple::HashRouter::setFlags : valid input"); std::lock_guard lock(mutex_); diff --git a/src/xrpld/app/misc/HashRouter.h b/src/xrpld/app/misc/HashRouter.h index d1d69623c1..60a0b01155 100644 --- a/src/xrpld/app/misc/HashRouter.h +++ b/src/xrpld/app/misc/HashRouter.h @@ -31,20 +31,59 @@ namespace ripple { -// TODO convert these macros to int constants or an enum -#define SF_BAD 0x02 // Temporarily bad -#define SF_SAVED 0x04 -#define SF_HELD 0x08 // Held by LedgerMaster after potential processing failure -#define SF_TRUSTED 0x10 // comes from trusted source +enum class HashRouterFlags : std::uint16_t { + // Public flags + UNDEFINED = 0x00, + BAD = 0x02, // Temporarily bad + SAVED = 0x04, + HELD = 0x08, // Held by LedgerMaster after potential processing failure + TRUSTED = 0x10, // Comes from a trusted source -// Private flags, used internally in apply.cpp. -// Do not attempt to read, set, or reuse. -#define SF_PRIVATE1 0x0100 -#define SF_PRIVATE2 0x0200 -#define SF_PRIVATE3 0x0400 -#define SF_PRIVATE4 0x0800 -#define SF_PRIVATE5 0x1000 -#define SF_PRIVATE6 0x2000 + // Private flags (used internally in apply.cpp) + // Do not attempt to read, set, or reuse. + PRIVATE1 = 0x0100, + PRIVATE2 = 0x0200, + PRIVATE3 = 0x0400, + PRIVATE4 = 0x0800, + PRIVATE5 = 0x1000, + PRIVATE6 = 0x2000 +}; + +constexpr HashRouterFlags +operator|(HashRouterFlags lhs, HashRouterFlags rhs) +{ + return static_cast( + static_cast>(lhs) | + static_cast>(rhs)); +} + +constexpr HashRouterFlags& +operator|=(HashRouterFlags& lhs, HashRouterFlags rhs) +{ + lhs = lhs | rhs; + return lhs; +} + +constexpr HashRouterFlags +operator&(HashRouterFlags lhs, HashRouterFlags rhs) +{ + return static_cast( + static_cast>(lhs) & + static_cast>(rhs)); +} + +constexpr HashRouterFlags& +operator&=(HashRouterFlags& lhs, HashRouterFlags rhs) +{ + lhs = lhs & rhs; + return lhs; +} + +constexpr bool +any(HashRouterFlags flags) +{ + return static_cast>(flags) != 0; +} class Config; @@ -101,14 +140,14 @@ private: peers_.insert(peer); } - int + HashRouterFlags getFlags(void) const { return flags_; } void - setFlags(int flagsToSet) + setFlags(HashRouterFlags flagsToSet) { flags_ |= flagsToSet; } @@ -154,7 +193,7 @@ private: } private: - int flags_ = 0; + HashRouterFlags flags_ = HashRouterFlags::UNDEFINED; std::set peers_; // This could be generalized to a map, if more // than one flag needs to expire independently. @@ -190,14 +229,17 @@ public: addSuppressionPeerWithStatus(uint256 const& key, PeerShortID peer); bool - addSuppressionPeer(uint256 const& key, PeerShortID peer, int& flags); + addSuppressionPeer( + uint256 const& key, + PeerShortID peer, + HashRouterFlags& flags); // Add a peer suppression and return whether the entry should be processed bool shouldProcess( uint256 const& key, PeerShortID peer, - int& flags, + HashRouterFlags& flags, std::chrono::seconds tx_interval); /** Set the flags on a hash. @@ -205,9 +247,9 @@ public: @return `true` if the flags were changed. `false` if unchanged. */ bool - setFlags(uint256 const& key, int flags); + setFlags(uint256 const& key, HashRouterFlags flags); - int + HashRouterFlags getFlags(uint256 const& key); /** Determines whether the hashed item should be relayed. diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index a7ddbe912c..1ac42579ba 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1207,7 +1207,7 @@ NetworkOPsImp::submitTransaction(std::shared_ptr const& iTrans) auto const txid = trans->getTransactionID(); auto const flags = app_.getHashRouter().getFlags(txid); - if ((flags & SF_BAD) != 0) + if ((flags & HashRouterFlags::BAD) != HashRouterFlags::UNDEFINED) { JLOG(m_journal.warn()) << "Submitted transaction cached bad"; return; @@ -1251,7 +1251,7 @@ NetworkOPsImp::preProcessTransaction(std::shared_ptr& transaction) { auto const newFlags = app_.getHashRouter().getFlags(transaction->getID()); - if ((newFlags & SF_BAD) != 0) + if ((newFlags & HashRouterFlags::BAD) != HashRouterFlags::UNDEFINED) { // cached bad JLOG(m_journal.warn()) << transaction->getID() << ": cached bad!\n"; @@ -1270,7 +1270,8 @@ NetworkOPsImp::preProcessTransaction(std::shared_ptr& transaction) { transaction->setStatus(INVALID); transaction->setResult(temINVALID_FLAG); - app_.getHashRouter().setFlags(transaction->getID(), SF_BAD); + app_.getHashRouter().setFlags( + transaction->getID(), HashRouterFlags::BAD); return false; } @@ -1289,7 +1290,8 @@ NetworkOPsImp::preProcessTransaction(std::shared_ptr& transaction) JLOG(m_journal.info()) << "Transaction has bad signature: " << reason; transaction->setStatus(INVALID); transaction->setResult(temBAD_SIGNATURE); - app_.getHashRouter().setFlags(transaction->getID(), SF_BAD); + app_.getHashRouter().setFlags( + transaction->getID(), HashRouterFlags::BAD); return false; } @@ -1412,7 +1414,8 @@ NetworkOPsImp::processTransactionSet(CanonicalTXSet const& set) JLOG(m_journal.trace()) << "Exception checking transaction: " << reason; } - app_.getHashRouter().setFlags(tx->getTransactionID(), SF_BAD); + app_.getHashRouter().setFlags( + tx->getTransactionID(), HashRouterFlags::BAD); continue; } @@ -1538,7 +1541,8 @@ NetworkOPsImp::apply(std::unique_lock& batchLock) e.transaction->setResult(e.result); if (isTemMalformed(e.result)) - app_.getHashRouter().setFlags(e.transaction->getID(), SF_BAD); + app_.getHashRouter().setFlags( + e.transaction->getID(), HashRouterFlags::BAD); #ifdef DEBUG if (e.result != tesSUCCESS) @@ -1626,7 +1630,8 @@ NetworkOPsImp::apply(std::unique_lock& batchLock) // (5) ledgers into the future. (Remember that an // unseated optional compares as less than all seated // values, so it has to be checked explicitly first.) - // 3. The SF_HELD flag is not set on the txID. (setFlags + // 3. The HashRouterFlags::BAD flag is not set on the txID. + // (setFlags // checks before setting. If the flag is set, it returns // false, which means it's been held once without one of // the other conditions, so don't hold it again. Time's @@ -1635,7 +1640,7 @@ NetworkOPsImp::apply(std::unique_lock& batchLock) if (e.local || (ledgersLeft && ledgersLeft <= LocalTxs::holdLedgers) || app_.getHashRouter().setFlags( - e.transaction->getID(), SF_HELD)) + e.transaction->getID(), HashRouterFlags::HELD)) { // transaction should be held JLOG(m_journal.debug()) diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index 8f7005d55c..c62c38c675 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -34,13 +34,13 @@ #include #include +namespace ripple { + // During an EscrowFinish, the transaction must specify both // a condition and a fulfillment. We track whether that // fulfillment matches and validates the condition. -#define SF_CF_INVALID SF_PRIVATE5 -#define SF_CF_VALID SF_PRIVATE6 - -namespace ripple { +constexpr HashRouterFlags SF_CF_INVALID = HashRouterFlags::PRIVATE5; +constexpr HashRouterFlags SF_CF_VALID = HashRouterFlags::PRIVATE6; /* Escrow @@ -663,7 +663,7 @@ EscrowFinish::preflight(PreflightContext const& ctx) // If we haven't checked the condition, check it // now. Whether it passes or not isn't important // in preflight. - if (!(flags & (SF_CF_INVALID | SF_CF_VALID))) + if (!any(flags & (SF_CF_INVALID | SF_CF_VALID))) { if (checkCondition(*fb, *cb)) router.setFlags(id, SF_CF_VALID); @@ -1064,7 +1064,7 @@ EscrowFinish::doApply() // It's unlikely that the results of the check will // expire from the hash router, but if it happens, // simply re-run the check. - if (cb && !(flags & (SF_CF_INVALID | SF_CF_VALID))) + if (cb && !any(flags & (SF_CF_INVALID | SF_CF_VALID))) { auto const fb = ctx_.tx[~sfFulfillment]; @@ -1081,7 +1081,7 @@ EscrowFinish::doApply() // If the check failed, then simply return an error // and don't look at anything else. - if (flags & SF_CF_INVALID) + if (any(flags & SF_CF_INVALID)) return tecCRYPTOCONDITION_ERROR; // Check against condition in the ledger entry: diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index 889a520032..e2e0adae45 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -27,11 +27,16 @@ namespace ripple { -// These are the same flags defined as SF_PRIVATE1-4 in HashRouter.h -#define SF_SIGBAD SF_PRIVATE1 // Signature is bad -#define SF_SIGGOOD SF_PRIVATE2 // Signature is good -#define SF_LOCALBAD SF_PRIVATE3 // Local checks failed -#define SF_LOCALGOOD SF_PRIVATE4 // Local checks passed +// These are the same flags defined as HashRouterFlags::PRIVATE1-4 in +// HashRouter.h +constexpr HashRouterFlags SF_SIGBAD = + HashRouterFlags::PRIVATE1; // Signature is bad +constexpr HashRouterFlags SF_SIGGOOD = + HashRouterFlags::PRIVATE2; // Signature is good +constexpr HashRouterFlags SF_LOCALBAD = + HashRouterFlags::PRIVATE3; // Local checks failed +constexpr HashRouterFlags SF_LOCALGOOD = + HashRouterFlags::PRIVATE4; // Local checks passed //------------------------------------------------------------------------------ @@ -66,11 +71,11 @@ checkValidity( return {Validity::Valid, ""}; } - if (flags & SF_SIGBAD) + if (any(flags & SF_SIGBAD)) // Signature is known bad return {Validity::SigBad, "Transaction has bad signature."}; - if (!(flags & SF_SIGGOOD)) + if (!any(flags & SF_SIGGOOD)) { // Don't know signature state. Check it. auto const requireCanonicalSig = @@ -88,12 +93,12 @@ checkValidity( } // Signature is now known good - if (flags & SF_LOCALBAD) + if (any(flags & SF_LOCALBAD)) // ...but the local checks // are known bad. return {Validity::SigGoodOnly, "Local checks failed."}; - if (flags & SF_LOCALGOOD) + if (any(flags & SF_LOCALGOOD)) // ...and the local checks // are known good. return {Validity::Valid, ""}; @@ -112,7 +117,7 @@ checkValidity( void forceValidity(HashRouter& router, uint256 const& txid, Validity validity) { - int flags = 0; + HashRouterFlags flags = HashRouterFlags::UNDEFINED; switch (validity) { case Validity::Valid: @@ -125,7 +130,7 @@ forceValidity(HashRouter& router, uint256 const& txid, Validity validity) // would be silly to call directly break; } - if (flags) + if (any(flags)) router.setFlags(txid, flags); } diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 1238833d0d..23b4760488 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1296,13 +1296,13 @@ PeerImp::handleTransaction( } // LCOV_EXCL_STOP - int flags; + HashRouterFlags flags; constexpr std::chrono::seconds tx_interval = 10s; if (!app_.getHashRouter().shouldProcess(txID, id_, flags, tx_interval)) { // we have seen this transaction recently - if (flags & SF_BAD) + if (any(flags & HashRouterFlags::BAD)) { fee_.update(Resource::feeUselessData, "known bad"); JLOG(p_journal_.debug()) << "Ignoring known bad tx " << txID; @@ -1329,7 +1329,7 @@ PeerImp::handleTransaction( { // Skip local checks if a server we trust // put the transaction in its open ledger - flags |= SF_TRUSTED; + flags |= HashRouterFlags::TRUSTED; } // for non-validator nodes only -- localPublicKey is set for @@ -2841,7 +2841,7 @@ PeerImp::doTransactions( void PeerImp::checkTransaction( - int flags, + HashRouterFlags flags, bool checkSignature, std::shared_ptr const& stx, bool batch) @@ -2866,7 +2866,8 @@ PeerImp::checkTransaction( (stx->getFieldU32(sfLastLedgerSequence) < app_.getLedgerMaster().getValidLedgerIndex())) { - app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); + app_.getHashRouter().setFlags( + stx->getTransactionID(), HashRouterFlags::BAD); charge(Resource::feeUselessData, "expired tx"); return; } @@ -2925,8 +2926,10 @@ PeerImp::checkTransaction( << "Exception checking transaction: " << validReason; } - // Probably not necessary to set SF_BAD, but doesn't hurt. - app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); + // Probably not necessary to set HashRouterFlags::BAD, but + // doesn't hurt. + app_.getHashRouter().setFlags( + stx->getTransactionID(), HashRouterFlags::BAD); charge( Resource::feeInvalidSignature, "check transaction signature failure"); @@ -2949,12 +2952,13 @@ PeerImp::checkTransaction( JLOG(p_journal_.trace()) << "Exception checking transaction: " << reason; } - app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); + app_.getHashRouter().setFlags( + stx->getTransactionID(), HashRouterFlags::BAD); charge(Resource::feeInvalidSignature, "tx (impossible)"); return; } - bool const trusted(flags & SF_TRUSTED); + bool const trusted = any(flags & HashRouterFlags::TRUSTED); app_.getOPs().processTransaction( tx, trusted, false, NetworkOPs::FailHard::no); } @@ -2962,7 +2966,8 @@ PeerImp::checkTransaction( { JLOG(p_journal_.warn()) << "Exception in " << __func__ << ": " << ex.what(); - app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); + app_.getHashRouter().setFlags( + stx->getTransactionID(), HashRouterFlags::BAD); using namespace std::string_literals; charge(Resource::feeInvalidData, "tx "s + ex.what()); } diff --git a/src/xrpld/overlay/detail/PeerImp.h b/src/xrpld/overlay/detail/PeerImp.h index d5f8e4d179..5aa49fd152 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -612,7 +613,7 @@ private: void checkTransaction( - int flags, + HashRouterFlags flags, bool checkSignature, std::shared_ptr const& stx, bool batch);