ARCHIVE/xahaud
3
0
Fork 0
mirror of https://github.com/Xahau/xahaud.git synced 2026-09-26 23:29:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(overlay): close active feature cutoff races

Browse Source
This commit is contained in:
Nicholas Dudfield
2026-08-27 16:48:12 +07:00
parent 12a18040db
commit df0a779fed
8 changed files with 152 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

79
src/test/overlay/tx_reduce_relay_test.cpp
View File

@@ -121,6 +121,7 @@ private:
void
run() override
{
run_++;
}
void
send(std::shared_ptr<Message> const&) override
@@ -137,11 +138,13 @@ private:
{
queueTx_ = 0;
sendTx_ = 0;
run_ = 0;
sid_ = 0;
}
inline static std::size_t sid_ = 0;
inline static std::uint16_t queueTx_ = 0;
inline static std::uint16_t sendTx_ = 0;
inline static std::uint16_t run_ = 0;
};
std::uint16_t lid_{0};
@@ -246,6 +249,10 @@ private:
testcase("required protocol feature gate");
jtx::Env env(*this);
auto& overlay = dynamic_cast<OverlayImpl&>(env.app().overlay());
std::vector<std::shared_ptr<PeerTest>> peers;
PeerTest::init();
lid_ = 0;
rid_ = 1;
boost::beast::http::fields legacy;
boost::beast::http::fields capable;
@@ -256,17 +263,42 @@ private:
BEAST_EXPECT(!overlay.isProtocolFeatureRequired(
ProtocolFeature::ConsensusEntropy));
BEAST_EXPECT(!overlay.missingRequiredProtocolFeatureInHandshake(
legacy));
legacy, make_protocol(2, 2)));
// Leave a legacy peer in the active map while eviction is posted to its
// strand. Proposal filtering must take effect synchronously with the
// atomic requirement, without waiting for close().
std::uint16_t disabled = 1;
addPeer(env, peers, disabled);
BEAST_EXPECT(PeerTest::run_ == 1);
overlay.requireProtocolFeature(ProtocolFeature::ConsensusEntropy);
BEAST_EXPECT(overlay.isProtocolFeatureRequired(
ProtocolFeature::ConsensusEntropy));
BEAST_EXPECT(
overlay.missingRequiredProtocolFeatureInHandshake(legacy) ==
overlay.missingRequiredProtocolFeatureInHandshake(
legacy, make_protocol(2, 2)) ==
ProtocolFeature::ConsensusEntropy);
BEAST_EXPECT(!overlay.missingRequiredProtocolFeatureInHandshake(
capable));
capable, make_protocol(2, 2)));
protocol::TMProposeSet proposal;
proposal.set_proposeseq(1);
overlay.broadcast(proposal);
BEAST_EXPECT(PeerTest::sendTx_ == 0);
// A capable peer admitted after the cutoff still runs and receives
// proposals; a later legacy peer is inserted only for lifecycle cleanup
// and is failed before run().
disabled = 0;
addPeer(env, peers, disabled);
BEAST_EXPECT(PeerTest::run_ == 2);
overlay.broadcast(proposal);
BEAST_EXPECT(PeerTest::sendTx_ == 1);
disabled = 1;
addPeer(env, peers, disabled);
BEAST_EXPECT(PeerTest::run_ == 2);
// Requiring an already-required feature is intentionally idempotent.
overlay.requireProtocolFeature(ProtocolFeature::ConsensusEntropy);
@@ -274,6 +306,45 @@ private:
ProtocolFeature::ConsensusEntropy));
}
void
testProtocolFeatureGateFromLedger()
{
testcase("active ledger installs protocol feature gate");
auto config = jtx::envconfig();
config->features.insert(featureConsensusEntropy);
jtx::Env env(*this, std::move(config));
auto& overlay = dynamic_cast<OverlayImpl&>(env.app().overlay());
BEAST_EXPECT(overlay.isProtocolFeatureRequired(
ProtocolFeature::ConsensusEntropy));
}
void
testGenericProtocolFeatureAdmission()
{
testcase("generic protocol feature admission");
jtx::Env env(*this);
auto& overlay = dynamic_cast<OverlayImpl&>(env.app().overlay());
boost::beast::http::fields headers;
overlay.requireProtocolFeature(
ProtocolFeature::ValidatorList2Propagation);
BEAST_EXPECT(
overlay.missingRequiredProtocolFeatureInHandshake(
headers, make_protocol(2, 1)) ==
ProtocolFeature::ValidatorList2Propagation);
BEAST_EXPECT(!overlay.missingRequiredProtocolFeatureInHandshake(
headers, make_protocol(2, 2)));
overlay.requireProtocolFeature(ProtocolFeature::LedgerReplay);
BEAST_EXPECT(
overlay.missingRequiredProtocolFeatureInHandshake(
headers, make_protocol(2, 2)) ==
ProtocolFeature::LedgerReplay);
headers.set("X-Protocol-Ctl", "ledgerreplay=1;");
BEAST_EXPECT(!overlay.missingRequiredProtocolFeatureInHandshake(
headers, make_protocol(2, 2)));
}
void
run() override
{
@@ -281,6 +352,8 @@ private:
std::set<Peer::id_t> skip = {0, 1, 2, 3, 4};
testConfig(log);
testProtocolFeatureGate();
testProtocolFeatureGateFromLedger();
testGenericProtocolFeatureAdmission();
// relay to all peers, no hash queue
testRelay("feature disabled", false, 10, 0, 10, 25, 10, 0);
// relay to nPeers - skip (10-5=5)

8
src/xrpld/app/consensus/RCLConsensus.cpp
View File

@@ -671,6 +671,14 @@ RCLConsensus::Adaptor::doAccept(
auto const newLCLHash = built.id();
JLOG(j_.debug()) << "Built ledger #" << built.seq() << ": " << newLCLHash;
// Once the accepted result has built the enable-amendment ledger, the
// protocol cutoff is a fait accompli. Publish it before status notification
// or any remaining accept work; beginConsensus repeats this idempotently to
// cover startup and catch-up from an already-enabled ledger.
if (built.ledger_->rules().enabled(featureConsensusEntropy))
app_.overlay().requireProtocolFeature(
ProtocolFeature::ConsensusEntropy);
// Tell directly connected peers that we have a new LCL
notify(protocol::neACCEPTED_LEDGER, built, haveCorrectLCL);

8
src/xrpld/overlay/Peer.h
View File

@@ -26,6 +26,7 @@
#include <xrpl/json/json_value.h>
#include <xrpl/protocol/PublicKey.h>
#include <array>
#include <string_view>
namespace ripple {
@@ -41,6 +42,13 @@ enum class ProtocolFeature {
ConsensusEntropy,
};
inline constexpr std::array allProtocolFeatures = {
ProtocolFeature::ValidatorListPropagation,
ProtocolFeature::ValidatorList2Propagation,
ProtocolFeature::LedgerReplay,
ProtocolFeature::ConsensusEntropy,
};
/** Stable diagnostic name for a negotiated peer-protocol feature. */
constexpr std::string_view
protocolFeatureName(ProtocolFeature feature)

23
src/xrpld/overlay/ProtocolFeatureRequirements.md
View File

@@ -39,23 +39,26 @@ For the observed rollout, the relevant ledgers are:
| 257 | contains `EnableAmendment`; its resulting rules enable CE | install the required-capability gate |
| 258 | first consensus round built with CE active | only capable sessions may participate |
`NetworkOPsImp::beginConsensus` is the boundary. Its `prevLedger` is the newly
closed ledger whose rules govern the round about to start. If those rules
enable Consensus Entropy, it calls:
As soon as the accepted consensus result has built ledger 257, `doAccept`
installs the requirement before status notification and the remainder of the
accept work. It does not wait for ledger 257 to become fully validated or for
the next round to start:
```cpp
app.overlay().requireProtocolFeature(
ProtocolFeature::ConsensusEntropy);
```
This happens before `RCLConsensus::startRound`. Thus accepting ledger 257
disconnects incompatible sessions before any ledger-258 proposal is created or
relayed. Local `VOTE`/`VETO` state is not used: a validator that locally vetoed
the amendment must still obey the rules of the ledger the network accepted.
`NetworkOPsImp::beginConsensus` repeats the same idempotent call from its
`prevLedger`, the newly closed ledger whose rules govern the round about to
start. Thus accepting ledger 257 disconnects incompatible sessions before any
ledger-258 proposal is created or relayed. Local `VOTE`/`VETO` state is not
used: a validator that locally vetoed the amendment must still obey the rules
of the ledger the network accepted.
The same path runs for the first consensus round after a restart or catch-up,
so a node starting from an already-enabled ledger installs the requirement
before joining consensus.
The `beginConsensus` path also runs for the first round after a restart or
catch-up, so a node starting from an already-enabled ledger installs the
requirement before joining consensus.
## Session behavior

3
src/xrpld/overlay/detail/ConnectAttempt.cpp
View File

@@ -364,7 +364,8 @@ ConnectAttempt::processResponse()
app_);
if (auto const missing =
overlay_.missingRequiredProtocolFeatureInHandshake(response_))
overlay_.missingRequiredProtocolFeatureInHandshake(
response_, *negotiatedProtocol))
return fail(
"Handshake missing required protocol feature " +
std::string(protocolFeatureName(*missing)));

21
src/xrpld/overlay/detail/OverlayImpl.cpp
View File

@@ -273,7 +273,8 @@ OverlayImpl::onHandoff(
app_);
if (auto const missing =
missingRequiredProtocolFeatureInHandshake(request))
missingRequiredProtocolFeatureInHandshake(
request, *negotiatedVersion))
throw std::runtime_error(
"Handshake missing required protocol feature " +
std::string(protocolFeatureName(*missing)));
@@ -632,6 +633,9 @@ OverlayImpl::activate(std::shared_ptr<PeerImp> const& peer)
<< "Rejected active peer missing required protocol feature "
<< protocolFeatureName(*missing) << " from "
<< peer->getRemoteAddress();
peer->fail(
"Missing required protocol feature " +
std::string(protocolFeatureName(*missing)));
return false;
}
@@ -1196,9 +1200,10 @@ OverlayImpl::isProtocolFeatureRequired(ProtocolFeature feature) const
std::optional<ProtocolFeature>
OverlayImpl::missingRequiredProtocolFeature(Peer const& peer) const
{
if (isProtocolFeatureRequired(ProtocolFeature::ConsensusEntropy) &&
!peer.supportsFeature(ProtocolFeature::ConsensusEntropy))
return ProtocolFeature::ConsensusEntropy;
for (auto const feature : allProtocolFeatures)
if (isProtocolFeatureRequired(feature) &&
!peer.supportsFeature(feature))
return feature;
return std::nullopt;
}
@@ -1206,7 +1211,10 @@ void
OverlayImpl::broadcast(protocol::TMProposeSet& m)
{
auto const sm = std::make_shared<Message>(m, protocol::mtPROPOSE_LEDGER);
for_each([&](std::shared_ptr<PeerImp>&& p) { p->send(sm); });
for_each([&](std::shared_ptr<PeerImp>&& p) {
if (!missingRequiredProtocolFeature(*p))
p->send(sm);
});
}
std::set<Peer::id_t>
@@ -1220,7 +1228,8 @@ OverlayImpl::relay(
auto const sm =
std::make_shared<Message>(m, protocol::mtPROPOSE_LEDGER, validator);
for_each([&](std::shared_ptr<PeerImp>&& p) {
if (toSkip->find(p->id()) == toSkip->end())
if (toSkip->find(p->id()) == toSkip->end() &&
!missingRequiredProtocolFeature(*p))
p->send(sm);
});
return *toSkip;

33
src/xrpld/overlay/detail/OverlayImpl.h
View File

@@ -279,11 +279,36 @@ public:
/** Return the first mandatory feature missing from handshake headers. */
template <class Headers>
std::optional<ProtocolFeature>
missingRequiredProtocolFeatureInHandshake(Headers const& headers) const
missingRequiredProtocolFeatureInHandshake(
Headers const& headers,
ProtocolVersion const& protocol) const
{
if (isProtocolFeatureRequired(ProtocolFeature::ConsensusEntropy) &&
!peerFeatureEnabled(headers, FEATURE_CONSENSUS_ENTROPY, true))
return ProtocolFeature::ConsensusEntropy;
for (auto const feature : allProtocolFeatures)
{
if (!isProtocolFeatureRequired(feature))
continue;
bool supported = false;
switch (feature)
{
case ProtocolFeature::ValidatorListPropagation:
supported = protocol >= make_protocol(2, 1);
break;
case ProtocolFeature::ValidatorList2Propagation:
supported = protocol >= make_protocol(2, 2);
break;
case ProtocolFeature::LedgerReplay:
supported = peerFeatureEnabled(
headers, FEATURE_LEDGER_REPLAY, true);
break;
case ProtocolFeature::ConsensusEntropy:
supported = peerFeatureEnabled(
headers, FEATURE_CONSENSUS_ENTROPY, true);
break;
}
if (!supported)
return feature;
}
return std::nullopt;
}

2
src/xrpld/overlay/detail/PeerImp.cpp
View File

@@ -866,7 +866,7 @@ PeerImp::doAccept()
}
if (!overlay_.activate(shared_from_this()))
return fail("Missing required protocol feature");
return;
// XXX Set timer: connection is in grace period to be useful.
// XXX Set timer: connection idle (idle may vary depending on connection