Skip to content

Commit d1e64da

Browse files
Merge #7639: feat: version 2 asset unlocks with stable txids and InstantSend locks (DIP-0027 amendment, v24)
d62becf fix(mempool): apply the asset unlock eviction check to package test acceptance (pasta) 38e953b fix(llmq): preserve legacy signature truncation bookkeeping (pasta) 1ba8ba0 fix(mempool): widen pending withdrawal accounting (pasta) 23c5045 fix(mempool): preserve package dependencies during unlock replacement (pasta) 8d91c72 fix(llmq): retain Platform recovered signatures per message (pasta) f906977 fix(mempool): reject duplicate withdrawal indexes before package admission (pasta) ce36a13 fix: protect asset unlock package parents (pasta) 5391ac4 fix(net): do not put the shared txid of a rejected asset unlock into the rejects filter (UdjinM6) 6fff8db fix(net): keep the lock of a held asset unlock when another instance is rejected (UdjinM6) 43d81ed test: reproduce asset unlock v2 lock removal and shared-txid reject filtering (UdjinM6) 61be75a test: exercise quorumSig in the asset unlock txid and commitment tests (pasta) 430fb7a fix(net): keep DSTX validation for version 2 asset unlocks and dedup them by txid too (pasta) 480f6e8 fix(instantsend): bound asset unlock lock retries and refuse locks on credit pool failure (pasta) 6965e50 fix(mempool): guard the credit pool lookup and hold one claimant per withdrawal index (pasta) 0325195 fix(evo): persist the credit pool snapshot once a block connection hits a cached pool (pasta) c93ee22 fix(primitives): compute the asset unlock instance hash on demand (pasta) 070f410 test: cover InstantSend locks on version 2 asset unlocks (pasta) 22af9d6 feat(instantsend): lock version 2 asset unlocks by their withdrawal index (pasta) 682a059 feat(mempool): track pending asset unlock amount and withdrawal indexes (pasta) f54ef8b test: cover version 2 asset unlocks in feature_asset_locks.py (pasta) 4fe11ce feat(rpc): expose asset unlock instance hashes and align platform signing with withdrawal re-signs (pasta) e22d5f4 feat(net): relay version 2 asset unlocks by instance hash (pasta) ac2039a feat(mempool): refresh pending withdrawals in place and retain expired version 2 asset unlocks (pasta) 8503c5d fix(evo): only write credit pool disk snapshots inside a block-scoped EvoDB transaction (pasta) 62845d2 feat(consensus): commit to asset unlock instance hashes in the coinbase transaction (pasta) ab1ceb3 feat(consensus): compute version 2 asset unlock txids with the quorum signing info zeroed (pasta) Pull request description: ## Issue being fixed or feature implemented Users want Platform→Core withdrawals to be rapidly respendable with InstantSend finality. Today that is impossible: an Asset Unlock can expire before it is mined, Platform then re-signs the withdrawal, and because the re-signed transaction has a different txid, any transaction spending the unmined unlock's outputs is invalidated — so spends of unmined unlocks can never be islocked. This PR implements **version 2 Asset Unlock transactions** (spec: dashpay/dips#189), activating with `DEPLOYMENT_V24`: the **txid itself is computed with the quorum signing info (`requestedHeight`, `quorumHash`, `quorumSig`) zeroed** — exactly and provably the only fields Platform changes when it re-signs an expired withdrawal. Every re-signed instance of one withdrawal is therefore *the same transaction*: children reference one stable txid forever and survive expiry and re-signing. This is segwit's txid/wtxid split applied to the quorum-sig fields — no aliasing in the mempool, UTXO set, or wallet layers; the spending model stays completely standard. On top of that, **the unlock itself is InstantSend-locked** as soon as it can be mined in the next block, using its withdrawal index as a synthetic input. An islock attests "this will be mined and nothing in consensus prevents it"; for an unlock that holds as long as Platform keeps re-signing, which it is obligated to do (there is no refund path), and signing only minable-now instances makes any failure a double fault. Once locked, the withdrawal is like any other locked transaction: children are ordinary islocked spends, the wallet trusts its outputs, and Platform→Core transfers become rapidly respendable. ## What was done? **Consensus — hashing rule** (`primitives/transaction`, `evo/assetlocktx`) - v2 payloads are serialized byte-identically to v1; the version byte (gated on v24, `bad-assetunlocktx-version-2`, mirroring Asset Lock v2) changes hashing: the txid zeroes the trailing 132 payload bytes. The full-serialization hash remains available as `GetInstanceHash()` (computed on demand for v2 unlocks, equal to the txid for every other transaction). - The signed message hash is unchanged — it zeroes only `quorumSig` and still commits to `requestedHeight`/`quorumHash` — and is now computed explicitly from the full serialization (using `GetHash()` on the sig-zeroed copy would silently zero all three fields under the new rule). Signature validity rules (48-block window, active-quorum-set+1 recency) are identical to v1. **Consensus — coinbase commitment** (`evo/cbtx`, `validation`, `node/miner`, `blockencodings`) - v2 txids exclude the sig bytes, so the block merkle root no longer commits to them. CbTx **version 4** (required post-v24) adds `merkleRootAssetUnlocks`: the merkle root over the instance hashes of the block's v2 unlocks (null when none). Verified in `CheckMerkleRoot` as a **mutation** check (`bad-cbtx-assetunlockmerkleroot`, `BLOCK_MUTATED`), mirroring segwit's witness commitment: a middleman can flip sig bytes without breaking the merkle root, and treating that as invalidity would let it poison an honest block's hash. - Compact block short IDs are computed from instance hashes (BIP152v2's wtxid move): a mempool entry holding a *different* re-signed instance of a mined withdrawal is requested via `getblocktxn` instead of being spliced into the reconstructed block; `FillBlock`'s existing `IsBlockMutated` check backstops short-ID collisions. **Mempool** (`validation`, `txmempool`, `node/transaction`, `node/miner`) - A re-signed instance shares the entry's txid; ATMP routes it through a refresh path that fully validates it and, when `requestedHeight` is higher, swaps the `CTransactionRef` **in place** — descendants, ancestry, and fee accounting untouched because everything the txid covers is identical. Stale/duplicate instances are rejected (`assetunlock-stale-instance`). `sendrawtransaction` submits refreshes instead of short-circuiting on the known txid. - v2 unlocks are **not expiry-evicted**: an expired instance waits in the mempool for its replacement, so children never die with it; the miner instead skips instances that aren't currently minable. Since unlocks have no inputs, a new outputs-already-known check prevents an already-mined instance from re-entering (and, for v2, lingering). - The mempool tracks the **pending withdrawal total** (outputs + fee of every unlock it holds, the quantity the credit pool charges) and a withdrawal-index map. The credit pool limit is enforced only at block connect, so this is what lets InstantSend tell an over-limit unlock from a minable one. Exposed as `getmempoolinfo.pendingassetunlocks`. Mining any instance of a withdrawal evicts every other instance claiming its index. - At most **one claimant per withdrawal index** is held: a second unlock claiming an index under a different txid (a v1 instance signed pre-fork re-signed as v2 post-fork, or a Platform fault) is rejected as `assetunlock-stale-instance` unless its `requestedHeight` is higher, in which case it evicts the held claimant and its descendants, mirroring the in-place refresh. Checked before signature verification. The credit pool lookup in ATMP is wrapped: a local reconstruction failure is a `TX_BAD_SPECIAL` rejection (no peer punishment) and EvoDB corruption an error state, never an escaped exception. **InstantSend** (`instantsend/*`, `validation`) - **The v2 unlock itself is islocked**, not just its children. Unlocks have no inputs, so the lock pins one synthetic outpoint: `{DIP-27 request id = SHA256d("plwdtx" ‖ index), 0}` (`instantsend::GetLockInputs`). Every instance of one withdrawal, whatever its version or txid, maps to that outpoint, so a lock binds the index to one txid, any other claimant conflicts through the ordinary outpoint conflict path, and a re-sign (same txid) leaves the lock intact. Wire format unchanged. - Masternodes sign the lock only when the unlock is **minable in the next block** (`CheckCanLockAssetUnlock`): stable-txid instance, passes the full special-tx check at the tip including its quorum signature, no other instance of its index in the mempool (a withdrawal signed as v1 pre-fork can be re-signed as v2 post-fork under a different txid), and the mempool's pending withdrawal total fits the credit pool's current limit. Platform pools withdrawals under the same limit, so a pending total above it indicates a fault and nothing is signed until the window clears. Both the height window and the limit move with the tip, so every tracked unmined unlock is re-evaluated on each connected block; a refresh re-triggers an attempt too. - Consequences that fall out for free: children are ordinary islocked spends (the rev-3 `CheckCanLock` exception is gone), the wallet trusts a locked withdrawal's outputs via `IsTxLockedByInstantSend`, and the mempool's time-based expiry already spares locked transactions. - Every `vin.empty()` early-out in InstantSend (including the IS-DB block hooks that mark locks mined and the block-connect conflict filter) goes through `HasLockInputs`. A peer islock on an unlock whose inputs are anything but the synthetic outpoint is dropped. Mined unlocks are tracked but not locked retroactively, since ChainLocks never wait for them. - `getassetunlockstatuses` reports `instantlock` for mempooled indexes. **P2P relay** (`net_processing`, `protocol`, `version`) - txid-based announcement can never propagate a refresh (known-txid dedup; rejects-filter poisoning). New `MSG_ASSET_UNLOCK` inventory type (protocol **70242**) announces v2 unlocks **by instance hash**; getdata is answered with a plain `tx` message; requests and the rejects filter are tracked per instance. Older peers get a `MSG_TX` announcement of the current instance and never see refreshes. **RPC & signing tooling** (`core_write`, `rpc/quorums`, `llmq/signing*`) - `instanceHash` in v2 unlock JSON. `platformsign` allows re-signing a request id with a different message hash. Platform recovered signatures are retained per message and retrieved by the requested message hash, so out-of-order delivery preserves both signatures. Share processing stops only for a matching Platform message, including on members that learned the earlier signature without voting. Each retained signature expires independently. Production Platform signing (Tenderdash vote extensions) is unaffected; this aligns Core's local signing path used by tests/tooling. **Tests** - Unit: txid invariance across the signing fields (and only those), `CMutableTransaction` agreement, msgHash semantics, v1 hashing unchanged, DIP-0027 worked-example vectors, CbTx unlock-root calculation. - Unit: lock inputs of an unlock (synthetic outpoint, same for every version/instance of an index, distinct per index; ordinary txs / commitments / coinbase unchanged); mempool pending amount and index map across add, refresh, cross-version duplicate, index-conflict eviction and removal. - Unit: ATMP rejects a staler claimant of a held withdrawal index before signature verification; credit pool snapshot persisted at a snapshot height when block assembly constructed the pool first; InstantSend tracker drops an unlocked unlock removed from the mempool and hands a queued unlock out once per trigger. - Functional (`feature_asset_locks.py`): pre-fork v2 rejection; spend of an unmined v2 unlock by its stable txid; refresh in place (same txid, child untouched, `instanceHash` rotates); `MSG_ASSET_UNLOCK` inv observed for both the initial instance and the refresh; stale-instance rejection; survival of the expired instance + child; window clearing; fresh re-sign mined together with the child; CbTx v4 commitment asserted against the mined instance hash. With InstantSend enabled: the unlock is **not** locked while the pending total exceeds the limit (an ordinary tx is), the wallet does not trust the child's output, the re-signed minable instance within the limit **is** locked with the withdrawal index as its single input, the child is then locked through the ordinary path and trusted by the wallet, and a second withdrawal refused on the limit is locked by the per-block retry once the window clears and it is refreshed. Cross-version claimants: a v2 instance signed at the same height as the held v1 instance is rejected, a v2 unlock wrapped in a `dstx` message goes through DSTX validation and is dropped, and a v2 instance signed one block later replaces the v1 claimant and gets locked. ## How Has This Been Tested? - `feature_asset_locks.py` passes locally (macOS arm64) including the extended `test_asset_unlock_v2` scenario; also `feature_llmq_is_retroactive.py`, `feature_llmq_is_cl_conflicts.py`, `feature_llmq_chainlocks.py`, `feature_llmq_singlenode.py`, `feature_notifications.py`, `rpc_netinfo.py`, `p2p_dstx.py`, `feature_protx_version.py`, `mempool_unbroadcast.py`, `interface_rest.py`, `wallet_basic.py`. - Full `test_dash` unit suite passes. - Lints: circular dependencies (two new expected entries registered), whitespace, python, assertions. - The DIP worked-example vectors produced by `dip-0027/dip-0027-txid-calc.py` match Core's hashing byte-for-byte (pinned in a unit test). ## Breaking Changes - **Consensus (v24 EHF, inactive until params are set):** v2 Asset Unlock payloads become acceptable and CbTx v4 becomes required once v24 activates; before activation both are rejected. **This must be code-complete before the v24 EHF parameters (bit 12, currently `NEVER_ACTIVE`) are finalized.** - **Hashing:** for v2 unlocks (which cannot exist pre-fork), `txid ≠ H(full serialization)`. Light clients verifying merkle proofs for these transactions and explorer libraries computing txids from raw bytes need the one scoped rule; SPV output tracking and spending are otherwise completely standard. - P2P: protocol bumped to 70242 for the `MSG_ASSET_UNLOCK` inventory type. Known follow-ups (deliberately out of scope): - Platform-side emitter PR (payload version byte + deterministic v24 gate on `core_chain_locked_height`); Platform's Tenderdash signing already produces the unchanged message hash. - Restart gap: `LoadMempool` re-runs acceptance, so an *expired* v2 instance (and its children) is dropped on restart until the refresh arrives; the islock itself is persisted in the IS DB and wallet rebroadcast heals it. Accepting an expired instance whose txid is islocked on reload is a possible refinement. - Ecosystem: anything computing txids from raw bytes (rust-dashcore `Transaction::txid()`, dash-spv, DashSync, dashj, explorers) needs the scoped v2 rule before activation. - p2p-level regression tests for the legacy-peer (<70242) `MSG_TX` announcement path and for the rejects-filter poisoning scenario a rejected instance is announced over p2p, then a fresh instance must still propagate. The current functional test exercises the mempool refresh and `MSG_ASSET_UNLOCK` inv end-to-end but drives the stale-instance rejection via `sendrawtransaction`. - The wallet keeps whatever instance it first saw (`AddToWallet` is a no-op on a known txid), so `gettransaction` may show a stale instance's `requestedHeight`/`quorumSig`; ZMQ/index consumers do observe each refresh. No fund-safety impact (outputs are identical across instances). - Multi-transaction `testmempoolaccept` still rejects held unlocks as duplicate txids; single-transaction submission and `submitpackage` support instance refresh. Package preflight rejects duplicate withdrawal indexes before any submission, including when all claimants are new, and both `submitpackage` and multi-transaction `testmempoolaccept` reject a package that spends a mempool claimant (or one of its descendants) that admitting a packaged unlock would evict. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation (dashpay/dips#189) - [ ] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: dc4439a2941f5b0f20676ff7deb47854b11503321d92b8c70ae7421eb9b74877803c2fbc0078764b5ba44dcf9747d532af9d9e94075d9c11db8853cde76170e1
2 parents 75ffaf5 + d62becf commit d1e64da

