Skip to content

feat(rs-sdk-ffi): add masternode contested-resource vote broadcast (FFI + Swift UI)#3883

Merged
QuantumExplorer merged 4 commits into
v3.1-devfrom
feat/contested-resource-vote
Jun 13, 2026
Merged

feat(rs-sdk-ffi): add masternode contested-resource vote broadcast (FFI + Swift UI)#3883
QuantumExplorer merged 4 commits into
v3.1-devfrom
feat/contested-resource-vote

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Jun 13, 2026

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

There was no broadcast FFI for casting votes on contested resources. packages/rs-sdk-ffi/src/voting/ and contested_resource/ contained only query functions (vote-poll lookups, vote state, voters, identity votes). As a result the SwiftExampleApp could read contested-resource vote tallies (ContestDetailView) but could not cast a vote.

This PR adds the missing vote write path: a Rust FFI binding, a thin Swift wrapper, and a minimal UI affordance to cast a contested-resource (DPNS masternode) vote.

What was done?

Rust SDK (confirmed, not changed)

The Rust SDK already exposes the broadcast path: dash_sdk::platform::transition::vote::PutVote on Vote, whose put_to_platform_and_wait_for_response(voter_pro_tx_hash, voting_public_key, sdk, signer, settings) builds a MasternodeVoteTransition via try_from_vote_with_signer and broadcasts it. The transition derives the voter identity from pro_tx_hash + masternode_voting_key.public_key_hash() and signs via sign_external.

FFI (packages/rs-sdk-ffi)

  • New dash_sdk_contested_resource_cast_vote in src/contested_resource/transitions/cast_vote.rs, wired through the new transitions submodule and re-exported from contested_resource/mod.rs.
  • It marshals the contested-document vote poll (contract_id, document_type_name, index_name, index_values_json) + a vote-choice discriminant (0 TowardsIdentity, 1 Abstain, 2 Lock) + optional contender id, assembles a Vote::ResourceVote, and calls the existing rs-sdk PutVote entry point. Nothing is stitched together that the SDK does not already expose as a single call.
  • The masternode voting key + signer are both derived from a supplied 32-byte voting private key: a SingleKeySigner signs, and the matching ECDSA_HASH160 / VOTING / HIGH IdentityPublicKey (data = hash160(pubkey), id 0) is constructed to mirror Platform's get_voter_identity_key_v0. SingleKeySigner::can_sign_with recomputes the same hash160, so key and signer agree by construction.
  • Index-value parsing (hex → Value::Bytes, otherwise Value::Text) matches the existing get_vote_state query FFI, so a poll readable via the read path is the same poll voted on here.

FFI signature:

struct DashSDKResult dash_sdk_contested_resource_cast_vote(
    const struct SDKHandle *sdk_handle,
    const char *contract_id,
    const char *document_type_name,
    const char *index_name,
    const char *index_values_json,
    uint8_t vote_choice,                 // 0=TowardsIdentity, 1=Abstain, 2=Lock
    const char *contender_identity_id,   // base58; required iff vote_choice==0
    const uint8_t *voter_pro_tx_hash,    // 32 bytes
    const uint8_t *voting_private_key,   // 32 bytes
    FFINetwork network);

Swift SDK + UI (packages/swift-sdk)

  • SDK.castContestedResourceVote(...) — a thin bridge in PlatformQueryExtensions.swift (marshal in → call → surface success/error out), plus a small processVoidResult helper.
  • New ContestedResourceVoteChoice enum (towardsIdentity / abstain / lock) in ContestVoteState.swift, kept in lockstep with the FFI discriminants.
  • ContestDetailView gains a "Cast a Masternode Vote" section and a sheet to pick a contender / Abstain / Lock and enter masternode credentials (pro_tx_hash + voting private key, both hex), with the masternode-only caveat spelled out in-UI.

How Has This Been Tested?

  • cargo fmt --all — clean.
  • cargo check -p rs-sdk-ffi --all-targets — passes.
  • cargo clippy -p rs-sdk-ffi — no warnings/errors on the new code.
  • cargo test -p rs-sdk-ffi cast_vote — 3 new unit tests pass (null SDK handle; TowardsIdentity-requires-contender; invalid choice).
  • cargo build -p rs-sdk-ffi — confirmed the cbindgen-generated rs-sdk-ffi.h exports dash_sdk_contested_resource_cast_vote with the exact signature the Swift wrapper calls.
  • The iOS app was not built here (per instructions; the orchestrator does combined iOS testing).

