From 68f559349c31de4610632374f44cf8aad4505764 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 5 May 2026 02:47:28 +0700 Subject: [PATCH] fix: preserve raw nTxType bytes on pre-DIP-0002 (version 0) transactions PR #675 made version=0 transactions decode as Classic, but it dropped the original on-wire `nTxType` u16. After decode, `consensus_encode`/ `txid` re-emitted `tx_type()` as 0, breaking byte-exact round-trip and changing the txid for the malformed mainnet transaction. Add a `TransactionType::ClassicalWithNonStandardVersionTypeBytes(u16)` variant (and a matching pseudo-payload `TransactionPayload:: ClassicalWithNonStandardVersionTypeBytesPayloadType(u16)`) that carry the raw u16 read from the wire. Encoding, txid, and sighash now emit those bytes verbatim, and the encoder skips the payload section since pre-DIP-0002 transactions have no payload section on the wire (not even a length prefix). `#[repr(u16)]` is dropped from `TransactionType` since the enum is no longer field-less; replaced `(... as u16)` casts with a new `to_u16()` method. Co-Authored-By: Claude Opus 4.7 (1M context) --- dash/src/blockdata/transaction/mod.rs | 89 +++++++++++++-- .../asset_unlock/unqualified_asset_unlock.rs | 2 +- .../transaction/special_transaction/mod.rs | 108 +++++++++++++----- dash/src/crypto/sighash.rs | 2 +- key-wallet/src/psbt/map/global.rs | 4 +- .../transaction_router/mod.rs | 28 +++-- 6 files changed, 182 insertions(+), 51 deletions(-) diff --git a/dash/src/blockdata/transaction/mod.rs b/dash/src/blockdata/transaction/mod.rs index eb13fadfb..d812630ad 100644 --- a/dash/src/blockdata/transaction/mod.rs +++ b/dash/src/blockdata/transaction/mod.rs @@ -199,15 +199,22 @@ impl Transaction { pub fn txid(&self) -> Txid { let mut enc = Txid::engine(); self.version.consensus_encode(&mut enc).expect("engines don't error"); - (self.tx_type() as u16).consensus_encode(&mut enc).expect("engines don't error"); + self.tx_type().to_u16().consensus_encode(&mut enc).expect("engines don't error"); self.input.consensus_encode(&mut enc).expect("engines don't error"); self.output.consensus_encode(&mut enc).expect("engines don't error"); self.lock_time.consensus_encode(&mut enc).expect("engines don't error"); if let Some(payload) = &self.special_transaction_payload { - let mut buf = Vec::new(); - payload.consensus_encode(&mut buf).expect("engines don't error"); - // this is so we get the size of the payload - buf.consensus_encode(&mut enc).expect("engines don't error"); + // Pre-DIP-0002 transactions have no payload section on the wire — keep + // the txid bit-identical to the on-chain bytes by skipping the section. + if !matches!( + payload, + TransactionPayload::ClassicalWithNonStandardVersionTypeBytesPayloadType(_) + ) { + let mut buf = Vec::new(); + payload.consensus_encode(&mut buf).expect("engines don't error"); + // this is so we get the size of the payload + buf.consensus_encode(&mut enc).expect("engines don't error"); + } } Txid::from_engine(enc) @@ -544,7 +551,7 @@ impl Encodable for Transaction { fn consensus_encode(&self, w: &mut W) -> Result { let mut len = 0; len += self.version.consensus_encode(w)?; - len += (self.tx_type() as u16).consensus_encode(w)?; + len += self.tx_type().to_u16().consensus_encode(w)?; // To avoid serialization ambiguity, no inputs means we use BIP141 serialization (see // `Transaction` docs for full explanation). let mut have_witness = self.input.is_empty(); @@ -582,10 +589,18 @@ impl Encodable for Transaction { } len += self.lock_time.consensus_encode(w)?; if let Some(payload) = &self.special_transaction_payload { - let mut buf = Vec::new(); - payload.consensus_encode(&mut buf)?; - // this is so we get the size of the payload - len += buf.consensus_encode(w)?; + // Pre-DIP-0002 transactions have no payload section on the wire (not even + // a length prefix). Skip the encoding block entirely so re-serialization + // matches the on-chain bytes verbatim. + if !matches!( + payload, + TransactionPayload::ClassicalWithNonStandardVersionTypeBytesPayloadType(_) + ) { + let mut buf = Vec::new(); + payload.consensus_encode(&mut buf)?; + // this is so we get the size of the payload + len += buf.consensus_encode(w)?; + } } Ok(len) } @@ -601,8 +616,13 @@ impl Decodable for Transaction { TransactionType::try_from(special_transaction_type_u16).map_err(|_| { encode::Error::UnknownSpecialTransactionType(special_transaction_type_u16) })? - } else { + } else if special_transaction_type_u16 == 0 { TransactionType::Classic + } else { + // Pre-DIP-0002 (version 0) transactions are logically Classic, but at least + // one mainnet tx put non-zero bytes in the type slot. Preserve the raw u16 + // so consensus_encode/txid keep matching the on-chain bytes. + TransactionType::ClassicalWithNonStandardVersionTypeBytes(special_transaction_type_u16) }; let input = Vec::::consensus_decode_from_finite_reader(r)?; // segwit @@ -1018,6 +1038,53 @@ mod tests { assert_eq!(realtx2.version, 0); } + #[test] + fn test_pre_dip2_classical_with_non_standard_version_type_bytes_roundtrip() { + use crate::blockdata::transaction::special_transaction::{ + TransactionPayload, TransactionType, + }; + + // Same tx as `test_transaction_version`'s second case, but with the on-wire + // `nTxType` slot set to a non-zero u16 (0x2A in little-endian => 0x002A). + // Pre-DIP-0002 these bytes were ignored by consensus but still part of the + // serialized transaction and therefore part of the txid pre-image. Bytes 2..4 + // are the `nTxType` field; bytes 0..2 are version=0. + let tx_bytes = Vec::from_hex("00002a000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0100f2052a01000000434104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac00000000").unwrap(); + let tx: Transaction = + deserialize(&tx_bytes).expect("decoder must accept pre-DIP-0002 raw type bytes"); + assert_eq!(tx.version, 0); + assert_eq!( + tx.tx_type(), + TransactionType::ClassicalWithNonStandardVersionTypeBytes(0x002A), + "raw on-wire u16 must be preserved on the decoded type" + ); + assert!(matches!( + tx.special_transaction_payload, + Some(TransactionPayload::ClassicalWithNonStandardVersionTypeBytesPayloadType(0x002A)) + )); + + // Round-trip: re-serializing must reproduce the original bytes. + let reser = serialize(&tx); + assert_eq!(reser, tx_bytes, "consensus_encode must round-trip raw type bytes"); + + // The txid must match the txid computed from the original on-wire bytes + // (double-SHA256 of the serialized transaction). Since the encoder now + // reproduces the input bytes exactly, the txid is the hash of those bytes. + assert_eq!(tx.txid(), Txid::hash(&tx_bytes), "txid must hash the original bytes"); + + // The txid must differ from the same transaction with type bytes zeroed — + // proves we no longer collapse different on-chain transactions to one id. + let mut zero_type_bytes = tx_bytes.clone(); + zero_type_bytes[2] = 0; + zero_type_bytes[3] = 0; + let zero_type_tx: Transaction = deserialize(&zero_type_bytes).unwrap(); + assert_ne!( + tx.txid(), + zero_type_tx.txid(), + "txids of pre-DIP-0002 txs must depend on the raw type bytes" + ); + } + #[test] fn tx_no_input_deserialization() { let tx_bytes = Vec::from_hex( diff --git a/dash/src/blockdata/transaction/special_transaction/asset_unlock/unqualified_asset_unlock.rs b/dash/src/blockdata/transaction/special_transaction/asset_unlock/unqualified_asset_unlock.rs index 5e03f30e1..454d35de0 100644 --- a/dash/src/blockdata/transaction/special_transaction/asset_unlock/unqualified_asset_unlock.rs +++ b/dash/src/blockdata/transaction/special_transaction/asset_unlock/unqualified_asset_unlock.rs @@ -138,7 +138,7 @@ impl Encodable for AssetUnlockBaseTransactionInfo { fn consensus_encode(&self, w: &mut W) -> Result { let mut len = 0; len += self.version.consensus_encode(w)?; - len += (AssetUnlock as u16).consensus_encode(w)?; + len += AssetUnlock.to_u16().consensus_encode(w)?; len += Vec::::new().consensus_encode(w)?; len += self.output.consensus_encode(w)?; len += self.lock_time.consensus_encode(w)?; diff --git a/dash/src/blockdata/transaction/special_transaction/mod.rs b/dash/src/blockdata/transaction/special_transaction/mod.rs index f781f5493..7f12bc8a8 100644 --- a/dash/src/blockdata/transaction/special_transaction/mod.rs +++ b/dash/src/blockdata/transaction/special_transaction/mod.rs @@ -27,14 +27,16 @@ use core::fmt::{Debug, Display, Formatter}; use bincode::{Decode, Encode}; use crate::blockdata::transaction::special_transaction::TransactionPayload::{ - AssetLockPayloadType, AssetUnlockPayloadType, CoinbasePayloadType, MnhfSignalPayloadType, - ProviderRegistrationPayloadType, ProviderUpdateRegistrarPayloadType, + AssetLockPayloadType, AssetUnlockPayloadType, + ClassicalWithNonStandardVersionTypeBytesPayloadType, CoinbasePayloadType, + MnhfSignalPayloadType, ProviderRegistrationPayloadType, ProviderUpdateRegistrarPayloadType, ProviderUpdateRevocationPayloadType, ProviderUpdateServicePayloadType, QuorumCommitmentPayloadType, }; use crate::blockdata::transaction::special_transaction::TransactionType::{ - AssetLock, AssetUnlock, Classic, Coinbase, MnhfSignal, ProviderRegistration, - ProviderUpdateRegistrar, ProviderUpdateRevocation, ProviderUpdateService, QuorumCommitment, + AssetLock, AssetUnlock, Classic, ClassicalWithNonStandardVersionTypeBytes, Coinbase, + MnhfSignal, ProviderRegistration, ProviderUpdateRegistrar, ProviderUpdateRevocation, + ProviderUpdateService, QuorumCommitment, }; use crate::blockdata::transaction::special_transaction::asset_lock::AssetLockPayload; use crate::blockdata::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload; @@ -84,6 +86,12 @@ pub enum TransactionPayload { AssetLockPayloadType(AssetLockPayload), /// A wrapper for an Asset Unlock payload AssetUnlockPayloadType(AssetUnlockPayload), + /// A pseudo-payload that carries the raw `nTxType` u16 read from a pre-DIP-0002 + /// transaction (`version == 0`). Older transactions on the chain were free to put + /// arbitrary bytes in the type slot; we keep the original value here so that + /// `consensus_encode` and `txid` continue to round-trip the on-wire bytes faithfully. + /// This variant has no payload section on the wire. + ClassicalWithNonStandardVersionTypeBytesPayloadType(u16), } impl Encodable for TransactionPayload { @@ -98,6 +106,7 @@ impl Encodable for TransactionPayload { MnhfSignalPayloadType(p) => p.consensus_encode(w), AssetLockPayloadType(p) => p.consensus_encode(w), AssetUnlockPayloadType(p) => p.consensus_encode(w), + ClassicalWithNonStandardVersionTypeBytesPayloadType(_) => Ok(0), } } } @@ -115,23 +124,28 @@ impl TransactionPayload { MnhfSignalPayloadType(_) => MnhfSignal, AssetLockPayloadType(_) => AssetLock, AssetUnlockPayloadType(_) => AssetUnlock, + ClassicalWithNonStandardVersionTypeBytesPayloadType(raw) => { + ClassicalWithNonStandardVersionTypeBytes(*raw) + } } } /// Gets the size of the special transaction payload #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { - // 1 byte is the size of the special transaction type - 1 + match self { - ProviderRegistrationPayloadType(p) => p.size(), - ProviderUpdateServicePayloadType(p) => p.size(), - ProviderUpdateRegistrarPayloadType(p) => p.size(), - ProviderUpdateRevocationPayloadType(p) => p.size(), - CoinbasePayloadType(p) => p.size(), - QuorumCommitmentPayloadType(p) => p.size(), - MnhfSignalPayloadType(p) => p.size(), - AssetLockPayloadType(p) => p.size(), - AssetUnlockPayloadType(p) => p.size(), + match self { + // 1 byte is the size of the special transaction type + ProviderRegistrationPayloadType(p) => 1 + p.size(), + ProviderUpdateServicePayloadType(p) => 1 + p.size(), + ProviderUpdateRegistrarPayloadType(p) => 1 + p.size(), + ProviderUpdateRevocationPayloadType(p) => 1 + p.size(), + CoinbasePayloadType(p) => 1 + p.size(), + QuorumCommitmentPayloadType(p) => 1 + p.size(), + MnhfSignalPayloadType(p) => 1 + p.size(), + AssetLockPayloadType(p) => 1 + p.size(), + AssetUnlockPayloadType(p) => 1 + p.size(), + // Pre-DIP-0002 transactions have no payload section on the wire. + ClassicalWithNonStandardVersionTypeBytesPayloadType(_) => 0, } } @@ -273,29 +287,37 @@ impl TransactionPayload { /// The first part for the version and the second part for the transaction /// type. /// +/// Pre-DIP-0002 transactions on Dash mainnet (`version == 0`) were free to put +/// arbitrary bytes in the type slot. The [`ClassicalWithNonStandardVersionTypeBytes`] +/// variant preserves the raw u16 read from the wire so that those transactions can +/// still round-trip through `consensus_encode` / `txid` without altering the on-chain +/// bytes or hashes. Logically these transactions behave as Classic. #[derive(Clone, Copy, PartialEq, Eq)] -#[repr(u16)] pub enum TransactionType { /// A Classic transaction - Classic = 0, + Classic, /// A Masternode Registration Transaction - ProviderRegistration = 1, + ProviderRegistration, /// A Masternode Update Service Transaction, used by the operator to signal changes to service - ProviderUpdateService = 2, + ProviderUpdateService, /// A Masternode Update Registrar Transaction, used by the owner to signal base changes - ProviderUpdateRegistrar = 3, + ProviderUpdateRegistrar, /// A Masternode Update Revocation Transaction, used by the operator to signal termination of service - ProviderUpdateRevocation = 4, + ProviderUpdateRevocation, /// A Coinbase Transaction, contained as the first transaction in each block - Coinbase = 5, + Coinbase, /// A Quorum Commitment Transaction, used to save quorum information to the state - QuorumCommitment = 6, + QuorumCommitment, /// A MNHF Signal Transaction, used by masternodes to signal consensus for hard fork activations - MnhfSignal = 7, + MnhfSignal, /// An Asset Lock Transaction, used to transfer credits to Dash Platform, by locking them until withdrawals occur - AssetLock = 8, + AssetLock, /// An Asset Unlock Transaction, used to withdraw credits from Dash Platform, by unlocking them - AssetUnlock = 9, + AssetUnlock, + /// A pre-DIP-0002 Classic transaction (`version == 0`) whose on-wire `nTxType` + /// bytes were non-zero. The wrapped value is the original u16 read from the wire, + /// which must be re-emitted verbatim during serialization to preserve the txid. + ClassicalWithNonStandardVersionTypeBytes(u16), } impl Debug for TransactionType { @@ -311,6 +333,9 @@ impl Debug for TransactionType { MnhfSignal => write!(f, "MNHF Signal Transaction"), AssetLock => write!(f, "Asset Lock Transaction"), AssetUnlock => write!(f, "Asset Unlock Transaction"), + ClassicalWithNonStandardVersionTypeBytes(raw) => { + write!(f, "Classic Transaction (pre-DIP-0002, raw type bytes 0x{raw:04x})") + } } } } @@ -328,6 +353,9 @@ impl Display for TransactionType { MnhfSignal => write!(f, "MNHF Signal"), AssetLock => write!(f, "Asset Lock"), AssetUnlock => write!(f, "Asset Unlock"), + ClassicalWithNonStandardVersionTypeBytes(raw) => { + write!(f, "Classic (pre-DIP-0002, raw 0x{raw:04x})") + } } } } @@ -369,18 +397,44 @@ impl TransactionType { } } + /// Returns the on-wire `u16` representation of the transaction type. + /// + /// For pre-DIP-0002 [`ClassicalWithNonStandardVersionTypeBytes`] this returns the + /// original raw bytes that were read from the chain so that re-encoding/hashing the + /// transaction reproduces the on-chain bytes verbatim. + pub fn to_u16(self) -> u16 { + match self { + Classic => 0, + ProviderRegistration => 1, + ProviderUpdateService => 2, + ProviderUpdateRegistrar => 3, + ProviderUpdateRevocation => 4, + Coinbase => 5, + QuorumCommitment => 6, + MnhfSignal => 7, + AssetLock => 8, + AssetUnlock => 9, + ClassicalWithNonStandardVersionTypeBytes(raw) => raw, + } + } + /// Decodes the payload based on the transaction type. pub fn consensus_decode( self, d: &mut R, ) -> Result, encode::Error> { + // Pre-DIP-0002 transactions and Classic transactions have no payload section + // on the wire — there isn't even a length prefix to consume. let _len = match self { - Classic => VarInt(0), + Classic | ClassicalWithNonStandardVersionTypeBytes(_) => VarInt(0), _ => VarInt::consensus_decode(d)?, }; Ok(match self { Classic => None, + ClassicalWithNonStandardVersionTypeBytes(raw) => { + Some(ClassicalWithNonStandardVersionTypeBytesPayloadType(raw)) + } ProviderRegistration => Some(ProviderRegistrationPayloadType( ProviderRegistrationPayload::consensus_decode(d)?, )), diff --git a/dash/src/crypto/sighash.rs b/dash/src/crypto/sighash.rs index bdb2917ab..2e839467e 100644 --- a/dash/src/crypto/sighash.rs +++ b/dash/src/crypto/sighash.rs @@ -632,7 +632,7 @@ impl> SighashCache { self.tx.borrow().version.consensus_encode(&mut writer)?; // nTransactionType (2): the nTxType of the transaction. - (self.tx.borrow().tx_type() as u16).consensus_encode(&mut writer)?; + self.tx.borrow().tx_type().to_u16().consensus_encode(&mut writer)?; // nLockTime (4): the nLockTime of the transaction. self.tx.borrow().lock_time.consensus_encode(&mut writer)?; diff --git a/key-wallet/src/psbt/map/global.rs b/key-wallet/src/psbt/map/global.rs index 97df0aa07..39505e4cf 100644 --- a/key-wallet/src/psbt/map/global.rs +++ b/key-wallet/src/psbt/map/global.rs @@ -35,7 +35,9 @@ impl Map for PartiallySignedTransaction { // without witnesses. let mut ret = Vec::new(); ret.extend(encode::serialize(&self.unsigned_tx.version)); - (self.unsigned_tx.tx_type() as u16) + self.unsigned_tx + .tx_type() + .to_u16() .consensus_encode(&mut ret) .expect("can't encode tx type"); let input = encode::serialize(&self.unsigned_tx.input); diff --git a/key-wallet/src/transaction_checking/transaction_router/mod.rs b/key-wallet/src/transaction_checking/transaction_router/mod.rs index e26152648..93bd98b13 100644 --- a/key-wallet/src/transaction_checking/transaction_router/mod.rs +++ b/key-wallet/src/transaction_checking/transaction_router/mod.rs @@ -42,26 +42,34 @@ impl TransactionRouter { /// Classify a transaction based on its type and payload pub fn classify_transaction(tx: &Transaction) -> TransactionType { // Check if it's a special transaction - if let Some(ref payload) = tx.special_transaction_payload { + let special_classification = tx.special_transaction_payload.as_ref().and_then(|payload| { match payload { TransactionPayload::ProviderRegistrationPayloadType(_) => { - TransactionType::ProviderRegistration + Some(TransactionType::ProviderRegistration) } TransactionPayload::ProviderUpdateRegistrarPayloadType(_) => { - TransactionType::ProviderUpdateRegistrar + Some(TransactionType::ProviderUpdateRegistrar) } TransactionPayload::ProviderUpdateServicePayloadType(_) => { - TransactionType::ProviderUpdateService + Some(TransactionType::ProviderUpdateService) } TransactionPayload::ProviderUpdateRevocationPayloadType(_) => { - TransactionType::ProviderUpdateRevocation + Some(TransactionType::ProviderUpdateRevocation) } - TransactionPayload::AssetLockPayloadType(_) => TransactionType::AssetLock, - TransactionPayload::AssetUnlockPayloadType(_) => TransactionType::AssetUnlock, - TransactionPayload::CoinbasePayloadType(_) => TransactionType::Coinbase, - TransactionPayload::QuorumCommitmentPayloadType(_) => TransactionType::Ignored, - TransactionPayload::MnhfSignalPayloadType(_) => TransactionType::Ignored, + TransactionPayload::AssetLockPayloadType(_) => Some(TransactionType::AssetLock), + TransactionPayload::AssetUnlockPayloadType(_) => Some(TransactionType::AssetUnlock), + TransactionPayload::CoinbasePayloadType(_) => Some(TransactionType::Coinbase), + TransactionPayload::QuorumCommitmentPayloadType(_) => { + Some(TransactionType::Ignored) + } + TransactionPayload::MnhfSignalPayloadType(_) => Some(TransactionType::Ignored), + // Pre-DIP-0002 transactions are logically Classic — fall through to the + // standard / coinbase / coinjoin classification below. + TransactionPayload::ClassicalWithNonStandardVersionTypeBytesPayloadType(_) => None, } + }); + if let Some(classification) = special_classification { + classification } else if tx.is_coin_base() { TransactionType::Coinbase } else if Self::is_coinjoin_transaction(tx) {