Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Makefile.test.include
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ BITCOIN_TESTS =\
test/governance_inv_tests.cpp \
test/governance_superblock_tests.cpp \
test/governance_validators_tests.cpp \
test/governance_vote_processing_tests.cpp \
test/governance_vote_wire_tests.cpp \
test/coinjoin_inouts_tests.cpp \
test/coinjoin_dstxmanager_tests.cpp \
Expand Down
365 changes: 365 additions & 0 deletions src/test/governance_vote_processing_tests.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,365 @@
// Copyright (c) 2026 The Dash Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <consensus/amount.h>
#include <evo/chainhelper.h>
#include <evo/deterministicmns.h>
#include <governance/governance.h>
#include <governance/object.h>
#include <governance/vote.h>
#include <index/txindex.h>
#include <interfaces/chain.h>
#include <key.h>
#include <key_io.h>
#include <masternode/meta.h>
#include <masternode/sync.h>
#include <messagesigner.h>
#include <netfulfilledman.h>
#include <primitives/transaction.h>
#include <script/standard.h>
#include <timedata.h>
#include <uint256.h>
#include <util/strencodings.h>
#include <util/time.h>
#include <validation.h>
#include <validationinterface.h>

#include <test/util/index.h>
#include <test/util/masternode.h>
#include <test/util/setup_common.h>

#include <boost/test/unit_test.hpp>

#include <chrono>
#include <memory>
#include <string>
#include <vector>

using namespace std::chrono_literals;

