ARCHIVE/xahaud
3
0
Fork 0
mirror of https://github.com/Xahau/xahaud.git synced 2026-08-23 16:30:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(consensus): make extended position identity explicit

Browse Source
This commit is contained in:
Nicholas Dudfield
2026-06-24 13:10:57 +07:00
parent f6d986bdbc
commit 9f6e7dd315
7 changed files with 57 additions and 124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
src/test/consensus/ExtendedPosition_test.cpp
View File

@@ -572,9 +572,9 @@ class ExtendedPosition_test : public beast::unit_test::suite
}
void
testEquality()
testTxSetIdentity()
{
testcase("Equality is txSetHash only");
testcase("Tx-set identity is explicit");
auto const txSet = makeHash("txset-eq");
auto const txSet2 = makeHash("txset-eq-2");
@@ -585,40 +585,36 @@ class ExtendedPosition_test : public beast::unit_test::suite
ExtendedPosition b{txSet};
b.myCommitment = makeHash("commit2-eq");
// Same txSetHash, different leaves -> equal
BEAST_EXPECT(a == b);
// Same txSetHash, different leaves -> same consensus tx-set key.
BEAST_EXPECT(positionTxSetID(a) == positionTxSetID(b));
// Same txSetHash, different commitSetHash -> still equal
// Same txSetHash, different commitSetHash -> same tx-set key
// (sub-state quorum handles commitSetHash agreement)
b.commitSetHash = makeHash("cs-eq");
BEAST_EXPECT(a == b);
BEAST_EXPECT(positionTxSetID(a) == positionTxSetID(b));
// Same txSetHash, different entropySetHash -> still equal
// Same txSetHash, different entropySetHash -> same tx-set key
b.entropySetHash = makeHash("es-eq");
BEAST_EXPECT(a == b);
BEAST_EXPECT(positionTxSetID(a) == positionTxSetID(b));
// Same txSetHash, different export signature digest -> still equal
// Same txSetHash, different export signature digest -> same tx-set key
b.exportSignaturesHash = makeHash("export-sigs-eq");
BEAST_EXPECT(a == b);
BEAST_EXPECT(positionTxSetID(a) == positionTxSetID(b));
// Same txSetHash, different participant diagnostics -> still equal
// Same txSetHash, different participant diagnostics -> same tx-set key
b.observedParticipantsHash = makeHash("participants-eq");
BEAST_EXPECT(a == b);
BEAST_EXPECT(positionTxSetID(a) == positionTxSetID(b));
// Different txSetHash -> not equal
// Different txSetHash -> different tx-set key
ExtendedPosition c{txSet2};
BEAST_EXPECT(a != c);
BEAST_EXPECT(positionTxSetID(a) != positionTxSetID(c));
BEAST_EXPECT(a == txSet);
BEAST_EXPECT(txSet == a);
BEAST_EXPECT(!(a != txSet));
BEAST_EXPECT(!(txSet != a));
BEAST_EXPECT(a != txSet2);
BEAST_EXPECT(txSet2 != a);
BEAST_EXPECT(positionTxSetID(a) == txSet);
BEAST_EXPECT(positionTxSetID(a) != txSet2);
a.updateTxSet(txSet2);
BEAST_EXPECT(a == txSet2);
BEAST_EXPECT(a != b);
BEAST_EXPECT(positionTxSetID(a) == txSet2);
BEAST_EXPECT(positionTxSetID(a) != positionTxSetID(b));
}
void
@@ -686,7 +682,8 @@ class ExtendedPosition_test : public beast::unit_test::suite
auto sameTxDifferentSidecar = pos;
sameTxDifferentSidecar.entropySetHash = makeHash("entropyset-other");
BEAST_EXPECT(pos == sameTxDifferentSidecar);
BEAST_EXPECT(
positionTxSetID(pos) == positionTxSetID(sameTxDifferentSidecar));
BEAST_EXPECT(sha512Half(pos) != sha512Half(sameTxDifferentSidecar));
}
@@ -699,7 +696,7 @@ public:
testSuppressionConsistency();
testPeerPosition();
testMalformedPayload();
testEquality();
testTxSetIdentity();
testExportSignatureDigest();
testStringJsonAndHash();
}

41
src/test/csf/Proposal.h
View File

