diff --git a/src/test/consensus/ConsensusExtensions_test.cpp b/src/test/consensus/ConsensusExtensions_test.cpp index 6ee063c92..057d3d359 100644 --- a/src/test/consensus/ConsensusExtensions_test.cpp +++ b/src/test/consensus/ConsensusExtensions_test.cpp @@ -397,6 +397,22 @@ struct FakeExtensions return exportQuorum; } + // Membership is a no-op in the FakeExtensions tick tests (every peer + // counts, local counts) so the gate behavior is unchanged; F1 active-view + // filtering is exercised directly against inspectTxConvergedSidecarPeers. + template + bool + isUNLReportMember(Id const&) const + { + return true; + } + + bool + localIsActiveValidator() const + { + return true; + } + std::size_t pendingCommitCount() const { @@ -777,11 +793,18 @@ class ConsensusExtensions_test : public beast::unit_test::suite harness.addPeer(3, std::nullopt); harness.addPeer(4, localHash, makeHash("other-tx-set")); + auto const exportHashOf = [](auto const& position) { + return position.exportSigSetHash; + }; + auto const allMembers = [](auto const&) { return true; }; + std::vector fetched; auto const state = detail::inspectTxConvergedSidecarPeers( harness.peers, harness.position, - [](auto const& position) { return position.exportSigSetHash; }, + true, + exportHashOf, + allMembers, [&](auto const& hash) { if (hash) fetched.push_back(*hash); @@ -804,12 +827,56 @@ class ConsensusExtensions_test : public beast::unit_test::suite auto const unpublishedState = detail::inspectTxConvergedSidecarPeers( harness.peers, harness.position, - [](auto const& position) { return position.exportSigSetHash; }, + true, + exportHashOf, + allMembers, [](auto const&) {}); BEAST_EXPECT(!unpublishedState.localPublished); BEAST_EXPECT(unpublishedState.alignedParticipants() == 0); BEAST_EXPECT(!unpublishedState.quorumAligned(1)); BEAST_EXPECT(unpublishedState.fullObservation()); + + // F1: the alignment-counting universe must be the active validator + // view, not the full trusted-proposer set. A trusted-but-non-active + // proposer (node 5) that tx-converges and aligns on the SAME hash must + // NOT inflate alignedParticipants() — otherwise two equivocation + // cohorts padded by non-active peers could each clear the gate. + harness.position.exportSigSetHash = localHash; + harness.addPeer(5, localHash); // trusted, but outside the active view + auto const activeOnly = [](auto const& id) { + return id != makeNode(5); // nodes 1..4 active; 5 is not + }; + + auto const padded = detail::inspectTxConvergedSidecarPeers( + harness.peers, + harness.position, + true, + exportHashOf, + allMembers, + [](auto const&) {}); + BEAST_EXPECT(padded.aligned == 2); // node 1 + node 5, unfiltered + + auto const filtered = detail::inspectTxConvergedSidecarPeers( + harness.peers, + harness.position, + true, + exportHashOf, + activeOnly, + [](auto const&) {}); + BEAST_EXPECT(filtered.aligned == 1); // node 5 excluded + BEAST_EXPECT(filtered.alignedParticipants() == 2); // node 1 + local + BEAST_EXPECT(!filtered.quorumAligned(3)); // padding can't reach quorum + + // The local +1 is likewise gated on local active-view membership. + auto const nonActiveLocal = detail::inspectTxConvergedSidecarPeers( + harness.peers, + harness.position, + false, + exportHashOf, + activeOnly, + [](auto const&) {}); + BEAST_EXPECT(!nonActiveLocal.localPublished); + BEAST_EXPECT(nonActiveLocal.alignedParticipants() == 1); // node 1 only } void diff --git a/src/test/csf/Peer.h b/src/test/csf/Peer.h index 6c6487379..1cdfbecd6 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -721,6 +721,12 @@ struct Peer return unlNodes_.count(nodeId) > 0; } + bool + localIsActiveValidator() const + { + return isUNLReportMember(peer.id); + } + void finalizeRoundEntropy( std::uint32_t seq, diff --git a/src/xrpld/app/consensus/ConsensusExtensions.cpp b/src/xrpld/app/consensus/ConsensusExtensions.cpp index 2a386bba1..d12f8d672 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.cpp +++ b/src/xrpld/app/consensus/ConsensusExtensions.cpp @@ -987,6 +987,18 @@ ConsensusExtensions::isUNLReportMember(NodeID const& nodeId) const return activeValidatorView()->containsNode(nodeId); } +bool +ConsensusExtensions::localIsActiveValidator() const +{ + // Our own sidecar position only counts toward alignment when this validator + // is itself in the active view — the same universe as the peer-membership + // filter and the entropy/export thresholds. + auto const& valKeys = app_.getValidatorKeys(); + if (!valKeys.keys || valKeys.nodeID == beast::zero) + return false; + return activeValidatorView()->containsNode(valKeys.nodeID); +} + ConsensusExtensions::ActiveValidatorViewPtr ConsensusExtensions::activeValidatorView() const { diff --git a/src/xrpld/app/consensus/ConsensusExtensions.h b/src/xrpld/app/consensus/ConsensusExtensions.h index 407bdf1e7..1f58952f2 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.h +++ b/src/xrpld/app/consensus/ConsensusExtensions.h @@ -318,6 +318,12 @@ public: bool isUNLReportMember(NodeID const& nodeId) const; + // True only when THIS node's own validator key is in the active view. Used + // to gate our own +1 in the sidecar alignment count to the same universe as + // the peer-membership filter and the entropy/export thresholds. + bool + localIsActiveValidator() const; + void generateEntropySecret(); diff --git a/src/xrpld/consensus/ConsensusExtensionsTick.h b/src/xrpld/consensus/ConsensusExtensionsTick.h index e9f9c673f..808aebed7 100644 --- a/src/xrpld/consensus/ConsensusExtensionsTick.h +++ b/src/xrpld/consensus/ConsensusExtensionsTick.h @@ -37,12 +37,19 @@ struct SidecarPeerAlignment } }; -template +template < + class PeerPositions, + class Position, + class GetHash, + class IsMember, + class OnMismatch> SidecarPeerAlignment inspectTxConvergedSidecarPeers( PeerPositions const& peerPositions, Position const& pos, + bool localIsMember, GetHash getHash, + IsMember isMember, OnMismatch onMismatch) { SidecarPeerAlignment state; @@ -50,9 +57,21 @@ inspectTxConvergedSidecarPeers( if (!localHash) return state; - state.localPublished = true; - for (auto const& [_, peerPos] : peerPositions) + // The alignment-counting universe must be the active validator view, the + // same denominator the entropy/export thresholds use (quorumThreshold / + // tier2Threshold are computed over that view). A trusted-but-non-active + // proposer can tx-converge and advertise a sidecar hash, but it must NOT + // pad alignedParticipants(): counting outside the active view inflates the + // universe N above originalViewSize and erodes the Tier-2 intersection + // margin (2t - N) below the Byzantine floor f, breaking equivocation + // uniqueness. Mirror buildEntropySet/hasQuorumOfCommits' containsNode + // filter, and only count our own +1 when this node is itself active. + state.localPublished = localIsMember; + for (auto const& [nodeId, peerPos] : peerPositions) { + if (!isMember(nodeId)) + continue; // outside the active view -> not in the counting + // universe auto const& pp = peerPos.proposal().position(); if (!(pp == pos)) continue; // not tx-converged @@ -705,9 +724,13 @@ extensionsTick(Ext& ext, Ctx const& ctx) return detail::inspectTxConvergedSidecarPeers( ctx.peerPositions, pos, + ext.localIsActiveValidator(), [](auto const& position) { return position.entropySetHash; }, + [&ext](auto const& nodeId) { + return ext.isUNLReportMember(nodeId); + }, [&](auto const& hash) { if (fetchMismatches) ext.fetchRngSetIfNeeded( @@ -1210,9 +1233,13 @@ extensionsTick(Ext& ext, Ctx const& ctx) return detail::inspectTxConvergedSidecarPeers( ctx.peerPositions, pos, + ext.localIsActiveValidator(), [](auto const& position) { return position.exportSigSetHash; }, + [&ext](auto const& nodeId) { + return ext.isUNLReportMember(nodeId); + }, [&](auto const& hash) { if (fetchMismatches) ext.fetchRngSetIfNeeded(