50 files changed

Lines changed: 2001 additions & 233 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎doc/release-notes-7639.md‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
Notable changes
2+
---------------
3+
4+
### Version 2 Asset Unlock transactions (v24)
5+
6+
Once the `v24` hard fork activates, Platform withdrawals are issued as version 2
7+
Asset Unlock transactions. Their transaction hash is computed with the quorum
8+
signing fields (`requestedHeight`, `quorumHash`, `quorumSig`) zeroed, so every
9+
instance Platform re-signs after an expiry is the same transaction with one
10+
stable txid. Spends of an unmined withdrawal's outputs therefore stay valid
11+
across re-signs.
12+
13+
A version 2 Asset Unlock is InstantSend-locked as soon as it can be mined in
14+
the next block: it carries a valid signature from a recent quorum, is inside
15+
its height window, no other instance of its withdrawal index is in the
16+
mempool, and the withdrawals pending in the mempool fit the credit pool's
17+
current limit. The lock pins the withdrawal index (as the outpoint
18+
`{DIP-27 request id, 0}`) to the txid. Spends of a locked withdrawal are
19+
ordinary InstantSend transactions and the wallet treats the withdrawal's
20+
outputs as trusted, so Platform-to-Core transfers become rapidly respendable.
21+
22+
Version 2 unlocks are relayed by instance hash (new inventory type
23+
`MSG_ASSET_UNLOCK`, protocol version 70242), kept in the mempool while expired
24+
awaiting a re-signed replacement, and committed to by the coinbase transaction
25+
(CbTx version 4, `merkleRootAssetUnlocks`).
26+
27+
Updated RPCs
28+
------------
29+
30+
- `getmempoolinfo` reports `pendingassetunlocks`, the sum of the withdrawal
31+
amounts of the Asset Unlock transactions in the mempool.
32+
- `getassetunlockstatuses` reports `instantlock` for mempooled withdrawals.
33+
- `getrawtransaction` and `decoderawtransaction` report `instanceHash` for
34+
version 2 Asset Unlock transactions.

