feat(swift-sdk): wire real token actions through platform-wallet#3548
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (19)
📝 Walkthrough📝 Walkthrough✨ Finishing Touches🧪 Generate unit tests (beta)
|
Collaborator
|
🔍 Review in progress — actively reviewing now (commit 21e2c5f) |
Replaces "Action coming soon" placeholders in TokenActionPermissionsView with end-to-end submission paths for all 12 token actions plus group-action propose/co-sign discovery. Per the swift-sdk architectural rule, all flows route through rs-platform-wallet via rs-platform-wallet-ffi (8 MB worker stack); rs-sdk-ffi is not called from Swift. rs-platform-wallet (wallet/tokens/wallet.rs) - 12 *_external_signer wrappers (transfer/burn/mint/claim/freeze/ unfreeze/destroy_frozen_funds/pause/resume/set_price/purchase/ update_config) sharing fetch_data_contract + resolve_signing_key helpers. - New group_queries module exposing pending_group_actions_external and group_action_signers_external (typed views over rs-sdk's GroupActionsQuery). rs-platform-wallet-ffi (new src/tokens/ module) - 12 C-ABI submission entry points + 2 group-action query entry points returning JSON. group_info.rs decodes the GroupStateTransitionInfoStatus tuple (kind/position/action_id/ action_is_proposer) shared across every group-capable action. - update_config takes a (tag, JSON payload) shape; only MaxSupply is wired today, other tags reject cleanly to keep the FFI open-ended for follow-up. Sources/SwiftDashSDK - TokenActions.swift: GroupActionMode (none/propose/signExisting), TokenConfigChange, async wrappers on ManagedPlatformWallet for every action. - TokenGroupActionQueries.swift: Codable types (TokenGroupAction/TokenGroupActionParams/TokenGroupActionSigner) + tokenPendingGroupActions / tokenGroupActionSigners. - KeychainSigner reused for signing across all actions. SwiftExampleApp - 12 form views under Views/TokenActions/ following the CreateIdentityView pattern (form -> async submit -> alert). - RecipientPickerView (own identities + base58 paste + DPNS lookup) reused by Transfer/Mint/Freeze/Unfreeze/DestroyFrozen. - PendingGroupActionsView lists open proposals with signature tally (X of Y power); CoSignProposalView reconstructs the proposal's params and submits with GroupActionMode.signExisting. - New TokenActionKind.setDirectPurchasePrice routes Set Price to its own row (separate from the deferred .changeDistribution rich-schedule editor). - Legacy TokenActionsView / TokenActionDetailView / TokenAction enum removed; TokensView row tap now navigates to TokenActionPermissionsView. Deferred (placeholder shows "deferred to a future release"): - Set Conventions / Change Distribution full editors (per agreed scope). - Co-sign for tiered TokenPricingSchedule::SetPrices and for config-update variants other than MaxSupply. Open TODOs (noted inline): - signing_key_id is advisory; Rust still resolves the canonical AUTH/MASTER-or-HIGH/ECDSA_SECP256K1 key. - No synchronous PersistentTokenBalance refresh after submit (relies on next sync). - PersistentToken doesn't yet expose current direct-purchase price or circulating supply; some forms can't fully pre-validate client-side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer
force-pushed
the
codex/swift-sdk-token-actions
branch
from
1c85419 to
9ab6b2f
Compare
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: "8a1d27d7634c4a08e0613fc0504b6132b20b18201e2d386112d8d4108133e760"
)Xcode manual integration:
|
![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: 14
🤖 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/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/Tokens/TokenGroupActionQueries.swift`:
- Around line 212-214: The decoder for TokenGroupActionQueries currently treats
a present but non-numeric pricePerToken as nil; in the init(from:) where you
read pricePerToken (using decodeIfPresent on key .pricePerToken) change the
logic so that if decodeIfPresent returns a non-nil String and UInt64(...) fails,
you throw a DecodingError.dataCorruptedError(forKey: .pricePerToken, in:
container, debugDescription: "pricePerToken must be an unsigned integer"),
rather than mapping it to nil and falling through to the .disable
direct-purchase semantics; keep successful numeric parsing to produce .setPrice
as before.
- Around line 303-313: The optional startAtActionId passed into
tokenPendingGroupActions must be validated to be exactly 32 bytes before
converting to a [UInt8] and sending across FFI; update tokenPendingGroupActions
to check if let start = startAtActionId { guard start.count == 32 else { throw
/* appropriate Error (e.g. create or reuse an SDK error like InvalidArgument) */
} ; let startBytes: [UInt8] = Array(start) } and only pass the pointer when
length==32 (otherwise fail early with a clear error mentioning startAtActionId
length).
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/RecipientPickerView.swift`:
- Around line 82-87: When mode changes, cancel any in-flight DPNS resolution
Task (the Task created by scheduleDpnsResolve, e.g., dpnsResolveTask) inside the
.onChange(of: mode) handler and clear selection; additionally, in the DPNS
resolution completion path (the async Task started by scheduleDpnsResolve)
validate that the current mode and DPNS input still match the values that
triggered that Task before assigning selection to avoid applying stale results.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/CoSignProposalView.swift`:
- Around line 266-272: The co-sign button can be enabled for proposals with
params == .directPurchase even though dispatch always throws for that case;
update the canCoSign computed property to explicitly return false when
proposal.params is .directPurchase (or implement a proper dispatch handling for
.directPurchase in the dispatch method), and make the same change to the other
enable/guard logic that mirrors this behavior (the duplicate boolean check
around the submit/action button referenced in the review). Ensure you reference
proposal.params and the dispatch function when making the change so the UI never
enables a path that will always fail.
- Around line 249-251: The conversion that returns UInt32 from a generic value
can trap on negative integers (e.g., in CoSignProposalView where the snippet
checks "if let i = value as? Int" and "if let n = value as? NSNumber"); update
those branches to guard that the numeric value is non‑negative before converting
(e.g., check i >= 0 and for NSNumber use its signed value like int64Value >= 0)
and return a safe default (0) when negative or invalid so the UInt32 initializer
never receives a negative input.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/PendingGroupActionsView.swift`:
- Around line 279-287: The position(of:token:) helper can return Int values
outside UInt16 range and cause a runtime trap when callers cast to UInt16;
update position(of:token:) to validate and only return values within
0...UInt16.max (65535) — for the mainControlGroupPosition path
(PersistentToken.mainControlGroupPosition) and the parsed Group:<n> branch,
check that the Int is >= 0 and <= 65535 and return nil otherwise so callers that
narrow to UInt16 won't crash.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenBurnActionView.swift`:
- Around line 169-177: In submit(), re-validate the parsedAmount against the
current wallet balance before proceeding: in addition to checking amount > 0 and
managedWallet, also ensure amount <= balanceValue (the same constraint used in
canSubmit) and set submitError appropriately if it exceeds the balance; update
the guard in submit() (referencing submit(), parsedAmount, managedWallet,
balanceValue, and submitError) so an over-burn cannot be submitted if the
balance changed between render and tap.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenClaimActionView.swift`:
- Line 157: The unchecked UInt16 conversion 'let position =
UInt16(token.position)' can trap for out-of-range values; replace it with a
guarded failable initializer (e.g., guard let position = UInt16(token.position)
else { ... }) and handle the failure path (return early, present an error, or
disable the action) following the safe pattern used in IdentityDetailView; apply
the same fix to all token action views that perform this conversion
(TokenClaimActionView, TokenUnfreezeActionView, TokenUpdateMaxSupplyActionView,
TokenTransferActionView, TokenSetPriceActionView, TokenPurchaseActionView,
TokenFreezeActionView, TokenMintActionView, TokenResumeActionView,
TokenPauseActionView, TokenBurnActionView, TokenDestroyFrozenFundsActionView) so
no unchecked UInt16(token.position) casts remain.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenFreezeActionView.swift`:
- Around line 149-151: The conversion UInt16($0) can trap for negative or >65535
values from inferredGroupPosition (which comes from
token.mainControlGroupPosition); instead validate the Int first: check
inferredGroupPosition is non-nil and within 0...Int(UInt16.max) (or handle
out-of-range by returning .none or surfacing a user error), then safely
construct UInt16 and use .propose(position:); update the logic around
inferredGroupPosition and GroupActionMode to perform this guarded range-check
before creating the UInt16.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenMintActionView.swift`:
- Around line 178-180: The code currently parses "Group:<position>" into a
signed Int (the if authorized.hasPrefix("Group:"), let pos = Int(...)) and later
converts that Int to UInt16 with UInt16(...) which can crash on out-of-range
values; update the parsing flow to validate bounds by replacing the blind
UInt16(...) conversions with UInt16(exactly:) and handle the nil case by
returning early or setting the appropriate error state (e.g., when converting
the parsed pos from the authorized string or wherever UInt16(...) is used around
TokenMintActionView), so negative or >65535 values do not cause a trap.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenTransferActionView.swift`:
- Line 155: The unchecked narrowing UInt16(token.position) can crash when
token.position > UInt16.max; update each TokenActions view (e.g.
TokenTransferActionView, TokenUpdateMaxSupplyActionView, TokenMintActionView,
TokenPauseActionView, TokenResumeActionView, TokenSetPriceActionView,
TokenClaimActionView, TokenBurnActionView, TokenUnfreezeActionView,
TokenDestroyFrozenFundsActionView, TokenFreezeActionView,
TokenPurchaseActionView) to use a safe conversion: either validate bounds with a
guard (token.position >= 0 && token.position <= Int(UInt16.max)) before casting,
or use guard let position = UInt16(exactly: token.position) and handle failure
(e.g. set submitError or return) following the pattern in
IdentityDetailView.swift so no runtime trap occurs.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenUnfreezeActionView.swift`:
- Around line 145-147: The code constructs groupAction with .propose(position:
UInt16($0)) which can trap if inferredGroupPosition contains a negative or
>65535 value; change the conversion to use UInt16(exactly:) and handle the nil
case (e.g., fall back to .none or another safe path) so out-of-range values
don't crash; update the expression that builds groupAction (referencing
inferredGroupPosition and GroupActionMode.propose / groupAction) to perform the
safe conversion and conditional behavior.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenUpdateMaxSupplyActionView.swift`:
- Around line 154-156: The code currently performs unchecked Int → UInt16
conversions that can crash; update the three sites to use UInt16(exactly:) and
handle the optional result: for the authorized parsing branch where you parse
pos from authorized.hasPrefix("Group:"), validate with UInt16(exactly: pos) and
return a safe optional or early-fail path instead of returning a raw Int; where
token.position is converted, replace UInt16(token.position) with guard-let uPos
= UInt16(exactly: token.position) and propagate/handle the failure; and inside
the map over inferredGroupPosition, use compactMap/guard with UInt16(exactly:)
so only valid UInt16 values are used (or surface an error) instead of
force-converting. Ensure you update callers/signatures as needed to
accept/propagate optionals or errors.
🪄 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: 2fc1c65d-8776-4efb-aa6f-1a6b469e5903
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
packages/rs-platform-wallet-ffi/Cargo.tomlpackages/rs-platform-wallet-ffi/src/lib.rspackages/rs-platform-wallet-ffi/src/tokens/burn.rspackages/rs-platform-wallet-ffi/src/tokens/claim.rspackages/rs-platform-wallet-ffi/src/tokens/destroy_frozen_funds.rspackages/rs-platform-wallet-ffi/src/tokens/freeze.rspackages/rs-platform-wallet-ffi/src/tokens/group_info.rspackages/rs-platform-wallet-ffi/src/tokens/group_queries.rspackages/rs-platform-wallet-ffi/src/tokens/mint.rspackages/rs-platform-wallet-ffi/src/tokens/mod.rspackages/rs-platform-wallet-ffi/src/tokens/pause.rspackages/rs-platform-wallet-ffi/src/tokens/purchase.rspackages/rs-platform-wallet-ffi/src/tokens/resume.rspackages/rs-platform-wallet-ffi/src/tokens/set_price.rspackages/rs-platform-wallet-ffi/src/tokens/transfer.rspackages/rs-platform-wallet-ffi/src/tokens/unfreeze.rspackages/rs-platform-wallet-ffi/src/tokens/update_config.rspackages/rs-platform-wallet/src/wallet/tokens/group_queries.rspackages/rs-platform-wallet/src/wallet/tokens/mod.rspackages/rs-platform-wallet/src/wallet/tokens/wallet.rspackages/swift-sdk/Sources/SwiftDashSDK/Models/TokenAction.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/Tokens/TokenActions.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/Tokens/TokenGroupActionQueries.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/RecipientPickerView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/CoSignProposalView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/PendingGroupActionsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenBurnActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenClaimActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenDestroyFrozenFundsActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenFreezeActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenMintActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenPauseActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenPurchaseActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenResumeActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenSetPriceActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenTransferActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenUnfreezeActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenUpdateMaxSupplyActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenDetailsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokensView.swift
💤 Files with no reviewable changes (1)
- packages/swift-sdk/Sources/SwiftDashSDK/Models/TokenAction.swift
`PersistentIdentity.identityIdString` is a computed property, not a stored SwiftData field. Using it in a `SortDescriptor` keypath crashes at runtime when the @query first materialises: Fatal error: Couldn't find \PersistentIdentity.identityIdString on PersistentIdentity with fields [...identityId..., ...balance..., ...] Replace the secondary-sort tiebreaker with `\.alias` (stored optional String, line 12 of the model). `identityIndex` remains the primary sort key. Affected three @query usages added in this PR — RecipientPickerView and the two identity pickers in TokenActionPermissionsView. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Token *actions* (transfer/burn/mint/claim/freeze/unfreeze/destroy_frozen_funds/
pause/resume/set_price/purchase/update_config) are identity-as-actor
operations — every method takes an `Identifier` for the actor identity,
and signing pulls a key off that identity. Hosting them on `TokenWallet`
(a sibling facade to `IdentityWallet`) forced FFI callers to keep
juggling `(wallet_handle, identity_id)` separately and put token
actions out of step with every other identity operation.
Mirror the DashPay merge that already happened on this codebase
(wallet/identity/network/mod.rs:7-15): move the actions onto
`IdentityWallet`, keep the wallet-level token *bookkeeping*
(watch/unwatch/sync/balance) on `TokenWallet`.
Layout after this refactor:
wallet/tokens/
wallet.rs # bookkeeping ONLY
group_queries.rs # token-contract-scoped reads (unchanged)
wallet/identity/network/tokens/ # NEW — 12 actions live here
mod.rs
helpers.rs # fetch_data_contract, resolve_signing_key,
# resolve_identity_and_signer (moved from
# TokenWallet, now pub(super) on IdentityWallet)
transfer.rs # token_transfer{,_with_signer,_with_external_signer}
burn.rs # token_burn...
mint.rs
claim.rs
freeze.rs
unfreeze.rs
destroy_frozen_funds.rs
pause.rs
resume.rs
set_price.rs
purchase.rs
update_config.rs
Method naming follows the established IdentityWallet convention seen in
transfer.rs / withdrawal.rs / dpns.rs:
token_<action>_with_external_signer (preferred for external signers)
token_<action>_with_signer
token_<action> (legacy helpers; constructs
IdentitySigner from manager)
`token_` prefix disambiguates from the existing `transfer_credits` family
on IdentityWallet.
FFI shims (rs-platform-wallet-ffi/src/tokens/*.rs) re-pointed:
let token_wallet = wallet.tokens().clone();
token_wallet.<action>_external_signer(identity_id, ...)
→
let identity_wallet = wallet.identity().clone();
identity_wallet.token_<action>_with_external_signer(identity_id, ...)
The 14 C-ABI symbols are byte-identical before vs after — Swift wrappers
need no changes, the SwiftExampleApp builds clean with -warnings-as-errors,
and 110 + 69 platform-wallet / platform-wallet-ffi tests pass.
The two FFI query files (group_queries.rs) keep calling wallet.tokens()
because pending_group_actions / group_action_signers are token-contract-
scoped reads, not identity-scoped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift (1)
724-798:⚠️ Potential issue | 🟠 MajorMissing
maxSupplyChangeRulesfrom group-capable rules check.The visibility gate uses
TokenGroupRuleResolver.relevantGroupPositions(for:)which correctly handlesMainGroupviatoken.mainControlGroupPositionin theposition(of:rule:token:)method. However,groupCapableRulesomitsmaxSupplyChangeRulesdespiteTokenUpdateMaxSupplyActionViewbeing explicitly documented as "Group-capable" (line 13-16). Tokens governed only bymainGroupfor max-supply changes will not expose the "Pending Group Actions" section.Add
token.maxSupplyChangeRulesto the returned array ingroupCapableRulesat line 268–276 inPendingGroupActionsView.swift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift` around lines 724 - 798, hasGroupCapableRule currently relies on TokenGroupRuleResolver.relevantGroupPositions(for:) but PendingGroupActionsView's groupCapableRules omits maxSupplyChangeRules, so tokens with only mainGroup max-supply governance won't show pending group actions; update the groupCapableRules array (in PendingGroupActionsView, referenced by groupCapableRules) to include token.maxSupplyChangeRules alongside token.transferRules, token.updateRules, token.updateMetadataRules, etc., so TokenGroupRuleResolver.relevantGroupPositions(for:) can detect MainGroup positions for max-supply changes and restore the "Pending Group Actions" visibility.
🧹 Nitpick comments (2)
packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs (1)
24-101: Factor the auth-key lookup into one helper.These two methods repeat the same wallet lookup, identity fetch, and
get_first_public_key_matching(...)criteria. Keeping that policy in one place will make the internal-signer and external-signer paths much harder to drift apart on the next token-key selection change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs` around lines 24 - 101, Extract the repeated wallet lookup, identity fetch, identity_index lookup and auth-key selection into a single private helper (e.g. token_resolve_identity_and_auth_key) that takes &self and identity_id and returns Result<(Identity, IdentityPublicKey, IdentityIndex), PlatformWalletError>; then make token_resolve_identity_and_signer call that helper to get (identity, signing_key, identity_index) and build IdentitySigner as before, and make token_resolve_signing_key call the helper and return the signing_key; preserve the same error variants and the get_first_public_key_matching criteria/arguments when moving the auth-key lookup into the new helper.packages/rs-platform-wallet/src/wallet/identity/network/tokens/mint.rs (1)
31-36: Use the mint builder's recipient setter for consistency and future-proofing.Line 35 writes
builder.recipient_iddirectly, buttoken_mint_with_signer(line 69) usesissued_to_identity_id(...). While currently equivalent, theissued_to_identity_idmethod has a TODO comment indicating future validation withminting_allow_choosing_destination. Using the setter ensures both paths remain consistent and the code is prepared for that validation when implemented.Proposed change
let mut builder = TokenMintTransitionBuilder::new(data_contract, token_position, *identity_id, amount); if let Some(recipient) = recipient_id { - builder.recipient_id = Some(recipient); + builder = builder.issued_to_identity_id(recipient); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-platform-wallet/src/wallet/identity/network/tokens/mint.rs` around lines 31 - 36, The code mutates TokenMintTransitionBuilder.recipient_id directly (builder.recipient_id = Some(recipient)) which bypasses the builder's issued_to_identity_id(...) setter used elsewhere (e.g., token_mint_with_signer) and may skip future validation; replace the direct assignment with a call to TokenMintTransitionBuilder::issued_to_identity_id (or builder.issued_to_identity_id(recipient)) so the builder's API and any upcoming minting_allow_choosing_destination checks are applied consistently across paths.
🤖 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-platform-wallet/src/wallet/identity/network/tokens/set_price.rs`:
- Around line 25-42: token_set_price currently always calls
TokenChangeDirectPurchasePriceTransitionBuilder::with_single_price(price) which
causes a zero price to be published instead of clearing the direct-purchase
schedule; change token_set_price to branch on the incoming price: if price == 0
invoke the builder variant that clears the direct-purchase schedule (pass
None/clear) to match the external-signer behavior, otherwise pass the provided
price (Some(price)); update the code around
TokenChangeDirectPurchasePriceTransitionBuilder::new and .with_single_price to
use the conditional path so managed-signer callers can disable direct purchase
by passing price == 0.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift`:
- Around line 466-487: The new .setDirectPurchasePrice row is causing
.changeDistribution to incorrectly fall back to
changeDirectPurchasePricingRules; update resolve() so .changeDistribution is
resolved from its own rule (e.g.,
token.distributionChangeRules?.changeDistributionRules) and not by reusing
changeDirectPurchasePricingRules, and ensure that when that specific
changeDistribution rule is nil you append ResolvedTokenAction(kind:
.changeDistribution, permission: .hidden) instead of routing to the deferred
placeholder—leave the existing .setDirectPurchasePrice logic as-is but stop
using changeDirectPurchasePricingRules as a fallback for .changeDistribution.
---
Outside diff comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift`:
- Around line 724-798: hasGroupCapableRule currently relies on
TokenGroupRuleResolver.relevantGroupPositions(for:) but
PendingGroupActionsView's groupCapableRules omits maxSupplyChangeRules, so
tokens with only mainGroup max-supply governance won't show pending group
actions; update the groupCapableRules array (in PendingGroupActionsView,
referenced by groupCapableRules) to include token.maxSupplyChangeRules alongside
token.transferRules, token.updateRules, token.updateMetadataRules, etc., so
TokenGroupRuleResolver.relevantGroupPositions(for:) can detect MainGroup
positions for max-supply changes and restore the "Pending Group Actions"
visibility.
---
Nitpick comments:
In `@packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs`:
- Around line 24-101: Extract the repeated wallet lookup, identity fetch,
identity_index lookup and auth-key selection into a single private helper (e.g.
token_resolve_identity_and_auth_key) that takes &self and identity_id and
returns Result<(Identity, IdentityPublicKey, IdentityIndex),
PlatformWalletError>; then make token_resolve_identity_and_signer call that
helper to get (identity, signing_key, identity_index) and build IdentitySigner
as before, and make token_resolve_signing_key call the helper and return the
signing_key; preserve the same error variants and the
get_first_public_key_matching criteria/arguments when moving the auth-key lookup
into the new helper.
In `@packages/rs-platform-wallet/src/wallet/identity/network/tokens/mint.rs`:
- Around line 31-36: The code mutates TokenMintTransitionBuilder.recipient_id
directly (builder.recipient_id = Some(recipient)) which bypasses the builder's
issued_to_identity_id(...) setter used elsewhere (e.g., token_mint_with_signer)
and may skip future validation; replace the direct assignment with a call to
TokenMintTransitionBuilder::issued_to_identity_id (or
builder.issued_to_identity_id(recipient)) so the builder's API and any upcoming
minting_allow_choosing_destination checks are applied consistently across paths.
🪄 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: 3cbbcb99-db52-4d77-8fea-de4a519c9e8e
📒 Files selected for processing (30)
packages/rs-platform-wallet-ffi/src/tokens/burn.rspackages/rs-platform-wallet-ffi/src/tokens/claim.rspackages/rs-platform-wallet-ffi/src/tokens/destroy_frozen_funds.rspackages/rs-platform-wallet-ffi/src/tokens/freeze.rspackages/rs-platform-wallet-ffi/src/tokens/mint.rspackages/rs-platform-wallet-ffi/src/tokens/pause.rspackages/rs-platform-wallet-ffi/src/tokens/purchase.rspackages/rs-platform-wallet-ffi/src/tokens/resume.rspackages/rs-platform-wallet-ffi/src/tokens/set_price.rspackages/rs-platform-wallet-ffi/src/tokens/transfer.rspackages/rs-platform-wallet-ffi/src/tokens/unfreeze.rspackages/rs-platform-wallet-ffi/src/tokens/update_config.rspackages/rs-platform-wallet/src/wallet/identity/network/mod.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/burn.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/claim.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/destroy_frozen_funds.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/freeze.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/mint.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/mod.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/pause.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/purchase.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/resume.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/set_price.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/transfer.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/unfreeze.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/update_config.rspackages/rs-platform-wallet/src/wallet/tokens/wallet.rspackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/RecipientPickerView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift
✅ Files skipped from review due to trivial changes (1)
- packages/rs-platform-wallet/src/wallet/identity/network/mod.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/rs-platform-wallet-ffi/src/tokens/transfer.rs
- packages/rs-platform-wallet-ffi/src/tokens/destroy_frozen_funds.rs
- packages/rs-platform-wallet-ffi/src/tokens/set_price.rs
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/RecipientPickerView.swift
- packages/rs-platform-wallet-ffi/src/tokens/claim.rs
ZocoLini
requested changes
Apr 27, 2026
| ) -> PlatformWalletFFIResult | ||
|
|
||
| @_silgen_name("platform_wallet_token_set_price") | ||
| func platform_wallet_token_set_price( |
Collaborator
There was a problem hiding this comment.
remove all symbols defined with @_silgen_name, they should be generated in the header files if they belong to the public API exposed by any of out FFI crates, that is the only way we can ensure that changes in the FFI crates get reflected in the public API Swift can see
Member
Author
There was a problem hiding this comment.
Agreed in principle — but flipping all platform-wallet-ffi callers to a generated header is gated by the duplicate-declaration issue documented in build_ios.sh:141-144, which references the upstream PR #3500 migration. Until that lands, every platform-wallet-ffi Swift caller (data_contract.rs, dpns.rs, dashpay.rs, dashpay_profile.rs, identity_*) uses @_silgen_name — these new token bindings follow that established convention. Happy to do the umbrella-header migration as a follow-up PR that flips them all at once; bundling it here would balloon the diff and conflict with the same migration when #3500 lands.
Collaborator
There was a problem hiding this comment.
I will do it myself after the merge, no blocking
…tySigner-backed plain methods The plain (no-signer) variants of identity-as-actor methods existed solely to construct an IdentitySigner / ManagedIdentitySigner internally and re-derive the private key from the wallet seed at signing time. That pattern (a) dies on watch-only wallets — no seed Rust-side, (b) deadlocks the Tokio worker when its derivation tries to blocking_read the wallet-manager lock from inside a signing future, and (c) duplicates the seed pipeline the FFI's external signer (KeychainSigner) already owns. Every method has _with_signer / _with_external_signer siblings that are the canonical surface; the plain wrappers were just footguns. Deleted: - ManagedIdentitySigner struct + impls (all of it — no remaining callers). - IdentityWallet::signer_for(...) factory returning ManagedIdentitySigner. - IdentityWallet::transfer_credits, withdraw_credits, update_identity, transfer_credits_to_addresses (4 plain methods, each subsumed by its _with_external_signer sibling). - 12 plain token_<action> methods (Wave 9 had added these mirroring the legacy shape; Wave 10 removes them — token_<action>_with_external_signer is the only path now). - tokens/helpers.rs::token_resolve_identity_and_signer (the IdentitySigner branch); the non-IdentitySigner helpers used by _with_signer and _with_external_signer paths stay. Deferred — IdentitySigner struct itself stays for now because 5 plain methods are still wired through dedicated FFI shims that Swift consumers call via no-signer overloads: - register_name → platform_wallet_register_dpns_name - send_contact_request → platform_wallet_send_contact_request - accept_contact_request → platform_wallet_accept_contact_request - create_profile / update_profile → platform_wallet_*_dashpay_profile Plus registration.rs::register_identity_with_funding still uses signer_for_identity. Removing IdentitySigner entirely requires migrating those Swift call sites (next commit). Validation: - cargo fmt --all clean - cargo check --workspace clean - cargo clippy -p platform-wallet -p platform-wallet-ffi --tests: no new warnings - cargo test -p platform-wallet --lib: 110/110 pass - cargo test -p platform-wallet-ffi --lib: 69/69 pass - build_ios.sh --target sim: BUILD SUCCEEDED - xcodebuild SwiftExampleApp iPhone 17 sim: BUILD SUCCEEDED - 14/14 token FFI symbols still exported - Swift code byte-identical (no Sources/ or SwiftExampleApp/ files modified) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ZocoLini
requested changes
Apr 27, 2026
| ); | ||
| } | ||
| return PlatformWalletFFIResult::ErrorNullPointer; | ||
| } |
Collaborator
There was a problem hiding this comment.
Also, this error system is kinda overkill, I guess you have it align with what we have in other places in platform-wallet-ffi, why returning a FFIResult and, at the same time, we defer out_error to set again the code but with a message this time. It would be way easier if you just return something like:
struct PlatformWalletFFIResult {
code: PlatformWalletFFIResultCode,
msg: *const ffi::c_char
}returning an struct like this and removing the out_error defer would allow us to implement the Into trait for all kind of errors, removing boilerplate, making everything more consistent, centralizing the error conversion, and making everything more readable. Right now, every place where we have to return an error is +8 lines and 3 indentations, kinda to much when we can use the ? op
Member
Author
There was a problem hiding this comment.
Agreed the boilerplate is heavy — out_error defer + early returns produces +8 lines and 3 indentations per error site. The struct-return shape with Into impls is the right design.
The refactor touches every pub unsafe extern "C" fn in rs-platform-wallet-ffi (~30+ functions across data_contract / dpns / dashpay / dashpay_profile / identity_* / tokens / addresses / spv) plus every Swift error-reading call site. Way larger blast radius than the token-actions PR scope. Happy to land it as a focused follow-up PR; would you prefer to drive that, or want me to take it?
| out_error: *mut PlatformWalletFFIError, | ||
| ) -> PlatformWalletFFIResult { | ||
| if signer_handle.is_null() { | ||
| if !out_error.is_null() { |
Collaborator
There was a problem hiding this comment.
sadly this cannot use the ? in any situation but, in key-wallet-ffi there are 2 macros to handle null pointers, you can grab then and adapt them to this error system. One of the macros just checks null ptrs and the other checks and derefs them
Member
Author
There was a problem hiding this comment.
Agreed — the key-wallet-ffi null-pointer macros (unwrap_or_null / similar) are the right tool to cut the per-arg-check boilerplate. They're tightly coupled to whichever error system we end up with, so I'd land them in the same follow-up that addresses the broader error-system refactor rather than half-adapt them here.
Finishes the antipattern removal Wave 10 started. Migrates the 5 remaining plain-method consumers (DPNS register, send/accept contact request, create/update DashPay profile) onto their _with_signer siblings; deletes the legacy FFI shims that exposed them; deletes the plain Rust methods; deletes IdentitySigner itself. The plain register_identity / register_identity_with_funding methods on IdentityWallet were the only callers of signer_for_identity. They weren't FFI-exposed (the registration FFI surface goes through register_identity_with_funding_external_signer). Removing them takes signer_for_identity with them, which takes IdentitySigner with it. Swift call-site migration: - 5 ManagedPlatformWallet no-signer overloads deleted (registerDpnsName, sendContactRequest, acceptContactRequest, createDashPayProfile, updateDashPayProfile). Callers must now pass a KeychainSigner — same pattern CreateIdentityView already uses everywhere else. - Only one app-level call site needed migration: FriendsView.acceptRequest now constructs `KeychainSigner(modelContainer: modelContext.container)` and calls the existing _with_signer overload. Every other call site was already on the _with_signer path. FFI shims deleted: - platform_wallet_register_dpns_name - platform_wallet_send_contact_request - platform_wallet_accept_contact_request - platform_wallet_create_dashpay_profile - platform_wallet_update_dashpay_profile Plus the private `create_or_update_profile` FFI helper that backed the two profile shims. Plain Rust methods deleted: - IdentityWallet::register_name - IdentityWallet::send_contact_request - IdentityWallet::accept_contact_request - IdentityWallet::create_profile - IdentityWallet::update_profile - IdentityWallet::register_identity - IdentityWallet::register_identity_with_funding - IdentityWallet::signer_for_identity (factory — only consumer left was register_identity_with_funding) `packages/rs-platform-wallet/src/wallet/signer.rs` deleted (the file held only IdentitySigner). `pub use signer::IdentitySigner` removed from `wallet/mod.rs`. `mod signer;` removed. Final grep — `IdentitySigner` matches only `PLAN.md` (historical architecture doc) in `rs-platform-wallet/`, `rs-platform-wallet-ffi/`, and `swift-sdk/`. Zero code references. wasm-dpp2's separate IdentitySigner wasm-bindgen wrapper is unaffected (different namespace). Validation: - cargo fmt --all clean - cargo check --workspace clean - cargo clippy -p platform-wallet -p platform-wallet-ffi --tests: no new warnings - cargo test -p platform-wallet --lib: 110/110 pass - cargo test -p platform-wallet-ffi --lib: 69/69 pass - build_ios.sh --target sim: BUILD SUCCEEDED - xcodebuild SwiftExampleApp iPhone 17 sim: BUILD SUCCEEDED - 14/14 token FFI symbols still exported Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/FriendsView.swift (1)
245-258:⚠️ Potential issue | 🟠 MajorGuard the new accept path against double-submits and surface failures.
This is now a real platform write, but the row can still be tapped repeatedly while the task is in flight, and
errorMessageis never rendered anywhere inFriendsView. A double tap can race two accepts, and a signer/wallet failure leaves the request stuck with no visible feedback.As per coding guidelines, "Use
NavigationLinkfor drill-down navigation and implement proper loading and error states in UI views."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/FriendsView.swift` around lines 245 - 258, The acceptRequest handler can be double-submitted and never surfaces errors; add an in-flight guard and proper error handling/display: introduce a per-request processing state (e.g. a Set or dictionary keyed by request.senderId) checked at the top of acceptRequest(_:), return early if already processing, add/clear the key around the Task to prevent double-submits, ensure the do/catch sets errorMessage on failure (include error.localizedDescription) and clears it on success, and update FriendsView to render errorMessage and a per-row loading indicator or disable the row while the request is in-flight; reference acceptRequest(_:), errorMessage, KeychainSigner, managed.getIncomingContactRequest(senderId:), and wallet.acceptContactRequest(..) to locate where to apply these changes.
🧹 Nitpick comments (2)
packages/rs-platform-wallet/src/wallet/identity/network/transfer.rs (1)
95-105: Nit: minor inconsistency inSignerRefusage between siblings.Here the call site passes
SignerRef(signer)by value (Line 105), while the siblingtransfer_credits_to_addresses_with_external_signerintransfer_to_addresses.rspasses&SignerRef(signer). Both compile (the SDK's generic bound differs slightly between the two call sites), but a brief comment or unifying the call style would aid readers cross-referencing the two transfer paths. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-platform-wallet/src/wallet/identity/network/transfer.rs` around lines 95 - 105, Unify the SignerRef usage or add a clarifying comment: change the call in transfer.rs where identity.transfer_credits(..., SignerRef(signer)) is passed to match the sibling transfer_credits_to_addresses_with_external_signer in transfer_to_addresses.rs by passing &SignerRef(signer), or alternatively add a one-line comment next to the current call explaining why a by-value SignerRef is used here (e.g., differing SDK generic bound), referencing the identity.transfer_credits call site and the transfer_credits_to_addresses_with_external_signer function to help future readers.packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs (1)
33-37: Avoid cloning the fullIdentityjust to extract one key.You clone the entire
Identity(potentially many keys + metadata) only to pull a single public key out of it on line 39–51, then drop it. You can hold the reference a moment longer and clone just the matched key:♻️ Suggested refactor
- let identity = info - .identity_manager - .identity(identity_id) - .map(|m| m.identity.clone()) - .ok_or(PlatformWalletError::IdentityNotFound(*identity_id))?; - - let signing_key = identity - .get_first_public_key_matching( + let managed = info + .identity_manager + .identity(identity_id) + .ok_or(PlatformWalletError::IdentityNotFound(*identity_id))?; + + let signing_key = managed + .identity + .get_first_public_key_matching( Purpose::AUTHENTICATION, [SecurityLevel::MASTER, SecurityLevel::HIGH].into(), [KeyType::ECDSA_SECP256K1].into(), false, ) .ok_or_else(|| { PlatformWalletError::InvalidIdentityData( "No authentication key found on identity".to_string(), ) })? .clone();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs` around lines 33 - 37, The code currently calls identity_manager.identity(identity_id).map(|m| m.identity.clone()) which clones the entire Identity just to extract a single public key; instead, keep the reference to the identity, locate the matching key from the referenced Identity (using the same matching logic currently at lines 39–51), clone only that PublicKey (or its bytes) and return it; change the flow to obtain a reference from identity_manager.identity(identity_id).ok_or(PlatformWalletError::IdentityNotFound(*identity_id))?, then search its keys and clone the matched key (avoid cloning Identity::identity), keeping PlatformWalletError::IdentityNotFound and identity_id usage the same.
🤖 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-platform-wallet-ffi/src/dashpay_profile.rs`:
- Around line 17-21: Remove the stale Swift FFI declarations
platform_wallet_create_dashpay_profile and
platform_wallet_update_dashpay_profile (they no longer exist in Rust) and
replace their call sites to use the existing Rust symbol
platform_wallet_create_or_update_dashpay_profile_with_signer; specifically,
delete the declarations in PlatformWalletFFI.swift (previously around lines
where those functions were declared) and update the calls from
ManagedPlatformWallet.swift (calls at the noted locations) and the example app
to invoke platform_wallet_create_or_update_dashpay_profile_with_signer, passing
the signer parameter and any required additional arguments and error/return
handling expected by the signer-backed variant; also update any bridging/header
references and signatures so the Swift side matches the Rust FFI function
signature exactly.
In `@packages/rs-platform-wallet-ffi/src/dashpay.rs`:
- Around line 14-24: Remove the stale Swift FFI declarations for
platform_wallet_send_contact_request and platform_wallet_accept_contact_request
from PlatformWalletFFI.swift: locate and delete the `@_silgen_name` bindings for
those two functions (the dead bindings around where
managed_identity_send_contact_request and
managed_identity_accept_contact_request are declared) so the Swift bridge only
exposes the actual Rust-exported signer-based variants
(managed_identity_send_contact_request and
managed_identity_accept_contact_request) and eliminate the misleading,
non-existent Rust symbol references.
In `@packages/rs-platform-wallet-ffi/src/dpns.rs`:
- Around line 4-19: Remove the stale Swift FFI declaration for the deleted Rust
symbol platform_wallet_register_dpns_name from the Swift bridge
(PlatformWalletFFI.swift) so it cannot be linked at build time; keep the
existing correct declaration and call-site that use
platform_wallet_register_dpns_name_with_signer (and the Swift wrapper
registerDpnsName) unchanged, simply delete the obsolete extern declaration for
platform_wallet_register_dpns_name to avoid link-time errors.
In `@packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs`:
- Around line 39-51: The current lookup in helpers.rs uses
identity.get_first_public_key_matching with Purpose::AUTHENTICATION,
SecurityLevel::{MASTER, HIGH}, and KeyType::ECDSA_SECP256K1 and raises
PlatformWalletError::InvalidIdentityData("No authentication key found on
identity") which is misleading when keys exist but don't match that exact
triplet; update the helper that selects the signing key (the
get_first_public_key_matching call and its surrounding code) to either implement
a fallback ladder over security levels (e.g., try MASTER → HIGH → CRITICAL →
MEDIUM) or broaden the SecurityLevel set, and always include the actual search
criteria and the identity id in the error (e.g., mention Purpose, KeyType,
attempted SecurityLevels, and identity identifier) instead of the generic
message; also update the doc comment above this helper to state that Rust
enforces the canonical signing_key selection policy (that signing_key_id from
FFI is advisory) so callers understand the fixed selection behavior.
---
Outside diff comments:
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/FriendsView.swift`:
- Around line 245-258: The acceptRequest handler can be double-submitted and
never surfaces errors; add an in-flight guard and proper error handling/display:
introduce a per-request processing state (e.g. a Set or dictionary keyed by
request.senderId) checked at the top of acceptRequest(_:), return early if
already processing, add/clear the key around the Task to prevent double-submits,
ensure the do/catch sets errorMessage on failure (include
error.localizedDescription) and clears it on success, and update FriendsView to
render errorMessage and a per-row loading indicator or disable the row while the
request is in-flight; reference acceptRequest(_:), errorMessage, KeychainSigner,
managed.getIncomingContactRequest(senderId:), and
wallet.acceptContactRequest(..) to locate where to apply these changes.
---
Nitpick comments:
In `@packages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rs`:
- Around line 33-37: The code currently calls
identity_manager.identity(identity_id).map(|m| m.identity.clone()) which clones
the entire Identity just to extract a single public key; instead, keep the
reference to the identity, locate the matching key from the referenced Identity
(using the same matching logic currently at lines 39–51), clone only that
PublicKey (or its bytes) and return it; change the flow to obtain a reference
from
identity_manager.identity(identity_id).ok_or(PlatformWalletError::IdentityNotFound(*identity_id))?,
then search its keys and clone the matched key (avoid cloning
Identity::identity), keeping PlatformWalletError::IdentityNotFound and
identity_id usage the same.
In `@packages/rs-platform-wallet/src/wallet/identity/network/transfer.rs`:
- Around line 95-105: Unify the SignerRef usage or add a clarifying comment:
change the call in transfer.rs where identity.transfer_credits(...,
SignerRef(signer)) is passed to match the sibling
transfer_credits_to_addresses_with_external_signer in transfer_to_addresses.rs
by passing &SignerRef(signer), or alternatively add a one-line comment next to
the current call explaining why a by-value SignerRef is used here (e.g.,
differing SDK generic bound), referencing the identity.transfer_credits call
site and the transfer_credits_to_addresses_with_external_signer function to help
future readers.
🪄 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: 62612b46-d944-4076-8069-18508ca3e3b2
📒 Files selected for processing (37)
packages/rs-platform-wallet-ffi/src/dashpay.rspackages/rs-platform-wallet-ffi/src/dashpay_profile.rspackages/rs-platform-wallet-ffi/src/dpns.rspackages/rs-platform-wallet-ffi/src/identity_registration_funded_with_signer.rspackages/rs-platform-wallet-ffi/src/identity_transfer.rspackages/rs-platform-wallet-ffi/src/identity_update.rspackages/rs-platform-wallet-ffi/src/identity_withdrawal.rspackages/rs-platform-wallet/src/lib.rspackages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rspackages/rs-platform-wallet/src/wallet/identity/network/dpns.rspackages/rs-platform-wallet/src/wallet/identity/network/identity_handle.rspackages/rs-platform-wallet/src/wallet/identity/network/profile.rspackages/rs-platform-wallet/src/wallet/identity/network/registration.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/burn.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/claim.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/destroy_frozen_funds.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/freeze.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/helpers.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/mint.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/pause.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/purchase.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/resume.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/set_price.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/transfer.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/unfreeze.rspackages/rs-platform-wallet/src/wallet/identity/network/tokens/update_config.rspackages/rs-platform-wallet/src/wallet/identity/network/transfer.rspackages/rs-platform-wallet/src/wallet/identity/network/transfer_to_addresses.rspackages/rs-platform-wallet/src/wallet/identity/network/update.rspackages/rs-platform-wallet/src/wallet/identity/network/withdrawal.rspackages/rs-platform-wallet/src/wallet/mod.rspackages/rs-platform-wallet/src/wallet/signer.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/FriendsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegisterNameView.swift
💤 Files with no reviewable changes (9)
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegisterNameView.swift
- packages/rs-platform-wallet-ffi/src/identity_transfer.rs
- packages/rs-platform-wallet-ffi/src/identity_update.rs
- packages/rs-platform-wallet/src/lib.rs
- packages/rs-platform-wallet-ffi/src/identity_withdrawal.rs
- packages/rs-platform-wallet-ffi/src/identity_registration_funded_with_signer.rs
- packages/rs-platform-wallet/src/wallet/signer.rs
- packages/rs-platform-wallet/src/wallet/identity/network/profile.rs
- packages/rs-platform-wallet/src/wallet/mod.rs
✅ Files skipped from review due to trivial changes (2)
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swift
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rs-platform-wallet/src/wallet/identity/network/tokens/burn.rs
- packages/rs-platform-wallet/src/wallet/identity/network/tokens/transfer.rs
- packages/rs-platform-wallet/src/wallet/identity/network/tokens/destroy_frozen_funds.rs
…idation, dead bindings CodeRabbit + ZocoLini review pass on PR #3548. Bug-class fixes only; the broader architectural notes (umbrella header migration, FFI error system overhaul) are deferred to follow-up PRs. Safe Int -> UInt16 / Int -> UInt32 narrowing (Swift traps on out-of-range): - All 12 token action views: replace UInt16(token.position) with guard let position = UInt16(exactly: token.position), surface a submitError on failure. - Group-capable forms (Burn, Mint, Freeze, Unfreeze, DestroyFrozen, Pause, Resume, SetPrice, UpdateMaxSupply): same pattern for the inferredGroupPosition narrowing inside the .propose(position:) construction. - Group:<n> string parsing helpers: validate the parsed Int fits in UInt16 inside the helper; out-of-range values are dropped. - PendingGroupActionsView.position(of:token:): same UInt16 bounds check. - CoSignProposalView.requiredPower: replace UInt32(i) with UInt32(exactly:) and 0-fallback for negative/over-range values. JSON decoder hardening (TokenGroupActionQueries.swift): - .setPrice case: throw DecodingError.dataCorruptedError on a present-but-non-numeric pricePerToken instead of silently mapping to nil (which would have meant "disable direct purchase"). - tokenPendingGroupActions: validate startAtActionId is exactly 32 bytes before forwarding to FFI; throw PlatformWalletError.serialization on length mismatch. UI lifecycle: - RecipientPickerView: cancel the in-flight DPNS resolve task and reset dpnsState on mode change AND on .onDisappear; in the resolution completion path, re-validate that mode and dpnsInput still match the values that triggered the resolve before mutating selection. Stops late-arriving resolutions from overwriting fresh selections in a different mode. - CoSignProposalView: canCoSign and coSignDisabledReason now treat .directPurchase the same as .other — disabled with "this proposal type isn't supported by the co-sign UI yet" — since the dispatch path always throws for .directPurchase. - TokenBurnActionView: submit() guard re-checks amount <= balanceValue (race between render and tap). - TokenPurchaseActionView: gate Buy on a priceKnown flag (currently hardcoded false until PersistentToken exposes a directPurchasePrice field). Drop the UInt64.max sentinel that turned Buy into a known- failing path; surface "Buy is disabled until the configured direct- purchase price is available locally" instead. - TokenActionPermissionsView: drop changeDirectPurchasePricingRules from the .changeDistribution fallback chain — that rule now backs the dedicated .setDirectPurchasePrice row, so leaving it in caused price-only tokens to surface a stray Change Distribution row. Rust: - helpers.rs: improved key-selection error to include the searched triplet (AUTHENTICATION + MASTER/HIGH + ECDSA_SECP256K1) and the identity id; added doc-comment noting the canonical-policy choice and that the FFI's signing_key_id is advisory. Dead Swift @_silgen_name bindings deleted (Wave 11 deleted the Rust functions; the Swift forward declarations were left behind, dead but hazardous if anyone added a caller): - platform_wallet_create_dashpay_profile - platform_wallet_update_dashpay_profile - platform_wallet_send_contact_request - platform_wallet_accept_contact_request - platform_wallet_register_dpns_name (no-signer variant only; _with_signer kept) Validation: - cargo fmt --all clean - cargo check -p platform-wallet -p platform-wallet-ffi clean - build_ios.sh --target sim with -warnings-as-errors: BUILD SUCCEEDED - xcodebuild SwiftExampleApp iPhone 17 sim: BUILD SUCCEEDED - 14/14 token FFI symbols still exported Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ZocoLini
approved these changes
Apr 27, 2026
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
3 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.
Summary
Replaces "Action coming soon" placeholders in
TokenActionPermissionsViewwith end-to-end submission paths for all 12 token actions, plus group-action propose/co-sign discovery. Stacks on top of #3539; retarget tov3.1-devonce that merges.Per
packages/swift-sdk/CLAUDE.md, every token flow routes throughrs-platform-walletviars-platform-wallet-ffi(8 MB worker stack);rs-sdk-ffiis not called from Swift.What ships
rs-platform-wallet — 12 new
*_external_signerwrappers sharingfetch_data_contract+resolve_signing_keyhelpers, plus a newgroup_queriesmodule exposingpending_group_actions_external/group_action_signers_external.rs-platform-wallet-ffi/src/tokens/ — 12 C-ABI submission entry points (
platform_wallet_token_{transfer,burn,mint,claim,freeze,unfreeze,destroy_frozen_funds,pause,resume,set_price,purchase,update_config}) + 2 query entry points returning JSON.group_info.rsdecodes the sharedGroupStateTransitionInfoStatustuple.Sources/SwiftDashSDK —
TokenActions.swiftaddsGroupActionMode(none/propose/signExisting),TokenConfigChange, and async wrappers onManagedPlatformWallet.TokenGroupActionQueries.swiftadds Codable models + the discovery methods.SwiftExampleApp — 12 form views under
Views/TokenActions/following theCreateIdentityViewpattern.RecipientPickerView(own identities + base58 paste + DPNS lookup) is reused by Transfer/Mint/Freeze/Unfreeze/DestroyFrozen.PendingGroupActionsViewlists open proposals with signature tally;CoSignProposalViewreconstructs the proposal's params and submits withGroupActionMode.signExisting.A new
TokenActionKind.setDirectPurchasePricerow separates Set Price (shipped) from.changeDistribution(deferred rich-schedule editor). LegacyTokenActionsView/TokenActionDetailView/TokenActionenum are removed.Deferred (placeholder shows "deferred to a future release")
TokenPricingSchedule::SetPricesand config-update variants other than MaxSupplyOpen TODOs noted inline
signing_key_idis advisory; Rust resolves the canonical AUTH/MASTER-or-HIGH/ECDSA_SECP256K1 keyPersistentTokenBalancerefresh after submit (relies on next sync)PersistentTokendoesn't yet expose current direct-purchase price or circulating supply, so some forms can't fully pre-validate client-sideTest plan
TokensViewrow tap navigates toTokenActionPermissionsView(no dead-end)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Cleanup