Skip to content

refactor(rs-sdk-ffi): use the rs-sdk voting-key helpers instead of its own - #4341

Merged
QuantumExplorer merged 1 commit into
v4.2-devfrom
refactor/ffi-use-rs-sdk-vote-keys
Aug 8, 2026
Merged

refactor(rs-sdk-ffi): use the rs-sdk voting-key helpers instead of its own#4341
QuantumExplorer merged 1 commit into
v4.2-devfrom
refactor/ffi-use-rs-sdk-vote-keys

Conversation

@QuantumExplorer

@QuantumExplorerQuantumExplorer commented Aug 8, 2026

Copy link
Copy Markdown
Member

Completes the move #4340 started. That PR added the masternode voting-key facts to rs-sdk but did not remove rs-sdk-ffi's copies, so v4.2-dev currently carries two implementations of the same Platform behaviour — free to drift, and exactly the duplication #4340 existed to end.

What moves

Deleted from the FFI, now taken from platform::transition::masternode_vote_keys:

  • fabricating the voting key Platform holds
  • select_voting_key
  • is_voting_key_failure (the diagnosis gate)
  • diagnose_voting_key_failure

Net −340 lines.

What stays

The FFI keeps what it actually owns:

  • pointer / CStr marshalling and null checks
  • the byte-order conversion at the boundary — callers pass wire order, and voter_identity_id takes a typed ProTxHash, so the conversion happens once, here, where the FFI's contract is defined
  • signer construction from the raw key (simple-signer is an FFI dependency, deliberately not an SDK one)
  • rendering VotingKeyProblem into a user-facing sentence

That last split is the point: rs-sdk reports the typed fact, the binding phrases it. Formatting in the SDK would hand every caller English they cannot localise.

Tests

Tests follow the code. Selection, the gate and the diagnosis are covered by #4340's 10 tests in rs-sdk. What remains here covers only what remains here — that both messages name their identifiers, and that neither contains whitespace runs, which is the defect that once rendered them with blank gaps mid-sentence on device.

5 tests in the module, fmt and clippy clean.

Risk

Behaviour-preserving. The fabricated key, the gate's three signature variants, and the diagnosis logic are the same code, now called rather than copied — verified by the surviving message tests and by the fact that votes cast successfully on the merged stack.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved voting-key handling for more reliable vote submission.
    • Added clearer, more specific error messages when vote broadcasts fail due to voting-key issues.
    • Improved diagnosis of voting failures, helping surface actionable details to host applications.

…s own
#4340 moved the masternode voting-key facts into rs-sdk, but nothing removed
the FFI's copies — so the tree carried two implementations of the same
Platform behaviour, free to drift. This finishes that move.
Deleted from the FFI and taken from `platform::transition::masternode_vote_keys`:
key fabrication, `select_voting_key`, the failure gate, and the diagnosis.
Net −340 lines.
What stays is what this layer actually owns: pointer/CStr marshalling, the
byte-order conversion at the boundary (callers pass wire order; the SDK
takes a typed `ProTxHash`), signer construction, and turning the SDK's typed
`VotingKeyProblem` into the sentence a host displays. That last split is the
point — rs-sdk reports the fact, the binding phrases it, so no caller
inherits English from the SDK.
Tests follow the code. Selection, the gate and the diagnosis are covered in
rs-sdk (10 tests); what remains here covers only the message rendering,
including that both messages name their identifiers and contain no
whitespace runs — the defect that once rendered them with blank gaps on
device.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actionsgithub-actionsBot added this to the v4.2.0 milestone Aug 8, 2026
@coderabbitai

coderabbitaiBot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1050c1b1-1086-4571-a209-b86646f0aa0c

📥 Commits

Reviewing files that changed from the base of the PR and between f446428 and c779ca0.

📒 Files selected for processing (1)
  • packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs

📝 Walkthrough

Walkthrough

The FFI now uses shared rs-sdk helpers to construct voting keys and diagnose typed voting-key failures after broadcast errors. Local identity fetching, key selection, and failure classification were removed. Tests validate diagnostic message formatting.

Changes

Voting-key diagnosis

Layer / File(s)Summary
Shared voting-key construction
packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs
The cast-vote flow uses voter_identity_voting_key and shared voting-key diagnosis imports instead of local key construction.
Typed broadcast failure diagnosis
packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs
Broadcast failures use is_voting_key_failure, convert the hash to ProTxHash, and delegate diagnosis to rs-sdk. The FFI formats NoVoterIdentity and NoUsableVotingKey results and preserves unrelated errors. Tests cover diagnostic message formatting.

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

Possibly related PRs

Suggested reviewers:shumkov, lklimek

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly and concisely describes the main change: replacing duplicated FFI voting-key logic with rs-sdk helpers.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ 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 refactor/ffi-use-rs-sdk-vote-keys

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

@QuantumExplorer
QuantumExplorer merged commit 7a7ec9f into v4.2-devAug 8, 2026
15 of 16 checks passed
@QuantumExplorer
QuantumExplorer deleted the refactor/ffi-use-rs-sdk-vote-keys branch August 8, 2026 10:29
@codecov

codecovBot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.61%. Comparing base (f446428) to head (c779ca0).
⚠️ Report is 1 commits behind head on v4.2-dev.

Additional details and impacted files
@@ Coverage Diff @@## v4.2-dev #4341 +/- ##
============================================
- Coverage 87.79% 87.61% -0.18% 
============================================
Files 2677 2704 +27 Lines 342371 345211 +2840 ============================================
+ Hits 300578 302473 +1895 - Misses 41793 42738 +945 
ComponentsCoverage Δ
dpp88.86% <ø> (ø)
drive86.25% <ø> (ø)
drive-abci89.66% <ø> (ø)
sdk∅ <ø> (∅)
dapi-client∅ <ø> (∅)
platform-version∅ <ø> (∅)
platform-value92.88% <ø> (ø)
platform-wallet∅ <ø> (∅)
drive-proof-verifier48.02% <ø> (ø)
🚀 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.

Sign up for freeto 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.

1 participant

@QuantumExplorer