fix: Use weighted median for close-time offset aggregation

This commit is contained in:
Bart
2026-07-16 16:30:47 -04:00
committed by Ayaz Salikhov
parent a5af6b4e4a
commit 981c256933
3 changed files with 82 additions and 15 deletions

View File

@@ -686,28 +686,17 @@ RCLConsensus::Adaptor::doAccept(
// close time reports, and update our clock.
if ((mode == ConsensusMode::Proposing || mode == ConsensusMode::Observing) && !consensusFail)
{
auto closeTime = rawCloseTimes.self;
JLOG(j_.info()) << "We closed at " << closeTime.time_since_epoch().count();
using usec64_t = std::chrono::duration<std::uint64_t>;
auto closeTotal = std::chrono::duration_cast<usec64_t>(closeTime.time_since_epoch());
JLOG(j_.info()) << "We closed at " << rawCloseTimes.self.time_since_epoch().count();
int closeCount = 1;
for (auto const& [t, v] : rawCloseTimes.peers)
{
JLOG(j_.info()) << std::to_string(v) << " time votes for "
<< std::to_string(t.time_since_epoch().count());
closeCount += v;
closeTotal += std::chrono::duration_cast<usec64_t>(t.time_since_epoch()) * v;
}
closeTotal += usec64_t(closeCount / 2); // for round to nearest
closeTotal /= closeCount;
// Use signed times since we are subtracting
using duration = std::chrono::duration<std::int32_t>;
using time_point = std::chrono::time_point<NetClock, duration>;
auto offset = time_point{closeTotal} - std::chrono::time_point_cast<duration>(closeTime);
// Median handles outliers better than mean.
auto const offset = medianCloseOffset(rawCloseTimes);
JLOG(j_.info()) << "Our close offset is estimated at " << offset.count() << " ("
<< closeCount << ")";

View File

@@ -22,6 +22,7 @@
#include <map>
#include <memory>
#include <optional>
#include <ranges>
#include <sstream>
#include <string>
#include <utility>
@@ -1580,7 +1581,13 @@ Consensus<Adaptor>::updateOurPositions(std::unique_ptr<std::stringstream> const&
JLOG(j_.info()) << ss.str();
CLOG(clog) << ss.str();
for (auto const& [t, v] : closeTimeVotes)
// Walk the votes highest-time first so that, among close times tied
// for the most votes, the earliest wins. The smaller value is the
// safer choice: without close-time consensus this round, the winner
// only updates our position for the next proposal, and a too-early
// time is bounded below by the prior ledger's close time. Only the
// tie-break changes; the bin with the most votes still wins.
for (auto const& [t, v] : std::views::reverse(closeTimeVotes))
{
JLOG(j_.debug()) << "CCTime: seq "
<< static_cast<std::uint32_t>(previousLedger_.seq()) + 1 << ": "

View File

@@ -9,7 +9,9 @@
#include <chrono>
#include <cstddef>
#include <cstdint>
#include <map>
#include <optional>
#include <string>
namespace xrpl {
@@ -190,6 +192,75 @@ struct ConsensusCloseTimes
NetClock::time_point self;
};
/**
* Offset of the network's close time relative to ours, using a weighted median.
*
* Treats the sample set as `{self x 1}` merged with `{t x w}` for each
* `(t, w)` in `times.peers`, in time order, and returns `(median - self)`
* in whole seconds. Uses the lower weighted median: the median is the
* earliest time at which the running weight reaches half the total, so an
* even total whose halfway point falls between two bins resolves to the
* earlier bin.
*
* @param times Our own close time and the weighted close times of peers.
* @return Weighted median of all close times minus our own, in whole seconds.
*/
inline std::chrono::seconds
medianCloseOffset(ConsensusCloseTimes const& times)
{
using namespace std::chrono;
using time_point = NetClock::time_point;
std::int64_t totalWeight = 1;
for (auto const& [_, w] : times.peers)
totalWeight += w;
std::int64_t const halfWeight = (totalWeight + 1) / 2;
std::optional<time_point> median{};
std::int64_t tally = 0;
bool selfPlaced = false;
// Accumulate weight in time order; the first bin to reach halfWeight is
// the (lower) weighted median. Returns true once that bin is found.
auto step = [&](time_point t, std::int64_t w) {
XRPL_ASSERT(tally < halfWeight, "xrpl::medianCloseOffset::step : median not yet found");
tally += w;
if (tally >= halfWeight)
{
median = t;
return true;
}
return false;
};
for (auto const& [t, w] : times.peers)
{
if (!selfPlaced && times.self <= t)
{
selfPlaced = true;
if (step(times.self, 1))
break;
}
if (step(t, w))
break;
}
if (!selfPlaced && !median)
step(times.self, 1);
if (!median)
{
// LCOV_EXCL_START
UNREACHABLE("xrpl::medianCloseOffset : median not found");
median = times.self;
// LCOV_EXCL_STOP
}
return duration_cast<seconds>(
duration<std::int64_t>{median->time_since_epoch().count()} -
duration<std::int64_t>{times.self.time_since_epoch().count()});
}
/**
* Whether we have or don't have a consensus
*/