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/Makefile.am

@@ -1306,6 +1306,7 @@ libdashkernel_la_SOURCES = \
   init/common.cpp \
   instantsend/db.cpp \
   instantsend/instantsend.cpp \
+  instantsend/lock.cpp \
   kernel/chain.cpp \
   kernel/checks.cpp \
   kernel/coinstats.cpp \

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

@@ -119,11 +119,13 @@ std::optional<CCreditPool> CCreditPoolManager::GetFromCache(const CBlockIndex& b
 
     const uint256 block_hash = block_index.GetBlockHash();
     CCreditPool pool;
-    {
-        LOCK(cache_mutex);
-        if (creditPoolCache.get(block_hash, pool)) {
-            return pool;
-        }
+    if (WITH_LOCK(cache_mutex, return creditPoolCache.get(block_hash, pool))) {
+        // The pool may have been constructed, and cached, outside a block-scoped transaction
+        // (mempool acceptance, template creation, RPC), where its snapshot cannot be written.
+        // Persist it from the first transaction-scoped lookup instead, or the snapshot would be
+        // lost for good: block connection only sees the cache hit and never constructs it again.
+        MaybeWriteSnapshot(block_hash, block_index.nHeight, pool);
+        return pool;
     }
     if (block_index.nHeight % DISK_SNAPSHOT_PERIOD == 0) {
         if (evoDb.Read(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) {
@@ -135,26 +137,33 @@ std::optional<CCreditPool> CCreditPoolManager::GetFromCache(const CBlockIndex& b
     return std::nullopt;
 }
 
-void CCreditPoolManager::AddToCache(const uint256& block_hash, int height, const CCreditPool &pool)
+void CCreditPoolManager::MaybeWriteSnapshot(const uint256& block_hash, int height, const CCreditPool& pool)
 {
-    if (height % DISK_SNAPSHOT_PERIOD == 0) {
-        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
-            // validation-state catch, and the block-connect catches must not
-            // translate this into a consensus rejection.
-            const std::string msg = strprintf("CCreditPoolManager::%s -- EvoDB credit pool mismatch for block %s",
-                                              __func__, block_hash.ToString());
-            AbortNode(msg);
-            throw EvoDbInconsistencyError(msg);
-        }
-    }
-    {
-        LOCK(cache_mutex);
-        creditPoolCache.insert(block_hash, pool);
+    // 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. GetFromCache() writes it once a transaction-scoped lookup hits
+    // the cached pool.
+    if (height % DISK_SNAPSHOT_PERIOD != 0 || !evoDb.HasActiveTransaction()) return;
+    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
+        // validation-state catch, and the block-connect catches must not
+        // translate this into a consensus rejection.
+        const std::string msg = strprintf("CCreditPoolManager::%s -- EvoDB credit pool mismatch for block %s", __func__,
+                                          block_hash.ToString());
+        AbortNode(msg);
+        throw EvoDbInconsistencyError(msg);
     }
 }
 
+void CCreditPoolManager::AddToCache(const uint256& block_hash, int height, const CCreditPool& pool)
+{
+    MaybeWriteSnapshot(block_hash, height, pool);
+    LOCK(cache_mutex);
+    creditPoolCache.insert(block_hash, pool);
+}
+
 CCreditPool CCreditPoolManager::ConstructCreditPool(const gsl::not_null<const CBlockIndex*> block_index, CCreditPool prev)
 {
     std::optional<CreditPoolDataPerBlock> opt_block_data = GetCreditDataFromBlock(block_index, m_chainman.GetConsensus());

src/evo/creditpool.h

@@ -146,6 +146,9 @@ class CCreditPoolManager
 private:
     std::optional<CCreditPool> GetFromCache(const CBlockIndex& block_index) EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex);
     void AddToCache(const uint256& block_hash, int height, const CCreditPool& pool) EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex);
+    /** Write the disk snapshot of a snapshot-height pool when a block-scoped EvoDB transaction
+     *  is open on this thread; a no-op otherwise. */
+    void MaybeWriteSnapshot(const uint256& block_hash, int height, const CCreditPool& pool);
 
     CCreditPool ConstructCreditPool(const gsl::not_null<const CBlockIndex*> block_index, CCreditPool prev)
         EXCLUSIVE_LOCKS_REQUIRED(!cache_mutex);