‎src/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,6 +1316,7 @@ libdashkernel_la_SOURCES = \
13161316
init/common.cpp \
13171317
instantsend/db.cpp \
13181318
instantsend/instantsend.cpp \
1319+
instantsend/lock.cpp \
13191320
kernel/chain.cpp \
13201321
kernel/checks.cpp \
13211322
kernel/coinstats.cpp \

‎src/blockencodings.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block) :
2626
prefilledtxn[0] = {0, block.vtx[0]};
2727
for (size_t i = 1; i < block.vtx.size(); i++) {
2828
const CTransaction& tx = *block.vtx[i];
29-
shorttxids[i - 1] = GetShortID(tx.GetHash());
29+
// Short IDs are computed from instance hashes so that a mempool entry holding a different
30+
// re-signed instance of a version 2 asset unlock (same txid, different quorum signing
31+
// info) is treated as missing and requested, instead of being spliced into the block and
32+
// failing the coinbase asset unlock commitment.
33+
shorttxids[i - 1] = GetShortID(tx.GetInstanceHash());
3034
}
3135
}
3236

‎src/core_write.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,9 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
324324
if (const auto opt_assetUnlockTx = GetTxPayload<CAssetUnlockPayload>(tx)) {
325325
entry.pushKV("assetUnlockTx", opt_assetUnlockTx->ToJson());
326326
}
327+
if (IsAssetUnlockWithStableTxid(tx)) {
328+
entry.pushKV("instanceHash", tx.GetInstanceHash().ToString());
329+
}
327330
} else if (tx.nType == TRANSACTION_PROVIDER_DISSOLVE) {
328331
if (const auto opt_proTx = GetTxPayload<CProDisTx>(tx)) {
329332
entry.pushKV("proDisTx", opt_proTx->ToJson());

‎src/evo/assetlocktx.cpp‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,6 @@ std::string CAssetLockPayload::ToString() const
103103
* Asset Unlock Transaction (withdrawals)
104104
*/
105105

106-
const std::string ASSETUNLOCK_REQUESTID_PREFIX = "plwdtx";
107-
108106
template <typename ScanQuorums, typename GetQuorum>
109107
static bool VerifyAssetUnlockSig(const CAssetUnlockPayload& payload, ScanQuorums&& scan_quorums,
110108
GetQuorum&& get_quorum, const uint256& msgHash,
@@ -141,7 +139,7 @@ static bool VerifyAssetUnlockSig(const CAssetUnlockPayload& payload, ScanQuorums
141139
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-quorum-internal-error");
142140
}
143141

144-
const uint256 requestId = ::SerializeHash(std::make_pair(ASSETUNLOCK_REQUESTID_PREFIX, payload.getIndex()));
142+
const uint256 requestId = ::SerializeHash(std::make_pair(ASSET_UNLOCK_REQUESTID_PREFIX, payload.getIndex()));
145143

146144
if (const llmq::SignHash signHash(llmqType, quorum->qc->quorumHash, requestId, msgHash);
147145
payload.getQuorumSig().VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get())) {
@@ -175,7 +173,8 @@ bool CAssetUnlockPayload::VerifySig(const llmq::CQuorumManager& qman, const CCha
175173
template <typename VerifySig>
176174
static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& verify_sig, const CTransaction& tx,
177175
gsl::not_null<const CBlockIndex*> pindexPrev,
178-
const std::optional<CRangesSet>& indexes, TxValidationState& state)
176+
const std::optional<CRangesSet>& indexes, bool is_v24_active,
177+
TxValidationState& state)
179178
{
180179
// Some checks depends from blockchain status also, such as `known indexes` and `withdrawal limits`
181180
// They are omitted here and done by CCreditPool
@@ -205,6 +204,9 @@ static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& ver
205204
if (assetUnlockTx.getVersion() == 0 || assetUnlockTx.getVersion() > CAssetUnlockPayload::CURRENT_VERSION) {
206205
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-version");
207206
}
207+
if (!is_v24_active && assetUnlockTx.getVersion() > CAssetUnlockPayload::INITIAL_VERSION) {
208+
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-version-2");
209+
}
208210

209211
if (indexes != std::nullopt && indexes->Contains(assetUnlockTx.getIndex())) {
210212
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-duplicated-index");
@@ -219,30 +221,32 @@ static bool CheckAssetUnlockTxImpl(const BlockManager& blockman, VerifySig&& ver
219221
const CAssetUnlockPayload payload_copy{assetUnlockTx.getVersion(), assetUnlockTx.getIndex(), assetUnlockTx.getFee(), assetUnlockTx.getRequestedHeight(), assetUnlockTx.getQuorumHash(), CBLSSignature{}};
220222
SetTxPayload(tx_copy, payload_copy);
221223

222-
uint256 msgHash = tx_copy.GetHash();
224+
// The signed message must commit to requestedHeight and quorumHash even though the version 2
225+
// txid excludes them, so hash the full serialization rather than using GetHash().
226+
uint256 msgHash = ::SerializeHash(tx_copy);
223227

224228
return verify_sig(assetUnlockTx, msgHash, pindexPrev, state);
225229
}
226230

227231
bool CheckAssetUnlockTx(const BlockManager& blockman, const llmq::CQuorumManager& qman, const CTransaction& tx,
228232
gsl::not_null<const CBlockIndex*> pindexPrev, const std::optional<CRangesSet>& indexes,
229-
TxValidationState& state)
233+
bool is_v24_active, TxValidationState& state)
230234
{
231235
return CheckAssetUnlockTxImpl(blockman, [&](const CAssetUnlockPayload& payload, const uint256& msg_hash,
232236
const CBlockIndex* pindex, TxValidationState& tx_state) {
233237
return payload.VerifySig(qman, msg_hash, pindex, tx_state);
234-
}, tx, pindexPrev, indexes, state);
238+
}, tx, pindexPrev, indexes, is_v24_active, state);
235239
}
236240

237241
bool CheckAssetUnlockTx(const BlockManager& blockman, const llmq::CQuorumManager& qman, const CChain& chain,
238242
const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev,
239-
const std::optional<CRangesSet>& indexes, TxValidationState& state)
243+
const std::optional<CRangesSet>& indexes, bool is_v24_active, TxValidationState& state)
240244
{
241245
AssertLockHeld(::cs_main);
242246
return CheckAssetUnlockTxImpl(blockman, [&](const CAssetUnlockPayload& payload, const uint256& msg_hash,
243247
const CBlockIndex* pindex, TxValidationState& tx_state) NO_THREAD_SAFETY_ANALYSIS {
244248
return payload.VerifySig(qman, chain, msg_hash, pindex, tx_state);
245-
}, tx, pindexPrev, indexes, state);
249+
}, tx, pindexPrev, indexes, is_v24_active, state);
246250
}
247251