namespace {
// TestChainSetup only accepts checkpointed chain lengths, so 107 blocks plus
// -dip3params=109:500 is the shortest chain a ProRegTx can be mined on. Same setup as
// TestChainDIP3Setup in evo_deterministicmns_tests.cpp.
constexpr int DIP3_ACTIVATION_HEIGHT{109};

void SignWithVotingKey(CGovernanceVote& vote, const CKey& key)
{
std::vector<unsigned char> sig;
BOOST_REQUIRE(CMessageSigner::SignMessage(vote.GetSignatureString(), sig, key));
vote.SetSignature(sig);
}

void SignWithOperatorKey(CGovernanceVote& vote, const CBLSSecretKey& key)
{
// Vote signatures are always basic-scheme, regardless of the currently active BLS scheme:
// CGovernanceVote::CheckSignature() deserializes and verifies with legacy=false.
vote.SetSignature(
key.Sign(vote.GetSignatureHash(), /*specificLegacyScheme=*/false).ToByteVector(/*specificLegacyScheme=*/false));
}

// A chain with one registered masternode whose voting (ECDSA) and operator (BLS) keys are known
// to the test, so votes can be signed for real and CGovernanceVote::CheckSignature is actually
// reached. Without a populated masternode list every CGovernanceVote::IsValid() call would
// short-circuit at GetMNByCollateral and no signature would ever be verified.
struct GovernanceVoteSetup : public TestChainSetup {
CKey mn_voting_key;
CBLSSecretKey mn_operator_key;
COutPoint mn_collateral;
SimpleUTXOMap utxos;
//! Fixed so a proposal can be rebuilt bit-for-bit (and keep its hash) once its fee tx is known.
int64_t proposal_time{0};
std::string proposal_payment_address;

GovernanceVoteSetup() :
TestChainSetup(DIP3_ACTIVATION_HEIGHT - 2, CBaseChainParams::REGTEST, {"-dip3params=109:500"})
{
// A failed BOOST_REQUIRE below throws, and a throwing constructor means no destructor runs.
// Tear the globals down by hand so one broken invariant here doesn't leave a live tx index
// (and a dangling m_node reference in it) for every later test in the binary.
try {
SetUp();
} catch (...) {
TearDown();
throw;
}
}

~GovernanceVoteSetup() { TearDown(); }

void SetUp()
{
// CGovernanceObject::IsCollateralValid() reads the proposal fee tx out of the tx index;
// without it no proposal can ever be accepted, orphan votes included.
g_txindex = std::make_unique<TxIndex>(interfaces::MakeChain(m_node), /*n_cache_size=*/1 << 20,
/*f_memory=*/true);
BOOST_REQUIRE(g_txindex->Start());
IndexWaitSynced(*g_txindex);

utxos = BuildSimpleUtxoMap(m_coinbase_txns);

// Activate DIP3, then register the masternode that votes in these tests.
MineBlocks(1);
auto protx = CreateProRegTx(*m_node.chainman, utxos, /*port=*/1, payout_script(), coinbaseKey, mn_voting_key,
mn_operator_key);
mn_collateral = COutPoint(protx.GetHash(), 0);
MineBlock({protx});
BOOST_REQUIRE(tip_mn_list().GetMNByCollateral(mn_collateral) != nullptr);

BOOST_REQUIRE(m_node.mn_metaman->LoadCache(/*load_cache=*/false));
BOOST_REQUIRE(m_node.netfulfilledman->LoadCache(/*load_cache=*/false));
// Governance relay and CheckAndRemove() are no-ops until the node considers itself synced.
// Two SwitchToNextAsset() calls get there from BLOCKCHAIN; the bound is just a loop guard.
for (int i = 0; i < 5 && !m_node.mn_sync->IsSynced(); ++i) {
m_node.mn_sync->SwitchToNextAsset();
}
BOOST_REQUIRE(m_node.mn_sync->IsSynced());

m_node.govman = std::make_unique<CGovernanceManager>(*m_node.mn_metaman, *m_node.chainman,
*m_node.chain_helper->superblocks, *m_node.dmnman,
*m_node.mn_sync);
BOOST_REQUIRE(m_node.govman->LoadCache(/*load_cache=*/false));

proposal_time = TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime());
CKey payment_key;
payment_key.MakeNewKey(true);
proposal_payment_address = EncodeDestination(PKHash(payment_key.GetPubKey()));
}

void TearDown()
{
// govman holds a reference into chain_helper, so it must go first (matches PrepareShutdown
// ordering in init.cpp).
m_node.govman.reset();
if (g_txindex) {
SyncWithValidationInterfaceQueue();
g_txindex->Stop();
g_txindex.reset();
}
}

CScript coinbase_script() const { return GetScriptForRawPubKey(coinbaseKey.GetPubKey()); }

//! Masternode payouts and the proposal fee change must go to a P2PKH script (bad-protx-payee
//! rejects the raw-pubkey coinbase script, IsCollateralValid() rejects anything else).
CScript payout_script() const { return GetScriptForDestination(PKHash(coinbaseKey.GetPubKey())); }

CDeterministicMNList tip_mn_list() const { return m_node.dmnman->GetListAtChainTip(); }

void MineBlock(const std::vector<CMutableTransaction>& txns)
{
const CBlock block = CreateAndProcessBlock(txns, coinbase_script());
const CBlockIndex* tip{WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Tip())};
BOOST_REQUIRE_EQUAL(tip->GetBlockHash(), block.GetHash());
m_node.dmnman->UpdatedBlockTip(tip);
SetMockTime(GetTime() + 1);
IndexWaitSynced(*g_txindex);
}

void MineBlocks(int num_blocks)
{
for (int i = 0; i < num_blocks; ++i) {
MineBlock({});
}
}

//! The proposal both the orphan vote and the fee tx below commit to. The object hash
//! deliberately excludes the collateral hash, so the same proposal can be built first with an
//! empty collateral (to learn its hash) and again once the fee tx paying to that hash exists.
CGovernanceObject MakeProposal(const uint256& collateral_hash) const
{
const std::string data{
strprintf("{\"type\":1,\"name\":\"test-proposal\",\"start_epoch\":%d,\"end_epoch\":%d,\"payment_amount\":1.0,\"payment_address\":\"%s\",\"url\":\"https://dash.org\"}",
proposal_time, proposal_time + 100000, proposal_payment_address)};
return CGovernanceObject{uint256{}, /*revision=*/1, proposal_time, collateral_hash, HexStr(data)};
}