Masternode-key caveat (important). Contested-resource votes are cast by masternodes using a masternode voting key tied to a pro_tx_hash. The SwiftExampleApp's regular wallets are not masternodes and have no voting key, so an end-to-end accepted vote is not testable from this app — a broadcast from a non-masternode reaches a deterministic authorization rejection on the platform side (voter identity / masternode not found / not authorized). The goal of this PR is correct construction + wiring so the transition is assembled, signed, and submitted properly, reaching that deterministic rejection rather than a client-side crash or a "not implemented" stub. The UI sheet accepts a pro_tx_hash + voting private key precisely so the path is exercisable end-to-end by someone holding real masternode voting credentials.

Breaking Changes

None. This is a purely additive FFI + SDK + example-app surface; no consensus behavior and no existing public API is changed. (Per project convention, feat!/fix! is reserved for consensus-breaking changes; SDK/FFI additions do not qualify.)

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

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features
    • Added masternode voting capability for contested resources (e.g., DPNS names)
    • Added Swift SDK method to cast votes on contested resources with vote choice options (Towards Identity, Abstain, Lock)
    • Added example UI for casting contested resource votes with masternode credentials

…+ Swift UI)

Adds the missing write path for casting a contested-resource (DPNS
masternode) vote. Previously rs-sdk-ffi exposed only contested-resource
*query* functions, so the app could read vote tallies but could not cast
a vote.

Rust FFI (rs-sdk-ffi):
- New `dash_sdk_contested_resource_cast_vote` under
  `contested_resource/transitions/cast_vote.rs`. It assembles a
  `Vote::ResourceVote` over the contested-document vote poll
  (contract_id, document_type_name, index_name, index_values) with a
  `ResourceVoteChoice` (TowardsIdentity / Abstain / Lock) and broadcasts
  it via the existing rs-sdk `PutVote::put_to_platform_and_wait_for_response`.
- The masternode voting key is derived from the supplied 32-byte voting
  private key: a `SingleKeySigner` signs, and the matching
  ECDSA_HASH160 / VOTING / HIGH `IdentityPublicKey` (data = hash160 of the
  public key) is built to mirror Platform's `get_voter_identity_key_v0`.
  `SingleKeySigner::can_sign_with` recomputes the same hash160, so the key
  and signer agree by construction. The voter identity is derived from
  pro_tx_hash + voting key by the transition itself.
- Index-value parsing matches the vote-state query FFI (hex -> Bytes,
  otherwise Text) so a poll readable via get_vote_state is the same poll
  voted on here.

Swift SDK + UI:
- `SDK.castContestedResourceVote(...)` thin bridge in
  PlatformQueryExtensions, plus a `ContestedResourceVoteChoice` enum.
- ContestDetailView gains a "Cast a Masternode Vote" section + sheet to
  pick a contender / Abstain / Lock and enter masternode credentials.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@QuantumExplorer, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 minute and 13 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b4f5dff2-3de2-44fb-a25f-0d546a7855f0

📥 Commits

Reviewing files that changed from the base of the PR and between 12c8fb3 and d85d9fc.

📒 Files selected for processing (6)
  • packages/rs-sdk-ffi/src/contested_resource/mod.rs
  • packages/rs-sdk-ffi/src/contested_resource/queries/resources.rs
  • packages/rs-sdk-ffi/src/contested_resource/queries/vote_state.rs
  • packages/rs-sdk-ffi/src/contested_resource/queries/voters_for_identity.rs
  • packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContestDetailView.swift
📝 Walkthrough

Walkthrough

This PR adds end-to-end contested-resource vote casting for masternodes: a Rust FFI layer exports vote validation and signing logic, a Swift SDK layer wraps the FFI with parameter marshalling, and an example app provides UI for credential entry and vote selection.

Changes

Contested Resource Vote Casting for Masternodes

