Compare commits

..

26 Commits

Author SHA1 Message Date
JCW
13039e1dfe Fix build errors for windows 2026-06-08 16:05:46 +01:00
JCW
6fab4d867f Fix clang-tidy erros 2026-06-08 15:47:42 +01:00
JCW
36ec0fb275 Address comments 2026-06-08 15:38:40 +01:00
JCW
8c7e29a13c Fix issues 2026-06-08 15:32:13 +01:00
JCW
014d8caaa7 Merge remote-tracking branch 'origin/develop' into a1q123456/remove-const-cast-from-tagged-cache 2026-06-08 15:25:05 +01:00
Jingchen
fe996913f5 Update src/test/basics/TaggedCache_test.cpp
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
2026-06-08 12:51:05 +01:00
JCW
b7a066caa1 Updated the approach after we introduced the use of canonlalize in DatabaseNodeImp.cpp 2026-06-06 18:13:47 +01:00
JCW
79d19f392c Merge remote-tracking branch 'origin/develop' into a1q123456/remove-const-cast-from-tagged-cache 2026-06-06 12:11:43 +01:00
JCW
b3e1937244 Address PR comments 2026-04-01 21:07:28 +01:00
Jingchen
e7f3241af3 Update src/test/basics/TaggedCache_test.cpp
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-04-01 21:05:57 +01:00
JCW
9c23371c46 Address PR comments 2026-04-01 21:04:20 +01:00
Jingchen
f2edebd919 Merge branch 'develop' into a1q123456/remove-const-cast-from-tagged-cache 2026-04-01 16:20:45 +01:00
JCW
a7ee324153 Split the function into 2 2026-04-01 16:19:53 +01:00
JCW
39c66b4da0 Simplify the code and address PR comments
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2026-03-18 22:23:53 +00:00
JCW
0bcf43a26e Simplify the code and address PR comments
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2026-03-18 22:06:17 +00:00
JCW
9b42bd288a Merge remote-tracking branch 'origin/develop' into a1q123456/remove-const-cast-from-tagged-cache
Signed-off-by: JCW <a1q123456@users.noreply.github.com>

# Conflicts:
#	include/xrpl/basics/TaggedCache.h
#	include/xrpl/basics/TaggedCache.ipp
#	src/test/basics/TaggedCache_test.cpp
2026-03-18 21:47:22 +00:00
JCW
95969cb95e Remove unrelated optimisation to make the PR easier to review
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2026-03-18 21:45:22 +00:00
JCW
bf4dc342c6 Fix formatting
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2025-10-29 14:36:18 +00:00
JCW
a71cd5d271 Address PR comments
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2025-10-28 11:31:45 +00:00
JCW
45baf7339c Merge remote-tracking branch 'origin/develop' into a1q123456/remove-const-cast-from-tagged-cache 2025-10-28 11:26:58 +00:00
JCW
2bc2930a28 Fix errors
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2025-09-18 20:16:36 +01:00
JCW
e837171f7c Fix formatting
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2025-09-18 20:16:36 +01:00
JCW
6f05bd035c Add test case and improve test coverage
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2025-09-18 20:16:36 +01:00
JCW
b98b42bbec Fix comment and fix formatting
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2025-09-18 20:16:36 +01:00
JCW
6e6ea4311b Add unittest
Signed-off-by: JCW <a1q123456@users.noreply.github.com>
2025-09-18 20:16:36 +01:00
JCW
f2271305e5 Fix attempt 2025-09-18 20:16:34 +01:00
19 changed files with 371 additions and 49 deletions

View File

@@ -1,5 +1,5 @@
{
"image_tag": "sha-63ffdc3",
"image_tag": "sha-8abe82e",
"configs": {
"ubuntu": [
{
@@ -67,7 +67,7 @@
"compiler": ["gcc"],
"build_type": ["Release"],
"arch": ["amd64"],
"image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-63ffdc3"
"image": "debian:bookworm"
}
],
@@ -76,7 +76,7 @@
"compiler": ["gcc"],
"build_type": ["Release"],
"arch": ["amd64"],
"image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-63ffdc3"
"image": "registry.access.redhat.com/ubi9/ubi:latest"
}
]
}

View File

@@ -22,8 +22,7 @@ on:
workflow_dispatch:
concurrency:
# Read `on-trigger.yml` for the rationale behind this concurrency group name.
group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && github.sha || github.ref }}
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
defaults:

View File