248252
bool GetAssetUnlockFee(const CTransaction& tx, CAmount& txfee, TxValidationState& state)

‎src/evo/assetlocktx.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,13 @@ class CAssetLockPayload
7575
class CAssetUnlockPayload
7676
{
7777
public:
78-
static constexpr uint8_t CURRENT_VERSION = 1;
78+
static constexpr uint8_t INITIAL_VERSION = 1;
79+
/** Serialized identically to version 1, but the transaction hash excludes the quorum signing
80+
* info (requestedHeight, quorumHash, quorumSig) so every re-signed instance of one withdrawal
81+
* shares one txid; see IsAssetUnlockWithStableTxid(). Gated on DEPLOYMENT_V24. */
82+
static constexpr uint8_t CURRENT_VERSION = 2;
7983
static constexpr auto SPECIALTX_TYPE = TRANSACTION_ASSET_UNLOCK;
84+
static_assert(CURRENT_VERSION >= ASSET_UNLOCK_STABLE_TXID_VERSION);
8085

8186
static constexpr size_t MAXIMUM_WITHDRAWALS = 32;
8287

@@ -163,10 +168,10 @@ class CAssetUnlockPayload
163168
};
164169

165170
bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool is_v24_active);
166-
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);
171+
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);
167172
bool CheckAssetUnlockTx(const node::BlockManager& blockman, const llmq::CQuorumManager& qman, const CChain& chain,
168173
const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev,
169-
const std::optional<CRangesSet>& indexes, TxValidationState& state)
174+
const std::optional<CRangesSet>& indexes, bool is_v24_active, TxValidationState& state)
170175
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
171176
bool GetAssetUnlockFee(const CTransaction& tx, CAmount& txfee, TxValidationState& state);
172177

