fix: ignore special transactions on block version 0 (pre DIP-0002)#675
Merged
QuantumExplorer merged 2 commits intodashpay:v0.42-devfrom May 4, 2026
Merged
fix: ignore special transactions on block version 0 (pre DIP-0002)#675QuantumExplorer merged 2 commits intodashpay:v0.42-devfrom
QuantumExplorer merged 2 commits intodashpay:v0.42-devfrom
Conversation
Contributor
📝 WalkthroughWalkthroughTransaction deserialization now conditionally interprets the ChangesTransaction Deserialization Logic
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
pshenmic
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #675 +/- ##
=============================================
+ Coverage 68.07% 68.76% +0.69%
=============================================
Files 319 320 +1
Lines 67661 67939 +278
=============================================
+ Hits 46057 46721 +664
+ Misses 21604 21218 -386
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
dash/src/blockdata/transaction/mod.rs (1)
595-606: ⚡ Quick winAdd a regression test for the malformed version-0 encoding.
This branch needs a unit test with a raw version-0 transaction whose type field is non-zero, so the mainnet edge case stays covered. At minimum, assert that deserialization succeeds and that the intended round-trip / txid behavior is locked in.
As per coding guidelines,
**/*.rs: Write unit tests for new functionality.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dash/src/blockdata/transaction/mod.rs` around lines 595 - 606, Add a unit test that constructs a raw version-0 transaction byte stream whose "type" field is non-zero and verifies deserialization succeeds and txid/round-trip behavior is preserved: create the test inside the transaction module (#[cfg(test)] mod tests) and feed the bytes to Transaction::consensus_decode_from_finite_reader (or the module's consensus_decode_from_finite_reader) to ensure it returns Ok(Transaction) rather than an UnknownSpecialTransactionType error; then re-serialize or compute the txid from the resulting Transaction and assert it matches the expected/round-tripped value. Use TransactionType and encode::Error in assertions to ensure the version-0 special-case remains covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dash/src/blockdata/transaction/mod.rs`:
- Around line 600-606: The fix must preserve the original on-wire
special_transaction_type_u16 for version == 0 while still treating it as
TransactionType::Classic for logic; update the struct/backing fields used by
TransactionType::try_from so that the raw u16 (or the full raw version word) is
stored (e.g., add/keep a raw_special_tx_type field) and use
TransactionType::Classic only for behavior, but have consensus_encode(), txid(),
and tx_type() re-emit the preserved raw u16 when serializing/hashing; change the
deserialization branch around
TransactionType::try_from(special_transaction_type_u16) so version == 0 assigns
TransactionType::Classic but also saves special_transaction_type_u16 into the
raw field, and update consensus_encode()/txid()/tx_type() to prefer the raw
field for output.
---
Nitpick comments:
In `@dash/src/blockdata/transaction/mod.rs`:
- Around line 595-606: Add a unit test that constructs a raw version-0
transaction byte stream whose "type" field is non-zero and verifies
deserialization succeeds and txid/round-trip behavior is preserved: create the
test inside the transaction module (#[cfg(test)] mod tests) and feed the bytes
to Transaction::consensus_decode_from_finite_reader (or the module's
consensus_decode_from_finite_reader) to ensure it returns Ok(Transaction) rather
than an UnknownSpecialTransactionType error; then re-serialize or compute the
txid from the resulting Transaction and assert it matches the
expected/round-tripped value. Use TransactionType and encode::Error in
assertions to ensure the version-0 special-case remains covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c9f0d00-6de1-4368-aa7d-be50d2abebe8
📒 Files selected for processing (1)
dash/src/blockdata/transaction/mod.rs
Comment on lines
+600
to
+606
| let special_transaction_type = if version != 0 { | ||
| TransactionType::try_from(special_transaction_type_u16).map_err(|_| { | ||
| encode::Error::UnknownSpecialTransactionType(special_transaction_type_u16) | ||
| })?; | ||
| })? | ||
| } else { | ||
| TransactionType::Classic | ||
| }; |
Contributor
There was a problem hiding this comment.
Preserve the raw type bits for version-0 transactions.
Line 600 fixes deserialization, but it also drops the original on-wire special_transaction_type_u16 when version == 0. After that, consensus_encode() and txid() re-emit/hash self.tx_type() as 0, so this malformed mainnet transaction no longer round-trips and its IDs can change after decode. If we need to treat pre-DIP-0002 transactions as Classic, we still need to retain the raw u16 (or full raw version word) somewhere so serialization and hashing stay faithful to the chain data.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dash/src/blockdata/transaction/mod.rs` around lines 600 - 606, The fix must
preserve the original on-wire special_transaction_type_u16 for version == 0
while still treating it as TransactionType::Classic for logic; update the
struct/backing fields used by TransactionType::try_from so that the raw u16 (or
the full raw version word) is stored (e.g., add/keep a raw_special_tx_type
field) and use TransactionType::Classic only for behavior, but have
consensus_encode(), txid(), and tx_type() re-emit the preserved raw u16 when
serializing/hashing; change the deserialization branch around
TransactionType::try_from(special_transaction_type_u16) so version == 0 assigns
TransactionType::Classic but also saves special_transaction_type_u16 into the
raw field, and update consensus_encode()/txid()/tx_type() to prefer the raw
field for output.
5 tasks
QuantumExplorer
added a commit
that referenced
this pull request
May 4, 2026
…ons (#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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
There is one malformed block in mainnet network with special transaction type set to 8192 on pre DIP-0002 blocks. This is a block before special transaction were introduced in the network, and shouldn't even been set to 8192 (it should have been just 0), however Dash Core software tolerated this and as a result such block was included in the chain.
Rust dashcore, when tries to decode such block from hex, tries to decode the field but fails to match to any known special transaction type from the codebase eventually failing with:
Here is the reference for that block
842284
Proposed solution
Simply decode all transaction as
TransactionType::Classicif the block version is 0Summary by CodeRabbit