Skip to content

Commit d67cc03

Browse files
fix: preserve raw nTxType bytes on pre-DIP-0002 (version 0) transactions (#726)
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) <noreply@anthropic.com>
1 parent ea01278 commit d67cc03

6 files changed

Lines changed: 182 additions & 51 deletions

File tree

dash/src/blockdata/transaction/mod.rs

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -199,15 +199,22 @@ impl Transaction {
199199
pub fn txid(&self) -> Txid {
200200
let mut enc = Txid::engine();
201201
self.version.consensus_encode(&mut enc).expect("engines don't error");
202-
(self.tx_type() as u16).consensus_encode(&mut enc).expect("engines don't error");
202+
self.tx_type().to_u16().consensus_encode(&mut enc).expect("engines don't error");
203203
self.input.consensus_encode(&mut enc).expect("engines don't error");
204204
self.output.consensus_encode(&mut enc).expect("engines don't error");
205205
self.lock_time.consensus_encode(&mut enc).expect("engines don't error");
206206
if let Some(payload) = &self.special_transaction_payload {
207-
let mut buf = Vec::new();
208-
payload.consensus_encode(&mut buf).expect("engines don't error");
209-
// this is so we get the size of the payload
210-
buf.consensus_encode(&mut enc).expect("engines don't error");
207+
// Pre-DIP-0002 transactions have no payload section on the wire — keep
208+
// the txid bit-identical to the on-chain bytes by skipping the section.
209+
if !matches!(
210+
payload,
211+
TransactionPayload::ClassicalWithNonStandardVersionTypeBytesPayloadType(_)
212+
) {
213+
let mut buf = Vec::new();
214+
payload.consensus_encode(&mut buf).expect("engines don't error");
215+
// this is so we get the size of the payload
216+
buf.consensus_encode(&mut enc).expect("engines don't error");
217+
}
211218
}
212219

213220
Txid::from_engine(enc)
@@ -544,7 +551,7 @@ impl Encodable for Transaction {
544551
fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
545552
let mut len = 0;
546553
len += self.version.consensus_encode(w)?;
547-
len += (self.tx_type() as u16).consensus_encode(w)?;
554+
len += self.tx_type().to_u16().consensus_encode(w)?;
548555
// To avoid serialization ambiguity, no inputs means we use BIP141 serialization (see
549556
// `Transaction` docs for full explanation).
550557
let mut have_witness = self.input.is_empty();
@@ -582,10 +589,18 @@ impl Encodable for Transaction {
582589
}
583590
len += self.lock_time.consensus_encode(w)?;
584591
if let Some(payload) = &self.special_transaction_payload {
585-
let mut buf = Vec::new();
586-
payload.consensus_encode(&mut buf)?;
587-
// this is so we get the size of the payload
588-
len += buf.consensus_encode(w)?;
592+
// Pre-DIP-0002 transactions have no payload section on the wire (not even
593+
// a length prefix). Skip the encoding block entirely so re-serialization
594+
// matches the on-chain bytes verbatim.
595+
if !matches!(
596+
payload,
597+
TransactionPayload::ClassicalWithNonStandardVersionTypeBytesPayloadType(_)
598+
) {
599+
let mut buf = Vec::new();
600+
payload.consensus_encode(&mut buf)?;
601+
// this is so we get the size of the payload
602+
len += buf.consensus_encode(w)?;
603+
}
589604
}
590605
Ok(len)
591606
}
@@ -601,8 +616,13 @@ impl Decodable for Transaction {
601616
TransactionType::try_from(special_transaction_type_u16).map_err(|_| {
602617
encode::Error::UnknownSpecialTransactionType(special_transaction_type_u16)
603618
})?
604-
} else {
619+
} else if special_transaction_type_u16 == 0 {
605620
TransactionType::Classic
621+
} else {
622+
// Pre-DIP-0002 (version 0) transactions are logically Classic, but at least
623+
// one mainnet tx put non-zero bytes in the type slot. Preserve the raw u16
624+
// so consensus_encode/txid keep matching the on-chain bytes.
625+
TransactionType::ClassicalWithNonStandardVersionTypeBytes(special_transaction_type_u16)
606626
};
607627
let input = Vec::<TxIn>::consensus_decode_from_finite_reader(r)?;
608628
// segwit
@@ -1018,6 +1038,53 @@ mod tests {
10181038
assert_eq!(realtx2.version, 0);
10191039
}
10201040

1041+
#[test]
1042+
fn test_pre_dip2_classical_with_non_standard_version_type_bytes_roundtrip() {
1043+
use crate::blockdata::transaction::special_transaction::{
1044+
TransactionPayload, TransactionType,
1045+
};
1046+
1047+
// Same tx as `test_transaction_version`'s second case, but with the on-wire
1048+
// `nTxType` slot set to a non-zero u16 (0x2A in little-endian => 0x002A).
1049+
// Pre-DIP-0002 these bytes were ignored by consensus but still part of the
1050+
// serialized transaction and therefore part of the txid pre-image. Bytes 2..4
1051+
// are the `nTxType` field; bytes 0..2 are version=0.
1052+
let tx_bytes = Vec::from_hex("00002a000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0100f2052a01000000434104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac00000000").unwrap();
1053+
let tx: Transaction =
1054+
deserialize(&tx_bytes).expect("decoder must accept pre-DIP-0002 raw type bytes");
1055+
assert_eq!(tx.version, 0);
1056+
assert_eq!(
1057+
tx.tx_type(),
1058+
TransactionType::ClassicalWithNonStandardVersionTypeBytes(0x002A),
1059+
"raw on-wire u16 must be preserved on the decoded type"
1060+
);
1061+
assert!(matches!(
1062+
tx.special_transaction_payload,
1063+
Some(TransactionPayload::ClassicalWithNonStandardVersionTypeBytesPayloadType(0x002A))
1064+
));
1065+
1066+
// Round-trip: re-serializing must reproduce the original bytes.
1067+
let reser = serialize(&tx);
1068+
assert_eq!(reser, tx_bytes, "consensus_encode must round-trip raw type bytes");
1069+
1070+
// The txid must match the txid computed from the original on-wire bytes
1071+
// (double-SHA256 of the serialized transaction). Since the encoder now
1072+
// reproduces the input bytes exactly, the txid is the hash of those bytes.
1073+
assert_eq!(tx.txid(), Txid::hash(&tx_bytes), "txid must hash the original bytes");
1074+
1075+
// The txid must differ from the same transaction with type bytes zeroed —
1076+
// proves we no longer collapse different on-chain transactions to one id.
1077+
let mut zero_type_bytes = tx_bytes.clone();
1078+
zero_type_bytes[2] = 0;
1079+
zero_type_bytes[3] = 0;
1080+
let zero_type_tx: Transaction = deserialize(&zero_type_bytes).unwrap();
1081+
assert_ne!(
1082+
tx.txid(),
1083+
zero_type_tx.txid(),
1084+
"txids of pre-DIP-0002 txs must depend on the raw type bytes"
1085+
);
1086+
}
1087+
10211088
#[test]
10221089
fn tx_no_input_deserialization() {
10231090
let tx_bytes = Vec::from_hex(

dash/src/blockdata/transaction/special_transaction/asset_unlock/unqualified_asset_unlock.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ impl Encodable for AssetUnlockBaseTransactionInfo {
138138
fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
139139
let mut len = 0;
140140
len += self.version.consensus_encode(w)?;
141-
len += (AssetUnlock as u16).consensus_encode(w)?;
141+
len += AssetUnlock.to_u16().consensus_encode(w)?;
142142
len += Vec::<TxIn>::new().consensus_encode(w)?;
143143
len += self.output.consensus_encode(w)?;
144144
len += self.lock_time.consensus_encode(w)?;

dash/src/blockdata/transaction/special_transaction/mod.rs

Lines changed: 81 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,16 @@ use core::fmt::{Debug, Display, Formatter};
2727
use bincode::{Decode, Encode};
2828

2929
use crate::blockdata::transaction::special_transaction::TransactionPayload::{
30-
AssetLockPayloadType, AssetUnlockPayloadType, CoinbasePayloadType, MnhfSignalPayloadType,
31-
ProviderRegistrationPayloadType, ProviderUpdateRegistrarPayloadType,
30+
AssetLockPayloadType, AssetUnlockPayloadType,
31+
ClassicalWithNonStandardVersionTypeBytesPayloadType, CoinbasePayloadType,
32+
MnhfSignalPayloadType, ProviderRegistrationPayloadType, ProviderUpdateRegistrarPayloadType,
3233
ProviderUpdateRevocationPayloadType, ProviderUpdateServicePayloadType,
3334
QuorumCommitmentPayloadType,
3435
};
3536
use crate::blockdata::transaction::special_transaction::TransactionType::{
36-
AssetLock, AssetUnlock, Classic, Coinbase, MnhfSignal, ProviderRegistration,
37-
ProviderUpdateRegistrar, ProviderUpdateRevocation, ProviderUpdateService, QuorumCommitment,
37+
AssetLock, AssetUnlock, Classic, ClassicalWithNonStandardVersionTypeBytes, Coinbase,
38+
MnhfSignal, ProviderRegistration, ProviderUpdateRegistrar, ProviderUpdateRevocation,
39+
ProviderUpdateService, QuorumCommitment,
3840
};
3941
use crate::blockdata::transaction::special_transaction::asset_lock::AssetLockPayload;
4042
use crate::blockdata::transaction::special_transaction::asset_unlock::qualified_asset_unlock::AssetUnlockPayload;
@@ -84,6 +86,12 @@ pub enum TransactionPayload {
8486
AssetLockPayloadType(AssetLockPayload),
8587
/// A wrapper for an Asset Unlock payload
8688
AssetUnlockPayloadType(AssetUnlockPayload),
89+
/// A pseudo-payload that carries the raw `nTxType` u16 read from a pre-DIP-0002
90+
/// transaction (`version == 0`). Older transactions on the chain were free to put
91+
/// arbitrary bytes in the type slot; we keep the original value here so that
92+
/// `consensus_encode` and `txid` continue to round-trip the on-wire bytes faithfully.
93+
/// This variant has no payload section on the wire.
94+
ClassicalWithNonStandardVersionTypeBytesPayloadType(u16),
8795
}
8896

8997
impl Encodable for TransactionPayload {
@@ -98,6 +106,7 @@ impl Encodable for TransactionPayload {
98106
MnhfSignalPayloadType(p) => p.consensus_encode(w),
99107
AssetLockPayloadType(p) => p.consensus_encode(w),
100108
AssetUnlockPayloadType(p) => p.consensus_encode(w),
109+
ClassicalWithNonStandardVersionTypeBytesPayloadType(_) => Ok(0),
101110
}
102111
}
103112
}
@@ -115,23 +124,28 @@ impl TransactionPayload {
115124
MnhfSignalPayloadType(_) => MnhfSignal,
116125
AssetLockPayloadType(_) => AssetLock,
117126
AssetUnlockPayloadType(_) => AssetUnlock,
127+
ClassicalWithNonStandardVersionTypeBytesPayloadType(raw) => {
128+
ClassicalWithNonStandardVersionTypeBytes(*raw)
129+
}
118130
}
119131
}
120132

121133
/// Gets the size of the special transaction payload
122134
#[allow(clippy::len_without_is_empty)]
123135
pub fn len(&self) -> usize {
124-
// 1 byte is the size of the special transaction type
125-
1 + match self {
126-
ProviderRegistrationPayloadType(p) => p.size(),
127-
ProviderUpdateServicePayloadType(p) => p.size(),
128-
ProviderUpdateRegistrarPayloadType(p) => p.size(),
129-
ProviderUpdateRevocationPayloadType(p) => p.size(),
130-
CoinbasePayloadType(p) => p.size(),
131-
QuorumCommitmentPayloadType(p) => p.size(),
132-
MnhfSignalPayloadType(p) => p.size(),
133-
AssetLockPayloadType(p) => p.size(),
134-
AssetUnlockPayloadType(p) => p.size(),
136+
match self {
137+
// 1 byte is the size of the special transaction type
138+
ProviderRegistrationPayloadType(p) => 1 + p.size(),
139+
ProviderUpdateServicePayloadType(p) => 1 + p.size(),
140+
ProviderUpdateRegistrarPayloadType(p) => 1 + p.size(),
141+
ProviderUpdateRevocationPayloadType(p) => 1 + p.size(),
142+
CoinbasePayloadType(p) => 1 + p.size(),
143+
QuorumCommitmentPayloadType(p) => 1 + p.size(),
144+
MnhfSignalPayloadType(p) => 1 + p.size(),
145+
AssetLockPayloadType(p) => 1 + p.size(),
146+
AssetUnlockPayloadType(p) => 1 + p.size(),
147+
// Pre-DIP-0002 transactions have no payload section on the wire.
148+
ClassicalWithNonStandardVersionTypeBytesPayloadType(_) => 0,
135149
}
136150
}
137151

@@ -273,29 +287,37 @@ impl TransactionPayload {
273287
/// The first part for the version and the second part for the transaction
274288
/// type.
275289
///
290+
/// Pre-DIP-0002 transactions on Dash mainnet (`version == 0`) were free to put
291+
/// arbitrary bytes in the type slot. The [`ClassicalWithNonStandardVersionTypeBytes`]
292+
/// variant preserves the raw u16 read from the wire so that those transactions can
293+
/// still round-trip through `consensus_encode` / `txid` without altering the on-chain
294+
/// bytes or hashes. Logically these transactions behave as Classic.
276295
#[derive(Clone, Copy, PartialEq, Eq)]
277-
#[repr(u16)]
278296
pub enum TransactionType {
279297
/// A Classic transaction
280-
Classic = 0,
298+
Classic,
281299
/// A Masternode Registration Transaction
282-
ProviderRegistration = 1,
300+
ProviderRegistration,
283301
/// A Masternode Update Service Transaction, used by the operator to signal changes to service
284-
ProviderUpdateService = 2,
302+
ProviderUpdateService,
285303
/// A Masternode Update Registrar Transaction, used by the owner to signal base changes
286-
ProviderUpdateRegistrar = 3,
304+
ProviderUpdateRegistrar,
287305
/// A Masternode Update Revocation Transaction, used by the operator to signal termination of service
288-
ProviderUpdateRevocation = 4,
306+
ProviderUpdateRevocation,
289307
/// A Coinbase Transaction, contained as the first transaction in each block
290-
Coinbase = 5,
308+
Coinbase,
291309
/// A Quorum Commitment Transaction, used to save quorum information to the state
292-
QuorumCommitment = 6,
310+
QuorumCommitment,
293311
/// A MNHF Signal Transaction, used by masternodes to signal consensus for hard fork activations
294-
MnhfSignal = 7,
312+
MnhfSignal,
295313
/// An Asset Lock Transaction, used to transfer credits to Dash Platform, by locking them until withdrawals occur
296-
AssetLock = 8,
314+
AssetLock,
297315
/// An Asset Unlock Transaction, used to withdraw credits from Dash Platform, by unlocking them
298-
AssetUnlock = 9,
316+
AssetUnlock,
317+
/// A pre-DIP-0002 Classic transaction (`version == 0`) whose on-wire `nTxType`
318+
/// bytes were non-zero. The wrapped value is the original u16 read from the wire,
319+
/// which must be re-emitted verbatim during serialization to preserve the txid.
320+
ClassicalWithNonStandardVersionTypeBytes(u16),
299321
}
300322

301323
impl Debug for TransactionType {
@@ -311,6 +333,9 @@ impl Debug for TransactionType {
311333
MnhfSignal => write!(f, "MNHF Signal Transaction"),
312334
AssetLock => write!(f, "Asset Lock Transaction"),
313335
AssetUnlock => write!(f, "Asset Unlock Transaction"),
336+
ClassicalWithNonStandardVersionTypeBytes(raw) => {
337+
write!(f, "Classic Transaction (pre-DIP-0002, raw type bytes 0x{raw:04x})")
338+
}
314339
}
315340
}
316341
}
@@ -328,6 +353,9 @@ impl Display for TransactionType {
328353
MnhfSignal => write!(f, "MNHF Signal"),
329354
AssetLock => write!(f, "Asset Lock"),
330355
AssetUnlock => write!(f, "Asset Unlock"),
356+
ClassicalWithNonStandardVersionTypeBytes(raw) => {
357+
write!(f, "Classic (pre-DIP-0002, raw 0x{raw:04x})")
358+
}
331359
}
332360
}
333361
}
@@ -369,18 +397,44 @@ impl TransactionType {
369397
}
370398
}
371399

400+
/// Returns the on-wire `u16` representation of the transaction type.
401+
///
402+
/// For pre-DIP-0002 [`ClassicalWithNonStandardVersionTypeBytes`] this returns the
403+
/// original raw bytes that were read from the chain so that re-encoding/hashing the
404+
/// transaction reproduces the on-chain bytes verbatim.
405+
pub fn to_u16(self) -> u16 {
406+
match self {
407+
Classic => 0,
408+
ProviderRegistration => 1,
409+
ProviderUpdateService => 2,
410+
ProviderUpdateRegistrar => 3,
411+
ProviderUpdateRevocation => 4,
412+
Coinbase => 5,
413+
QuorumCommitment => 6,
414+
MnhfSignal => 7,
415+
AssetLock => 8,
416+
AssetUnlock => 9,
417+
ClassicalWithNonStandardVersionTypeBytes(raw) => raw,
418+
}
419+
}
420+
372421
/// Decodes the payload based on the transaction type.
373422
pub fn consensus_decode<R: io::Read + ?Sized>(
374423
self,
375424
d: &mut R,
376425
) -> Result<Option<TransactionPayload>, encode::Error> {
426+
// Pre-DIP-0002 transactions and Classic transactions have no payload section
427+
// on the wire — there isn't even a length prefix to consume.
377428
let _len = match self {
378-
Classic => VarInt(0),
429+
Classic | ClassicalWithNonStandardVersionTypeBytes(_) => VarInt(0),
379430
_ => VarInt::consensus_decode(d)?,
380431
};
381432

382433
Ok(match self {
383434
Classic => None,
435+
ClassicalWithNonStandardVersionTypeBytes(raw) => {
436+
Some(ClassicalWithNonStandardVersionTypeBytesPayloadType(raw))
437+
}
384438
ProviderRegistration => Some(ProviderRegistrationPayloadType(
385439
ProviderRegistrationPayload::consensus_decode(d)?,
386440
)),

dash/src/crypto/sighash.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ impl<R: Borrow<Transaction>> SighashCache<R> {
632632
self.tx.borrow().version.consensus_encode(&mut writer)?;
633633

634634
// nTransactionType (2): the nTxType of the transaction.
635-
(self.tx.borrow().tx_type() as u16).consensus_encode(&mut writer)?;
635+
self.tx.borrow().tx_type().to_u16().consensus_encode(&mut writer)?;
636636

637637
// nLockTime (4): the nLockTime of the transaction.
638638
self.tx.borrow().lock_time.consensus_encode(&mut writer)?;

key-wallet/src/psbt/map/global.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ impl Map for PartiallySignedTransaction {
3535
// without witnesses.
3636
let mut ret = Vec::new();
3737
ret.extend(encode::serialize(&self.unsigned_tx.version));
38-
(self.unsigned_tx.tx_type() as u16)
38+
self.unsigned_tx
39+
.tx_type()
40+
.to_u16()
3941
.consensus_encode(&mut ret)
4042
.expect("can't encode tx type");
4143
let input = encode::serialize(&self.unsigned_tx.input);

0 commit comments

Comments
 (0)