chore: More fixes for bad renames - #7092
Conversation
There was a problem hiding this comment.
Pull request overview
Follow-up cleanup to revert/standardize a set of prior “bad renames” by aligning identifiers (and many diagnostic/assert strings) with the current function/type names, including several Io→IO and *_snake_case→camelCase adjustments.
Changes:
- Standardize diagnostic strings in
XRPL_ASSERT/UNREACHABLEto match current symbol names (e.g.,onEndpoints,toString,doResolve). - Rename core types from
BaseUint→BaseUIntandIo*→IO*across headers and tests. - Align gRPC secure gateway messages with the
secure_gatewayconfig naming.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/xrpld/peerfinder/detail/Logic.h | Update XRPL_ASSERT context strings to onEndpoints. |
| src/xrpld/peerfinder/detail/Livecache.h | Update XRPL_ASSERT context strings for HopsT. |
| src/xrpld/peerfinder/detail/Handouts.h | Update XRPL_ASSERT context string to handoutOne. |
| src/xrpld/peerfinder/detail/Bootcache.cpp | Update XRPL_ASSERT context strings to onSuccess/onFailure. |
| src/xrpld/overlay/detail/OverlayImpl.cpp | Update XRPL_ASSERT context strings to addActive. |
| src/xrpld/overlay/detail/Handshake.cpp | Rename BaseUint<512> usage to BaseUInt<512> for SSL message hashing. |
| src/xrpld/app/rdb/backend/detail/Node.cpp | Update UNREACHABLE context string to toString. |
| src/xrpld/app/main/GRPCServer.cpp | Align log/exception messages to secure_gateway config naming. |
| src/xrpld/app/main/Application.cpp | Rename IoLatencySampler→IOLatencySampler and probe type to IOLatencyProbe. |
| src/test/protocol/STIssue_test.cpp | Update tests to use BaseUInt instead of BaseUint. |
| src/test/beast/beast_io_latency_probe_test.cpp | Update test probe type to IOLatencyProbe. |
| src/test/basics/base_uint_test.cpp | Update base uint tests to BaseUInt and related helper names. |
| src/test/app/Vault_test.cpp | Update BaseUint<256> usage in test transaction fields to BaseUInt<256>. |
| src/test/app/MultiSign_test.cpp | Update BaseUint<256>::fromVoid usage to BaseUInt<256>::fromVoid. |
| src/libxrpl/tx/applySteps.cpp | Update UNREACHABLE context strings to invokePre*/invokeApply. |
| src/libxrpl/server/Port.cpp | Change printed label to secure_gateway nets:. |
| src/libxrpl/protocol/Quality.cpp | Update XRPL_ASSERT context strings to ceilInImpl/ceilOutImpl/composedQuality. |
| src/libxrpl/protocol/Indexes.cpp | Update XRPL_ASSERT context string to NextT::operator(). |
| src/libxrpl/ledger/Dir.cpp | Update XRPL_ASSERT context strings to Dir::ConstIterator methods; minor formatting. |
| src/libxrpl/beast/clock/basic_seconds_clock.cpp | Update XRPL_ASSERT context string to SecondsClockThread dtor. |
| src/libxrpl/basics/ResolverAsio.cpp | Update XRPL_ASSERT context strings to doStop/doResolve. |
| src/libxrpl/basics/Log.cpp | Update UNREACHABLE context string to Logs::toString. |
| include/xrpl/shamap/SHAMapItem.h | Update XRPL_ASSERT context string to makeShamapitem. |
| include/xrpl/shamap/SHAMap.h | Update XRPL_ASSERT context string to SHAMap::ConstIterator. |
| include/xrpl/server/detail/ServerImpl.h | Rename member/accessor types from IoList to IOList. |
| include/xrpl/server/detail/io_list.h | Rename IoList to IOList and update related references/messages. |
| include/xrpl/server/detail/Door.h | Update inheritance from IoList::Work to IOList::Work. |
| include/xrpl/server/detail/BasePeer.h | Update inheritance from IoList::Work to IOList::Work. |
| include/xrpl/server/detail/BaseHTTPPeer.h | Update inheritance from IoList::Work to IOList::Work. |
| include/xrpl/protocol/UintTypes.h | Update core protocol typedefs to use BaseUInt. |
| include/xrpl/protocol/STObject.h | Update setFieldH160 signature/definition to use BaseUInt. |
| include/xrpl/protocol/STBitString.h | Update value_type/setValue to use BaseUInt. |
| include/xrpl/protocol/Serializer.h | Update bitstring serialize/parse templates to use BaseUInt. |
| include/xrpl/protocol/AccountID.h | Update AccountID typedef to use BaseUInt. |
| include/xrpl/beast/container/detail/aged_unordered_container.h | Update XRPL_ASSERT context string to maybeRehash. |
| include/xrpl/beast/asio/io_latency_probe.h | Rename IoLatencyProbe to IOLatencyProbe and update messages/assert strings. |
| include/xrpl/basics/spinlock.h | Update XRPL_ASSERT context string to PackedSpinlock. |
| include/xrpl/basics/scope.h | Update XRPL_ASSERT context string to ScopeUnlock. |
| include/xrpl/basics/safe_cast.h | Update XRPL_ASSERT context strings to safeDowncast. |
| include/xrpl/basics/random.h | Update XRPL_ASSERT context string to randInt. |
| include/xrpl/basics/partitioned_unordered_map.h | Update XRPL_ASSERT context string to PartitionedUnorderedMap. |
| include/xrpl/basics/base_uint.h | Rename BaseUint to BaseUInt, update operators/helpers and associated typedefs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -62,7 +62,7 @@ struct IsContiguousContainer<Slice> : std::true_type | |||
| number of bits. | |||
| */ | |||
| template <std::size_t Bits, class Tag = void> | |||
There was a problem hiding this comment.
I think we are a little bit past this stage with breaking changes 🏴☠️
| @@ -21,22 +21,22 @@ class IoList final | |||
| void | |||
| destroy(); | |||
|
|
|||
| friend class IoList; | |||
| IoList* ios_ = nullptr; | |||
| friend class IOList; | |||
| IOList* ios_ = nullptr; | |||
|
|
|||
| @@ -30,12 +30,12 @@ class IoLatencyProbe | |||
| bool cancel_{false}; | |||
|
|
|||
| public: | |||
| IoLatencyProbe(duration const& period, boost::asio::io_context& ios) | |||
| IOLatencyProbe(duration const& period, boost::asio::io_context& ios) | |||
| : period_(period), ios_(ios), timer_(ios_) | |||
| { | |||
| } | |||
|
|
|||
| ~IoLatencyProbe() | |||
| ~IOLatencyProbe() | |||
| { | |||
| std::unique_lock<decltype(mutex_)> lock(mutex_); | |||
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7092 +/- ##
=======================================
Coverage 82.1% 82.1%
=======================================
Files 1010 1010
Lines 76023 76040 +17
Branches 7368 7370 +2
=======================================
+ Hits 62403 62417 +14
- Misses 13620 13623 +3
🚀 New features to boost your workflow:
|
| XRPL_ASSERT( | ||
| loadFactor() <= maxLoadFactor(), | ||
| "beast::detail::AgedUnorderedContainer::maybe_rehash : maximum " | ||
| "beast::detail::AgedUnorderedContainer::maybeRehash : maximum " |
There was a problem hiding this comment.
I recall that in the previous PR it was pointed out that certain names had to be maintained with underscores, and I thought that included the various Beast containers and some of the overriding functions.
Should AgedUnorderedContainer remain as-is, or should that be restored to aged_unordered_container that it was in the past, and similarly for maybeRehash back to maybe_rehash?
There was a problem hiding this comment.
I think not. Everything that needed to stay, stayed to make things compile. Everything that could be renamed got renamed. Seems to work
High Level Overview of Change
Follow up to #7091 fixing more findings.
Also renames "Io" to "IO" in some cases where it makes sense.
API Impact
No impact.