feat: version 2 asset unlocks with stable txids and InstantSend locks (DIP-0027 amendment, v24)

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.

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());
     }
 }
 

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<CAssetUnlockPayload>(tx)) {
             entry.pushKV("assetUnlockTx", opt_assetUnlockTx->ToJson());
         }
+        if (IsAssetUnlockWithStableTxid(tx)) {
+            entry.pushKV("instanceHash", tx.GetInstanceHash().ToString());
+        }
     }
 
     if (have_undo) {

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 <typename ScanQuorums, typename GetQuorum>
 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 <typename VerifySig>
 static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& verify_sig, const CTransaction& tx,
                                    gsl::not_null<const CBlockIndex*> pindexPrev,
-                                   const std::optional<CRangesSet>& indexes, TxValidationState& state)
+                                   const std::optional<CRangesSet>& 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<const CBlockIndex*> pindexPrev, const std::optional<CRangesSet>& 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<const CBlockIndex*> pindexPrev,
-                        const std::optional<CRangesSet>& indexes, TxValidationState& state)
+                        const std::optional<CRangesSet>& 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)

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<const CBlockIndex*> pindexPrev, const std::optional<CRangesSet>& indexes, TxValidationState& state);
+bool CheckAssetUnlockTx(const node::BlockManager& blockman, const llmq::CQuorumManager& qman, const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, const std::optional<CRangesSet>& 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<const CBlockIndex*> pindexPrev,
-                        const std::optional<CRangesSet>& indexes, TxValidationState& state)
+                        const std::optional<CRangesSet>& indexes, bool is_v24_active, TxValidationState& state)
     EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 bool GetAssetUnlockFee(const CTransaction& tx, CAmount& txfee, TxValidationState& state);
 

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<uint256> 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<uint16_t>(nVersion), nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString(), bestCLHeightDiff, bestCLSignature.ToString(),
-        creditPoolBalance / COIN, creditPoolBalance % COIN);
+        creditPoolBalance / COIN, creditPoolBalance % COIN, merkleRootAssetUnlocks.ToString());
 }
 
 std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex)

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<CCbTx::Version> : 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<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex);
 

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;

src/evo/creditpool.cpp

@@ -137,7 +137,11 @@ std::optional<CCreditPool> 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

src/evo/evodb.h

@@ -115,6 +115,16 @@ class CEvoDB
 
     std::unique_ptr<CEvoDBScopedCommitter> 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

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<CCbTx>(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<CCbTx>(*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(),

src/instantsend/db.cpp

@@ -203,8 +203,7 @@ void CInstantSendDb::WriteBlockInstantSendLocks(const gsl::not_null<std::shared_
     LOCK(cs_db);
     CDBBatch batch(*db);
     for (const auto& tx : pblock->vtx) {
-        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_null<std::shared
     LOCK(cs_db);
     CDBBatch batch(*db);
     for (const auto& tx : pblock->vtx) {
-        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());