Layer / File(s) Summary
Rust FFI vote-casting contract and implementation
packages/rs-sdk-ffi/cbindgen.toml, packages/rs-sdk-ffi/src/contested_resource/mod.rs, packages/rs-sdk-ffi/src/contested_resource/transitions/mod.rs, packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs
ContestedResourceVoteChoiceFFI enum discriminates vote types (0=TowardsIdentity, 1=Abstain, 2=Lock); dash_sdk_contested_resource_cast_vote FFI entrypoint validates contract/choice/credentials, derives secp256k1 public key and IdentityPublicKeyV0 from the 32-byte private key, and broadcasts the vote via SDK; unit tests cover null handles, missing contender id for TowardsIdentity, and invalid choice values; cbindgen config exports the enum to C headers.
Swift SDK FFI wrapper and error handling
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ContestVoteState.swift, packages/swift-sdk/Sources/SwiftDashSDK/FFI/PlatformQueryExtensions.swift
Swift ContestedResourceVoteChoice enum mirrors FFI discriminants with ffiTag mapping; SDK.castContestedResourceVote(...) validates 32-byte proTxHash and votingPrivateKey, JSON-encodes indexValues, derives optional contender id, and calls the Rust FFI with network context; processVoidResult(...) validates FFI result, frees error pointers, and throws on failure.
Example app vote-casting UI and flow
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContestDetailView.swift
Added vote-casting state (sheet visibility, casting guard, result banner); hardcoded DPNS contract id/type/index; castVoteSection displays optional result banner, disables "Cast Vote" button when SDK missing or casting in progress, and explains masternode-only requirement; CastVoteSheet modal collects hex-encoded pro_tx_hash and voting key (validated to exactly 64 hex chars = 32 bytes) and vote choice; async castVote(...) implementation calls SDK method and refreshes vote state on success.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ContestDetailView
  participant CastVoteSheet
  participant SDK.castContestedResourceVote
  participant dash_sdk_contested_resource_cast_vote
  participant Platform
  User->>ContestDetailView: tap "Cast Vote…"
  ContestDetailView->>CastVoteSheet: show sheet modal
  User->>CastVoteSheet: enter pro_tx_hash, voting_key, select choice
  User->>CastVoteSheet: tap Submit
  CastVoteSheet->>ContestDetailView: onSubmit callback
  ContestDetailView->>SDK.castContestedResourceVote: castContestedResourceVote(dpnsContractId, choice, hex→bytes)
  SDK.castContestedResourceVote->>SDK.castContestedResourceVote: validate 32-byte credentials, JSON-encode indexValues
  SDK.castContestedResourceVote->>dash_sdk_contested_resource_cast_vote: call FFI with marshalled params
  dash_sdk_contested_resource_cast_vote->>dash_sdk_contested_resource_cast_vote: derive keys, validate vote, construct ResourceVote
  dash_sdk_contested_resource_cast_vote->>Platform: put_to_platform_and_wait_for_response
  Platform-->>dash_sdk_contested_resource_cast_vote: result
  dash_sdk_contested_resource_cast_vote-->>SDK.castContestedResourceVote: DashSDKResult
  SDK.castContestedResourceVote-->>ContestDetailView: success or throw error
  ContestDetailView->>CastVoteSheet: close sheet
  ContestDetailView->>ContestDetailView: refresh voteState, show result banner
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

  • shumkov
  • thepastaclaw
  • llbartekll

Poem

🐰 A hop through the voting lanes so fine,
With keys derived in cryptographic line,
From Rust to Swift the ballots fly,
Master nodes cast their choice up high!
The modal pops, credentials in hand,
Democracy blooms across the land. 🗳️✨

🚥 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 clearly and specifically describes the main change: adding masternode contested-resource vote broadcast functionality via FFI and Swift UI.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/contested-resource-vote

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.

@thepastaclaw

thepastaclaw commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

✅ Review complete (commit d85d9fc)

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

PR adds a well-scoped FFI write path that delegates to the existing rs-sdk PutVote flow, with correct null/length checks, a Zeroizing private-key buffer, and a derived ECDSA_HASH160 voter key that matches SingleKeySigner::can_sign_with by construction. Two suggestions worth addressing before merge: the Swift hex validators silently truncate odd-length input, and the cross-language vote-choice discriminant is hand-mirrored rather than emitted by cbindgen. One nitpick on test coverage.

