Skip to content

fix(drive): wire v1 dispatcher for deduct_from_prefunded_specialized_balance#3510

Closed
QuantumExplorer wants to merge 1 commit into
v3.1-devfrom
claude/heuristic-chatelet-bd6478
Closed

fix(drive): wire v1 dispatcher for deduct_from_prefunded_specialized_balance#3510
QuantumExplorer wants to merge 1 commit into
v3.1-devfrom
claude/heuristic-chatelet-bd6478

Conversation

@QuantumExplorer
Copy link
Copy Markdown
Member

@QuantumExplorer QuantumExplorer commented Apr 21, 2026

Issue being fixed or feature implemented

The top-level `Drive::deduct_from_prefunded_specialized_balance` dispatcher in
`packages/rs-drive/src/drive/prefunded_specialized_balances/deduct_from_prefunded_specialized_balance/mod.rs` only matches
feature version `0`, but platform versions v3–v7 set
`DrivePrefundedSpecializedMethodVersions::deduct_from_prefunded_specialized_balance` to `1`
(see v3.rs:100, v4.rs:100, v5.rs:102, v6.rs:104, v7.rs:102). On any v3+ platform the helper
unconditionally returned:

```
Error::Drive(DriveError::UnknownVersionMismatch {
method: "deduct_from_prefunded_specialized_balance",
known_versions: vec![0],
received: 1,
})
```

Production impact

None observed. A grep of the workspace shows the helper has no production callers —
the real state-transition path is routed through
`deduct_from_prefunded_specialized_balance_operations` (from `util/batch/drive_op_batch/prefunded_specialized_balance.rs:56`),
which already handles both v0 and v1 and has been correct since #2422. However, the helper is
part of the public `Drive` API, so any external `rs-drive` consumer who did call it on v3+
would hit an `UnknownVersionMismatch` error.

Adjacent latent issue (not fixed here)

While investigating, I noticed the `_operations` dispatcher reads the wrong platform-version field:

```rust
// deduct_from_prefunded_specialized_balance_operations/mod.rs:41-45
match platform_version
.drive
.methods
.prefunded_specialized_balances
.deduct_from_prefunded_specialized_balance // <-- the top-level field, not _operations
```

Compare to the `add` sibling, which correctly reads `.add_prefunded_specialized_balance_operations`. This
misread is likely why v3+ bumped the top-level field to `1` rather than the `_operations` field —
it was the de facto control knob for `_operations` dispatch.

Fixing this properly requires swapping the `1` from the top-level to `_operations` across
v3–v7, which is consensus-adjacent (same observable `_operations` behavior — both read the
same value either way — but I did not want to bundle a platform-version edit into this PR).
Happy to open a follow-up if reviewers agree it should be cleaned up.

What was done?

  • Added `deduct_from_prefunded_specialized_balance/v1/mod.rs` with a v1 impl. Its body is identical to v0 because the v0 impl
    is already version-agnostic: it delegates to `deduct_from_prefunded_specialized_balance_operations`,
    which dispatches to `_operations_v0` / `_operations_v1` based on the platform version. So v1 just
    collects those operations and applies the resulting grovedb batch.
  • Registered the v1 module and added the match arm in the top-level dispatcher.
  • Updated `known_versions: vec![0]` → `vec![0, 1]` in the `_operations` dispatcher error arm — it
    has handled both versions since feat(platform)!: distribute prefunded specialized balances after vote #2422 and the error message was stale.

No platform-version files were touched, so there is no consensus risk.

How Has This Been Tested?

  • `cargo build -p drive --lib` — clean build.
  • `cargo clippy -p drive --lib --no-deps` — no warnings.
  • `cargo fmt -p drive`.

Did not add a unit test because the helper is not called from production or any existing test
harness; setting up a Drive fixture purely to round-trip the dispatcher would be out of proportion
to the fix. The match statement's compile-time coverage plus the existing `_operations` test
coverage (which the v0/v1 top-level bodies both delegate to) guard against regression.

Breaking Changes

None. This restores previously expected behavior of a public method that was broken on v3+ and
keeps behavior identical on v0–v2.

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

  • New Features
    • Prefunded specialized balance deduction operations now support version 1 in addition to the existing version 0, expanding available options for users.

…balance

