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

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

@@ -175,7 +175,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 +201,9 @@ static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& ver
     if (assetUnlockTx.getVersion() == 0 || assetUnlockTx.getVersion() > CAssetUnlockPayload::CURRENT_VERSION) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-version");
     }
+    if (!is_v24_active && assetUnlockTx.getVersion() > CAssetUnlockPayload::INITIAL_VERSION) {
+        return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-version-2");
+    }
 
     if (indexes != std::nullopt && indexes->Contains(assetUnlockTx.getIndex())) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-duplicated-index");
@@ -214,30 +218,32 @@ static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& ver
     const CAssetUnlockPayload payload_copy{assetUnlockTx.getVersion(), assetUnlockTx.getIndex(), assetUnlockTx.getFee(), assetUnlockTx.getRequestedHeight(), assetUnlockTx.getQuorumHash(), CBLSSignature{}};
     SetTxPayload(tx_copy, payload_copy);
 
-    uint256 msgHash = tx_copy.GetHash();
+    // The signed message must commit to requestedHeight and quorumHash even though the version 2
+    // txid excludes them, so hash the full serialization rather than using GetHash().
+    uint256 msgHash = ::SerializeHash(tx_copy);
 
     return verify_sig(assetUnlockTx, msgHash, pindexPrev, state);
 }
 
 bool CheckAssetUnlockTx(const BlockManager& blockman, const llmq::CQuorumManager& qman, const CTransaction& tx,
                         gsl::not_null<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,11 @@ bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationSta
         if ((isV20 && cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) || (!isV20 && cbTx.nVersion >= CCbTx::Version::CLSIG_AND_BALANCE)) {
             return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
         }
+
+        if ((is_v24_active && cbTx.nVersion < CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS) ||
+            (!is_v24_active && cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS)) {
+            return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
+        }
     }
 
     return true;
@@ -147,11 +152,27 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
     return true;
 }
 
+uint256 CalcCbTxMerkleRootAssetUnlocks(const CBlock& block)
+{
+    // Instance hashes cover the quorum signing info that the txids of these transactions - and
+    // therefore the block's merkle root - exclude. Two instances of one withdrawal share a txid,
+    // so duplicate leaves imply a duplicate transaction, which the block merkle-root check
+    // (CheckMerkleRoot, run before this) already rejects; no mutated check is needed here.
+    std::vector<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/signing.cpp

@@ -189,6 +189,12 @@ bool InstantSendSigner::CheckCanLock(const COutPoint& outpoint, bool printDebug,
 
     auto mempoolTx = m_mempool.get(outpoint.hash);
     if (mempoolTx) {
+        if (IsAssetUnlockWithStableTxid(*mempoolTx)) {
+            // An unmined version 2 asset unlock was quorum-signed, so the withdrawal is
+            // irreversible on Platform and will be re-signed until mined under this same txid.
+            // Spends of its outputs may therefore be locked before it is mined.
+            return outpoint.n < mempoolTx->vout.size();
+        }
         if (printDebug) {
             LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: parent mempool TX %s is not locked\n", __func__,
                      txHash.ToString(), outpoint.hash.ToString());

src/llmq/signing.cpp

@@ -542,16 +542,27 @@ bool CSigningManager::ProcessRecoveredSig(const std::shared_ptr<const CRecovered
         if (db.GetRecoveredSigById(llmqType, recoveredSig->getId(), 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