fix(swift-sdk): contracts integration polish#3604
Merged
QuantumExplorer merged 19 commits intov3.1-devfrom May 6, 2026
Merged
Conversation
…Transfer/Burn The Transfer/Burn screens read balance from `identity.tokenBalances` (SwiftData), which isn't reliably populated by the time the user opens the screen — so users with non-zero balances were seeing "Balance 0" and "Amount exceeds your balance." Forward the value the parent already fetched via `sdk.getIdentityTokenBalances` and use it as the source of truth, falling back to the persisted row when not provided. The Amount field also treated user input as raw u64 units while the balance is shown decimal-scaled, so typing "5" against a 4.44 balance would silently pass as 5 raw units (0.00000005 tokens). Parse input as a decimal in display units, scale to raw units by `token.decimals`, and accept both "." and "," as separators. Switch the keyboard to `.decimalPad` and add the missing balance guard to Transfer's submit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y, refresh balance on Permissions screen Mint, Direct Purchase, and Update Max Supply forms were treating user input as raw u64 token units while their displays show decimal-scaled values, so typing "1" against an 8-decimal token would mint/buy 1 raw unit (0.00000001 of a token) and barely move the recipient's balance. Parse input as a decimal in display units and scale by `token.decimals` before calling the FFI; switch the keyboards to `.decimalPad`. Format Mint's "Max supply" and Update Max Supply's "Current max supply" in display units too, so the on-screen comparison reads in the same unit the user is about to type. Extract the parse/format helpers to a shared `TokenAmountFormatting` utility now that five action views need them. Also drive balance refresh from the Permissions screen so paths that don't pre-fetch — e.g. opening Transfer/Burn from the Contracts tab — populate the balance themselves via `sdk.getIdentityTokenBalances` when the resolved identity changes, instead of falling back to the stale `PersistentTokenBalance` row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…missions screen `PersistentToken.id` is a SwiftData uniqueness key (`contractId + position.bigEndian`), not the on-chain canonical token id the SDK's balance lookup expects. Passing that key meant the result map was always empty, so `fetchedBalance` defaulted to 0 on paths that didn't pre-seed `initialBalance` (e.g. opening Transfer from the Contracts tab). Derive the canonical id via `sdk.calculateTokenId(contractId:position:)` — same shape `IdentityDetailView` already uses — before calling `getIdentityTokenBalances`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… identity, drop dead matcher arm - `parseTokenAmount`: `Decimal(string:)` accepts a valid prefix and drops the rest, so `"5abc"` parsed as 5 and a pasted `"1,234.56"` (after `,`→`.` normalization → `"1.234.56"`) parsed as 1.234, letting the user submit a materially different raw amount than they typed. Validate the normalized string against a strict `\d+(\.\d+)?` (or `\.\d+`) grammar before scaling. - `TokenActionPermissionsView.refreshTokenBalance`: clear `fetchedBalance` to nil on entry and capture the identity at start; only commit the result if `resolvedIdentity` still matches. Without this, switching identity via the picker would leave the previous identity's balance forwarded into Transfer/Burn until the new fetch completed (or forever if it failed silently). - Transfer/Burn `matchingBalance`: drop the always-false `tb.tokenId == token.id.toBase58String()` arm — `tb.tokenId` is the canonical on-chain token id while `token.id` is a SwiftData uniqueness key (`contractId + position`). Keep the relationship-based comparison, which works when SwiftData has linked the rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… nil The Mint screen's "Mint to self" toggle was passing `recipient: nil` to the FFI, which Rust interprets as "use the contract's configured `newTokensDestinationIdentity`" — not "send to the caller." On a token where `mintingAllowChoosingDestination` is true and the configured destination is null (a perfectly valid mintable contract), every mint-to-self attempt failed with "Destination identity for minting not set." Branch on the contract's `mintingAllowChoosingDestination`: when the contract permits a caller-supplied destination, pass our own `identity.identityId` explicitly so "to self" actually means "to the caller." When the contract forbids overriding the destination, keep the existing `nil` passthrough so fixed-destination contracts continue to mint to their configured `newTokensDestinationIdentity`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, and feature badges
Four small correctness bugs in the contracts surface where the UI told
the user "you can do this" when the contract or the app actually
couldn't. Grouped because they share a single theme — the action
resolver, the mint screen, and the token-detail badges should all
agree on what's actually possible.
- Mint resolver: deny when authorized but no destination is reachable.
Previously, a token with mint authorization but
`mintingAllowChoosingDestination == false` and
`newTokensDestinationIdentity == nil` showed Mint as allowed; the
user could fill in an amount and submit, only to be rejected by
Platform with "Destination identity for minting not set."
- DirectPurchase resolver: deny with a clear "price not available
locally yet" reason. `TokenPurchaseActionView.priceKnown` is
hard-coded `false` until the configured purchase price lands on
`PersistentToken`, so the Buy button was permanently disabled and
routing the user to that screen via an "allowed" row was dishonest.
- "Mint to self" toggle label: relabel to "Use configured destination"
when `mintingAllowChoosingDestination == false`. The toggle is
force-on/disabled in that case, but tokens go to the contract's
configured destination, not the caller — the literal "Mint to self"
label contradicted the surrounding info banner.
- "Can be X" badges in TokenDetailsView: route through a new
`ChangeControlRules.hasAuthorizedTakers` helper that checks both
rule existence AND that `authorizedToMakeChange != "NoOne"`. A rule
shipped-but-locked-off (the canonical pattern for safety-by-default
contract templates) now correctly shows as not available, matching
what the actions screen already says ("Freeze: no one is
authorized") for the same token.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tch Tasks
Two related async-state safety bugs across the token-action surface,
unified under one theme: no late writes to stale view state.
- TokenActionPermissionsView.refreshTokenBalance: the previous
identity-equality guard didn't cover A → B → A. An older A-fetch
resolving after a fresher one would pass the equality check and
overwrite the newer value with stale data. Replace with a per-view
generation counter; only the latest in-flight fetch may commit. The
previously silent `catch { }` now also prints a diagnostic
breadcrumb so failed fetches are observable from the console
instead of disappearing.
- All 12 token action views + CoSignProposalView: each `submit()` /
`dispatch()` Task wrote back to `@State` from a late `MainActor.run`
with no cancellation or generation guard. If the user popped the
view and re-pushed it before the broadcast resolved, the late
callback wrote on a stale view instance, producing intermittent
"Modifying state during view update" warnings and misdirected
`dismiss()` calls in nested-modal flows. Same per-view generation
pattern across every site: increment in submit, capture locally,
guard every late write.
14 files, near-identical 4-line additions. Pattern is inlined rather
than abstracted because each submit's parameter capture, validation,
and error wording diverges enough that a shared helper would obscure
more than it deduplicates.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Make the identity hex string on the Identity Details screen copyable. Replaces the plain Label with an HStack rendering the same content plus an explicit clipboard-icon Button on the right that writes the hex to UIPasteboard and fires a success haptic. The hex Text also gets `.textSelection(.enabled)` so users who long-press get the standard iOS text selection menu as a secondary path. `.contextMenu` was tried first but doesn't fire reliably on a Label inside a List row — the row's own gesture eats the long-press. An explicit visible button is also more discoverable than long-press. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… map before FFI
`RegisterContractSourceView` flattens the chain-shape
`{ "<position>": { ... } }` groups map into an array with an injected
`id` for the form. The Rust V1 contract format expects a
`BTreeMap<GroupContractPosition, Group>` though, so the assembler
rejected the array with "expected a map, got sequence" and no
group-bearing contract could be registered. Restore the map shape
in `executeDataContractCreate` right before the FFI call.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… Details The hex row is the raw byte form most users see in logs and errors, but most platform tooling (contract JSON group members, FFI args, dashmate) consumes base58 — so it deserves equal visibility rather than being hidden behind a long-press or menu. Two rows, two one-tap copy buttons, both text-selectable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ing destination Co-signing a group token mint failed with "Choosing token mint recipient not allowed" because the chain stores `TokenEvent::Mint` with the resolved `newTokensDestinationIdentity` baked in (non-optional), the FFI surfaces that resolved id back as a non-optional recipient, and the co-sign view replays it as `issued_to_identity_id = Some(...)` — which the rs-drive transformer rejects when the contract has `minting_allow_choosing_destination = false`. Normalize at the wallet-helper chokepoint: in `token_mint_with_signer`, read the token configuration off the data contract and drop any non-`None` `recipient_id` when the rule forbids it. The chain then resolves the destination from `newTokensDestinationIdentity` as it already does for the proposer's `None`-shaped submission, so the co-sign replay round-trips cleanly. Chain consensus rules and the FFI signature are untouched. Module doc-comment also corrected — `recipient_id == None` defers to the contract's destination identity, not the caller's. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`PersistentToken.isPaused` was only set once, from `startAsPaused`,
during initial contract parse. Tokens paused in another session, on
another device, or before this fix shipped left the local flag stuck
at false — so `TokenActionPermissionsView` gated the Resume row on
`!token.isPaused` and showed it locked with the misleading "Token is
not paused" subtitle while the chain happily rejected any subsequent
operation with "is paused".
Two changes, layered:
- `TokenActionPermissionsView` now fetches `getTokenStatuses` on
appearance and reconciles the local flag against the chain truth.
Cheap one-shot query (`{ paused: Bool }` per token id), idempotent
write (only saves when the value actually changed), and silent
fallback on failure.
- `TokenPauseActionView` and `TokenResumeActionView` flip the flag
locally in their single-signer success branches so the next view
paint reflects the action without waiting for the next chain
refetch. Group propose-mode submissions don't flip — those just
store a pending action; the threshold-crossing co-signer path is
out of scope here (see the broader audit notes).
The action-side mutations are immediate-feedback shortcuts; the
view-side refetch is the chain-truth backstop that fixes the
"already paused before this fix" case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…axSupply Same family as the pause/resume staleness fix (`238f5e4b2`). The single-signer success branch on TokenUpdateMaxSupplyActionView used to just dismiss, leaving `PersistentToken.maxSupply` stuck at the value parsed from the contract at registration time. Re-opening the form showed "Current max supply: <old value>" until the contract was manually re-fetched. Materialize the submitted target (string-encoded raw u64, or nil for "no cap") into a local before the Task closure, then write it back onto `token.maxSupply` in the success branch. Save the model context explicitly to survive a main-context that may not autosave. Only applies for single-signer (.none) submissions — propose-mode just stores a pending group action and the config doesn't change until a co-signer crosses the threshold. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tion views `git diff --check origin/v3.1-dev...HEAD` flagged both files at EOF. One byte each — strip the extra newline so the file ends with the struct's closing brace plus a single trailing newline (POSIX shape). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds Rust preflight recipient normalization for token mints and a broad Swift SDK update: precise token amount parsing/formatting, balance-aware wiring, generation-based guards to prevent stale async UI updates, UI tweaks, removal of some views, and transition data normalization. ChangesToken Mint Recipient Normalization
Swift SDK: Balance-awareness, Race Guards, Amount Formatting, and UI Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
Collaborator
Review GateCommit:
|
![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: 3
🧹 Nitpick comments (5)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenPauseActionView.swift (1)
180-183: ⚡ Quick winAdd explicit error handling for local persistence in token action success paths.
The optimistic state updates (
token.isPaused = true/falseand similar) are followed bytry? modelContext.save(), which silently swallows save failures. If persistence fails, the UI appears fresh until the next reload, then regresses to the server state. Replacetry?with explicit error handling to capture and surface these failures. This pattern appears in at leastTokenPauseActionView,TokenResumeActionView, andTokenUpdateMaxSupplyActionView— consider applying the same fix across all token action views.🤖 Prompt for 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. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenPauseActionView.swift` around lines 180 - 183, The optimistic local state updates in TokenPauseActionView (and similarly in TokenResumeActionView and TokenUpdateMaxSupplyActionView) use try? modelContext.save(), which silences persistence failures; replace this with a do-catch around try modelContext.save() to explicitly handle errors: log the error (or surface it via the view's error state/alert) and rollback the optimistic change (e.g., reset token.isPaused to its prior value) if saving fails, ensuring the UI and local model remain consistent with persistence results.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/TokenAmountFormatting.swift (1)
80-81: 💤 Low valueNit:
divisor == 0branch is unreachable.
pow(Decimal(10), dec)withdec >= 0(clamped on line 78) is never 0, so the ternary'sdivisor == 0 ? rawDecimal : …arm is dead. Not harmful, but the simpler form removes a misleading hint that scaling can fall back to the raw value.♻️ Simplify
- let scaled = divisor == 0 ? rawDecimal : (rawDecimal / divisor) + let scaled = rawDecimal / divisor🤖 Prompt for 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. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/TokenAmountFormatting.swift` around lines 80 - 81, In TokenAmountFormatting.swift, simplify the scaling logic by removing the unreachable ternary branch that checks divisor == 0: since dec is clamped >= 0, divisor = pow(Decimal(10), dec) can never be zero, so replace the ternary using divisor and scaled with a direct division (rawDecimal / divisor) and remove the dead-path comment/condition; keep the pow(Decimal(10), dec) calculation and variable names (divisor, scaled) so the change is minimal and easy to locate.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenUpdateMaxSupplyActionView.swift (1)
250-252: 💤 Low valueSilent
try?on the local cap write.If
modelContext.save()fails, the user still sees a successful dismissal even though the locally-cachedtoken.maxSupplymay be inconsistent with the broadcast value until the next refetch. Surfacing the error (or at least logging it) would make this self-diagnosable when it actually breaks. Minor — the next sync will reconcile, so the worst case is a transient stale display.🛠 Surface the save failure
if case .none = groupAction { token.maxSupply = newMaxSupplyValue - try? modelContext.save() + do { + try modelContext.save() + } catch { + print("⚠️ Failed to persist updated maxSupply: \(error)") + } }🤖 Prompt for 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. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenUpdateMaxSupplyActionView.swift` around lines 250 - 252, The silent try? on modelContext.save() in TokenUpdateMaxSupplyActionView hides save failures after setting token.maxSupply; replace the silent call with proper error handling: call try modelContext.save() in a do/catch, log the caught error (e.g., via Logger or print), and surface failure to the user (e.g., show an alert or prevent dismissal) instead of always dismissing; ensure the code paths referencing token.maxSupply and the dismissal only proceed on successful save in the do block and include the error details in the log/catch block for diagnosability.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift (1)
887-915: 💤 Low valueConsider adding a generation guard symmetric with
refreshTokenBalance.
refreshTokenStatuswritestoken.isPausedand persists viamodelContext.save()from aMainActor.runblock that may resolve after the view has navigated away or the parent re-rendered. Practically this is fine today (the write is idempotent and only flips when the value differs), but if a future change makes this trigger more often (e.g. on identity switch or a pull-to-refresh) you'd benefit from the same monotonic-counter pattern you already added for the balance path so a slow status fetch can't overwrite a fresher one. Not a blocker.🤖 Prompt for 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. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift` around lines 887 - 915, Add a generation-guard to refreshTokenStatus mirroring the pattern used in refreshTokenBalance: introduce a view-scoped integer generation counter (e.g., tokenStatusGeneration), increment it immediately before starting the async work, capture the currentGeneration in the async flow, and after awaiting sdk calls check that currentGeneration == tokenStatusGeneration; if it differs return early so stale responses can't overwrite a newer state. Only perform the MainActor.run write to token.isPaused and call modelContext.save() when the generation matches. Use the existing function names refreshTokenStatus, token.isPaused, and modelContext.save() to locate where to add the counter and checks.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swift (1)
117-168: 💤 Low valueSide-by-side hex/base58 rows with explicit copy buttons read well.
.buttonStyle(.borderless)correctly isolates the tap targets inside the List row, and theaccessibilityLabels differentiate the two copy buttons for VoiceOver users. Optional follow-up: the two rows are structurally identical apart from icon, value, and a11y label — extracting a smallIdentityIDCopyRow(icon:value:accessibilityLabel:)helper would remove the duplication and make future tweaks (e.g. uniform haptic style, sizing) a one-spot change. Not a blocker.♻️ Optional helper extraction
+private struct IdentityIDCopyRow: View { + let icon: String + let value: String + let accessibilityLabel: String + + var body: some View { + HStack(alignment: .top, spacing: 6) { + Image(systemName: icon) + .foregroundColor(.secondary) + .font(.caption) + Text(value) + .font(.caption) + .foregroundColor(.secondary) + .textSelection(.enabled) + Spacer(minLength: 4) + Button { + UIPasteboard.general.string = value + UINotificationFeedbackGenerator().notificationOccurred(.success) + } label: { + Image(systemName: "doc.on.doc") + .font(.caption) + .foregroundColor(.blue) + } + .buttonStyle(.borderless) + .accessibilityLabel(accessibilityLabel) + } + } +}Then both rows collapse to:
IdentityIDCopyRow( icon: "number", value: identity.identityIdString, accessibilityLabel: "Copy identity ID (hex)" ) IdentityIDCopyRow( icon: "textformat.abc", value: identity.identityIdBase58, accessibilityLabel: "Copy identity ID (base58)" )🤖 Prompt for 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. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swift` around lines 117 - 168, Extract the duplicated HStack copy-row into a small reusable SwiftUI View struct named IdentityIDCopyRow(icon:value:accessibilityLabel:) and replace the two HStack blocks that use identity.identityIdString and identity.identityIdBase58 with two calls to IdentityIDCopyRow(...). The new IdentityIDCopyRow should render the same Image, Text (with .font(.caption), .foregroundColor(.secondary), .textSelection(.enabled)), Spacer, and Button behavior (UIPasteboard.general.string assignment, UINotificationFeedbackGenerator().notificationOccurred(.success), .buttonStyle(.borderless), and the passed accessibilityLabel) so visual and a11y behavior is identical to the original HStack implementations.
🤖 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/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenDetailsView.swift`:
- Around line 174-176: The label "Started as paused" is inaccurate because
token.isPaused now reflects live chain state; update the TokenFeatureRow that
uses token.isPaused (the instance where TokenFeatureRow(label: "Started as
paused", isEnabled: token.isPaused)) to use a label such as "Currently paused"
(or another present-tense phrase) so the UI reflects the current paused state
rather than an immutable launch flag.
- Around line 404-413: The supply section currently treats any non-nil
maxSupplyChangeRules as changeable; update the logic in TokenDetailsView's
supplySection to use ChangeControlRules.hasAuthorizedTakers instead: when
computing "Max Supply Changeable" (and any related UI state), replace the
nil-check on maxSupplyChangeRules with a check that
maxSupplyChangeRules?.hasAuthorizedTakers == true (or guard-let then inspect
hasAuthorizedTakers) so rules with authorizedToMakeChange == "NoOne" are treated
as not changeable; ensure you reference ChangeControlRules.hasAuthorizedTakers
and maxSupplyChangeRules in your change so the display matches the rest of the
screen.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionDetailView.swift`:
- Around line 1667-1676: The re-keying loop silently drops entries when an entry
has a missing/unsupported id or when two entries share the same position id;
update the logic in TransitionDetailView where parsed, entry, rawId, byPosition
and groups are handled to fail loudly instead of continuing: validate that rawId
exists and is either Int or String (do not quietly continue), and detect
duplicate keys before assigning into byPosition (throw or assert/fatalError with
a clear message referencing the duplicate key and the offending entries), so
callers (and RegisterContractSourceView expectations) get an immediate,
descriptive failure rather than silent overwrite or drop.
---
Nitpick comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/TokenAmountFormatting.swift`:
- Around line 80-81: In TokenAmountFormatting.swift, simplify the scaling logic
by removing the unreachable ternary branch that checks divisor == 0: since dec
is clamped >= 0, divisor = pow(Decimal(10), dec) can never be zero, so replace
the ternary using divisor and scaled with a direct division (rawDecimal /
divisor) and remove the dead-path comment/condition; keep the pow(Decimal(10),
dec) calculation and variable names (divisor, scaled) so the change is minimal
and easy to locate.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swift`:
- Around line 117-168: Extract the duplicated HStack copy-row into a small
reusable SwiftUI View struct named
IdentityIDCopyRow(icon:value:accessibilityLabel:) and replace the two HStack
blocks that use identity.identityIdString and identity.identityIdBase58 with two
calls to IdentityIDCopyRow(...). The new IdentityIDCopyRow should render the
same Image, Text (with .font(.caption), .foregroundColor(.secondary),
.textSelection(.enabled)), Spacer, and Button behavior
(UIPasteboard.general.string assignment,
UINotificationFeedbackGenerator().notificationOccurred(.success),
.buttonStyle(.borderless), and the passed accessibilityLabel) so visual and a11y
behavior is identical to the original HStack implementations.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift`:
- Around line 887-915: Add a generation-guard to refreshTokenStatus mirroring
the pattern used in refreshTokenBalance: introduce a view-scoped integer
generation counter (e.g., tokenStatusGeneration), increment it immediately
before starting the async work, capture the currentGeneration in the async flow,
and after awaiting sdk calls check that currentGeneration ==
tokenStatusGeneration; if it differs return early so stale responses can't
overwrite a newer state. Only perform the MainActor.run write to token.isPaused
and call modelContext.save() when the generation matches. Use the existing
function names refreshTokenStatus, token.isPaused, and modelContext.save() to
locate where to add the counter and checks.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenPauseActionView.swift`:
- Around line 180-183: The optimistic local state updates in
TokenPauseActionView (and similarly in TokenResumeActionView and
TokenUpdateMaxSupplyActionView) use try? modelContext.save(), which silences
persistence failures; replace this with a do-catch around try
modelContext.save() to explicitly handle errors: log the error (or surface it
via the view's error state/alert) and rollback the optimistic change (e.g.,
reset token.isPaused to its prior value) if saving fails, ensuring the UI and
local model remain consistent with persistence results.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenUpdateMaxSupplyActionView.swift`:
- Around line 250-252: The silent try? on modelContext.save() in
TokenUpdateMaxSupplyActionView hides save failures after setting
token.maxSupply; replace the silent call with proper error handling: call try
modelContext.save() in a do/catch, log the caught error (e.g., via Logger or
print), and surface failure to the user (e.g., show an alert or prevent
dismissal) instead of always dismissing; ensure the code paths referencing
token.maxSupply and the dismissal only proceed on successful save in the do
block and include the error details in the log/catch block for diagnosability.
🪄 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: 8b863aaa-7e7a-469e-9fd0-a46cd56eceda
📒 Files selected for processing (22)
packages/rs-platform-wallet/src/wallet/identity/network/tokens/mint.rspackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/TokenAmountFormatting.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContractsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DynamicDocumentFormView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/OptionsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/CoSignProposalView.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/TransitionDetailView.swift
💤 Files with no reviewable changes (3)
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/OptionsView.swift
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DynamicDocumentFormView.swift
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContractsView.swift
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: "9c4d28187fb971185b85a1ca9a80b117e9b744b65af35c61b36b679dff01b7f4"
)Xcode manual integration:
|
Eight small follow-ups across the contracts integration touch points: - TokenDetailsView: "Started as paused" → "Currently paused" — the flag now reflects live chain state (post-isPaused-reconciliation), not the immutable startAsPaused config. - TokenDetailsView: "Max Supply Changeable" now gates on `hasAuthorizedTakers` so contracts whose maxSupplyChangeRules pin `authorizedToMakeChange: NoOne` (e.g. bartek05053-style permanently locked) read "No" instead of "Yes". - TokenAmountFormatting: drop unreachable `divisor == 0` ternary — pow(Decimal(10), dec) with dec >= 0 is always >= 1. - IdentityDetailView: extract IdentityIDCopyRow helper for the hex + base58 rows so future tweaks (haptics, sizing) are one-spot. - TokenActionPermissionsView: add tokenStatusGeneration mirror of balanceFetchGeneration so a future pull-to-refresh / post-action poll trigger can't reintroduce the A→B→A overwrite bug. - TokenPause / TokenResume / TokenUpdateMaxSupply / refreshTokenStatus: replace `try? modelContext.save()` with do-catch + named-view print breadcrumb. The chain action has already succeeded, so a SwiftData hiccup isn't user-facing — keep the silent UI dismiss but make the failure self-diagnosable from console logs. - TransitionDetailView: group re-keying now throws SDKError.serializationError on missing-id, non-Int/non-String id, or duplicate position. The upstream form guarantees these can't happen today, but silent corruption surfacing later as a confusing chain rejection is bad enough that defensive validation is cheap insurance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
thepastaclaw
reviewed
May 6, 2026
Collaborator
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
PR is a focused UX/correctness pass over the Swift example app's contracts/tokens flows plus a narrow Rust-side normalizer in the mint helper. The Rust change is correct; the Swift changes are mostly defensive. A few residual issues remain: the format/parse pair for token amounts is asymmetric under grouping separators, the threshold-crossing co-sign path doesn't reconcile local token state, refreshTokenStatus silently relaxes the local pause flag on shape mismatch, and the contract-update parser still has the array/map shape that the create-side just fixed (currently masked by an unconditional notImplemented).
Reviewed commit: a9aae73
🟡 4 suggestion(s) | 💬 2 nitpick(s)
2 additional findings
🟡 suggestion: Threshold-crossing co-signs never reconcile local token state
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/CoSignProposalView.swift (lines 354-383)
CoSignProposalView treats every successful co-sign the same: stop the spinner, show "Signed". It never updates token.isPaused / token.maxSupply and never triggers a status refresh, even when this signature is the one that crosses the group threshold and lands the change on chain. Compare to the single-signer paths — TokenPauseActionView, TokenResumeActionView, and TokenUpdateMaxSupplyActionView all flip the local model in their groupAction == .none branch. After a 2-of-2 pause the user lands back on a screen showing the old paused state until something else triggers refreshTokenStatus (maxSupply has no equivalent refresh path at all, so it stays stale even after the user returns to TokenActionPermissionsView). The dispatch knows the proposal type — switch on proposal.params and apply the same local model write the single-signer path does when the co-sign succeeds and the MintResult/etc. indicates the action actually executed (not just buffered).
🟡 suggestion: executeDataContractUpdate's `newGroups` parser still has the array shape the create-side just fixed
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionDetailView.swift (lines 1854-1862)
The create-side path (1654–1693) was just fixed to re-key the form's groups array back into a position-keyed object before crossing the FFI, because the Rust V1 contract format expects BTreeMap<GroupContractPosition, Group> and rejects an array with expected a map, got sequence. The update-side here still parses newGroups as [[String: Any]]? and would forward exactly the same array-shaped JSON. It does not currently misbehave only because the function throws SDKError.notImplemented at line 1884 before any FFI call. The moment the platform-wallet updateDataContract sibling lands and replaces that throw without revisiting this parser, the same boundary failure will recur. Mirror the create-side re-keying here so the create/update FFI input shapes stay symmetric.
🤖 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/Utils/TokenAmountFormatting.swift`:
- [SUGGESTION] lines 77-91: formatTokenAmount + parseTokenAmount are not round-trip safe under grouping separators
`formatTokenAmount` sets `usesGroupingSeparator = true`, so a US-locale balance renders as e.g. `"1,234.56789012"`. If a user copies any portion of that display value into one of the action views' Amount field, `parseTokenAmount` first does `"," -> "."` normalization before the strict grammar check. The strict grammar correctly rejects `"1,234.56"` (becomes `"1.234.56"`), but `"1,234"` (becomes `"1.234"`) is well-formed and parses as `1.234` — i.e. ~1000× smaller than what the user copied. The fix is asymmetric: either disable grouping in `formatTokenAmount` (`formatter.usesGroupingSeparator = false`) so the displayed value is also a valid input, or strip grouping separators before normalization in `parseTokenAmount`.
In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/CoSignProposalView.swift`:
- [SUGGESTION] lines 354-383: Threshold-crossing co-signs never reconcile local token state
`CoSignProposalView` treats every successful co-sign the same: stop the spinner, show "Signed". It never updates `token.isPaused` / `token.maxSupply` and never triggers a status refresh, even when this signature is the one that crosses the group threshold and lands the change on chain. Compare to the single-signer paths — `TokenPauseActionView`, `TokenResumeActionView`, and `TokenUpdateMaxSupplyActionView` all flip the local model in their `groupAction == .none` branch. After a 2-of-2 pause the user lands back on a screen showing the old paused state until something else triggers `refreshTokenStatus` (`maxSupply` has no equivalent refresh path at all, so it stays stale even after the user returns to TokenActionPermissionsView). The dispatch knows the proposal type — switch on `proposal.params` and apply the same local model write the single-signer path does when the co-sign succeeds and the `MintResult`/etc. indicates the action actually executed (not just buffered).
In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift`:
- [SUGGESTION] lines 895-933: refreshTokenStatus treats a missing/malformed chain entry as 'not paused' and persists it
The fall-through in `refreshTokenStatus` defaults `chainPaused = false` whenever the response map doesn't yield a `[String: Any]` entry with a `paused: Bool` field, then commits that to `PersistentToken.isPaused` and saves. Every token gets a `TokenStatus` written at creation in rs-drive (including for `start_as_paused = true`), so the missing-entry case in practice means a transient parsing edge case, an FFI shape change, or a partial response — in any of those, defaulting to `false` and persisting it silently relaxes the local Pause/Resume gate and overwrites the value seeded from `startAsPaused` at registration. The `try` block catches transport errors and preserves the local flag; only commit `chainPaused` when the entry actually parsed (treat shape-mismatch the same as the catch-block).
In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionDetailView.swift`:
- [SUGGESTION] lines 1854-1862: executeDataContractUpdate's `newGroups` parser still has the array shape the create-side just fixed
The create-side path (1654–1693) was just fixed to re-key the form's groups array back into a position-keyed object before crossing the FFI, because the Rust V1 contract format expects `BTreeMap<GroupContractPosition, Group>` and rejects an array with `expected a map, got sequence`. The update-side here still parses `newGroups` as `[[String: Any]]?` and would forward exactly the same array-shaped JSON. It does not currently misbehave only because the function throws `SDKError.notImplemented` at line 1884 before any FFI call. The moment the platform-wallet `updateDataContract` sibling lands and replaces that throw without revisiting this parser, the same boundary failure will recur. Mirror the create-side re-keying here so the create/update FFI input shapes stay symmetric.
Comment on lines
+77
to
+91
| func formatTokenAmount(_ raw: UInt64, decimals: Int) -> String { | ||
| let dec = max(0, decimals) | ||
| let rawDecimal = Decimal(raw) | ||
| // `dec` is clamped to `>= 0` above, so `pow(Decimal(10), dec)` is | ||
| // always >= 1 — the previous `divisor == 0 ? rawDecimal : …` guard | ||
| // was unreachable and only obscured the scaling intent. | ||
| let divisor = pow(Decimal(10), dec) | ||
| let scaled = rawDecimal / divisor | ||
| let formatter = NumberFormatter() | ||
| formatter.numberStyle = .decimal | ||
| formatter.maximumFractionDigits = dec | ||
| formatter.minimumFractionDigits = 0 | ||
| formatter.usesGroupingSeparator = true | ||
| return formatter.string(from: scaled as NSNumber) ?? "\(raw)" | ||
| } |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: formatTokenAmount + parseTokenAmount are not round-trip safe under grouping separators
formatTokenAmount sets usesGroupingSeparator = true, so a US-locale balance renders as e.g. "1,234.56789012". If a user copies any portion of that display value into one of the action views' Amount field, parseTokenAmount first does "," -> "." normalization before the strict grammar check. The strict grammar correctly rejects "1,234.56" (becomes "1.234.56"), but "1,234" (becomes "1.234") is well-formed and parses as 1.234 — i.e. ~1000× smaller than what the user copied. The fix is asymmetric: either disable grouping in formatTokenAmount (formatter.usesGroupingSeparator = false) so the displayed value is also a valid input, or strip grouping separators before normalization in parseTokenAmount.
source: ['claude']
🤖 Fix this 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/Utils/TokenAmountFormatting.swift`:
- [SUGGESTION] lines 77-91: formatTokenAmount + parseTokenAmount are not round-trip safe under grouping separators
`formatTokenAmount` sets `usesGroupingSeparator = true`, so a US-locale balance renders as e.g. `"1,234.56789012"`. If a user copies any portion of that display value into one of the action views' Amount field, `parseTokenAmount` first does `"," -> "."` normalization before the strict grammar check. The strict grammar correctly rejects `"1,234.56"` (becomes `"1.234.56"`), but `"1,234"` (becomes `"1.234"`) is well-formed and parses as `1.234` — i.e. ~1000× smaller than what the user copied. The fix is asymmetric: either disable grouping in `formatTokenAmount` (`formatter.usesGroupingSeparator = false`) so the displayed value is also a valid input, or strip grouping separators before normalization in `parseTokenAmount`.
Comment on lines
+895
to
933
| private func refreshTokenStatus() async { | ||
| tokenStatusGeneration &+= 1 | ||
| let gen = tokenStatusGeneration | ||
| guard let sdk = appState.sdk else { return } | ||
| guard let position = UInt16(exactly: token.position) else { return } | ||
| let contractIdString = token.contractId.toBase58String() | ||
| do { | ||
| let canonicalTokenId = try sdk.calculateTokenId( | ||
| contractId: contractIdString, | ||
| position: position | ||
| ) | ||
| let statuses = try await sdk.getTokenStatuses(tokenIds: [canonicalTokenId]) | ||
| // Shape: `{ "<base58_token_id>": { "paused": Bool } | null }`. | ||
| // A `null` status means the chain has no pause record for | ||
| // this token yet (never paused) — treat as not-paused. | ||
| let chainPaused: Bool | ||
| if let entry = statuses[canonicalTokenId] as? [String: Any], | ||
| let paused = entry["paused"] as? Bool { | ||
| chainPaused = paused | ||
| } else { | ||
| chainPaused = false | ||
| } | ||
| await MainActor.run { | ||
| // Late-arriving status responses are dropped if a fresher | ||
| // refresh has already started — same shape as the balance | ||
| // path's generation guard. | ||
| guard self.tokenStatusGeneration == gen else { return } | ||
| guard token.isPaused != chainPaused else { return } | ||
| token.isPaused = chainPaused | ||
| do { | ||
| try modelContext.save() | ||
| } catch { | ||
| print("⚠️ refreshTokenStatus: failed to persist isPaused flip for \(contractIdString):\(token.position): \(error)") | ||
| } | ||
| } | ||
| } catch { | ||
| print("⚠️ refreshTokenStatus failed for token at \(contractIdString):\(token.position): \(error)") | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: refreshTokenStatus treats a missing/malformed chain entry as 'not paused' and persists it
The fall-through in refreshTokenStatus defaults chainPaused = false whenever the response map doesn't yield a [String: Any] entry with a paused: Bool field, then commits that to PersistentToken.isPaused and saves. Every token gets a TokenStatus written at creation in rs-drive (including for start_as_paused = true), so the missing-entry case in practice means a transient parsing edge case, an FFI shape change, or a partial response — in any of those, defaulting to false and persisting it silently relaxes the local Pause/Resume gate and overwrites the value seeded from startAsPaused at registration. The try block catches transport errors and preserves the local flag; only commit chainPaused when the entry actually parsed (treat shape-mismatch the same as the catch-block).
source: ['claude']
🤖 Fix this 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/TokenActionPermissionsView.swift`:
- [SUGGESTION] lines 895-933: refreshTokenStatus treats a missing/malformed chain entry as 'not paused' and persists it
The fall-through in `refreshTokenStatus` defaults `chainPaused = false` whenever the response map doesn't yield a `[String: Any]` entry with a `paused: Bool` field, then commits that to `PersistentToken.isPaused` and saves. Every token gets a `TokenStatus` written at creation in rs-drive (including for `start_as_paused = true`), so the missing-entry case in practice means a transient parsing edge case, an FFI shape change, or a partial response — in any of those, defaulting to `false` and persisting it silently relaxes the local Pause/Resume gate and overwrites the value seeded from `startAsPaused` at registration. The `try` block catches transport errors and preserves the local flag; only commit `chainPaused` when the entry actually parsed (treat shape-mismatch the same as the catch-block).
Comment on lines
+42
to
+53
| guard let entered = Decimal(string: normalized), entered >= 0 else { | ||
| return nil | ||
| } | ||
|
|
||
| let dec = max(0, decimals) | ||
| let multiplier = pow(Decimal(10), dec) | ||
| var scaled = entered * multiplier | ||
| var rounded = Decimal() | ||
| NSDecimalRound(&rounded, &scaled, 0, .down) | ||
|
|
||
| if rounded < 0 || rounded > Decimal(UInt64.max) { return nil } | ||
| return (rounded as NSDecimalNumber).uint64Value |
Collaborator
There was a problem hiding this comment.
💬 Nitpick: entered >= 0 and rounded < 0 checks are unreachable after isWellFormedDecimal
isWellFormedDecimal only accepts ASCII digits and a single ., so Decimal(string:) cannot return a negative value. The entered >= 0 and rounded < 0 guards are dead code. Harmless, but the comments next to them imply runtime defensiveness that isn't actually exercised — trim them or replace with a // grammar guarantees non-negative comment.
source: ['claude']
Comment on lines
+59
to
+68
| let recipient_id = if !data_contract | ||
| .expected_token_configuration(token_position) | ||
| .map_err(dash_sdk::Error::Protocol)? | ||
| .distribution_rules() | ||
| .minting_allow_choosing_destination() | ||
| { | ||
| None | ||
| } else { | ||
| recipient_id | ||
| }; |
Collaborator
There was a problem hiding this comment.
💬 Nitpick: Silent recipient override deserves a tracing breadcrumb
The fix is correct for the co-sign replay path. But the helper is generic — token_mint_with_external_signer and any other in-tree caller now silently overrides a caller-supplied recipient_id whenever the contract sets minting_allow_choosing_destination = false. Before this change, such a call would propagate a chain rejection (ChoosingTokenMintRecipientNotAllowed); after, it succeeds and mints to newTokensDestinationIdentity instead, with no error. The doc comment documents this, but a tracing::debug! (or warn! when the dropped value differs from the configured destination) would make the override observable in logs without changing semantics. Not blocking.
source: ['claude']
Four follow-ups across the same files the previous chore commit touched. Two fixes for real bugs (one shipped in this PR), two inline cleanups while we're already in those files. - TokenAmountFormatting.formatTokenAmount: turn off `usesGroupingSeparator`. With it on, a US-locale balance of `1234.56` rendered as `"1,234.56"` while parseTokenAmount happily accepts `"1,234"` (after `,→.` normalization → `"1.234"`) as `1.234` — a thousand times smaller. Disabling grouping makes every formatter output a valid input for the parser. Decimal separator still honors locale. - TokenAmountFormatting.parseTokenAmount: drop the unreachable `entered >= 0` and `rounded < 0` guards. `isWellFormedDecimal` rejects anything outside `\d` and `.`, so the inputs to `Decimal(string:)` and `NSDecimalRound` are non-negative by construction. Keep the `> UInt64.max` overflow check next door — that one is genuinely reachable. - TokenActionPermissionsView.refreshTokenStatus: when the chain response yields no `paused: Bool` for the token, bail with a log breadcrumb instead of defaulting to `false` and persisting it. rs-drive writes a TokenStatus row at token creation, so a missing/malformed entry indicates a transient parse edge or FFI shape change — preserving the local flag matches the surrounding catch-block behavior on transport errors. - mint.token_mint_with_signer: emit `tracing::debug!` when the helper actually overrides a caller-supplied recipient_id to None (no log when it was already None). Pre-fix behavior was a chain rejection; post-fix is a silent successful mint to `newTokensDestinationIdentity`. The breadcrumb makes that observable in logs without changing semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenDetailsView.swift (1)
416-426: ⚡ Quick winVerify raw value before promoting helper, and confirm the refactor is actually needed.
The raw value for
AuthorizedActionTakers.noOneis correctly defined as"NoOne", and the comparison inhasAuthorizedTakerscorrectly matches it.TokenActionEvaluatordoes use the same comparison pattern (line 174 ofTokenActionPermissionsView.swift), confirming the shared logic.However,
hasAuthorizedTakersis currently used only withinTokenDetailsView.swift(lines 155–182) and is not duplicated elsewhere. Consider promoting it to a shared location only when other views actually need to reuse it; for now, thefileprivatescope is sufficient. If you do promote it, a good home would be alongsideChangeControlRulesinTokenTypes.swiftor a smallTokenRulesextension file.🤖 Prompt for 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. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenDetailsView.swift` around lines 416 - 426, The current fileprivate extension on ChangeControlRules defines hasAuthorizedTakers by comparing authorizedToMakeChange to AuthorizedActionTakers.noOne.rawValue; confirm that the raw value "NoOne" is correct (it is) and do not promote this helper out of fileprivate unless it is actually reused elsewhere—leave it as-is in TokenDetailsView if it’s only used there; if you decide to promote it, move the extension next to ChangeControlRules (e.g., in TokenTypes.swift or a new TokenRules extension file) and update callers (like TokenActionEvaluator and TokenDetailsView) to reference the shared hasAuthorizedTakers to avoid duplication.
🤖 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/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swift`:
- Around line 843-846: The code clears fetchedBalance unconditionally causing
the seeded initialBalance to vanish during refreshes; change this so
fetchedBalance is only cleared when the user actually switches identities:
introduce/ use a persisted identity marker (e.g., lastFetchedIdentity or
previousIdentity) and compare it to the current identity inside the async
refresh in TokenActionPermissionsView; only run await MainActor.run {
self.fetchedBalance = nil } when the identity changed (lastFetchedIdentity !=
currentIdentity) and then update lastFetchedIdentity = currentIdentity,
otherwise leave fetchedBalance (and the provided initialBalance) intact until
the fetch completes and then replace fetchedBalance with the fetched value.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionDetailView.swift`:
- Around line 1662-1693: The parser for the groupsJson currently assumes parsed
JSON is an array ([[String: Any]]) and throws "Invalid groups JSON" for the
canonical position-keyed map shape; update the parsing in TransitionDetailView
to accept either an array or a dictionary: after JSONSerialization(with: data)
assign to a let value (e.g. parsedValue) and branch—if it's an array ([[String:
Any]]) run the existing loop building byPosition (using
entry/removeValue(forKey: "id"), key logic and duplicate check), else if it's a
dictionary ([String: Any]) validate/convert its values to the expected entry
shape and assign directly to groups; keep the same error cases (missing id, bad
id type, duplicates) when normalizing array entries and ensure the final
variable groups is set to the normalized map (byPosition or the parsed
dictionary).
---
Nitpick comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenDetailsView.swift`:
- Around line 416-426: The current fileprivate extension on ChangeControlRules
defines hasAuthorizedTakers by comparing authorizedToMakeChange to
AuthorizedActionTakers.noOne.rawValue; confirm that the raw value "NoOne" is
correct (it is) and do not promote this helper out of fileprivate unless it is
actually reused elsewhere—leave it as-is in TokenDetailsView if it’s only used
there; if you decide to promote it, move the extension next to
ChangeControlRules (e.g., in TokenTypes.swift or a new TokenRules extension
file) and update callers (like TokenActionEvaluator and TokenDetailsView) to
reference the shared hasAuthorizedTakers to avoid duplication.
🪄 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: b5c6bc4f-553b-4f94-b241-412ae6e906de
📒 Files selected for processing (9)
packages/rs-platform-wallet/src/wallet/identity/network/tokens/mint.rspackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/TokenAmountFormatting.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenPauseActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenResumeActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActions/TokenUpdateMaxSupplyActionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenDetailsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionDetailView.swift
`TokenActionPermissionsView.refreshTokenBalance` used to clear `fetchedBalance` to nil unconditionally at the top of every refresh to guard against an A→B identity switch leaking A's balance into B's Transfer/Burn pickers. But that wiped the parent-supplied `initialBalance` (seeded from `IdentityDetailView`) before the async fetch returned, so the first paint showed a transient zero, and a failed/slow fetch lost the seed permanently — which the per-action views then surfaced as a possibly-empty fallback. Track which identity the current `fetchedBalance` belongs to via a new `lastFetchedBalanceIdentityId` marker, seed it from the parent's identity when an `initialBalance` is supplied, and only clear when the active identity has actually changed. The pre-existing `balanceFetchGeneration` counter still guards the A→B→A overwrite race that motivated the original clear, so the safety property is preserved without the collateral damage. Stamps the marker on every successful fetch so subsequent refreshes on the same identity skip the clear path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The groups re-keying loop in TransitionDetailView.executeDataContractCreate
(landed in `608a2fec7`, hardened in `a9aae7377`) hard-cast to
`[[String: Any]]` and threw "Invalid groups JSON" on anything else.
That's correct for `RegisterContractSourceView`'s flattened-array
input, but it broke any caller passing the canonical chain-shape
`{ "<position>": { ... } }` map — including pasting normal contract
JSON into the manual transition builder. Regression I introduced
in `608a2fec7`.
Switch on the parsed shape: accept the canonical map as a
passthrough, keep the existing array branch (and its missing-id /
unsupported-id-type / duplicate-key validation from `a9aae7377`)
for the form-flattened input. Side benefit: `try?` becomes `try`
on the `JSONSerialization` call, so genuine parse errors surface
with their underlying message instead of getting collapsed into
the same generic "Invalid groups JSON" as a shape mismatch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer
approved these changes
May 6, 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.
Issue being fixed or feature implemented
Iterative polish to the contracts integration in the Swift example app, surfaced through end-to-end testing on a 2-of-2 group token contract. The flow "register contract → propose mint → co-sign → pause/resume → transfer" hit several distinct bugs that this branch resolves, plus a handful of UI/UX gaps in identity display and contract registration.
What was done?
Grouped by theme.
Group co-sign mint (the longest-tail bug)
Token state freshness
Identity display
Token amount handling
Cleanup
How Has This Been Tested?
Known follow-up items (not in this PR, called out for the reviewer):
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Removed