diff --git a/doc/release-notes-7639.md b/doc/release-notes-7639.md index d65ac94e787c..70f33f1f6922 100644 --- a/doc/release-notes-7639.md +++ b/doc/release-notes-7639.md @@ -22,7 +22,12 @@ 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`). +(CbTx version 4, `merkleRootAssetUnlocks`). An expired instance is also +accepted into the mempool (from peers and when reloading `mempool.dat`) if its +signature was valid at a height within its window, so its spends propagate; it is +neither mined nor InstantSend-locked until Platform re-signs it. Relaying an +Asset Unlock outside the receiving node's height window or quorum range is no +longer penalized. Updated RPCs ------------ diff --git a/src/evo/assetlocktx.cpp b/src/evo/assetlocktx.cpp index 78d0590365ae..d20421a8f8ff 100644 --- a/src/evo/assetlocktx.cpp +++ b/src/evo/assetlocktx.cpp @@ -122,14 +122,17 @@ static bool VerifyAssetUnlockSig(const CAssetUnlockPayload& payload, ScanQuorums const int quorums_to_scan = llmq_params_opt->signingActiveQuorumCount + 1; const auto quorums = scan_quorums(llmqType, pindexTip, quorums_to_scan); + // The quorum recency and height window checks depend on the validating node's tip: the same + // transaction is valid a block earlier or later, so an honest peer at another height relays + // it. They fail as TX_BAD_SPECIAL so that relaying such a transaction is not punished. if (bool isActive = std::any_of(quorums.begin(), quorums.end(), [&](const auto &q) { return q->qc->quorumHash == payload.getQuorumHash(); }); !isActive) { - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-too-old-quorum"); + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlock-too-old-quorum"); } if (static_cast(pindexTip->nHeight) < payload.getRequestedHeight() || pindexTip->nHeight >= payload.getHeightToExpiry()) { LogPrint(BCLog::CREDITPOOL, "Asset unlock tx %d with requested height %d could not be accepted on height: %d\n", payload.getIndex(), payload.getRequestedHeight(), pindexTip->nHeight); - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-too-late"); + return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlock-too-late"); } const auto quorum = get_quorum(llmqType, payload.getQuorumHash()); diff --git a/src/validation.cpp b/src/validation.cpp index d9e8fd69c5e4..2dcb799b81f2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -730,6 +730,14 @@ class MemPoolAccept std::optional TryAssetUnlockRefresh(const CTransactionRef& ptx, const ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + // Validate a transaction's special payload for admission. A version 2 asset unlock past its + // height window is kept in the mempool awaiting a re-signed instance rather than expiry-evicted, + // so it is validated against its window instead of the tip: nodes that did not hold it (after a + // restart, or a new peer) admit it and its descendants too. It stays unminable and is not + // InstantSend-locked until refreshed, as the miner and the lock signer validate against the tip. + bool CheckSpecialTxForMempool(const CTransaction& tx, bool is_v24_active, TxValidationState& state) + EXCLUSIVE_LOCKS_REQUIRED(cs_main); + // Enforce package mempool ancestor/descendant limits (distinct from individual // ancestor/descendant limits done in PreChecks). bool PackageMempoolChecks(const std::vector& txns, @@ -792,6 +800,32 @@ class MemPoolAccept CTxMemPool::Limits m_limits; }; +bool MemPoolAccept::CheckSpecialTxForMempool(const CTransaction& tx, bool is_v24_active, TxValidationState& state) +{ + const CChain& chain{m_active_chainstate.m_chain}; + const auto check_at = [&](const CBlockIndex* pindex, + TxValidationState& check_state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + return m_chain_helper.special_tx->CheckSpecialTx(tx, pindex, is_v24_active, m_active_chainstate.CoinsTip(), + /*check_sigs=*/true, check_state); + }; + const auto payload{IsAssetUnlockWithStableTxid(tx) ? GetTxPayload(tx) : std::nullopt}; + if (!payload) return check_at(chain.Tip(), state); + + const int64_t tip_height{chain.Height()}; + const int64_t first_height{payload->getRequestedHeight()}; + const int64_t last_height{first_height + CAssetUnlockPayload::HEIGHT_DIFF_EXPIRING - 1}; + const int64_t oldest_height{tip_height - + m_pool.m_expiry / m_active_chainstate.m_chainman.GetConsensus().PowTargetSpacing()}; + if (last_height >= tip_height || first_height < oldest_height) return check_at(chain.Tip(), state); + + // Expired: admit it if it was minable at some height of its window. A quorum signing it is + // active at the window's first height, or was mined after it and is active at the last one. + if (check_at(chain[first_height], state)) return true; + if (state.GetRejectReason() != "bad-assetunlock-too-old-quorum") return false; + state = TxValidationState{}; + return check_at(chain[last_height], state); +} + bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) { AssertLockHeld(cs_main); @@ -1068,8 +1102,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // NOTE: we use UTXO here and do NOT allow mempool txes as masternode collaterals const CBlockIndex* tip{m_active_chainstate.m_chain.Tip()}; const bool is_v24_active{DeploymentActiveAfter(tip, m_active_chainstate.m_chainman, Consensus::DEPLOYMENT_V24)}; - if (!m_chain_helper.special_tx->CheckSpecialTx(tx, tip, is_v24_active, m_active_chainstate.CoinsTip(), true, state)) - return false; + if (!CheckSpecialTxForMempool(tx, is_v24_active, state)) return false; if (m_pool.existsProviderTxConflict(tx)) { return state.Invalid(TxValidationResult::TX_CONFLICT, "protx-dup"); diff --git a/test/functional/feature_asset_locks.py b/test/functional/feature_asset_locks.py index 27c51c589393..56536643ba39 100755 --- a/test/functional/feature_asset_locks.py +++ b/test/functional/feature_asset_locks.py @@ -528,6 +528,13 @@ def test_asset_unlocks(self, node_wallet, node, pubkey): self.generate(node, 1) self.check_mempool_result(tx=asset_unlock_tx_too_late, result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-too-late'}) + self.log.info("A peer relaying an unlock outside the tip's height window is not punished") + # Validity depends on the tip, so an honest peer a block behind relays it. The + # Misbehaving line is logged even for this whitelisted (noban) peer. + late_peer = node.add_p2p_connection(P2PInterface()) + with node.assert_debug_log(expected_msgs=["bad-assetunlock-too-late"], unexpected_msgs=["Misbehaving"]): + late_peer.send_and_ping(msg_tx(asset_unlock_tx_too_late)) + node.disconnect_p2ps() block_to_reconsider = node.getbestblockhash() self.log.info("Test block invalidation with asset unlock tx...") @@ -1028,6 +1035,55 @@ def child_output(): assert child_txid in mempool assert retry_txid in mempool + self.log.info("A node without the expired instances admits them and the child from a peer") + expired_hexes = [node_wallet.getrawtransaction(txid) for txid in (stable_txid, retry_txid, child_txid)] + # Stopping dumps node's mempool, which the -persistmempool=0 run neither loads nor overwrites + self.restart_node(1, self.extra_args[1] + ["-persistmempool=0"]) + assert stable_txid not in node.getrawmempool() + expired_peer = node.add_p2p_connection(P2PInterface()) + with node.assert_debug_log(expected_msgs=[], unexpected_msgs=["Misbehaving"]): + for tx_hex in expired_hexes: + expired_peer.send_and_ping(msg_tx(tx_from_hex(tx_hex))) + mempool = node.getrawmempool() + assert stable_txid in mempool + assert retry_txid in mempool + assert child_txid in mempool + + self.log.info("An expired instance must still carry a signature valid at its requestedHeight") + forged = tx_from_hex(expired_hexes[0]) + forged_payload = CAssetUnlockTx() + forged_payload.deserialize(BytesIO(forged.vExtraPayload)) + forged_payload.index = 810 + forged.vExtraPayload = forged_payload.serialize() + forged_peer = node.add_p2p_connection(P2PInterface()) + with node.assert_debug_log(expected_msgs=["bad-assetunlock-not-verified", "Misbehaving: peer=", "(0 -> 100)"]): + forged_peer.send_and_ping(msg_tx(forged)) + node.disconnect_p2ps() + + self.log.info("A requestedHeight overflowing the height window is rejected as too late") + overflow_unlock = self.create_assetunlock(812, COIN, pubkey, version=2, requested_height=2**32 - HEIGHT_DIFF_EXPIRING) + assert_equal(node.testmempoolaccept([overflow_unlock.serialize().hex()])[0]['reject-reason'], 'bad-assetunlock-too-late') + + self.log.info("An expired instance signed by a quorum mined after its requestedHeight is admitted") + late_index = 811 + late_quorum = self.mninfo[0].get_node(self).quorum("selectquorum", llmq_type_test, self.create_assetunlock_request_id(late_index))["quorumHash"] + late_quorum_mined_height = node.getblock(node.quorum("info", llmq_type_test, late_quorum)["minedBlock"])["height"] + late_quorum_unlock = self.create_assetunlock(late_index, COIN, pubkey, version=2, requested_height=late_quorum_mined_height - 1) + blocks_to_expiry = late_quorum_mined_height - 1 + HEIGHT_DIFF_EXPIRING - node.getblockcount() + if blocks_to_expiry > 0: + # node is still disconnected from node_wallet after its restart above + self.generate(node, blocks_to_expiry, sync_fun=self.no_op) + assert_equal(node.testmempoolaccept([late_quorum_unlock.serialize().hex()])[0]['allowed'], True) + + self.log.info("The expired instances and the child survive a restart") + self.restart_node(1, self.extra_args[1]) + mempool = node.getrawmempool() + assert stable_txid in mempool + assert retry_txid in mempool + assert child_txid in mempool + self.connect_nodes(1, 0) + self.sync_all() + self.log.info("Flush leftover withdrawals from earlier phases and clear the window; the expired instance keeps waiting for a re-sign") # Pending unlocks from the limit tests would otherwise consume the cleared window and # crowd this test's withdrawal out of the block. Mine until each of them is either mined