🟡 2 suggestion(s) | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContestDetailView.swift`:
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContestDetailView.swift:489-498: Hex guards silently accept odd-length input, producing wrong masternode credentials
  `Data(hexString:)` in `PlatformWalletTypes.swift` computes `let len = hexString.count / 2` and walks the string in 2-char steps, so a 65-character `pro_tx_hash` or voting private key passes through as 32 bytes with the trailing nibble dropped — and both `proTxHash.count == 32` / `votingKey.count == 32` guards then succeed. The result is a vote signed with a key that differs from what the masternode operator typed, with no error surfaced. Since this is security-sensitive credential input newly introduced here, validate the trimmed hex length (and even-ness) before decoding rather than relying on the post-decode byte count alone.

In `packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs:60-62: Vote-choice discriminant is hand-mirrored across the FFI rather than emitted
  `VOTE_CHOICE_TOWARDS_IDENTITY/ABSTAIN/LOCK` are bare `pub const u8` values that cbindgen does not surface in the generated C header, and `ContestVoteState.swift:68-74` hand-mirrors them as `0/1/2` with only a comment to keep the two sides in lockstep. If a future change to this Rust enum renumbers or inserts a variant, both sides will continue to compile cleanly and silently misinterpret votes (e.g. a `Lock` becoming an `Abstain`). Expose the mapping as a `#[repr(u8)]` enum so cbindgen emits it and Swift imports the canonical values rather than redeclaring them.

Comment thread packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContestDetailView.swift Outdated
Comment thread packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs Outdated
Comment thread packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs
@github-actions

Copy link
Copy Markdown
Contributor

✅ DashSDKFFI.xcframework built for this PR.

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
  name: "DashSDKFFI",
  url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
  checksum: "ecfef93b5ffddf83dddfb6f6ac46801d198651339556449feac3278f2785632f"
)

Xcode manual integration:

  • Download 'DashSDKFFI.xcframework' artifact from the run link above.
  • Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
  • If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

@QuantumExplorer QuantumExplorer changed the title feat(sdk-ffi): add masternode contested-resource vote broadcast (FFI + Swift UI) feat(rs-sdk-ffi): add masternode contested-resource vote broadcast (FFI + Swift UI) Jun 13, 2026
…ation

Address review feedback on the contested-resource vote path:

- Replace the three bare `pub const u8` vote-choice constants with a
  `#[repr(u8)]` `ContestedResourceVoteChoiceFFI` enum and force-emit it
  via the cbindgen `[export] include` list, so the discriminants live in
  the generated C header as a single source of truth. The `vote_choice`
  FFI parameter stays a plain `u8` (a C caller can pass any byte, and
  materializing an out-of-range `#[repr(u8)]` value would be UB), and the
  internal match compares against the enum's `as u8` discriminants with a
  defensive `other` arm. Swift's `ffiTag` now returns the generated
  `ContestedResourceVoteChoiceFFI` type instead of a bare `UInt8` so the
  two sides can't silently drift.

- Harden the cast-vote sheet in ContestDetailView: `Data(hexString:)`
  decodes `count / 2` bytes, so an odd-length (e.g. 65-char) pro_tx_hash
  or voting key silently dropped its trailing nibble and still passed the
  `count == 32` guard, signing with the wrong key. Validate the trimmed
  hex length is exactly 64 before decoding both inputs.