//! Burns the proposal fee to an OP_RETURN committing to the object hash, the way
//! `gobject prepare` does, and buries it under the confirmations the collateral check needs.
uint256 ConfirmProposalCollateral(const uint256& govobj_hash)
{
CMutableTransaction tx;
// IsCollateralValid() rejects the fee tx unless every output is P2PKH or unspendable, so
// the change cannot go back to the burn script.
const auto spent = FundTransaction(*m_node.chainman, tx, utxos,
CScript() << OP_RETURN << ToByteVector(govobj_hash),
GOVERNANCE_PROPOSAL_FEE_TX, /*script_change=*/payout_script());
SignTransaction(tx, spent, coinbaseKey);

MineBlock({tx});
MineBlocks(GOVERNANCE_FEE_CONFIRMATIONS - 1);
return tx.GetHash();
}

//! A vote's hash covers everything but its signature, so votes that should be distinguishable
//! (e.g. a re-signed vote) need distinct timestamps.
CGovernanceVote MakeVote(const uint256& parent_hash, vote_signal_enum_t signal, vote_outcome_enum_t outcome,
std::chrono::seconds time_offset = 0s) const
{
CGovernanceVote vote{mn_collateral, parent_hash, signal, outcome};
vote.SetTime(GetAdjustedTime() + time_offset);
return vote;
}
};
} // namespace

BOOST_FIXTURE_TEST_SUITE(governance_vote_processing_tests, GovernanceVoteSetup)

// A vote that arrives before its parent object must be kept and replayed once the object shows up,
// otherwise every vote that races ahead of its proposal is lost for good.
BOOST_AUTO_TEST_CASE(orphan_vote_is_cached_and_applied_when_parent_arrives)
{
auto& govman = *m_node.govman;

// Build the proposal but keep it out of the manager: the vote arrives first.
const uint256 parent_hash{MakeProposal(uint256{}).GetHash()};
BOOST_REQUIRE(!govman.HaveObjectForHash(parent_hash));

CGovernanceVote vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES)};
SignWithVotingKey(vote, mn_voting_key);
// Guard: if the masternode or its signature were not recognized the rest of this test would
// pass for the wrong reason (an unverifiable vote is cached as an orphan just the same).
BOOST_REQUIRE(vote.IsValid(tip_mn_list(), /*useVotingKey=*/true));

CGovernanceException exception;
uint256 hash_to_request;
BOOST_CHECK(!govman.ProcessVote(vote, exception, hash_to_request));
BOOST_CHECK_EQUAL(exception.GetType(), GOVERNANCE_EXCEPTION_WARNING);
BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0);
// The caller uses this to ask the sending peer for the missing object.
BOOST_CHECK_EQUAL(hash_to_request, parent_hash);
BOOST_CHECK(govman.GetOrphanVoteObjectHashes() == std::vector<uint256>{parent_hash});

const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)};
CGovernanceObject proposal{MakeProposal(collateral_hash)};
BOOST_REQUIRE_EQUAL(proposal.GetHash(), parent_hash);
govman.AddGovernanceObject(proposal, /*peer_str=*/"");
BOOST_REQUIRE(govman.HaveObjectForHash(parent_hash));

// The orphan vote is now applied to the object it was waiting for and is no longer pending.
auto stored = govman.FindConstGovernanceObject(parent_hash);
BOOST_REQUIRE(stored != nullptr);
BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1);
BOOST_CHECK_EQUAL(govman.GetCurrentVotes(parent_hash, mn_collateral).size(), 1U);
BOOST_CHECK(govman.GetOrphanVoteObjectHashes().empty());
Comment on lines +240 to +245

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Orphan-replay assertions don't confirm the vote is indexed by the manager

CGovernanceManager::CheckOrphanVotes() (governance.cpp) replays a cached orphan vote by calling govobj.ProcessVote() directly — unlike CGovernanceManager::ProcessVote(), it never inserts the vote into cmapVoteToObject. That means HaveVoteForHash() returns false for an orphan vote that was just successfully applied through this path, so NetGovernance::ProcessGetData() (which requires HaveVoteForHash() before calling SerializeVoteForHash()) can't serve the inv that RelayVote() announced during the same replay to a peer that asks for it. This test only checks object-local counters (stored->GetAbsoluteYesCount(...), govman.GetCurrentVotes(...).size()) after the replay, never govman.HaveVoteForHash(vote.GetHash()), so it can't distinguish 'fully recovered and indexed' from 'applied to the object but invisible to the manager.' Add a HaveVoteForHash() assertion after the replay so the test's own claim of full recovery is actually verified — the underlying manager-indexing gap in CheckOrphanVotes() itself is pre-existing production behavior, not something this test PR needs to fix.

source: ['codex']


// A peer re-sending the same vote must not be punished for it.
CGovernanceException replay_exception;
uint256 replay_hash_to_request;
BOOST_CHECK(!govman.ProcessVote(vote, replay_exception, replay_hash_to_request));
BOOST_CHECK_EQUAL(replay_exception.GetNodePenalty(), 0);
BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1);
}

// Votes are only counted if they carry a signature from a registered masternode.
BOOST_AUTO_TEST_CASE(unsigned_and_unknown_masternode_votes_are_rejected)
{
auto& govman = *m_node.govman;

const CGovernanceObject proposal{MakeProposal(uint256::ONE)};
const uint256 parent_hash{proposal.GetHash()};
govman.AddGovernanceObjectForTesting(proposal);
BOOST_REQUIRE(govman.HaveObjectForHash(parent_hash));

// Forged signature: right masternode, wrong key.
CKey attacker_key;
attacker_key.MakeNewKey(true);
CGovernanceVote forged{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES)};
SignWithVotingKey(forged, attacker_key);

CGovernanceException exception;
uint256 hash_to_request;
BOOST_CHECK(!govman.ProcessVote(forged, exception, hash_to_request));
BOOST_CHECK_EQUAL(exception.GetType(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR);
BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 20);
BOOST_CHECK(!govman.HaveVoteForHash(forged.GetHash()));

// Re-sending a vote already known to be invalid is still punished, without re-verifying it.
CGovernanceException repeat_exception;
BOOST_CHECK(!govman.ProcessVote(forged, repeat_exception, hash_to_request));
BOOST_CHECK_EQUAL(repeat_exception.GetType(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR);
BOOST_CHECK_EQUAL(repeat_exception.GetNodePenalty(), 20);

// Correctly signed, but the outpoint belongs to no masternode.
CKey stranger_key;
stranger_key.MakeNewKey(true);
CGovernanceVote stranger{COutPoint{uint256S("42"), 0}, parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES};
stranger.SetTime(GetAdjustedTime());
SignWithVotingKey(stranger, stranger_key);

CGovernanceException stranger_exception;
BOOST_CHECK(!govman.ProcessVote(stranger, stranger_exception, hash_to_request));
BOOST_CHECK_EQUAL(stranger_exception.GetType(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR);
BOOST_CHECK_EQUAL(stranger_exception.GetNodePenalty(), 20);

// A vote dated too far in the future is rejected even with a valid signature.
CGovernanceVote from_the_future{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 2h)};
SignWithVotingKey(from_the_future, mn_voting_key);

CGovernanceException future_exception;
BOOST_CHECK(!govman.ProcessVote(from_the_future, future_exception, hash_to_request));
BOOST_CHECK_EQUAL(future_exception.GetType(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR);
BOOST_CHECK_EQUAL(future_exception.GetNodePenalty(), 20);

auto stored = govman.FindConstGovernanceObject(parent_hash);
BOOST_REQUIRE(stored != nullptr);
BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 0);
}

// Funding a proposal is the one decision reserved for the voting key; every other signal may be
// signed by the operator (BLS) key. Mixing the two up must not be accepted either way around.
//
// Rate checks are live on this path (unlike the orphan replay above, which disables them), so each
// vote below has to be the first *accepted* one for its (masternode, signal) pair: a rejected vote
// leaves last_update at epoch 0, but a second accepted vote for the same signal would be turned
// away by the 1h GOVERNANCE_UPDATE_MIN limit instead of the rule under test.
BOOST_AUTO_TEST_CASE(proposal_funding_votes_require_the_voting_key)
{
auto& govman = *m_node.govman;

const CGovernanceObject proposal{MakeProposal(uint256::ONE)};
const uint256 parent_hash{proposal.GetHash()};
govman.AddGovernanceObjectForTesting(proposal);
auto stored = govman.FindConstGovernanceObject(parent_hash);
BOOST_REQUIRE(stored != nullptr);

// Operator key on a funding vote: valid BLS signature, wrong key for this signal.
CGovernanceVote funding_by_operator{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES)};
SignWithOperatorKey(funding_by_operator, mn_operator_key);
BOOST_REQUIRE(funding_by_operator.IsValid(tip_mn_list(), /*useVotingKey=*/false));

