test: improve coverage for dpp and drive-proof-verifier#3504
Merged
QuantumExplorer merged 2 commits intoApr 21, 2026
Conversation
Add 171 new unit tests across 10 files targeting the highest bang-for-buck coverage gaps identified from Codecov: - rs-drive-proof-verifier/proof.rs (45% → covered): 31 tests exercising the Length trait impls for all four container variants, IntoOption semantics, parse_key_request_type happy/error branches, and FromProof decode-error paths for Identity (NoProofInResult, EmptyResponseMetadata, EmptyVersion, ProtocolError), Identity by public_key_hash, Identity by non-unique hash (all four decode branches), IdentityBalances/DataContracts bad id length, GetProtocolVersionUpgradeVoteStatus, GetPathElements, GetPrefundedSpecializedBalance, GetEpochsInfo overflow, and BroadcastStateTransition garbage bytes. A module-scoped UnreachableContextProvider proves errors exit before any crypto. - rs-dpp/state_transition/mod.rs (55% → covered): 25 tests hitting name(), state_transition_type(), is_identity_signed(), signature/owner_id/signature_public_key_id accessors, setters (including MasternodeVote no-op), required_number_of_private_keys default arm, active_version_range ALL_VERSIONS arm, unique_identifiers/transaction_id determinism, From impls via derive_more, and PlatformSerializable roundtrip. - rs-dpp/data_contract/created_data_contract (20% → covered): 14 tests covering from_contract_and_identity_nonce, all accessors, From impl to DataContract, both serialize paths producing identical bytes, deserialize roundtrip + garbage/empty error arms. - rs-dpp/identity/identity_public_key/ (52% → covered): 69 tests across 5 files. purpose.rs (13), security_level.rs (15), contract_bounds (9), v0/methods (9 for public_key_hash + validate_private_key_bytes error paths), mod.rs (23 for is_master/max_possible_size_key/default_versioned/random key generators/main_keys helpers). - rs-dpp/identity/identity_factory.rs (43% → covered): 14 tests across create_from_buffer happy+error paths, all create_identity_*_transition variants including V0/V1 withdrawal branches and UnknownVersionMismatch. - rs-dpp/document/specialized_document_factory (44% → covered): 18 tests hitting get-version error arms, create_document happy paths, create_state_transition Create/Delete/Replace branches, NoDocumentsSupplied/MismatchedOwners/RevisionAbsent/InvalidInitialRevision error arms, nonce accumulator, and extended-document roundtrip. No production bugs surfaced. All tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 47 minutes and 11 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request adds comprehensive unit test coverage across multiple modules in the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Comment |
Collaborator
Review GateCommit:
|
QuantumExplorer
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3504 +/- ##
============================================
+ Coverage 85.05% 85.30% +0.25%
============================================
Files 2476 2476
Lines 269313 270705 +1392
============================================
+ Hits 229062 230938 +1876
+ Misses 40251 39767 -484
🚀 New features to boost your workflow:
|
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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 (10)
packages/rs-dpp/src/identity/identity_public_key/v0/methods/mod.rs (1)
244-445: Optional: extract a test helper to reduce duplicatedIdentityPublicKeyV0literals.The new tests repeat the full
IdentityPublicKeyV0 { id: 0, purpose: …, security_level: …, contract_bounds: None, key_type, data, read_only: false, disabled_at: None }literal 9 times, differing only inkey_typeanddata. A small helper (or a..Default::default()spread, sinceIdentityPublicKeyV0derivesDefault) would make intent clearer and reduce churn if the struct gains new fields.♻️ Example refactor
fn key_with(key_type: KeyType, data: Vec<u8>) -> IdentityPublicKeyV0 { IdentityPublicKeyV0 { key_type, data: platform_value::BinaryData::new(data), ..Default::default() } }Then each test becomes a one-liner like:
let key = key_with(KeyType::ECDSA_SECP256K1, vec![1u8; 32]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/identity/identity_public_key/v0/methods/mod.rs` around lines 244 - 445, Extract a small test helper to avoid repeating the IdentityPublicKeyV0 literal: add a function like key_with(key_type: KeyType, data: Vec<u8>) -> IdentityPublicKeyV0 that constructs an IdentityPublicKeyV0 using the provided key_type and data (wrapping data with platform_value::BinaryData::new) and fills the rest via ..Default::default(); then replace the repeated literals in tests like test_public_key_hash_empty_data_errors, test_public_key_hash_ecdsa_wrong_length_errors, test_public_key_hash_bls_wrong_length_errors, test_public_key_hash_bls_returns_ripemd160_sha256_of_data, test_public_key_hash_ecdsa_hash160_returns_data_itself, test_public_key_hash_bip13_script_hash_returns_data_itself, test_public_key_hash_hash160_wrong_length_errors, test_validate_private_key_bytes_bip13_script_hash_is_unsupported, test_validate_private_key_bytes_ecdsa_secret_key_parse_error_returns_false, and test_validate_private_key_bytes_ecdsa_hash160_secret_key_parse_error_returns_false to call key_with(...) with the differing key_type and data.packages/rs-dpp/src/identity/identity_public_key/purpose.rs (1)
223-236: Consider renamingfull_range()or adding a doc comment to clarify SYSTEM is excluded.The method returns 6 of 7 Purpose variants, intentionally omitting SYSTEM (which appears to be system-internal only). The doc comment "The full range of purposes" is misleading; consider either renaming to
assignable_purposes()/user_purposes()or adding a doc comment explaining that SYSTEM is excluded by design.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/identity/identity_public_key/purpose.rs` around lines 223 - 236, Rename or clarify Purpose::full_range() to indicate it excludes SYSTEM by either renaming the method (e.g., assignable_purposes() or user_purposes()) or keeping the name but adding a doc comment explicitly stating that SYSTEM is intentionally omitted; update any callers/tests (e.g., the test_purpose_full_range_contents test and references to full_range()) to use the new name or reflect the clarified behavior and ensure the doc comment mentions that full_range()/assignable_purposes() returns the six purpose variants (AUTHENTICATION, ENCRYPTION, DECRYPTION, TRANSFER, VOTING, OWNER) and excludes SYSTEM which is system-internal only.packages/rs-dpp/src/data_contract/created_data_contract/mod.rs (2)
328-340: Assert the full round-trip value, not just nonce and ID.This can pass even if serialization drops or mutates other contract fields. Since
CreatedDataContractalready implementsPartialEq, compare the restored value directly.Strengthen the round-trip assertion
let restored = CreatedDataContract::versioned_deserialize(&bytes, false, platform_version) .expect("deserialize should succeed"); - assert_eq!(restored.identity_nonce(), created.identity_nonce()); - assert_eq!(restored.data_contract().id(), created.data_contract().id()); + assert_eq!(restored, created);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/data_contract/created_data_contract/mod.rs` around lines 328 - 340, The test serialize_roundtrip_via_platform_version only compares identity_nonce() and data_contract().id(), which can miss other mutated fields; update the assertion to compare the whole CreatedDataContract using PartialEq by replacing the two field-level asserts with a single assert_eq!(restored, created) after versioned_deserialize (keep the same setup using sample_created, serialize_to_bytes_with_platform_version, and CreatedDataContract::versioned_deserialize to perform the round-trip).
386-405: Validate both values returned by the helper.
data_contract_and_identity_nonce_owned()returns the contract format and nonce, but this test ignores the contract. A swapped or wrong contract payload would still pass.Cover the returned contract format too
- let (decoded, _consumed) = bincode::borrow_decode_from_slice::< + let (decoded, consumed) = bincode::borrow_decode_from_slice::< CreatedDataContractInSerializationFormat, _, >(&bytes, config) .expect("raw bincode decode should succeed"); - let (_contract_fmt, nonce) = decoded.data_contract_and_identity_nonce_owned(); + assert_eq!(consumed, bytes.len()); + let (contract_fmt, nonce) = decoded.data_contract_and_identity_nonce_owned(); + let contract = DataContract::try_from_platform_versioned( + contract_fmt, + false, + &mut vec![], + platform_version, + ) + .expect("contract format should convert back"); + assert_eq!(contract.id(), created.data_contract().id()); assert_eq!(nonce, created.identity_nonce());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/data_contract/created_data_contract/mod.rs` around lines 386 - 405, The test only asserts the returned nonce from data_contract_and_identity_nonce_owned(); update it to also validate the returned contract format: destructure as let (contract_fmt, nonce) = decoded.data_contract_and_identity_nonce_owned(); then assert_eq!(nonce, created.identity_nonce()) and assert_eq!(contract_fmt, created.data_contract()) (or the appropriate accessor on sample_created() that returns the expected contract) so the contract payload is also verified; use CreatedDataContractInSerializationFormat::data_contract_and_identity_nonce_owned(), sample_created(), and created.identity_nonce() to locate the code to change.packages/rs-dpp/src/document/specialized_document_factory/v0/mod.rs (3)
930-954: Assertion doesn't cover what the test name claims.
create_state_transition_replace_on_mutable_document_increments_revisiondoesn't actually assert that the revision was incremented — it only checkstransitions_lenand the nonce. Consider reaching into the produced transition (DocumentReplaceTransition) and assertingrevision == INITIAL_REVISION + 1, otherwise a regression indocument.set_revision(...revision + 1)insidedocument_replace_transitionswould silently slip through.Example assertion
let batch = factory .create_state_transition(entries, &mut nonce_counter) .expect("replace transition should be built"); assert_eq!(batch.transitions_len(), 1); assert_eq!(batch.owner_id(), owner_id); + // Replace must bump the revision from INITIAL_REVISION to INITIAL_REVISION + 1. + match batch.transitions().first().expect("one transition") { + DocumentTransition::Replace(r) => { + assert_eq!(r.base().revision(), INITIAL_REVISION + 1); + } + other => panic!("expected Replace, got {other:?}"), + } let key = (owner_id, data_contract.id()); assert_eq!(*nonce_counter.get(&key).unwrap(), 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/document/specialized_document_factory/v0/mod.rs` around lines 930 - 954, The test create_state_transition_replace_on_mutable_document_increments_revision currently never checks the produced transition's revision; update the test to extract the created transition from batch (the result of factory.create_state_transition), cast/inspect it as a DocumentReplaceTransition, and assert its revision equals INITIAL_REVISION + 1 (i.e., the document's revision was incremented). Locate this in the test body after batch is built and add an assertion against the transition's revision field on the DocumentReplaceTransition to prevent regressions in the document.set_revision logic.
1161-1183: Negative-path tests are a bit permissive.Both
create_extended_from_document_buffer_invalid_type_failsandcreate_extended_from_document_buffer_bad_bytes_failsonly assertis_err(). Since the two code paths produce distinctly shaped errors (InvalidDocumentTypeErrorvs. a deserialization error fromDocument::from_bytes), matching on the specific variant would better protect against future refactors silently collapsing them into a generic error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/document/specialized_document_factory/v0/mod.rs` around lines 1161 - 1183, Update the two tests so they assert the specific error variants instead of only is_err(): in create_extended_from_document_buffer_invalid_type_fails call factory.create_extended_from_document_buffer and assert the error is the InvalidDocumentTypeError variant (e.g., by pattern matching or using assert_matches!), and in create_extended_from_document_buffer_bad_bytes_fails assert that the error comes from the deserialization failure raised by Document::from_bytes (match the deserialization/error variant returned by create_extended_from_document_buffer). Use the actual enum/variant names returned by the factory error type when matching to ensure the tests fail if the error shape changes.
801-828: Test name promises more than it asserts.
create_document_uses_given_time_based_propertiesnever checks thatblock_time(12345) orcore_block_height(67) actually ended up on the document. As written this is really just "create works with non-zero block time". Consider assertingdoc.created_at()/ any core-height-derived field to match the test name, or rename to e.g.create_document_with_block_time_succeeds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/document/specialized_document_factory/v0/mod.rs` around lines 801 - 828, The test create_document_uses_given_time_based_properties currently never verifies the supplied block_time/core_block_height; update the test to assert that the document's time fields reflect the inputs by checking doc.created_at() and doc.updated_at() equal Some(block_time) (or the concrete return type used by Document) and also assert the core-height-derived field (e.g., doc.core_height() or the specific getter used in this codebase) equals core_block_height; if those accessors do not exist, either add the appropriate getters or rename the test to create_document_with_block_time_succeeds to match the current assertions.packages/rs-dpp/src/identity/identity_factory.rs (2)
452-474: Good negative-path coverage forcreate_from_buffer.Both the garbage-bytes and empty-buffer branches exercise the
SerializedObjectParsingErrormapping. Assertingresult.is_err()is acceptable, but if you want to lock in the error shape (so a future refactor can't silently swallow the deserialization error into a different variant), matching onErr(ProtocolError::ConsensusError(...))/BasicError::SerializedObjectParsingErrorwould be stronger.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/identity/identity_factory.rs` around lines 452 - 474, Update the two tests (create_from_buffer_fails_on_garbage and create_from_buffer_fails_on_empty) to assert the specific error shape instead of only is_err(); call IdentityFactory::create_from_buffer and match the returned Result to Err(ProtocolError::ConsensusError(BasicError::SerializedObjectParsingError(_))) (or use matches!/assert_matches! to keep it concise), preserving the existing #[cfg(feature = "validation")] argument; this locks the expectation to the SerializedObjectParsingError variant wrapped in ProtocolError::ConsensusError so future changes won't silently change the error type.
688-711: Revision assertion is coincidentally correct — make intent explicit.
create_identity_update_transitiondoesidentity.revision() + 1, so pre-seedingv0.revision = 5and assertingupdate.revision() == 6is fine. Consider adding a brief assertion on the starting revision (e.g.assert_eq!(identity.revision(), 5);before callingcreate_identity_update_transition) so a future change toIdentityV0::revisiondefault or the+1arithmetic fails loudly at the right spot rather than producing a confusing equality failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/identity/identity_factory.rs` around lines 688 - 711, Add an explicit assertion of the identity's starting revision before calling create_identity_update_transition so the test documents and locks the precondition: after seeding v0.revision = 5, add assert_eq!(identity.revision(), 5); immediately before calling IdentityFactory::create_identity_update_transition(identity, 33, ...), then keep the existing check that update.revision() == 6; this makes the intent around IdentityV0::revision and the +1 behavior explicit and fails closer to the root if defaults change.packages/rs-dpp/src/state_transition/mod.rs (1)
1808-1818: Consider also covering the error branch ofdeserialize_from_bytes_in_version.The happy-path is covered, but the interesting branch (version not in
active_version_rangeandprotocol_version <= 268435456) only fires underall(feature = "state-transitions", feature = "validation"). A feature-gated negative test would close the real coverage gap on this function; as written, under configurations without both features, the function is effectively a thin wrapper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-dpp/src/state_transition/mod.rs` around lines 1808 - 1818, Add a feature-gated negative test that exercises StateTransition::deserialize_from_bytes_in_version's error branch when the provided protocol version is outside the active_version_range and protocol_version <= 268435456; specifically, create a test (annotated with #[cfg(all(feature = "state-transitions", feature = "validation"))]) that serializes a sample StateTransition (e.g., sample_transfer_st()), mutates the serialized bytes or the passed PlatformVersion to a version not included in active_version_range, calls StateTransition::deserialize_from_bytes_in_version(&bytes, PlatformVersion::new(<small_or_invalid_version>)) and asserts that it returns the expected error (rather than Ok), referencing the functions StateTransition::deserialize_from_bytes_in_version, PlatformVersion, and the active_version_range check so the test fails if the error branch is not taken.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-drive-proof-verifier/src/proof.rs`:
- Around line 3310-3328: The test currently triggers a decode error path
(NoProofInResult) because it uses default platform::GetIdentityRequest/Response;
change the test to construct a response that successfully decodes but yields
None from maybe_from_proof so that Identity::from_proof maps that None to
Error::NotFound. Specifically, keep using Identity and the FromProof::from_proof
wrapper but replace the response (and/or request) so maybe_from_proof would
return Ok(None) (for example, a valid GetIdentityResponse with an empty
proof/result), then call <Identity as
FromProof<platform::GetIdentityRequest>>::from_proof(...) and assert that the
returned error matches Error::NotFound; keep provider as unreachable_provider(),
Network::Testnet and default_platform_version() unchanged.
---
Nitpick comments:
In `@packages/rs-dpp/src/data_contract/created_data_contract/mod.rs`:
- Around line 328-340: The test serialize_roundtrip_via_platform_version only
compares identity_nonce() and data_contract().id(), which can miss other mutated
fields; update the assertion to compare the whole CreatedDataContract using
PartialEq by replacing the two field-level asserts with a single
assert_eq!(restored, created) after versioned_deserialize (keep the same setup
using sample_created, serialize_to_bytes_with_platform_version, and
CreatedDataContract::versioned_deserialize to perform the round-trip).
- Around line 386-405: The test only asserts the returned nonce from
data_contract_and_identity_nonce_owned(); update it to also validate the
returned contract format: destructure as let (contract_fmt, nonce) =
decoded.data_contract_and_identity_nonce_owned(); then assert_eq!(nonce,
created.identity_nonce()) and assert_eq!(contract_fmt, created.data_contract())
(or the appropriate accessor on sample_created() that returns the expected
contract) so the contract payload is also verified; use
CreatedDataContractInSerializationFormat::data_contract_and_identity_nonce_owned(),
sample_created(), and created.identity_nonce() to locate the code to change.
In `@packages/rs-dpp/src/document/specialized_document_factory/v0/mod.rs`:
- Around line 930-954: The test
create_state_transition_replace_on_mutable_document_increments_revision
currently never checks the produced transition's revision; update the test to
extract the created transition from batch (the result of
factory.create_state_transition), cast/inspect it as a
DocumentReplaceTransition, and assert its revision equals INITIAL_REVISION + 1
(i.e., the document's revision was incremented). Locate this in the test body
after batch is built and add an assertion against the transition's revision
field on the DocumentReplaceTransition to prevent regressions in the
document.set_revision logic.
- Around line 1161-1183: Update the two tests so they assert the specific error
variants instead of only is_err(): in
create_extended_from_document_buffer_invalid_type_fails call
factory.create_extended_from_document_buffer and assert the error is the
InvalidDocumentTypeError variant (e.g., by pattern matching or using
assert_matches!), and in create_extended_from_document_buffer_bad_bytes_fails
assert that the error comes from the deserialization failure raised by
Document::from_bytes (match the deserialization/error variant returned by
create_extended_from_document_buffer). Use the actual enum/variant names
returned by the factory error type when matching to ensure the tests fail if the
error shape changes.
- Around line 801-828: The test create_document_uses_given_time_based_properties
currently never verifies the supplied block_time/core_block_height; update the
test to assert that the document's time fields reflect the inputs by checking
doc.created_at() and doc.updated_at() equal Some(block_time) (or the concrete
return type used by Document) and also assert the core-height-derived field
(e.g., doc.core_height() or the specific getter used in this codebase) equals
core_block_height; if those accessors do not exist, either add the appropriate
getters or rename the test to create_document_with_block_time_succeeds to match
the current assertions.
In `@packages/rs-dpp/src/identity/identity_factory.rs`:
- Around line 452-474: Update the two tests (create_from_buffer_fails_on_garbage
and create_from_buffer_fails_on_empty) to assert the specific error shape
instead of only is_err(); call IdentityFactory::create_from_buffer and match the
returned Result to
Err(ProtocolError::ConsensusError(BasicError::SerializedObjectParsingError(_)))
(or use matches!/assert_matches! to keep it concise), preserving the existing
#[cfg(feature = "validation")] argument; this locks the expectation to the
SerializedObjectParsingError variant wrapped in ProtocolError::ConsensusError so
future changes won't silently change the error type.
- Around line 688-711: Add an explicit assertion of the identity's starting
revision before calling create_identity_update_transition so the test documents
and locks the precondition: after seeding v0.revision = 5, add
assert_eq!(identity.revision(), 5); immediately before calling
IdentityFactory::create_identity_update_transition(identity, 33, ...), then keep
the existing check that update.revision() == 6; this makes the intent around
IdentityV0::revision and the +1 behavior explicit and fails closer to the root
if defaults change.
In `@packages/rs-dpp/src/identity/identity_public_key/purpose.rs`:
- Around line 223-236: Rename or clarify Purpose::full_range() to indicate it
excludes SYSTEM by either renaming the method (e.g., assignable_purposes() or
user_purposes()) or keeping the name but adding a doc comment explicitly stating
that SYSTEM is intentionally omitted; update any callers/tests (e.g., the
test_purpose_full_range_contents test and references to full_range()) to use the
new name or reflect the clarified behavior and ensure the doc comment mentions
that full_range()/assignable_purposes() returns the six purpose variants
(AUTHENTICATION, ENCRYPTION, DECRYPTION, TRANSFER, VOTING, OWNER) and excludes
SYSTEM which is system-internal only.
In `@packages/rs-dpp/src/identity/identity_public_key/v0/methods/mod.rs`:
- Around line 244-445: Extract a small test helper to avoid repeating the
IdentityPublicKeyV0 literal: add a function like key_with(key_type: KeyType,
data: Vec<u8>) -> IdentityPublicKeyV0 that constructs an IdentityPublicKeyV0
using the provided key_type and data (wrapping data with
platform_value::BinaryData::new) and fills the rest via ..Default::default();
then replace the repeated literals in tests like
test_public_key_hash_empty_data_errors,
test_public_key_hash_ecdsa_wrong_length_errors,
test_public_key_hash_bls_wrong_length_errors,
test_public_key_hash_bls_returns_ripemd160_sha256_of_data,
test_public_key_hash_ecdsa_hash160_returns_data_itself,
test_public_key_hash_bip13_script_hash_returns_data_itself,
test_public_key_hash_hash160_wrong_length_errors,
test_validate_private_key_bytes_bip13_script_hash_is_unsupported,
test_validate_private_key_bytes_ecdsa_secret_key_parse_error_returns_false, and
test_validate_private_key_bytes_ecdsa_hash160_secret_key_parse_error_returns_false
to call key_with(...) with the differing key_type and data.
In `@packages/rs-dpp/src/state_transition/mod.rs`:
- Around line 1808-1818: Add a feature-gated negative test that exercises
StateTransition::deserialize_from_bytes_in_version's error branch when the
provided protocol version is outside the active_version_range and
protocol_version <= 268435456; specifically, create a test (annotated with
#[cfg(all(feature = "state-transitions", feature = "validation"))]) that
serializes a sample StateTransition (e.g., sample_transfer_st()), mutates the
serialized bytes or the passed PlatformVersion to a version not included in
active_version_range, calls
StateTransition::deserialize_from_bytes_in_version(&bytes,
PlatformVersion::new(<small_or_invalid_version>)) and asserts that it returns
the expected error (rather than Ok), referencing the functions
StateTransition::deserialize_from_bytes_in_version, PlatformVersion, and the
active_version_range check so the test fails if the error branch is not taken.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b07eba3f-8e4f-4161-a0fc-9d3b9eea62c9
📒 Files selected for processing (10)
packages/rs-dpp/src/data_contract/created_data_contract/mod.rspackages/rs-dpp/src/document/specialized_document_factory/v0/mod.rspackages/rs-dpp/src/identity/identity_factory.rspackages/rs-dpp/src/identity/identity_public_key/contract_bounds/mod.rspackages/rs-dpp/src/identity/identity_public_key/mod.rspackages/rs-dpp/src/identity/identity_public_key/purpose.rspackages/rs-dpp/src/identity/identity_public_key/security_level.rspackages/rs-dpp/src/identity/identity_public_key/v0/methods/mod.rspackages/rs-dpp/src/state_transition/mod.rspackages/rs-drive-proof-verifier/src/proof.rs
Per CodeRabbit review: the old test used default request/response which triggered NoProofInResult before the from_proof wrapper could map a real Ok(None) to Error::NotFound — so it was actually testing decode-error propagation, not the wrapper's None-to-NotFound logic. Introduce a local MissingFromProof impl whose maybe_from_proof_with_metadata returns Ok((None, ..)), and assert the wrapper maps that to Error::NotFound. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer
deleted the
test/coverage-proof-verifier-and-dpp-factories
branch
This was referenced Apr 21, 2026
5 tasks
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 being fixed or feature implemented
After merging #3503, global coverage sits at 85.05%. This is the first of a planned multi-PR push toward 90%. Targets the files with the lowest coverage-to-line-count ratio identified from the Codecov tree:
rs-drive-proof-verifier/src/proof.rsrs-dpp/state_transition/mod.rsrs-dpp/document/specialized_document_factoryrs-dpp/identity/identity_public_keyrs-dpp/identity/identity_factory.rsrs-dpp/data_contract/created_data_contractWhat was done?
171 new unit tests added across 10 files. All inline
#[cfg(test)] mod testsblocks — no new integration dependencies, no mocks feature required.rs-drive-proof-verifier/proof.rs — 31 tests
Vec<Option<T>>,Option<T>,Vec<(K, Option<T>)>,BTreeMap<K, Option<T>>,IndexMap<K, Option<T>>IntoOptionabsence-preserving semantics (empty→None, non-empty→Some(self))parse_key_request_type— all 3 error branches + 3 happy paths (AllKeys,SpecificKeys,SearchKey)FromProofdecode-error paths that fire before any crypto:Identity(4 error arms),Identity by public_key_hash(wrong length),Identity by non-unique public_key_hash(4 decode paths),IdentityBalances,DataContracts,GetProtocolVersionUpgradeVoteStatus,GetPathElements,GetPrefundedSpecializedBalance,GetEpochsInfooverflow,BroadcastStateTransitiongarbage bytes. A module-scopedUnreachableContextProviderpanics on any method call, proving error paths exit before verification.rs-dpp/state_transition/mod.rs — 25 tests
Covers
name(),state_transition_type(),is_identity_signed(),signature(),owner_id(),signature_public_key_id(),user_fee_increase()and their setters,required_number_of_private_keys()default arm,active_version_range()ALL_VERSIONSarm,unique_identifiers()/transaction_id()determinism + mutation sensitivity,From<IdentityCreditTransferTransition>,From<MasternodeVoteTransition>, andPlatformSerializable/PlatformDeserializableroundtrip.rs-dpp/data_contract/created_data_contract — 14 tests
Tests
from_contract_and_identity_nonce, all accessors (data_contract,data_contract_owned,data_contract_mut,data_contract_and_identity_nonce),set_identity_nonce,From<CreatedDataContract> for DataContract, bothPlatformSerializableWithPlatformVersionpaths produce identical bytes, andPlatformDeserializableWithPotentialValidationFromVersionedStructureroundtrip + garbage/empty error arms.rs-dpp/identity/identity_public_key/ — 69 tests across 5 files
TryFrom<u8>/TryFrom<i32>valid + invalid,From<Purpose>for byte arrays,Display,Default, ordering,full_range(),searchable_purposes()TryFrom<u8>happy +UnknownSecurityLevelError, byte conversions,DefaultisHIGH,stronger_security_thanfull matrix (not reflexive),stronger_or_equal_security_than(reflexive)new_from_type(0)/new_from_type(1)happy + error paths,contract_bounds_type_from_str, all accessorspublic_key_hash()ECDSA/BLS/HASH160/BIP13 branches +EmptyPublicKeyDataError+ParsingErroron wrong-length;validate_private_key_bytesfor BIP13_SCRIPT_HASH returnsNotSupported; invalid secret-key bytes returnOk(false)(not Err)is_masteracross all security levels,max_possible_size_keystructure,default_versioned, enum wrapper getters/setters (set_disabled_at/remove_disabled_at/data_owned), random key generators (deterministic seed, correct attributes for master/critical/high/voting/owner/transfer),main_keys_with_random_authentication_keys_with_rngerror path (count < 2) and positional assertionsrs-dpp/identity/identity_factory.rs — 14 tests
create_from_bufferhappy +SerializedObjectParsingErrorcreate_identity_create_transition_from_identity,create_identity_with_create_transition_computes_id_from_asset_lock_proofcreate_identity_topup_transition,create_identity_credit_transfer_transitioncreate_identity_credit_withdrawal_transitionV0 missing-output-scriptProtocolError::Generic+ V0 with-script + V1 (None script accepted) +UnknownVersionMismatchcreate_identity_update_transitionwith adds + with disabled keyscreate_instant_lock_proof_preserves_output_indexrs-dpp/document/specialized_document_factory/v0/mod.rs — 18 tests
PlatformVersion::geterror paths fornew,create_document,create_document_without_time_based_propertiescreate_documententropy generator determinism + time-based property injectioncreate_state_transitionCreate/Delete/Replace happy paths includingdocument_create_transitions,document_delete_transitions,document_replace_transitionsNoDocumentsSuppliedError, mismatched-owners error,RevisionAbsentErrorin all 3 helpers,InvalidInitialRevisionError, nonce accumulator incrementscreate_extended_from_document_bufferroundtrip + invalid-type + bad-bytes error branchesHow Has This Been Tested?
cargo test -p drive-proof-verifier— 67 passed (31 new + 36 pre-existing)cargo test -p dpp --lib identity_public_key— 124 passedcargo test -p dpp --lib identity_factory— 12 passedcargo test -p dpp --lib specialized_document_factory— 29 passedcargo test -p dpp --lib created_data_contract— 17 passedcargo test -p dpp --lib 'state_transition::tests'— 31 passedcargo check --tests -p dpp/cargo check --tests -p drive-proof-verifier— clean (no new warnings)cargo fmt -p dpp -p drive-proof-verifier— cleanNo production bugs were surfaced. One documented quirk preserved in tests:
Purpose::full_range()intentionally excludesSYSTEMdespite its name.Breaking Changes
None. Tests only.
Checklist
For repository code-owners and collaborators only
Summary by CodeRabbit