‎src/evo/cbtx.cpp‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
using node::ReadBlockFromDisk;
2323

24-
bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationState& state)
24+
bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, bool is_v24_active, TxValidationState& state)
2525
{
2626
if (cbTx.nVersion == CCbTx::Version::INVALID || cbTx.nVersion >= CCbTx::Version::UNKNOWN) {
2727
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
@@ -41,6 +41,14 @@ bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationSta
4141
if ((isV20 && cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) || (!isV20 && cbTx.nVersion >= CCbTx::Version::CLSIG_AND_BALANCE)) {
4242
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
4343
}
44+
45+
// The asset unlock commitment extends the version 3 fields, so it is only required once
46+
// both forks are active (tests may activate v24 on a chain where v20 never activates).
47+
const bool requires_unlock_root{is_v24_active && isV20};
48+
if ((requires_unlock_root && cbTx.nVersion < CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS) ||
49+
(!requires_unlock_root && cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS)) {
50+
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
51+
}
4452
}
4553

4654
return true;
@@ -147,11 +155,27 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
147155
return true;
148156
}
149157

158+
uint256 CalcCbTxMerkleRootAssetUnlocks(const CBlock& block)
159+
{
160+
// Instance hashes cover the quorum signing info that the txids of these transactions - and
161+
// therefore the block's merkle root - exclude. Two instances of one withdrawal share a txid,
162+
// so duplicate leaves imply a duplicate transaction, which the block merkle-root check
163+
// (CheckMerkleRoot, run before this) already rejects; no mutated check is needed here.
164+
std::vector<uint256> instance_hashes;
165+
for (const auto& tx : block.vtx) {
166+
// The miner calls this while the coinbase slot is still an empty placeholder
167+
if (tx && IsAssetUnlockWithStableTxid(*tx)) {
168+
instance_hashes.push_back(tx->GetInstanceHash());
169+
}
170+
}
171+
return ComputeMerkleRoot(std::move(instance_hashes));
172+
}
173+
150174
std::string CCbTx::ToString() const
151175
{
152-
return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, creditPoolBalance=%d.%08d)",
176+
return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, creditPoolBalance=%d.%08d, merkleRootAssetUnlocks=%s)",
153177
static_cast<uint16_t>(nVersion), nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString(), bestCLHeightDiff, bestCLSignature.ToString(),
154-
creditPoolBalance / COIN, creditPoolBalance % COIN);
178+
creditPoolBalance / COIN, creditPoolBalance % COIN, merkleRootAssetUnlocks.ToString());
155179
}
156180

