diff --git a/depends/packages/miniupnpc.mk b/depends/packages/miniupnpc.mk index e63752406107..6a0cfa8cc166 100644 --- a/depends/packages/miniupnpc.mk +++ b/depends/packages/miniupnpc.mk @@ -1,6 +1,6 @@ package=miniupnpc $(package)_version=2.2.7 -$(package)_download_path=https://miniupnp.tuxfamily.org/files/ +$(package)_download_path=http://miniupnp.free.fr/files/ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=b0c3a27056840fd0ec9328a5a9bac3dc5e0ec6d2e8733349cf577b0aa1e70ac1 $(package)_patches=dont_leak_info.patch cmake_get_src_addr.patch fix_windows_snprintf.patch diff --git a/src/core_write.cpp b/src/core_write.cpp index a3ec9b5643cb..e7627095d5dc 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -289,11 +289,11 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry } if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - if (const auto opt_proTx = GetTxPayload(tx)) { + if (const auto opt_proTx = GetTxPayload(tx); opt_proTx && opt_proTx->netInfo) { entry.pushKV("proRegTx", opt_proTx->ToJson()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - if (const auto opt_proTx = GetTxPayload(tx)) { + if (const auto opt_proTx = GetTxPayload(tx); opt_proTx && opt_proTx->netInfo) { entry.pushKV("proUpServTx", opt_proTx->ToJson()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 8cbceb5d6180..7b7a29ba4c4c 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -1093,6 +1093,11 @@ bool CSpecialTxProcessor::UndoSpecialTxsInBlock(const Chainstate& chainstate, co return true; } +void CSpecialTxProcessor::RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null pindex) +{ + m_qblockman.RemoveMineableCommitmentsForBlock(block, pindex); +} + bool CSpecialTxProcessor::CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const CCbTx& cbTx, CAmount blockSubsidy, BlockValidationState& state) { diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 95a64bc23157..a6b5f0e9863c 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -97,6 +97,8 @@ class CSpecialTxProcessor EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool UndoSpecialTxsInBlock(const Chainstate& chainstate, const CBlock& block, const CBlockIndex* pindex, MNListUpdates& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null pindex) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); // the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 138cd5ac8f7f..3ec521886f34 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -102,6 +103,7 @@ bool CGovernanceManager::LoadCache(bool load_cache) assert(m_db != nullptr); is_loaded = load_cache ? m_db->Load(*this) : m_db->Store(*this); if (is_loaded && load_cache) { + nCachedBlockHeight = WITH_LOCK(::cs_main, return m_chainman.ActiveChain().Height()); CheckAndRemove(); InitOnLoad(); } @@ -1190,10 +1192,14 @@ void CGovernanceManager::RemoveInvalidVotes() std::vector changedKeyMNs; for (const auto& [internalId, pdmnState] : diff.updatedMNs) { - auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(internalId); - // BuildDiff will construct itself with MNs that we already have knowledge - // of, meaning that fetch operations should never fail. - assert(oldDmn); + auto newDmn = tip_mn_list.GetMNByInternalId(internalId); + if (!Assume(newDmn)) { + continue; + } + auto oldDmn = lastMNListForVotingKeys->GetMN(newDmn->proTxHash); + if (!Assume(oldDmn)) { + continue; + } if ((pdmnState.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && pdmnState.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) { changedKeyMNs.emplace_back(oldDmn->collateralOutpoint); } else if ((pdmnState.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && pdmnState.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) { @@ -1202,9 +1208,9 @@ void CGovernanceManager::RemoveInvalidVotes() } for (const auto& id : diff.removedMns) { auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(id); - // BuildDiff will construct itself with MNs that we already have knowledge - // of, meaning that fetch operations should never fail. - assert(oldDmn); + if (!Assume(oldDmn)) { + continue; + } changedKeyMNs.emplace_back(oldDmn->collateralOutpoint); } diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index ed8c3d0eeb98..69c6aa399c71 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -21,6 +21,7 @@ namespace { constexpr std::chrono::seconds GOVERNANCE_FUDGE_WINDOW{2h}; +constexpr int NO_VOTE_DEADLINE_BLOCKS{3}; } // anonymous namespace GovernanceSigner::GovernanceSigner(CDeterministicMNManager& dmnman, CGovernanceManager& govman, @@ -178,7 +179,8 @@ std::optional GovernanceSigner::CreateGovernanceTrigger return std::make_optional(gov_sb); } -void GovernanceSigner::VoteGovernanceTriggers(const std::optional& trigger_opt, int nHeight) +void GovernanceSigner::VoteGovernanceTriggers(const std::optional& trigger_opt, + std::optional superblock_height, int nHeight) { // only active masternodes can vote on triggers if (m_mn_activeman.GetProTxHash().IsNull()) return; @@ -189,7 +191,7 @@ void GovernanceSigner::VoteGovernanceTriggers(const std::optionalhash) { // Skip actual trigger LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for trigger:%s, we voted yes for it already\n", __func__, trigger_hash.ToString()); @@ -296,19 +305,23 @@ bool GovernanceSigner::VoteFundingTrigger(const uint256& nHash, const vote_outco bool GovernanceSigner::HasAlreadyVotedFundingTrigger() const { - return votedFundingYesTriggerHash.has_value(); + return votedFundingYesTrigger.has_value(); } void GovernanceSigner::ResetVotedFundingTrigger() { - votedFundingYesTriggerHash = std::nullopt; + votedFundingYesTrigger = std::nullopt; } void GovernanceSigner::UpdatedBlockTip(const CBlockIndex* pindex) { + if (votedFundingYesTrigger.has_value() && pindex->nHeight >= votedFundingYesTrigger->superblock_height) { + ResetVotedFundingTrigger(); + } const auto sb_opt = CreateSuperblockCandidate(pindex->nHeight); const auto trigger_opt = CreateGovernanceTrigger(sb_opt); - VoteGovernanceTriggers(trigger_opt, pindex->nHeight); + VoteGovernanceTriggers(trigger_opt, sb_opt ? std::make_optional(sb_opt->GetBlockHeight()) : std::nullopt, + pindex->nHeight); CSuperblock_sptr pSuperblock; if (m_superblocks.GetBestSuperblock(m_dmnman.GetListAtChainTip(), pSuperblock, pindex->nHeight)) { ResetVotedFundingTrigger(); diff --git a/src/governance/signing.h b/src/governance/signing.h index c58ef4357303..e6bfb8c1d5aa 100644 --- a/src/governance/signing.h +++ b/src/governance/signing.h @@ -36,7 +36,11 @@ class GovernanceSigner const CMasternodeSync& m_mn_sync; private: - std::optional votedFundingYesTriggerHash{std::nullopt}; + struct FundingTriggerVote { + uint256 hash; + int superblock_height{0}; + }; + std::optional votedFundingYesTrigger{std::nullopt}; public: GovernanceSigner() = delete; @@ -54,7 +58,8 @@ class GovernanceSigner std::optional CreateGovernanceTrigger(const std::optional& sb_opt); std::optional CreateSuperblockCandidate(int nHeight) const; void ResetVotedFundingTrigger(); - void VoteGovernanceTriggers(const std::optional& trigger_opt, int nHeight); + void VoteGovernanceTriggers(const std::optional& trigger_opt, + std::optional superblock_height, int nHeight); }; #endif // BITCOIN_GOVERNANCE_SIGNING_H diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 82b0a87a81a1..24385a7bd728 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -254,8 +255,7 @@ bool CQuorumBlockProcessor::ProcessBlock(Chainstate& chainstate, const CBlock& b // ProcessCommitment accepts a commitment identical to the stored one, so // rotation's multiple commitments per type need an explicit per-block check. - // It runs before any commitment is processed, because ProcessCommitment - // updates in-memory state that a rejected block would not restore. + // It runs before the signature checks, so a duplicate is rejected cheaply. // Null commitments stay exempt: they were never subject to bad-qc-dup. std::set> seen_quorums; for (const auto& [_, qc] : qcs) { @@ -445,12 +445,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(Chainstate& chainstate, int nHeigh // invalidation should not depend on that. DropQcHashesCache(); - { - LOCK(minableCommitmentsCs); - mapMinedCommitmentBlockCache.erase(qc.llmqType, qc.quorumHash); - minableCommitmentsByQuorum.erase(cacheKey); - minableCommitments.erase(::SerializeHash(qc)); - } + WITH_LOCK(minableCommitmentsCs, mapMinedCommitmentBlockCache.erase(qc.llmqType, qc.quorumHash)); LogPrint(BCLog::LLMQ, "%s -- processed commitment from block. type=%d, quorumIndex=%d, quorumHash=%s\n", __func__, std23::to_underlying(qc.llmqType), qc.quorumIndex, quorumHash.ToString()); @@ -561,6 +556,23 @@ bool CQuorumBlockProcessor::UndoBlock(const Chainstate& chainstate, const CBlock return true; } +void CQuorumBlockProcessor::RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null pindex) +{ + AssertLockHeld(::cs_main); + + std::multimap qcs; + if (BlockValidationState dummy; !Assume(GetCommitmentsFromBlock(block, pindex, qcs, dummy))) { + return; + } + + LOCK(minableCommitmentsCs); + for (const auto& [_, qc] : qcs) { + if (qc.IsNull()) continue; + minableCommitmentsByQuorum.erase(std::make_pair(qc.llmqType, qc.quorumHash)); + minableCommitments.erase(::SerializeHash(qc)); + } +} + bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, gsl::not_null pindex, std::multimap& ret, BlockValidationState& state) { AssertLockHeld(::cs_main); diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index c248a13fd29f..396ab1245310 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -108,6 +108,9 @@ class CQuorumBlockProcessor bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs, !m_qc_hashes_cache_mutex); bool UndoBlock(const Chainstate& chainstate, const CBlock& block, gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs, !m_qc_hashes_cache_mutex); + //! Drop the commitments mined by a block that has been fully connected from the mineable pool. + void RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null pindex) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs); //! it returns hash of commitment if it should be relay, otherwise nullopt std::optional AddMineableCommitment(const CFinalCommitment& fqc) EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index 267a5bc372a4..04ad6bf8816d 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -52,7 +52,12 @@ void CDKGPendingMessages::PushPendingMessage(NodeId from, const uint256& sender_ count++; seenMessages.emplace(hash); - pendingMessages.emplace_back(from, std::move(pm)); + if (from == -1) { + // Our own message must not wait behind a backlog of peer messages before it is relayed + pendingMessages.emplace_front(from, std::move(pm)); + } else { + pendingMessages.emplace_back(from, std::move(pm)); + } } std::list CDKGPendingMessages::PopPendingMessages(size_t maxCount) diff --git a/src/llmq/dkgsessionhandler.h b/src/llmq/dkgsessionhandler.h index f52bbc81c416..eb3d560343fe 100644 --- a/src/llmq/dkgsessionhandler.h +++ b/src/llmq/dkgsessionhandler.h @@ -71,7 +71,8 @@ class CDKGPendingMessages * Enqueue a serialized DKG message under @p from with content hash @p hash. * @p sender_protx keys the per-proTx quota: the sender's MNAuth-verified * proTxHash for remote messages, or this node's own for messages it - * produced itself (@p from == -1). Drops the message silently on quota + * produced itself (@p from == -1), which go to the front of the queue. + * Drops the message silently on quota * overflow or duplicate hash; quota-dropped messages are not marked seen, * so another peer with budget can re-deliver them. Caller is responsible * for hashing the payload and (for real peers) routing the erase-request diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index 5b68ffb0059d..02a2a81b8456 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -130,11 +130,22 @@ void CDKGSessionManager::WriteVerifiedVvecContribution(Consensus::LLMQType llmqT s << CBLSPublicKeyVersionWrapper(pubkey, false); } db->Write(std::make_tuple(DB_VVEC, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), Span{s}); + + // Reading the vvec back at commit time costs a point decompression and subgroup check per coefficient + LOCK(contributionsCacheCs); + auto& entry = contributionsCache[{llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash}]; + entry.entryTime = SteadyClock::now(); + entry.vvec = vvec; } void CDKGSessionManager::WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution) { db->Write(std::make_tuple(DB_SKCONTRIB, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), skContribution); + + LOCK(contributionsCacheCs); + if (auto it = contributionsCache.find({llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash}); it != contributionsCache.end()) { + it->second.skContribution = skContribution; + } } void CDKGSessionManager::WriteEncryptedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSIESMultiRecipientObjects& contributions) @@ -228,13 +239,10 @@ void CDKGSessionManager::CleanupCache() const { LOCK(contributionsCacheCs); const auto curTime = SteadyClock::now(); - for (auto it = contributionsCache.begin(); it != contributionsCache.end(); ) { - if (curTime - it->second.entryTime > MAX_CONTRIBUTION_CACHE_TIME) { - it = contributionsCache.erase(it); - } else { - ++it; - } - } + // A vvec still referenced elsewhere (by its DKG session) costs nothing to keep and is expensive to reload + std::erase_if(contributionsCache, [&](const auto& entry) { + return entry.second.vvec.use_count() == 1 && curTime - entry.second.entryTime > MAX_CONTRIBUTION_CACHE_TIME; + }); } void CDKGSessionManager::CleanupOldContributions() const diff --git a/src/llmq/dkgsessionmgr.h b/src/llmq/dkgsessionmgr.h index cf1cdfa52d1e..8e19cd1f1aad 100644 --- a/src/llmq/dkgsessionmgr.h +++ b/src/llmq/dkgsessionmgr.h @@ -140,8 +140,10 @@ class CDKGSessionManager bool GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const; // Contributions are written while in the DKG - void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec); - void WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution); + void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec) + EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs); + void WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution) + EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs); bool GetVerifiedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const std::vector& validMembers, std::vector& memberIndexesRet, std::vector& vvecsRet, diff --git a/src/net.cpp b/src/net.cpp index c248af0f4ff9..9e484785b456 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1771,6 +1771,7 @@ std::pair CConnman::SocketSendData(CNode& node) const LogPrint(BCLog::NET, "socket send error for peer=%d: %s\n", node.GetId(), NetworkErrorString(nErr)); node.fDisconnect = true; } + node.fCanSendData = false; } break; } diff --git a/src/qt/mnsharesession.cpp b/src/qt/mnsharesession.cpp index cf96952a193d..5f6bcad18b66 100644 --- a/src/qt/mnsharesession.cpp +++ b/src/qt/mnsharesession.cpp @@ -104,19 +104,6 @@ bool SameShare(const MnShareSession::Share& a, const MnShareSession::Share& b) a.rewardAddress == b.rewardAddress; } -bool SameContribution(const MnShareSession::Contribution& a, const MnShareSession::Contribution& b) -{ - if (a.inputs.size() != b.inputs.size() || a.hasChange != b.hasChange) return false; - if (a.hasChange && (a.changeAddress != b.changeAddress || a.changeAmount != b.changeAmount)) return false; - for (size_t i = 0; i < a.inputs.size(); ++i) { - if (a.inputs[i].txid.compare(b.inputs[i].txid, Qt::CaseInsensitive) != 0 || - a.inputs[i].vout != b.inputs[i].vout || a.inputs[i].sequence != b.inputs[i].sequence) { - return false; - } - } - return true; -} - bool IsTxidHex(const QString& txid) { return txid.size() == 64 && IsHex(txid.toStdString()); @@ -191,6 +178,19 @@ bool DecodeSharedProTx(const QString& protx_hex, CMutableTransaction& tx, CProRe } // anonymous namespace +bool MnShareSession::SameContribution(const Contribution& a, const Contribution& b) +{ + if (a.inputs.size() != b.inputs.size() || a.hasChange != b.hasChange) return false; + if (a.hasChange && (a.changeAddress != b.changeAddress || a.changeAmount != b.changeAmount)) return false; + for (size_t i = 0; i < a.inputs.size(); ++i) { + if (a.inputs[i].txid.compare(b.inputs[i].txid, Qt::CaseInsensitive) != 0 || + a.inputs[i].vout != b.inputs[i].vout || a.inputs[i].sequence != b.inputs[i].sequence) { + return false; + } + } + return true; +} + namespace { //! U+202F NARROW NO-BREAK SPACE. BitcoinUnits groups thousands with a plain //! thin space, which a layout may break a number across. diff --git a/src/qt/mnsharesession.h b/src/qt/mnsharesession.h index 0dc8f0176ce0..fae57afc8662 100644 --- a/src/qt/mnsharesession.h +++ b/src/qt/mnsharesession.h @@ -313,6 +313,9 @@ class MnShareSession //! Whether a row with `share`'s owner address has the same amount and addresses here bool hasShare(const Share& share) const; + //! Same inputs, in order and with their sequences, and the same change. + //! Labels and changeIndex are ignored. + static bool SameContribution(const Contribution& a, const Contribution& b); //! Whether a contribution with `contribution`'s label spends the same coins and pays the same change here bool hasContribution(const Contribution& contribution) const; diff --git a/src/qt/sharedmncreatedialog.cpp b/src/qt/sharedmncreatedialog.cpp index 5588e7236022..e6c85718582c 100644 --- a/src/qt/sharedmncreatedialog.cpp +++ b/src/qt/sharedmncreatedialog.cpp @@ -1986,8 +1986,18 @@ void SharedMnCreateDialog::refreshContributionPage() const CAmount target{contributionTarget()}; CoinSelection selection; QString error; + CAmount again_total{0}; if (target <= 0) { m_coins_label->setText(tr("Choose which participant you are.")); + } else if (const auto* again{reservableAgain(&again_total)}) { + const QString coins{CoinCount(again->inputs.size())}; + m_coins_label->setText( + again->hasChange + ? tr("Uses %1 (%2) as reserved before; change %3 returns to the same address of yours.") + .arg(coins, FormatAmount(m_wallet_model, again_total), + FormatAmount(m_wallet_model, again->changeAmount)) + : tr("Uses %1 (%2) as reserved before, with no change left over.") + .arg(coins, FormatAmount(m_wallet_model, again_total))); } else if (selectCoins(target, selection, error)) { const QString coins{CoinCount(selection.inputs.size())}; m_coins_label->setText( @@ -2358,6 +2368,7 @@ void SharedMnCreateDialog::startSession() unlockAllContributedCoins(); m_coins_released = false; m_released_contributions.clear(); + m_built_contributions.clear(); m_session = MnShareSession(); m_role = Role::Coordinator; m_my_share = 0; @@ -3372,23 +3383,19 @@ CAmount SharedMnCreateDialog::contributionTarget() const return m_role == Role::Coordinator ? share + m_fee_field->value() : share; } -bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, QString& error) const +bool SharedMnCreateDialog::isDust(CAmount value) const { - selection = CoinSelection{}; - error.clear(); - if (m_wallet_model == nullptr) { - error = tr("No wallet is available."); - return false; - } - if (target <= 0) { - error = tr("Select the participant that is you."); - return false; - } + const CTxOut probe{value, GetScriptForDestination(PKHash{CKeyID{}})}; + return IsDust(probe, m_node.getDustRelayFee()); +} +std::vector> SharedMnCreateDialog::fundingCandidates() const +{ + std::vector> candidates; + if (m_wallet_model == nullptr) return candidates; // listCoins() already holds back unconfirmed coins the wallet does not // trust, so an InstantSend-locked coin or this wallet's own unconfirmed // change can fund a share, as it can any other payment - std::vector> candidates; for (const auto& [dest, coins] : m_wallet_model->wallet().listCoins()) { for (const auto& [outpoint, txout] : coins) { if (txout.is_spent) continue; @@ -3397,6 +3404,58 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, candidates.emplace_back(txout.txout.nValue, outpoint); } } + return candidates; +} + +bool SharedMnCreateDialog::fundsShareAgain(const MnShareSession::Contribution& released, CAmount* total_out) const +{ + if (m_wallet_model == nullptr || released.inputs.empty()) return false; + const auto candidates{fundingCandidates()}; + CAmount total{0}; + for (const auto& input : released.inputs) { + const COutPoint outpoint{uint256S(input.txid.toStdString()), input.vout}; + const auto coin{std::find_if(candidates.begin(), candidates.end(), + [&](const auto& candidate) { return candidate.second == outpoint; })}; + if (coin == candidates.end()) return false; + total += coin->first; + } + if (total_out != nullptr) *total_out = total; + const CAmount target{contributionTarget()}; + if (released.hasChange) return total - released.changeAmount == target; + // Without change, a dust remainder goes to the fee, as selectCoins() leaves it + return total == target || (total > target && isDust(total - target)); +} + +const MnShareSession::Contribution* SharedMnCreateDialog::reservableAgain(CAmount* total_out) const +{ + if (m_my_share < 0 || static_cast(m_my_share) >= m_session.shares().size()) return nullptr; + const QString& label{m_session.shares()[m_my_share].label}; + // Only a contribution this wallet built itself goes back: one absorbed from + // a reply under this share's name can carry anything the sender chose + for (auto released{m_released_contributions.rbegin()}; released != m_released_contributions.rend(); ++released) { + if (released->label != label) continue; + const auto built{std::find_if(m_built_contributions.begin(), m_built_contributions.end(), [&](const auto& c) { + return c.label == label && MnShareSession::SameContribution(c, *released); + })}; + if (built != m_built_contributions.end() && fundsShareAgain(*built, total_out)) return &*built; + } + return nullptr; +} + +bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, QString& error) const +{ + selection = CoinSelection{}; + error.clear(); + if (m_wallet_model == nullptr) { + error = tr("No wallet is available."); + return false; + } + if (target <= 0) { + error = tr("Select the participant that is you."); + return false; + } + + std::vector> candidates{fundingCandidates()}; // Largest first keeps the input count, and with it everyone's signing work // and the transaction fee, as small as possible. std::sort(candidates.begin(), candidates.end(), @@ -3405,13 +3464,9 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, // A change output below the dust threshold cannot be relayed, so keep // adding coins until the change clears it; if nothing does, drop the // change and let the remainder go to the fee. - const auto is_dust = [this](CAmount value) { - const CTxOut probe{value, GetScriptForDestination(PKHash{CKeyID{}})}; - return IsDust(probe, m_node.getDustRelayFee()); - }; size_t used{0}; for (; used < candidates.size(); ++used) { - if (selection.total >= target && (selection.total == target || !is_dust(selection.total - target))) break; + if (selection.total >= target && (selection.total == target || !isDust(selection.total - target))) break; selection.total += candidates[used].first; MnShareSession::Input input; input.txid = QString::fromStdString(candidates[used].second.hash.ToString()); @@ -3428,7 +3483,7 @@ bool SharedMnCreateDialog::selectCoins(CAmount target, CoinSelection& selection, return false; } const CAmount change{selection.total - target}; - selection.change = (change > 0 && !is_dust(change)) ? change : 0; + selection.change = (change > 0 && !isDust(change)) ? change : 0; return true; } @@ -3475,28 +3530,37 @@ void SharedMnCreateDialog::reserveCoins() return; } - CoinSelection selection; - if (!selectCoins(contributionTarget(), selection, error)) { - showError(error); - return; - } + // Whoever replied before a release holds the contribution as it was then, + // and refuses locked terms with any other. While its coins are still free + // and fund the same amount, reserve exactly that contribution again. MnShareSession::Contribution contribution; - contribution.label = share.label; - contribution.inputs = selection.inputs; - if (selection.change > 0) { - const QString change_address{MasternodeWidgetUtil::freshAddress(m_wallet_model, error)}; - if (change_address.isEmpty()) { + const auto* const again{reservableAgain()}; + if (again != nullptr) { + contribution = *again; + } else { + CoinSelection selection; + if (!selectCoins(contributionTarget(), selection, error)) { showError(error); return; } - contribution.hasChange = true; - contribution.changeAddress = change_address; - contribution.changeAmount = selection.change; + contribution.label = share.label; + contribution.inputs = selection.inputs; + if (selection.change > 0) { + const QString change_address{MasternodeWidgetUtil::freshAddress(m_wallet_model, error)}; + if (change_address.isEmpty()) { + showError(error); + return; + } + contribution.hasChange = true; + contribution.changeAddress = change_address; + contribution.changeAmount = selection.change; + } } if (!m_session.addContribution(contribution, error)) { showError(error); return; } + if (again == nullptr) m_built_contributions.push_back(contribution); setContributionLocked(contribution, /*lock=*/true); m_coins_released = false; // Coins reserved again are no longer released diff --git a/src/qt/sharedmncreatedialog.h b/src/qt/sharedmncreatedialog.h index 9a33083524a1..808ad1088c84 100644 --- a/src/qt/sharedmncreatedialog.h +++ b/src/qt/sharedmncreatedialog.h @@ -21,6 +21,7 @@ #include #include #include +#include #include struct SharedMnOwnFunding; @@ -274,6 +275,16 @@ private Q_SLOTS: void followNewerInvitation(const MnShareSession& imported); // Funding + //! Whether a P2PKH output of `value` would be dust + bool isDust(CAmount value) const; + //! This wallet's unspent, unlocked, spendable coins that may fund a share + std::vector> fundingCandidates() const; + //! Whether `released`'s coins are all still funding candidates, and fund + //! exactly what the share needs now + bool fundsShareAgain(const MnShareSession::Contribution& released, CAmount* total_out = nullptr) const; + //! The most recently released contribution of this wallet's share that + //! Reserve would put back as it was, with its input total, or nullptr + const MnShareSession::Contribution* reservableAgain(CAmount* total_out = nullptr) const; //! Confirmed, unlocked, spendable wallet coins covering `target`, largest //! first, with the change kept above the dust threshold when possible bool selectCoins(CAmount target, CoinSelection& selection, QString& error) const; @@ -409,6 +420,9 @@ private Q_SLOTS: //! less any coins reserved again since. A reply to an earlier invitation //! still carries them, and only Reserve may put them back. std::vector m_released_contributions; + //! Contributions Reserve has reserved, built from this wallet's own coin + //! selection or put back: the only ones it may put back after a release + std::vector m_built_contributions; Role m_role{Role::Undecided}; int m_my_share{-1}; std::optional m_validation_page; diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index df8888215cad..9b9266630f4f 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -22,6 +22,7 @@ #include +#include #include #include #include @@ -47,11 +48,14 @@ void EditAddressAndSubmit( const QString& label, const QString& address, QString expected_msg) { QString warning_text; + // A successful edit opens no message box, and the answer must not wait + // for the next one, which belongs to whoever opens it + QObject scope; dialog->findChild("labelEdit")->setText(label); dialog->findChild("addressEdit")->setText(address); - ConfirmMessage(&warning_text, 5ms); + ConfirmMessage(&warning_text, 5ms, &scope); dialog->accept(); QCOMPARE(warning_text, expected_msg); } @@ -167,6 +171,13 @@ void TestAddAddressesToSendBook(interfaces::Node& node) check_addbook_size(4); QCOMPARE(table_view->model()->rowCount(), 3); + // The next message box is answered by whoever opens it + QMessageBox later(QMessageBox::Question, QString(), QString(), QMessageBox::Yes | QMessageBox::No); + later.setDefaultButton(QMessageBox::Yes); + QTimer::singleShot(100ms, &later, [&later] { later.button(QMessageBox::No)->click(); }); + later.exec(); + QCOMPARE(later.clickedButton(), later.button(QMessageBox::No)); + auto search_line = address_book.findChild("searchLineEdit"); search_line->setText(r_label); diff --git a/src/qt/test/sharedmnwizardtests.cpp b/src/qt/test/sharedmnwizardtests.cpp index 50ae27f77f61..f4967448649c 100644 --- a/src/qt/test/sharedmnwizardtests.cpp +++ b/src/qt/test/sharedmnwizardtests.cpp @@ -75,6 +75,7 @@ using MasternodeTestUtil::FreshOperatorPubKey; using MasternodeTestUtil::FreshP2PKHAddress; using MasternodeTestUtil::DraftReply; using MasternodeTestUtil::MakeSpendingWallet; +using MasternodeTestUtil::MakeTestWallet; using MasternodeTestUtil::PrepareRegistration; using MasternodeTestUtil::PreparedRegistration; using MasternodeTestUtil::WalletGuard; @@ -400,6 +401,228 @@ void SharedMnWizardTests::coordinatorResumesSavedSession() QCOMPARE(int(participant.currentPage()), int(SharedMnCreateDialog::PageContribution)); } +void SharedMnWizardTests::reservingAgainKeepsEarlierReplies() +{ + TestChain100Setup test; + m_node.setContext(&test.m_node); + WalletContext& context{*m_node.walletLoader().context()}; + const auto wallet{MakeTestWallet(m_node, context, "coord")}; + QVERIFY(wallet != nullptr); + WalletGuard guard{context, wallet}; + CTxDestination owner; + COutPoint coin; + COutPoint other_coin; + COutPoint twice_coin; + COutPoint dust_coin; + COutPoint dust_change_coin; + COutPoint sequence_coin; + COutPoint larger_coin; + { + LOCK(wallet->cs_wallet); + const auto destination{wallet->GetNewDestination("")}; + QVERIFY(destination); + owner = *destination; + const CBlockIndex* tip{ + WITH_LOCK(test.m_node.chainman->GetMutex(), return test.m_node.chainman->ActiveChain().Tip())}; + wallet->SetLastBlockProcessed(tip->nHeight, tip->GetBlockHash()); + CMutableTransaction funding; + funding.vin.emplace_back(GetRandHash(), 0); + funding.vout.emplace_back(450 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/0}) != nullptr); + coin = COutPoint{funding.GetHash(), 0}; + CMutableTransaction other_funding; + other_funding.vin.emplace_back(GetRandHash(), 0); + other_funding.vout.emplace_back(420 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(other_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/1}) != nullptr); + other_coin = COutPoint{other_funding.GetHash(), 0}; + // 410 for a contribution listing its coin twice; one with a dust + // remainder over a share of 400 and the default 0.001 fee + CMutableTransaction more_funding; + more_funding.vin.emplace_back(GetRandHash(), 0); + more_funding.vout.emplace_back(410 * COIN, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(400 * COIN + COIN / 1000 + 100, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(400 * COIN + COIN / 1000 + 100, GetScriptForDestination(owner)); + more_funding.vout.emplace_back(405 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(more_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/2}) != nullptr); + twice_coin = COutPoint{more_funding.GetHash(), 0}; + dust_coin = COutPoint{more_funding.GetHash(), 1}; + dust_change_coin = COutPoint{more_funding.GetHash(), 2}; + sequence_coin = COutPoint{more_funding.GetHash(), 3}; + // A larger coin a fresh selection would prefer, held back until needed + CMutableTransaction larger_funding; + larger_funding.vin.emplace_back(GetRandHash(), 0); + larger_funding.vout.emplace_back(600 * COIN, GetScriptForDestination(owner)); + QVERIFY(wallet->AddToWallet(MakeTransactionRef(larger_funding), + wallet::TxStateConfirmed{tip->GetBlockHash(), tip->nHeight, /*index=*/3}) != nullptr); + larger_coin = COutPoint{larger_funding.GetHash(), 0}; + } + MasternodeTestUtil::GuiModels models{m_node}; + QVERIFY2(models.ok, qPrintable(QString::fromStdString(models.error.translated))); + WalletModel wallet_model(interfaces::MakeWallet(context, wallet), models.client); + wallet_model.wallet().lockCoin(larger_coin, /*write_to_db=*/false); + + // This wallet coordinates and has answered for alice's row, but has not + // reserved its coins yet + CBLSSecretKey operator_secret; + operator_secret.MakeNewKey(); + MnShareSession draft{InvitationSession()}; + draft.setPrepareWallet(QStringLiteral("coord")); + draft.terms().operatorPubKey = + QString::fromStdString(operator_secret.GetPublicKey().ToString(/*specificLegacyScheme=*/false)); + draft.shares()[0].ownerAddress = QString::fromStdString(EncodeDestination(owner)); + draft.shares()[0].refundAddress = FreshP2PKHAddress(); + + SharedMnCreateDialog dialog(m_node, &wallet_model, /*parent=*/nullptr); + dialog.handleImportedText(draft.toJsonString()); + QCOMPARE(int(dialog.m_role), int(SharedMnCreateDialog::Role::Coordinator)); + QCOMPARE(dialog.myShareIndex(), 0); + dialog.goToPage(SharedMnCreateDialog::PageContribution); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + const MnShareSession::Contribution sent{*dialog.myContribution()}; + QVERIFY(sent.hasChange); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + + // bob and carol answer the invitation that carried this contribution + const MnShareSession invitation{dialog.m_session}; + const MnShareSession bob{DraftReply(invitation, 1, FakeTxid('2'))}; + const MnShareSession carol{DraftReply(invitation, 2, FakeTxid('3'))}; + QVERIFY(bob.hasContribution(sent)); + + // The coordinator releases its coins, takes in bob's reply, and reserves + // again: the same coins and change, which bob's copy already lists + dialog.releaseCoins(); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + dialog.handleImportedText(bob.toJsonString()); + QVERIFY2(dialog.m_error_label->text().isEmpty(), qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.myContribution() == nullptr); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + + // The coins card describes those coins, not the larger one that has + // become available since and a fresh selection would take + const auto amount = [&](CAmount value) { return SharedMnFormatAmount(SharedMnDisplayUnit(&wallet_model), value); }; + wallet_model.wallet().unlockCoin(larger_coin); + dialog.refreshContributionPage(); + const QString preview{dialog.m_coins_label->text()}; + QVERIFY2(preview.contains(amount(sent.changeAmount)) && !preview.contains(amount(600 * COIN)), qPrintable(preview)); + wallet_model.wallet().lockCoin(larger_coin, /*write_to_db=*/false); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + + // So the terms locked from it are the ones bob agreed to answer + dialog.handleImportedText(carol.toJsonString()); + QVERIFY2(dialog.m_error_label->text().isEmpty(), qPrintable(dialog.m_error_label->text())); + QVERIFY(dialog.allDetailsCollected()); + MnShareSession locked{dialog.m_session}; + const PreparedRegistration prepared{PrepareRegistration(locked, operator_secret)}; + QString error; + QVERIFY2(locked.freeze(prepared.txHex, prepared.consentHashHex, prepared.collateralIndex, error), + qPrintable(error)); + MnShareSession bob_copy{bob}; + QVERIFY2(bob_copy.adoptLockedTerms(locked, error), qPrintable(error)); + + // A contribution a reply lists under alice's name is never reserved again + // after a release, however well it funds the share: this wallet goes back + // to the one it built. First, one spending another coin of this wallet + // with its change paid elsewhere. + dialog.releaseCoins(); + MnShareSession planting{dialog.m_session}; + MnShareSession::Contribution planted; + planted.label = sent.label; + planted.inputs = {{QString::fromStdString(other_coin.hash.ToString()), other_coin.n}}; + planted.hasChange = true; + planted.changeAddress = FreshP2PKHAddress(); + planted.changeAmount = 420 * COIN - (400 * COIN + dialog.m_fee_field->value()); + QVERIFY2(planting.addContribution(planted, error), qPrintable(error)); + dialog.handleImportedText(planting.toJsonString()); + QVERIFY(dialog.m_session.hasContribution(planted)); + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(!dialog.m_session.hasContribution(planted)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + QVERIFY(!wallet_model.wallet().isLockedCoin(other_coin)); + + // One listing a coin twice, with this wallet's own change address + const auto plant_and_reserve_again = [&](const MnShareSession::Contribution& contribution) { + dialog.releaseCoins(); + MnShareSession reply{dialog.m_session}; + QString plant_error; + QVERIFY2(reply.addContribution(contribution, plant_error), qPrintable(plant_error)); + dialog.handleImportedText(reply.toJsonString()); + QVERIFY(dialog.m_session.hasContribution(contribution)); + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + }; + MnShareSession::Contribution twice{planted}; + const MnShareSession::Input twice_input{QString::fromStdString(twice_coin.hash.ToString()), twice_coin.n}; + twice.inputs = {twice_input, twice_input}; + twice.changeAddress = sent.changeAddress; + twice.changeAmount = 2 * 410 * COIN - (400 * COIN + dialog.m_fee_field->value()); + plant_and_reserve_again(twice); + QVERIFY(!dialog.m_session.hasContribution(twice)); + QVERIFY(dialog.m_session.hasContribution(sent)); + + // One with 100 duffs of change to this wallet, which could never be relayed + MnShareSession::Contribution dust_change{twice}; + dust_change.inputs = {{QString::fromStdString(dust_change_coin.hash.ToString()), dust_change_coin.n}}; + dust_change.changeAmount = 100; + plant_and_reserve_again(dust_change); + QVERIFY(!dialog.m_session.hasContribution(dust_change)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(!wallet_model.wallet().isLockedCoin(dust_change_coin)); + + // One that funds the share exactly, with change to this wallet, but whose + // input has a relative-lock sequence this wallet refuses to sign + MnShareSession::Contribution relative{twice}; + relative.inputs = {{QString::fromStdString(sequence_coin.hash.ToString()), sequence_coin.n, /*sequence=*/1}}; + relative.changeAmount = 405 * COIN - (400 * COIN + dialog.m_fee_field->value()); + plant_and_reserve_again(relative); + QVERIFY(!dialog.m_session.hasContribution(relative)); + QVERIFY(dialog.m_session.hasContribution(sent)); + QVERIFY(!wallet_model.wallet().isLockedCoin(sequence_coin)); + + // Without change, a dust remainder over the share is left to the fee. A + // contribution Reserve built that way is reserved again as it was. + dialog.releaseCoins(); + const std::vector held{coin, other_coin, twice_coin, dust_change_coin, sequence_coin}; + for (const auto& outpoint : held) { + wallet_model.wallet().lockCoin(outpoint, /*write_to_db=*/false); + } + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + const MnShareSession::Contribution no_change{*dialog.myContribution()}; + QVERIFY(!no_change.hasChange); + QCOMPARE(no_change.inputs.size(), size_t{1}); + QCOMPARE(no_change.inputs[0].txid, QString::fromStdString(dust_coin.hash.ToString())); + for (const auto& outpoint : held) { + wallet_model.wallet().unlockCoin(outpoint); + } + dialog.releaseCoins(); + dialog.reserveCoins(); + QVERIFY(dialog.m_session.hasContribution(no_change)); + QVERIFY(wallet_model.wallet().isLockedCoin(dust_coin)); + QVERIFY(!wallet_model.wallet().isLockedCoin(coin)); + + // With a different fee the released coins no longer fund the share as they + // did, so reserving again selects anew and earlier replies are out of date + dialog.releaseCoins(); + dialog.m_fee_field->setValue(dialog.m_fee_field->value() + COIN / 1000); + dialog.reserveCoins(); + QVERIFY2(dialog.myContribution() != nullptr, qPrintable(dialog.m_error_label->text())); + QVERIFY(!dialog.m_session.hasContribution(sent)); + QCOMPARE(dialog.myContribution()->changeAmount, sent.changeAmount - COIN / 1000); + QVERIFY(wallet_model.wallet().isLockedCoin(coin)); + wallet_model.wallet().unlockCoin(coin); +} + void SharedMnWizardTests::coordinatorResumesFullySignedSession() { TestingSetup test{CBaseChainParams::REGTEST}; diff --git a/src/qt/test/sharedmnwizardtests.h b/src/qt/test/sharedmnwizardtests.h index 7c96caad531f..24e961845949 100644 --- a/src/qt/test/sharedmnwizardtests.h +++ b/src/qt/test/sharedmnwizardtests.h @@ -29,6 +29,7 @@ private Q_SLOTS: void participantPagesAndRoleInference(); void participantCannotChooseATakenShare(); void coordinatorResumesSavedSession(); + void reservingAgainKeepsEarlierReplies(); void coordinatorResumesFullySignedSession(); void signedSessionReservation(); void unauthorisedSignedInputsAreDetected(); diff --git a/src/qt/test/util.cpp b/src/qt/test/util.cpp index d91b90fbba29..2646c7be4df2 100644 --- a/src/qt/test/util.cpp +++ b/src/qt/test/util.cpp @@ -13,9 +13,9 @@ #include #include -void ConfirmMessage(QString* text, std::chrono::milliseconds msec) +void ConfirmMessage(QString* text, std::chrono::milliseconds msec, QObject* context) { - QTimer::singleShot(msec, [text]() { + QTimer::singleShot(msec, context, [text]() { for (QWidget* widget : QApplication::topLevelWidgets()) { if (widget->inherits("QMessageBox")) { QMessageBox* messageBox = qobject_cast(widget); diff --git a/src/qt/test/util.h b/src/qt/test/util.h index a1788ca74e61..aa9253c25d6c 100644 --- a/src/qt/test/util.h +++ b/src/qt/test/util.h @@ -10,6 +10,7 @@ #include QT_BEGIN_NAMESPACE +class QObject; class QString; QT_END_NAMESPACE @@ -18,7 +19,9 @@ QT_END_NAMESPACE * * @param text - Optionally store dialog text. * @param msec - Number of milliseconds to pause before triggering the callback. + * @param context - Optionally cancel the callback when this object is destroyed, + * so it cannot answer a later message box or outlive `text`. */ -void ConfirmMessage(QString* text, std::chrono::milliseconds msec); +void ConfirmMessage(QString* text, std::chrono::milliseconds msec, QObject* context = nullptr); #endif // BITCOIN_QT_TEST_UTIL_H diff --git a/src/test/governance_vote_processing_tests.cpp b/src/test/governance_vote_processing_tests.cpp index e2f28c79128e..1b87452778ff 100644 --- a/src/test/governance_vote_processing_tests.cpp +++ b/src/test/governance_vote_processing_tests.cpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include #include #include @@ -88,8 +90,8 @@ struct GovernanceVoteSetup : public TestChainSetup { int64_t proposal_time{0}; std::string proposal_payment_address; - GovernanceVoteSetup() : - TestChainSetup(DIP3_ACTIVATION_HEIGHT - 2, CBaseChainParams::REGTEST, {"-dip3params=109:500"}) + explicit GovernanceVoteSetup(const char* dip3_params = "-dip3params=109:500") : + TestChainSetup(DIP3_ACTIVATION_HEIGHT - 2, CBaseChainParams::REGTEST, {dip3_params}) { // 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 @@ -232,6 +234,73 @@ struct TestGovernanceStore : GovernanceStore { BOOST_FIXTURE_TEST_SUITE(governance_vote_processing_tests, GovernanceVoteSetup) +struct GovernanceReorgSetup : GovernanceVoteSetup { + GovernanceReorgSetup() : + GovernanceVoteSetup("-dip3params=109:109") + { + } +}; + +BOOST_FIXTURE_TEST_CASE(votes_follow_masternode_identity_across_reorg, GovernanceReorgSetup) +{ + auto& chainman = *m_node.chainman; + auto& govman = *m_node.govman; + CKey owner_key, other_owner_key, voting_key; + CBLSSecretKey operator_key, other_operator_key; + voting_key.MakeNewKey(true); + const auto registration = CreateProRegTx(chainman, utxos, 2, payout_script(), coinbaseKey, owner_key, operator_key); + const auto other_registration = CreateProRegTx(chainman, utxos, 3, payout_script(), coinbaseKey, other_owner_key, + other_operator_key); + MineBlock({registration}); + CBlockIndex* fork_block{WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip())}; + const auto old_id = tip_mn_list().GetMN(registration.GetHash())->GetInternalId(); + + CProUpRegTx update; + update.nVersion = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + update.proTxHash = registration.GetHash(); + update.pubKeyOperator.Set(operator_key.GetPublicKey(), /*specificLegacyScheme=*/bls::bls_legacy_scheme); + update.keyIDVoting = voting_key.GetPubKey().GetID(); + update.scriptPayout = payout_script(); + CMutableTransaction tx; + tx.nVersion = 3; + tx.nType = TRANSACTION_PROVIDER_UPDATE_REGISTRAR; + const auto spent = FundTransaction(chainman, tx, utxos, payout_script(), COIN); + update.inputsHash = CalcTxInputsHash(CTransaction(tx)); + BOOST_REQUIRE(CHashSigner::SignHash(::SerializeHash(update), owner_key, update.vchSig)); + SetTxPayload(tx, update); + SignTransaction(tx, spent, coinbaseKey); + MineBlock({tx}); + govman.UpdatedBlockTip(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip())); + + const auto proposal = MakeProposal(uint256::ONE); + const auto parent_hash = proposal.GetHash(); + govman.AddGovernanceObjectForTesting(proposal); + auto unchanged_vote = MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES); + SignWithVotingKey(unchanged_vote, mn_voting_key); + CGovernanceVote changed_vote{COutPoint{registration.GetHash(), 0}, parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES}; + SignWithVotingKey(changed_vote, voting_key); + CGovernanceException exception; + uint256 hash_to_request; + BOOST_REQUIRE(govman.ProcessVote(unchanged_vote, exception, hash_to_request)); + BOOST_REQUIRE(govman.ProcessVote(changed_vote, exception, hash_to_request)); + BOOST_REQUIRE(govman.HaveVoteForHash(changed_vote.GetHash())); + + BlockValidationState state; + BOOST_REQUIRE(chainman.ActiveChainstate().InvalidateBlock(state, fork_block)); + MineBlock({other_registration}); + MineBlock({registration}); + BOOST_REQUIRE(old_id != tip_mn_list().GetMN(registration.GetHash())->GetInternalId()); + govman.UpdatedBlockTip(WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip())); + + BOOST_CHECK(!govman.HaveVoteForHash(changed_vote.GetHash())); + BOOST_CHECK(govman.GetCurrentVotes(parent_hash, changed_vote.GetMasternodeOutpoint()).empty()); + BOOST_CHECK(govman.HaveVoteForHash(unchanged_vote.GetHash())); + BOOST_CHECK_EQUAL(govman.GetCurrentVotes(parent_hash, mn_collateral).size(), 1U); + const auto stored = govman.FindConstGovernanceObject(parent_hash); + BOOST_REQUIRE(stored != nullptr); + BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1); +} + // 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) diff --git a/src/test/llmq_dkg_tests.cpp b/src/test/llmq_dkg_tests.cpp index 6595cf457a02..d2de8564211d 100644 --- a/src/test/llmq_dkg_tests.cpp +++ b/src/test/llmq_dkg_tests.cpp @@ -60,6 +60,23 @@ BOOST_AUTO_TEST_CASE(pending_messages_own_messages_share_quota_path) BOOST_CHECK_EQUAL(pending.PopPendingMessages(3).size(), 2U); } +BOOST_AUTO_TEST_CASE(pending_messages_own_message_is_processed_first) +{ + using namespace llmq; + + CDKGPendingMessages pending{/*_maxMessagesPerProTx=*/2}; + pending.PushPendingMessage(/*from=*/1, MakeTestHash(0xa1), MakeDKGMessage(), MakeTestHash(1)); + pending.PushPendingMessage(/*from=*/2, MakeTestHash(0xa2), MakeDKGMessage(), MakeTestHash(2)); + pending.PushPendingMessage(/*from=*/-1, MakeTestHash(0xee), MakeDKGMessage(), MakeTestHash(3)); + + const auto msgs = pending.PopPendingMessages(3); + BOOST_REQUIRE_EQUAL(msgs.size(), 3U); + auto it = msgs.begin(); + BOOST_CHECK_EQUAL((it++)->first, -1); + BOOST_CHECK_EQUAL((it++)->first, 1); + BOOST_CHECK_EQUAL((it++)->first, 2); +} + BOOST_AUTO_TEST_CASE(pending_messages_quota_survives_reconnect) { using namespace llmq; diff --git a/src/validation.cpp b/src/validation.cpp index 71a55d70615b..7813be6921b9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2925,6 +2925,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // Block is committed: keep the scheme it switched to (fJustCheck dry runs returned above). bls_scheme_guard.Commit(); + m_chain_helper->special_tx->RemoveMineableCommitmentsForBlock(block, pindex); if (this == &m_chainman.ActiveChainstate() && mnlist_updates.diff.HasChanges()) { GetMainSignals().NotifyMasternodeListChanged(false, mnlist_updates.old_list, mnlist_updates.diff); @@ -5218,6 +5219,7 @@ bool Chainstate::RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& in return false; } RecordBackgroundMNListHash(pindex, mnlist_updates.new_list); + m_chain_helper->special_tx->RemoveMineableCommitmentsForBlock(block, pindex); for (size_t i = 0; i < block.vtx.size(); i++) { const CTransactionRef& tx = block.vtx[i]; diff --git a/test/functional/feature_governance_trigger_voting.py b/test/functional/feature_governance_trigger_voting.py new file mode 100755 index 000000000000..836d8e43e3a6 --- /dev/null +++ b/test/functional/feature_governance_trigger_voting.py @@ -0,0 +1,444 @@ +#!/usr/bin/env python3 +# 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. +"""Test how masternodes vote on superblock triggers when their approved proposal set changes. + +A masternode creates or votes on triggers only from its block tip callback. To control which +masternodes have acted on a block before a trigger exists, the masternode that creates a trigger +is stopped while the block is mined and started again afterwards, so it creates the trigger while +catching up. A masternode that is stopped while proposal votes are cast keeps its earlier view of +the approved proposals. Each scenario uses its own short-lived proposals, so such a view only +matters within that scenario. +""" + +import json + +from test_framework.test_framework import BitcoinTestFramework, DashTestFramework +from test_framework.util import assert_equal + +BUDGET_ARGS = ["-budgetparams=10:10:10"] +SB_CYCLE = 20 +SB_MATURITY_WINDOW = 10 +NO_VOTE_DEADLINE_BLOCKS = 3 # src/governance/signing.cpp +GOVERNANCE_FEE_CONFIRMATIONS = 6 +PROPOSAL_LIFETIME = 60 * 60 +# Long enough for the proposals of the previous scenario to leave the payment window, and for every +# masternode to get clear of the trigger rate limit +SCENARIO_GAP = 4 * 60 * 60 + + +class GovernanceTriggerVotingTest(DashTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + + def set_test_params(self): + self.set_dash_test_params(6, 5, [BUDGET_ARGS] * 6) + + def online_nodes(self): + return [node for node in self.nodes if node.running] + + def online_mns(self): + return [mn for mn in self.mninfo if self.nodes[mn.nodeIdx].running] + + def flush_relay(self): + for node in self.online_nodes(): + node.mockscheduler(1) + + def wait_relayed(self, predicate): + def check(): + # Runs the once-a-second governance relay flush on every node + self.flush_relay() + return predicate() + self.wait_until(check, timeout=10) + + def mine(self): + nodes = self.online_nodes() + self.bump_mocktime(1, nodes=nodes) + self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks(nodes)) + # Masternodes act on a block only from the validation callback + for node in nodes: + node.syncwithvalidationinterfacequeue() + + def mine_to(self, height): + while self.nodes[0].getblockcount() < height: + self.mine() + assert_equal(self.nodes[0].getblockcount(), height) + + def connect_to_controller(self, idx): + running = [self.nodes[mn.nodeIdx] for mn in self.online_mns() if mn.nodeIdx != idx] + for node in running: + node.setmnthreadactive(False) + BitcoinTestFramework.connect_nodes(self, idx, 0) + for node in running: + node.setmnthreadactive(True) + + def stop_mn(self, mn): + self.stop_node(mn.nodeIdx) + + def start_mn(self, mn): + node = self.nodes[mn.nodeIdx] + node.mocktime = self.mocktime + self.start_masternode(mn) + self.connect_to_controller(mn.nodeIdx) + self.sync_blocks(self.online_nodes()) + node.syncwithvalidationinterfacequeue() + + def restart_mn(self, mn): + self.stop_mn(mn) + self.start_mn(mn) + + def payee(self, block_height): + winners = self.nodes[0].masternode("winners") + # Payees are projected for one rotation only, and each masternode is paid once per rotation + while str(block_height) not in winners: + block_height -= len(self.mninfo) + payee_address = winners[str(block_height)] + matches = [mn for mn in self.mninfo if self.payout_address[mn.proTxHash] in payee_address] + assert_equal(len(matches), 1) + return matches[0] + + def own_vote(self, mn, trigger_hash): + """The masternode's own funding vote on the object, as known to its own node.""" + collateral_hash, collateral_index = self.collateral[mn.proTxHash] + votes = self.nodes[mn.nodeIdx].gobject("getcurrentvotes", trigger_hash, collateral_hash, str(collateral_index)) + outcomes = [v.split(":")[2] for v in votes.values() if v.split(":")[3] == "funding"] + assert len(outcomes) <= 1 + return outcomes[0] if outcomes else None + + def triggers_for(self, node, sb_height): + return {h: t for h, t in node.gobject("list", "valid", "triggers").items() + if json.loads(t["DataString"])["event_block_height"] == sb_height} + + def trigger_addresses(self, trigger_hash): + data = json.loads(self.nodes[0].gobject("get", trigger_hash)["DataString"]) + return set(data["payment_addresses"].split("|")) + + def wait_trigger_everywhere(self, trigger_hash): + self.wait_relayed(lambda: all(trigger_hash in node.gobject("list", "valid", "triggers") + for node in self.online_nodes())) + + def paid_addresses(self, height): + assert_equal(self.nodes[0].getblockcount(), height) + coinbase = self.nodes[0].getblock(self.nodes[0].getblockhash(height), 2)["tx"][0] + return {txout["scriptPubKey"].get("address") for txout in coinbase["vout"]} + + def create_proposals(self, count): + node = self.nodes[0] + created_at = self.mocktime + prepared = [] + for _ in range(count): + address = node.getnewaddress() + proposal = { + "type": 1, + "name": f"Proposal_{len(self.proposal_address)}", + "start_epoch": created_at, + "end_epoch": created_at + PROPOSAL_LIFETIME, + "payment_amount": 1, + "payment_address": address, + "url": "https://dash.org", + } + proposal_hex = json.dumps(proposal).encode().hex() + collateral = node.gobject("prepare", "0", 1, created_at, proposal_hex) + prepared.append((proposal_hex, collateral, address)) + self.sync_mempools(self.online_nodes()) + # Collateral transactions without an InstantSend lock are only mined once they are this old + self.bump_mocktime(10 * 60 + 1, nodes=self.online_nodes()) + for _ in range(GOVERNANCE_FEE_CONFIRMATIONS): + self.mine() + proposals = [] + for proposal_hex, collateral, address in prepared: + proposal_hash = node.gobject("submit", "0", 1, created_at, proposal_hex, collateral) + self.proposal_address[proposal_hash] = address + proposals.append(proposal_hash) + self.wait_relayed(lambda: all(set(proposals) <= set(n.gobject("list", "valid", "proposals")) + for n in self.online_nodes())) + return proposals + + def set_approval(self, changes): + """Approve (True) or drop (False) proposals, each change cast with a voting key that has not + voted on that proposal yet, and wait until every running node sees the result.""" + for proposal_hash, approved in changes.items(): + voter = self.mninfo[self.proposal_voters.get(proposal_hash, 0)] + self.proposal_voters[proposal_hash] = self.proposal_voters.get(proposal_hash, 0) + 1 + result = self.nodes[0].gobject("vote-alias", proposal_hash, "funding", "yes" if approved else "no", + voter.proTxHash) + assert_equal(result["overall"], "Voted successfully 1 time(s) and failed 0 time(s).") + + def seen_everywhere(): + return all((n.gobject("get", h)["FundingResult"]["AbsoluteYesCount"] >= 1) == approved + for h, approved in changes.items() for n in self.online_nodes()) + self.wait_relayed(seen_everywhere) + + def start_scenario(self, proposal_count): + assert_equal(len(self.online_nodes()), self.num_nodes) + self.bump_mocktime(SCENARIO_GAP, update_schedulers=False) + proposals = self.create_proposals(proposal_count) + tip = self.nodes[0].getblockcount() + sb_height = tip - tip % SB_CYCLE + SB_CYCLE + if sb_height - SB_MATURITY_WINDOW - 1 <= tip: + sb_height += SB_CYCLE + return sb_height, proposals + + def create_first_trigger(self, sb_height): + """Mine the first block of the maturity window while its trigger creator is stopped, then start + the creator. Every other running masternode has acted on that block before the trigger exists.""" + window_start = sb_height - SB_MATURITY_WINDOW + assert_equal(self.nodes[0].getblockcount(), window_start - 1) + creator = self.payee(window_start + 1) + self.stop_mn(creator) + self.mine() + assert_equal(self.triggers_for(self.nodes[0], sb_height), {}) + self.start_mn(creator) + triggers = self.triggers_for(self.nodes[creator.nodeIdx], sb_height) + assert_equal(len(triggers), 1) + trigger = next(iter(triggers)) + self.wait_trigger_everywhere(trigger) + assert_equal(self.own_vote(creator, trigger), "yes") + for mn in self.online_mns(): + if mn is not creator: + assert_equal(self.own_vote(mn, trigger), None) + return creator, trigger + + def run_test(self): + node = self.nodes[0] + governance_info = node.getgovernanceinfo() + assert_equal(governance_info["superblockcycle"], SB_CYCLE) + assert_equal(governance_info["superblockmaturitywindow"], SB_MATURITY_WINDOW) + assert_equal(governance_info["governanceminquorum"], 1) + + protx_info = {mn.proTxHash: node.protx("info", mn.proTxHash) for mn in self.mninfo} + self.payout_address = {h: info["state"]["payoutAddress"] for h, info in protx_info.items()} + self.collateral = {h: (info["collateralHash"], info["collateralIndex"]) for h, info in protx_info.items()} + self.proposal_address = {} + self.proposal_voters = {} + + self.test_single_change_is_snapshot() + self.test_creator_votes_after_unfunded_cycle(restart=False) + self.test_creator_votes_after_unfunded_cycle(restart=True) + self.test_own_no_vote_is_kept() + self.test_yes_vote_survives_tip_below_window() + # Leaves a masternode that does not know the trigger the superblock pays, so it runs last + self.test_no_votes_wait_for_deadline() + + def test_single_change_is_snapshot(self): + self.log.info("A single change of the approved set: the trigger for the later set is paid") + sb_height, (p_kept, p_dropped) = self.start_scenario(2) + self.set_approval({p_kept: True, p_dropped: True}) + self.mine_to(sb_height - SB_MATURITY_WINDOW - 1) + creator, first_trigger = self.create_first_trigger(sb_height) + assert_equal(self.trigger_addresses(first_trigger), + {self.proposal_address[p_kept], self.proposal_address[p_dropped]}) + + self.set_approval({p_dropped: False}) + second_creator = self.payee(self.nodes[0].getblockcount() + 2) + self.mine() + triggers = self.triggers_for(self.nodes[second_creator.nodeIdx], sb_height) + assert_equal(len(triggers), 2) + second_trigger = next(h for h in triggers if h != first_trigger) + self.wait_trigger_everywhere(second_trigger) + assert_equal(self.trigger_addresses(second_trigger), {self.proposal_address[p_kept]}) + self.mine() + + for node in self.nodes: + assert_equal(set(self.triggers_for(node, sb_height)), {first_trigger, second_trigger}) + for mn in self.mninfo: + if mn is creator: + assert_equal(self.own_vote(mn, first_trigger), "yes") + assert_equal(self.own_vote(mn, second_trigger), "no") + else: + assert_equal(self.own_vote(mn, first_trigger), "no") + assert_equal(self.own_vote(mn, second_trigger), "yes") + self.mine_to(sb_height) + paid = self.paid_addresses(sb_height) + assert self.proposal_address[p_kept] in paid + assert self.proposal_address[p_dropped] not in paid + + def test_creator_votes_after_unfunded_cycle(self, restart): + if restart: + self.log.info("A trigger creator that re-learns its YES after a restart votes YES in the next cycle " + "after its trigger is not funded") + else: + self.log.info("A trigger creator votes YES in the next cycle after its trigger is not funded") + sb_height, (proposal,) = self.start_scenario(1) + address = self.proposal_address[proposal] + self.set_approval({proposal: True}) + self.mine_to(sb_height - SB_MATURITY_WINDOW - 1) + creator, trigger = self.create_first_trigger(sb_height) + creator_node = self.nodes[creator.nodeIdx] + if restart: + # The creator keeps its approved set: it misses the vote that empties it + self.stop_mn(creator) + + self.log.info("With no approved proposal, masternodes vote NO on the trigger at the next block") + self.set_approval({proposal: False}) + self.mine() + for mn in self.online_mns(): + if mn is not creator: + assert_equal(self.own_vote(mn, trigger), "no") + if restart: + with creator_node.assert_debug_log([f"Not voting YES-FUNDING for trigger:{trigger}, we voted yes for it already"]): + self.start_mn(creator) + assert_equal(self.own_vote(creator, trigger), "yes") + self.mine_to(sb_height) + assert address not in self.paid_addresses(sb_height) + + self.log.info("The proposal is approved again: every masternode votes YES on the next trigger and it is paid") + next_sb_height = sb_height + SB_CYCLE + self.set_approval({proposal: True}) + self.mine_to(next_sb_height - SB_MATURITY_WINDOW - 1) + + def one_trigger_everywhere(): + self.flush_relay() + return all(len(self.triggers_for(n, next_sb_height)) == 1 for n in self.online_nodes()) + while True: + assert self.nodes[0].getblockcount() < next_sb_height - NO_VOTE_DEADLINE_BLOCKS - 2 + self.mine() + if self.wait_until(one_trigger_everywhere, timeout=5, do_assert=False): + break + next_trigger = next(iter(self.triggers_for(self.nodes[0], next_sb_height))) + # Every masternode knows the trigger and acts on it at this block + self.mine() + assert_equal(self.own_vote(creator, next_trigger), "yes") + for mn in self.mninfo: + assert_equal(self.own_vote(mn, next_trigger), "yes") + self.mine_to(next_sb_height) + assert address in self.paid_addresses(next_sb_height) + + def test_own_no_vote_is_kept(self): + self.log.info("After the approved set is emptied and restored, a NO on the matching trigger is kept " + "and another trigger still gets NO") + sb_height, (p_main, p_extra) = self.start_scenario(2) + self.set_approval({p_main: True, p_extra: True}) + self.mine_to(sb_height - SB_MATURITY_WINDOW - 1) + window_start = sb_height - SB_MATURITY_WINDOW + # The masternode paid at window_start + 3 keeps the approved set {p_main, p_extra} and creates its + # trigger once the others are back to {p_main} + late = self.payee(window_start + 3) + assert late is not self.payee(window_start + 1) + self.stop_mn(late) + self.set_approval({p_extra: False}) + creator, trigger = self.create_first_trigger(sb_height) + assert_equal(self.trigger_addresses(trigger), {self.proposal_address[p_main]}) + + self.set_approval({p_main: False}) + self.mine() + holders = [mn for mn in self.online_mns() if mn is not creator] + for mn in holders: + assert_equal(self.own_vote(mn, trigger), "no") + + self.set_approval({p_main: True}) + with self.nodes[holders[0].nodeIdx].assert_debug_log([f"Not voting YES-FUNDING for trigger:{trigger}, we voted no for it already"]): + self.mine() + assert_equal(self.nodes[0].getblockcount(), window_start + 2) + self.start_mn(late) + late_triggers = self.triggers_for(self.nodes[late.nodeIdx], sb_height) + assert_equal(len(late_triggers), 1) + other_trigger = next(iter(late_triggers)) + self.wait_trigger_everywhere(other_trigger) + assert_equal(self.trigger_addresses(other_trigger), + {self.proposal_address[p_main], self.proposal_address[p_extra]}) + self.mine() + + for mn in holders: + assert_equal(self.own_vote(mn, trigger), "no") + assert_equal(self.own_vote(mn, other_trigger), "no") + assert_equal(self.own_vote(creator, trigger), "yes") + assert_equal(self.own_vote(creator, other_trigger), "no") + assert_equal(self.own_vote(late, other_trigger), "yes") + self.mine_to(sb_height) + paid = self.paid_addresses(sb_height) + assert self.proposal_address[p_main] not in paid + assert self.proposal_address[p_extra] not in paid + + def test_yes_vote_survives_tip_below_window(self): + self.log.info("A YES is kept across a tip below the maturity window: no second YES after re-entering it") + sb_height, (p_kept, p_dropped) = self.start_scenario(2) + self.set_approval({p_kept: True, p_dropped: True}) + window_start = sb_height - SB_MATURITY_WINDOW + self.mine_to(window_start - 1) + creator, trigger = self.create_first_trigger(sb_height) + + fork_point = self.nodes[0].getblockhash(window_start - 1) + for node in self.nodes: + node.invalidateblock(fork_point) + assert_equal(node.getblockcount(), window_start - 2) + assert window_start - 2 > sb_height - SB_CYCLE + self.set_approval({p_dropped: False}) + # A tip below the window + self.mine() + assert_equal(self.payee(window_start + 1), creator) + # The creator is the payee again but has voted YES already, so it creates no trigger + self.mine() + assert_equal(set(self.triggers_for(self.nodes[creator.nodeIdx], sb_height)), {trigger}) + new_creator = self.payee(self.nodes[0].getblockcount() + 2) + self.mine() + new_triggers = set(self.triggers_for(self.nodes[new_creator.nodeIdx], sb_height)) - {trigger} + assert_equal(len(new_triggers), 1) + new_trigger = new_triggers.pop() + self.wait_trigger_everywhere(new_trigger) + assert_equal(self.trigger_addresses(new_trigger), {self.proposal_address[p_kept]}) + self.mine() + + for node in self.nodes: + assert_equal(set(self.triggers_for(node, sb_height)), {trigger, new_trigger}) + assert_equal(self.own_vote(creator, trigger), "yes") + assert_equal(self.own_vote(creator, new_trigger), "no") + for mn in self.mninfo: + if mn is not creator: + assert_equal(self.own_vote(mn, new_trigger), "yes") + self.mine_to(sb_height) + paid = self.paid_addresses(sb_height) + assert self.proposal_address[p_kept] in paid + assert self.proposal_address[p_dropped] not in paid + + def test_no_votes_wait_for_deadline(self): + self.log.info("Without a trigger for its own candidate, a masternode votes NO on other triggers only " + f"in the last {NO_VOTE_DEADLINE_BLOCKS} blocks before the superblock") + sb_height, (p_a, p_b) = self.start_scenario(2) + self.set_approval({p_a: True, p_b: True}) + self.mine_to(sb_height - SB_MATURITY_WINDOW - 1) + # The masternode paid at sb_height - 5 keeps the approved set {p_a, p_b}: it is stopped until the + # block before the one it is paid in, then creates its trigger while catching up + late = self.payee(sb_height - 5) + # The masternode paid at sb_height - 6 is restarted after the approved set changes late + waiting = self.payee(sb_height - 6) + assert late is not self.payee(sb_height - SB_MATURITY_WINDOW + 1) + assert waiting is not self.payee(sb_height - SB_MATURITY_WINDOW + 1) + self.stop_mn(late) + self.set_approval({p_b: False}) + creator, trigger = self.create_first_trigger(sb_height) + self.mine() + for mn in self.online_mns(): + assert_equal(self.own_vote(mn, trigger), "yes") + + self.log.info("The approved set changes after every running masternode voted YES") + self.set_approval({p_a: False, p_b: True}) + self.mine_to(sb_height - 7) + # It was the payee at this block but has voted YES already, so it created no trigger + assert_equal(set(self.triggers_for(self.nodes[waiting.nodeIdx], sb_height)), {trigger}) + self.restart_mn(waiting) + self.mine() + # Its candidate has no trigger, and it is not the payee + assert_equal(set(self.triggers_for(self.nodes[waiting.nodeIdx], sb_height)), {trigger}) + self.start_mn(late) + late_triggers = self.triggers_for(self.nodes[late.nodeIdx], sb_height) + assert_equal(len(late_triggers), 1) + other_trigger = next(iter(late_triggers)) + self.wait_trigger_everywhere(other_trigger) + assert_equal(self.trigger_addresses(other_trigger), {self.proposal_address[p_a], self.proposal_address[p_b]}) + assert_equal(self.own_vote(waiting, other_trigger), None) + + while self.nodes[0].getblockcount() + 1 + NO_VOTE_DEADLINE_BLOCKS < sb_height: + assert self.payee(self.nodes[0].getblockcount() + 2) is not waiting + self.mine() + assert_equal(self.own_vote(waiting, other_trigger), None) + assert_equal(self.own_vote(creator, other_trigger), "no") + assert_equal(self.nodes[0].getblockcount(), sb_height - NO_VOTE_DEADLINE_BLOCKS - 1) + self.mine() + assert_equal(self.own_vote(waiting, other_trigger), "no") + assert_equal(self.own_vote(waiting, trigger), "yes") + + +if __name__ == '__main__': + GovernanceTriggerVotingTest().main() diff --git a/test/functional/feature_llmq_dkgerrors.py b/test/functional/feature_llmq_dkgerrors.py index a150a4dfc5b7..4c466279a8ce 100755 --- a/test/functional/feature_llmq_dkgerrors.py +++ b/test/functional/feature_llmq_dkgerrors.py @@ -116,6 +116,17 @@ def test_qc(self): # TODO: test signers # TODO: test validMembers + # The commitment is mineable again after invalidateblock and must stay so when a block that + # carries it is rejected by a check that runs after the commitments were processed + assert self.node_has_quorum_commitment(self.nodes[0], quorumHash, 100) + bad_block = copy.deepcopy(block) + bad_block.vtx[0].vout[0].nValue += 1 + bad_block.vtx[0].rehash() + bad_block.hashMerkleRoot = bad_block.calc_merkle_root() + bad_block.solve() + assert_equal(self.nodes[0].submitblock(bad_block.serialize().hex()), 'bad-cb-amount') + assert self.node_has_quorum_commitment(self.nodes[0], quorumHash, 100) + self.nodes[0].reconsiderblock(best) # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(self.nodes)) diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 0e381fdd4dab..f5fe8d2c1ea9 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -12,10 +12,16 @@ import urllib.parse +from test_framework.blocktools import create_block, create_coinbase from test_framework.messages import ( BLOCK_HEADER_SIZE, COIN, + COutPoint, + CTransaction, + CTxIn, + CTxOut, ) +from test_framework.script import CScript, OP_TRUE from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -449,5 +455,44 @@ def run_test(self): self.sync_all() assert_equal(self.test_rest_request(f"/deploymentinfo/{header_only_hash}"), self.nodes[0].getdeploymentinfo(header_only_hash)) + self.log.info("Test rendering stored ProTx payloads with unsupported versions") + node = self.nodes[0] + tip = node.getbestblockhash() + protxes = [] + for tx_type in (1, 2): + for version in (0, 65535): + tx = CTransaction() + tx.nVersion = 3 + tx.nType = tx_type + tx.vin = [CTxIn(COutPoint(0x1234, len(protxes)))] + tx.vout = [CTxOut(1000, CScript([OP_TRUE]))] + tx.vExtraPayload = version.to_bytes(2, "little") + tx.calc_sha256() + protxes.append(tx) + block = create_block( + tmpl=node.getblocktemplate(), + coinbase=create_coinbase(node.getblockcount() + 1, dip4_activated=True, v20_activated=True), + txlist=protxes, + ) + block.solve() + assert_equal(node.submitblock(block.serialize().hex()), "bad-protx-version") + assert_equal(node.getblock(block.hash, 0), block.serialize().hex()) + + for deprecated_service in (False, True): + if deprecated_service: + self.restart_node(0, extra_args=self.extra_args[0] + ["-deprecatedrpc=service"]) + self.test_rest_request(f"/block/notxdetails/{block.hash}") + block_json = self.test_rest_request(f"/block/{block.hash}") + assert_equal(block_json, node.getblock(block.hash, 2)) + assert_equal(len(block_json["tx"]), len(block.vtx)) + for tx, tx_json in zip(protxes, block_json["tx"][1:]): + field = "proRegTx" if tx.nType == 1 else "proUpServTx" + for decoded in (tx_json, node.decoderawtransaction(tx.serialize().hex()), + node.getrawtransaction(tx.hash, True, block.hash)): + assert field not in decoded + assert_equal(decoded["extraPayload"], tx.vExtraPayload.hex()) + assert_equal(node.getbestblockhash(), tip) + assert_equal(self.test_rest_request("/chaininfo")["bestblockhash"], tip) + if __name__ == '__main__': RESTTest().main() diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py index 7b5226591c43..e702f543109b 100755 --- a/test/functional/rpc_deprecated.py +++ b/test/functional/rpc_deprecated.py @@ -63,11 +63,13 @@ def test_malformed_deprecated_service_rawtransactions(self): self.log.info("Test malformed provider payloads do not crash -deprecatedrpc=service") node = self.nodes[2] block_count = node.getblockcount() - for tx_hex in [ - "03000100000000000000020000", # ProRegTx with only nVersion=0 payload - "03000200000000000000020000", # ProUpServTx with only nVersion=0 payload + for tx_hex, payload_field in [ + ("03000100000000000000020000", "proRegTx"), # ProRegTx with only nVersion=0 payload + ("03000200000000000000020000", "proUpServTx"), # ProUpServTx with only nVersion=0 payload ]: - assert_raises_rpc_error(-1, "Internal bug detected: obj.netInfo", node.decoderawtransaction, tx_hex) + decoded = node.decoderawtransaction(tx_hex) + assert payload_field not in decoded + assert_equal(decoded["extraPayload"], "0000") assert_equal(node.getblockcount(), block_count) if __name__ == '__main__': diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 4d6bee430d26..001ae3b0d334 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -136,6 +136,7 @@ 'feature_masternode_params_devnet.py', # NOTE: needs dash_hash to pass 'feature_governance.py --descriptors', 'feature_governance_cl.py --descriptors', + 'feature_governance_trigger_voting.py --descriptors', 'rpc_verifyislock.py', 'feature_dip3_deterministicmns.py --descriptors', # NOTE: needs dash_hash to pass 'feature_notifications.py',