[shumkov]

Adds Transaction::deserialize_with_legacy_asset_lock_framing, a frozen legacy decoder in its own module (dash/src/blockdata/transaction/legacy_asset_lock.rs) that reads an asset lock's payload after its length prefix whatever length the prefix announces. Dash Platform needs it to keep decoding instant asset lock proofs the way every Platform release did.

Why Platform needs it

An instant asset lock proof carries a Core asset lock transaction as raw bytes the sender chooses. Before #1154, this crate read the asset lock payload length prefix and ignored its value. Since #1154 it reads every special payload strictly through its prefix, as Core does. Neither the InstantSend lock (checked against txid() of the re-encoded transaction) nor the Platform transition's signature covers the prefix, so anyone relaying a pending funding transition can change it. Platform releases built on earlier pins accept and charge for such a proof; a strict decoder refuses it. Mixed node binaries would disagree on a paid transition.

Platform v5.0.0-rc.1 restored the old reading with a byte-level workaround on its side (dashpay/platform#5403: zero the type bytes of a copy, decode as classic up to the payload, read and ignore the prefix, decode the payload). This PR moves that decoding into rust-dashcore as a named function, so Platform can drop the workaround.

The function

impl Transaction {
    pub fn deserialize_with_legacy_asset_lock_framing(
        bytes: &[u8],
    ) -> Result<Transaction, encode::Error>;
}

• Asset lock (version ≥ 3 read as i16, type 8): inputs, the BIP144 form when there are no inputs (as for a classic transaction), outputs, lock time, then a CompactSize that must be minimal but whose value is ignored, then the payload (version byte, credit outputs), which must end the bytes.
• Anything else goes to deserialize.

It is standalone rather than a mode of Decodable for Transaction, and it writes the payload layout out instead of calling AssetLockPayload::consensus_decode. That way a later transaction rule or a new asset lock payload layout cannot change what it accepts. It makes the same finite-reader calls as the transaction decoder, so its output, errors included, is identical to the Platform workaround's.

It lives in its own private module, an impl Transaction block in legacy_asset_lock.rs, so transaction/mod.rs keeps only the mod line. The name and the doc comments say plainly that it is a legacy decoder and that its framing must never change, and the doc tells callers to prefer deserialize.

It is deliberately neither #[deprecated] nor behind a cargo feature. Platform has to call it for as long as it must agree with its older binaries and replay its chain, so there is nothing to migrate to, and a feature would only allow building Platform without it.

Tests

dash/tests/legacy_asset_lock.rs (46 tests) pins absolute results: fixed bytes, txids and error variants, built from a testnet asset lock Core relayed (txid 81a1d138…5dcc5581).

• Accepted:
  • 1/3/5/9-byte prefixes with any value;
  • versions 4 and 0x7fff;
  • the witness form, including a witness on one of two inputs;
  • the zero-input form;
  • a 274-byte payload read after a 1-byte prefix;
  • a payload with no credit outputs;
  • payload versions 0/2/3/255 with one layout.
• Refused:
  • non-minimal prefix or credit-output count;
  • every kind of truncation;
  • a byte after the payload, counted by the prefix or not (also 1/4/32-byte appended fields at every payload version);
  • segwit flags 0 and 2;
  • the flag without witnesses;
  • a payload after a classic transaction.
• Delegated (compared with deserialize): a mainnet coinbase, a testnet ProRegTx, an unknown type, type 8 below version 3.
• Seeded round-trip property: whatever it decodes reads back unchanged from its encoding (and through deserialize at payload version 1).

Each of these changes to the function turns tests red:

Change	Red
delegating everything to deserialize	22 / 46
dropping the segwit rule	12
checking any instead of all witnesses empty	2
accepting only version 3	2
comparing the version unsigned	1
refusing empty credit outputs	2
dropping the trailing-bytes check	6

A differential run against Platform's workaround, over 16M seeded mutations on dev and on 7302d0a6, found no Ok/Err, decoded-transaction or error-string mismatch.

cargo test -p dashcore --all-features, cargo clippy -p dashcore --all-features --all-targets -- -D warnings and RUSTDOCFLAGS=-D warnings cargo doc -p dashcore --all-features --no-deps pass.

Platform pin

Platform v5.0 pins integration/v5.0-core-v24 (7302d0a6 + Core v24 fixes). The commits here are cherry-picked there too (a5ae6375, edc4d430), with two adaptations: the import conflict with #1157, and hex::decode in the test helper, since hex-conservative is not a dependency at that rev. All 46 tests pass there unchanged.

🤖 Generated with Claude Code

PR Hygiene · 28c1a04

• Bots — coderabbitai skipped after its own rate limit
• Self-review — post /self-reviewed
• Build green
• Approvals
  • files with no dedicated owner (dash/src/blockdata/transaction/mod.rs, dash/tests/legacy_asset_lock.rs) — QuantumExplorer or ZocoLini or xdustinface

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

Summary by CodeRabbit

• New Features
  • Added support for decoding legacy AssetLock transactions using their historical payload framing, including witness-encoded transactions. The decoder also handles other transaction types through standard decoding, allowing callers to use one entry point for both legacy AssetLock data and regular transactions.
• Bug Fixes
  • Invalid witness flags, incomplete payloads, and trailing data are rejected during decoding.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: dashpay/rust-dashcore/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: c7b7c071-e26b-4217-9fbc-ebaccc5664f3

📥 Commits

Reviewing files that changed from the base of the PR and between a6cdc20 and f9f195c.

📒 Files selected for processing: 3

• dash/src/blockdata/transaction/legacy_asset_lock.rs
• dash/src/blockdata/transaction/mod.rs
• dash/tests/legacy_asset_lock.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The change adds a transaction decoder for legacy asset-lock framing. It handles payload and witness data, rejects specified malformed inputs, and uses the standard deserializer for other transactions. Tests cover decoding, errors, compatibility, and round trips.

Changes: Legacy asset-lock decoding

Layer / File(s)	Summary
Asset-lock framing decoder and validation dash/src/blockdata/transaction/mod.rs, dash/src/blockdata/transaction/legacy_asset_lock.rs, dash/tests/legacy_asset_lock.rs	The transaction module declares the decoder, which parses legacy asset-lock payloads and handles witness framing. Tests cover payload lengths and versions, malformed and truncated data, non-asset-lock transactions, and deterministic mutation round trips.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to f9f19

The legacy decoder appears mergeable with normal checks; the supplied evidence establishes no issue requiring a fix before merge.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: adding Dash asset-lock decoding that ignores the payload length prefix.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.01%. Comparing base (a6cdc20) to head (f9f195c).
⚠️ Report is 6 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1167      +/-   ##
==========================================
+ Coverage   78.76%   79.01%   +0.25%     
==========================================
  Files         302      303       +1     
  Lines       80644    81293     +649     
==========================================
+ Hits        63518    64236     +718     
+ Misses      17126    17057      -69     

Flag	Coverage Δ
core	77.55% <100.00%> (+0.29%)	⬆️
ffi	52.12% <ø> (+0.16%)	⬆️
rpc	51.61% <ø> (+2.25%)	⬆️
spv	92.76% <ø> (+0.09%)	⬆️
wallet	82.19% <ø> (+0.06%)	⬆️

Files with missing lines	Coverage Δ
...ash/src/blockdata/transaction/legacy_asset_lock.rs	100.00% <100.00%> (ø)
dash/src/blockdata/transaction/mod.rs	90.00% <ø> (ø)

... and 34 files with indirect coverage changes

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.