CGovernanceException exception;
uint256 hash_to_request;
BOOST_CHECK(!govman.ProcessVote(funding_by_operator, exception, hash_to_request));
BOOST_CHECK_EQUAL(exception.GetType(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR);
BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 20);
BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 0);

// Voting key on a funding vote: accepted and counted. Dated a second later so it is a vote in
// its own right rather than the rejected one above with a different signature glued on.
CGovernanceVote funding_by_voting_key{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 1s)};
SignWithVotingKey(funding_by_voting_key, mn_voting_key);

CGovernanceException funding_exception;
BOOST_CHECK(govman.ProcessVote(funding_by_voting_key, funding_exception, hash_to_request));
BOOST_CHECK(govman.HaveVoteForHash(funding_by_voting_key.GetHash()));
BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1);

// Operator key on a non-funding signal: accepted and counted.
CGovernanceVote valid_by_operator{MakeVote(parent_hash, VOTE_SIGNAL_VALID, VOTE_OUTCOME_YES)};
SignWithOperatorKey(valid_by_operator, mn_operator_key);

CGovernanceException valid_exception;
BOOST_CHECK(govman.ProcessVote(valid_by_operator, valid_exception, hash_to_request));
BOOST_CHECK(govman.HaveVoteForHash(valid_by_operator.GetHash()));
BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_VALID), 1);
Comment on lines +349 to +356

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: No negative test for a forged BLS (operator-key) signature

CGovernanceVote::CheckSignature(const CBLSPublicKey&) is only ever exercised with the real, correct operator key (SignWithOperatorKey(valid_by_operator, mn_operator_key) at line 351). There's no case that signs a vote with an unrelated, freshly generated BLS secret key and asserts rejection — so a regression that makes BLS verification always succeed would slip through this suite while the equivalent ECDSA case is explicitly covered (the attacker_key path in unsigned_and_unknown_masternode_votes_are_rejected, lines 265-276). Add a case that signs a VOTE_SIGNAL_VALID vote with a fresh, unrelated CBLSSecretKey and asserts GOVERNANCE_EXCEPTION_PERMANENT_ERROR with penalty 20 and a zero vote count, mirroring the existing forged-ECDSA case, before submitting the real operator-signed vote.

source: ['codex']


// A known good vote received twice is dropped quietly, not punished.
CGovernanceException duplicate_exception;
BOOST_CHECK(!govman.ProcessVote(funding_by_voting_key, duplicate_exception, hash_to_request));
BOOST_CHECK_EQUAL(duplicate_exception.GetNodePenalty(), 0);
BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1);
}

BOOST_AUTO_TEST_SUITE_END()
8 changes: 7 additions & 1 deletion src/test/util/masternode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ SimpleUTXOMap BuildSimpleUtxoMap(const std::vector<CTransactionRef>& txs)

SimpleUTXOMap FundTransaction(const ChainstateManager& chainman, CMutableTransaction& tx, SimpleUTXOMap& utxos,
const CScript& script_payout, CAmount amount)
{
return FundTransaction(chainman, tx, utxos, script_payout, amount, /*script_change=*/script_payout);
}

SimpleUTXOMap FundTransaction(const ChainstateManager& chainman, CMutableTransaction& tx, SimpleUTXOMap& utxos,
const CScript& script_payout, CAmount amount, const CScript& script_change)
{
CAmount change;
auto inputs = WITH_LOCK(::cs_main, return SelectUTXOs(chainman.ActiveChain(), utxos, amount, change));
Expand All @@ -74,7 +80,7 @@ SimpleUTXOMap FundTransaction(const ChainstateManager& chainman, CMutableTransac
}
tx.vout.emplace_back(amount, script_payout);
if (change != 0) {
tx.vout.emplace_back(change, script_payout);
tx.vout.emplace_back(change, script_change);
}
return inputs;
}
Expand Down
Loading
Loading