From 1c501854774133589bfc3fd7f05916726d3b4511 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Wed, 7 Oct 2026 11:30:42 +0700 Subject: [PATCH] fix(drive-abci): sign the asset unlock serialization hash instead of the txid Core verifies an asset unlock's quorumSig against the hash of its full serialization with only quorumSig zeroed, which covers requestedHeight and quorumHash. Platform used the txid as the withdrawal vote extension, which is that hash for payload version 1. Version 2 asset unlocks (Core v24, dashpay/dash#7639) compute the txid with requestedHeight, quorumHash and quorumSig zeroed, so once rust-dashcore's txid() follows that rule (dashpay/rust-dashcore#1047), signing it would make every version 2 withdrawal signature fail verification in Core (dashpay/platform#4908, item 3). Hash the serialization explicitly. For every version 1 asset unlock, the only payload version any protocol version builds, the bytes are identical to the txid, so no protocol version changes behaviour. The test Tenderdash simulator now reuses tx_to_extend_vote_extension instead of its own copy of the txid-based extension. The tests guard the future rust-dashcore change and cannot go red against the current pin, where txid() and the serialization hash coincide. A temporary mutation emulating the version 2 txid rule failed both version 2 tests while the version 1 equivalence test passed. Co-Authored-By: Claude Opus 5.5 --- packages/rs-drive-abci/src/mimic/mod.rs | 20 +-- .../unsigned_withdrawal_txs/v0/mod.rs | 127 +++++++++++++++++- 2 files changed, 136 insertions(+), 11 deletions(-) diff --git a/packages/rs-drive-abci/src/mimic/mod.rs b/packages/rs-drive-abci/src/mimic/mod.rs index 9434a5572b9..e6aff2b98eb 100644 --- a/packages/rs-drive-abci/src/mimic/mod.rs +++ b/packages/rs-drive-abci/src/mimic/mod.rs @@ -8,7 +8,7 @@ use crate::execution::types::block_execution_context::v0::BlockExecutionContextV use crate::execution::types::block_state_info::v0::BlockStateInfoV0Getters; use crate::mimic::test_quorum::TestQuorumInfo; use crate::platform_types::withdrawal::unsigned_withdrawal_txs::v0::{ - make_extend_vote_request_id, UnsignedWithdrawalTxs, + tx_to_extend_vote_extension, UnsignedWithdrawalTxs, }; use crate::rpc::core::CoreRPCLike; use ciborium::Value as CborValue; @@ -29,14 +29,14 @@ use rand::{Rng, SeedableRng}; use tenderdash_abci::proto::abci::response_verify_vote_extension::VerifyStatus; use tenderdash_abci::proto::abci::tx_record::TxAction; use tenderdash_abci::proto::abci::{ - CommitInfo, ExecTxResult, RequestExtendVote, RequestFinalizeBlock, RequestPrepareProposal, - RequestProcessProposal, RequestVerifyVoteExtension, ResponsePrepareProposal, - ValidatorSetUpdate, + CommitInfo, ExecTxResult, ExtendVoteExtension, RequestExtendVote, RequestFinalizeBlock, + RequestPrepareProposal, RequestProcessProposal, RequestVerifyVoteExtension, + ResponsePrepareProposal, ValidatorSetUpdate, }; use tenderdash_abci::proto::google::protobuf::Timestamp; use tenderdash_abci::proto::types::{ Block, BlockId, CanonicalVote, Data, EvidenceList, Header, PartSetHeader, SignedMsgType, - StateId, VoteExtension, VoteExtensionType, + StateId, VoteExtension, }; use tenderdash_abci::proto::ToMillis; use tenderdash_abci::signatures::Hashable; @@ -464,11 +464,15 @@ impl FullAbciApplication<'_, C> { .unsigned_withdrawal_transactions .iter() .map(|tx| { - let sign_request_id = Some(make_extend_vote_request_id(tx)); + let ExtendVoteExtension { + r#type, + extension, + sign_request_id, + } = tx_to_extend_vote_extension(tx); VoteExtension { - r#type: VoteExtensionType::ThresholdRecoverRaw as i32, - extension: tx.txid().to_byte_array().to_vec(), + r#type, + extension, sign_request_id, signature: vec![0; 96], //todo: signature } diff --git a/packages/rs-drive-abci/src/platform_types/withdrawal/unsigned_withdrawal_txs/v0/mod.rs b/packages/rs-drive-abci/src/platform_types/withdrawal/unsigned_withdrawal_txs/v0/mod.rs index 88ae225e11b..f25a47d2ce3 100644 --- a/packages/rs-drive-abci/src/platform_types/withdrawal/unsigned_withdrawal_txs/v0/mod.rs +++ b/packages/rs-drive-abci/src/platform_types/withdrawal/unsigned_withdrawal_txs/v0/mod.rs @@ -1,7 +1,7 @@ //! Withdrawal transactions definitions and processing -use dpp::dashcore::consensus::Encodable; -use dpp::dashcore::hashes::Hash; +use dpp::dashcore::consensus::{serialize, Encodable}; +use dpp::dashcore::hashes::{sha256d, Hash}; use dpp::dashcore::transaction::special_transaction::TransactionPayload::AssetUnlockPayloadType; use dpp::dashcore::{Transaction, VarInt}; use std::collections::{BTreeMap, HashMap}; @@ -161,7 +161,11 @@ impl From<&UnsignedWithdrawalTxs> for Vec { pub(crate) fn tx_to_extend_vote_extension(tx: &Transaction) -> ExtendVoteExtension { let request_id = make_extend_vote_request_id(tx); - let extension = tx.txid().as_byte_array().to_vec(); + // `tx` must be unsigned. Core verifies `quorumSig` against the hash of the full serialization + // with only `quorumSig` zeroed, so the message covers `requestedHeight` and `quorumHash`. For + // a version 1 asset unlock this hash is the txid; the txid of a version 2 asset unlock leaves + // those fields out, so it is not the signed message. + let extension = sha256d::Hash::hash(&serialize(tx)).to_byte_array().to_vec(); ExtendVoteExtension { r#type: VoteExtensionType::ThresholdRecoverRaw as i32, @@ -187,3 +191,120 @@ pub(crate) fn make_extend_vote_request_id(asset_unlock_tx: &Transaction) -> Vec< request_id } + +#[cfg(test)] +mod tests { + use super::*; + use crate::test::helpers::withdrawals::unsigned_withdrawal_transactions; + use dpp::dashcore::bls_sig_utils::BLSSignature; + use dpp::dashcore::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload; + use dpp::dashcore::transaction::special_transaction::asset_unlock::request_info::AssetUnlockRequestInfo; + use dpp::dashcore::transaction::special_transaction::asset_unlock::unqualified_asset_unlock::AssetUnlockBasePayload; + use dpp::dashcore::{PubkeyHash, QuorumHash, ScriptBuf, TxOut}; + use std::str::FromStr; + + /// An unsigned version 2 asset unlock paying 1 Dash to the P2PKH script of `[0x11; 20]`, + /// the transaction of the DIP-0027 worked examples. + fn unsigned_version_2_asset_unlock( + index: u64, + request_height: u32, + quorum_hash: [u8; 32], + ) -> Transaction { + Transaction { + version: 3, + lock_time: 0, + input: vec![], + output: vec![TxOut { + value: 100_000_000, + script_pubkey: ScriptBuf::new_p2pkh(&PubkeyHash::from_byte_array([0x11; 20])), + }], + special_transaction_payload: Some(AssetUnlockPayloadType(AssetUnlockPayload { + base: AssetUnlockBasePayload { + version: 2, + index, + fee: 70_000, + }, + request_info: AssetUnlockRequestInfo { + request_height, + quorum_hash: QuorumHash::from_byte_array(quorum_hash), + }, + quorum_sig: BLSSignature::from([0; 96]), + })), + } + } + + /// Parses a hash displayed in reversed byte order, as Core prints it, into raw bytes. + fn displayed_hash_bytes(displayed: &str) -> Vec { + sha256d::Hash::from_str(displayed) + .expect("expected a valid hash") + .to_byte_array() + .to_vec() + } + + #[test] + fn should_sign_the_full_serialization_of_a_version_2_unlock_not_its_txid() { + // DIP-0027 publishes the version 2 txid of its worked example at index 101: the hash of + // the full serialization with `requestedHeight`, `quorumHash` and `quorumSig` zeroed. + let dip_0027_txid = displayed_hash_bytes( + "3c4db73c8356407a5d7c78df5045bd280f2dc4fd644b06c4bfbdead3d5ae41cf", + ); + + // With the signing info zeroed the message and the txid coincide, which pins the + // serialization and byte order of the message to Core's. + let zeroed_signing_info = unsigned_version_2_asset_unlock(101, 0, [0; 32]); + assert_eq!( + tx_to_extend_vote_extension(&zeroed_signing_info).extension, + dip_0027_txid + ); + + // The worked example is requested at height 500 by quorum hash one. Core's message + // covers both, so it is the hash of the serialization that includes them, not the txid. + let mut quorum_hash_one = [0; 32]; + quorum_hash_one[0] = 1; + let worked_example = unsigned_version_2_asset_unlock(101, 500, quorum_hash_one); + let extension = tx_to_extend_vote_extension(&worked_example).extension; + + assert_ne!(extension, dip_0027_txid); + assert_eq!( + extension, + displayed_hash_bytes( + "7df5cfea6865226753795ccdd8536781a179f2767febf4281e2236e178dc4a50" + ) + ); + } + + #[test] + fn should_sign_a_different_message_when_a_version_2_unlock_is_re_signed() { + // A re-signed version 2 unlock keeps its txid, but Core verifies the new signature + // against a message covering the new `requestedHeight` and `quorumHash`. Signing the + // txid would make every re-signed withdrawal fail verification in Core. + let first_instance = unsigned_version_2_asset_unlock(101, 500, [1; 32]); + let re_signed_at_new_height = unsigned_version_2_asset_unlock(101, 700, [1; 32]); + let re_signed_by_new_quorum = unsigned_version_2_asset_unlock(101, 500, [2; 32]); + + let first_extension = tx_to_extend_vote_extension(&first_instance).extension; + + assert_ne!( + tx_to_extend_vote_extension(&re_signed_at_new_height).extension, + first_extension + ); + assert_ne!( + tx_to_extend_vote_extension(&re_signed_by_new_quorum).extension, + first_extension + ); + } + + #[test] + fn should_keep_signing_the_txid_of_version_1_unlocks() { + // The txid of a version 1 unlock is the hash of its full serialization, so for every + // unlock Platform builds as version 1 the vote extension stays byte for byte the same. + let transactions = unsigned_withdrawal_transactions(500); + + for transaction in transactions.iter() { + assert_eq!( + tx_to_extend_vote_extension(transaction).extension, + transaction.txid().to_byte_array().to_vec() + ); + } + } +}