157181
std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex)

‎src/evo/cbtx.h‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class CCbTx
3434
MERKLE_ROOT_MNLIST = 1,
3535
MERKLE_ROOT_QUORUMS = 2,
3636
CLSIG_AND_BALANCE = 3,
37+
MERKLE_ROOT_ASSETUNLOCKS = 4,
3738
UNKNOWN,
3839
};
3940

@@ -45,6 +46,10 @@ class CCbTx
4546
uint32_t bestCLHeightDiff{0};
4647
CBLSSignature bestCLSignature;
4748
CAmount creditPoolBalance{0};
49+
/** Merkle root over the instance hashes of the block's version 2+ asset unlock transactions
50+
* (block order; null when there are none). Their txids exclude the quorum signing info, so
51+
* the block's merkle root does not commit to it; this root restores that commitment. */
52+
uint256 merkleRootAssetUnlocks;
4853

4954
SERIALIZE_METHODS(CCbTx, obj)
5055
{
@@ -56,6 +61,9 @@ class CCbTx
5661
READWRITE(COMPACTSIZE(obj.bestCLHeightDiff));
5762
READWRITE(obj.bestCLSignature);
5863
READWRITE(obj.creditPoolBalance);
64+
if (obj.nVersion >= Version::MERKLE_ROOT_ASSETUNLOCKS) {
65+
READWRITE(obj.merkleRootAssetUnlocks);
66+
}
5967
}
6068
}
6169