@@ -20,8 +20,7 @@ on:
workflow_dispatch:
concurrency:
# Read `on-trigger.yml` for the rationale behind this concurrency group name.
group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && github.sha || github.ref }}
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
defaults:

View File

@@ -14,7 +14,7 @@ on:
jobs:
# Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks.
run-hooks:
uses: XRPLF/actions/.github/workflows/pre-commit.yml@312aaab296060ff89d7f798dcab59f019bea6e02
uses: XRPLF/actions/.github/workflows/pre-commit.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81
with:
runs_on: ubuntu-latest
container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }'

View File

@@ -41,13 +41,13 @@ env:
jobs:
build:
runs-on: ubuntu-latest
container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3
container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e
steps:
- name: Checkout repository
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
- name: Prepare runner
uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
with:
enable_ccache: false

View File

@@ -113,7 +113,7 @@ jobs:
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
- name: Prepare runner
uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
with:
enable_ccache: ${{ inputs.ccache_enabled }}

View File

@@ -29,14 +29,14 @@ jobs:
if: ${{ inputs.check_only_changed }}
permissions:
contents: read
uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@312aaab296060ff89d7f798dcab59f019bea6e02
uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@224f3c48d3014d082a1129237b8291ff0b0a331f
run-clang-tidy:
name: Run clang tidy
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/xrpld/nix-debian:sha-63ffdc3"
container: "ghcr.io/xrplf/xrpld/nix-debian:sha-8abe82e"
permissions:
contents: read
issues: write
@@ -45,7 +45,7 @@ jobs:
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
- name: Prepare runner
uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
with:
enable_ccache: false

View File

@@ -68,6 +68,31 @@ jobs:
timeout-minutes: 30
steps:
# Packaging runs in a vanilla distro image, so the tooling has to come
# from the distro's archive: debhelper for deb, rpm-build (and the
# systemd / find-debuginfo macros it depends on) for rpm. Run this
# before actions/checkout so the latter can use git (real history) for
# build_pkg.sh's SOURCE_DATE_EPOCH; otherwise it falls back to a tarball
# download and the timestamp comes from wall-clock time.
- name: Install packaging tooling (deb)
if: ${{ matrix.distro == 'debian' }}
run: |
export DEBIAN_FRONTEND=noninteractive
apt-get update
apt-get install -y --no-install-recommends \
ca-certificates \
debhelper \
git
- name: Install packaging tooling (rpm)
if: ${{ matrix.distro == 'rhel' }}
run: |
dnf install -y --setopt=install_weak_deps=False \
git \
rpm-build \
redhat-rpm-config \
systemd-rpm-macros
- name: Checkout repository
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3

View File

@@ -40,7 +40,7 @@ defaults:
jobs:
upload:
runs-on: ubuntu-latest
container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3
container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e
steps:
- name: Checkout repository
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3

View File

@@ -67,7 +67,7 @@ jobs:
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
- name: Prepare runner
uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
with:
enable_ccache: false

View File