- Use the real base58 DPNS contract id in the cast-vote tests so they
  pass the 32-byte length check and actually reach the missing-contender
  and invalid-vote-choice branches they assert on, instead of
  short-circuiting on a bogus "DPNS" contract id.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs`:
- Around line 354-371: The tests (test_cast_vote_null_handle and the other two
mentioned) create temporary CStrings via CString::new(...).unwrap().as_ptr()
which are dropped immediately causing use-after-free when calling
dash_sdk_contested_resource_cast_vote; fix by creating named CString variables
(e.g., let contract_id = CString::new(DPNS_CONTRACT_ID).unwrap(); let domain =
CString::new("domain").unwrap(); etc.) and pass contract_id.as_ptr(),
domain.as_ptr(), ... into dash_sdk_contested_resource_cast_vote so the backing
memory lives for the duration of the FFI call; apply this change in
test_cast_vote_null_handle, test_cast_vote_towards_identity_requires_contender,
and test_cast_vote_invalid_choice.
🪄 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: 76c432df-7baa-4c30-ac28-e5c83b78a8fd

📥 Commits

Reviewing files that changed from the base of the PR and between 65042eb and 12c8fb3.

📒 Files selected for processing (7)
  • packages/rs-sdk-ffi/cbindgen.toml
  • packages/rs-sdk-ffi/src/contested_resource/mod.rs
  • packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs
  • packages/rs-sdk-ffi/src/contested_resource/transitions/mod.rs
  • packages/swift-sdk/Sources/SwiftDashSDK/FFI/PlatformQueryExtensions.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ContestVoteState.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContestDetailView.swift

Comment thread packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Latest delta (12c8fb3) is a focused hardening pass that resolves all three prior PastaClaw findings: ContestDetailView now requires exact 64-char hex length before decoding (prior #1 FIXED), cast_vote.rs exports a force-included #[repr(u8)] ContestedResourceVoteChoiceFFI enum through cbindgen and matches the FFI boundary u8 against as u8 discriminants (prior #2 FIXED), and the unit tests now use the real base58 DPNS contract id so the missing-contender / invalid-choice arms are actually reached (prior #3 FIXED). One carried-forward in-scope concern remains: the hex-vs-text heuristic on index_values_json (cast_vote.rs:208-222) is mirrored from the read-path FFI, but on this new write/signing path a hex-looking text label is silently signed against a different poll than the user-facing value — flagged independently by both claude-general and codex-security-auditor. Two minor nits (dead ?? hex fallback in resolvedChoice and leaked FFI error pointers in unit tests) are kept as nitpicks. Lower-value findings around the cstr lifetime signature and the remaining Swift literal cast to ContestedResourceVoteChoiceFFI(0/1/2) were dropped — the C-header export is now correct and Swift's Clang importer does not import C #defines, so the literal-cast pattern is what's actually expressible.

🟡 1 suggestion(s) | 💬 2 nitpick(s)

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs:208-222: Hex-or-text auto-detection silently re-encodes even-length all-hex labels as bytes when signing a vote
  Each `index_values_json` element whose chars are all ASCII hex and whose length is even is decoded as `Value::Bytes`; otherwise `Value::Text`. The read-path FFI uses the same heuristic, but here the result is fed straight into a signed `MasternodeVoteTransition`. A perfectly valid DPNS-style text label that happens to be even-length all-hex — e.g. `"abcd"`, `"cafe"`, `"face"`, `"deadbeef"`, `"1234"` — is encoded as `Value::Bytes` (e.g. `[0xab,0xcd]`) instead of `Value::Text("abcd")`, so the broadcast vote targets a different (likely non-existent) poll than the operator sees in the UI. The failure mode on the read side is "empty results"; on the write side it becomes "signed-and-broadcast vote against the wrong poll, with a deterministic platform rejection" with no signal back to the caller that the value was reinterpreted. Mirroring the query FFI for symmetry is a defensible design choice, but for a signing surface it would be safer to require an explicit type tag — e.g. a `"0x..."` prefix for bytes, or accept index values as `{"text":...}` / `{"bytes_hex":...}` objects — and apply the same encoding to the query FFI so the two stay in lockstep.

Comment thread packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs Outdated
Comment thread packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContestDetailView.swift Outdated
Comment thread packages/rs-sdk-ffi/src/contested_resource/transitions/cast_vote.rs
… stay valid

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Incremental review of da40c87 over 12c8fb3. Latest delta is test-only: every CString is now bound to a local before its pointer is taken, fixing the prior use-after-free in the three unit tests. Three prior findings remain valid against unchanged production/test code and are carried forward as one suggestion and two nitpicks; no new in-scope blockers.

💬 1 nitpick(s)

2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Comment thread packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContestDetailView.swift Outdated
…dy vote FFI

Replace the unsafe hex/text auto-detection on the contested-resource vote
path with an explicit type tag: an index value is decoded as `Value::Bytes`
only when it carries a `"0x"` prefix (hex-decode the remainder), otherwise it
is taken verbatim as `Value::Text`. The previous content heuristic silently
re-encoded even-length all-hex DPNS labels (`"abcd"`, `"cafe"`, `"deadbeef"`,
`"1234"`) as bytes, so a signed-and-broadcast vote could target the wrong
vote poll with no signal back to the caller.

The parsing now lives in one shared `parse_index_value` helper in
`contested_resource/mod.rs`, called from the cast-vote write path and the
three read-path query FFIs (`vote_state`, `resources`, `voters_for_identity`)
so write and read stay in lockstep. Added unit tests asserting an even-length
all-hex label without `0x` decodes as `Value::Text` and a `"0x..."` value
decodes as `Value::Bytes`. Updated the doc comments that described the old
hex-auto-detect behavior.

Swift: `resolvedChoice` in ContestDetailView no longer falls back to the raw
hex string when `Data(hexString:)` returns nil; it now returns an optional and
the Submit button surfaces a clear error instead of passing un-decodable hex
downstream to the FFI as if it were base58.

Also free the `DashSDKError` allocation in the three cast_vote unit tests via
`dash_sdk_error_free` after the assertions, so the error struct and message
string no longer leak (models the caller contract, keeps leak sanitizers
happy).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@QuantumExplorer QuantumExplorer merged commit 43bb144 into v3.1-dev Jun 13, 2026
3 checks passed
@QuantumExplorer QuantumExplorer deleted the feat/contested-resource-vote branch June 13, 2026 20:22

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

All three prior findings are resolved at HEAD: the signing-path hex heuristic is replaced with the explicit 0x-tagged parse_index_value helper, resolvedChoice returns nil instead of falling back to raw hex, and the three cast_vote unit tests now call dash_sdk_error_free. Two new in-scope concerns remain on the index-value parsing change: (1) the read-path FFIs (get_vote_state, get_resources, get_voters_for_identity) now silently reinterpret previously-byte queries as text, and (2) using 0x as the byte/text discriminator is still ambiguous for valid DPNS text labels that begin with 0x. A minor nitpick on a redundant SecretKey reparse is also kept.

🟡 2 suggestion(s) | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-sdk-ffi/src/contested_resource/mod.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/contested_resource/mod.rs:29-42: `parse_index_value` silently changes query-side index-value semantics
  The shared helper now backs not only the new `cast_vote` write path but also the existing query FFIs (`vote_state`, `resources`, `voters_for_identity`). Before this PR a bare even-length hex string such as `"deadbeef"` was decoded to `Value::Bytes`; at HEAD the same input produces `Value::Text`, with no error and no compatibility shim. Any existing C/Swift caller that intentionally passed unprefixed hex to query a `Value::Bytes` index will now silently query a different poll and get an empty result. The hardening on the signing path is correct (never auto-encode on a signing surface), but the read path's behavior change deserves either input validation that rejects bare hex on reads with an explanatory error, or an explicit CHANGELOG/migration note plus a deprecation period before flipping query semantics. As written, the only signal an external caller gets is silently-empty result sets.
- [SUGGESTION] packages/rs-sdk-ffi/src/contested_resource/mod.rs:30-41: `0x` prefix as type tag is ambiguous for valid DPNS text labels starting with `0x`
  Replacing the even-length-all-hex heuristic with an explicit `0x` prefix fixes the `"deadbeef"` case, but the prefix itself is still meaningful text: a DPNS contested label like `"0xdeadbeef"` matches DPNS's alphanumeric label rules and is signable. With the current parser, such a label routed through the Swift `castContestedResourceVote` path (which forwards the user-visible label string verbatim) is signed as `Value::Bytes([0xde, 0xad, 0xbe, 0xef])` instead of `Value::Text("0xdeadbeef")`, so the masternode broadcasts a vote against a different vote-poll encoding than the one displayed in `ContestDetailView`. The vote is rejected or misdirected rather than counted for the displayed contest. A structured tag — e.g. `{ "type": "text"|"bytes", "value": ... }` on the FFI surface, or pre-encoding the Swift caller's text labels with an unambiguous wrapper — removes the residual ambiguity on a signing surface. Treat any prefix-based magic on a signing input as a half-fix.

Comment on lines +29 to +42
pub(crate) fn parse_index_value(value_str: String) -> Result<Value, String> {
match value_str.strip_prefix("0x") {
Some(hex_str) => {
let bytes = hex::decode(hex_str).map_err(|e| {
format!(
"Index value '{}' has a 0x prefix but is not valid hex: {}",
value_str, e
)
})?;
Ok(Value::Bytes(bytes))
}
None => Ok(Value::Text(value_str)),
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: parse_index_value silently changes query-side index-value semantics

The shared helper now backs not only the new cast_vote write path but also the existing query FFIs (vote_state, resources, voters_for_identity). Before this PR a bare even-length hex string such as "deadbeef" was decoded to Value::Bytes; at HEAD the same input produces Value::Text, with no error and no compatibility shim. Any existing C/Swift caller that intentionally passed unprefixed hex to query a Value::Bytes index will now silently query a different poll and get an empty result. The hardening on the signing path is correct (never auto-encode on a signing surface), but the read path's behavior change deserves either input validation that rejects bare hex on reads with an explanatory error, or an explicit CHANGELOG/migration note plus a deprecation period before flipping query semantics. As written, the only signal an external caller gets is silently-empty result sets.

source: ['codex', 'claude']

Comment on lines +30 to +41
match value_str.strip_prefix("0x") {
Some(hex_str) => {
let bytes = hex::decode(hex_str).map_err(|e| {
format!(
"Index value '{}' has a 0x prefix but is not valid hex: {}",
value_str, e
)
})?;
Ok(Value::Bytes(bytes))
}
None => Ok(Value::Text(value_str)),
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: 0x prefix as type tag is ambiguous for valid DPNS text labels starting with 0x

Replacing the even-length-all-hex heuristic with an explicit 0x prefix fixes the "deadbeef" case, but the prefix itself is still meaningful text: a DPNS contested label like "0xdeadbeef" matches DPNS's alphanumeric label rules and is signable. With the current parser, such a label routed through the Swift castContestedResourceVote path (which forwards the user-visible label string verbatim) is signed as Value::Bytes([0xde, 0xad, 0xbe, 0xef]) instead of Value::Text("0xdeadbeef"), so the masternode broadcasts a vote against a different vote-poll encoding than the one displayed in ContestDetailView. The vote is rejected or misdirected rather than counted for the displayed contest. A structured tag — e.g. { "type": "text"|"bytes", "value": ... } on the FFI surface, or pre-encoding the Swift caller's text labels with an unambiguous wrapper — removes the residual ambiguity on a signing surface. Treat any prefix-based magic on a signing input as a half-fix.

source: ['codex']

Comment on lines +292 to +296
let secp = Secp256k1::new();
let secret_key = SecretKey::from_byte_array(&key_array)
.map_err(|e| invalid(&format!("Invalid voting private key: {}", e)))?;
let public_key = PublicKey::from_secret_key(&secp, &secret_key);
let voting_address = hash160::Hash::hash(&public_key.serialize()).to_byte_array();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Voting public key is derived from a second SecretKey::from_byte_array instead of reusing the signer's key

SingleKeySigner::new_from_slice already parses and validates the 32 voting-key bytes as a dashcore::PrivateKey. cast_vote_inner then reparses the same bytes a second time as secp256k1::SecretKey purely to derive the compressed public key for the ECDSA_HASH160 voting-key data. There is no behavioral need for the second parse — the signer already owns an equivalent key and could expose the compressed public key. The duplicate parse keeps a redundant copy of the raw key in scope alongside the Zeroizing buffer and creates two independent definitions of "valid voting private key" that can drift if validation in one path tightens later. Reuse the signer's key to keep a single source of truth and reduce raw-key residue on the signing path.

source: ['claude']

@codecov

codecov Bot commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.20%. Comparing base (65042eb) to head (d85d9fc).
⚠️ Report is 13 commits behind head on v3.1-dev.

Additional details and impacted files
@@              Coverage Diff              @@
##           v3.1-dev    #3883       +/-   ##
=============================================
- Coverage     87.22%   71.20%   -16.03%     
=============================================
  Files          2641       20     -2621     
  Lines        328569     2837   -325732     
=============================================
- Hits         286597     2020   -284577     
+ Misses        41972      817    -41155     
Components Coverage Δ
dpp ∅ <ø> (∅)
drive ∅ <ø> (∅)
drive-abci ∅ <ø> (∅)
sdk ∅ <ø> (∅)
dapi-client ∅ <ø> (∅)
platform-version ∅ <ø> (∅)
platform-value ∅ <ø> (∅)
platform-wallet ∅ <ø> (∅)
drive-proof-verifier ∅ <ø> (∅)
🚀 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 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