The top-level `Drive::deduct_from_prefunded_specialized_balance` dispatcher
only matched version 0, but platform versions v3-v7 set the
`deduct_from_prefunded_specialized_balance` feature version to 1, causing
the helper to unconditionally return `UnknownVersionMismatch` on any v3+
platform. The helper currently has no production callers (the real state
transition path goes through the `_operations` sibling, which already
handles both v0 and v1), so this has not impacted consensus, but the helper
is part of the public `Drive` API and is effectively dead on all recent
platform versions.

This adds a v1 impl mirroring v0 (v0's body is already version-agnostic,
since version selection for the actual deduction logic happens inside the
`_operations` dispatcher it delegates to) and wires the match arm. Also
corrects the stale `known_versions: vec![0]` in the `_operations`
dispatcher error arm to `vec![0, 1]`, since it has handled both versions
since #2422.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3d88d63c-b375-44b9-a93a-33897e45167b

📥 Commits

Reviewing files that changed from the base of the PR and between 4355c15 and 7f43c05.

📒 Files selected for processing (3)
  • packages/rs-drive/src/drive/prefunded_specialized_balances/deduct_from_prefunded_specialized_balance/mod.rs
  • packages/rs-drive/src/drive/prefunded_specialized_balances/deduct_from_prefunded_specialized_balance/v1/mod.rs
  • packages/rs-drive/src/drive/prefunded_specialized_balances/deduct_from_prefunded_specialized_balance_operations/mod.rs

📝 Walkthrough

Walkthrough

The pull request adds version 1 support for the deduct_from_prefunded_specialized_balance platform method by introducing a new v1 module implementation, updating the dispatcher to route v1 calls, and updating error reporting to indicate both versions [0, 1] are now supported.

Changes

Cohort / File(s) Summary
Version 1 Support Addition
packages/rs-drive/src/drive/prefunded_specialized_balances/deduct_from_prefunded_specialized_balance/mod.rs, packages/rs-drive/src/drive/prefunded_specialized_balances/deduct_from_prefunded_specialized_balance/v1/mod.rs, packages/rs-drive/src/drive/prefunded_specialized_balances/deduct_from_prefunded_specialized_balance_operations/mod.rs
Introduced v1 module with a new deduct_from_prefunded_specialized_balance_v1() method that wraps existing operations and applies them via grovedb. Updated dispatcher to route version 1 calls and updated error reporting to reflect known_versions as [0, 1].

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 wiggles nose with glee
New version one hops into the dance,
Prefunded balances deduct with perfect stance,
The dispatcher routes both old and new,
Version zero and one—a pair so true! ✨

🚥 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 accurately describes the primary change: adding v1 dispatcher support for the deduct_from_prefunded_specialized_balance method. It is specific, concise, and clearly indicates the main objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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/heuristic-chatelet-bd6478

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added this to the v3.1.0 milestone Apr 21, 2026
@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented Apr 21, 2026

Review Gate

Commit: 7f43c055

  • Debounce: 103m ago (need 30m)

  • CI checks: build failure: JS packages (@dashevo/wasm-dpp) / Tests, JS packages (@dashevo/wasm-dpp2) / Tests, codecov/patch

  • CodeRabbit review: comment found

  • Off-peak hours: off-peak (12:42 PM PT Tuesday)

  • Run review now (check to override)

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.44%. Comparing base (4355c15) to head (7f43c05).
⚠️ Report is 2 commits behind head on v3.1-dev.

Files with missing lines Patch % Lines
...educt_from_prefunded_specialized_balance/v1/mod.rs 0.00% 23 Missing ⚠️
...s/deduct_from_prefunded_specialized_balance/mod.rs 0.00% 6 Missing ⚠️
...om_prefunded_specialized_balance_operations/mod.rs 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##           v3.1-dev    #3510      +/-   ##
============================================
+ Coverage     85.30%   85.44%   +0.13%     
============================================
  Files          2476     2477       +1     
  Lines        270705   272920    +2215     
============================================
+ Hits         230938   233187    +2249     
+ Misses        39767    39733      -34     
Components Coverage Δ
dpp 83.20% <ø> (+<0.01%) ⬆️
drive 85.03% <0.00%> (+0.31%) ⬆️
drive-abci 87.49% <ø> (ø)
sdk ∅ <ø> (∅)
dapi-client ∅ <ø> (∅)
platform-version ∅ <ø> (∅)
platform-value 92.10% <ø> (ø)
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.

@QuantumExplorer
Copy link
Copy Markdown
Member Author

No, we removed the method instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants