fix(dpp): use DIP-0002 version 3 in asset-lock tx fixtures

[QuantumExplorer]

Issue being fixed or feature implemented

After the rust-dashcore bump in #3617 (v0.42-dev, commit 428b60d), three round-trip tests in packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs started failing deterministically on v3.1-dev:

• test_raw_instant_lock_proof_preserves_output_index
• test_raw_instant_lock_proof_round_trip
• test_try_from_value_round_trip

All panicked with DecodingError("parse failed: data not consumed entirely when explicitly deserializing").

What was done?

Root cause. rust-dashcore PR dashpay/rust-dashcore#726 (in commit d6dd5da) tightened Transaction decoding to fix a real mainnet round-trip bug:

• version >= 3 → resolve nTxType via TransactionType::try_from, consume the payload normally.
• version < 3 and nTxType == 0 → Classic, no payload section.
• version < 3 and nTxType != 0 → new ClassicalWithNonStandardVersionTypeBytes(raw) variant, no payload section is consumed (treated like Classic on the wire).

The fixture in packages/rs-dpp/src/tests/fixtures/instant_asset_lock_proof_fixture.rs was constructing a Transaction with version: 0 plus Some(AssetLockPayloadType(...)). Encodable derives tx_type() from the payload (AssetLock, u16 = 8) and writes the payload bytes after lock_time. Post-#726, decode now hits the new ClassicalWithNonStandardVersionTypeBytes(8) branch and never reads those payload bytes — leaving them unconsumed and tripping the data not consumed entirely error.

The fixture was always invalid: real DIP-0002 special transactions (including AssetLock) use version: 3. The old decoder masked the bug because it didn't gate behavior on version. The dashcore change is intentional and shouldn't be reverted.

Fix. Two-line change to make the fixture's transaction match real DIP-0002 wire format:

• packages/rs-dpp/src/tests/fixtures/instant_asset_lock_proof_fixture.rs — set version: 3 (was 0) on the AssetLock transaction.
• packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs — updated test_transaction_accessor's version assertion from 0 to 3 to match.

No dpp serialization-code change is needed; the fixture was the only thing constructing a malformed version=0 + special payload combination.

How Has This Been Tested?

$ cargo test -p dpp --lib instant_asset_lock_proof::tests
test result: ok. 21 passed; 0 failed; 0 ignored

$ cargo test -p dpp --lib
test result: ok. 3422 passed; 0 failed; 6 ignored

cargo fmt --all clean.

Breaking Changes

None. Test-fixture-only change.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

Summary by CodeRabbit

• Tests
  • Updated instant asset lock transaction fixtures to use the DIP-0002 transaction version so validation tests reflect the spec.
  • Made voting strategy tests resilient by locating contenders by identifier before asserting document presence and vote counts.
  • Updated an identity/document test expected root hash.
  • Adjusted expected total-credit balances in two withdrawal strategy tests.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: abc676dd-c81f-421b-b5ef-86130d312137

📥 Commits

Reviewing files that changed from the base of the PR and between b71e1af and 56776ea.

📒 Files selected for processing (5)

• packages/rs-dpp/src/identity/state_transition/asset_lock_proof/validate_asset_lock_transaction_structure/v0/mod.rs
• packages/rs-drive-abci/tests/strategy_tests/test_cases/identity_and_document_tests.rs
• packages/rs-drive-abci/tests/strategy_tests/test_cases/voting_tests.rs
• packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs
• packages/strategy-tests/src/transitions.rs

✅ Files skipped from review due to trivial changes (3)

• packages/strategy-tests/src/transitions.rs
• packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs
• packages/rs-dpp/src/identity/state_transition/asset_lock_proof/validate_asset_lock_transaction_structure/v0/mod.rs

---

📝 Walkthrough

Walkthrough

Fixtures and test helpers for instant asset lock now construct Transactions with version 3; unit tests asserting transaction versions were updated. Additionally, multiple strategy tests adjusted voting contender selection to identifier-based lookup, updated expected root hash, and tweaked two withdrawal balance assertions.

Changes

Instant Asset Lock Proof Version Compliance

Layer / File(s)	Summary
Fixture and fixture-usage updates packages/rs-dpp/src/tests/fixtures/instant_asset_lock_proof_fixture.rs, packages/strategy-tests/src/transitions.rs, packages/rs-dpp/src/identity/state_transition/asset_lock_proof/validate_asset_lock_transaction_structure/v0/mod.rs	Transaction fixtures and test builders now construct instant asset lock Transaction objects with version: 3 instead of 0.
Test assertion updates packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs	Unit test assertion updated to expect fixture transaction version == 3.

Strategy Tests Expectations

Layer / File(s)	Summary
Voting tests: contender selection and counts packages/rs-drive-abci/tests/strategy_tests/test_cases/voting_tests.rs	Contender verification changed from position-based (first/last) to identifier-based lookup; expected document hex values and per-identity vote_count assertions updated accordingly.
Identity/document root and withdrawal balances packages/rs-drive-abci/tests/strategy_tests/test_cases/identity_and_document_tests.rs, packages/rs-drive-abci/tests/strategy_tests/test_cases/withdrawal_tests.rs	Updated expected grove root_hash constant and adjusted two hard-coded total_identity_balances expected values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A tiny hop from zero to three,
Fixtures align, tests agree,
Contenders found by their name,
Balances trimmed, constants tame,
The build sighs—green as can be.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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: updating AssetLock transaction fixtures to use DIP-0002 version 3 instead of version 0, which is the core purpose of this fix.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/intelligent-shaw-492497

---

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 and usage tips.}

[thepastaclaw]

Review Gate

Commit: 56776ead

• ⏳ Debounce: 0m ago (need 30m)

• ⏳ CI checks: checks still running (2 pending)

• ✅ CodeRabbit review: comment found

• ✅ Off-peak hours: off-peak (12:45 AM PT Friday)

• Run review now (check to override)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.29%. Comparing base (c7cb8e6) to head (56776ea).

Additional details and impacted files

@@            Coverage Diff            @@
##           v3.1-dev    #3621   +/-   ##
=========================================
  Coverage     88.29%   88.29%           
=========================================
  Files          2479     2479           
  Lines        301659   301659           
=========================================
+ Hits         266346   266355    +9     
+ Misses        35313    35304    -9     

Components	Coverage Δ
dpp	87.96% <100.00%> (+<0.01%)	⬆️
drive	87.37% <ø> (ø)
drive-abci	90.25% <ø> (+<0.01%)	⬆️
sdk	∅ <ø> (∅)
dapi-client	∅ <ø> (∅)
platform-version	∅ <ø> (∅)
platform-value	92.17% <ø> (ø)
platform-wallet	∅ <ø> (∅)
drive-proof-verifier	55.66% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[thepastaclaw]

Code Review

I verified the checked-out worktree at 64a45ab083edc23c004dd98f7fafd625b3309248 and reviewed the three commits that make up this PR’s asset-lock fix and follow-up test rebaseline. The changed fixture and helper builders now consistently use outer Transaction.version = 3 for AssetLock special transactions while leaving the payload’s internal version at 0, which matches DIP-0002 and explains the txid and identity-id shifts reflected in the strategy-test updates. I did not find a concrete correctness, consensus, or test-masking issue in the reviewed changes; targeted dpp asset-lock tests passed in this checkout.

Reviewed commit: 64a45ab

[QuantumExplorer]

Self Reviewed