Skip to content

Conversation

@xdustinface
Copy link
Collaborator

@xdustinface xdustinface commented Jan 31, 2026

The block serialization needed by the sync rewrite which uses #397 fails for special transactions/coinbase because we don't have segwit support. This is intermediate fix. Im have a PR to fully drop support for it (#400) but its not quite ready and i want to look a bit more into it first once the sync stuff is done.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved handling of special transaction types (AssetUnlock, QuorumCommitment, MnhfSignal, Coinbase) to ensure correct serialization and deserialization during transaction processing.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 31, 2026

📝 Walkthrough

Walkthrough

Expanded special transaction type handling in serialization and deserialization to disable SegWit processing for AssetUnlock, QuorumCommitment, MnhfSignal, and Coinbase transaction types. Updated supporting documentation to reflect the broader scope of non-input transaction types.

Changes

Cohort / File(s) Summary
SegWit Handling for Special Transaction Types
dash/src/blockdata/transaction/mod.rs
Extended Encodable and Decodable implementations to disable SegWit-related processing (have_witness/segwit flag) for AssetUnlock, QuorumCommitment, MnhfSignal, and Coinbase transaction types. Updated comments to clarify rationale for blocking SegWit on these special transaction types.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Hoppy times with special types so rare,
No SegWit witnesses need they share,
Coinbase, Quorum, Asset too—
Signal strong what Dash should do,
Serialization safe and clean,
Simplest fix we've ever seen!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding missing special transaction types (AssetUnlock, QuorumCommitment, MnhfSignal, Coinbase) to the serialization logic to handle SegWit-related processing correctly.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/block-serilization

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants