diff --git a/doc/release-notes-7639.md b/doc/release-notes-7639.md new file mode 100644 index 000000000000..d65ac94e787c --- /dev/null +++ b/doc/release-notes-7639.md @@ -0,0 +1,34 @@ +Notable changes +--------------- + +### Version 2 Asset Unlock transactions (v24) + +Once the `v24` hard fork activates, Platform withdrawals are issued as version 2 +Asset Unlock transactions. Their transaction hash is computed with the quorum +signing fields (`requestedHeight`, `quorumHash`, `quorumSig`) zeroed, so every +instance Platform re-signs after an expiry is the same transaction with one +stable txid. Spends of an unmined withdrawal's outputs therefore stay valid +across re-signs. + +A version 2 Asset Unlock is InstantSend-locked as soon as it can be mined in +the next block: it carries a valid signature from a recent quorum, is inside +its height window, no other instance of its withdrawal index is in the +mempool, and the withdrawals pending in the mempool fit the credit pool's +current limit. The lock pins the withdrawal index (as the outpoint +`{DIP-27 request id, 0}`) to the txid. Spends of a locked withdrawal are +ordinary InstantSend transactions and the wallet treats the withdrawal's +outputs as trusted, so Platform-to-Core transfers become rapidly respendable. + +Version 2 unlocks are relayed by instance hash (new inventory type +`MSG_ASSET_UNLOCK`, protocol version 70242), kept in the mempool while expired +awaiting a re-signed replacement, and committed to by the coinbase transaction +(CbTx version 4, `merkleRootAssetUnlocks`). + +Updated RPCs +------------ + +- `getmempoolinfo` reports `pendingassetunlocks`, the sum of the withdrawal + amounts of the Asset Unlock transactions in the mempool. +- `getassetunlockstatuses` reports `instantlock` for mempooled withdrawals. +- `getrawtransaction` and `decoderawtransaction` report `instanceHash` for + version 2 Asset Unlock transactions. 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..f9c1e77238eb 100644 --- a/src/evo/assetlocktx.cpp +++ b/src/evo/assetlocktx.cpp @@ -103,8 +103,6 @@ std::string CAssetLockPayload::ToString() const * Asset Unlock Transaction (withdrawals) */ -const std::string ASSETUNLOCK_REQUESTID_PREFIX = "plwdtx"; - template static bool VerifyAssetUnlockSig(const CAssetUnlockPayload& payload, ScanQuorums&& scan_quorums, GetQuorum&& get_quorum, const uint256& msgHash, @@ -141,7 +139,7 @@ static bool VerifyAssetUnlockSig(const CAssetUnlockPayload& payload, ScanQuorums return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-quorum-internal-error"); } - const uint256 requestId = ::SerializeHash(std::make_pair(ASSETUNLOCK_REQUESTID_PREFIX, payload.getIndex())); + const uint256 requestId = ::SerializeHash(std::make_pair(ASSET_UNLOCK_REQUESTID_PREFIX, payload.getIndex())); if (const llmq::SignHash signHash(llmqType, quorum->qc->quorumHash, requestId, msgHash); payload.getQuorumSig().VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get())) { @@ -175,7 +173,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 +199,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 +216,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..de075ca3ce6a 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,14 @@ 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"); } + + // The asset unlock commitment extends the version 3 fields, so it is only required once + // both forks are active (tests may activate v24 on a chain where v20 never activates). + const bool requires_unlock_root{is_v24_active && isV20}; + if ((requires_unlock_root && cbTx.nVersion < CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS) || + (!requires_unlock_root && cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS)) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); + } } return true; @@ -147,11 +155,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/db.cpp b/src/instantsend/db.cpp index 99ed60f36b1b..bd07f39c5457 100644 --- a/src/instantsend/db.cpp +++ b/src/instantsend/db.cpp @@ -203,8 +203,7 @@ void CInstantSendDb::WriteBlockInstantSendLocks(const gsl::not_nullvtx) { - if (tx->IsCoinBase() || tx->vin.empty()) { - // coinbase and TXs with no inputs can't be locked + if (!HasLockInputs(*tx)) { continue; } uint256 islockHash = GetInstantSendLockHashByTxidInternal(tx->GetHash()); @@ -222,8 +221,7 @@ void CInstantSendDb::RemoveBlockInstantSendLocks(const gsl::not_nullvtx) { - if (tx->IsCoinBase() || tx->vin.empty()) { - // coinbase and TXs with no inputs can't be locked + if (!HasLockInputs(*tx)) { continue; } uint256 islockHash = GetInstantSendLockHashByTxidInternal(tx->GetHash()); diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp index a7f929c294d2..2e799c3b60f1 100644 --- a/src/instantsend/instantsend.cpp +++ b/src/instantsend/instantsend.cpp @@ -130,7 +130,7 @@ void CInstantSendManager::AddPendingISLock(const uint256& hash, const instantsen void CInstantSendManager::TransactionIsRemoved(const CTransactionRef& tx) { - if (tx->vin.empty()) { + if (!instantsend::HasLockInputs(*tx)) { return; } @@ -182,9 +182,9 @@ void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx, const CBlock if (did_insert) { nonLockedTxInfo.tx = tx; - for (const auto& in : tx->vin) { - nonLockedTxs[in.prevout.hash].children.emplace(tx->GetHash()); - nonLockedTxsByOutpoints.emplace(in.prevout, tx->GetHash()); + for (const auto& outpoint : instantsend::GetLockInputs(*tx)) { + nonLockedTxs[outpoint.hash].children.emplace(tx->GetHash()); + nonLockedTxsByOutpoints.emplace(outpoint, tx->GetHash()); } } } @@ -222,14 +222,14 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChild WITH_LOCK(cs_pendingRetry, pendingRetryTxs.erase(txid)); if (info.tx) { - for (const auto& in : info.tx->vin) { - if (auto jt = nonLockedTxs.find(in.prevout.hash); jt != nonLockedTxs.end()) { + for (const auto& outpoint : instantsend::GetLockInputs(*info.tx)) { + if (auto jt = nonLockedTxs.find(outpoint.hash); jt != nonLockedTxs.end()) { jt->second.children.erase(txid); if (!jt->second.tx && jt->second.children.empty()) { nonLockedTxs.erase(jt); } } - nonLockedTxsByOutpoints.erase(in.prevout); + nonLockedTxsByOutpoints.erase(outpoint); } } @@ -239,6 +239,16 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChild __func__, txid.ToString(), retryChildren, retryChildrenCount); } +void CInstantSendManager::RetryUnminedAssetUnlocks() +{ + LOCK2(cs_nonLocked, cs_pendingRetry); + for (const auto& [txid, info] : nonLockedTxs) { + if (info.tx && !info.pindexMined && info.tx->IsPlatformTransfer()) { + pendingRetryTxs.emplace(txid); + } + } +} + std::vector CInstantSendManager::PrepareTxToRetry() { std::vector txns{}; @@ -397,8 +407,8 @@ instantsend::InstantSendLockPtr CInstantSendManager::GetConflictingLock(const CT return nullptr; } - for (const auto& in : tx.vin) { - auto otherIsLock = db.GetInstantSendLockByInput(in.prevout); + for (const auto& outpoint : instantsend::GetLockInputs(tx)) { + auto otherIsLock = db.GetInstantSendLockByInput(outpoint); if (!otherIsLock) { continue; } diff --git a/src/instantsend/instantsend.h b/src/instantsend/instantsend.h index 5e901cb82662..25b7e5bbf6ed 100644 --- a/src/instantsend/instantsend.h +++ b/src/instantsend/instantsend.h @@ -103,6 +103,10 @@ class CInstantSendManager EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_timingsTxSeen); void RemoveNonLockedTx(const uint256& txid, bool retryChildren) EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry); + /** 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. */ + void RetryUnminedAssetUnlocks() EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry); instantsend::InstantSendLockPtr AttachISLockToTx(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks); diff --git a/src/instantsend/lock.cpp b/src/instantsend/lock.cpp index f768c033708f..8fdd4586c3ea 100644 --- a/src/instantsend/lock.cpp +++ b/src/instantsend/lock.cpp @@ -42,4 +42,22 @@ uint256 GenInputLockRequestId(const COutPoint& outpoint) { return ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, outpoint)); } + +std::vector GetLockInputs(const CTransaction& tx) +{ + if (IsAssetUnlockPayload(tx)) { + return {COutPoint{::SerializeHash(std::make_pair(ASSET_UNLOCK_REQUESTID_PREFIX, GetAssetUnlockIndex(tx))), 0}}; + } + std::vector inputs; + inputs.reserve(tx.vin.size()); + for (const auto& in : tx.vin) { + inputs.push_back(in.prevout); + } + return inputs; +} + +bool HasLockInputs(const CTransaction& tx) +{ + return !tx.IsCoinBase() && (!tx.vin.empty() || IsAssetUnlockPayload(tx)); +} } // namespace instantsend diff --git a/src/instantsend/lock.h b/src/instantsend/lock.h index 852fc67bebb5..21ae39776448 100644 --- a/src/instantsend/lock.h +++ b/src/instantsend/lock.h @@ -15,6 +15,7 @@ #include class COutPoint; +class CTransaction; namespace instantsend { struct InstantSendLock { @@ -52,6 +53,19 @@ struct InstantSendLock { uint256 GenInputLockRequestId(const COutPoint& outpoint); +/** The outpoints an InstantSend lock on this transaction pins. These are the transaction's + * prevouts, except for asset unlock transactions, which have no inputs: an unlock's lock pins + * the synthetic outpoint {DIP-27 signing request id of its withdrawal index, 0}. Every + * instance of one withdrawal, whatever its version or txid, maps to that same outpoint, so a + * lock binds the withdrawal index to one txid and any other transaction claiming the index + * conflicts with it through the ordinary outpoint conflict handling. */ +std::vector GetLockInputs(const CTransaction& tx); + +/** Whether GetLockInputs() is non-empty: the transaction spends outputs or is an asset unlock. + * Coinbase, quorum commitment and other input-less special transactions cannot be locked or + * conflict with a lock. */ +bool HasLockInputs(const CTransaction& tx); + using InstantSendLockPtr = std::shared_ptr; } // namespace instantsend diff --git a/src/instantsend/net_instantsend.cpp b/src/instantsend/net_instantsend.cpp index 07e86fd780f4..0be2fd178153 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -222,21 +222,12 @@ Uint256HashSet NetInstantSend::ApplyVerificationResults( } namespace { -template - requires std::same_as || std::same_as -Uint256HashSet GetIdsFromLockable(const std::vector& vec) +Uint256HashSet GetIdsFromLockable(const std::vector& outpoints) { Uint256HashSet ret{}; - if (vec.empty()) return ret; - ret.reserve(vec.size()); - for (const auto& in : vec) { - if constexpr (std::is_same_v) { - ret.emplace(instantsend::GenInputLockRequestId(in)); - } else if constexpr (std::is_same_v) { - ret.emplace(instantsend::GenInputLockRequestId(in.prevout)); - } else { - assert(false); - } + ret.reserve(outpoints.size()); + for (const auto& outpoint : outpoints) { + ret.emplace(instantsend::GenInputLockRequestId(outpoint)); } return ret; } @@ -376,6 +367,17 @@ void NetInstantSend::ProcessInstantSendLock(NodeId from, const uint256& hash, co uint256 hashBlock{}; auto tx = GetTransaction(nullptr, &m_mempool, islock->txid, Params().GetConsensus(), hashBlock); const bool found_transaction{tx != nullptr}; + if (found_transaction && islock->inputs != instantsend::GetLockInputs(*tx)) { + // A lock must pin exactly the transaction's lock inputs. For an asset unlock that is + // the withdrawal's synthetic outpoint; anything else would poison conflict tracking of + // unrelated coins. (Ordinary transactions can only fail this with a lock whose txid + // does not match its inputs, which the signing quorum never produces.) A lock for a + // transaction we do not have yet is parked and comes back through here once the + // transaction arrives; until then its inputs are trusted like any quorum-signed lock's. + LogPrintf("NetInstantSend::%s -- txid=%s, islock=%s: lock inputs do not match the transaction, peer=%d\n", + __func__, islock->txid.ToString(), hash.ToString(), from); + return; + } // we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally const auto minedHeight = GetBlockHeight(m_is_manager, m_chainman.ActiveChainstate(), hashBlock); if (found_transaction) { @@ -442,7 +444,7 @@ void NetInstantSend::WorkThreadMain() void NetInstantSend::TransactionAddedToMempool(const CTransactionRef& tx, int64_t, uint64_t mempool_sequence) { - if (!m_is_manager.IsInstantSendEnabled() || !m_mn_sync.IsBlockchainSynced() || tx->vin.empty()) { + if (!m_is_manager.IsInstantSendEnabled() || !m_mn_sync.IsBlockchainSynced() || !instantsend::HasLockInputs(*tx)) { return; } @@ -463,7 +465,7 @@ void NetInstantSend::ClearConflicting(const Uint256HashMap& to_ for (const auto& [_, tx] : to_delete) { m_is_manager.RemoveNonLockedTx(tx->GetHash(), false); if (m_signer) { - m_signer->ClearInputsFromQueue(GetIdsFromLockable(tx->vin)); + m_signer->ClearInputsFromQueue(GetIdsFromLockable(instantsend::GetLockInputs(*tx))); } } } @@ -491,6 +493,11 @@ void NetInstantSend::RemoveMempoolConflictsForLock(const uint256& hash, const in for (const auto& p : toDelete) { m_mempool.removeRecursive(*p.second, MemPoolRemovalReason::CONFLICT); } + // Asset unlocks have no inputs, so mapNextTx cannot reveal another instance of the + // locked withdrawal; the mempool tracks them by index instead. + if (const auto locked_tx = m_mempool.get(islock.txid)) { + m_mempool.removeAssetUnlockConflicts(*locked_tx); + } } ClearConflicting(toDelete); } @@ -535,13 +542,15 @@ void NetInstantSend::BlockConnected(const std::shared_ptr& pblock, if (m_mn_sync.IsBlockchainSynced()) { const bool has_chainlock = m_chainlocks.HasChainLock(pindex->nHeight, pindex->GetBlockHash()); for (const auto& tx : pblock->vtx) { - if (tx->IsCoinBase() || tx->vin.empty()) { - // coinbase and TXs with no inputs can't be locked + if (!instantsend::HasLockInputs(*tx)) { continue; } if (!m_is_manager.IsLocked(tx->GetHash()) && !has_chainlock) { - if (m_signer) { + // A mined asset unlock is not locked retroactively: its lock exists to make + // pending withdrawals spendable, and ChainLocks do not wait for it. It is still + // tracked so a lock binding its withdrawal index to another txid is detected. + if (m_signer && !tx->vin.empty()) { m_signer->ProcessTx(*tx, true, Params().GetConsensus()); } // TX is not locked, so make sure it is tracked @@ -551,6 +560,9 @@ void NetInstantSend::BlockConnected(const std::shared_ptr& pblock, m_is_manager.RemoveNonLockedTx(tx->GetHash(), true); } } + // The new tip may have moved a pending withdrawal into its height window or freed + // enough of the credit pool limit for every pending withdrawal to fit + m_is_manager.RetryUnminedAssetUnlocks(); } m_is_manager.WriteBlockISLocks(pblock, pindex); } diff --git a/src/instantsend/signing.cpp b/src/instantsend/signing.cpp index 73a42cd099c3..22ce263404e1 100644 --- a/src/instantsend/signing.cpp +++ b/src/instantsend/signing.cpp @@ -7,6 +7,11 @@ #include #include #include +#include +#include +#include +#include +#include #include #include #include @@ -113,10 +118,10 @@ void InstantSendSigner::HandleNewInputLockRecoveredSig(const llmq::CRecoveredSig } if (LogAcceptDebug(BCLog::INSTANTSEND)) { - for (const auto& in : tx->vin) { - if (GenInputLockRequestId(in.prevout) == recoveredSig.getId()) { + for (const auto& outpoint : GetLockInputs(*tx)) { + if (GenInputLockRequestId(outpoint) == recoveredSig.getId()) { LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: got recovered sig for input %s\n", __func__, - txid.ToString(), in.prevout.ToStringShort()); + txid.ToString(), outpoint.ToStringShort()); break; } } @@ -167,6 +172,9 @@ void InstantSendSigner::ProcessPendingRetryLockTxs(const std::vector(tx); + if (!opt_payload) return log_refusal("bad payload"); + + LOCK2(::cs_main, m_mempool.cs); + if (!m_mempool.exists(tx.GetHash())) return log_refusal("not in mempool"); + // A withdrawal signed as version 1 before v24 activation may be re-signed as version 2 after + // it; both instances then claim the index under different txids. Locking the version 2 + // instance while the version 1 one is minable risks the lock losing to a ChainLock. + if (m_mempool.GetAssetUnlockTxidsByIndex(opt_payload->getIndex()).size() != 1) { + return log_refusal("another instance of this withdrawal index is in the mempool"); + } + + Chainstate& chainstate = m_chainman.ActiveChainstate(); + const CBlockIndex* tip = chainstate.m_chain.Tip(); + // Minable in the next block: inside its height window and signed by a recent quorum + TxValidationState state; + if (!chainstate.ChainHelper().special_tx->CheckSpecialTx(tx, tip, chainstate.CoinsTip(), /*check_sigs=*/true, state)) { + return log_refusal(state.ToString()); + } + // Fits the withdrawal limit alongside every other pending withdrawal: the limit is enforced + // only when a block is connected, so an over-limit unlock is otherwise indistinguishable from + // a minable one in the mempool. Platform pools withdrawals under the same daily limit, so + // the pending total exceeding it means something is wrong and nothing is locked until the + // window clears rather than guessing which withdrawals miners will pick. + const CCreditPool pool = chainstate.ChainHelper().GetCreditPool(tip); + if (const CAmount pending{m_mempool.GetPendingAssetUnlockAmount()}; pending > pool.currentLimit) { + return log_refusal(strprintf("pending withdrawals %d exceed the credit pool limit %d", pending, pool.currentLimit)); + } + return true; +} + bool InstantSendSigner::CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, const Consensus::Params& params) const { @@ -313,19 +364,20 @@ void InstantSendSigner::ProcessTx(const CTransaction& tx, bool fRetroactive, con bool InstantSendSigner::TrySignInputLocks(const CTransaction& tx, bool fRetroactive, Consensus::LLMQType llmqType, const Consensus::Params& params) { + const std::vector inputs{GetLockInputs(tx)}; std::vector ids; - ids.reserve(tx.vin.size()); + ids.reserve(inputs.size()); size_t alreadyVotedCount = 0; - for (const auto& in : tx.vin) { - auto id = GenInputLockRequestId(in.prevout); + for (const auto& outpoint : inputs) { + auto id = GenInputLockRequestId(outpoint); ids.emplace_back(id); uint256 otherTxHash; if (m_sigman.GetVoteForId(params.llmqTypeDIP0024InstantSend, id, otherTxHash)) { if (otherTxHash != tx.GetHash()) { LogPrintf("%s -- txid=%s: input %s is conflicting with previous vote for tx %s\n", __func__, - tx.GetHash().ToString(), in.prevout.ToStringShort(), otherTxHash.ToString()); + tx.GetHash().ToString(), outpoint.ToStringShort(), otherTxHash.ToString()); return false; } alreadyVotedCount++; @@ -345,17 +397,17 @@ bool InstantSendSigner::TrySignInputLocks(const CTransaction& tx, bool fRetroact } LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: trying to vote on %d inputs\n", __func__, tx.GetHash().ToString(), - tx.vin.size()); + inputs.size()); - for (const auto i : util::irange(tx.vin.size())) { - const auto& in = tx.vin[i]; + for (const auto i : util::irange(inputs.size())) { + const auto& outpoint = inputs[i]; auto& id = ids[i]; WITH_LOCK(cs_input_requests, inputRequestIds.emplace(id)); LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: trying to vote on input %s with id %s. fRetroactive=%d\n", - __func__, tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString(), fRetroactive); + __func__, tx.GetHash().ToString(), outpoint.ToStringShort(), id.ToString(), fRetroactive); if (m_shareman.AsyncSignIfMember(llmqType, id, tx.GetHash(), {}, fRetroactive)) { LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: voted on input %s with id %s\n", __func__, - tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString()); + tx.GetHash().ToString(), outpoint.ToStringShort(), id.ToString()); } } @@ -366,8 +418,12 @@ void InstantSendSigner::TrySignInstantSendLock(const CTransaction& tx) { const auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; - for (const auto& in : tx.vin) { - auto id = GenInputLockRequestId(in.prevout); + InstantSendLock islock; + islock.txid = tx.GetHash(); + islock.inputs = GetLockInputs(tx); + + for (const auto& outpoint : islock.inputs) { + auto id = GenInputLockRequestId(outpoint); if (!m_sigman.HasRecoveredSig(llmqType, id, tx.GetHash())) { return; } @@ -376,12 +432,6 @@ void InstantSendSigner::TrySignInstantSendLock(const CTransaction& tx) LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: got all recovered sigs, creating InstantSendLock\n", __func__, tx.GetHash().ToString()); - InstantSendLock islock; - islock.txid = tx.GetHash(); - for (const auto& in : tx.vin) { - islock.inputs.emplace_back(in.prevout); - } - auto id = islock.GetRequestId(); if (m_sigman.HasRecoveredSigForId(llmqType, id)) { diff --git a/src/instantsend/signing.h b/src/instantsend/signing.h index 18d1b083aed1..fc8017d3bde7 100644 --- a/src/instantsend/signing.h +++ b/src/instantsend/signing.h @@ -93,6 +93,11 @@ class InstantSendSigner final : public llmq::CRecoveredSigsListener [[nodiscard]] bool CheckCanLock(const CTransaction& tx, bool printDebug, const Consensus::Params& params) const; [[nodiscard]] bool CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, const Consensus::Params& params) const; + /** Whether an asset unlock may be locked: a version 2 instance that is minable in the next + * block (valid quorum signature, inside its height window) while every withdrawal pending + * in the mempool fits the credit pool limit, with no other instance of its withdrawal + * index in the mempool. */ + [[nodiscard]] bool CheckCanLockAssetUnlock(const CTransaction& tx, bool printDebug) const; void HandleNewInputLockRecoveredSig(const llmq::CRecoveredSig& recoveredSig, const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!cs_creating); 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..d634cd45e5a7 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 && fV20Active_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 (cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS) { + 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