diff --git a/ci/test/00_setup_env_native_multiprocess.sh b/ci/test/00_setup_env_native_multiprocess.sh index fbb065a1ae95..3783471d0da3 100755 --- a/ci/test/00_setup_env_native_multiprocess.sh +++ b/ci/test/00_setup_env_native_multiprocess.sh @@ -7,7 +7,9 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_multiprocess -case "$(uname -m)" in +# DEPENDS_ARCH overrides the machine arch (uname -m) when the depends are +# computed on a different machine than the one that builds them (CI check-cache). +case "${DEPENDS_ARCH:-$(uname -m)}" in aarch64) export HOST=aarch64-linux-gnu ;; diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh index 562b9e6d7780..c9f051d98627 100755 --- a/ci/test/00_setup_env_native_tsan.sh +++ b/ci/test/00_setup_env_native_tsan.sh @@ -7,7 +7,9 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_tsan -case "$(uname -m)" in +# DEPENDS_ARCH overrides the machine arch (uname -m) when the depends are +# computed on a different machine than the one that builds them (CI check-cache). +case "${DEPENDS_ARCH:-$(uname -m)}" in aarch64) export HOST=aarch64-linux-gnu ;; diff --git a/doc/release-notes-27217.md b/doc/release-notes-27217.md new file mode 100644 index 000000000000..ab1452fc9420 --- /dev/null +++ b/doc/release-notes-27217.md @@ -0,0 +1,6 @@ +Wallet +------ + +* Address Purposes strings are now restricted to the currently known values of "send", "receive", and "refund". + Wallets that have unrecognized purpose strings will have loading warnings, and the `listlabels` + RPC will raise an error if an unrecognized purpose is requested. diff --git a/doc/release-notes-7831.md b/doc/release-notes-7831.md new file mode 100644 index 000000000000..3544e0b87877 --- /dev/null +++ b/doc/release-notes-7831.md @@ -0,0 +1,8 @@ +P2P and network changes +----------------------- + +- Governance object signatures larger than 96 bytes are now rejected, both from + the network and from the on-disk governance cache (`governance.dat`). The + cache format version was bumped for this: on the first start after + upgrading, the existing cache is discarded and governance objects and votes + are synced again from the network. (#7831) diff --git a/doc/release-notes-7842.md b/doc/release-notes-7842.md new file mode 100644 index 000000000000..9ce37eff3d8e --- /dev/null +++ b/doc/release-notes-7842.md @@ -0,0 +1,21 @@ +Wallet +------ + +- Creating an HD wallet with a mnemonic passphrase longer than 248 bytes (not characters) through `upgradetohd`, + `createwallet` or `-mnemonicpassphrase` is now rejected, and one longer than 256 bytes is always rejected. + Existing wallets are not affected. (#7842) + + Seed derivation uses the BIP39 salt `"mnemonic"` followed by the passphrase. Dash Core 19.2.0 started cutting that + salt at 256 bytes to stop a crash, so every passphrase byte after the 248th has been ignored since then. Two + passphrases that differ only after that point derive the same seed. Before 23.0.0 the salt was also cut at the + first NUL byte. + + The derivation is left as it is. Using the full passphrase would bring back the pre-19.2.0 derivation, and a wallet + created with a long passphrase on 19.2.0 or later could then no longer be recovered from its mnemonic. Instead, new + wallets can't be given a passphrase that the derivation would truncate. + + To recover a wallet that was created on 19.2.0 or later with a passphrase of 249 to 256 bytes, start the node with + the new `-allow-deprecated-long-mnemonics` option, which accepts passphrases of up to 256 bytes and derives the same + (truncated) seed as before. A wallet created before 19.2.0 with a passphrase longer than 248 bytes can't be + recovered from its mnemonic on current versions, with or without the option. Restore it from a wallet backup + instead. diff --git a/doc/release-notes-7857.md b/doc/release-notes-7857.md new file mode 100644 index 000000000000..d6781a125242 --- /dev/null +++ b/doc/release-notes-7857.md @@ -0,0 +1,13 @@ +Mining and mempool policy +------------------------- + +- When a block template includes an operator key change or revocation for a masternode, it now + places the masternode's pending `ProUpServTx` signed with the key being replaced before it. Such + a service update could otherwise confirm after the change in the same block and restore the + previous operator's service and payout details. A service update that cannot be placed before + the change, or that does not fit in the block with it, is left out of that block. (#7857) +- The mempool now rejects with `protx-dup` a `ProUpServTx` that spends, directly or through other + pending transactions, an output of a pending operator key change or revocation, which it can only + confirm after, or of a pending asset lock, asset unlock or MNHF signal, which could keep it out of + a block. A service update funded from such outputs can be sent once they confirm, signed with the + operator key that is current then. (#7857) diff --git a/doc/release-notes-7858.md b/doc/release-notes-7858.md new file mode 100644 index 000000000000..fd931fc8fea8 --- /dev/null +++ b/doc/release-notes-7858.md @@ -0,0 +1,8 @@ +InstantSend changes +------------------- + +- Masternodes no longer InstantSend-lock provider transactions (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx and the + other provider special transactions). They are mined once they have been in the mempool for 10 minutes, and + transactions that spend their outputs can be locked once the block that includes the provider transaction is + ChainLocked. Software that waits for an islock on a provider transaction should wait for a ChainLock or confirmations + instead. (#7858) diff --git a/src/Makefile.am b/src/Makefile.am index b5e88df4d4e4..44fb7be39ced 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -489,7 +489,6 @@ BITCOIN_CORE_H = \ wallet/external_signer_scriptpubkeyman.h \ wallet/fees.h \ wallet/hdchain.h \ - wallet/ismine.h \ wallet/load.h \ wallet/platformkeys.h \ wallet/platformtypes.h \ @@ -501,6 +500,7 @@ BITCOIN_CORE_H = \ wallet/spend.h \ wallet/sqlite.h \ wallet/transaction.h \ + wallet/types.h \ wallet/wallet.h \ wallet/walletdb.h \ wallet/wallettool.h \ diff --git a/src/active/dkgsessionhandler.cpp b/src/active/dkgsessionhandler.cpp index cea8553d48f5..c11fcd3d18cd 100644 --- a/src/active/dkgsessionhandler.cpp +++ b/src/active/dkgsessionhandler.cpp @@ -87,16 +87,23 @@ bool ActiveDKGSessionHandler::InitNewQuorum(gsl::not_null pQ return false; } - curSession = std::make_unique(m_bls_worker, m_dmnman, m_dkgdbgman, m_qdkgsman, m_mn_metaman, - m_qsnapman, m_mn_activeman, m_chainman, m_sporkman, - pQuorumBaseBlockIndex, params); + auto next_session = std::make_shared(m_bls_worker, m_dmnman, m_dkgdbgman, m_qdkgsman, + m_mn_metaman, m_qsnapman, m_mn_activeman, m_chainman, + m_sporkman, pQuorumBaseBlockIndex, params); - if (!curSession->Init(m_mn_activeman.GetProTxHash(), quorumIndex)) { + { + std::shared_ptr previous_session; + WITH_LOCK(cs_session, previous_session.swap(curSession)); + } + + if (!next_session->Init(m_mn_activeman.GetProTxHash(), quorumIndex)) { LogPrintf("ActiveDKGSessionHandler::%s -- height[%d] quorum initialization failed for %s qi[%d]\n", __func__, pQuorumBaseBlockIndex->nHeight, params.name, quorumIndex); return false; } + WITH_LOCK(cs_session, curSession = std::move(next_session)); + LogPrintf("ActiveDKGSessionHandler::%s -- height[%d] quorum initialization OK for %s qi[%d]\n", __func__, pQuorumBaseBlockIndex->nHeight, params.name, quorumIndex); return true; } @@ -166,7 +173,8 @@ void ActiveDKGSessionHandler::WaitForNewQuorum(const uint256& oldQuorumHash) con void ActiveDKGSessionHandler::SleepBeforePhase(QuorumPhase curPhase, const uint256& expectedQuorumHash, double randomSleepFactor, const WhileWaitFunc& runWhileWaiting) const { - if (!curSession->AreWeMember()) { + const auto session = GetCurSession(); + if (!session || !session->AreWeMember()) { // Non-members do not participate and do not create any network load, no need to sleep. return; } @@ -187,7 +195,7 @@ void ActiveDKGSessionHandler::SleepBeforePhase(QuorumPhase curPhase, const uint2 // Don't expect perfect block times and thus reduce the phase time to be on the secure side (caller chooses factor) double adjustedPhaseSleepTimePerMember = phaseSleepTimePerMember * randomSleepFactor; - int64_t sleepTime = static_cast(adjustedPhaseSleepTimePerMember * curSession->GetMyMemberIndex().value_or(0)); + int64_t sleepTime = static_cast(adjustedPhaseSleepTimePerMember * session->GetMyMemberIndex().value_or(0)); const auto endTime = SteadyClock::now() + std::chrono::milliseconds{sleepTime}; int heightTmp{currentHeight.load()}; int heightStart{heightTmp}; @@ -248,22 +256,31 @@ void ActiveDKGSessionHandler::HandlePhase(QuorumPhase curPhase, QuorumPhase next bool ActiveDKGSessionHandler::GetContribution(const uint256& hash, CDKGContribution& ret) const { - return curSession && curSession->GetContribution(hash, ret); + const auto session = GetCurSession(); + return session && session->GetContribution(hash, ret); } bool ActiveDKGSessionHandler::GetComplaint(const uint256& hash, CDKGComplaint& ret) const { - return curSession && curSession->GetComplaint(hash, ret); + const auto session = GetCurSession(); + return session && session->GetComplaint(hash, ret); } bool ActiveDKGSessionHandler::GetJustification(const uint256& hash, CDKGJustification& ret) const { - return curSession && curSession->GetJustification(hash, ret); + const auto session = GetCurSession(); + return session && session->GetJustification(hash, ret); } bool ActiveDKGSessionHandler::GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const { - return curSession && curSession->GetPrematureCommitment(hash, ret); + const auto session = GetCurSession(); + return session && session->GetPrematureCommitment(hash, ret); +} + +std::shared_ptr ActiveDKGSessionHandler::GetCurSession() const +{ + return WITH_LOCK(cs_session, return curSession); } QuorumPhase ActiveDKGSessionHandler::GetPhase() const diff --git a/src/active/dkgsessionhandler.h b/src/active/dkgsessionhandler.h index d6684c74c6ba..11764fe17bd1 100644 --- a/src/active/dkgsessionhandler.h +++ b/src/active/dkgsessionhandler.h @@ -61,7 +61,8 @@ class ActiveDKGSessionHandler final : public llmq::CDKGSessionHandler private: std::atomic stopRequested{false}; std::atomic currentHeight{-1}; - std::unique_ptr curSession{nullptr}; + mutable Mutex cs_session; + std::shared_ptr curSession GUARDED_BY(cs_session); mutable Mutex cs_phase_qhash; QuorumPhase phase GUARDED_BY(cs_phase_qhash){QuorumPhase::Idle}; @@ -81,10 +82,12 @@ class ActiveDKGSessionHandler final : public llmq::CDKGSessionHandler public: //! CDKGSessionHandler - bool GetContribution(const uint256& hash, CDKGContribution& ret) const override; - bool GetComplaint(const uint256& hash, CDKGComplaint& ret) const override; - bool GetJustification(const uint256& hash, CDKGJustification& ret) const override; - bool GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const override; + bool GetContribution(const uint256& hash, CDKGContribution& ret) const override EXCLUSIVE_LOCKS_REQUIRED(!cs_session); + bool GetComplaint(const uint256& hash, CDKGComplaint& ret) const override EXCLUSIVE_LOCKS_REQUIRED(!cs_session); + bool GetJustification(const uint256& hash, CDKGJustification& ret) const override + EXCLUSIVE_LOCKS_REQUIRED(!cs_session); + bool GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const override + EXCLUSIVE_LOCKS_REQUIRED(!cs_session); QuorumPhase GetPhase() const override EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); void UpdatedBlockTip(const CBlockIndex* pindexNew) override EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); @@ -96,12 +99,12 @@ class ActiveDKGSessionHandler final : public llmq::CDKGSessionHandler int QuorumIndex() const { return quorumIndex; } bool QuorumsWatch() const { return m_quorums_watch; } uint256 GetCurrentQuorumHash() const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); - CDKGSession* GetCurSession() { return curSession.get(); } + std::shared_ptr GetCurSession() const EXCLUSIVE_LOCKS_REQUIRED(!cs_session); void RequestStop() { stopRequested = true; } bool IsStopRequested() const { return stopRequested; } - bool InitNewQuorum(gsl::not_null pQuorumBaseBlockIndex); + bool InitNewQuorum(gsl::not_null pQuorumBaseBlockIndex) EXCLUSIVE_LOCKS_REQUIRED(!cs_session); /** * @param curPhase current QuorumPhase @@ -114,10 +117,11 @@ class ActiveDKGSessionHandler final : public llmq::CDKGSessionHandler const WhileWaitFunc& shouldNotWait = [] { return false; }) const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); void WaitForNewQuorum(const uint256& oldQuorumHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); void SleepBeforePhase(QuorumPhase curPhase, const uint256& expectedQuorumHash, double randomSleepFactor, - const WhileWaitFunc& runWhileWaiting) const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); + const WhileWaitFunc& runWhileWaiting) const + EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash, !cs_session); void HandlePhase(QuorumPhase curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash, double randomSleepFactor, const StartPhaseFunc& startPhaseFunc, - const WhileWaitFunc& runWhileWaiting) EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); + const WhileWaitFunc& runWhileWaiting) EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash, !cs_session); private: std::pair GetPhaseAndQuorumHash() const EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash); diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 27dde4e95fa6..785054bd3b44 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -678,6 +678,33 @@ void CCoinJoinClientManager::UpdatedSuccessBlock() nCachedLastSuccessBlock = nCachedBlockHeight; } +// NOTE: the record which owns the locks and the persistent locks themselves are +// separate writes and each write is its own implicit transaction, so commit them in +// one explicit database transaction: a crash in between must neither leave +// persistently locked coins behind with nothing tracking them (nothing would ever +// release them again) nor persist the record without its locks (on restart the +// missing lock reads as a manual unlock, the entry is dropped and the input becomes +// selectable again while the finalized mixing transaction may still be in flight). +// If no transaction can be started, don't persist anything rather than risk exactly +// those partial states. The locks of all entries are written, not only of the ones +// just added: entries added while nothing could be persisted are locked in memory only. +// A lock released manually in the meantime is left alone, the next check drops its entry. +static bool PersistPendingObservations(const CWallet& wallet, wallet::WalletBatch& batch, + const std::map& pending) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +{ + if (!batch.TxnBegin()) return false; + bool fPersisted{batch.WriteCoinJoinPendingObs(pending)}; + for (auto it = pending.begin(); fPersisted && it != pending.end(); ++it) { + if (wallet.IsLockedCoin(it->first)) fPersisted = batch.WriteLockedUTXO(it->first); + } + if (!fPersisted) { + batch.TxnAbort(); + return false; + } + return batch.TxnCommit(); +} + void CCoinJoinClientManager::AddPendingObservation(const std::vector& outpoints) { AssertLockNotHeld(cs_pending_obs); @@ -691,42 +718,24 @@ void CCoinJoinClientManager::AddPendingObservation(const std::vector& const int64_t nNow{GetTime()}; for (const auto& outpoint : outpoints) { m_pending_obs.emplace(outpoint, nNow); - } - - // NOTE: the record which owns the locks and the persistent locks themselves are - // separate writes and each write is its own implicit transaction, so commit them in - // one explicit database transaction: a crash in between must neither leave - // persistently locked coins behind with nothing tracking them (nothing would ever - // release them again) nor persist the record without its locks (on restart the - // missing lock reads as a manual unlock, the entry is dropped and the input becomes - // selectable again while the finalized mixing transaction may still be in flight). - // If no transaction can be started, don't persist anything rather than risk exactly - // those partial states. Likewise if the existing record could not be read - // (LoadPendingObservations() left m_pending_obs_loaded unset): overwriting it would - // permanently orphan the locks it still tracks. - const bool fTxn{m_pending_obs_loaded && batch.TxnBegin()}; - bool fPersisted{fTxn && batch.WriteCoinJoinPendingObs(m_pending_obs)}; - for (const auto& outpoint : outpoints) { - // The coins are locked in memory already (see PrepareDenominate), this only - // persists the lock so that a restart before the finalized transaction is - // observed cannot make the input available for selection again. Stop writing - // once anything failed, the transaction is aborted as a whole below. - if (!m_wallet->LockCoin(outpoint, fPersisted ? &batch : nullptr)) fPersisted = false; + // The coins are locked in memory already (see PrepareDenominate), the lock is + // persisted below so that a restart before the finalized transaction is observed + // cannot make the input available for selection again + m_wallet->LockCoin(outpoint); WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::%s -- %s is locked until the finalized mixing transaction is observed\n", __func__, outpoint.ToStringShort()); } - if (fPersisted) { - fPersisted = batch.TxnCommit(); - } else if (fTxn) { - batch.TxnAbort(); - } - if (!fPersisted) { + // Never overwrite a record which could not be read (LoadPendingObservations() left + // m_pending_obs_loaded unset), that would permanently orphan the locks it still tracks + m_pending_obs_dirty = !m_pending_obs_loaded || !PersistPendingObservations(*m_wallet, batch, m_pending_obs); + if (m_pending_obs_dirty) { // The in-memory lock still protects these inputs for as long as this process - // runs, but a restart would make them selectable again while a valid mixing - // transaction spending them may already be in flight. Nothing we can do about - // it here beyond making the failure loud - the wallet database is broken. + // runs, but a restart before CheckPendingObservations() manages to persist them + // would make them selectable again while a valid mixing transaction spending them + // may already be in flight. Nothing more we can do about it here beyond making + // the failure loud - the wallet database is broken. LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to persist locks for %d successfully mixed input(s), " /* Continued */ - "they will not survive a restart\n", + "they will not survive a restart until this succeeds\n", __func__, outpoints.size()); } } @@ -778,11 +787,14 @@ void CCoinJoinClientManager::CheckPendingObservations(const CTxMemPool& mempool) AssertLockNotHeld(cs_pending_obs); // nothing to do in the common case, don't touch the wallet at all - if (WITH_LOCK(cs_pending_obs, return m_pending_obs_loaded && m_pending_obs.empty())) return; + if (WITH_LOCK(cs_pending_obs, return m_pending_obs_loaded && m_pending_obs.empty() && !m_pending_obs_dirty)) return; LOCK(m_wallet->cs_wallet); LOCK(cs_pending_obs); + // Entries which could not be persisted before (e.g. added while the record could not + // be read, see AddPendingObservation()) are locked in memory only, retry persisting them + bool fChanged{m_pending_obs_dirty}; if (!m_pending_obs_loaded) { // Read-only, no need to checkpoint the database on the way out wallet::WalletBatch batch_load(m_wallet->GetDatabase(), /*_fFlushOnClose=*/false); @@ -800,7 +812,6 @@ void CCoinJoinClientManager::CheckPendingObservations(const CTxMemPool& mempool) const int64_t nNow{GetTime()}; const bool fSynced{m_mn_sync.IsBlockchainSynced()}; - bool fChanged{false}; for (auto it = m_pending_obs.begin(); it != m_pending_obs.end();) { const COutPoint& outpoint = it->first; if (!m_wallet->IsLockedCoin(outpoint)) { @@ -862,9 +873,15 @@ void CCoinJoinClientManager::CheckPendingObservations(const CTxMemPool& mempool) // it may still track locks nothing else would ever release. An entry released // above but left in such a record self-heals: once the record is readable again // the entry reloads, its coin is no longer locked and it is dropped right here. - if (fChanged && m_pending_obs_loaded && !get_batch().WriteCoinJoinPendingObs(m_pending_obs)) { - LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to persist %d pending observation(s)\n", __func__, - m_pending_obs.size()); + if (fChanged && m_pending_obs_loaded) { + const bool fPersisted{PersistPendingObservations(*m_wallet, get_batch(), m_pending_obs)}; + // Retried on every pass until it succeeds, only complain when it starts failing + if (!fPersisted && !m_pending_obs_dirty) { + LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to persist %d pending observation(s), " /* Continued */ + "will keep retrying\n", + __func__, m_pending_obs.size()); + } + m_pending_obs_dirty = !fPersisted; } } diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 0bc68d893501..124dbc8aa93b 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -217,6 +217,9 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client //! Whether the failure to read the record has been reported already, the read is //! retried for as long as this node runs and a corrupt record never becomes readable bool m_pending_obs_load_failed GUARDED_BY(cs_pending_obs){false}; + //! Whether m_pending_obs could not be persisted last time (e.g. entries are locked in + //! memory only), every check retries the write until it succeeds + bool m_pending_obs_dirty GUARDED_BY(cs_pending_obs){false}; /// Populate m_pending_obs from the wallet database, once per run void LoadPendingObservations(wallet::WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_pending_obs); diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 6403c6b2a0a6..292e672f0a3f 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -371,7 +371,8 @@ bool CCoinJoinBaseSession::IsValidInOuts(Chainstate& active_chainstate, const ll nFees -= txout.nValue; } - CCoinsViewMemPool viewMemPool(WITH_LOCK(::cs_main, return &active_chainstate.CoinsTip()), mempool); + LOCK(::cs_main); + CCoinsViewMemPool viewMemPool(&active_chainstate.CoinsTip(), mempool); for (const auto& txin : vin) { LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- txin=%s\n", __func__, txin.ToString()); diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 8146bb269542..b06652025431 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -42,7 +42,6 @@ CCoinJoinServer::CCoinJoinServer(PeerManagerInternal* peer_manager, ChainstateMa m_mn_activeman{mn_activeman}, m_mn_sync{mn_sync}, m_isman{isman}, - vecSessionCollaterals{}, fUnitTest{false} { } @@ -87,7 +86,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) return; } - if (WITH_LOCK(cs_coinjoin, return vecSessionCollaterals.empty())) { + if (WITH_LOCK(cs_coinjoin, return m_session_collaterals.empty())) { { const auto hasQueue = m_queueman.TryHasQueueFromMasternode(m_mn_activeman.GetOutPoint()); if (!hasQueue.has_value()) return; @@ -233,14 +232,14 @@ void CCoinJoinServer::ProcessDSSIGNFINALTX(CNode& peer, CDataStream& vRecv) // Only accept signatures while we are actually collecting them, and only // from peers that are active participants in this session. Otherwise a // stray or unauthenticated peer could abort the session for everyone. - if (nState != POOL_STATE_SIGNING) { - LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- wrong state, nState=%d, peer=%d\n", - nState.load(), peer.GetId()); - PushStatus(peer, STATUS_REJECTED, ERR_SESSION); - return; - } + int session_id{0}; { LOCK(cs_coinjoin); + if (nState != POOL_STATE_SIGNING) { + LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- wrong state, nState=%d, peer=%d\n", nState.load(), peer.GetId()); + PushStatus(peer, STATUS_REJECTED, ERR_SESSION); + return; + } const bool is_participant = std::ranges::any_of( vecEntries, [&peer](const auto& entry) { return entry.addr == peer.addr; }); if (!is_participant) { @@ -249,6 +248,7 @@ void CCoinJoinServer::ProcessDSSIGNFINALTX(CNode& peer, CDataStream& vRecv) PushStatus(peer, STATUS_REJECTED, ERR_INVALID_INPUT); return; } + session_id = nSessionID; } const size_t max_txins{CoinJoin::GetMaxPoolInputOutputCount()}; @@ -265,18 +265,30 @@ void CCoinJoinServer::ProcessDSSIGNFINALTX(CNode& peer, CDataStream& vRecv) LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- vecTxIn.size() %s\n", vecTxIn.size()); - int nTxInIndex = 0; - int nTxInsCount = static_cast(vecTxIn.size()); - - for (const auto& txin : vecTxIn) { - nTxInIndex++; - if (!AddScriptSig(txin)) { - LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- AddScriptSig() failed at %d/%d, session: %d\n", nTxInIndex, nTxInsCount, nSessionID); - LOCK(cs_coinjoin); - RelayStatus(STATUS_REJECTED); + { + LOCK(cs_coinjoin); + // Apply the whole message in one critical section, against the signing session it was + // checked for above. The lock was released while the body was decoded, and releasing it + // between inputs would let a timeout reset the session halfway through one sender's + // already-received signatures, after charging that sender as a non-signer. + if (nSessionID != session_id || nState != POOL_STATE_SIGNING) { + LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- session %d is no longer signing, peer=%d\n", session_id, + peer.GetId()); + PushStatus(peer, STATUS_REJECTED, ERR_SESSION); return; } - LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- AddScriptSig() %d/%d success\n", nTxInIndex, nTxInsCount); + int nTxInIndex = 0; + int nTxInsCount = static_cast(vecTxIn.size()); + for (const auto& txin : vecTxIn) { + nTxInIndex++; + if (!AddScriptSig(txin)) { + LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- AddScriptSig() failed at %d/%d, session: %d\n", nTxInIndex, + nTxInsCount, session_id); + RelayStatus(STATUS_REJECTED); + return; + } + LogPrint(BCLog::COINJOIN, "DSSIGNFINALTX -- AddScriptSig() %d/%d success\n", nTxInIndex, nTxInsCount); + } } // all is good CheckPool(); @@ -286,8 +298,7 @@ void CCoinJoinServer::SetNull() { AssertLockHeld(cs_coinjoin); // MN side - vecSessionCollaterals.clear(); - setSessionCollateralPrevouts.clear(); + m_session_collaterals.Clear(); m_fRebalanceSession = false; m_fHasLegacyParticipant = false; m_mapDeclaredShapes.clear(); @@ -301,12 +312,20 @@ void CCoinJoinServer::SetNull() // void CCoinJoinServer::CheckPool() { + AssertLockNotHeld(cs_coinjoin); + + // Both the scheduler thread and the message-handling thread get here. Skip the round if the + // other one is already in it rather than blocking msghand behind its mempool work: whichever + // thread holds the lock is performing the same check we would. + TRY_LOCK(cs_check_pool, lock_check_pool); + if (!lock_check_pool) return; + // Every decision below reads several pieces of session state at once, so take them as one // snapshot. Sampling them separately let the message-handling thread commit an entry // between two reads: the side counts could be read before the last entry arrived while the // entry count was read after it, so a session that was about to finalize looked like one // whose entries were all in but left a side uncovered - and got reset. Reading - // vecSessionCollaterals without the lock could also race SetNull() clearing it. + // the session collaterals without the lock could also race SetNull() clearing them. const auto snap = GetPoolSnapshot(); if (snap.entries != 0) @@ -320,7 +339,7 @@ void CCoinJoinServer::CheckPool() if (snap.state == POOL_STATE_ACCEPTING_ENTRIES && snap.entries == snap.collaterals) { if (snap.sides.IsCovered()) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- FINALIZE TRANSACTIONS\n"); - CreateFinalTransaction(snap.session_id); + CreateFinalTransaction(snap.session_id, /*charge_fees=*/false); return; } // The participant set is frozen once entries are being accepted, so this session can @@ -328,13 +347,18 @@ void CCoinJoinServer::CheckPool() // timeout. Shouldn't happen: admission checks the declared shapes up front. LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- all entries received but a session denom side is uncovered (%d in, %d out), resetting session\n", snap.sides.inputs, snap.sides.outputs); - // Tell the participants before dropping them, so they release their inputs and collateral - // right away instead of waiting out their own timeout. Must precede SetNull(), which - // clears the entries this iterates. - RelayCompletedTransaction(ERR_SESSION); // Only drop the session the snapshot described; the message-handling thread may have - // reset and restarted one while we were relaying. - WITH_LOCK(cs_coinjoin, if (IsCurrentSession(snap.session_id)) SetNull()); + // reset and restarted one since. Capture its participants in the same step, then tell + // them, so they release their inputs and collateral right away instead of waiting out + // their own timeout. + std::vector participants; + { + LOCK(cs_coinjoin); + if (!IsCurrentSession(snap.session_id)) return; + participants = GetParticipantAddrs(); + SetNull(); + } + RelayCompletedTransaction(snap.session_id, participants, ERR_SESSION); return; } @@ -342,17 +366,15 @@ void CCoinJoinServer::CheckPool() // If we timed out while accepting entries, then if we have more than minimum, create final tx if (snap.state == POOL_STATE_ACCEPTING_ENTRIES && CCoinJoinServer::HasTimedOut() && snap.sides.IsCovered() && snap.entries >= static_cast(CoinJoin::GetMinPoolParticipants())) { - // Punish misbehaving participants - ChargeFees(); - // Try to complete this session ignoring the misbehaving ones - CreateFinalTransaction(snap.session_id); + // Punish misbehaving participants and try to complete this session ignoring them + CreateFinalTransaction(snap.session_id, /*charge_fees=*/true); return; } // If we have all the signatures, try to compile the transaction - if (snap.state == POOL_STATE_SIGNING && IsSignaturesComplete()) { + if (snap.state == POOL_STATE_SIGNING && snap.signatures_complete) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- SIGNING\n"); - CommitFinalTransaction(); + CommitFinalTransaction(snap.session_id); return; } } @@ -361,65 +383,93 @@ CCoinJoinServer::PoolSnapshot CCoinJoinServer::GetPoolSnapshot() const { AssertLockNotHeld(cs_coinjoin); LOCK(cs_coinjoin); - return PoolSnapshot{nSessionID, nState, vecEntries.size(), vecSessionCollaterals.size(), - GetMixSideCountsLocked()}; + return PoolSnapshot{nSessionID, nState, vecEntries.size(), m_session_collaterals.size(), + GetMixSideCountsLocked(), IsSignaturesComplete()}; } -void CCoinJoinServer::CreateFinalTransaction(int session_id) +void CCoinJoinServer::CreateFinalTransaction(int session_id, bool charge_fees) { AssertLockNotHeld(cs_coinjoin); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- FINALIZE TRANSACTIONS\n"); - LOCK(cs_coinjoin); - - // The decision to finalize came from a snapshot taken before this lock, so make sure it - // still describes an eligible live session. An entry can finish validation and commit while - // the timeout path charges fees, changing a covered side from empty to a lone participant. - // Check the live entries under the same lock used to build the transaction so that entry - // admission cannot invalidate the decision before the state moves to signing. - if (!IsCurrentSession(session_id)) { - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- session changed, not finalizing\n"); - return; - } - const auto sides = GetMixSideCountsLocked(); - if (vecEntries.size() < static_cast(CoinJoin::GetMinPoolParticipants()) || !sides.IsCovered()) { - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- session no longer eligible, entries=%d, sides=%d/%d\n", - vecEntries.size(), sides.inputs, sides.outputs); - // Deliberately no timer refresh: on the timeout path the session stays timed out and - // the scheduler's next CheckTimeout() resets it. The missing side already had the full - // entry window, so waiting another one would only keep everyone else's coins locked. - return; - } + CTransactionRef collateral_to_charge; + { + LOCK(cs_coinjoin); - CMutableTransaction txNew; + // The decision to finalize came from a snapshot taken before this lock, so make sure it + // still describes an eligible live session. An entry can finish validation and commit in + // between, changing a covered side from empty to a lone participant. Check the live + // entries under the same lock used to build the transaction so that entry admission + // cannot invalidate the decision before the state moves to signing. + if (!IsCurrentSession(session_id)) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- session changed, not finalizing\n"); + return; + } + const auto sides = GetMixSideCountsLocked(); + if (vecEntries.size() < static_cast(CoinJoin::GetMinPoolParticipants()) || !sides.IsCovered()) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- session no longer eligible, entries=%d, sides=%d/%d\n", + vecEntries.size(), sides.inputs, sides.outputs); + // Deliberately no timer refresh: on the timeout path the session stays timed out and + // the scheduler's next CheckTimeout() resets it. The missing side already had the full + // entry window, so waiting another one would only keep everyone else's coins locked. + return; + } - // make our new transaction - for (const auto& entry : vecEntries) { - for (const auto& txout : entry.vecTxOut) { - txNew.vout.push_back(txout); + // Selecting the offenders and moving to signing below happen under one lock, so a + // participant whose entry commits before the cutoff can no longer be charged as missing. + if (charge_fees) { + collateral_to_charge = SelectCollateralToCharge(); } - for (const auto& txdsin : entry.vecTxDSIn) { - txNew.vin.push_back(txdsin); + + CMutableTransaction txNew; + + // make our new transaction + for (const auto& entry : vecEntries) { + for (const auto& txout : entry.vecTxOut) { + txNew.vout.push_back(txout); + } + for (const auto& txdsin : entry.vecTxDSIn) { + txNew.vin.push_back(txdsin); + } } - } - sort(txNew.vin.begin(), txNew.vin.end(), CompareInputBIP69()); - sort(txNew.vout.begin(), txNew.vout.end(), CompareOutputBIP69()); + sort(txNew.vin.begin(), txNew.vin.end(), CompareInputBIP69()); + sort(txNew.vout.begin(), txNew.vout.end(), CompareOutputBIP69()); - finalMutableTransaction = txNew; - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- finalMutableTransaction=%s", /* Continued */ - txNew.ToString()); + finalMutableTransaction = txNew; + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- finalMutableTransaction=%s", /* Continued */ + txNew.ToString()); - // request signatures from clients - SetState(POOL_STATE_SIGNING); - RelayFinalTransaction(CTransaction(finalMutableTransaction)); + // request signatures from clients + SetState(POOL_STATE_SIGNING); + RelayFinalTransaction(CTransaction(finalMutableTransaction)); + } + + if (collateral_to_charge) { + ConsumeReservedCollateral(collateral_to_charge); + } } -void CCoinJoinServer::CommitFinalTransaction() +void CCoinJoinServer::CommitFinalTransaction(int session_id) { AssertLockNotHeld(cs_coinjoin); - CTransactionRef finalTransaction = WITH_LOCK(cs_coinjoin, return MakeTransactionRef(finalMutableTransaction)); + CTransactionRef finalTransaction; + std::vector participants; + std::vector collaterals; + { + LOCK(cs_coinjoin); + // Committing a session that is already gone would push a cleared finalMutableTransaction + // through ATMP and notify the participants of a failure that never happened. + if (nSessionID != session_id || nState != POOL_STATE_SIGNING) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- session %d is gone, not committing\n", + session_id); + return; + } + finalTransaction = MakeTransactionRef(finalMutableTransaction); + participants = GetParticipantAddrs(); + collaterals = m_session_collaterals.txs(); + } uint256 hashTx = finalTransaction->GetHash(); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- finalTransaction=%s", /* Continued */ @@ -432,9 +482,9 @@ void CCoinJoinServer::CommitFinalTransaction() if (!lockMain || !ATMPIfSaneFee(m_chainman, finalTransaction)) { LogPrint(BCLog::COINJOIN, /* Continued */ "CCoinJoinServer::CommitFinalTransaction -- ATMPIfSaneFee() error: Transaction not valid\n"); - WITH_LOCK(cs_coinjoin, SetNull()); // not much we can do in this case, just notify clients - RelayCompletedTransaction(ERR_INVALID_TX); + RelayCompletedTransaction(session_id, participants, ERR_INVALID_TX); + ResetSigningSessionIfCurrent(session_id); return; } } @@ -455,14 +505,14 @@ void CCoinJoinServer::CommitFinalTransaction() m_peer_manager->PeerRelayInv(inv); // Tell the clients it was successful - RelayCompletedTransaction(MSG_SUCCESS); + RelayCompletedTransaction(session_id, participants, MSG_SUCCESS); // Randomly charge clients - ChargeRandomFees(); + ChargeRandomFees(collaterals); // Reset LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- COMPLETED -- RESETTING\n"); - WITH_LOCK(cs_coinjoin, SetNull()); + ResetSigningSessionIfCurrent(session_id); } // @@ -477,18 +527,25 @@ void CCoinJoinServer::CommitFinalTransaction() // transaction for the client to be able to enter the pool. This transaction is kept by the Masternode // until the transaction is either complete or fails. // -void CCoinJoinServer::ChargeFees() const +/* + * Select one offender while cs_coinjoin still binds the state, entries and collaterals to the + * same session. The caller must close the relevant admission path before releasing the lock and + * consuming the returned collateral, so a late entry or signature cannot make the snapshot stale. + * The returned collateral stays reserved until ConsumeReservedCollateral() has run. + */ +CTransactionRef CCoinJoinServer::SelectCollateralToCharge() { - AssertLockNotHeld(cs_coinjoin); + AssertLockHeld(cs_coinjoin); //we don't need to charge collateral for every offence. - if (GetRand(/*nMax=*/100) > 33) return; + if (GetRand(/*nMax=*/100) > 33) return {}; std::vector vecOffendersCollaterals; + const PoolState state{nState}; + const size_t nSessionCollaterals{m_session_collaterals.size()}; - if (nState == POOL_STATE_ACCEPTING_ENTRIES) { - LOCK(cs_coinjoin); - for (const auto& txCollateral : vecSessionCollaterals) { + if (state == POOL_STATE_ACCEPTING_ENTRIES) { + for (const auto& txCollateral : m_session_collaterals.txs()) { bool fFound = std::ranges::any_of(vecEntries, [&txCollateral](const auto& entry) { return *entry.txCollateral == *txCollateral; }); @@ -496,21 +553,19 @@ void CCoinJoinServer::ChargeFees() const // This queue entry didn't send us the promised transaction if (!fFound) { LogPrint(BCLog::COINJOIN, /* Continued */ - "CCoinJoinServer::ChargeFees -- found uncooperative node (didn't send transaction), found " - "offence\n"); + "CCoinJoinServer::SelectCollateralToCharge -- found uncooperative node (didn't send " + "transaction), found offence\n"); vecOffendersCollaterals.push_back(txCollateral); } } - } - - if (nState == POOL_STATE_SIGNING) { + } else if (state == POOL_STATE_SIGNING) { // who didn't sign? - LOCK(cs_coinjoin); for (const auto& entry : vecEntries) { for (const auto& txdsin : entry.vecTxDSIn) { if (!txdsin.fHasSig) { LogPrint(BCLog::COINJOIN, /* Continued */ - "CCoinJoinServer::ChargeFees -- found uncooperative node (didn't sign), found offence\n"); + "CCoinJoinServer::SelectCollateralToCharge -- found uncooperative node (didn't sign), " + "found offence\n"); vecOffendersCollaterals.push_back(entry.txCollateral); } } @@ -518,23 +573,23 @@ void CCoinJoinServer::ChargeFees() const } // no offences found - if (vecOffendersCollaterals.empty()) return; + if (vecOffendersCollaterals.empty()) return {}; //mostly offending? Charge sometimes - if (vecOffendersCollaterals.size() >= vecSessionCollaterals.size() - 1 && GetRand(/*nMax=*/100) > 33) return; + if (vecOffendersCollaterals.size() >= nSessionCollaterals - 1 && GetRand(/*nMax=*/100) > 33) return {}; //everyone is an offender? That's not right - if (vecOffendersCollaterals.size() >= vecSessionCollaterals.size()) return; + if (vecOffendersCollaterals.size() >= nSessionCollaterals) return {}; //charge one of the offenders randomly Shuffle(vecOffendersCollaterals.begin(), vecOffendersCollaterals.end(), FastRandomContext()); - if (nState == POOL_STATE_ACCEPTING_ENTRIES || nState == POOL_STATE_SIGNING) { - LogPrint(BCLog::COINJOIN, /* Continued */ - "CCoinJoinServer::ChargeFees -- found uncooperative node (didn't %s transaction), charging fees: %s", - (nState == POOL_STATE_SIGNING) ? "sign" : "send", vecOffendersCollaterals[0]->ToString()); - ConsumeCollateral(vecOffendersCollaterals[0]); - } + LogPrint(BCLog::COINJOIN, /* Continued */ + "CCoinJoinServer::SelectCollateralToCharge -- found uncooperative node (didn't %s transaction), charging " + "fees: %s", + (state == POOL_STATE_SIGNING) ? "sign" : "send", vecOffendersCollaterals[0]->ToString()); + m_collaterals_being_charged.push_back(vecOffendersCollaterals[0]); + return vecOffendersCollaterals[0]; } /* @@ -549,17 +604,11 @@ void CCoinJoinServer::ChargeFees() const stop these kinds of attacks 1 in 10 successful transactions are charged. This adds up to a cost of 0.001DRK per transaction on average. */ -void CCoinJoinServer::ChargeRandomFees() const +void CCoinJoinServer::ChargeRandomFees(const std::vector& collaterals) const { AssertLockNotHeld(cs_coinjoin); - std::vector session_collaterals; - { - LOCK(cs_coinjoin); - session_collaterals = vecSessionCollaterals; - } - - for (const auto& txCollateral : session_collaterals) { + for (const auto& txCollateral : collaterals) { if (GetRand(/*nMax=*/100) > 10) return; LogPrint(BCLog::COINJOIN, /* Continued */ "CCoinJoinServer::ChargeRandomFees -- charging random fees, txCollateral=%s", txCollateral->ToString()); @@ -578,7 +627,29 @@ void CCoinJoinServer::ConsumeCollateral(const CTransactionRef& txref) const } } -void CCoinJoinServer::ConsumeCollateralIfCurrentSession(int session_id, const CTransactionRef& txref) const +void CCoinJoinServer::ConsumeReservedCollateral(const CTransactionRef& txref) +{ + AssertLockNotHeld(cs_coinjoin); + ConsumeCollateral(txref); + + // From here on the mempool, not the reservation, keeps a consumed collateral out of new sessions. + LOCK(cs_coinjoin); + const auto it = std::ranges::find(m_collaterals_being_charged, txref); + if (Assume(it != m_collaterals_being_charged.end())) m_collaterals_being_charged.erase(it); +} + +bool CCoinJoinServer::SpendsCollateralBeingCharged(const CMutableTransaction& txCollateral) const +{ + AssertLockHeld(cs_coinjoin); + return std::ranges::any_of(m_collaterals_being_charged, [&txCollateral](const CTransactionRef& charged) { + return std::ranges::any_of(charged->vin, [&txCollateral](const CTxIn& charged_in) { + return std::ranges::any_of(txCollateral.vin, + [&charged_in](const CTxIn& txin) { return txin.prevout == charged_in.prevout; }); + }); + }); +} + +void CCoinJoinServer::ConsumeCollateralIfCurrentSession(int session_id, const CTransactionRef& txref) { AssertLockNotHeld(cs_coinjoin); @@ -586,14 +657,18 @@ void CCoinJoinServer::ConsumeCollateralIfCurrentSession(int session_id, const CT // we get here the scheduler thread may have timed the session out and started another one. // Charging then would spend the collateral of a participant that is no longer in any // session - it may even be a replay of an innocent participant's collateral. - const bool fStillCurrent = - WITH_LOCK(cs_coinjoin, return IsCurrentSession(session_id) && HasSessionCollateral(txref)); + bool fStillCurrent{false}; + { + LOCK(cs_coinjoin); + fStillCurrent = IsCurrentSession(session_id) && HasSessionCollateral(txref); + if (fStillCurrent) m_collaterals_being_charged.push_back(txref); + } if (!fStillCurrent) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- session changed, not consuming collateral %s\n", __func__, txref->GetHash().ToString()); return; } - ConsumeCollateral(txref); + ConsumeReservedCollateral(txref); } bool CCoinJoinServer::IsCurrentSession(int session_id) const @@ -605,7 +680,7 @@ bool CCoinJoinServer::IsCurrentSession(int session_id) const bool CCoinJoinServer::HasSessionCollateral(const CTransactionRef& txref) const { AssertLockHeld(cs_coinjoin); - return std::ranges::any_of(vecSessionCollaterals, + return std::ranges::any_of(m_session_collaterals.txs(), [&txref](const CTransactionRef& ref) { return *ref == *txref; }); } @@ -618,6 +693,19 @@ bool CCoinJoinServer::HasTimedOut() const return GetTime() - nTimeLastSuccessfulStep >= nTimeout; } +bool ShouldResetTimedOutSession(const CoinJoinSessionProgress& session) +{ + if (!session.timed_out) return false; + + const bool can_advance{(session.state == POOL_STATE_QUEUE && session.queue_ready) || + (session.state == POOL_STATE_ACCEPTING_ENTRIES && + (session.entries == session.collaterals || + (session.entries >= static_cast(CoinJoin::GetMinPoolParticipants()) && + session.sides.IsCovered()))) || + (session.state == POOL_STATE_SIGNING && session.signatures_complete)}; + return !can_advance; +} + // // Check for extraneous timeout // @@ -625,13 +713,41 @@ void CCoinJoinServer::CheckTimeout() { m_queueman.CheckQueue(); - // Too early to do anything - if (!CCoinJoinServer::HasTimedOut()) return; + // CheckPool can be finalizing or committing on the message-handling thread. Skipping this tick + // keeps timeout reset and finalization/commit single-flight without blocking the scheduler. + TRY_LOCK(cs_check_pool, lock_check_pool); + if (!lock_check_pool) return; + + CTransactionRef collateral_to_charge; + { + LOCK(cs_coinjoin); + + // Decide while holding the lock so selecting an offender and closing the session form one + // atomic cutoff for late entries and signatures. + // CheckForCompleteQueue() and CheckPool() run before this method on the scheduler thread, + // but a final collateral, entry, or signature can arrive after their snapshots. The + // message-handling thread then skips its own CheckPool() if this scheduler tick still holds + // cs_check_pool. A session that can now advance takes priority over resetting it; the next + // scheduler tick will perform the transition, finalization, or commit. + if (!ShouldResetTimedOutSession({.state = nState, + .timed_out = CCoinJoinServer::HasTimedOut(), + .queue_ready = IsSessionReady(), + .entries = vecEntries.size(), + .collaterals = m_session_collaterals.size(), + .sides = GetMixSideCountsLocked(), + .signatures_complete = IsSignaturesComplete()})) { + return; + } - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckTimeout -- %s timed out -- resetting\n", - (nState == POOL_STATE_SIGNING) ? "Signing" : "Session"); - ChargeFees(); - WITH_LOCK(cs_coinjoin, SetNull()); + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckTimeout -- %s timed out -- resetting\n", + (nState == POOL_STATE_SIGNING) ? "Signing" : "Session"); + collateral_to_charge = SelectCollateralToCharge(); + SetNull(); + } + + if (collateral_to_charge) { + ConsumeReservedCollateral(collateral_to_charge); + } } /* @@ -641,6 +757,8 @@ void CCoinJoinServer::CheckTimeout() */ void CCoinJoinServer::CheckForCompleteQueue() { + AssertLockNotHeld(cs_coinjoin); + int session_denom; size_t participants; { @@ -649,7 +767,7 @@ void CCoinJoinServer::CheckForCompleteQueue() SetState(POOL_STATE_ACCEPTING_ENTRIES); session_denom = nSessionDenom; - participants = vecSessionCollaterals.size(); + participants = m_session_collaterals.size(); } CCoinJoinQueue dsq(session_denom, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(), GetAdjustedTime(), true); @@ -733,7 +851,7 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag return false; } - if (static_cast(GetEntriesCountLocked()) >= vecSessionCollaterals.size()) { + if (static_cast(GetEntriesCountLocked()) >= m_session_collaterals.size()) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: entries is full!\n", __func__); nMessageIDRet = ERR_ENTRIES_FULL; return false; @@ -872,10 +990,9 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag bool CCoinJoinServer::AddScriptSig(const CTxIn& txinNew) { - AssertLockNotHeld(cs_coinjoin); + AssertLockHeld(cs_coinjoin); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- scriptSig=%s\n", ScriptToAsmStr(txinNew.scriptSig).substr(0, 24)); - LOCK(cs_coinjoin); for (const auto& entry : vecEntries) { if (std::ranges::any_of(entry.vecTxDSIn, [&txinNew](const auto& txdsin) { return txdsin.scriptSig == txinNew.scriptSig; })) { @@ -911,8 +1028,7 @@ bool CCoinJoinServer::AddScriptSig(const CTxIn& txinNew) // Check to make sure everything is signed bool CCoinJoinServer::IsSignaturesComplete() const { - AssertLockNotHeld(cs_coinjoin); - LOCK(cs_coinjoin); + AssertLockHeld(cs_coinjoin); return std::ranges::all_of(vecEntries, [](const auto& entry) { return std::ranges::all_of(entry.vecTxDSIn, [](const auto& txdsin) { return txdsin.fHasSig; }); @@ -938,15 +1054,6 @@ bool CCoinJoinServer::IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& n return true; } -void CCoinJoinServer::CommitSessionCollateral(const CMutableTransaction& txCollateral) -{ - AssertLockHeld(cs_coinjoin); - vecSessionCollaterals.push_back(MakeTransactionRef(txCollateral)); - for (const auto& txin : txCollateral.vin) { - setSessionCollateralPrevouts.insert(txin.prevout); - } -} - //! Which side of the session denomination a participant will occupy, from its declared dsa //! direction. A participant that declares nothing is mixing 1:1 and occupies both sides. static CoinJoin::MixShape DeclaredShape(const CCoinJoinAccept& dsa) @@ -998,6 +1105,8 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, int nPeerVers return false; } + int nDenom{0}; + size_t nParticipants{0}; { LOCK(cs_coinjoin); @@ -1008,6 +1117,11 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, int nPeerVers nMessageIDRet = ERR_MODE; return false; } + if (SpendsCollateralBeingCharged(dsa.txCollateral)) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- collateral is being charged\n"); + nMessageIDRet = ERR_INVALID_COLLATERAL; + return false; + } // start new session nMessageIDRet = MSG_NOERR; @@ -1019,21 +1133,25 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, int nPeerVers SetState(POOL_STATE_QUEUE); - CommitSessionCollateral(dsa.txCollateral); + m_session_collaterals.Add(dsa.txCollateral); + nDenom = nSessionDenom; + nParticipants = m_session_collaterals.size(); } if (!fUnitTest) { //broadcast that I'm accepting entries, only if it's the first entry through - CCoinJoinQueue dsq(nSessionDenom, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(), - GetAdjustedTime(), false); + CCoinJoinQueue dsq(nDenom, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(), GetAdjustedTime(), false); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString()); dsq.vchSig = m_mn_activeman.SignBasic(dsq.GetSignatureHash()); m_peer_manager->PeerRelayDSQ(dsq); m_queueman.AddQueue(std::move(dsq)); } - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- new session created, nSessionID: %d nSessionDenom: %d (%s) vecSessionCollaterals.size(): %d CoinJoin::GetMaxPoolParticipants(): %d\n", - nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), vecSessionCollaterals.size(), CoinJoin::GetMaxPoolParticipants()); + LogPrint(BCLog::COINJOIN, /* Continued */ + "CCoinJoinServer::CreateNewSession -- new session created, nSessionID: %d nSessionDenom: %d (%s) " + "participants: %d CoinJoin::GetMaxPoolParticipants(): %d\n", + nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), nParticipants, + CoinJoin::GetMaxPoolParticipants()); return true; } @@ -1120,22 +1238,25 @@ bool CCoinJoinServer::AddUserToExistingSession(const CCoinJoinAccept& dsa, int n // IsSessionReady() can now hold a full session back waiting for a missing counterparty, so // the participant limit has to be enforced here rather than implied by session readiness - if (static_cast(vecSessionCollaterals.size()) >= CoinJoin::GetMaxPoolParticipants()) { + if (static_cast(m_session_collaterals.size()) >= CoinJoin::GetMaxPoolParticipants()) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddUserToExistingSession -- session is full\n"); nMessageIDRet = ERR_QUEUE_FULL; return false; } - // Session collaterals are only ever test-accepted, never added to the mempool, so nothing - // pins their identity: the same UTXO can be re-signed into arbitrarily many distinct txids. - // Match on input prevouts so a resent or replayed dsa cannot be counted as a new participant. - for (const auto& txin : dsa.txCollateral.vin) { - if (setSessionCollateralPrevouts.contains(txin.prevout)) { - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddUserToExistingSession -- collateral %s spends prevout %s already committed to this session\n", - dsa.txCollateral.GetHash().ToString(), txin.prevout.ToStringShort()); - nMessageIDRet = ERR_ALREADY_HAVE; - return false; - } + // A resent or replayed dsa must not be counted as a new participant; see SessionCollaterals. + if (const auto prevout = m_session_collaterals.FindCommittedPrevout(dsa.txCollateral)) { + LogPrint(BCLog::COINJOIN, /* Continued */ + "CCoinJoinServer::AddUserToExistingSession -- collateral %s spends prevout %s already committed to " + "this session\n", + dsa.txCollateral.GetHash().ToString(), prevout->ToStringShort()); + nMessageIDRet = ERR_ALREADY_HAVE; + return false; + } + if (SpendsCollateralBeingCharged(dsa.txCollateral)) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddUserToExistingSession -- collateral is being charged\n"); + nMessageIDRet = ERR_INVALID_COLLATERAL; + return false; } // count new user as accepted to an existing session @@ -1146,10 +1267,13 @@ bool CCoinJoinServer::AddUserToExistingSession(const CCoinJoinAccept& dsa, int n m_fRebalanceSession |= dsa.IsRebalance(); m_fHasLegacyParticipant |= nPeerVersion < COINJOIN_REBALANCE_VERSION; m_mapDeclaredShapes.emplace(dsa.txCollateral.GetHash(), DeclaredShape(dsa)); - CommitSessionCollateral(dsa.txCollateral); + m_session_collaterals.Add(dsa.txCollateral); - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddUserToExistingSession -- new user accepted, nSessionID: %d nSessionDenom: %d (%s) vecSessionCollaterals.size(): %d CoinJoin::GetMaxPoolParticipants(): %d\n", - nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), vecSessionCollaterals.size(), CoinJoin::GetMaxPoolParticipants()); + LogPrint(BCLog::COINJOIN, /* Continued */ + "CCoinJoinServer::AddUserToExistingSession -- new user accepted, nSessionID: %d nSessionDenom: %d (%s) " + "participants: %d CoinJoin::GetMaxPoolParticipants(): %d\n", + nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), m_session_collaterals.size(), + CoinJoin::GetMaxPoolParticipants()); return true; } @@ -1157,15 +1281,17 @@ bool CCoinJoinServer::AddUserToExistingSession(const CCoinJoinAccept& dsa, int n // Returns true if either max size has been reached or if the mix timed out and min size was reached bool CCoinJoinServer::IsSessionReady() const { + AssertLockHeld(cs_coinjoin); + if (nState == POOL_STATE_QUEUE) { // PRIVACY: don't start mixing until each side of the session denomination is occupied // by nobody or by at least two participants. A session that never attracts the missing // counterparty simply expires in queue state, where no collateral is charged. if (!GetDeclaredSideCounts().IsCovered()) return false; - if (static_cast(vecSessionCollaterals.size()) >= CoinJoin::GetMaxPoolParticipants()) { + if (static_cast(m_session_collaterals.size()) >= CoinJoin::GetMaxPoolParticipants()) { return true; } - if (CCoinJoinServer::HasTimedOut() && static_cast(vecSessionCollaterals.size()) >= CoinJoin::GetMinPoolParticipants()) { + if (CCoinJoinServer::HasTimedOut() && static_cast(m_session_collaterals.size()) >= CoinJoin::GetMinPoolParticipants()) { return true; } } @@ -1239,30 +1365,49 @@ void CCoinJoinServer::RelayStatus(PoolStatusUpdate nStatusUpdate, PoolMessage nM } } -void CCoinJoinServer::RelayCompletedTransaction(PoolMessage nMessageID) +void CCoinJoinServer::RelayCompletedTransaction(int session_id, const std::vector& participants, + PoolMessage nMessageID) { AssertLockNotHeld(cs_coinjoin); - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- nSessionID: %d nSessionDenom: %d (%s)\n", - __func__, nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom)); + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- nSessionID: %d\n", __func__, session_id); - // final mixing tx with empty signatures should be relayed to mixing participants only - LOCK(cs_coinjoin); - for (const auto& entry : vecEntries) { - bool fOk = connman.ForNode(entry.addr, [&nMessageID, this](CNode* pnode) { + for (const auto& addr : participants) { + const bool fOk = connman.ForNode(addr, [&nMessageID, session_id, this](CNode* pnode) { CNetMsgMaker msgMaker(pnode->GetCommonVersion()); - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::DSCOMPLETE, nSessionID.load(), nMessageID)); + connman.PushMessage(pnode, msgMaker.Make(NetMsgType::DSCOMPLETE, session_id, nMessageID)); return true; }); if (!fOk) { - // no such node? maybe client disconnected or our own connection went down - RelayStatus(STATUS_REJECTED); - break; + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- participant disconnected before completion\n", __func__); } } } +std::vector CCoinJoinServer::GetParticipantAddrs() const +{ + AssertLockHeld(cs_coinjoin); + std::vector participants; + participants.reserve(vecEntries.size()); + std::ranges::transform(vecEntries, std::back_inserter(participants), [](const auto& entry) { return entry.addr; }); + return participants; +} + +void CCoinJoinServer::ResetSigningSessionIfCurrent(int session_id) +{ + AssertLockNotHeld(cs_coinjoin); + LOCK(cs_coinjoin); + if (nSessionID != session_id || nState != POOL_STATE_SIGNING) { + LogPrint(BCLog::COINJOIN, /* Continued */ + "CCoinJoinServer::%s -- signing session %d is no longer current, not resetting\n", __func__, session_id); + return; + } + SetNull(); +} + void CCoinJoinServer::SetState(PoolState nStateNew) { + AssertLockHeld(cs_coinjoin); + if (nStateNew == POOL_STATE_ERROR) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::SetState -- Can't set state to ERROR as a Masternode. \n"); return; diff --git a/src/coinjoin/server.h b/src/coinjoin/server.h index 475808c3052b..d5abb3d218b7 100644 --- a/src/coinjoin/server.h +++ b/src/coinjoin/server.h @@ -13,6 +13,7 @@ #include #include +#include #include class CActiveMasternodeManager; @@ -27,6 +28,27 @@ class CTxMemPool; class UniValue; +/** What the server's timeout check reads from a mixing session to decide whether to reset it */ +struct CoinJoinSessionProgress { + PoolState state{POOL_STATE_IDLE}; + bool timed_out{false}; + //! The queue has enough collaterals covering both sides to start accepting entries + bool queue_ready{false}; + size_t entries{0}; + size_t collaterals{0}; + //! The sides of the session denomination occupied by the submitted entries + CoinJoin::MixSideCounts sides{}; + bool signatures_complete{false}; +}; + +/** + * Should a session be reset for timing out? Not while it can still make the transition that + * CheckForCompleteQueue() or CheckPool() would make for it: start accepting entries, finalize + * (or, with an entry for every collateral but a side uncovered, reset with the participants + * told), or commit. + */ +bool ShouldResetTimedOutSession(const CoinJoinSessionProgress& session); + /** Used to keep track of current status of mixing pool */ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler @@ -48,12 +70,47 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler // Session state and entry admission live in the protected section so unit tests can seed // and drive them through a test subclass. - // Mixing uses collateral transactions to trust parties entering the pool - // to behave honestly. If they don't it takes their money. - std::vector vecSessionCollaterals; - // Input prevouts of every transaction in vecSessionCollaterals, so a dsa whose collateral - // reuses one of them can be rejected without rescanning them all. - std::unordered_set setSessionCollateralPrevouts GUARDED_BY(cs_coinjoin); + /// The collateral transactions of every peer admitted to the current session. + /// + /// Mixing uses collateral transactions to trust parties entering the pool to behave + /// honestly. If they don't it takes their money. + /// + /// Session collaterals are only ever test-accepted, never added to the mempool, so nothing + /// pins their identity: the same UTXO can be re-signed into arbitrarily many distinct txids. + /// Matching on input prevouts is what makes a resent or replayed dsa recognisable as the + /// same participant. + class SessionCollaterals + { + public: + void Add(const CMutableTransaction& txCollateral) + { + m_txs.push_back(MakeTransactionRef(txCollateral)); + for (const auto& txin : txCollateral.vin) { + m_prevouts.insert(txin.prevout); + } + } + void Clear() + { + m_txs.clear(); + m_prevouts.clear(); + } + //! The first input of txCollateral that an already admitted collateral also spends, if any. + std::optional FindCommittedPrevout(const CMutableTransaction& txCollateral) const + { + for (const auto& txin : txCollateral.vin) { + if (m_prevouts.contains(txin.prevout)) return txin.prevout; + } + return std::nullopt; + } + const std::vector& txs() const { return m_txs; } + size_t size() const { return m_txs.size(); } + bool empty() const { return m_txs.empty(); } + + private: + std::vector m_txs; + std::unordered_set m_prevouts; + }; + SessionCollaterals m_session_collaterals GUARDED_BY(cs_coinjoin); // Post-V24: true once a participant has been admitted that declared a promotion/demotion, // i.e. the final transaction may come out unbalanced. Latched on admission rather than @@ -74,26 +131,45 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler /// Add a clients entry to the pool bool AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessageIDRet) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); - /// Record an accepted collateral and index its input prevouts - void CommitSessionCollateral(const CMutableTransaction& txCollateral) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); - /// Build and relay the final transaction if the live session is still eligible - void CreateFinalTransaction(int session_id) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + /// Build and relay the final transaction if the live session is still eligible, charging one + /// missing participant first if charge_fees is set + void CreateFinalTransaction(int session_id, bool charge_fees) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); private: bool fUnitTest; + /// Serializes CheckPool() against itself and against CheckTimeout(). CheckPool() runs both on + /// the scheduler thread and on the message-handling thread, and its finalize and commit steps + /// have to be single-shot: relaying DSFINALTX twice makes every client sign twice, and the + /// duplicate signatures then abort the session for all of them. CheckTimeout() uses the same + /// guard so it cannot reset a session during finalization or commit. Always acquired with + /// TRY_LOCK, so a contended caller skips the round rather than blocking msghand. + Mutex cs_check_pool; + /// Add signature to a txin - bool AddScriptSig(const CTxIn& txin) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + bool AddScriptSig(const CTxIn& txin) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); - /// Charge fees to bad actors (Charge clients a fee if they're abusive) - void ChargeFees() const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + /// Collaterals chosen to be charged whose consumption has not finished yet. They are chosen + /// under cs_coinjoin, but ConsumeCollateral() takes cs_main and runs after cs_coinjoin is + /// released, by which time the session may have been reset. Until then admission refuses any + /// collateral that spends one of their inputs, so a charged participant cannot be readmitted + /// into the next session with a collateral that is about to be spent. Survives SetNull(). + std::vector m_collaterals_being_charged GUARDED_BY(cs_coinjoin); + /// Does txCollateral spend an input of a collateral that is being charged? + bool SpendsCollateralBeingCharged(const CMutableTransaction& txCollateral) const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); + + /// Choose one bad actor whose collateral should be consumed, if any, and reserve it. The + /// result must be passed to ConsumeReservedCollateral(). + CTransactionRef SelectCollateralToCharge() EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); /// Rarely charge fees to pay miners - void ChargeRandomFees() const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + void ChargeRandomFees(const std::vector& collaterals) const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); /// Consume collateral in cases when peer misbehaved. Takes cs_main, which this class never /// takes under cs_coinjoin. void ConsumeCollateral(const CTransactionRef& txref) const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + /// Consume a collateral reserved in m_collaterals_being_charged, then release the reservation + void ConsumeReservedCollateral(const CTransactionRef& txref) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); /// Consume collateral, but only while session_id is still the live session holding it - void ConsumeCollateralIfCurrentSession(int session_id, const CTransactionRef& txref) const + void ConsumeCollateralIfCurrentSession(int session_id, const CTransactionRef& txref) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); /// Is session_id still the session we are accepting entries for? @@ -108,15 +184,22 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler size_t entries{0}; size_t collaterals{0}; CoinJoin::MixSideCounts sides; + bool signatures_complete{false}; }; PoolSnapshot GetPoolSnapshot() const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); /// Is txref one of the collaterals accepted into the current session? bool HasSessionCollateral(const CTransactionRef& txref) const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); /// Check for process - void CheckPool(); + void CheckPool() EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin, !cs_check_pool); - void CommitFinalTransaction() EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + void CommitFinalTransaction(int session_id) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + /// Tell the given participants of session_id how it ended. Side-effect free: the participant + /// list is captured by the caller, so a reset or replacement session cannot be notified. + void RelayCompletedTransaction(int session_id, const std::vector& participants, PoolMessage nMessageID) + EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + /// Reset the pool, but only while session_id is still the signing session being completed + void ResetSigningSessionIfCurrent(int session_id) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); /// Is this nDenom and txCollateral acceptable? bool IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) const; @@ -126,23 +209,27 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler bool IsSessionReady() const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); /// Check that all inputs are signed. (Are all inputs signed?) - bool IsSignaturesComplete() const EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + bool IsSignaturesComplete() const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); /// Check to make sure a given input matches an input in the pool and its scriptSig is valid bool IsInputScriptSigValid(const CTxIn& txin) const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); - // Set the 'state' value, with some logging and capturing when the state changed - void SetState(PoolState nStateNew); + // Set the 'state' value, with some logging and capturing when the state changed. + // Requires cs_coinjoin so that a transition and the session data it describes are always + // observed together: code that revalidates nState under the lock must not have it changed + // out from under it by a concurrent transition. + void SetState(PoolState nStateNew) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); /// Relay mixing Messages void RelayFinalTransaction(const CTransaction& txFinal) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); void PushStatus(CNode& peer, PoolStatusUpdate nStatusUpdate, PoolMessage nMessageID) const; void RelayStatus(PoolStatusUpdate nStatusUpdate, PoolMessage nMessageID = MSG_NOERR) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); - void RelayCompletedTransaction(PoolMessage nMessageID) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + /// Addresses of the participants that submitted an entry to the current session + std::vector GetParticipantAddrs() const EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); void ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); void ProcessDSQUEUE(NodeId from, CDataStream& vRecv); - void ProcessDSVIN(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); - void ProcessDSSIGNFINALTX(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + void ProcessDSVIN(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin, !cs_check_pool); + void ProcessDSSIGNFINALTX(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin, !cs_check_pool); void SetNull() override EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); @@ -156,14 +243,15 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman); ~CCoinJoinServer() override; - void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) override; + void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) override + EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin, !cs_check_pool); bool ProcessGetData(CNode& pfrom, const CInv& inv, const CNetMsgMaker& msgMaker) override; bool AlreadyHave(const CInv& inv) override; void Schedule(CScheduler& scheduler) override; bool HasTimedOut() const; - void CheckTimeout(); - void CheckForCompleteQueue(); + void CheckTimeout() EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin, !cs_check_pool); + void CheckForCompleteQueue() EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); void GetJsonInfo(UniValue& obj) const; }; diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 804e16cf2263..d46c777e6a35 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -61,6 +61,7 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-mintxfee=", "-paytxfee=", "-txconfirmtarget=", + "-allow-deprecated-long-mnemonics", "-hdseed=", "-mnemonic=", "-mnemonicpassphrase=", diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index df40c6378756..cafe65b023f4 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -17,12 +17,14 @@ #include #include #include +#include #include #include #include #include #include +#include using node::ReadBlockFromDisk; @@ -41,7 +43,11 @@ static bool GetDataFromUnlockTx(const CTransaction& tx, CAmount& toUnlock, uint6 if (!MoneyRange(txout.nValue)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-txout-outofrange"); } - toUnlock += txout.nValue; + const auto total = CheckedAdd(toUnlock, txout.nValue); + if (!total) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-overflow"); + } + toUnlock = *total; } return true; } @@ -288,6 +294,16 @@ CCreditPoolDiff::CCreditPoolDiff(CCreditPool starter, const CBlockIndex* pindexP } } +CAmount CCreditPoolDiff::GetTotalLocked() const +{ + // Subtract withdrawals first to preserve a representable net balance. + auto total = CheckedAdd(pool.locked, -sessionUnlocked); + if (total) total = CheckedAdd(*total, sessionLocked); + if (total) total = CheckedAdd(*total, platformReward); + if (!total) throw std::overflow_error("credit-pool balance overflow"); + return *total; +} + bool CCreditPoolDiff::Lock(const CTransaction& tx, TxValidationState& state) { if (const auto opt_assetLockTx = GetTxPayload(tx); !opt_assetLockTx) { @@ -297,7 +313,11 @@ bool CCreditPoolDiff::Lock(const CTransaction& tx, TxValidationState& state) for (const CTxOut& txout : tx.vout) { if (const CScript& script = txout.scriptPubKey; script.empty() || script[0] != OP_RETURN) continue; - sessionLocked += txout.nValue; + const auto total = CheckedAdd(sessionLocked, txout.nValue); + if (!total) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-lock-overflow"); + } + sessionLocked = *total; return true; } @@ -313,7 +333,8 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state, s return false; } - if (sessionUnlocked + toUnlock > pool.currentLimit) { + const auto total = CheckedAdd(sessionUnlocked, toUnlock); + if (!total || *total > pool.currentLimit) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-too-much"); } @@ -323,7 +344,7 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state, s newIndexes.insert(index); if (inserted_index) *inserted_index = index; - sessionUnlocked += toUnlock; + sessionUnlocked = *total; return true; } diff --git a/src/evo/creditpool.h b/src/evo/creditpool.h index bb3891a37403..e7676beace54 100644 --- a/src/evo/creditpool.h +++ b/src/evo/creditpool.h @@ -100,10 +100,9 @@ class CCreditPoolDiff { /** * this function returns total amount of credits for the next block + * Throws if the total cannot be represented as CAmount. */ - CAmount GetTotalLocked() const { - return pool.locked + sessionLocked - sessionUnlocked + platformReward; - } + CAmount GetTotalLocked() const; std::string ToString() const { return strprintf("CCreditPoolDiff(sessionLocked=%lld, sessionUnlocked=%lld, platforomReward=%lld, newIndexes=%lld, pool=%s)", sessionLocked, sessionUnlocked, platformReward, newIndexes.size(), pool.ToString()); diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 89997142a884..8241cdcab704 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -726,7 +726,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_nullnHeight; mnListDiffsCache.emplace(pindex->GetBlockHash(), diff); - mnListsCache.emplace(newList.GetBlockHash(), newList); + mnListsCache.insert_or_assign(newList.GetBlockHash(), newList); } catch (const std::exception& e) { LogPrintf("CDeterministicMNManager::%s -- internal error: %s\n", __func__, e.what()); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "failed-dmn-block"); @@ -1229,6 +1229,9 @@ CDeterministicMNManager::RecalcDiffsResult CDeterministicMNManager::RecalculateA // Storage for recalculated diffs if we plan to repair std::vector> recalculated_diffs; + // Snapshots can't be rebuilt from diffs, so in repair mode a missing one means the repair failed + auto& missing_snapshot_errors = repair ? result.repair_errors : result.verification_errors; + // Process each pair of consecutive snapshots for (size_t i = 0; i < snapshot_blocks.size() - 1; ++i) { const CBlockIndex* from_index = snapshot_blocks[i]; @@ -1252,7 +1255,7 @@ CDeterministicMNManager::RecalcDiffsResult CDeterministicMNManager::RecalculateA __func__, from_index->nHeight); } else { // Any other missing snapshot is critical corruption beyond our repair capability - result.verification_errors.push_back(strprintf("CRITICAL: Snapshot missing at height %d. " + missing_snapshot_errors.push_back(strprintf("CRITICAL: Snapshot missing at height %d. " "This cannot be repaired by this tool - full reindex required.", from_index->nHeight)); return result; } @@ -1260,7 +1263,7 @@ CDeterministicMNManager::RecalcDiffsResult CDeterministicMNManager::RecalculateA if (!has_to_snapshot) { // Missing target snapshot is always critical - we cannot repair snapshots, only diffs - result.verification_errors.push_back(strprintf("CRITICAL: Snapshot missing at height %d. " + missing_snapshot_errors.push_back(strprintf("CRITICAL: Snapshot missing at height %d. " "This cannot be repaired by this tool - full reindex required.", to_index->nHeight)); return result; } diff --git a/src/evo/dmnstate.h b/src/evo/dmnstate.h index a6072a23b557..10bc95d2d972 100644 --- a/src/evo/dmnstate.h +++ b/src/evo/dmnstate.h @@ -259,10 +259,12 @@ class CDeterministicMNStateDiff } else if constexpr (BaseType::mask == Field_pubKeyOperator) { // CBLSLazyPublicKey::operator== compares the underlying key and ignores its BLS // encoding, but a scheme migration re-encodes the same key (legacy->basic). That must - // be captured in the diff -- GetHash() is scheme-dependent -- or a diff-reconstructed - // list keeps the old encoding while an online-built list has the new one, and the two - // diverge (mnUniquePropertyMap included). - if (member.get(a).GetHash() != member.get(b).GetHash()) { + // be captured in the diff or a diff-reconstructed list keeps the old encoding while an + // online-built list has the new one, and the two diverge (mnUniquePropertyMap included). + // GetHash() only covers the encoded bytes, and identical bytes decode to a different key + // under the other scheme, so a scheme change must be captured even when the bytes match. + if (member.get(a).GetHash() != member.get(b).GetHash() || + (a.nVersion == ProTxVersion::LegacyBLS) != (b.nVersion == ProTxVersion::LegacyBLS)) { member.get(state) = member.get(b); fields |= member.mask; } diff --git a/src/evo/netinfo.cpp b/src/evo/netinfo.cpp index 701f7c46cde0..b5c806706d92 100644 --- a/src/evo/netinfo.cpp +++ b/src/evo/netinfo.cpp @@ -291,7 +291,7 @@ NetInfoStatus MnNetInfo::ValidateService(const CService& service) if (!service.IsIPv4()) { return NetInfoStatus::BadType; } - if (Params().RequireRoutableExternalIP() && !service.IsRoutable()) { + if (Params().RequireRoutableExternalIP() && (service.IsRFC1918() || !service.IsRoutable())) { return NetInfoStatus::NotRoutable; } @@ -383,19 +383,18 @@ std::string MnNetInfo::ToString() const bool ExtNetInfo::HasAddrPortDuplicates() const { std::set known{}; - for (const auto& entry : m_all_entries) { + for (const auto& entry : GetEntries()) { if (auto [_, inserted] = known.insert(entry); !inserted) { return true; } } - ASSERT_IF_DEBUG(known.size() == m_all_entries.size()); return false; } bool ExtNetInfo::IsAddrPortDuplicate(const NetInfoEntry& candidate) const { - return std::any_of(m_all_entries.begin(), m_all_entries.end(), - [&candidate](const auto& entry) { return candidate == entry; }); + const auto entries{GetEntries()}; + return std::any_of(entries.begin(), entries.end(), [&candidate](const auto& entry) { return candidate == entry; }); } bool ExtNetInfo::HasAddrDuplicates(const NetInfoList& entries) @@ -443,9 +442,6 @@ NetInfoStatus ExtNetInfo::ProcessCandidate(const NetInfoPurpose purpose, const N auto [_, status] = m_data.try_emplace(purpose, std::vector({candidate})); assert(status); // We did just check to see if our value already existed, try_emplace shouldn't fail } - - // Candidate successfully added, update cache - m_all_entries.push_back(candidate); return NetInfoStatus::Success; } @@ -457,7 +453,7 @@ NetInfoStatus ExtNetInfo::ValidateService(const NetInfoPurpose purpose, const CS if (!service.IsCJDNS() && !service.IsI2P() && !service.IsIPv4() && !service.IsIPv6() && !service.IsTor()) { return NetInfoStatus::BadType; } - if (Params().RequireRoutableExternalIP() && !service.IsRoutable()) { + if (Params().RequireRoutableExternalIP() && (service.IsRFC1918() || !service.IsRoutable())) { return NetInfoStatus::NotRoutable; } const uint16_t service_port{service.GetPort()}; @@ -553,10 +549,11 @@ NetInfoStatus ExtNetInfo::AddEntry(const NetInfoPurpose purpose, const std::stri NetInfoList ExtNetInfo::GetEntries(std::optional purpose_opt) const { if (!purpose_opt) { - // Without a purpose code specified, we're being requested for a flattened list of all - // entries. ExtNetInfo is an append-only structure, so we can avoid re-calculating a - // list of all entries by maintaining a small cache. - return m_all_entries; + NetInfoList ret; + for (const auto& [_, entries] : m_data) { + ret.insert(ret.end(), entries.begin(), entries.end()); + } + return ret; } if (!IsValidPurpose(*purpose_opt)) return NetInfoList{}; const auto& it{m_data.find(*purpose_opt)}; @@ -594,9 +591,6 @@ NetInfoStatus ExtNetInfo::Validate() const return NetInfoStatus::Duplicate; } for (const auto& [purpose, entries] : m_data) { - if (!IsValidPurpose(purpose)) { - return NetInfoStatus::Malformed; - } if (entries.empty()) { // Purpose if present in map must have at least one entry return NetInfoStatus::Malformed; @@ -610,27 +604,18 @@ NetInfoStatus ExtNetInfo::Validate() const return NetInfoStatus::Duplicate; } for (const auto& entry : entries) { - if (!entry.IsTriviallyValid()) { - // Trivially invalid NetInfoEntry, no point checking against consensus rules - return NetInfoStatus::Malformed; - } + NetInfoStatus ret{NetInfoStatus::Malformed}; if (const auto& service_opt{entry.GetAddrPort()}) { - if (auto ret{ValidateService(purpose, *service_opt)}; ret != NetInfoStatus::Success) { - // Stores CService underneath but doesn't pass validation rules - return ret; - } + ret = ValidateService(purpose, *service_opt); } else if (const auto domain_opt{entry.GetDomainPort()}) { if (purpose != NetInfoPurpose::PLATFORM_HTTPS) { // Domains only allowed for Platform HTTPS API return NetInfoStatus::BadInput; } - if (auto ret{ValidateDomainPort(*domain_opt)}; ret != NetInfoStatus::Success) { - // Stores DomainPort underneath but doesn't pass validation rules - return ret; - } - } else { - // Doesn't store valid type underneath - return NetInfoStatus::Malformed; + ret = ValidateDomainPort(*domain_opt); + } + if (ret != NetInfoStatus::Success) { + return ret; } } } diff --git a/src/evo/netinfo.h b/src/evo/netinfo.h index ead025ca6629..17afab6e7feb 100644 --- a/src/evo/netinfo.h +++ b/src/evo/netinfo.h @@ -400,9 +400,6 @@ class ExtNetInfo final : public NetInfoInterface uint8_t m_version{CURRENT_VERSION}; std::map m_data{}; - // memory only - NetInfoList m_all_entries{}; - public: ExtNetInfo() = default; template @@ -425,12 +422,16 @@ class ExtNetInfo final : public NetInfoInterface if (m_version == 0 || m_version > CURRENT_VERSION) { return; // Don't bother with unknown versions } - s >> m_data; - - // Regenerate internal cache - m_all_entries.clear(); - for (const auto& [_, entries] : m_data) { - m_all_entries.insert(m_all_entries.end(), entries.begin(), entries.end()); + // Purposes must be known and ascending, so that one object has exactly one encoding + m_data.clear(); + const auto n_purposes{ReadCompactSize(s)}; + for (size_t i{0}; i < n_purposes; ++i) { + NetInfoPurpose purpose; + s >> purpose; + if (!IsValidPurpose(purpose) || (!m_data.empty() && purpose <= m_data.rbegin()->first)) { + throw std::ios_base::failure("ExtNetInfo: purpose unknown or out of order"); + } + s >> m_data.try_emplace(purpose).first->second; } } @@ -449,7 +450,6 @@ class ExtNetInfo final : public NetInfoInterface { m_version = CURRENT_VERSION; m_data.clear(); - m_all_entries.clear(); } private: diff --git a/src/evo/providertx.cpp b/src/evo/providertx.cpp index c10b312a441a..c6a5014b9c0d 100644 --- a/src/evo/providertx.cpp +++ b/src/evo/providertx.cpp @@ -319,11 +319,6 @@ bool CProRegTx::IsTriviallyValid(TxValidationState& state) const // pass the state returned by the function above return false; } - for (const auto& entry : netInfo->GetEntries()) { - if (!entry.IsTriviallyValid()) { - return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-bad"); - } - } if (nOperatorReward > 10000) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-operator-reward"); @@ -430,11 +425,6 @@ bool CProUpServTx::IsTriviallyValid(TxValidationState& state) const // pass the state returned by the function above return false; } - for (const auto& entry : netInfo->GetEntries()) { - if (!entry.IsTriviallyValid()) { - return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-bad"); - } - } return true; } diff --git a/src/evo/providertx_service.cpp b/src/evo/providertx_service.cpp index aa9426567183..5732a5162554 100644 --- a/src/evo/providertx_service.cpp +++ b/src/evo/providertx_service.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -23,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -728,6 +730,69 @@ std::optional InsertUnanimousSignatures(node::NodeContext& node return std::nullopt; } +//! The share signatures on a registrar update commit to its payload and input outpoints but +//! not to its outputs. Before re-signing the fee inputs, make sure the transaction has the shape +//! the wallet gave it when funding it: unlocked coins of one address of this wallet, change (if +//! any) back to that address, and a fee no higher than the wallet's maximum. +std::optional CheckRegistrarUpdateFunding(Wallet& wallet, const CMutableTransaction& tx) +{ + const auto single_address_error{[] { + return Error(ProviderTxErrorCode::WALLET_ERROR, + "a shared registrar update must spend from and return change to a single address"); + }}; + std::vector outpoints; + outpoints.reserve(tx.vin.size()); + for (const CTxIn& input : tx.vin) { + outpoints.push_back(input.prevout); + } + const auto coins{wallet.getCoins(outpoints)}; + std::optional fee_source; + CAmount input_total{0}; + for (size_t i = 0; i < coins.size(); ++i) { + const CScript& script{coins[i].txout.scriptPubKey}; + CTxDestination source; + if (coins[i].depth_in_main_chain < 0 || !wallet.isSpendable(script) || !ExtractDestination(script, source)) { + return Error(ProviderTxErrorCode::WALLET_ERROR, + "a shared registrar update must be combined by the wallet that funded it"); + } + // Masternode collaterals are locked, and funding never selects a locked coin + if (wallet.isLockedCoin(outpoints[i])) { + return Error(ProviderTxErrorCode::WALLET_ERROR, "a shared registrar update must not spend a locked coin"); + } + if (fee_source && source != *fee_source) return single_address_error(); + fee_source = source; + input_total += coins[i].txout.nValue; + if (!MoneyRange(input_total)) { + return Error(ProviderTxErrorCode::INVALID_PARAMETER, "transaction input value out of range"); + } + } + if (!fee_source) { + return Error(ProviderTxErrorCode::WALLET_ERROR, + "a shared registrar update must be combined by the wallet that funded it"); + } + CAmount output_total{0}; + for (const CTxOut& output : tx.vout) { + if (!MoneyRange(output.nValue)) { + return Error(ProviderTxErrorCode::INVALID_PARAMETER, "transaction output value out of range"); + } + output_total += output.nValue; + if (!MoneyRange(output_total)) { + return Error(ProviderTxErrorCode::INVALID_PARAMETER, "transaction output value out of range"); + } + // Funding without change leaves a zero-valued marker output + if (output.nValue == 0) continue; + if (CTxDestination change; !ExtractDestination(output.scriptPubKey, change) || change != *fee_source) { + return single_address_error(); + } + } + if (const CAmount fee{input_total - output_total}; fee > wallet.getDefaultMaxTxFee()) { + return Error(ProviderTxErrorCode::WALLET_ERROR, + strprintf("a shared registrar update fee of %s exceeds this wallet's maximum fee (%s)", + FormatMoney(fee), FormatMoney(wallet.getDefaultMaxTxFee()))); + } + return std::nullopt; +} + } // namespace interfaces::ProviderTxCapabilities GetCapabilities(node::NodeContext& node, std::optional basic_override) @@ -1232,9 +1297,10 @@ ProviderTxResult CombineShared(node::NodeContext& node, Wa "a shared registrar update must be combined by the wallet that funded it"); } if (auto error{CheckWallet(*wallet)}) return *error; + if (auto error{CheckRegistrarUpdateFunding(*wallet, tx)}) return *error; // Inserting the signatures changed the payload, so the fee inputs signed at prepare time // are stale; re-sign them with this wallet - return Finish(node, *wallet, tx, request.submit, /*allow_partial=*/true); + return Finish(node, *wallet, tx, request.submit); } return Error(ProviderTxErrorCode::INVALID_PARAMETER, "transaction is not a shared masternode transaction"); } diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 28f59ec6ced5..b1576f527508 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -306,10 +306,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu return RebuildListFromBlock(block, pindexPrev, is_v24_active, oldList, view, debugLogs, state, mnListRet); } -bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_null pindexPrev, - bool is_v24_active, const CDeterministicMNList& prevList, - const CCoinsViewCache& view, bool debugLogs, BlockValidationState& state, - CDeterministicMNList& mnListRet) +CDeterministicMNList CSpecialTxProcessor::BeginListForBlock(gsl::not_null pindexPrev, + const CDeterministicMNList& prevList) const { // Verify that prevList either represents an empty/initial state (default-constructed), // or it matches the previous block's hash. @@ -322,8 +320,6 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul newList.SetBlockHash(uint256()); // we can't know the final block hash, so better not return a (invalid) block hash newList.SetHeight(nHeight); - auto payee = prevList.GetMNPayee(pindexPrev); - // we iterate the prevList here and update the newList // this is only valid as long these have not diverged at this point, which is the case as long as we don't add // code above this loop that modifies newList @@ -344,390 +340,404 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul newList.DecreaseScores(); - const bool isMNRewardReallocation{DeploymentActiveAfter(pindexPrev, m_consensus_params, Consensus::DEPLOYMENT_MN_RR)}; + return newList; +} - // we skip the coinbase - for (int i = 1; i < static_cast(block.vtx.size()); i++) { - const CTransaction& tx = *block.vtx[i]; +bool CSpecialTxProcessor::ApplyTxToList(const CTransaction& tx, gsl::not_null pindexPrev, + bool is_v24_active, const CCoinsViewCache& view, bool debugLogs, + CDeterministicMNList& newList, BlockValidationState& state) const +{ + const int nHeight = pindexPrev->nHeight + 1; - if (!tx.IsSpecialTxVersion()) { - // only interested in special TXs - continue; + if (!tx.IsSpecialTxVersion()) { + // only interested in special TXs + return true; + } + + if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); } + auto& proTx = *opt_proTx; - if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } - auto& proTx = *opt_proTx; + auto dmn = std::make_shared(newList.GetTotalRegisteredCount(), proTx.nType); + dmn->proTxHash = tx.GetHash(); - auto dmn = std::make_shared(newList.GetTotalRegisteredCount(), proTx.nType); - dmn->proTxHash = tx.GetHash(); + // collateralOutpoint is either pointing to an external collateral or to the ProRegTx itself + if (proTx.collateralOutpoint.hash.IsNull()) { + dmn->collateralOutpoint = COutPoint(tx.GetHash(), proTx.collateralOutpoint.n); + } else { + dmn->collateralOutpoint = proTx.collateralOutpoint; + } - // collateralOutpoint is either pointing to an external collateral or to the ProRegTx itself - if (proTx.collateralOutpoint.hash.IsNull()) { - dmn->collateralOutpoint = COutPoint(tx.GetHash(), proTx.collateralOutpoint.n); - } else { - dmn->collateralOutpoint = proTx.collateralOutpoint; + // Complain about spent collaterals only when we process the tip. + // This is safe because blocks below the tip were verified + // when they were connected initially. + if (!view.GetBestBlock().IsNull()) { + Coin coin; + CAmount expectedCollateral = GetMnType(proTx.nType).collat_amount; + if (!proTx.collateralOutpoint.hash.IsNull() && (!view.GetCoin(dmn->collateralOutpoint, coin) || + coin.IsSpent() || coin.out.nValue != expectedCollateral)) { + // should actually never get to this point as CheckProRegTx should have handled this case. + // We do this additional check nevertheless to be 100% sure + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-collateral"); } + } - // Complain about spent collaterals only when we process the tip. - // This is safe because blocks below the tip were verified - // when they were connected initially. - if (!view.GetBestBlock().IsNull()) { - Coin coin; - CAmount expectedCollateral = GetMnType(proTx.nType).collat_amount; - if (!proTx.collateralOutpoint.hash.IsNull() && (!view.GetCoin(dmn->collateralOutpoint, coin) || - coin.IsSpent() || coin.out.nValue != expectedCollateral)) { - // should actually never get to this point as CheckProRegTx should have handled this case. - // We do this additional check nevertheless to be 100% sure - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-collateral"); - } + auto replacedDmn = newList.GetMNByCollateral(dmn->collateralOutpoint); + if (replacedDmn != nullptr) { + // This might only happen with a ProRegTx that refers an external collateral + // In that case the new ProRegTx will replace the old one. This means the old one is removed + // and the new one is added like a completely fresh one, which is also at the bottom of the payment list + newList.RemoveMN(replacedDmn->proTxHash); + if (debugLogs) { + LogPrintf("%s -- MN %s removed from list because collateral was used for " /* Continued */ + "a new ProRegTx. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", + __func__, replacedDmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), + nHeight, newList.GetCounts().total()); } + } - auto replacedDmn = newList.GetMNByCollateral(dmn->collateralOutpoint); - if (replacedDmn != nullptr) { - // This might only happen with a ProRegTx that refers an external collateral - // In that case the new ProRegTx will replace the old one. This means the old one is removed - // and the new one is added like a completely fresh one, which is also at the bottom of the payment list - newList.RemoveMN(replacedDmn->proTxHash); - if (debugLogs) { - LogPrintf("%s -- MN %s removed from list because collateral was used for " /* Continued */ - "a new ProRegTx. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", - __func__, replacedDmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), - nHeight, newList.GetCounts().total()); + for (const auto& entry : proTx.netInfo->GetEntries()) { + if (const auto service_opt{entry.GetAddrPort()}) { + if (newList.HasUniqueProperty(*service_opt)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); } - } - - for (const auto& entry : proTx.netInfo->GetEntries()) { - if (const auto service_opt{entry.GetAddrPort()}) { - if (newList.HasUniqueProperty(*service_opt)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); - } - } else if (const auto domain_opt{entry.GetDomainPort()}) { - if (newList.HasUniqueProperty(*domain_opt)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); - } - } else { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); + } else if (const auto domain_opt{entry.GetDomainPort()}) { + if (newList.HasUniqueProperty(*domain_opt)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); } + } else { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); } - if (proTx.IsShared()) { - for (const auto& share : proTx.shares) { - if (newList.HasUniqueProperty(share.keyIDOwner)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); - } - } - if (newList.HasUniqueProperty(proTx.pubKeyOperator)) { + } + if (proTx.IsShared()) { + for (const auto& share : proTx.shares) { + if (newList.HasUniqueProperty(share.keyIDOwner)) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); } - } else if (newList.HasUniqueProperty(proTx.keyIDOwner) || newList.HasUniqueProperty(proTx.pubKeyOperator)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); - } - - dmn->nOperatorReward = proTx.nOperatorReward; - - auto dmnState = std::make_shared(proTx); - dmnState->nRegisteredHeight = nHeight; - if (proTx.netInfo->IsEmpty()) { - // start in banned pdmnState as we need to wait for a ProUpServTx - dmnState->BanIfNotBanned(nHeight); } - dmn->pdmnState = dmnState; - - // CheckProRegTx ran against pindexPrev, so transactions in this same block are invisible - // to each other and two of them could claim one operator key under different encodings. - // Re-probe the list as rebuilt so far. AddMN() reports a duplicate by throwing, which - // would escape block assembly, so reject cleanly here instead. - if (is_v24_active && - newList.HasOperatorKeyUnderAnyScheme(dmn->pdmnState->pubKeyOperator.Get(), /*self=*/uint256())) { + if (newList.HasUniqueProperty(proTx.pubKeyOperator)) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); } + } else if (newList.HasUniqueProperty(proTx.keyIDOwner) || newList.HasUniqueProperty(proTx.pubKeyOperator)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); + } - newList.AddMN(dmn); + dmn->nOperatorReward = proTx.nOperatorReward; - if (debugLogs) { - LogPrintf("%s -- MN %s added at height %d: %s\n", __func__, tx.GetHash().ToString(), nHeight, - proTx.ToString()); - } - } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } - if (!IsValidMnType(opt_proTx->nType)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type"); - } + auto dmnState = std::make_shared(proTx); + dmnState->nRegisteredHeight = nHeight; + if (proTx.netInfo->IsEmpty()) { + // start in banned pdmnState as we need to wait for a ProUpServTx + dmnState->BanIfNotBanned(nHeight); + } + dmn->pdmnState = dmnState; - for (const auto& entry : opt_proTx->netInfo->GetEntries()) { - if (const auto service_opt{entry.GetAddrPort()}) { - if (newList.HasUniqueProperty(*service_opt) && - newList.GetUniquePropertyMN(*service_opt)->proTxHash != opt_proTx->proTxHash) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); - } - } else if (const auto domain_opt{entry.GetDomainPort()}) { - if (newList.HasUniqueProperty(*domain_opt) && - newList.GetUniquePropertyMN(*domain_opt)->proTxHash != opt_proTx->proTxHash) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); - } - } else { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); - } - } + // CheckProRegTx ran against pindexPrev, so transactions in this same block are invisible + // to each other and two of them could claim one operator key under different encodings. + // Re-probe the list as rebuilt so far. AddMN() reports a duplicate by throwing, which + // would escape block assembly, so reject cleanly here instead. + if (is_v24_active && + newList.HasOperatorKeyUnderAnyScheme(dmn->pdmnState->pubKeyOperator.Get(), /*self=*/uint256())) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); + } - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); - } - if (opt_proTx->nType != dmn->nType) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type-mismatch"); - } + newList.AddMN(dmn); - auto newState = std::make_shared(*dmn->pdmnState); - const uint16_t current_version{static_cast(newState->nVersion)}; - const uint16_t target_version{is_v24_active ? std::max(current_version, opt_proTx->nVersion) - : current_version}; - if (!ApplyProUpServTx(*newState, *opt_proTx, is_v24_active, state)) { - return false; - } - if (newState->IsBanned()) { - // only revive when all keys are set (a shared masternode has a null keyIDOwner; - // its share owner keys are immutable and always set) - if (newState->pubKeyOperator != CBLSLazyPublicKey() && !newState->keyIDVoting.IsNull() && - (newState->IsShared() || !newState->keyIDOwner.IsNull())) { - newState->Revive(nHeight); - if (debugLogs) { - LogPrintf("%s -- MN %s revived at height %d\n", __func__, opt_proTx->proTxHash.ToString(), nHeight); - } - } - } + if (debugLogs) { + LogPrintf("%s -- MN %s added at height %d: %s\n", __func__, tx.GetHash().ToString(), nHeight, + proTx.ToString()); + } + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } + if (!IsValidMnType(opt_proTx->nType)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type"); + } - // Migrating a legacy masternode to the basic scheme re-encodes its stored key - // (SetStateVersion), moving it to the basic-scheme slot. Per-transaction checks ran - // against pindexPrev, so re-check against the list as rebuilt so far: if another - // masternode holds this key under either encoding, the re-key in UpdateMN() would throw - // out of block assembly. - if (is_v24_active && IsSchemeMigration(current_version, target_version) && - newList.HasOperatorKeyUnderAnyScheme(dmn->pdmnState->pubKeyOperator.Get(), - /*self=*/opt_proTx->proTxHash)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); + for (const auto& entry : opt_proTx->netInfo->GetEntries()) { + if (const auto service_opt{entry.GetAddrPort()}) { + if (newList.HasUniqueProperty(*service_opt) && + newList.GetUniquePropertyMN(*service_opt)->proTxHash != opt_proTx->proTxHash) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); + } + } else if (const auto domain_opt{entry.GetDomainPort()}) { + if (newList.HasUniqueProperty(*domain_opt) && + newList.GetUniquePropertyMN(*domain_opt)->proTxHash != opt_proTx->proTxHash) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); + } + } else { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); } + } - newList.UpdateMN(opt_proTx->proTxHash, newState); - if (debugLogs) { - LogPrintf("%s -- MN %s updated at height %d: %s\n", __func__, opt_proTx->proTxHash.ToString(), nHeight, - opt_proTx->ToString()); - } - } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } + auto dmn = newList.GetMN(opt_proTx->proTxHash); + if (!dmn) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); + } + if (opt_proTx->nType != dmn->nType) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type-mismatch"); + } - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); - } - if (dmn->pdmnState->IsShared()) { - // shared masternodes are updated via ProUpShareTx / ProUpSharedRegTx exclusively - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-shared-mn"); - } - auto newState = std::make_shared(*dmn->pdmnState); - const uint16_t old_version{static_cast(newState->nVersion)}; - const bool operator_changed{newState->pubKeyOperator != opt_proTx->pubKeyOperator}; - const uint16_t target_version{is_v24_active ? std::max(old_version, opt_proTx->nVersion) - : (operator_changed ? opt_proTx->nVersion : old_version)}; - - // Per-transaction checks ran against pindexPrev, so an earlier transaction in this same - // block is invisible to them. Re-evaluate against the list as rebuilt so far: this update - // moves the operator key to a new unique-property slot if it rotates the key or crosses - // the legacy->basic boundary (which re-encodes the key), and if that slot is held by - // another masternode the re-key in UpdateMN() would throw out of block assembly. Reject - // cleanly. Scoped to those two cases so a pre-existing cross-scheme pair's non-migrating - // routine update is not blocked. - { - const bool migrating{IsSchemeMigration(old_version, target_version)}; - if (is_v24_active && (operator_changed || migrating) && - newList.HasOperatorKeyUnderAnyScheme(opt_proTx->pubKeyOperator.Get(), - /*self=*/opt_proTx->proTxHash)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); + auto newState = std::make_shared(*dmn->pdmnState); + const uint16_t current_version{static_cast(newState->nVersion)}; + const uint16_t target_version{is_v24_active ? std::max(current_version, opt_proTx->nVersion) + : current_version}; + if (!ApplyProUpServTx(*newState, *opt_proTx, is_v24_active, state)) { + return false; + } + if (newState->IsBanned()) { + // only revive when all keys are set (a shared masternode has a null keyIDOwner; + // its share owner keys are immutable and always set) + if (newState->pubKeyOperator != CBLSLazyPublicKey() && !newState->keyIDVoting.IsNull() && + (newState->IsShared() || !newState->keyIDOwner.IsNull())) { + newState->Revive(nHeight); + if (debugLogs) { + LogPrintf("%s -- MN %s revived at height %d\n", __func__, opt_proTx->proTxHash.ToString(), nHeight); } } + } - if (operator_changed) { - // reset all operator related fields and put MN into PoSe-banned state in case the operator key changes - newState->ResetOperatorFields(); - newState->BanIfNotBanned(nHeight); - newState->pubKeyOperator = opt_proTx->pubKeyOperator; - } - newState->keyIDVoting = opt_proTx->keyIDVoting; - // SetStateVersion() re-encodes the operator key to target_version's scheme, so the stored - // key stays consistent with its version whether it was carried in this payload (possibly - // under a different version's encoding) or migrated in place. - if (!SetStateVersion(*newState, target_version, dmn->nType, state)) { - return false; - } - if (target_version >= ProTxVersion::ExtAddr) { - newState->payouts = opt_proTx->nVersion >= ProTxVersion::ExtAddr - ? opt_proTx->payouts - : LegacyPayoutAsList(opt_proTx->scriptPayout); - newState->scriptPayout.clear(); - } else { - newState->scriptPayout = opt_proTx->scriptPayout; - newState->payouts.clear(); - } + // Migrating a legacy masternode to the basic scheme re-encodes its stored key + // (SetStateVersion), moving it to the basic-scheme slot. Per-transaction checks ran + // against pindexPrev, so re-check against the list as rebuilt so far: if another + // masternode holds this key under either encoding, the re-key in UpdateMN() would throw + // out of block assembly. + if (is_v24_active && IsSchemeMigration(current_version, target_version) && + newList.HasOperatorKeyUnderAnyScheme(dmn->pdmnState->pubKeyOperator.Get(), + /*self=*/opt_proTx->proTxHash)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); + } - newList.UpdateMN(opt_proTx->proTxHash, newState); + newList.UpdateMN(opt_proTx->proTxHash, newState); + if (debugLogs) { + LogPrintf("%s -- MN %s updated at height %d: %s\n", __func__, opt_proTx->proTxHash.ToString(), nHeight, + opt_proTx->ToString()); + } + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } - if (debugLogs) { - LogPrintf("%s -- MN %s updated at height %d: %s\n", __func__, opt_proTx->proTxHash.ToString(), nHeight, - opt_proTx->ToString()); - } - } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + auto dmn = newList.GetMN(opt_proTx->proTxHash); + if (!dmn) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); + } + if (dmn->pdmnState->IsShared()) { + // shared masternodes are updated via ProUpShareTx / ProUpSharedRegTx exclusively + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-shared-mn"); + } + auto newState = std::make_shared(*dmn->pdmnState); + const uint16_t old_version{static_cast(newState->nVersion)}; + const bool operator_changed{newState->pubKeyOperator != opt_proTx->pubKeyOperator}; + const uint16_t target_version{is_v24_active ? std::max(old_version, opt_proTx->nVersion) + : (operator_changed ? opt_proTx->nVersion : old_version)}; + + // Per-transaction checks ran against pindexPrev, so an earlier transaction in this same + // block is invisible to them. Re-evaluate against the list as rebuilt so far: this update + // moves the operator key to a new unique-property slot if it rotates the key or crosses + // the legacy->basic boundary (which re-encodes the key), and if that slot is held by + // another masternode the re-key in UpdateMN() would throw out of block assembly. Reject + // cleanly. Scoped to those two cases so a pre-existing cross-scheme pair's non-migrating + // routine update is not blocked. + { + const bool migrating{IsSchemeMigration(old_version, target_version)}; + if (is_v24_active && (operator_changed || migrating) && + newList.HasOperatorKeyUnderAnyScheme(opt_proTx->pubKeyOperator.Get(), + /*self=*/opt_proTx->proTxHash)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); } + } - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); - } - auto newState = std::make_shared(*dmn->pdmnState); - const uint16_t old_version{static_cast(newState->nVersion)}; - // ResetOperatorFields() drops nVersion back to LegacyBLS. Post-v24 the state version must be - // preserved (never silently downgraded), so pick the target the same max-based way the registrar - // path does and restore it below. Pre-v24 keep the historical reset-to-legacy behaviour. - uint16_t target_version{ProTxVersion::LegacyBLS}; - if (is_v24_active) { - target_version = std::max(old_version, opt_proTx->nVersion); - } + if (operator_changed) { + // reset all operator related fields and put MN into PoSe-banned state in case the operator key changes newState->ResetOperatorFields(); - if (target_version >= ProTxVersion::BasicBLS && !SetStateVersion(*newState, target_version, dmn->nType, state)) { - return false; - } newState->BanIfNotBanned(nHeight); - newState->nRevocationReason = opt_proTx->nReason; + newState->pubKeyOperator = opt_proTx->pubKeyOperator; + } + newState->keyIDVoting = opt_proTx->keyIDVoting; + // SetStateVersion() re-encodes the operator key to target_version's scheme, so the stored + // key stays consistent with its version whether it was carried in this payload (possibly + // under a different version's encoding) or migrated in place. + if (!SetStateVersion(*newState, target_version, dmn->nType, state)) { + return false; + } + if (target_version >= ProTxVersion::ExtAddr) { + newState->payouts = opt_proTx->nVersion >= ProTxVersion::ExtAddr + ? opt_proTx->payouts + : LegacyPayoutAsList(opt_proTx->scriptPayout); + newState->scriptPayout.clear(); + } else { + newState->scriptPayout = opt_proTx->scriptPayout; + newState->payouts.clear(); + } - newList.UpdateMN(opt_proTx->proTxHash, newState); + newList.UpdateMN(opt_proTx->proTxHash, newState); - if (debugLogs) { - LogPrintf("%s -- MN %s revoked operator key at height %d: %s\n", __func__, - opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); - } - } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SHARE) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } + if (debugLogs) { + LogPrintf("%s -- MN %s updated at height %d: %s\n", __func__, opt_proTx->proTxHash.ToString(), nHeight, + opt_proTx->ToString()); + } + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); - } - if (!dmn->pdmnState->IsShared() || opt_proTx->shareIndex >= dmn->pdmnState->shares.size()) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-proupshare-index"); - } + auto dmn = newList.GetMN(opt_proTx->proTxHash); + if (!dmn) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); + } + auto newState = std::make_shared(*dmn->pdmnState); + const uint16_t old_version{static_cast(newState->nVersion)}; + // ResetOperatorFields() drops nVersion back to LegacyBLS. Post-v24 the state version must be + // preserved (never silently downgraded), so pick the target the same max-based way the registrar + // path does and restore it below. Pre-v24 keep the historical reset-to-legacy behaviour. + uint16_t target_version{ProTxVersion::LegacyBLS}; + if (is_v24_active) { + target_version = std::max(old_version, opt_proTx->nVersion); + } + newState->ResetOperatorFields(); + if (target_version >= ProTxVersion::BasicBLS && !SetStateVersion(*newState, target_version, dmn->nType, state)) { + return false; + } + newState->BanIfNotBanned(nHeight); + newState->nRevocationReason = opt_proTx->nReason; - auto newState = std::make_shared(*dmn->pdmnState); - newState->shares[opt_proTx->shareIndex].scriptReward = opt_proTx->scriptReward; + newList.UpdateMN(opt_proTx->proTxHash, newState); - // CheckProUpShareTx ran against pindexPrev, so a shared registrar update earlier in - // this block is invisible to it and could have moved the voting key onto this share's - // scripts. Recheck the separation rule against the state as rebuilt so far. - if (!IsShareListVotingKeySafe(newState->shares, newState->keyIDVoting)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-proupshare-payee-reuse"); - } + if (debugLogs) { + LogPrintf("%s -- MN %s revoked operator key at height %d: %s\n", __func__, + opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + } + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SHARE) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } - newList.UpdateMN(opt_proTx->proTxHash, newState); + auto dmn = newList.GetMN(opt_proTx->proTxHash); + if (!dmn) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); + } + if (!dmn->pdmnState->IsShared() || opt_proTx->shareIndex >= dmn->pdmnState->shares.size()) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-proupshare-index"); + } - if (debugLogs) { - LogPrintf("%s -- MN %s updated share at height %d: %s\n", __func__, opt_proTx->proTxHash.ToString(), - nHeight, opt_proTx->ToString()); - } - } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SHARED_REGISTRAR) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } + auto newState = std::make_shared(*dmn->pdmnState); + newState->shares[opt_proTx->shareIndex].scriptReward = opt_proTx->scriptReward; - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); - } - if (!dmn->pdmnState->IsShared()) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-proupsharedreg-not-shared"); - } + // CheckProUpShareTx ran against pindexPrev, so a shared registrar update earlier in + // this block is invisible to it and could have moved the voting key onto this share's + // scripts. Recheck the separation rule against the state as rebuilt so far. + if (!IsShareListVotingKeySafe(newState->shares, newState->keyIDVoting)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-proupshare-payee-reuse"); + } - auto newState = std::make_shared(*dmn->pdmnState); - const uint16_t old_version{static_cast(newState->nVersion)}; - const bool operator_changed{newState->pubKeyOperator != opt_proTx->pubKeyOperator}; - if (operator_changed) { - // Operator-key change semantics match ProUpRegTx: reset all operator related - // fields and put the MN into PoSe-banned state until a new ProUpServTx arrives - newState->ResetOperatorFields(); - newState->BanIfNotBanned(nHeight); - newState->pubKeyOperator = opt_proTx->pubKeyOperator; - } - newState->keyIDVoting = opt_proTx->keyIDVoting; - // Shared masternodes are always ExtAddr or later; restore the state version that - // ResetOperatorFields() cleared - if (!SetStateVersion(*newState, old_version, dmn->nType, state)) { - return false; - } - if (operator_changed) { - newState->pubKeyOperator.SetLegacy(false); - } + newList.UpdateMN(opt_proTx->proTxHash, newState); - // CheckProUpSharedRegTx ran against pindexPrev, so transactions earlier in this same - // block are invisible to it. Re-evaluate against the list as rebuilt so far: another - // update may have taken the operator key (under either encoding; the re-key in - // UpdateMN() would throw out of block assembly), and a share reward update may have - // moved a share script onto the new voting key. - if (operator_changed && newList.HasOperatorKeyUnderAnyScheme(opt_proTx->pubKeyOperator.Get(), - /*self=*/opt_proTx->proTxHash)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); - } - if (!IsShareListVotingKeySafe(newState->shares, newState->keyIDVoting)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-proupsharedreg-payee-reuse"); - } + if (debugLogs) { + LogPrintf("%s -- MN %s updated share at height %d: %s\n", __func__, opt_proTx->proTxHash.ToString(), + nHeight, opt_proTx->ToString()); + } + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SHARED_REGISTRAR) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } - newList.UpdateMN(opt_proTx->proTxHash, newState); + auto dmn = newList.GetMN(opt_proTx->proTxHash); + if (!dmn) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); + } + if (!dmn->pdmnState->IsShared()) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-proupsharedreg-not-shared"); + } - if (debugLogs) { - LogPrintf("%s -- MN %s updated shared registrar at height %d: %s\n", __func__, - opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + auto newState = std::make_shared(*dmn->pdmnState); + const uint16_t old_version{static_cast(newState->nVersion)}; + const bool operator_changed{newState->pubKeyOperator != opt_proTx->pubKeyOperator}; + if (operator_changed) { + // Operator-key change semantics match ProUpRegTx: reset all operator related + // fields and put the MN into PoSe-banned state until a new ProUpServTx arrives + newState->ResetOperatorFields(); + newState->BanIfNotBanned(nHeight); + newState->pubKeyOperator = opt_proTx->pubKeyOperator; + } + newState->keyIDVoting = opt_proTx->keyIDVoting; + // Shared masternodes are always ExtAddr or later; restore the state version that + // ResetOperatorFields() cleared + if (!SetStateVersion(*newState, old_version, dmn->nType, state)) { + return false; + } + if (operator_changed) { + newState->pubKeyOperator.SetLegacy(false); + } + + // CheckProUpSharedRegTx ran against pindexPrev, so transactions earlier in this same + // block are invisible to it. Re-evaluate against the list as rebuilt so far: another + // update may have taken the operator key (under either encoding; the re-key in + // UpdateMN() would throw out of block assembly), and a share reward update may have + // moved a share script onto the new voting key. + if (operator_changed && newList.HasOperatorKeyUnderAnyScheme(opt_proTx->pubKeyOperator.Get(), + /*self=*/opt_proTx->proTxHash)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); + } + if (!IsShareListVotingKeySafe(newState->shares, newState->keyIDVoting)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-proupsharedreg-payee-reuse"); + } + + newList.UpdateMN(opt_proTx->proTxHash, newState); + + if (debugLogs) { + LogPrintf("%s -- MN %s updated shared registrar at height %d: %s\n", __func__, + opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + } + } else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) { + const auto opt_qc = GetTxPayload(tx); + if (!opt_qc) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-payload"); + } + if (!opt_qc->commitment.IsNull()) { + const auto& llmq_params_opt = Params().GetLLMQ(opt_qc->commitment.llmqType); + if (!llmq_params_opt.has_value()) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-commitment-type"); } - } else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) { - const auto opt_qc = GetTxPayload(tx); - if (!opt_qc) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-payload"); + int qcnHeight = int(opt_qc->nHeight); + int quorumHeight = qcnHeight - (qcnHeight % llmq_params_opt->dkgInterval) + + int(opt_qc->commitment.quorumIndex); + auto pQuorumBaseBlockIndex = pindexPrev->GetAncestor(quorumHeight); + if (!pQuorumBaseBlockIndex || pQuorumBaseBlockIndex->GetBlockHash() != opt_qc->commitment.quorumHash) { + // we should actually never get into this case as validation should have caught it...but let's be sure + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-quorum-hash"); } - if (!opt_qc->commitment.IsNull()) { - const auto& llmq_params_opt = Params().GetLLMQ(opt_qc->commitment.llmqType); - if (!llmq_params_opt.has_value()) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-commitment-type"); - } - int qcnHeight = int(opt_qc->nHeight); - int quorumHeight = qcnHeight - (qcnHeight % llmq_params_opt->dkgInterval) + - int(opt_qc->commitment.quorumIndex); - auto pQuorumBaseBlockIndex = pindexPrev->GetAncestor(quorumHeight); - if (!pQuorumBaseBlockIndex || pQuorumBaseBlockIndex->GetBlockHash() != opt_qc->commitment.quorumHash) { - // we should actually never get into this case as validation should have caught it...but let's be sure - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-quorum-hash"); - } - // The commitment has already been validated at this point, so it's safe to use members of it + // The commitment has already been validated at this point, so it's safe to use members of it - const auto members = llmq::utils::GetAllQuorumMembers(opt_qc->commitment.llmqType, - {m_dmnman, m_qsnapman, m_chainman, - pQuorumBaseBlockIndex}); - HandleQuorumCommitment(opt_qc->commitment, members, debugLogs, newList); - } + const auto members = llmq::utils::GetAllQuorumMembers(opt_qc->commitment.llmqType, + {m_dmnman, m_qsnapman, m_chainman, + pQuorumBaseBlockIndex}); + HandleQuorumCommitment(opt_qc->commitment, members, debugLogs, newList); } } + return true; +} + +bool CSpecialTxProcessor::FinishListForBlock(const CBlock& block, gsl::not_null pindexPrev, + const CDeterministicMNList& prevList, bool debugLogs, + CDeterministicMNList newList, BlockValidationState& state, + CDeterministicMNList& mnListRet) const +{ + const int nHeight = pindexPrev->nHeight + 1; + auto payee = prevList.GetMNPayee(pindexPrev); + const bool isMNRewardReallocation{DeploymentActiveAfter(pindexPrev, m_consensus_params, Consensus::DEPLOYMENT_MN_RR)}; + // we skip the coinbase for (int i = 1; i < static_cast(block.vtx.size()); i++) { const CTransaction& tx = *block.vtx[i]; @@ -794,6 +804,56 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul return true; } +bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_null pindexPrev, + bool is_v24_active, const CDeterministicMNList& prevList, + const CCoinsViewCache& view, bool debugLogs, BlockValidationState& state, + CDeterministicMNList& mnListRet) +{ + CDeterministicMNList newList = BeginListForBlock(pindexPrev, prevList); + + // we skip the coinbase + for (int i = 1; i < static_cast(block.vtx.size()); i++) { + if (!ApplyTxToList(*block.vtx[i], pindexPrev, is_v24_active, view, debugLogs, newList, state)) { + return false; + } + } + + return FinishListForBlock(block, pindexPrev, prevList, debugLogs, newList, state, mnListRet); +} + +CMNListBuilder::CMNListBuilder(const CSpecialTxProcessor& processor, const CDeterministicMNList& prevList, + gsl::not_null pindexPrev, bool is_v24_active, + const CCoinsViewCache& view, bool debugLogs) : + m_processor{&processor}, + m_prev_list{prevList}, + m_pindex_prev{pindexPrev}, + m_is_v24_active{is_v24_active}, + m_view{&view}, + m_debug_logs{debugLogs}, + m_list{processor.BeginListForBlock(pindexPrev, prevList)} +{ +} + +CMNListBuilder::CMNListBuilder(const CSpecialTxProcessor& processor, gsl::not_null pindexPrev, + bool is_v24_active, const CCoinsViewCache& view, bool debugLogs) : + CMNListBuilder(processor, processor.m_dmnman.GetListForBlock(pindexPrev), pindexPrev, is_v24_active, view, + debugLogs) +{ + AssertLockHeld(::cs_main); +} + +bool CMNListBuilder::ApplyTx(const CTransaction& tx, BlockValidationState& state) +{ + return m_processor->ApplyTxToList(tx, m_pindex_prev, m_is_v24_active, *m_view, m_debug_logs, m_list, state); +} + +bool CMNListBuilder::Finish(const CBlock& block, BlockValidationState& state, CDeterministicMNList& mnListRet) const +{ + return m_processor->FinishListForBlock(block, m_pindex_prev, m_prev_list, m_debug_logs, m_list, state, mnListRet); +} + +void CSpecialTxProcessor::UpdatedMNListTip(gsl::not_null pindex) { m_dmnman.UpdatedBlockTip(pindex); } + bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(Chainstate& chainstate, const CChain& chain, const CBlock& block, const CBlockIndex* pindex, SpecialTxRules rules, const CCoinsViewCache& view, CAmount blockSubsidy, bool fJustCheck, diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index c90858d22062..7f220d21ab29 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -6,6 +6,7 @@ #define BITCOIN_EVO_SPECIALTXMAN_H #include +#include #include #include // IWYU pragma: export #include @@ -20,7 +21,6 @@ class CCbTx; class CChain; class CCoinsViewCache; class CCreditPoolManager; -class CDeterministicMNList; class CDeterministicMNManager; class CProDisTx; class CRangesSet; @@ -108,7 +108,23 @@ class CSpecialTxProcessor const CDeterministicMNList& prevList, const CCoinsViewCache& view, bool debugLogs, BlockValidationState& state, CDeterministicMNList& mnListRet); + /** Passthrough to CDeterministicMNManager::UpdatedBlockTip */ + void UpdatedMNListTip(gsl::not_null pindex); + private: + friend class CMNListBuilder; + + // The three phases of RebuildListFromBlock(): per-block updates, one provider transaction, and + // collateral spends plus payee bookkeeping once every transaction has been applied. + CDeterministicMNList BeginListForBlock(gsl::not_null pindexPrev, + const CDeterministicMNList& prevList) const; + bool ApplyTxToList(const CTransaction& tx, gsl::not_null pindexPrev, bool is_v24_active, + const CCoinsViewCache& view, bool debugLogs, CDeterministicMNList& newList, + BlockValidationState& state) const; + bool FinishListForBlock(const CBlock& block, gsl::not_null pindexPrev, + const CDeterministicMNList& prevList, bool debugLogs, CDeterministicMNList newList, + BlockValidationState& state, CDeterministicMNList& mnListRet) const; + bool CheckSpecialTxInner(const CChain* chain, const CTransaction& tx, const CBlockIndex* pindexPrev, SpecialTxRules rules, const CCoinsViewCache& view, const std::optional& indexes, bool check_sigs, TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); @@ -116,6 +132,38 @@ class CSpecialTxProcessor CAmount blockSubsidy, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); }; +/** + * Builds the masternode list for a block one transaction at a time, with the same result as + * CSpecialTxProcessor::RebuildListFromBlock() over the same transactions. Block assembly uses it to + * try a package on a copy and keep the copy only if every transaction in it applies, so conflicting + * transactions are left out instead of failing the whole template. Copying is cheap: the list is + * backed by persistent maps. + */ +class CMNListBuilder +{ +public: + CMNListBuilder(const CSpecialTxProcessor& processor, const CDeterministicMNList& prevList, + gsl::not_null pindexPrev, bool is_v24_active, const CCoinsViewCache& view, + bool debugLogs); + //! Starts from the list at pindexPrev, as BuildNewListFromBlock() does. + CMNListBuilder(const CSpecialTxProcessor& processor, gsl::not_null pindexPrev, + bool is_v24_active, const CCoinsViewCache& view, bool debugLogs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + + //! Same result, and same exceptions, as RebuildListFromBlock() processing this transaction next. + [[nodiscard]] bool ApplyTx(const CTransaction& tx, BlockValidationState& state); + //! Collateral spends and payee bookkeeping for the whole block. Does not modify the builder. + [[nodiscard]] bool Finish(const CBlock& block, BlockValidationState& state, CDeterministicMNList& mnListRet) const; + +private: + const CSpecialTxProcessor* m_processor; + CDeterministicMNList m_prev_list; + gsl::not_null m_pindex_prev; + bool m_is_v24_active; + const CCoinsViewCache* m_view; + bool m_debug_logs; + CDeterministicMNList m_list; +}; + /** * This helper does some trivial validations that doesn't depends on collateral and * masternode list. Use CheckPro*Tx for the full validation of transaction, diff --git a/src/governance/common.h b/src/governance/common.h index 8272fd5008c9..0078b8572e58 100644 --- a/src/governance/common.h +++ b/src/governance/common.h @@ -10,6 +10,7 @@ #include +#include #include #include @@ -33,6 +34,8 @@ namespace Governance { class Object { public: + static constexpr size_t MAX_SIGNATURE_SIZE{96}; + Object() = default; Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& nCollateralHash, const std::string& strDataHex); @@ -79,7 +82,7 @@ class Object obj.masternodeOutpoint ); if (!(s.GetType() & SER_GETHASH)) { - READWRITE(obj.vchSig); + READWRITE(LIMITED_VECTOR(obj.vchSig, MAX_SIGNATURE_SIZE)); } } }; diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 6e34c5c56e95..c64fb419227a 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -28,7 +28,7 @@ #include -const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-16"; +const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-17"; namespace { constexpr std::chrono::seconds GOVERNANCE_DELETION_DELAY{10min}; @@ -345,18 +345,25 @@ void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& insert_o LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Before trigger block, GetDataAsPlainString = %s, nObjectType = %d\n", Assert(govobj)->GetDataAsPlainString(), std23::to_underlying(govobj->GetObjectType())); + // Count the attempt before AddTrigger can reject it: MasternodeRateCheck allows any + // masternode without a rate-buffer entry, so a key whose triggers all fail AddTrigger + // would otherwise never be throttled. + MasternodeRateUpdate(*govobj); + if (govobj->GetObjectType() == GovernanceObject::TRIGGER && !m_superblocks.AddTrigger(govobj, nCachedBlockHeight)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- undo adding invalid trigger object: hash = %s\n", nHash.ToString()); govobj->PrepareDeletion(GetTime().count()); return; } + // Only objects we keep may be announced. Scheduling this before the AddTrigger + // check would make us re-announce, and serve on GETDATA, a trigger we just + // undid and marked for deletion. + ScheduleTriggerRelay(*govobj); + LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- %s new, received from peer %s\n", strHash, peer_str); RelayObject(*govobj); - // Update the rate buffer - MasternodeRateUpdate(*govobj); - m_mn_sync.BumpAssetLastTime("CGovernanceManager::AddGovernanceObject"); // WE MIGHT HAVE PENDING/ORPHAN VOTES FOR THIS OBJECT @@ -705,15 +712,23 @@ void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) const COutPoint& masternodeOutpoint = govobj.GetMasternodeOutpoint(); auto it = mapLastMasternodeObject.try_emplace(masternodeOutpoint, /*fStatusOKIn=*/true).first; - int64_t nTimestamp = govobj.GetCreationTime(); - it->second.triggerBuffer.AddTimestamp(nTimestamp); + it->second.triggerBuffer.AddTimestamp(govobj.GetCreationTime()); + it->second.fStatusOK = true; +} + +void CGovernanceManager::ScheduleTriggerRelay(const CGovernanceObject& govobj) +{ + AssertLockHeld(cs_store); - if (nTimestamp > GetTime() + count_seconds(MAX_TIME_FUTURE_DEVIATION) - count_seconds(RELIABLE_PROPAGATION_TIME)) { - // schedule additional relay for the object + if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return; + + // A trigger created this close to the future-deviation limit is still too new for + // peers with a lagging clock to accept, so re-announce it once it has aged past + // RELIABLE_PROPAGATION_TIME (see CheckPostponedObjects). + if (govobj.GetCreationTime() > + GetTime() + count_seconds(MAX_TIME_FUTURE_DEVIATION) - count_seconds(RELIABLE_PROPAGATION_TIME)) { setAdditionalRelayObjects.insert(govobj.GetHash()); } - - it->second.fStatusOK = true; } bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus) diff --git a/src/governance/governance.h b/src/governance/governance.h index 33e7996def9a..04536cb5ef04 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -202,11 +202,8 @@ class GovernanceStore void Serialize(Stream &s) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store) { LOCK(cs_store); - // TODO: Remove the historical invalid-vote-cache field on the next disk-format version bump. - const CacheMap empty_invalid_votes{MAX_CACHE_SIZE}; s << SERIALIZATION_VERSION_STRING << mapErasedGovernanceObjects - << empty_invalid_votes << cmmapOrphanVotes << mapObjects << mapLastMasternodeObject @@ -225,10 +222,7 @@ class GovernanceStore return; } - // TODO: Stop consuming the historical invalid-vote-cache field on the next disk-format version bump. - CacheMap discarded_invalid_votes; s >> mapErasedGovernanceObjects - >> discarded_invalid_votes >> cmmapOrphanVotes >> mapObjects >> mapLastMasternodeObject @@ -404,6 +398,11 @@ class CGovernanceManager : public GovernanceStore void MasternodeRateUpdate(const CGovernanceObject& govobj) EXCLUSIVE_LOCKS_REQUIRED(cs_store); + /** Queue a deferred re-announcement for a trigger that is too new to propagate + * reliably yet. Only call this for triggers we are keeping. */ + void ScheduleTriggerRelay(const CGovernanceObject& govobj) + EXCLUSIVE_LOCKS_REQUIRED(cs_store); + bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed) EXCLUSIVE_LOCKS_REQUIRED(cs_store); diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp index 5235b881286d..c89e672a9196 100644 --- a/src/instantsend/instantsend.cpp +++ b/src/instantsend/instantsend.cpp @@ -134,16 +134,11 @@ void CInstantSendManager::TransactionIsRemoved(const CTransactionRef& tx) return; } + WITH_LOCK(cs_timingsTxSeen, timingsTxSeen.erase(tx->GetHash())); instantsend::InstantSendLockPtr islock = GetInstantSendLockByTxid(tx->GetHash()); if (islock == nullptr) { - // An unlocked asset unlock leaving the mempool (evicted by another instance of its - // withdrawal index, expired, or trimmed) is gone for good under this txid. It has no - // inputs, so the conflict cleanup keyed on spent outpoints never reaches its entry; - // drop it here or it would be re-queued for locking on every block. - if (tx->IsPlatformTransfer()) { - RemoveNonLockedTx(tx->GetHash(), false); - } + RemoveNonLockedTx(tx->GetHash(), false, /*keepMined=*/!tx->IsPlatformTransfer()); return; } @@ -211,7 +206,7 @@ void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx, const CBlock tx->GetHash().ToString(), pindexMined ? pindexMined->GetBlockHash().ToString() : ""); } -void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChildren) +void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChildren, bool keepMined) { LOCK(cs_nonLocked); @@ -220,6 +215,9 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChild return; } const auto& info = it->second; + if (keepMined && info.pindexMined) { + return; + } size_t retryChildrenCount = 0; if (retryChildren) { @@ -241,11 +239,16 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChild nonLockedTxs.erase(jt); } } - nonLockedTxsByOutpoints.erase(outpoint); + if (auto kt = nonLockedTxsByOutpoints.find(outpoint); + kt != nonLockedTxsByOutpoints.end() && kt->second == txid) { + nonLockedTxsByOutpoints.erase(kt); + } } } nonLockedTxs.erase(it); + // A tx that is no longer tracked won't be locked, so its seen time would never be consumed + WITH_LOCK(cs_timingsTxSeen, timingsTxSeen.erase(txid)); LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, retryChildren=%d, retryChildrenCount=%d\n", __func__, txid.ToString(), retryChildren, retryChildrenCount); diff --git a/src/instantsend/instantsend.h b/src/instantsend/instantsend.h index 4b18c5f1e6ff..2a451004bdae 100644 --- a/src/instantsend/instantsend.h +++ b/src/instantsend/instantsend.h @@ -101,8 +101,8 @@ class CInstantSendManager void AddNonLockedTx(const CTransactionRef& tx, const CBlockIndex* pindexMined) EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_timingsTxSeen); - void RemoveNonLockedTx(const uint256& txid, bool retryChildren) - EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry); + void RemoveNonLockedTx(const uint256& txid, bool retryChildren, bool keepMined = false) + EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry, !cs_timingsTxSeen); /** Queue every tracked unmined asset unlock for another locking attempt. Whether an unlock * may be locked depends on the tip (its height window and the credit pool limit), so this * runs on each connected block. */ @@ -141,7 +141,7 @@ class CInstantSendManager instantsend::InstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid) const; void TransactionIsRemoved(const CTransactionRef& tx) - EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache, !cs_nonLocked, !cs_pendingRetry); + EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache, !cs_nonLocked, !cs_pendingRetry, !cs_timingsTxSeen); void RemoveConflictingLock(const uint256& islockHash, const instantsend::InstantSendLock& islock) EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache); void TryEmplacePendingLock(const uint256& hash, const NodeId id, const instantsend::InstantSendLockPtr& islock) @@ -165,7 +165,7 @@ class CInstantSendManager bool IsInstantSendEnabled() const; Uint256HashMap RemoveConfirmedInstantSendLocks(const CBlockIndex* pindex) - EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry); + EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry, !cs_timingsTxSeen); }; } // namespace llmq diff --git a/src/instantsend/net_instantsend.cpp b/src/instantsend/net_instantsend.cpp index 6c51289ad749..6fb5dbcc9e7a 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -127,12 +127,6 @@ std::unique_ptr NetInstantSend::BuildVeri auto id = islock->GetRequestId(); - // no need to verify an ISLOCK if we already have verified the recovered sig that belongs to it - if (m_sigman.HasRecoveredSig(llmq_params.type, id, islock->txid)) { - data->alreadyVerified++; - continue; - } - auto cycleHeightOpt = GetBlockHeight(m_is_manager, m_chainman.ActiveChainstate(), islock->cycleHash); if (!cycleHeightOpt) { data->batchVerifier.badSources.emplace(nodeId); @@ -156,6 +150,13 @@ std::unique_ptr NetInstantSend::BuildVeri data->batchVerifier.badMessages.emplace(hash); continue; } + llmq::CRecoveredSig recovered_sig; + if (m_sigman.GetRecoveredSig(llmq_params.type, id, islock->txid, recovered_sig) && + recovered_sig.getQuorumHash() == quorum->qc->quorumHash && recovered_sig.sig.Get() == sig) { + data->alreadyVerified++; + continue; + } + uint256 signHash = llmq::SignHash{llmq_params.type, quorum->qc->quorumHash, id, islock->txid}.Get(); data->batchVerifier.PushMessage(nodeId, hash, signHash, sig, quorum->qc->quorumPublicKey); data->verifyCount++; diff --git a/src/instantsend/signing.cpp b/src/instantsend/signing.cpp index bf5749a174a3..2e321e3074c1 100644 --- a/src/instantsend/signing.cpp +++ b/src/instantsend/signing.cpp @@ -33,6 +33,25 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe using node::GetTransaction; namespace instantsend { +namespace { +bool IsProviderTx(const CTransaction& tx) +{ + if (!tx.IsSpecialTxVersion()) return false; + switch (tx.nType) { + case TRANSACTION_PROVIDER_REGISTER: + case TRANSACTION_PROVIDER_UPDATE_SERVICE: + case TRANSACTION_PROVIDER_UPDATE_REGISTRAR: + case TRANSACTION_PROVIDER_UPDATE_REVOKE: + case TRANSACTION_PROVIDER_DISSOLVE: + case TRANSACTION_PROVIDER_UPDATE_SHARE: + case TRANSACTION_PROVIDER_UPDATE_SHARED_REGISTRAR: + return true; + default: + return false; + } +} +} // anonymous namespace + InstantSendSigner::InstantSendSigner(const ChainstateManager& chainman, const chainlock::Chainlocks& chainlocks, llmq::CInstantSendManager& isman, llmq::CSigningManager& sigman, llmq::CSigSharesManager& shareman, llmq::CQuorumManager& qman, @@ -173,6 +192,13 @@ bool InstantSendSigner::CheckCanLock(const CTransaction& tx, bool printDebug, co if (tx.IsPlatformTransfer()) { return CheckCanLockAssetUnlock(tx, printDebug); } + if (IsProviderTx(tx)) { + if (printDebug) { + LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: provider txs are not locked\n", __func__, + tx.GetHash().ToString()); + } + return false; + } if (tx.vin.empty()) { // can't lock TXs without inputs (e.g. quorum commitments) return false; @@ -424,6 +450,8 @@ bool InstantSendSigner::TrySignInputLocks(const CTransaction& tx, bool fRetroact void InstantSendSigner::TrySignInstantSendLock(const CTransaction& tx) { + if (IsProviderTx(tx)) return; + const auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; InstantSendLock islock; diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index e42bfe09d29d..e99d7db8a719 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -43,6 +43,7 @@ struct NodeContext; namespace wallet { class CCoinControl; class CWallet; +enum class AddressPurpose; enum isminetype : unsigned int; enum class RescanStatus : uint8_t; struct CRecipient; @@ -172,7 +173,7 @@ class Wallet virtual bool haveWatchOnly() = 0; //! Add or update address. - virtual bool setAddressBook(const CTxDestination& dest, const std::string& name, const std::string& purpose) = 0; + virtual bool setAddressBook(const CTxDestination& dest, const std::string& name, const std::optional& purpose) = 0; // Remove address. virtual bool delAddressBook(const CTxDestination& dest) = 0; @@ -181,7 +182,7 @@ class Wallet virtual bool getAddress(const CTxDestination& dest, std::string* name, wallet::isminetype* is_mine, - std::string* purpose) = 0; + wallet::AddressPurpose* purpose) = 0; //! Get wallet address list. virtual std::vector getAddresses() const = 0; @@ -398,7 +399,7 @@ class Wallet using AddressBookChangedFn = std::function; virtual std::unique_ptr handleAddressBookChanged(AddressBookChangedFn fn) = 0; @@ -491,11 +492,11 @@ struct WalletAddress { CTxDestination dest; wallet::isminetype is_mine; + wallet::AddressPurpose purpose; std::string name; - std::string purpose; - WalletAddress(CTxDestination dest, wallet::isminetype is_mine, std::string name, std::string purpose) - : dest(std::move(dest)), is_mine(is_mine), name(std::move(name)), purpose(std::move(purpose)) + WalletAddress(CTxDestination dest, wallet::isminetype is_mine, wallet::AddressPurpose purpose, std::string name) + : dest(std::move(dest)), is_mine(is_mine), purpose(std::move(purpose)), name(std::move(name)) { } }; diff --git a/src/llmq/net_dkg.cpp b/src/llmq/net_dkg.cpp index a0c82915d96b..da1f2481a0b4 100644 --- a/src/llmq/net_dkg.cpp +++ b/src/llmq/net_dkg.cpp @@ -851,7 +851,7 @@ void NetDKG::HandleDKGRound(ActiveDKGSessionHandler& handler) active.dkgdbgman.MarkPhaseAdvanced(handler.params.type, handler.QuorumIndex(), QuorumPhase::Initialized); - auto* curSession = handler.GetCurSession(); + const auto curSession = handler.GetCurSession(); if (handler.params.is_single_member()) { auto finalCommitment = curSession->FinalizeSingleCommitment(); if (!finalCommitment.IsNull()) { // it can be null only if we are not member diff --git a/src/llmq/net_signing.cpp b/src/llmq/net_signing.cpp index 32cca59ac834..2e70d79d520b 100644 --- a/src/llmq/net_signing.cpp +++ b/src/llmq/net_signing.cpp @@ -417,7 +417,7 @@ void NetSigning::ProcessPendingSigShares( // we didn't check this earlier because we use a lazy BLS signature and tried to avoid doing the expensive // deserialization in the message thread if (!sig.IsValid()) { - BanNode(nodeId); + batchVerifier.badSources.emplace(nodeId); // don't process any additional shares from this node break; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 53e4834bdbcf..c41876bc606c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4221,6 +4221,15 @@ void PeerManagerImpl::ProcessMessage( m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); } + // Announce our sporks to every new peer so a node that lost or never had them + // picks them up without having to ask. Like spork relay, this includes masternode + // connections, which only become relay-capable after MNAUTH. + if (m_chainparams.IsTestChain()) { + for (const auto& spork : m_sporkman.ActiveSporks()) { + PushInv(*peer, CInv{MSG_SPORK, spork.GetHash()}); + } + } + if (!RejectIncomingTxs(pfrom)) { // Tell our peer that he should send us CoinJoin queue messages m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDDSQUEUE, true)); diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 7009839ab8bc..c6e12fb2a24a 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -215,13 +216,26 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc pblock->nTime = TicksSinceEpoch(NodeClock::now()); m_lock_time_cutoff = pindexPrev->GetMedianTimePast(); + // The masternode list as built by the transactions selected so far. Provider transactions are + // applied to it as they are selected, so the ones that do not fit the list are left out. + std::optional mn_list_builder; if (fDIP0003Active_context) { + // No list-transition logs: a package's trial may still be discarded, and block connection + // logs the transitions that actually happen. + mn_list_builder.emplace(*m_chain_helper.special_tx, pindexPrev, fV24Active_context, m_chainstate.CoinsTip(), + /*debugLogs=*/false); for (const Consensus::LLMQParams& params : llmq::GetEnabledQuorumParams(m_chainstate.m_chainman, pindexPrev)) { std::vector vqcTx; if (m_quorum_block_processor.GetMineableCommitmentsTx(params, nHeight, vqcTx)) { for (const auto& qcTx : vqcTx) { + BlockValidationState state; + if (!mn_list_builder->ApplyTx(*qcTx, state)) { + throw std::runtime_error( + strprintf("%s: quorum commitment does not apply to the masternode list: %s", __func__, + state.ToString())); + } pblock->vtx.emplace_back(qcTx); pblocktemplate->vTxFees.emplace_back(0); pblocktemplate->vTxSigOps.emplace_back(0); @@ -236,7 +250,8 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc int nDescendantsUpdated = 0; if (m_mempool) { LOCK(m_mempool->cs); - addPackageTxs(*m_mempool, nPackagesSelected, nDescendantsUpdated, pindexPrev); + addPackageTxs(*m_mempool, nPackagesSelected, nDescendantsUpdated, pindexPrev, + mn_list_builder ? &*mn_list_builder : nullptr); } const auto time_1{SteadyClock::now()}; @@ -283,9 +298,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc BlockValidationState state; CDeterministicMNList mn_list; - const bool is_v24_active{DeploymentActiveAfter(pindexPrev, m_chainstate.m_chainman, Consensus::DEPLOYMENT_V24)}; - if (!m_chain_helper.special_tx->BuildNewListFromBlock(*pblock, pindexPrev, is_v24_active, - m_chainstate.CoinsTip(), true, state, mn_list)) { + if (!mn_list_builder->Finish(*pblock, state, mn_list)) { throw std::runtime_error(strprintf("%s: BuildNewListFromBlock failed: %s", __func__, state.ToString())); } if (!CalcCbTxMerkleRootMNList(cbTx.merkleRootMNList, mn_list.to_sml(), state)) { @@ -495,7 +508,8 @@ void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::ve // Each time through the loop, we compare the best transaction in // mapModifiedTxs with the next transaction in the mempool to decide what // transaction package to work on next. -void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated, const CBlockIndex* const pindexPrev) +void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated, + const CBlockIndex* const pindexPrev, CMNListBuilder* mn_list_builder) { AssertLockHeld(mempool.cs); @@ -510,6 +524,16 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // This map with signals is used only to find duplicates auto signals = m_chain_helper.ehf_manager->GetSignalsStage(pindexPrev); const SpecialTxRules special_tx_rules{GetSpecialTxRules(pindexPrev, m_chainstate.m_chainman)}; + // Masternodes whose operator key a transaction already in the block changes or revokes. A + // ProUpServTx of theirs must not follow: one left out of the key change's package, whether it + // could not be put first, did not fit or did not pass TestPackageTransactions() then (which can + // change while the template is built), may come up later. + std::set keyChangedMNs; + const auto serviceUpdateTarget = [](const CTransaction& tx) -> std::optional { + if (tx.nType != TRANSACTION_PROVIDER_UPDATE_SERVICE) return std::nullopt; + const auto opt_proTx = GetTxPayload(tx); + return opt_proTx ? std::make_optional(opt_proTx->proTxHash) : std::nullopt; + }; // mapModifiedTx will store sorted packages after they are modified // because some of their txs are already in the block @@ -616,6 +640,50 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele onlyUnconfirmed(ancestors); ancestors.insert(iter); + // A pending ProUpServTx signed with the operator key that a key change in this package + // replaces must not follow the key change in the block: it would restore the previous + // operator's service fields. Mine such updates first. One that cannot be in this block now, + // whose package block assembly could not reliably put first (the mempool rejects those on + // entry, but a reorg can return such ancestors later), or whose package has an update of a + // masternode whose key already changed in the block, is left out: keyChangedMNs keeps it out + // of the rest of the block. If those pulled in do not fit, they are all left out instead of + // holding the key change back, which pending updates could otherwise do forever. + CTxMemPool::setEntries serviceUpdates; + for (const auto& entry : ancestors) { + for (const auto& update : mempool.GetServiceUpdatesBeforeKeyChange(entry)) { + if (inBlock.count(update) || serviceUpdates.count(update)) { + continue; + } + auto updatePackage{mempool.AssumeCalculateMemPoolAncestors(__func__, *update, + CTxMemPool::Limits::NoLimits(), + /*fSearchForParents=*/false)}; + onlyUnconfirmed(updatePackage); + updatePackage.insert(update); + const bool followsKeyChange{std::any_of(updatePackage.begin(), updatePackage.end(), + [&](const CTxMemPool::txiter& it) { + const auto target{serviceUpdateTarget(it->GetTx())}; + return target && keyChangedMNs.count(*target); + })}; + if (!followsKeyChange && TestPackageTransactions(updatePackage) && + !mempool.IsUnorderableServiceUpdate(update->GetTx(), updatePackage)) { + serviceUpdates.insert(updatePackage.begin(), updatePackage.end()); + } + } + } + if (!serviceUpdates.empty()) { + uint64_t extraSize{0}; + unsigned int extraSigOps{0}; + for (const auto& entry : serviceUpdates) { + if (!ancestors.count(entry)) { + extraSize += entry->GetTxSize(); + extraSigOps += entry->GetSigOpCount(); + } + } + if (!TestPackage(packageSize + extraSize, packageSigOps + extraSigOps)) { + serviceUpdates.clear(); + } + } + // Test if all tx's are Final and safe if (!TestPackageTransactions(ancestors)) { if (fUsingModified) { @@ -625,15 +693,38 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele continue; } - // Package can be added. Sort the entries in a valid order. + // Package can be added. Sort the entries in a valid order, the service updates first. std::vector sortedEntries; - SortForBlock(ancestors, sortedEntries); + SortForBlock(serviceUpdates, sortedEntries); + CTxMemPool::setEntries rest; + for (const auto& entry : ancestors) { + if (!serviceUpdates.count(entry)) { + rest.insert(entry); + } + } + std::vector sortedRest; + SortForBlock(rest, sortedRest); + sortedEntries.insert(sortedEntries.end(), sortedRest.begin(), sortedRest.end()); + ancestors.insert(serviceUpdates.begin(), serviceUpdates.end()); auto packageSignals = signals; + std::vector packageKeyChangedMNs; std::vector creditPoolTransactions; bool validPackage{true}; for (const auto& entry : sortedEntries) { const auto& tx = entry->GetTx(); + if (const auto target = serviceUpdateTarget(tx); + target && (keyChangedMNs.count(*target) || + std::find(packageKeyChangedMNs.begin(), packageKeyChangedMNs.end(), *target) != + packageKeyChangedMNs.end())) { + LogPrintf("%s: package tx %s skipped, it would follow a key change of masternode %s\n", __func__, + tx.GetHash().ToString(), target->ToString()); + validPackage = false; + break; + } + if (const auto target = mempool.GetKeyChangeTarget(*entry)) { + packageKeyChangedMNs.push_back(*target); + } if (std::optional signal = extractEHFSignal(tx); signal != std::nullopt) { if (!packageSignals.emplace(*signal, 0).second) { LogPrintf("%s: package tx %s skipped due to duplicate EHF signal %d\n", __func__, @@ -661,6 +752,34 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele } } + // Provider transactions are checked against the tip on entry, so two of them can each be + // valid alone and still conflict in one block. Apply the package to a copy of the list as + // built so far and skip it if any of its transactions does not apply, instead of producing a + // template whose masternode list cannot be built. + std::optional package_mn_list; + if (validPackage && mn_list_builder != nullptr && + std::any_of(sortedEntries.begin(), sortedEntries.end(), + [](const auto& entry) { return entry->GetTx().IsSpecialTxVersion(); })) { + package_mn_list.emplace(*mn_list_builder); + for (const auto& entry : sortedEntries) { + const auto& tx = entry->GetTx(); + BlockValidationState state; + std::string error; + try { + if (!package_mn_list->ApplyTx(tx, state)) error = state.ToString(); + } catch (const std::runtime_error& e) { + // AddMN()/UpdateMN() report a unique-property conflict by throwing + error = e.what(); + } + if (!error.empty()) { + LogPrintf("%s: package tx %s skipped, does not apply to the masternode list: %s\n", __func__, + tx.GetHash().ToString(), error); + validPackage = false; + break; + } + } + } + if (validPackage && creditPoolDiff != std::nullopt && !creditPoolTransactions.empty()) { TxValidationState state; if (!creditPoolDiff->ProcessLockUnlockTransactions(creditPoolTransactions, state)) { @@ -680,6 +799,10 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // This transaction will make it in; reset the failed counter. nConsecutiveFailed = 0; signals = std::move(packageSignals); + keyChangedMNs.insert(packageKeyChangedMNs.begin(), packageKeyChangedMNs.end()); + if (package_mn_list) { + *mn_list_builder = std::move(*package_mn_list); + } for (size_t i = 0; i < sortedEntries.size(); ++i) { AddToBlock(sortedEntries[i]); diff --git a/src/node/miner.h b/src/node/miner.h index 18d27b8e834a..5bc649b33c82 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -26,6 +26,7 @@ class CChainParams; class CChainstateHelper; class CConnman; class CEvoDB; +class CMNListBuilder; class CScript; struct LLMQContext; @@ -202,9 +203,12 @@ class BlockAssembler // Methods for how to add transactions to a block. /** Add transactions based on feerate including unconfirmed ancestors * Increments nPackagesSelected / nDescendantsUpdated with corresponding - * statistics from the package selection (for logging statistics). */ + * statistics from the package selection (for logging statistics). + * Provider transactions are applied to mn_list_builder (if set) and packages that do not + * apply are skipped. */ void addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated, - const CBlockIndex* pindexPrev) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, mempool.cs); + const CBlockIndex* pindexPrev, CMNListBuilder* mn_list_builder) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main, mempool.cs); // helper functions for addPackageTxs() /** Remove confirmed (inBlock) entries from given set */ diff --git a/src/node/sync_manager.cpp b/src/node/sync_manager.cpp index 2f11b9f3cb31..5d170e79a2d9 100644 --- a/src/node/sync_manager.cpp +++ b/src/node/sync_manager.cpp @@ -201,17 +201,6 @@ void SyncManager::ProcessTick() continue; } - // SPORK : ALWAYS ASK FOR SPORKS AS WE SYNC - - if (!m_netfulfilledman.HasFulfilledRequest(pnode->addr, "spork-sync")) { - // always get sporks first, only request once from each peer - m_netfulfilledman.AddFulfilledRequest(pnode->addr, "spork-sync"); - // get current network sporks - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::GETSPORKS)); - LogPrint(BCLog::MNSYNC, "Sync Tick -- nTick %d asset_id %d -- requesting sporks from peer=%d\n", nTick, - asset_id, pnode->GetId()); - } - if (asset_id == MASTERNODE_SYNC_BLOCKCHAIN) { int64_t nTimeSyncTimeout = snap.Nodes().size() > 3 ? MASTERNODE_SYNC_TICK_SECONDS : MASTERNODE_SYNC_TIMEOUT_SECONDS; diff --git a/src/psbt.h b/src/psbt.h index a79a8f3a1456..f12e96299ef9 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -822,8 +822,13 @@ struct PartiallySignedTransaction inputs.push_back(input); // Make sure the non-witness utxo matches the outpoint - if (input.non_witness_utxo && input.non_witness_utxo->GetHash() != tx->vin[i].prevout.hash) { - throw std::ios_base::failure("Non-witness UTXO does not match outpoint hash"); + if (input.non_witness_utxo) { + if (input.non_witness_utxo->GetHash() != tx->vin[i].prevout.hash) { + throw std::ios_base::failure("Non-witness UTXO does not match outpoint hash"); + } + if (tx->vin[i].prevout.n >= input.non_witness_utxo->vout.size()) { + throw std::ios_base::failure("Input specifies output index that does not exist"); + } } ++i; } diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index aae13a4a8ff9..ce13a7701743 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -9,6 +9,7 @@ #include #include +#include #include @@ -52,17 +53,16 @@ struct AddressTableEntryLessThan }; /* Determine address type from address purpose */ -static AddressTableEntry::Type translateTransactionType(const QString &strPurpose, bool isMine) +static AddressTableEntry::Type translateTransactionType(wallet::AddressPurpose purpose, bool isMine) { - AddressTableEntry::Type addressType = AddressTableEntry::Hidden; // "refund" addresses aren't shown, and change addresses aren't returned by getAddresses at all. - if (strPurpose == "send") - addressType = AddressTableEntry::Sending; - else if (strPurpose == "receive") - addressType = AddressTableEntry::Receiving; - else if (strPurpose == "unknown" || strPurpose == "") // if purpose not set, guess - addressType = (isMine ? AddressTableEntry::Receiving : AddressTableEntry::Sending); - return addressType; + switch (purpose) { + case wallet::AddressPurpose::SEND: return AddressTableEntry::Sending; + case wallet::AddressPurpose::RECEIVE: return AddressTableEntry::Receiving; + case wallet::AddressPurpose::REFUND: return AddressTableEntry::Hidden; + // No default case to allow for compiler to warn + } + assert(false); } // Private implementation @@ -82,7 +82,7 @@ class AddressTablePriv for (const auto& address : wallet.getAddresses()) { AddressTableEntry::Type addressType = translateTransactionType( - QString::fromStdString(address.purpose), address.is_mine); + address.purpose, address.is_mine); cachedAddressTable.append(AddressTableEntry(addressType, QString::fromStdString(address.name), QString::fromStdString(EncodeDestination(address.dest)))); @@ -94,7 +94,7 @@ class AddressTablePriv std::sort(cachedAddressTable.begin(), cachedAddressTable.end(), AddressTableEntryLessThan()); } - void updateEntry(const QString &address, const QString &label, bool isMine, const QString &purpose, int status) + void updateEntry(const QString &address, const QString &label, bool isMine, wallet::AddressPurpose purpose, int status) { // Find address / label in model QList::iterator lower = std::lower_bound( @@ -236,7 +236,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value, if(!index.isValid()) return false; AddressTableEntry *rec = static_cast(index.internalPointer()); - std::string strPurpose = (rec->type == AddressTableEntry::Sending ? "send" : "receive"); + wallet::AddressPurpose purpose = rec->type == AddressTableEntry::Sending ? wallet::AddressPurpose::SEND : wallet::AddressPurpose::RECEIVE; editStatus = OK; if(role == Qt::EditRole) @@ -250,7 +250,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value, editStatus = NO_CHANGES; return false; } - walletModel->wallet().setAddressBook(curAddress, value.toString().toStdString(), strPurpose); + walletModel->wallet().setAddressBook(curAddress, value.toString().toStdString(), purpose); } else if(index.column() == Address) { CTxDestination newAddress = DecodeDestination(value.toString().toStdString()); // Refuse to set invalid address, set error status and return false @@ -279,7 +279,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value, // Remove old entry walletModel->wallet().delAddressBook(curAddress); // Add new entry with new address - walletModel->wallet().setAddressBook(newAddress, value.toString().toStdString(), strPurpose); + walletModel->wallet().setAddressBook(newAddress, value.toString().toStdString(), purpose); } } return true; @@ -331,7 +331,7 @@ QModelIndex AddressTableModel::index(int row, int column, const QModelIndex &par } void AddressTableModel::updateEntry(const QString &address, - const QString &label, bool isMine, const QString &purpose, int status) + const QString &label, bool isMine, wallet::AddressPurpose purpose, int status) { // Update address book model from Dash core priv->updateEntry(address, label, isMine, purpose, status); @@ -361,7 +361,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con } } // Add entry - walletModel->wallet().setAddressBook(DecodeDestination(strAddress), strLabel, "send"); + walletModel->wallet().setAddressBook(DecodeDestination(strAddress), strLabel, wallet::AddressPurpose::SEND); } else if(type == Receive) { @@ -412,18 +412,18 @@ QString AddressTableModel::labelForAddress(const QString &address) const return QString(); } -QString AddressTableModel::purposeForAddress(const QString &address) const +std::optional AddressTableModel::purposeForAddress(const QString &address) const { - std::string purpose; + wallet::AddressPurpose purpose; if (getAddressData(address, /* name= */ nullptr, &purpose)) { - return QString::fromStdString(purpose); + return purpose; } - return QString(); + return std::nullopt; } bool AddressTableModel::getAddressData(const QString &address, std::string* name, - std::string* purpose) const { + wallet::AddressPurpose* purpose) const { CTxDestination destination = DecodeDestination(address.toStdString()); return walletModel->wallet().getAddress(destination, name, /* is_mine= */ nullptr, purpose); } diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h index 54eda1fed79a..b80e52a274fa 100644 --- a/src/qt/addresstablemodel.h +++ b/src/qt/addresstablemodel.h @@ -7,6 +7,8 @@ #include