@@ -9,6 +9,7 @@
#include <xrpl/beast/insight/Insight.h>
#include <atomic>
#include <cstddef>
#include <functional>
#include <mutex>
#include <thread>
@@ -17,6 +18,22 @@
namespace xrpl {
namespace detail {
// Replace-policy tags selecting how TaggedCache::canonicalizeImpl resolves a
// collision when the key already exists (defined in TaggedCache.ipp):
// - ReplaceCached: always replace the cached value with `data`. `data` is
// never written back and may be const.
// - ReplaceClient: keep the cached value and write it back into `data` (the
// client's pointer), which must therefore be writable.
// - ReplaceDynamically: call the supplied callback to decide per call; `data`
// is written back when the cached value is kept, so it must be writable.
struct ReplaceCached;
struct ReplaceClient;
struct ReplaceDynamically;
} // namespace detail
/** Map/cache combination.
This class implements a cache and a map. The cache keeps objects alive
in the map. The map allows multiple code paths that reference objects
@@ -96,6 +113,32 @@ public:
bool
del(key_type const& key, bool valid);
private:
// Selects the `data` parameter type of canonicalizeImpl from the replace
// policy: const for detail::ReplaceCached (never written back), otherwise
// writable.
template <typename Policy>
using CanonicalizeClientPointerType = std::conditional_t<
std::is_same_v<detail::ReplaceCached, Policy>,
SharedPointerType const&,
SharedPointerType&>;
/** Shared implementation of the canonicalize family.
`policy` selects how a collision is resolved when `key` already exists:
detail::ReplaceCached, detail::ReplaceClient or
detail::ReplaceDynamically. For ReplaceDynamically `replaceCallback` is
invoked with the existing strong pointer and returns whether to replace
the cached value with `data`; for the tag policies it is unused.
*/
template <class Policy, class Callback = std::nullptr_t>
bool
canonicalizeImpl(
key_type const& key,
CanonicalizeClientPointerType<Policy> data,
Policy policy,
Callback&& replaceCallback = nullptr);
public:
/** Replace aliased objects with originals.
@@ -104,19 +147,49 @@ public:
This routine eliminates the duplicate and performs a replacement
on the callers shared pointer if needed.
`replaceCallback` is a callable taking the existing strong pointer and
returning whether to replace the cached value with `data` (true) or to
keep the cached value and write it back into `data` (false). Because the
write-back case mutates `data`, `data` must be writable.
@param key The key corresponding to the object
@param data A shared pointer to the data corresponding to the object.
@param replace Function that decides if cache should be replaced
@param replaceCallback A callable (existing strong pointer -> bool).
@return `true` If the key already existed.
*/
template <class R>
template <class Callback>
bool
canonicalize(key_type const& key, SharedPointerType& data, R&& replaceCallback);
canonicalize(key_type const& key, SharedPointerType& data, Callback&& replaceCallback);
/** Insert/update the canonical entry for `key`, always replacing the
cached value with `data`.
If an entry already exists for `key`, the cached value is unconditionally
replaced with `data`; otherwise `data` is inserted. `data` is never
written back, so it may be const.
@param key The key corresponding to the object.
@param data A shared pointer to the data corresponding to the object.
@return `true` If the key already existed.
*/
bool
canonicalizeReplaceCache(key_type const& key, SharedPointerType const& data);
/** Insert the canonical entry for `key`, keeping any existing cached value.
If an entry already exists for `key`, the cached value is kept and
written back into `data` so the caller ends up with the canonical
object; otherwise `data` is inserted. Because `data` may be overwritten
it must be writable.
@param key The key corresponding to the object.
@param data A shared pointer to the data corresponding to the object;
updated to the canonical value when one already exists.
@return `true` If the key already existed.
*/
bool
canonicalizeReplaceClient(key_type const& key, SharedPointerType& data);

View File

@@ -5,6 +5,30 @@
namespace xrpl {
namespace detail {
// Replace-policy tags selecting how TaggedCache::canonicalizeImpl resolves a
// collision when the key already exists:
// - ReplaceCached: always replace the cached value with `data`. `data` is
// never written back and may be const.
// - ReplaceClient: keep the cached value and write it back into `data` (the
// client's pointer), which must therefore be writable.
// - ReplaceDynamically: call the supplied callback to decide per call; `data`
// is written back when the cached value is kept, so it must be writable.
struct ReplaceCached
{
};
struct ReplaceClient
{
};
struct ReplaceDynamically
{
};
} // namespace detail
template <
class Key,
class T,
@@ -300,13 +324,29 @@ template <
class Hash,
class KeyEqual,
class Mutex>
template <class R>
template <class Policy, class Callback>
inline bool
TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
canonicalize(key_type const& key, SharedPointerType& data, R&& replaceCallback)
canonicalizeImpl(
key_type const& key,
CanonicalizeClientPointerType<Policy> data,
[[maybe_unused]] Policy policy,
[[maybe_unused]] Callback&& replaceCallback)
{
// Return canonical value, store if needed, refresh in cache
// Return values: true=we had the data already
// `Policy` is one of:
// - detail::ReplaceCached: always replace the cached value with `data`;
// `data` is never written back and may be const.
// - detail::ReplaceClient: keep the cached value and write it back into
// `data` (the client's pointer), which must therefore be writable.
// - detail::ReplaceDynamically: call `replaceCallback` to decide at run
// time; `data` must be writable.
// For the latter two the write-back below requires a mutable `data`, so
// passing a const argument is a compile error.
constexpr bool replaceCached = std::is_same_v<Policy, detail::ReplaceCached>;
std::scoped_lock const lock(mutex_);
auto cit = cache_.find(key);
@@ -324,13 +364,14 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
Entry& entry = cit->second;
entry.touch(clock_.now());
auto shouldReplace = [&] {
if constexpr (std::is_invocable_r_v<bool, R>)
auto shouldReplaceCached = [&] {
if constexpr (replaceCached)
{
// The reason for this extra complexity is for intrusive
// strong/weak combo getting a strong is relatively expensive
// and not needed for many cases.
return replaceCallback();
return true;
}
else if constexpr (std::is_same_v<Policy, detail::ReplaceClient>)
{
return false;
}
else
{
@@ -340,11 +381,11 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
if (entry.isCached())
{
if (shouldReplace())
if (shouldReplaceCached())
{
entry.ptr = data;
}
else
else if constexpr (!replaceCached)
{
data = entry.ptr.getStrong();
}
@@ -356,11 +397,11 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
if (cachedData)
{
if (shouldReplace())
if (shouldReplaceCached())
{
entry.ptr = data;
}
else
else if constexpr (!replaceCached)
{
entry.ptr.convertToStrong();
data = cachedData;
@@ -376,6 +417,24 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
return false;
}
template <
class Key,
class T,
bool IsKeyCache,
class SharedWeakUnionPointer,
class SharedPointerType,
class Hash,
class KeyEqual,
class Mutex>
template <class Callback>
inline bool
TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
canonicalize(key_type const& key, SharedPointerType& data, Callback&& replaceCallback)
{
return canonicalizeImpl(
key, data, detail::ReplaceDynamically{}, std::forward<Callback>(replaceCallback));
}
template <
class Key,
class T,
@@ -389,7 +448,7 @@ inline bool
TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
canonicalizeReplaceCache(key_type const& key, SharedPointerType const& data)
{
return canonicalize(key, const_cast<SharedPointerType&>(data), []() { return true; });
return canonicalizeImpl(key, data, detail::ReplaceCached{});
}
template <
@@ -405,7 +464,7 @@ inline bool
TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
canonicalizeReplaceClient(key_type const& key, SharedPointerType& data)
{
return canonicalize(key, data, []() { return false; });
return canonicalizeImpl(key, data, detail::ReplaceClient{});
}
template <

View File

@@ -1,5 +1,7 @@
#include <test/unit_test/SuiteJournal.h>
#include <xrpl/basics/IntrusivePointer.h>
#include <xrpl/basics/IntrusiveRefCounts.h>
#include <xrpl/basics/TaggedCache.h>
#include <xrpl/basics/TaggedCache.ipp> // IWYU pragma: keep
#include <xrpl/basics/chrono.h>
@@ -8,6 +10,7 @@
#include <xrpl/protocol/Protocol.h>
#include <memory>
#include <utility>
namespace xrpl {
@@ -133,6 +136,113 @@ public:
BEAST_EXPECT(c.getCacheSize() == 0);
BEAST_EXPECT(c.getTrackSize() == 0);
}
{
BEAST_EXPECT(!c.insert(5, "five"));
BEAST_EXPECT(c.getCacheSize() == 1);
BEAST_EXPECT(c.size() == 1);
{
auto const p1 = c.fetch(5);
BEAST_EXPECT(p1 != nullptr);
BEAST_EXPECT(c.getCacheSize() == 1);
BEAST_EXPECT(c.size() == 1);
// Advance the clock a lot
++clock;
c.sweep();
BEAST_EXPECT(c.getCacheSize() == 0);
BEAST_EXPECT(c.size() == 1);
auto p2 = std::make_shared<std::string>("five_2");
BEAST_EXPECT(c.canonicalizeReplaceCache(5, p2));
BEAST_EXPECT(c.getCacheSize() == 1);
BEAST_EXPECT(c.size() == 1);
// Make sure the caller's original pointer is unchanged
BEAST_EXPECT(p1.get() != p2.get());
BEAST_EXPECT(*p2 == "five_2");
auto const p3 = c.fetch(5);
BEAST_EXPECT(p3 != nullptr);
BEAST_EXPECT(p3.get() == p2.get());
BEAST_EXPECT(p3.get() != p1.get());
}
++clock;
c.sweep();
BEAST_EXPECT(c.getCacheSize() == 0);
BEAST_EXPECT(c.size() == 0);
}
{
testcase("intrptr");
struct MyRefCountObject : IntrusiveRefCounts
{
std::string data;
// Needed to support weak intrusive pointers
virtual void
partialDestructor() {};
MyRefCountObject() = default;
explicit MyRefCountObject(std::string data) : data(std::move(data))
{
}
bool
operator==(std::string const& other) const
{
return data == other;
}
};
using IntrPtrCache = TaggedCache<
Key,
MyRefCountObject,
/*IsKeyCache*/ false,
intr_ptr::SharedWeakUnionPtr<MyRefCountObject>,
intr_ptr::SharedPtr<MyRefCountObject>>;
IntrPtrCache intrPtrCache("IntrPtrTest", 1, 1s, clock, journal);
intrPtrCache.canonicalizeReplaceCache(1, intr_ptr::makeShared<MyRefCountObject>("one"));
BEAST_EXPECT(intrPtrCache.getCacheSize() == 1);
BEAST_EXPECT(intrPtrCache.size() == 1);
{
{
intrPtrCache.canonicalizeReplaceCache(
1, intr_ptr::makeShared<MyRefCountObject>("one_replaced"));
auto p = intrPtrCache.fetch(1);
BEAST_EXPECT(*p == "one_replaced");
// Advance the clock a lot
++clock;
intrPtrCache.sweep();
BEAST_EXPECT(intrPtrCache.getCacheSize() == 0);
BEAST_EXPECT(intrPtrCache.size() == 1);
intrPtrCache.canonicalizeReplaceCache(
1, intr_ptr::makeShared<MyRefCountObject>("one_replaced_2"));
auto p2 = intrPtrCache.fetch(1);
BEAST_EXPECT(*p2 == "one_replaced_2");
intrPtrCache.del(1, true);
}
intrPtrCache.canonicalizeReplaceCache(
1, intr_ptr::makeShared<MyRefCountObject>("one_replaced_3"));
auto p3 = intrPtrCache.fetch(1);
BEAST_EXPECT(*p3 == "one_replaced_3");
}
++clock;
intrPtrCache.sweep();
BEAST_EXPECT(intrPtrCache.getCacheSize() == 0);
BEAST_EXPECT(intrPtrCache.size() == 0);
}
}
};

View File

@@ -36,6 +36,7 @@ public:
testcase("Convert protocol version to string");
BEAST_EXPECT(to_string(makeProtocol(1, 3)) == "XRPL/1.3");
BEAST_EXPECT(to_string(makeProtocol(2, 0)) == "XRPL/2.0");
BEAST_EXPECT(to_string(makeProtocol(2, 1)) == "XRPL/2.1");
BEAST_EXPECT(to_string(makeProtocol(10, 10)) == "XRPL/10.10");
{
@@ -58,10 +59,11 @@ public:
testcase("Protocol version negotiation");
BEAST_EXPECT(negotiateProtocolVersion("RTXP/1.2") == std::nullopt);
BEAST_EXPECT(negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/2.1") == std::nullopt);
BEAST_EXPECT(
negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/2.1") == makeProtocol(2, 1));
BEAST_EXPECT(negotiateProtocolVersion("XRPL/2.2") == makeProtocol(2, 2));
BEAST_EXPECT(
negotiateProtocolVersion("RTXP/1.2, XRPL/2.1, XRPL/2.2, XRPL/2.3, XRPL/999.999") ==
negotiateProtocolVersion("RTXP/1.2, XRPL/2.2, XRPL/2.3, XRPL/999.999") ==
makeProtocol(2, 2));
BEAST_EXPECT(negotiateProtocolVersion("XRPL/999.999, WebSocket/1.0") == std::nullopt);
BEAST_EXPECT(negotiateProtocolVersion("") == std::nullopt);

View File

@@ -53,4 +53,4 @@ foreach(module IN LISTS test_modules)
)
endforeach()
gtest_discover_tests(xrpl_tests DISCOVERY_TIMEOUT 60)
gtest_discover_tests(xrpl_tests)

View File

@@ -727,8 +727,17 @@ ValidatorList::sendValidatorList(
HashRouter& hashRouter,
beast::Journal j)
{
// v1 messages are no longer supported.
std::size_t const messageVersion = 2;
std::size_t messageVersion = 0;
if (peer.supportsFeature(ProtocolFeature::ValidatorList2Propagation))
{
messageVersion = 2;
}
else if (peer.supportsFeature(ProtocolFeature::ValidatorListPropagation))
{
messageVersion = 1;
}
if (messageVersion == 0u)
return;
auto const [newPeerSequence, numVLs] = buildValidatorListMessages(
messageVersion, peerSequence, maxSequence, rawVersion, rawManifest, blobInfos, messages);
if (newPeerSequence != 0u)
@@ -757,11 +766,24 @@ ValidatorList::sendValidatorList(
"xrpl::ValidatorList::sendValidatorList : sent or one message");
if (sent)
{
JLOG(j.debug()) << "Sent " << messages.size()
<< " validator list collection(s) containing " << numVLs
<< " validator list(s) for " << strHex(publisherKey)
<< " with sequence range " << peerSequence << ", " << newPeerSequence
<< " to " << peer.fingerprint();
if (messageVersion > 1)
{
JLOG(j.debug()) << "Sent " << messages.size()
<< " validator list collection(s) containing " << numVLs
<< " validator list(s) for " << strHex(publisherKey)
<< " with sequence range " << peerSequence << ", "
<< newPeerSequence << " to " << peer.fingerprint();
}
else
{
XRPL_ASSERT(
numVLs == 1,
"xrpl::ValidatorList::sendValidatorList : one validator "
"list");
JLOG(j.debug()) << "Sent validator list for " << strHex(publisherKey)
<< " with sequence " << newPeerSequence << " to "
<< peer.fingerprint();
}
}
}
}
@@ -836,9 +858,16 @@ ValidatorList::broadcastBlobs(
if (toSkip)
{
// Build v2 messages on demand and reuse them when possible. Messages
// are indexed by the peer's `publisherListSequence`; for each sequence,
// we only send VLs with higher sequences.
// We don't know what messages or message versions we're sending
// until we examine our peer's properties. Build the message(s) on
// demand, but reuse them when possible.
// This will hold a v1 message with only the current VL if we have
// any peers that don't support v2
std::vector<ValidatorList::MessageWithHash> messages1;
// This will hold v2 messages indexed by the peer's
// `publisherListSequence`. For each `publisherListSequence`, we'll
// only send the VLs with higher sequences.
std::map<std::size_t, std::vector<ValidatorList::MessageWithHash>> messages2;
// If any peers are found that are worth considering, this list will
// be built to hold info for all of the valid VLs.
@@ -858,6 +887,8 @@ ValidatorList::broadcastBlobs(
{
if (blobInfos.empty())
buildBlobInfos(blobInfos, lists);
auto const v2 =
peer->supportsFeature(ProtocolFeature::ValidatorList2Propagation);
sendValidatorList(
*peer,
peerSequence,
@@ -866,10 +897,11 @@ ValidatorList::broadcastBlobs(
lists.rawVersion,
lists.rawManifest,
blobInfos,
messages2[peerSequence],
v2 ? messages2[peerSequence] : messages1,
hashRouter,
j);
// Don't send it next time.
// Even if the peer doesn't support the messages,
// suppress it so it'll be ignored next time.
hashRouter.addSuppressionPeer(hash, peer->id());
}
}

View File

@@ -14,6 +14,8 @@ class Charge;
} // namespace Resource
enum class ProtocolFeature {
ValidatorListPropagation,
ValidatorList2Propagation,
LedgerReplay,
};

View File

@@ -545,6 +545,10 @@ PeerImp::supportsFeature(ProtocolFeature f) const
{
switch (f)
{
case ProtocolFeature::ValidatorListPropagation:
return protocol_ >= makeProtocol(2, 1);
case ProtocolFeature::ValidatorList2Propagation:
return protocol_ >= makeProtocol(2, 2);
case ProtocolFeature::LedgerReplay:
return ledgerReplayEnabled_;
}
@@ -888,7 +892,7 @@ PeerImp::doProtocolStart()
onReadMessage(error_code(), 0);
// Send all the validator lists that have been loaded
if (inbound_)
if (inbound_ && supportsFeature(ProtocolFeature::ValidatorListPropagation))
{
app_.getValidators().forEachAvailable(
[&](std::string const& manifest,
@@ -2271,6 +2275,14 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMValidatorList> const& m)
{
try
{
if (!supportsFeature(ProtocolFeature::ValidatorListPropagation))
{
JLOG(pJournal_.debug()) << "ValidatorList: received validator list from peer using "
<< "protocol version " << to_string(protocol_)
<< " which shouldn't support this feature.";
fee_.update(Resource::kFeeUselessData, "unsupported peer");
return;
}
onValidatorListMessage(
"ValidatorList", m->manifest(), m->version(), ValidatorList::parseBlobs(*m));
}
@@ -2287,6 +2299,14 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMValidatorListCollection> const& m
{
try
{
if (!supportsFeature(ProtocolFeature::ValidatorList2Propagation))
{
JLOG(pJournal_.debug()) << "ValidatorListCollection: received validator list from peer "
<< "using protocol version " << to_string(protocol_)
<< " which shouldn't support this feature.";
fee_.update(Resource::kFeeUselessData, "unsupported peer");
return;
}
if (m->version() < 2)
{
JLOG(pJournal_.debug())

View File

@@ -26,6 +26,7 @@ namespace xrpl {
*/
constexpr ProtocolVersion const kSupportedProtocolList[]{
{2, 1},
{2, 2},
};