fix(platform)!: apply the review of contract group key bounds

[QuantumExplorer]

Issue being fixed or feature implemented

Follow-up to #4793 (authentication keys bound to a contract group), which merged before its code review findings were applied. This PR applies them.

What was done?

Behaviour:

• Drive read the contract group twice per bound key and billed both reads. new_from_single_key read the group's info while building the apply info, then fetch_bound_root_with_fee read it again when the references were written. Building the apply info needs nothing from the group, so only the second read remains; it is the one that checks the group exists.
• Transform v2 resolved memberships for a batch that had already failed. A transform result carrying errors returns from the processor before the bounds check, so the read was wasted and its fee never billed. The read now runs only for an error-free result.

Cleanup:

• IdentityPublicKeyInCreation::first_bound_to_a_contract_group replaces three copies of the same match (the protocol version gate, the shielded builder, the shielded proof validator).
• The ContractGroupMember import in contract_bounds/mod.rs is gated on state-transitions, like its only users, so a build without that feature no longer warns.
• The contract group test fixtures (register_group, join_group, single_owner_info, ...) move from the private query::contract_group_queries test module to test/helpers/contract_groups.rs; the batch and key bounds validator tests use them instead of their own copies.
• A dpp test that had landed between another test and the comment describing it moves below that pair; four new tests are renamed to start with "should".
• The protocol docs said memberships are read once per batch member. They are read and billed once per distinct contract.

Not in this PR: the wallet FFI already emits bounds kind 3 while the Kotlin and Swift layers on v4.2-dev only know kinds 0 to 2. That is what #4800 fixes, so it should merge soon after #4793 did.

How Has This Been Tested?

New tests:

• CheckTx is asserted next to block processing for all six membership cases of the document test: valid inside the group, 20014 outside it.
• An identity update registers a group-bound key through the whole pipeline: a paid 41001 while the group is missing, success once it exists, the key indexed under the group, and the bounds kept after revocation (which exercises the refresh path).
• Every entry of PLATFORM_VERSIONS pairs batch advanced structure v1 with transform v2, the invariant behind the CorruptedCodeExecution arm of the bounds check.
• The transformer test gained a batch that fails in state: nothing is resolved for it. Removing the new guard makes this case fail.
• Drive: building the apply info of a group bound queues no operation of its own.

Ran locally: the affected tests of dpp, drive, drive-abci (321 tests across contract groups, identity create and update, key bounds, shielded proof), platform-wallet-ffi and rs-unified-sdk-jni; cargo clippy -p dpp -p drive -p drive-abci --all-targets -- -D warnings; cargo fmt --all -- --check.

Breaking Changes

Registering or refreshing a key bound to a contract group is billed one group read instead of two. This only exists in protocol version 14, which is unreleased, but nodes built before and after this PR disagree on that fee.

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

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation

  • Clarified contract-group-bound authentication key behavior for batch transactions.
  • Documented that each distinct contract’s group membership is read and billed once per batch.
  • Clarified that members outside the group continue to fail validation with the applicable consensus error.

• Bug Fixes

  • Prevented contract-group membership resolution when batch validation has already failed.
  • Deferred membership reads until they are required, avoiding unnecessary operations during transaction processing.

• Tests

  • Expanded coverage for contract-group-bound keys, validation failures, registration, revocation, and protocol-version compatibility.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ad5de88f-b5b6-4880-ba5d-b5571f9aa07e

📥 Commits

Reviewing files that changed from the base of the PR and between da14366 and 137d38b.

📒 Files selected for processing (21)

• docs/protocol/contract-bound-authentication-keys.md
• docs/protocol/contract-groups.md
• packages/rs-dpp/src/identity/identity_public_key/contract_bounds/mod.rs
• packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs
• packages/rs-dpp/src/state_transition/mod.rs
• packages/rs-dpp/src/state_transition/state_transitions/identity/public_key_in_creation/mod.rs
• packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs
• packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs
• packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v2/mod.rs
• packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/contract_group_bound_auth.rs
• packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs
• packages/rs-drive-abci/src/query/contract_group_queries/contract_group_info/v0/mod.rs
• packages/rs-drive-abci/src/query/contract_group_queries/contract_group_members/v0/mod.rs
• packages/rs-drive-abci/src/query/contract_group_queries/contract_groups_for_contract/v0/mod.rs
• packages/rs-drive-abci/src/query/contract_group_queries/mod.rs
• packages/rs-drive-abci/src/test/helpers/contract_groups.rs
• packages/rs-drive-abci/src/test/helpers/mod.rs
• packages/rs-drive/src/drive/identity/contract_info/keys/mod.rs
• packages/rs-platform-wallet-ffi/src/identity_persistence.rs
• packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs
• packages/rs-unified-sdk-jni/src/pubkey_rows.rs

