test(dpp): add deserialization failure tests and fix stale test structs

[thepastaclaw]

Summary

Add negative deserialization tests, fix stale test struct definitions, and complete discarded error round-trips in rs-dpp.

Changes

C-08: Complete discarded round-trips in error serialization tests

• unsupported_protocol_version_error.rs and unsupported_version_error.rs had tests that serialized a ConsensusError but assigned the result to _cbor (discarding it). The commented-out deserialization used an old Value-based API.
• Replaced with working PlatformDeserializable round-trip: serialize → deserialize → assert equality.

C-14: Fix stale IdentityCreditWithdrawalTransition test structs

• The test module defined 10 progressively-growing structs (V01–V010) with fields (protocol_version, transition_type, revision) that don't exist in the actual IdentityCreditWithdrawalTransitionV0.
• Replaced with 9 structs (V01–V09) that progressively add the real fields: identity_id, amount, core_fee_per_byte, pooling, output_script, nonce, user_fee_increase, signature_public_key_id, signature.

C-10: Add negative deserialization tests for StateTransition

• deserialize_empty_bytes_should_fail — empty slice returns Err
• deserialize_single_byte_should_fail — single 0xFF byte returns Err
• deserialize_truncated_bytes_should_fail — truncated to half, minus last byte, first byte only
• deserialize_corrupted_bytes_should_not_panic — bit-flip in middle of payload
• deserialize_many_empty_list — empty vec returns Ok(vec![])
• deserialize_many_with_invalid_entry — invalid bytes returns Err
• deserialize_many_with_valid_entries — round-trip 3 valid transitions

Testing

All 487 tests pass with cargo test -p dpp --features random-identities,state-transition-signing.

Summary by CodeRabbit

• Tests
  • Expanded test coverage for serialization and deserialization operations with comprehensive error handling validation.
  • Added tests for corrupted and invalid data scenarios to verify proper failure modes and system robustness.
  • Enhanced identity transaction validation testing to ensure data consistency and reliable error recovery.

Validation

1. What was tested

   • CI checks on this PR (Rust packages linting, tests, builds)

2. Results

   • All Rust CI checks passing

3. Environment

   • GitHub Actions CI for dashpay/platform

[coderabbitai]

📝 Walkthrough

Walkthrough

Test improvements across error handling and state transition modules, adding round-trip serialization/deserialization validation and expanding deserialization test coverage with new edge case scenarios.

Changes

Cohort / File(s)	Summary
Consensus Error Tests packages/rs-dpp/src/errors/consensus/basic/unsupported_protocol_version_error.rs, packages/rs-dpp/src/errors/consensus/basic/unsupported_version_error.rs	Tests refactored to verify round-trip serialization and deserialization of ConsensusError with platform versions instead of direct CBOR computation. Added PlatformDeserializable imports.
State Transition Deserialization Tests packages/rs-dpp/src/state_transition/serialization.rs	Comprehensive unit test suite added covering empty bytes, truncated streams, corrupted payloads, and deserialize_many operations. Tests include guards for random-identities feature and negative/positive test cases.
Identity Withdrawal Transition Tests packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v0/mod.rs	Test variant structs updated to replace deprecated protocol_version and transition_type fields with identity_id and additional fields (amount, core_fee_per_byte, pooling, output_script, nonce, user_fee_increase, signature_public_key_id, signature). Import adjustments made for unused declarations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Round-trip hops through serialize streams,
Test assertions validate our dreams,
Corrupted bytes meet their end,
As deserialization tests defend,
Identity fields fresh and bright,
Our transitions pass each test tonight! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: adding deserialization failure tests and fixing stale test structs. It directly matches the PR objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/rs-dpp/src/state_transition/serialization.rs (1)

433-511: Optional: drop the random-identities feature gate by not requiring a full Identity.

Both deserialize_truncated_bytes_should_fail and deserialize_corrupted_bytes_should_not_panic use Identity::random_identity exclusively to obtain identity.id(). The Identifier::random() helper in platform_value has no random-identities dependency and would produce an equally valid Identifier. Removing the full identity construction lets these tests run in every CI configuration.

♻️ Proposed simplification for both tests

-    #[test]
-    #[cfg(feature = "random-identities")]
+    #[test]
     fn deserialize_truncated_bytes_should_fail() {
-        let platform_version = PlatformVersion::latest();
-        let identity = Identity::random_identity(5, Some(5), platform_version)
-            .expect("expected a random identity");
         let transition = IdentityCreditWithdrawalTransitionV0 {
-            identity_id: identity.id(),
+            identity_id: platform_value::Identifier::random(),
             amount: 5000000,

Apply the same substitution to deserialize_corrupted_bytes_should_not_panic.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/rs-dpp/src/state_transition/serialization.rs` around lines 433 -
511, Tests deserialize_truncated_bytes_should_fail and
deserialize_corrupted_bytes_should_not_panic are gated on the random-identities
feature only to obtain identity.id(); replace that by generating a lightweight
Identifier via Identifier::random() (from platform_value) and use its value for
identity_id in IdentityCreditWithdrawalTransitionV0 so the tests no longer need
the random-identities feature; remove the #[cfg(feature = "random-identities")]
annotation from both test functions and update imports to include
platform_value::Identifier (or the crate path used) so
Identity::random_identity(...) calls and the feature gate can be dropped while
keeping IdentityCreditWithdrawalTransitionV0, StateTransition,
serialize_to_bytes and deserialize_from_bytes usage unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/rs-dpp/src/state_transition/serialization.rs`:
- Around line 433-511: Tests deserialize_truncated_bytes_should_fail and
deserialize_corrupted_bytes_should_not_panic are gated on the random-identities
feature only to obtain identity.id(); replace that by generating a lightweight
Identifier via Identifier::random() (from platform_value) and use its value for
identity_id in IdentityCreditWithdrawalTransitionV0 so the tests no longer need
the random-identities feature; remove the #[cfg(feature = "random-identities")]
annotation from both test functions and update imports to include
platform_value::Identifier (or the crate path used) so
Identity::random_identity(...) calls and the feature gate can be dropped while
keeping IdentityCreditWithdrawalTransitionV0, StateTransition,
serialize_to_bytes and deserialize_from_bytes usage unchanged.

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[PastaPastaPasta]

seems useful to have over not