@@ -68,11 +76,12 @@ class CCbTx
6876
};
6977
template<> struct is_serializable_enum<CCbTx::Version> : std::true_type {};
7078

71-
bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationState& state);
79+
bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, bool is_v24_active, TxValidationState& state);
7280

7381
bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPrev,
7482
const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet,
7583
BlockValidationState& state);
84+
uint256 CalcCbTxMerkleRootAssetUnlocks(const CBlock& block);
7685

7786
std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex);
7887
std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlock& block, int32_t height);

‎src/evo/core_write.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ UniValue CCbTx::ToJson() const
143143
ret.pushKV("bestCLHeightDiff", bestCLHeightDiff);
144144
ret.pushKV("bestCLSignature", bestCLSignature.ToString());
145145
ret.pushKV("creditPoolBalance", ValueFromAmount(creditPoolBalance));
146+
if (nVersion >= CCbTx::Version::MERKLE_ROOT_ASSETUNLOCKS) {
147+
ret.pushKV("merkleRootAssetUnlocks", merkleRootAssetUnlocks.ToString());
148+
}
146149
}
147150
}
148151
return ret;

‎src/evo/creditpool.cpp‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,13 @@ std::optional<CCreditPool> CCreditPoolManager::GetFromCache(const CBlockIndex& b
119119

120120
const uint256 block_hash = block_index.GetBlockHash();
121121
CCreditPool pool;
122-
{
123-
LOCK(cache_mutex);
124-
if (creditPoolCache.get(block_hash, pool)) {
125-
return pool;
126-
}
122+
if (WITH_LOCK(cache_mutex, return creditPoolCache.get(block_hash, pool))) {
123+
// The pool may have been constructed, and cached, outside a block-scoped transaction
124+
// (mempool acceptance, template creation, RPC), where its snapshot cannot be written.
125+
// Persist it from the first transaction-scoped lookup instead, or the snapshot would be
126+
// lost for good: block connection only sees the cache hit and never constructs it again.
127+
MaybeWriteSnapshot(block_hash, block_index.nHeight, pool);
128+
return pool;
127129
}
128130
if (block_index.nHeight % DISK_SNAPSHOT_PERIOD == 0) {
129131
if (evoDb.Read(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) {
@@ -135,26 +137,33 @@ std::optional<CCreditPool> CCreditPoolManager::GetFromCache(const CBlockIndex& b
135137
return std::nullopt;
136138
}
137139

138-
void CCreditPoolManager::AddToCache(const uint256& block_hash, int height, const CCreditPool &pool)
140+
void CCreditPoolManager::MaybeWriteSnapshot(const uint256& block_hash, int height, const CCreditPool& pool)
139141
{
140-
if (height % DISK_SNAPSHOT_PERIOD == 0) {
141-
if (!evoDb.WriteDerived(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) {
142-
// A mismatch is local EvoDB corruption, not a statement about the
143-
// block. Abort here: some callers (miner, RPC) never pass through a
144-
// validation-state catch, and the block-connect catches must not
145-
// translate this into a consensus rejection.
146-
const std::string msg = strprintf("CCreditPoolManager::%s -- EvoDB credit pool mismatch for block %s",
147-
__func__, block_hash.ToString());
148-
AbortNode(msg);
149-
throw EvoDbInconsistencyError(msg);
150-
}
151-
}
152-
{
153-
LOCK(cache_mutex);
154-
creditPoolCache.insert(block_hash, pool);
142+
// The disk snapshot is an optimization; skip it outside a block-scoped EvoDB transaction
143+
// (e.g. a pool constructed on a cold cache during mempool acceptance or template creation),
144+
// where the write would never be committed and would trip the clean-transaction assertion
145+
// at the next root commit. GetFromCache() writes it once a transaction-scoped lookup hits
146+
// the cached pool.
147+
if (height % DISK_SNAPSHOT_PERIOD != 0 || !evoDb.HasActiveTransaction()) return;
148+
if (!evoDb.WriteDerived(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) {
149+
// A mismatch is local EvoDB corruption, not a statement about the
150+
// block. Abort here: some callers (miner, RPC) never pass through a
151+
// validation-state catch, and the block-connect catches must not
152+
// translate this into a consensus rejection.
153+
const std::string msg = strprintf("CCreditPoolManager::%s -- EvoDB credit pool mismatch for block %s", __func__,
154+
block_hash.ToString());
155+
AbortNode(msg);
156+
throw EvoDbInconsistencyError(msg);
155157
}
156158
}
157159

160+
void CCreditPoolManager::AddToCache(const uint256& block_hash, int height, const CCreditPool& pool)
161+
{
162+
MaybeWriteSnapshot(block_hash, height, pool);
163+
LOCK(cache_mutex);
164+
creditPoolCache.insert(block_hash, pool);
165+
}
166+
158167
CCreditPool CCreditPoolManager::ConstructCreditPool(const gsl::not_null<const CBlockIndex*> block_index, CCreditPool prev)
159168
{
160169
std::optional<CreditPoolDataPerBlock> opt_block_data = GetCreditDataFromBlock(block_index, m_chainman.GetConsensus());

0 commit comments

Comments
 (0)