💤 Files with no reviewable changes (1)

• packages/rs-drive-abci/src/query/contract_group_queries/mod.rs

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

---

📝 Walkthrough

Walkthrough

The change centralizes contract-group key detection, defers group reads during apply-info construction, skips membership resolution after validation errors, adds validation coverage, moves contract-group fixtures into shared test helpers, and updates protocol documentation.

Changes

Contract-group resolution

Layer / File(s)	Summary
Shared contract-group key detection packages/rs-dpp/..., packages/rs-drive-abci/...	Adds first_bound_to_a_contract_group and uses it for protocol gating and shielded validation.
Deferred group reads and validation gating packages/rs-drive/..., packages/rs-drive-abci/..., docs/protocol/*	Defers contract-group reads until bound references are written. Batch validation skips membership reads when earlier validation returns errors. Documentation describes one read and charge per distinct touched contract.
Validation and state-transition coverage packages/rs-drive-abci/execution/validation/...	Adds coverage for CheckTx results, failed transformers, protocol-version configuration, and registration and revocation of contract-group-bound keys.
Shared fixtures and test organization packages/rs-drive-abci/test/helpers/..., packages/rs-drive-abci/query/..., packages/rs-platform-wallet-ffi/..., packages/rs-unified-sdk-jni/...	Moves contract-group fixtures to shared test helpers and updates imports and test names.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant BatchTransition
  participant Transformer
  participant MembershipResolver
  participant CheckTx
  BatchTransition->>Transformer: transform batch
  Transformer->>MembershipResolver: resolve memberships when validation has no errors
  MembershipResolver->>MembershipResolver: read each distinct touched contract
  BatchTransition->>CheckTx: check transaction
  CheckTx-->>BatchTransition: matching validation result

Loading

Merge Risk: ⚪ Minimal · up to 137d3

The contract-group changes do not leave a concrete unresolved merge risk.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title identifies a platform fix for contract-group key bounds, which matches the pull request's main behavioral and review-driven changes.
Docstring Coverage	✅ Passed	Docstring coverage is 81.25% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 18 files. (2 skipped: 2…
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.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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

[thepastaclaw]

🕓 Queued for automated review — 9th in line, estimated start in ~7 h (commit 137d38b)
Estimated review time once started: ~1.5 h (two-phase automated review; median of recent runs).

• Request priority review — click to move this review to the front of the queue.

[QuantumExplorer]

Approved

[codecov]

Codecov Report

❌ Patch coverage is 74.72924% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.28%. Comparing base (da14366) to head (137d38b).
⚠️ Report is 1 commits behind head on v4.2-dev.

Files with missing lines	Patch %	Lines
...ransition/state_transitions/identity_update/mod.rs	79.79%	39 Missing ⚠️
packages/rs-dpp/src/state_transition/mod.rs	56.25%	14 Missing ⚠️
...drive/src/drive/identity/contract_info/keys/mod.rs	62.50%	9 Missing ⚠️
...lded/builder/identity_create_from_shielded_pool.rs	0.00%	3 Missing ⚠️
...transitions/identity/public_key_in_creation/mod.rs	70.00%	3 Missing ⚠️
...alidate_identity_public_key_contract_bounds/mod.rs	85.71%	1 Missing ⚠️
...transition/state_transitions/batch/state/v2/mod.rs	75.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           v4.2-dev    #4801      +/-   ##
============================================
- Coverage     78.11%   77.28%   -0.84%     
============================================
  Files          2916     2916              
  Lines        419165   422948    +3783     
============================================
- Hits         327434   326863     -571     
- Misses        91731    96085    +4354     

Components	Coverage Δ
dpp	75.95% <56.52%> (-0.52%)	⬇️
drive	79.14% <62.50%> (-0.29%)	⬇️
drive-abci	76.60% <80.19%> (-2.13%)	⬇️
sdk	∅ <ø> (∅)
dapi-client	∅ <ø> (∅)
platform-version	∅ <ø> (∅)
platform-value	86.60% <ø> (ø)
platform-wallet	∅ <ø> (∅)
drive-proof-verifier	27.80% <ø> (ø)

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