From c44dea3acfd7b2bfbbd17b640f32e32ffb91020f Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Fri, 6 Feb 2026 09:03:26 +0700 Subject: [PATCH] fix(consensus): resolve commit-reveal pipeline bugs enabling non-zero entropy Three critical fixes that unblock the RNG commit-reveal pipeline: - Remove entropy secret regeneration in ConvergingTx->ConvergingCommit transition that was overwriting the onClose() secret, breaking reveal verification against the original commitment - Change ExtendedPosition operator== to compare txSetHash only, preventing deadlock where nodes transitioning sub-states at different times would break haveConsensus() for all peers - Self-seed own commitment and reveal into pending collections so the node counts toward its own quorum checks Also adds ExtendedPosition_test with signing, suppression, serialization round-trip and equality tests, iterator safety fix in BuildLedger, wire compatibility early-return, and RNG debug logging throughout the pipeline. --- Builds/CMake/RippledCore.cmake | 1 + src/ripple/app/consensus/RCLConsensus.cpp | 69 +++- src/ripple/app/consensus/RCLCxPeerPos.cpp | 22 +- src/ripple/app/consensus/RCLCxPeerPos.h | 53 ++- src/ripple/app/ledger/impl/BuildLedger.cpp | 61 ++-- src/ripple/consensus/Consensus.h | 41 +-- src/ripple/overlay/impl/PeerImp.cpp | 12 +- src/test/consensus/ExtendedPosition_test.cpp | 321 +++++++++++++++++++ 8 files changed, 501 insertions(+), 79 deletions(-) create mode 100644 src/test/consensus/ExtendedPosition_test.cpp diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 74ff58f60..b732b66a1 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -848,6 +848,7 @@ if (tests) #]===============================] src/test/consensus/ByzantineFailureSim_test.cpp src/test/consensus/Consensus_test.cpp + src/test/consensus/ExtendedPosition_test.cpp src/test/consensus/DistributedValidatorsSim_test.cpp src/test/consensus/LedgerTiming_test.cpp src/test/consensus/LedgerTrie_test.cpp diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index a86c100df..42bb5de9d 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -223,6 +223,22 @@ RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) auto const posSlice = positionData.slice(); prop.set_currenttxhash(posSlice.data(), posSlice.size()); + JLOG(j_.info()) << "RNG: propose seq=" << proposal.proposeSeq() + << " wireBytes=" << posSlice.size() << " commit=" + << (proposal.position().myCommitment ? "yes" : "no") + << " reveal=" + << (proposal.position().myReveal ? "yes" : "no"); + + // Self-seed our own reveal so we count toward reveal quorum + // (harvestRngData only sees peer proposals, not our own). + if (proposal.position().myReveal) + { + auto const ownNodeId = validatorKeys_.nodeID; + pendingReveals_[ownNodeId] = *proposal.position().myReveal; + nodeIdToKey_[ownNodeId] = validatorKeys_.publicKey; + JLOG(j_.debug()) << "RNG: self-seeded reveal for " << ownNodeId; + } + prop.set_previousledger( proposal.prevLedger().begin(), proposal.prevLedger().size()); prop.set_proposeseq(proposal.proposeSeq()); @@ -397,12 +413,42 @@ RCLConsensus::Adaptor::onClose( // Needed because of the move below. auto const setHash = initialSet->getHash().as_uint256(); + ExtendedPosition pos{setHash}; + + // Bootstrap commit-reveal: generate entropy and include commitment + // in our very first proposal so peers can collect it during consensus. + if (proposing && prevLedger->rules().enabled(featureConsensusEntropy)) + { + generateEntropySecret(); + pos.myCommitment = sha512Half( + myEntropySecret_, + validatorKeys_.publicKey, + prevLedger->info().seq + 1); + + // Seed our own commitment into pendingCommits_ so we count + // toward quorum (harvestRngData only sees peer proposals). + auto const ownNodeId = validatorKeys_.nodeID; + pendingCommits_[ownNodeId] = *pos.myCommitment; + nodeIdToKey_[ownNodeId] = validatorKeys_.publicKey; + + JLOG(j_.info()) << "RNG: onClose bootstrap seq=" + << (prevLedger->info().seq + 1) + << " commitment=" << *pos.myCommitment; + } + else + { + JLOG(j_.debug()) << "RNG: onClose skipped (proposing=" << proposing + << " amendment=" + << prevLedger->rules().enabled(featureConsensusEntropy) + << ")"; + } + return Result{ std::move(initialSet), RCLCxPeerPos::Proposal{ initialLedger->info().parentHash, RCLCxPeerPos::Proposal::seqJoin, - ExtendedPosition{setHash}, + std::move(pos), closeTime, app_.timeKeeper().closeTime(), validatorKeys_.nodeID}}; @@ -1080,13 +1126,21 @@ RCLConsensus::Adaptor::quorumThreshold() const bool RCLConsensus::Adaptor::hasQuorumOfCommits() const { - return pendingCommits_.size() >= quorumThreshold(); + auto threshold = quorumThreshold(); + bool result = pendingCommits_.size() >= threshold; + JLOG(j_.debug()) << "RNG: hasQuorumOfCommits? " << pendingCommits_.size() + << "/" << threshold << " -> " << (result ? "YES" : "no"); + return result; } bool RCLConsensus::Adaptor::hasMinimumReveals() const { - return pendingReveals_.size() >= quorumThreshold(); + auto threshold = quorumThreshold(); + bool result = pendingReveals_.size() >= threshold; + JLOG(j_.debug()) << "RNG: hasMinimumReveals? " << pendingReveals_.size() + << "/" << threshold << " -> " << (result ? "YES" : "no"); + return result; } bool @@ -1184,6 +1238,11 @@ RCLConsensus::Adaptor::injectEntropyPseudoTx( CanonicalTXSet& retriableTxs, LedgerIndex seq) { + JLOG(j_.info()) << "RNG: injectEntropy seq=" << seq + << " commits=" << pendingCommits_.size() + << " reveals=" << pendingReveals_.size() + << " failed=" << entropyFailed_; + uint256 finalEntropy; bool hasEntropy = false; @@ -1258,6 +1317,10 @@ RCLConsensus::Adaptor::harvestRngData( PublicKey const& publicKey, ExtendedPosition const& position) { + JLOG(j_.debug()) << "RNG: harvestRngData from " << nodeId + << " commit=" << (position.myCommitment ? "yes" : "no") + << " reveal=" << (position.myReveal ? "yes" : "no"); + // Store nodeId -> publicKey mapping for deterministic ordering nodeIdToKey_[nodeId] = publicKey; diff --git a/src/ripple/app/consensus/RCLCxPeerPos.cpp b/src/ripple/app/consensus/RCLCxPeerPos.cpp index 24c6b81aa..db9f4c299 100644 --- a/src/ripple/app/consensus/RCLCxPeerPos.cpp +++ b/src/ripple/app/consensus/RCLCxPeerPos.cpp @@ -47,16 +47,8 @@ RCLCxPeerPos::RCLCxPeerPos( bool RCLCxPeerPos::checkSign() const { - // Use proposalUniqueId to ensure full ExtendedPosition is covered - auto const signingHash = proposalUniqueId( - proposal_.position(), - proposal_.prevLedger(), - proposal_.proposeSeq(), - proposal_.closeTime(), - publicKey_.slice(), - Slice{nullptr, 0}); // Exclude signature for signing hash - - return verifyDigest(publicKey(), signingHash, signature(), false); + return verifyDigest( + publicKey(), proposal_.signingHash(), signature(), false); } Json::Value @@ -79,15 +71,13 @@ proposalUniqueId( Slice const& publicKey, Slice const& signature) { + // This is for suppression/dedup only, NOT for signing. + // Must include all fields that distinguish proposals. Serializer s(512); - s.add32(HashPrefix::proposal); + position.add(s); + s.addBitString(previousLedger); s.add32(proposeSeq); s.add32(closeTime.time_since_epoch().count()); - s.addBitString(previousLedger); - - // Serialize full ExtendedPosition (TxSet + Sets + Leaves) - position.add(s); - s.addVL(publicKey); s.addVL(signature); diff --git a/src/ripple/app/consensus/RCLCxPeerPos.h b/src/ripple/app/consensus/RCLCxPeerPos.h index f0ae6499f..9758a1568 100644 --- a/src/ripple/app/consensus/RCLCxPeerPos.h +++ b/src/ripple/app/consensus/RCLCxPeerPos.h @@ -40,21 +40,24 @@ namespace ripple { /** Extended position for consensus with RNG entropy support. - Carries both the consensus targets (set hashes that require agreement) - and pipelined leaves (per-validator data transported via gossip). + Carries the tx-set hash (the core convergence target), RNG set hashes + (agreed via sub-state quorum, not via operator==), and per-validator + leaves (unique to each proposer, piggybacked on proposals). Critical design: - - operator== excludes leaves (allows convergence with unique leaves) - - add() includes ALL fields (prevents signature stripping attacks) + - operator== compares txSetHash ONLY (sub-states handle the rest) + - add() includes ALL fields for signing (prevents stripping attacks) */ struct ExtendedPosition { - // === Consensus Targets (Agreement Required) === + // === Core Convergence Target === uint256 txSetHash; + + // === RNG Set Hashes (sub-state quorum, not in operator==) === std::optional commitSetHash; std::optional entropySetHash; - // === Pipelined Leaves (No Agreement Required) === + // === Per-Validator Leaves (unique per proposer) === std::optional myCommitment; std::optional myReveal; @@ -76,13 +79,36 @@ struct ExtendedPosition txSetHash = set; } - // CRITICAL: Exclude leaves from equality - consensus only on set hashes + // TODO: replace operator== with a named method (e.g. txSetMatches()) + // so call sites read as intent, not as "full equality". Overloading + // operator== to ignore most fields is surprising and fragile. + // + // CRITICAL: Only compare txSetHash for consensus convergence. + // + // Why not commitSetHash / entropySetHash? + // Nodes transition through sub-states (ConvergingTx → ConvergingCommit + // → ConvergingReveal) at slightly different times. If we included + // commitSetHash here, a node that transitions first would set it, + // making its position "different" from peers who haven't transitioned + // yet — deadlocking haveConsensus() for everyone. + // + // Instead, the sub-state machine in phaseEstablish handles agreement + // on those fields via quorum checks (hasQuorumOfCommits, etc.). + // + // Implications to consider: + // - Two nodes with the same txSetHash but different commitSetHash + // will appear to "agree" from the convergence engine's perspective. + // This is intentional: tx consensus must not be blocked by RNG. + // - A malicious node could propose a different commitSetHash without + // affecting tx convergence. This is safe because commitSetHash + // disagreement is caught by the sub-state quorum checks, and the + // entropy result is verified deterministically from collected reveals. + // - Leaves (myCommitment, myReveal) are also excluded — they are + // per-validator data unique to each proposer. bool operator==(ExtendedPosition const& other) const { - return txSetHash == other.txSetHash && - commitSetHash == other.commitSetHash && - entropySetHash == other.entropySetHash; + return txSetHash == other.txSetHash; } bool @@ -122,6 +148,11 @@ struct ExtendedPosition { s.addBitString(txSetHash); + // Wire compatibility: if no extensions, emit exactly 32 bytes + // so legacy nodes that expect a plain uint256 work unchanged. + if (!commitSetHash && !entropySetHash && !myCommitment && !myReveal) + return; + std::uint8_t flags = 0; if (commitSetHash) flags |= 0x01; @@ -221,7 +252,7 @@ class RCLCxPeerPos { public: //< The type of the proposed position (uses ExtendedPosition for RNG - //support) + // support) using Proposal = ConsensusProposal; /** Constructor diff --git a/src/ripple/app/ledger/impl/BuildLedger.cpp b/src/ripple/app/ledger/impl/BuildLedger.cpp index d6fd49a43..b7a86bb50 100644 --- a/src/ripple/app/ledger/impl/BuildLedger.cpp +++ b/src/ripple/app/ledger/impl/BuildLedger.cpp @@ -106,38 +106,41 @@ applyTransactions( // CRITICAL: Apply consensus entropy pseudo-tx FIRST before any other // transactions. This ensures hooks can read entropy during this ledger. - for (auto it = txns.begin(); it != txns.end(); ++it) + for (auto it = txns.begin(); it != txns.end(); /* manual */) { - if (it->second->getTxnType() == ttCONSENSUS_ENTROPY) + if (it->second->getTxnType() != ttCONSENSUS_ENTROPY) { - auto const txid = it->first.getTXID(); - JLOG(j.debug()) << "Applying entropy tx FIRST: " << txid; - - try - { - auto const result = - applyTransaction(app, view, *it->second, true, tapNONE, j); - - if (result == ApplyResult::Success) - { - ++count; - JLOG(j.debug()) << "Entropy tx applied successfully"; - } - else - { - failed.insert(txid); - JLOG(j.warn()) << "Entropy tx failed to apply"; - } - } - catch (std::exception const& ex) - { - JLOG(j.warn()) << "Entropy tx throws: " << ex.what(); - failed.insert(txid); - } - - txns.erase(it); - break; // Only one entropy tx per ledger + ++it; + continue; } + + auto const txid = it->first.getTXID(); + JLOG(j.debug()) << "Applying entropy tx FIRST: " << txid; + + try + { + auto const result = + applyTransaction(app, view, *it->second, true, tapNONE, j); + + if (result == ApplyResult::Success) + { + ++count; + JLOG(j.debug()) << "Entropy tx applied successfully"; + } + else + { + failed.insert(txid); + JLOG(j.warn()) << "Entropy tx failed to apply"; + } + } + catch (std::exception const& ex) + { + JLOG(j.warn()) << "Entropy tx throws: " << ex.what(); + failed.insert(txid); + } + + it = txns.erase(it); + break; // Only one entropy tx per ledger } // Attempt to apply all of the retriable transactions diff --git a/src/ripple/consensus/Consensus.h b/src/ripple/consensus/Consensus.h index 02215641b..fe80720b7 100644 --- a/src/ripple/consensus/Consensus.h +++ b/src/ripple/consensus/Consensus.h @@ -24,12 +24,12 @@ #include #include #include -#include #include #include #include #include #include +#include #include #include #include @@ -701,7 +701,7 @@ Consensus::startRoundInternal( deadNodes_.clear(); // Reset RNG state for new round if adaptor supports it - if constexpr (requires(Adaptor& a) { a.clearRngState(); }) + if constexpr (requires(Adaptor & a) { a.clearRngState(); }) { adaptor_.clearRngState(); } @@ -807,13 +807,17 @@ Consensus::peerProposalInternal( } // Harvest RNG data from proposal if adaptor supports it - if constexpr (requires(Adaptor& a, PeerPosition_t const& pp) { + if constexpr (requires(Adaptor & a, PeerPosition_t const& pp) { a.harvestRngData( pp.proposal().nodeID(), pp.publicKey(), pp.proposal().position()); }) { + JLOG(j_.debug()) << "RNG: peerProposal from " << peerID << " commit=" + << (newPeerProp.position().myCommitment ? "yes" : "no") + << " reveal=" + << (newPeerProp.position().myReveal ? "yes" : "no"); adaptor_.harvestRngData( peerID, newPeerPos.publicKey(), newPeerProp.position()); } @@ -1326,36 +1330,37 @@ Consensus::phaseEstablish() } // --- RNG Sub-state Checkpoints (if adaptor supports RNG) --- - if constexpr (requires(Adaptor& a) { + if constexpr (requires(Adaptor & a) { a.hasQuorumOfCommits(); a.buildCommitSet(); a.generateEntropySecret(); }) { + JLOG(j_.debug()) << "RNG: phaseEstablish estState=" + << static_cast(estState_); + if (estState_ == EstablishState::ConvergingTx) { if (adaptor_.hasQuorumOfCommits()) { auto commitSetHash = adaptor_.buildCommitSet(); - adaptor_.generateEntropySecret(); + // Keep the same entropy secret from onClose() — do NOT + // regenerate. The commitment in the commitSet was built + // from that original secret; regenerating would make the + // later reveal fail verification. auto newPos = result_->position.position(); newPos.commitSetHash = commitSetHash; - newPos.myCommitment = sha512Half( - adaptor_.getEntropySecret(), - adaptor_.validatorKey(), - previousLedger_.seq() + typename Ledger_t::Seq{1}); result_->position.changePosition( - newPos, - asCloseTime(result_->position.closeTime()), - now_); + newPos, asCloseTime(result_->position.closeTime()), now_); if (mode_.get() == ConsensusMode::proposing) adaptor_.propose(result_->position); estState_ = EstablishState::ConvergingCommit; - JLOG(j_.debug()) << "RNG: transitioned to ConvergingCommit"; + JLOG(j_.debug()) << "RNG: transitioned to ConvergingCommit" + << " commitSet=" << commitSetHash; return; // Wait for next tick } } @@ -1366,20 +1371,20 @@ Consensus::phaseEstablish() newPos.myReveal = adaptor_.getEntropySecret(); result_->position.changePosition( - newPos, - asCloseTime(result_->position.closeTime()), - now_); + newPos, asCloseTime(result_->position.closeTime()), now_); if (mode_.get() == ConsensusMode::proposing) adaptor_.propose(result_->position); estState_ = EstablishState::ConvergingReveal; - JLOG(j_.debug()) << "RNG: transitioned to ConvergingReveal"; + JLOG(j_.debug()) << "RNG: transitioned to ConvergingReveal" + << " reveal=" << adaptor_.getEntropySecret(); return; // Wait for next tick } else if (estState_ == EstablishState::ConvergingReveal) { - bool timeout = result_->roundTime.read() > parms.ledgerMAX_CONSENSUS; + bool timeout = + result_->roundTime.read() > parms.ledgerMAX_CONSENSUS; bool ready = false; if ((haveConsensus() && adaptor_.hasMinimumReveals()) || timeout) diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 872404d60..eaa00ca22 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -1935,7 +1935,8 @@ PeerImp::onMessage(std::shared_ptr const& m) return; } - // Position data must be at least 32 bytes (txSetHash), previous ledger exactly 32 + // Position data must be at least 32 bytes (txSetHash), previous ledger + // exactly 32 if (set.currenttxhash().size() < 32 || !stringIsUint256Sized(set.previousledger())) { @@ -1956,12 +1957,19 @@ PeerImp::onMessage(std::shared_ptr const& m) if (!isTrusted && app_.config().RELAY_UNTRUSTED_PROPOSALS == -1) return; - // Deserialize ExtendedPosition (handles both legacy 32-byte and extended formats) + // Deserialize ExtendedPosition (handles both legacy 32-byte and extended + // formats) auto const positionSlice = makeSlice(set.currenttxhash()); SerialIter sit(positionSlice); ExtendedPosition const position = ExtendedPosition::fromSerialIter(sit, positionSlice.size()); + JLOG(p_journal_.debug()) + << "RNG: recv proposal size=" << positionSlice.size() + << " commit=" << (position.myCommitment ? "yes" : "no") + << " reveal=" << (position.myReveal ? "yes" : "no") << " from " + << toBase58(TokenType::NodePublic, publicKey); + uint256 const prevLedger{set.previousledger()}; NetClock::time_point const closeTime{NetClock::duration{set.closetime()}}; diff --git a/src/test/consensus/ExtendedPosition_test.cpp b/src/test/consensus/ExtendedPosition_test.cpp new file mode 100644 index 000000000..86ea74bb8 --- /dev/null +++ b/src/test/consensus/ExtendedPosition_test.cpp @@ -0,0 +1,321 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include + +namespace ripple { +namespace test { + +class ExtendedPosition_test : public beast::unit_test::suite +{ + // Generate deterministic test hashes + static uint256 + makeHash(char const* label) + { + return sha512Half(Slice(label, std::strlen(label))); + } + + void + testSerializationRoundTrip() + { + testcase("Serialization round-trip"); + + // Empty position (legacy compat) + { + auto const txSet = makeHash("txset-a"); + ExtendedPosition pos{txSet}; + + Serializer s; + pos.add(s); + + // Should be exactly 32 bytes (no flags byte) + BEAST_EXPECT(s.getDataLength() == 32); + + SerialIter sit(s.slice()); + auto deserialized = + ExtendedPosition::fromSerialIter(sit, s.getDataLength()); + + BEAST_EXPECT(deserialized.txSetHash == txSet); + BEAST_EXPECT(!deserialized.myCommitment); + BEAST_EXPECT(!deserialized.myReveal); + BEAST_EXPECT(!deserialized.commitSetHash); + BEAST_EXPECT(!deserialized.entropySetHash); + } + + // Position with commitment + { + auto const txSet = makeHash("txset-b"); + auto const commit = makeHash("commit-b"); + + ExtendedPosition pos{txSet}; + pos.myCommitment = commit; + + Serializer s; + pos.add(s); + + // 32 (txSet) + 1 (flags) + 32 (commitment) = 65 + BEAST_EXPECT(s.getDataLength() == 65); + + SerialIter sit(s.slice()); + auto deserialized = + ExtendedPosition::fromSerialIter(sit, s.getDataLength()); + + BEAST_EXPECT(deserialized.txSetHash == txSet); + BEAST_EXPECT(deserialized.myCommitment == commit); + BEAST_EXPECT(!deserialized.myReveal); + } + + // Position with all fields + { + auto const txSet = makeHash("txset-c"); + auto const commitSet = makeHash("commitset-c"); + auto const entropySet = makeHash("entropyset-c"); + auto const commit = makeHash("commit-c"); + auto const reveal = makeHash("reveal-c"); + + ExtendedPosition pos{txSet}; + pos.commitSetHash = commitSet; + pos.entropySetHash = entropySet; + pos.myCommitment = commit; + pos.myReveal = reveal; + + Serializer s; + pos.add(s); + + // 32 + 1 + 32 + 32 + 32 + 32 = 161 + BEAST_EXPECT(s.getDataLength() == 161); + + SerialIter sit(s.slice()); + auto deserialized = + ExtendedPosition::fromSerialIter(sit, s.getDataLength()); + + BEAST_EXPECT(deserialized.txSetHash == txSet); + BEAST_EXPECT(deserialized.commitSetHash == commitSet); + BEAST_EXPECT(deserialized.entropySetHash == entropySet); + BEAST_EXPECT(deserialized.myCommitment == commit); + BEAST_EXPECT(deserialized.myReveal == reveal); + } + } + + void + testSigningConsistency() + { + testcase("Signing hash consistency"); + + // The signing hash from ConsensusProposal::signingHash() must match + // what a receiver would compute via the same function after + // deserializing the ExtendedPosition from the wire. + + auto const [pk, sk] = randomKeyPair(KeyType::secp256k1); + auto const nodeId = calcNodeID(pk); + auto const prevLedger = makeHash("prevledger"); + auto const closeTime = + NetClock::time_point{NetClock::duration{1234567}}; + + // Test with commitment (the case that was failing) + { + auto const txSet = makeHash("txset-sign"); + auto const commit = makeHash("commitment-sign"); + + ExtendedPosition pos{txSet}; + pos.myCommitment = commit; + + using Proposal = + ConsensusProposal; + + Proposal prop{ + prevLedger, + Proposal::seqJoin, + pos, + closeTime, + NetClock::time_point{}, + nodeId}; + + // Sign it (same as propose() does) + auto const signingHash = prop.signingHash(); + auto sig = signDigest(pk, sk, signingHash); + + // Serialize position to wire format + Serializer positionData; + pos.add(positionData); + auto const posSlice = positionData.slice(); + + // Deserialize (same as PeerImp::onMessage does) + SerialIter sit(posSlice); + auto const receivedPos = + ExtendedPosition::fromSerialIter(sit, posSlice.size()); + + // Reconstruct proposal on receiver side + Proposal receivedProp{ + prevLedger, + Proposal::seqJoin, + receivedPos, + closeTime, + NetClock::time_point{}, + nodeId}; + + // The signing hash must match + BEAST_EXPECT(receivedProp.signingHash() == signingHash); + + // Verify signature (same as checkSign does) + BEAST_EXPECT( + verifyDigest(pk, receivedProp.signingHash(), sig, false)); + } + + // Test without commitment (legacy case) + { + auto const txSet = makeHash("txset-legacy"); + ExtendedPosition pos{txSet}; + + using Proposal = + ConsensusProposal; + + Proposal prop{ + prevLedger, + Proposal::seqJoin, + pos, + closeTime, + NetClock::time_point{}, + nodeId}; + + auto const signingHash = prop.signingHash(); + auto sig = signDigest(pk, sk, signingHash); + + Serializer positionData; + pos.add(positionData); + + SerialIter sit(positionData.slice()); + auto const receivedPos = ExtendedPosition::fromSerialIter( + sit, positionData.getDataLength()); + + Proposal receivedProp{ + prevLedger, + Proposal::seqJoin, + receivedPos, + closeTime, + NetClock::time_point{}, + nodeId}; + + BEAST_EXPECT(receivedProp.signingHash() == signingHash); + BEAST_EXPECT( + verifyDigest(pk, receivedProp.signingHash(), sig, false)); + } + } + + void + testSuppressionConsistency() + { + testcase("Suppression hash consistency"); + + // proposalUniqueId must produce the same result on sender and + // receiver when given the same ExtendedPosition data. + + auto const [pk, sk] = randomKeyPair(KeyType::secp256k1); + auto const prevLedger = makeHash("prevledger-supp"); + auto const closeTime = + NetClock::time_point{NetClock::duration{1234567}}; + std::uint32_t const proposeSeq = 0; + + auto const txSet = makeHash("txset-supp"); + auto const commit = makeHash("commitment-supp"); + + ExtendedPosition pos{txSet}; + pos.myCommitment = commit; + + // Sign (to get a real signature for suppression) + using Proposal = ConsensusProposal; + Proposal prop{ + prevLedger, + proposeSeq, + pos, + closeTime, + NetClock::time_point{}, + calcNodeID(pk)}; + + auto sig = signDigest(pk, sk, prop.signingHash()); + + // Sender computes suppression + auto const senderSuppression = + proposalUniqueId(pos, prevLedger, proposeSeq, closeTime, pk, sig); + + // Simulate wire: serialize and deserialize + Serializer positionData; + pos.add(positionData); + SerialIter sit(positionData.slice()); + auto const receivedPos = + ExtendedPosition::fromSerialIter(sit, positionData.getDataLength()); + + // Receiver computes suppression + auto const receiverSuppression = proposalUniqueId( + receivedPos, prevLedger, proposeSeq, closeTime, pk, sig); + + BEAST_EXPECT(senderSuppression == receiverSuppression); + } + + void + testEquality() + { + testcase("Equality is txSetHash only"); + + auto const txSet = makeHash("txset-eq"); + auto const txSet2 = makeHash("txset-eq-2"); + + ExtendedPosition a{txSet}; + a.myCommitment = makeHash("commit1-eq"); + + ExtendedPosition b{txSet}; + b.myCommitment = makeHash("commit2-eq"); + + // Same txSetHash, different leaves -> equal + BEAST_EXPECT(a == b); + + // Same txSetHash, different commitSetHash -> still equal + // (sub-state quorum handles commitSetHash agreement) + b.commitSetHash = makeHash("cs-eq"); + BEAST_EXPECT(a == b); + + // Same txSetHash, different entropySetHash -> still equal + b.entropySetHash = makeHash("es-eq"); + BEAST_EXPECT(a == b); + + // Different txSetHash -> not equal + ExtendedPosition c{txSet2}; + BEAST_EXPECT(a != c); + } + +public: + void + run() override + { + testSerializationRoundTrip(); + testSigningConsistency(); + testSuppressionConsistency(); + testEquality(); + } +}; + +BEAST_DEFINE_TESTSUITE(ExtendedPosition, consensus, ripple); + +} // namespace test +} // namespace ripple