diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index f12b45b49354..9c5fbc71d32a 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -26,7 +26,11 @@ CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block) : prefilledtxn[0] = {0, block.vtx[0]}; for (size_t i = 1; i < block.vtx.size(); i++) { const CTransaction& tx = *block.vtx[i]; - shorttxids[i - 1] = GetShortID(tx.GetHash()); + // Short IDs are computed from instance hashes so that a mempool entry holding a different + // re-signed instance of a version 2 asset unlock (same txid, different quorum signing + // info) is treated as missing and requested, instead of being spliced into the block and + // failing the coinbase asset unlock commitment. + shorttxids[i - 1] = GetShortID(tx.GetInstanceHash()); } } diff --git a/src/core_write.cpp b/src/core_write.cpp index e94b0af4add7..fe45f3eca039 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -324,6 +324,9 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry if (const auto opt_assetUnlockTx = GetTxPayload(tx)) { entry.pushKV("assetUnlockTx", opt_assetUnlockTx->ToJson()); } + if (IsAssetUnlockWithStableTxid(tx)) { + entry.pushKV("instanceHash", tx.GetInstanceHash().ToString()); + } } if (have_undo) { diff --git a/src/evo/assetlocktx.cpp b/src/evo/assetlocktx.cpp index 9388e1c29b15..d8d9a27fbaaa 100644 --- a/src/evo/assetlocktx.cpp +++ b/src/evo/assetlocktx.cpp @@ -175,7 +175,8 @@ bool CAssetUnlockPayload::VerifySig(const llmq::CQuorumManager& qman, const CCha template static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& verify_sig, const CTransaction& tx, gsl::not_null pindexPrev, - const std::optional& indexes, TxValidationState& state) + const std::optional& indexes, bool is_v24_active, + TxValidationState& state) { // Some checks depends from blockchain status also, such as `known indexes` and `withdrawal limits` // They are omitted here and done by CCreditPool @@ -200,6 +201,9 @@ static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& ver if (assetUnlockTx.getVersion() == 0 || assetUnlockTx.getVersion() > CAssetUnlockPayload::CURRENT_VERSION) { return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-version"); } + if (!is_v24_active && assetUnlockTx.getVersion() > CAssetUnlockPayload::INITIAL_VERSION) { + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-version-2"); + } if (indexes != std::nullopt && indexes->Contains(assetUnlockTx.getIndex())) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-duplicated-index"); @@ -214,30 +218,32 @@ static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& ver const CAssetUnlockPayload payload_copy{assetUnlockTx.getVersion(), assetUnlockTx.getIndex(), assetUnlockTx.getFee(), assetUnlockTx.getRequestedHeight(), assetUnlockTx.getQuorumHash(), CBLSSignature{}}; SetTxPayload(tx_copy, payload_copy); - uint256 msgHash = tx_copy.GetHash(); + // The signed message must commit to requestedHeight and quorumHash even though the version 2 + // txid excludes them, so hash the full serialization rather than using GetHash(). + uint256 msgHash = ::SerializeHash(tx_copy); return verify_sig(assetUnlockTx, msgHash, pindexPrev, state); } bool CheckAssetUnlockTx(const BlockManager& blockman, const llmq::CQuorumManager& qman, const CTransaction& tx, gsl::not_null pindexPrev, const std::optional& indexes, - TxValidationState& state) + bool is_v24_active, TxValidationState& state) { return CheckAssetUnlockTxImpl(blockman, [&](const CAssetUnlockPayload& payload, const uint256& msg_hash, const CBlockIndex* pindex, TxValidationState& tx_state) { return payload.VerifySig(qman, msg_hash, pindex, tx_state); - }, tx, pindexPrev, indexes, state); + }, tx, pindexPrev, indexes, is_v24_active, state); } bool CheckAssetUnlockTx(const BlockManager& blockman, const llmq::CQuorumManager& qman, const CChain& chain, const CTransaction& tx, gsl::not_null pindexPrev, - const std::optional& indexes, TxValidationState& state) + const std::optional& indexes, bool is_v24_active, TxValidationState& state) { AssertLockHeld(::cs_main); return CheckAssetUnlockTxImpl(blockman, [&](const CAssetUnlockPayload& payload, const uint256& msg_hash, const CBlockIndex* pindex, TxValidationState& tx_state) NO_THREAD_SAFETY_ANALYSIS { return payload.VerifySig(qman, chain, msg_hash, pindex, tx_state); - }, tx, pindexPrev, indexes, state); + }, tx, pindexPrev, indexes, is_v24_active, state); } bool GetAssetUnlockFee(const CTransaction& tx, CAmount& txfee, TxValidationState& state) diff --git a/src/evo/assetlocktx.h b/src/evo/assetlocktx.h index 3dea2c5e13a8..ae1165f402b0 100644 --- a/src/evo/assetlocktx.h +++ b/src/evo/assetlocktx.h @@ -75,8 +75,13 @@ class CAssetLockPayload class CAssetUnlockPayload { public: - static constexpr uint8_t CURRENT_VERSION = 1; + static constexpr uint8_t INITIAL_VERSION = 1; + /** Serialized identically to version 1, but the transaction hash excludes the quorum signing + * info (requestedHeight, quorumHash, quorumSig) so every re-signed instance of one withdrawal + * shares one txid; see IsAssetUnlockWithStableTxid(). Gated on DEPLOYMENT_V24. */ + static constexpr uint8_t CURRENT_VERSION = 2; static constexpr auto SPECIALTX_TYPE = TRANSACTION_ASSET_UNLOCK; + static_assert(CURRENT_VERSION >= ASSET_UNLOCK_STABLE_TXID_VERSION); static constexpr size_t MAXIMUM_WITHDRAWALS = 32; @@ -163,10 +168,10 @@ class CAssetUnlockPayload }; bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool is_v24_active); -bool CheckAssetUnlockTx(const node::BlockManager& blockman, const llmq::CQuorumManager& qman, const CTransaction& tx, gsl::not_null pindexPrev, const std::optional& indexes, TxValidationState& state); +bool CheckAssetUnlockTx(const node::BlockManager& blockman, const llmq::CQuorumManager& qman, const CTransaction& tx, gsl::not_null pindexPrev, const std::optional& indexes, bool is_v24_active, TxValidationState& state); bool CheckAssetUnlockTx(const node::BlockManager& blockman, const llmq::CQuorumManager& qman, const CChain& chain, const CTransaction& tx, gsl::not_null pindexPrev, - const std::optional& indexes, TxValidationState& state) + const std::optional& indexes, bool is_v24_active, TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool GetAssetUnlockFee(const CTransaction& tx, CAmount& txfee, TxValidationState& state); diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 856d8d74ee6c..ad9f6f2e8fef 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -21,7 +21,7 @@ using node::ReadBlockFromDisk; -bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationState& state) +bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, bool is_v24_active, TxValidationState& state) { if (cbTx.nVersion == CCbTx::Version::INVALID || cbTx.nVersion >= CCbTx::Version::UNKNOWN) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); @@ -41,6 +41,11 @@ bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationSta if ((isV20 && cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) || (!isV20 && cbTx.nVersion >= CCbTx::Version::CLSIG_AND_BALANCE)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); } + + if ((is_v24_active && cbTx.nVersion < CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS) || + (!is_v24_active && cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS)) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); + } } return true; @@ -147,11 +152,27 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre return true; } +uint256 CalcCbTxMerkleRootAssetUnlocks(const CBlock& block) +{ + // Instance hashes cover the quorum signing info that the txids of these transactions - and + // therefore the block's merkle root - exclude. Two instances of one withdrawal share a txid, + // so duplicate leaves imply a duplicate transaction, which the block merkle-root check + // (CheckMerkleRoot, run before this) already rejects; no mutated check is needed here. + std::vector instance_hashes; + for (const auto& tx : block.vtx) { + // The miner calls this while the coinbase slot is still an empty placeholder + if (tx && IsAssetUnlockWithStableTxid(*tx)) { + instance_hashes.push_back(tx->GetInstanceHash()); + } + } + return ComputeMerkleRoot(std::move(instance_hashes)); +} + std::string CCbTx::ToString() const { - return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, creditPoolBalance=%d.%08d)", + return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, creditPoolBalance=%d.%08d, merkleRootAssetUnlocks=%s)", static_cast(nVersion), nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString(), bestCLHeightDiff, bestCLSignature.ToString(), - creditPoolBalance / COIN, creditPoolBalance % COIN); + creditPoolBalance / COIN, creditPoolBalance % COIN, merkleRootAssetUnlocks.ToString()); } std::optional> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex) diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index da6d92f802a8..bb552a78c653 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -34,6 +34,7 @@ class CCbTx MERKLE_ROOT_MNLIST = 1, MERKLE_ROOT_QUORUMS = 2, CLSIG_AND_BALANCE = 3, + MERKLE_ROOT_ASSETUNLOCKS = 4, UNKNOWN, }; @@ -45,6 +46,10 @@ class CCbTx uint32_t bestCLHeightDiff{0}; CBLSSignature bestCLSignature; CAmount creditPoolBalance{0}; + /** Merkle root over the instance hashes of the block's version 2+ asset unlock transactions + * (block order; null when there are none). Their txids exclude the quorum signing info, so + * the block's merkle root does not commit to it; this root restores that commitment. */ + uint256 merkleRootAssetUnlocks; SERIALIZE_METHODS(CCbTx, obj) { @@ -56,6 +61,9 @@ class CCbTx READWRITE(COMPACTSIZE(obj.bestCLHeightDiff)); READWRITE(obj.bestCLSignature); READWRITE(obj.creditPoolBalance); + if (obj.nVersion >= Version::MERKLE_ROOT_ASSETUNLOCKS) { + READWRITE(obj.merkleRootAssetUnlocks); + } } } @@ -68,11 +76,12 @@ class CCbTx }; template<> struct is_serializable_enum : std::true_type {}; -bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationState& state); +bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, bool is_v24_active, TxValidationState& state); bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet, BlockValidationState& state); +uint256 CalcCbTxMerkleRootAssetUnlocks(const CBlock& block); std::optional> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex); diff --git a/src/evo/core_write.cpp b/src/evo/core_write.cpp index 4ebe78675f2d..3d97569ad215 100644 --- a/src/evo/core_write.cpp +++ b/src/evo/core_write.cpp @@ -143,6 +143,9 @@ UniValue CCbTx::ToJson() const ret.pushKV("bestCLHeightDiff", bestCLHeightDiff); ret.pushKV("bestCLSignature", bestCLSignature.ToString()); ret.pushKV("creditPoolBalance", ValueFromAmount(creditPoolBalance)); + if (nVersion >= CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS) { + ret.pushKV("merkleRootAssetUnlocks", merkleRootAssetUnlocks.ToString()); + } } } return ret; diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index 239b951a27d0..668e01a3d8f9 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -137,7 +137,11 @@ std::optional CCreditPoolManager::GetFromCache(const CBlockIndex& b void CCreditPoolManager::AddToCache(const uint256& block_hash, int height, const CCreditPool &pool) { - if (height % DISK_SNAPSHOT_PERIOD == 0) { + // The disk snapshot is an optimization; skip it outside a block-scoped EvoDB transaction + // (e.g. a pool constructed on a cold cache during mempool acceptance or template creation), + // where the write would never be committed and would trip the clean-transaction assertion + // at the next root commit. A skipped snapshot is reconstructed from an earlier one. + if (height % DISK_SNAPSHOT_PERIOD == 0 && evoDb.HasActiveTransaction()) { if (!evoDb.WriteDerived(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) { // A mismatch is local EvoDB corruption, not a statement about the // block. Abort here: some callers (miner, RPC) never pass through a diff --git a/src/evo/evodb.h b/src/evo/evodb.h index 40d37b48229d..11b47ed0ec6a 100644 --- a/src/evo/evodb.h +++ b/src/evo/evodb.h @@ -115,6 +115,16 @@ class CEvoDB std::unique_ptr BeginTransaction(EvoDbIdentity identity = EvoDbIdentity::NORMAL) EXCLUSIVE_LOCKS_REQUIRED(!cs); + /** Whether a block-scoped transaction is open. Writes performed outside one are never + * committed and trip the clean-transaction assertion at the next root commit, so callers + * reachable from transaction-less contexts (mempool acceptance, mining, RPC) must skip + * optional persistence when this is false. */ + bool HasActiveTransaction() const EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + LOCK(cs); + return active_transaction.has_value(); + } + CurTransaction& GetCurTransaction() EXCLUSIVE_LOCKS_REQUIRED(cs) { AssertLockHeld(cs); // lock must be held from outside as long as the DB transaction is used diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index d8b89b43a89b..26c122ee44d1 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -229,7 +229,7 @@ static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, llmq::CQuorumSn return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-invalid"); } if (const auto opt_cbTx = GetTxPayload(tx)) { - return CheckCbTx(*opt_cbTx, pindexPrev, state); + return CheckCbTx(*opt_cbTx, pindexPrev, DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_V24), state); } else { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-payload"); } @@ -241,9 +241,11 @@ static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, llmq::CQuorumSn CheckMNHFTx(chainman, qman, tx, pindexPrev, state); case TRANSACTION_ASSET_LOCK: return CheckAssetLockTx(tx, state, DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_V24)); - case TRANSACTION_ASSET_UNLOCK: - return chain ? CheckAssetUnlockTx(chainman.m_blockman, qman, *chain, tx, pindexPrev, indexes, state) : - CheckAssetUnlockTx(chainman.m_blockman, qman, tx, pindexPrev, indexes, state); + case TRANSACTION_ASSET_UNLOCK: { + const bool is_v24_active{DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_V24)}; + return chain ? CheckAssetUnlockTx(chainman.m_blockman, qman, *chain, tx, pindexPrev, indexes, is_v24_active, state) : + CheckAssetUnlockTx(chainman.m_blockman, qman, tx, pindexPrev, indexes, is_v24_active, state); + } } } catch (const std::exception& e) { LogPrintf("%s -- failed: %s\n", __func__, e.what()); @@ -714,7 +716,8 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(Chainstate& chainstate, const } if (opt_cbTx = GetTxPayload(*tx); opt_cbTx) { TxValidationState tx_state; - if (!CheckCbTx(*opt_cbTx, pindex->pprev, tx_state)) { + if (!CheckCbTx(*opt_cbTx, pindex->pprev, + DeploymentActiveAfter(pindex->pprev, m_chainman, Consensus::DEPLOYMENT_V24), tx_state)) { assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS || tx_state.GetResult() == TxValidationResult::TX_BAD_SPECIAL); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), diff --git a/src/instantsend/signing.cpp b/src/instantsend/signing.cpp index 73a42cd099c3..d14e7646a408 100644 --- a/src/instantsend/signing.cpp +++ b/src/instantsend/signing.cpp @@ -189,6 +189,12 @@ bool InstantSendSigner::CheckCanLock(const COutPoint& outpoint, bool printDebug, auto mempoolTx = m_mempool.get(outpoint.hash); if (mempoolTx) { + if (IsAssetUnlockWithStableTxid(*mempoolTx)) { + // An unmined version 2 asset unlock was quorum-signed, so the withdrawal is + // irreversible on Platform and will be re-signed until mined under this same txid. + // Spends of its outputs may therefore be locked before it is mined. + return outpoint.n < mempoolTx->vout.size(); + } if (printDebug) { LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: parent mempool TX %s is not locked\n", __func__, txHash.ToString(), outpoint.hash.ToString()); diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index 21824b2f687b..e8e33588762b 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -542,16 +542,27 @@ bool CSigningManager::ProcessRecoveredSig(const std::shared_ptrgetId(), otherRecoveredSig)) { auto otherSignHash = otherRecoveredSig.buildSignHash(); if (signHash.Get() != otherSignHash.Get()) { - // this should really not happen, as each masternode is participating in only one vote, - // even if it's a member of multiple quorums. so a majority is only possible on one quorum and one msgHash per id - LogPrintf("CSigningManager::%s -- conflicting recoveredSig for signHash=%s, id=%s, msgHash=%s, otherSignHash=%s\n", __func__, - signHash.ToString(), recoveredSig->getId().ToString(), recoveredSig->getMsgHash().ToString(), otherSignHash.ToString()); + if (llmqType == Params().GetConsensus().llmqTypePlatform) { + // Platform re-signs expired withdrawals under the same request id with a new + // message hash; the latest recovered sig supersedes the previous one. The + // truncate and the write below are separate batches; a crash in between only + // loses a sig that Platform will produce again on the next re-sign. + LogPrint(BCLog::LLMQ, "CSigningManager::%s -- replacing recoveredSig for platform signHash=%s, id=%s, msgHash=%s, otherSignHash=%s\n", __func__, + signHash.ToString(), recoveredSig->getId().ToString(), recoveredSig->getMsgHash().ToString(), otherSignHash.ToString()); + db.TruncateRecoveredSig(llmqType, recoveredSig->getId()); + } else { + // this should really not happen, as each masternode is participating in only one vote, + // even if it's a member of multiple quorums. so a majority is only possible on one quorum and one msgHash per id + LogPrintf("CSigningManager::%s -- conflicting recoveredSig for signHash=%s, id=%s, msgHash=%s, otherSignHash=%s\n", __func__, + signHash.ToString(), recoveredSig->getId().ToString(), recoveredSig->getMsgHash().ToString(), otherSignHash.ToString()); + return false; + } } else { // Looks like we're trying to process a recSig that is already known. This might happen if the same // recSig comes in through regular QRECSIG messages and at the same time through some other message // which allowed to reconstruct a recSig (e.g. ISLOCK). In this case, just bail out. + return false; } - return false; } else { // This case is very unlikely. It can only happen when cleanup caused this specific recSig to vanish // between the HasRecoveredSigForId and GetRecoveredSigById call. If that happens, treat it as if we diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 87b9864edbb9..ae693ec8c7c4 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -812,6 +812,9 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const ui LogPrintf("%s -- already voted for id=%s and msgHash=%s. Signing for different " /* Continued */ "msgHash=%s\n", __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); + // Drop any recovered sig for the previous message so the new signing session + // is not short-circuited by the by-id lookups in the share pipeline + sigman.TruncateRecoveredSig(llmqType, id); hasVoted = false; } else { LogPrintf("%s -- already voted for id=%s and msgHash=%s. Not voting on " /* Continued */ diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e25a9a1230c5..f838b0ddc152 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -669,10 +669,14 @@ class PeerManagerImpl final : public PeerManager void AddObjectAnnouncement(const CNode& node, const CInv& inv, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - /** Delete all announcements of a transaction across all peers, under both inv types it may + /** Delete all announcements of a transaction across all peers, under the inv types it may * have been announced with (MSG_TX and MSG_DSTX). */ void ForgetTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** As above, additionally forgetting the MSG_ASSET_UNLOCK announcement (by instance hash) + * used for version 2 asset unlocks. */ + void ForgetTx(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** Helper to process result of external handlers of message */ void PostProcessMessage(MessageProcessingResult&& ret, NodeId node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); @@ -1638,6 +1642,14 @@ void PeerManagerImpl::ForgetTx(const uint256& txid) m_object_request.ForgetTxHash(CInv(MSG_DSTX, txid)); } +void PeerManagerImpl::ForgetTx(const CTransaction& tx) +{ + ForgetTx(tx.GetHash()); + if (IsAssetUnlockWithStableTxid(tx)) { + m_object_request.ForgetTxHash(CInv(MSG_ASSET_UNLOCK, tx.GetInstanceHash())); + } +} + size_t PeerManagerImpl::GetRequestedObjectCount(NodeId nodeid) const { AssertLockHeld(cs_main); @@ -1871,7 +1883,7 @@ void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx) return; if (!vExtraTxnForCompact.size()) vExtraTxnForCompact.resize(max_extra_txn); - vExtraTxnForCompact[vExtraTxnForCompactIt] = std::make_pair(tx->GetHash(), tx); + vExtraTxnForCompact[vExtraTxnForCompactIt] = std::make_pair(tx->GetInstanceHash(), tx); vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % max_extra_txn; } @@ -2122,7 +2134,7 @@ void PeerManagerImpl::BlockConnected(const std::shared_ptr& pblock } for (const auto& ptx : pblock->vtx) { // Confirmed transactions no longer need to be requested. - ForgetTx(ptx->GetHash()); + ForgetTx(*ptx); } } @@ -2132,6 +2144,11 @@ void PeerManagerImpl::BlockConnected(const std::shared_ptr& pblock LOCK(m_recent_confirmed_transactions_mutex); for (const auto& ptx : pblock->vtx) { m_recent_confirmed_transactions.insert(ptx->GetHash()); + // Version 2 asset unlocks are announced and deduplicated by instance hash; record the + // mined instance so AlreadyHave() stops re-requesting it once it leaves the mempool. + if (IsAssetUnlockWithStableTxid(*ptx)) { + m_recent_confirmed_transactions.insert(ptx->GetInstanceHash()); + } } } @@ -2293,6 +2310,7 @@ bool PeerManagerImpl::AlreadyHave(const CInv& inv) { case MSG_TX: case MSG_DSTX: + case MSG_ASSET_UNLOCK: { if (m_chainman.ActiveChain().Tip()->GetBlockHash() != hashRecentRejectsChainTip) { @@ -2304,6 +2322,19 @@ bool PeerManagerImpl::AlreadyHave(const CInv& inv) m_recent_rejects.reset(); } + if (inv.IsMsgAssetUnlock()) { + // inv.hash is the instance hash of a version 2 asset unlock. Rejects and mined + // instances are tracked by instance hash, so a rejected or already-mined stale + // instance never blocks a fresher re-signed instance of the same withdrawal + // (which shares its txid) yet is not endlessly re-requested either. + if (WITH_LOCK(m_recent_confirmed_transactions_mutex, + return m_recent_confirmed_transactions.contains(inv.hash))) { + return true; + } + return m_recent_rejects.contains(inv.hash) || + m_mempool.GetAssetUnlockByInstanceHash(inv.hash) != nullptr; + } + if (m_orphanage.HaveTx(inv.hash)) return true; { @@ -2825,6 +2856,12 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& CTransactionRef PeerManagerImpl::FindTxForGetData(const CNode* peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) { auto txinfo = m_mempool.info(txid); + if (!txinfo.tx) { + // A MSG_ASSET_UNLOCK getdata identifies a version 2 asset unlock by its instance hash + if (const auto unlock_tx = m_mempool.GetAssetUnlockByInstanceHash(txid)) { + txinfo = m_mempool.info(unlock_tx->GetHash()); + } + } if (txinfo.tx) { // If a TX could have been INVed in reply to a MEMPOOL request, // or is older than UNCONDITIONAL_RELAY_DELAY, permit the request @@ -3536,6 +3573,18 @@ static bool CanAnnounceDstxTo(const CCoinJoinBroadcastTx& dstx, int peer_version peer_version >= COINJOIN_REBALANCE_VERSION; } +//! The inventory type and hash to announce a mempool transaction with to a peer at the given +//! negotiated protocol version. Version 2 asset unlocks are announced by instance hash (MSG_ASSET_UNLOCK) +//! so a re-signed instance of a withdrawal the peer already has (sharing its txid) is still announced; +//! a getdata for it is served as a plain `tx`. Older peers get MSG_TX and never learn of refreshes. +static std::pair GetTxAnnouncement(const CTransaction& tx, int peer_version) +{ + if (IsAssetUnlockWithStableTxid(tx) && peer_version >= ASSET_UNLOCK_INV_VERSION) { + return {MSG_ASSET_UNLOCK, tx.GetInstanceHash()}; + } + return {MSG_TX, tx.GetHash()}; +} + // do_return signals the caller to stop further processing of the DSTX. struct DSTXValidationResult { DSTXValidationScore score; @@ -4772,9 +4821,15 @@ void PeerManagerImpl::ProcessMessage( const CTransaction& tx = *ptx; const uint256& txid = ptx->GetHash(); - AddKnownInv(*peer, txid); - - CInv inv(nInvType, tx.GetHash()); + // Version 2 asset unlocks are announced, requested and deduplicated by instance hash so + // that a re-signed instance of a withdrawal already in the mempool (sharing its txid) + // still propagates. + const bool is_stable_unlock{IsAssetUnlockWithStableTxid(tx)}; + if (is_stable_unlock) nInvType = MSG_ASSET_UNLOCK; + const uint256& relay_hash{is_stable_unlock ? tx.GetInstanceHash() : txid}; + AddKnownInv(*peer, relay_hash); + + CInv inv(nInvType, relay_hash); { LOCK(cs_main); // A MSG_TX request may be answered with a DSTX message and vice versa (a getdata for @@ -4782,6 +4837,9 @@ void PeerManagerImpl::ProcessMessage( // type the request was tracked under. m_object_request.ReceivedResponse(pfrom.GetId(), CInv(MSG_TX, txid)); m_object_request.ReceivedResponse(pfrom.GetId(), CInv(MSG_DSTX, txid)); + if (is_stable_unlock) { + m_object_request.ReceivedResponse(pfrom.GetId(), CInv(MSG_ASSET_UNLOCK, relay_hash)); + } } // Process custom logic, no matter if tx will be accepted to mempool later or not @@ -4825,7 +4883,7 @@ void PeerManagerImpl::ProcessMessage( m_dstxman.AddDSTX(dstx); } - ForgetTx(tx.GetHash()); + ForgetTx(tx); _RelayTransaction(tx.GetHash()); m_orphanage.AddChildrenToWorkSet(tx, peer->m_id); @@ -4892,8 +4950,8 @@ void PeerManagerImpl::ProcessMessage( m_isman.TransactionIsRemoved(ptx); } } else { - m_recent_rejects.insert(tx.GetHash()); - ForgetTx(tx.GetHash()); + m_recent_rejects.insert(relay_hash); + ForgetTx(tx); if (RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); } @@ -6492,7 +6550,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) tx_relay->m_tx_inventory_to_send.erase(hash); if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; - int nInvType = MSG_TX; + auto [nInvType, announce_hash] = GetTxAnnouncement(*txinfo.tx, pto->GetCommonVersion()); // A DSTX this peer would reject as malformed is announced as a plain // transaction instead of being dropped: the peer still gets the transaction // (ProcessGetData serves a NetMsgType::TX for it), just without the mixing @@ -6500,8 +6558,8 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (const auto dstx = m_dstxman.GetDSTX(hash); dstx && CanAnnounceDstxTo(dstx, pto->GetCommonVersion())) { nInvType = MSG_DSTX; } - tx_relay->m_tx_inventory_known_filter.insert(hash); - queueAndMaybePushInv(CInv(nInvType, hash)); + tx_relay->m_tx_inventory_known_filter.insert(announce_hash); + queueAndMaybePushInv(CInv(nInvType, announce_hash)); const auto islock = m_isman.GetInstantSendLockByTxid(hash); if (islock == nullptr) continue; @@ -6550,17 +6608,17 @@ bool PeerManagerImpl::SendMessages(CNode* pto) uint256 hash = *it; // Remove it from the to-be-sent set tx_relay->m_tx_inventory_to_send.erase(it); - // Check if not in the filter already - if (tx_relay->m_tx_inventory_known_filter.contains(hash)) { - continue; - } // Not in the mempool anymore? don't bother sending it. auto txinfo = m_mempool.info(hash); if (!txinfo.tx) { continue; } + auto [nInvType, announce_hash] = GetTxAnnouncement(*txinfo.tx, pto->GetCommonVersion()); + // Check if not in the filter already + if (tx_relay->m_tx_inventory_known_filter.contains(announce_hash)) { + continue; + } if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; - int nInvType = MSG_TX; // See the mempool-request path above: a DSTX this peer would reject as // malformed is downgraded to a plain transaction announcement rather than // withheld, so pre-rebalance peers still receive it. @@ -6568,7 +6626,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) nInvType = MSG_DSTX; } // Send - State(pto->GetId())->m_recently_announced_invs.insert(hash); + State(pto->GetId())->m_recently_announced_invs.insert(announce_hash); nRelayedTransactions++; { // Expire old relay messages @@ -6578,13 +6636,13 @@ bool PeerManagerImpl::SendMessages(CNode* pto) g_relay_expiration.pop_front(); } - auto ret = mapRelay.emplace(hash, std::move(txinfo.tx)); + auto ret = mapRelay.emplace(announce_hash, std::move(txinfo.tx)); if (ret.second) { g_relay_expiration.emplace_back(current_time + RELAY_TX_CACHE_TIME, ret.first); } } - tx_relay->m_tx_inventory_known_filter.insert(hash); - queueAndMaybePushInv(CInv(nInvType, hash)); + tx_relay->m_tx_inventory_known_filter.insert(announce_hash); + queueAndMaybePushInv(CInv(nInvType, announce_hash)); } } } diff --git a/src/node/miner.cpp b/src/node/miner.cpp index f40d761d46a3..4579d08fc03c 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -199,6 +199,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc const bool fDIP0003Active_context{DeploymentActiveAfter(pindexPrev, chainparams.GetConsensus(), Consensus::DEPLOYMENT_DIP0003)}; const bool fDIP0008Active_context{DeploymentActiveAfter(pindexPrev, chainparams.GetConsensus(), Consensus::DEPLOYMENT_DIP0008)}; const bool fV20Active_context{DeploymentActiveAfter(pindexPrev, chainparams.GetConsensus(), Consensus::DEPLOYMENT_V20)}; + const bool fV24Active_context{DeploymentActiveAfter(pindexPrev, m_chainstate.m_chainman, Consensus::DEPLOYMENT_V24)}; // Limit size to between 1K and MaxBlockSize()-1K for sanity: m_options.nBlockMaxSize = std::max(1000, std::min(MaxBlockSize(fDIP0001Active_context) - 1000, m_options.nBlockMaxSize)); @@ -268,7 +269,9 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc CCbTx cbTx; - if (fV20Active_context) { + if (fV24Active_context) { + cbTx.nVersion = CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS; + } else if (fV20Active_context) { cbTx.nVersion = CCbTx::Version::CLSIG_AND_BALANCE; } else if (fDIP0008Active_context) { cbTx.nVersion = CCbTx::Version::MERKLE_ROOT_QUORUMS; @@ -304,6 +307,10 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc } cbTx.creditPoolBalance = creditPoolDiff->GetTotalLocked(); + + if (fV24Active_context) { + cbTx.merkleRootAssetUnlocks = CalcCbTxMerkleRootAssetUnlocks(*pblock); + } } } @@ -622,6 +629,20 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele } } if (tx.IsSpecialTxVersion() && (tx.nType == TRANSACTION_ASSET_LOCK || tx.nType == TRANSACTION_ASSET_UNLOCK)) { + // Version 2 asset unlocks are not expiry-evicted: an expired instance stays in + // the mempool awaiting a re-signed replacement. Skip instances that are not + // currently minable (expired height window or stale quorum) instead of + // producing an invalid template. + if (IsAssetUnlockWithStableTxid(tx)) { + TxValidationState state; + if (!m_chain_helper.special_tx->CheckSpecialTx(tx, m_chainstate.m_chain.Tip(), + m_chainstate.CoinsTip(), /*check_sigs=*/true, state)) { + LogPrintf("%s: package tx %s skipped, asset unlock instance not currently minable: %s\n", __func__, + tx.GetHash().ToString(), state.ToString()); + validPackage = false; + break; + } + } creditPoolTransactions.emplace_back(entry->GetSharedTx()); } } diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index a66c036bbfda..51cc1fd8e46e 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -55,7 +55,13 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t // So if the output does exist, then this transaction exists in the chain. if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN; } - if (auto mempool_tx = node.mempool->get(txid); mempool_tx) { + const auto mempool_tx = node.mempool->get(txid); + // A version 2 asset unlock sharing a mempool entry's txid but carrying different quorum + // signing info is a re-signed instance of that withdrawal and must reach the mempool, + // which accepts it as an in-place refresh. + const bool is_unlock_refresh{mempool_tx && IsAssetUnlockWithStableTxid(*tx) && + mempool_tx->GetInstanceHash() != tx->GetInstanceHash()}; + if (mempool_tx && !is_unlock_refresh) { // There's already a transaction in the mempool with this txid. Don't // try to submit this transaction to the mempool (since it'll be // rejected as a TX_CONFLICT), but do attempt to reannounce the mempool diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index a56f8c3677f4..04ce81e0c841 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -9,6 +9,7 @@ #include #include