@@ -53,54 +53,13 @@ struct RngPosition
{
}
operator TxSet::ID() const
{
return txSetHash;
}
void
updateTxSet(TxSet::ID txSet)
{
txSetHash = txSet;
}
bool
operator==(RngPosition const& other) const
{
return txSetHash == other.txSetHash;
}
bool
operator!=(RngPosition const& other) const
{
return !(*this == other);
}
bool
operator==(TxSet::ID txSet) const
{
return txSetHash == txSet;
}
bool
operator!=(TxSet::ID txSet) const
{
return txSetHash != txSet;
}
};
inline bool
operator==(TxSet::ID txSet, RngPosition const& pos)
{
return pos == txSet;
}
inline bool
operator!=(TxSet::ID txSet, RngPosition const& pos)
{
return pos != txSet;
}
inline std::string
to_string(RngPosition const& pos)
{

53
src/xrpld/app/consensus/RCLCxPeerPos.h
View File

@@ -42,11 +42,11 @@ namespace ripple {
/** Extended position for consensus with RNG entropy support.
Carries the tx-set hash (the core convergence target), RNG set hashes
(agreed via sub-state quorum, not via operator==), and per-validator
(agreed via sub-state quorum), and per-validator
leaves (unique to each proposer, piggybacked on proposals).
Critical design:
- operator== compares txSetHash ONLY (sub-states handle the rest)
- consensus convergence compares txSetHash explicitly
- add() includes ALL fields for signing (prevents stripping attacks)
*/
struct ExtendedPosition
@@ -71,12 +71,6 @@ struct ExtendedPosition
{
}
// Implicit conversion for legacy compatibility
operator uint256() const
{
return txSetHash;
}
// Helper to update TxSet while preserving sidecar data
void
updateTxSet(uint256 const& set)
@@ -84,7 +78,7 @@ struct ExtendedPosition
txSetHash = set;
}
// CRITICAL: Only compare txSetHash for consensus convergence.
// CRITICAL: consensus convergence compares txSetHash only.
//
// Why not commitSetHash / entropySetHash?
// Nodes transition through sub-states (ConvergingTx → ConvergingCommit
@@ -107,42 +101,11 @@ struct ExtendedPosition
// - Leaves (myCommitment, myReveal) are also excluded — they are
// per-validator data unique to each proposer.
//@@start rng-extended-position-equality
bool
operator==(ExtendedPosition const& other) const
{
return txSetHash == other.txSetHash;
}
bool
operator!=(ExtendedPosition const& other) const
{
return !(*this == other);
}
// Comparison with uint256 (compares txSetHash only)
bool
operator==(uint256 const& hash) const
{
return txSetHash == hash;
}
bool
operator!=(uint256 const& hash) const
{
return txSetHash != hash;
}
friend bool
operator==(uint256 const& hash, ExtendedPosition const& pos)
{
return pos.txSetHash == hash;
}
friend bool
operator!=(uint256 const& hash, ExtendedPosition const& pos)
{
return pos.txSetHash != hash;
}
// No operator== and no implicit uint256 conversion on purpose: comparing
// an ExtendedPosition as if it were a whole value is misleading because
// consensus convergence intentionally ignores sidecar hashes and leaves.
// Callers that need tx-set identity compare txSetHash explicitly, or use
// the generic positionTxSetID(position) helper.
//@@end rng-extended-position-equality
// CRITICAL: Include ALL fields for signing (prevents stripping attacks)

19
src/xrpld/consensus/Consensus.h
View File

@@ -931,13 +931,14 @@ Consensus<Adaptor>::peerProposalInternal(
<< "/" << newPeerProp.position();
{
auto const ait = acquired_.find(newPeerProp.position());
auto const txSetID = positionTxSetID(newPeerProp.position());
auto const ait = acquired_.find(txSetID);
if (ait == acquired_.end())
{
// acquireTxSet will return the set if it is available, or
// spawn a request for it and return nullopt/nullptr. It will call
// gotTxSet once it arrives
if (auto set = adaptor_.acquireTxSet(newPeerProp.position()))
if (auto set = adaptor_.acquireTxSet(txSetID))
gotTxSet(now_, *set);
else
JLOG(j_.debug()) << "Don't have tx set for peer";
@@ -1012,12 +1013,12 @@ Consensus<Adaptor>::gotTxSet(
// Our position is added to acquired_ as soon as we create it,
// so this txSet must differ
XRPL_ASSERT(
id != result_->position.position(),
id != positionTxSetID(result_->position.position()),
"ripple::Consensus::gotTxSet : updated transaction set");
bool any = false;
for (auto const& [nodeId, peerPos] : currPeerPositions_)
{
if (peerPos.proposal().position() == id)
if (positionTxSetID(peerPos.proposal().position()) == id)
{
updateDisputes(nodeId, txSet);
any = true;
@@ -1730,7 +1731,7 @@ Consensus<Adaptor>::closeLedger(std::unique_ptr<std::stringstream> const& clog)
for (auto const& pit : currPeerPositions_)
{
auto const& pos = pit.second.proposal().position();
auto const it = acquired_.find(pos);
auto const it = acquired_.find(positionTxSetID(pos));
if (it != acquired_.end())
createDisputes(it->second, clog);
}
@@ -1948,7 +1949,7 @@ Consensus<Adaptor>::updateOurPositions(
for (auto const& [nodeId, peerPos] : currPeerPositions_)
{
Proposal_t const& p = peerPos.proposal();
if (p.position() == newID)
if (positionTxSetID(p.position()) == newID)
updateDisputes(nodeId, result_->txns);
}
}
@@ -1977,7 +1978,8 @@ Consensus<Adaptor>::haveConsensus(
for (auto const& [nodeId, peerPos] : currPeerPositions_)
{
Proposal_t const& peerProp = peerPos.proposal();
if (peerProp.position() == ourPosition)
if (positionTxSetID(peerProp.position()) ==
positionTxSetID(ourPosition))
{
++agree;
}
@@ -2209,7 +2211,8 @@ Consensus<Adaptor>::createDisputes(
for (auto const& [nodeId, peerPos] : currPeerPositions_)
{
Proposal_t const& peerProp = peerPos.proposal();
auto const cit = acquired_.find(peerProp.position());
auto const cit =
acquired_.find(positionTxSetID(peerProp.position()));
if (cit != acquired_.end() &&
dtx.setVote(nodeId, cit->second.exists(txID)))
peerUnchangedCounter_ = 0;

8
src/xrpld/consensus/ConsensusExtensionsTick.h
View File

@@ -73,7 +73,7 @@ inspectTxConvergedSidecarPeers(
continue; // outside the active view -> not in the counting
// universe
auto const& pp = peerPos.proposal().position();
if (!(pp == pos))
if (positionTxSetID(pp) != positionTxSetID(pos))
continue; // not tx-converged
++state.txConverged;
@@ -445,7 +445,7 @@ extensionsTick(Ext& ext, Ctx const& ctx)
for (auto const& [nodeId, peerPos] : ctx.peerPositions)
{
auto const& peerPosition = peerPos.proposal().position();
if (!(peerPosition == ourPos))
if (peerPosition.txSetHash != ourPos.txSetHash)
continue;
ext.fetchRngSetIfNeeded(
peerPosition.commitSetHash, Ext::SidecarKind::commit);
@@ -483,7 +483,7 @@ extensionsTick(Ext& ext, Ctx const& ctx)
for (auto const& [nodeId, peerPos] : ctx.peerPositions)
{
auto const& peerPosition = peerPos.proposal().position();
if (!(peerPosition == ourPos))
if (peerPosition.txSetHash != ourPos.txSetHash)
continue;
if (note(peerPosition))
return true;
@@ -939,7 +939,7 @@ extensionsTick(Ext& ext, Ctx const& ctx)
for (auto const& [_, peerPos] : ctx.peerPositions)
{
auto const& pp = peerPos.proposal().position();
if (!(pp == pos))
if (positionTxSetID(pp) != positionTxSetID(pos))
continue; // not tx-converged
if (!pp.exportSigSetHash)
continue;

13
src/xrpld/consensus/ConsensusProposal.h
View File

@@ -29,6 +29,16 @@
#include <sstream>
namespace ripple {
template <class Position_t>
auto const&
positionTxSetID(Position_t const& position)
{
if constexpr (requires { position.txSetHash; })
return position.txSetHash;
else
return position;
}
/** Represents a proposed position taken during a round of consensus.
During consensus, peers seek agreement on a set of transactions to
@@ -275,7 +285,8 @@ operator==(
ConsensusProposal<NodeID_t, LedgerID_t, Position_t> const& b)
{
return a.nodeID() == b.nodeID() && a.proposeSeq() == b.proposeSeq() &&
a.prevLedger() == b.prevLedger() && a.position() == b.position() &&
a.prevLedger() == b.prevLedger() &&
positionTxSetID(a.position()) == positionTxSetID(b.position()) &&
a.closeTime() == b.closeTime() && a.seenTime() == b.seenTime();
}
} // namespace ripple

2
src/xrpld/consensus/ConsensusTypes.h
View File

@@ -272,7 +272,7 @@ struct ConsensusResult
: txns{std::move(s)}, position{std::move(p)}
{
XRPL_ASSERT(
txns.id() == position.position(),
txns.id() == positionTxSetID(position.position()),
"ripple::ConsensusResult : valid inputs");
}