Skip to content

fix(swift-example-app): unhide Create Identity submit button and auto-dismiss sheet on success#3694

Open
llbartekll wants to merge 4 commits into
v3.1-devfrom
fix/swift-example-create-identity-amount-prefill
Open

fix(swift-example-app): unhide Create Identity submit button and auto-dismiss sheet on success#3694
llbartekll wants to merge 4 commits into
v3.1-devfrom
fix/swift-example-create-identity-amount-prefill

Conversation

@llbartekll
Copy link
Copy Markdown
Contributor

@llbartekll llbartekll commented May 20, 2026

Issue being fixed or feature implemented

On the SwiftExampleApp Create Identity sheet with Platform Payment funding:

  1. The submit button silently disappeared when the prefilled amount rounded above the available balance (e.g. 0.99984186 DASH"0.999842" via %g, which exceeds the balance and trips the credits <= balance gate in canSubmit).
  2. After a successful registration the sheet stayed open and re-rendered the registration-index picker with a red Index #N is already taken error against the slot the user had just used.

What was done?

  • Prefill the amount field by truncating to 4 decimal places, so the value is guaranteed <= balance.
  • Call dismiss() immediately after a successful Platform Payment registration. The new identity surfaces on the Identities tab via its @Query.
  • Drop dead createdIdentityId @State (written by both submit paths but never read).

Core-funded path is intentionally untouched — its multi-step progress + Done flow is meaningful for the asset-lock build/broadcast.

How Has This Been Tested?

  • xcodebuild build passes on iPhone 17 simulator.
  • Manually verified the original repro on testnet: button now appears and the sheet auto-dismisses on success.

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved identity creation flow responsiveness by dismissing the sheet immediately upon successful payment submission and identity completion.
    • Enhanced DASH amount prefill formatting with consistent 4-decimal precision display, automatically trimming unnecessary trailing zeros for improved clarity.

Review Change Stack

llbartekll and others added 2 commits May 20, 2026 11:46
… when prefill rounds above balance

The amount field's default-string formatter used `String(format: "%g",
dash)`, whose 6-sig-fig rounding can push the prefilled value above
the actual account balance (e.g. 0.99984186 DASH → "0.999842" →
99,984,200,000 credits vs. 99,984,186,000 available). `canSubmit`
requires `credits <= balance`, so the form silently hid the submit
section and the user saw no way to register the identity.

Format the prefill via integer arithmetic on the smallest-units value
so the string is an exact decimal representation of the underlying
credits/duffs, guaranteeing a clean round-trip through
`parsedAmountCredits` / `parsedAmountDuffs`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… Payment success

The Platform Payment success branch persisted the new identity but
left the form mounted. SwiftUI then re-evaluated the registration
index picker against `usedIdentityIndices(for:)`, which now contained
the just-used slot, so the user saw a red "Index #N is already taken"
error sitting on top of a successful registration with no clear next
step.

Dismiss the sheet immediately after the save completes. The Identities
tab's `@Query` picks up the new row automatically, so there's nothing
left for the form to show.

Also drop the dead `createdIdentityId` state. Both submit paths
assigned it on success but nothing ever read it; the doc comment
claimed it drove a success banner / auto-dismiss that didn't exist.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added this to the v3.1.0 milestone May 20, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1aa7db57-36c6-48fa-9b2f-1bc13658bb12

📥 Commits

Reviewing files that changed from the base of the PR and between d66b84a and 50ef472.

📒 Files selected for processing (1)
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift

📝 Walkthrough

Walkthrough

Removes local createdIdentityId state; success/failure render via the coordinator terminal phase. Platform payment completion clears isCreating and dismisses the sheet. Default DASH prefill now truncates to 4 decimals, formats with fixed 4-decimal output, then trims trailing zeros.

Changes

CreateIdentityView Success Flow and Amount Formatting

Layer / File(s) Summary
Success flow simplification
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift
Removed createdIdentityId @State. Platform Payment completion now clears isCreating and calls dismiss() after persisting and bookkeeping instead of setting createdIdentityId. Controller observer .completed no longer assigns createdIdentityId; it updates only submitError and isCreating while retaining activeController.
Default DASH amount formatting
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift
Added prefillDecimalPlaces = 4. defaultAmountString(for:) computes a shared Double DASH value, truncates to 4 decimal places (round-down), formats with fixed 4-decimal precision, then trims trailing zeros and an optional trailing decimal point, replacing prior %g-based formatting.

🎯 3 (Moderate) | ⏱️ ~20 minutes

"I hopped through code with tiny feet,
Removed a state and made the flow complete,
Four decimals dance, then trailing zeros fall,
A tidy sheet closes—controller says all. 🐇✨"

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the two main fixes: unhiding the submit button and auto-dismissing the sheet on success, which directly correspond to the primary changes described in the PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/swift-example-create-identity-amount-prefill

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

✅ 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: "3fdfc06e47d7d4e3e46273e1850604318fced7f8414419f3e15d99865d3084a5"
)

Xcode manual integration:

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

Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

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

Code Review

Small, focused UX fix to the Create Identity sheet. The exact-decimal formatter is a real improvement and the post-success dismiss() is correct. However, the claimed 'exact round-trip' invariant is not actually achieved end-to-end because both parsers still go through Double; this works for the specific repro balance but drifts again for large balances (>~90,071 DASH worth of credits). A drop-in fix using the existing parseTokenAmount helper exists.

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

3 additional finding(s)

suggestion: Exact prefill formatter still feeds a Double-based parser; round-trip breaks at large balances

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1313)

defaultAmountString(for:) / amountString(smallestUnits:decimalsPerWhole:) now emit an exact decimal string, but parsedAmountCredits and parsedAmountDuffs still parse that string via Double(trimmed) and rescale by creditsPerDash/duffsPerDash. That only fixes the specific 0.99984186 DASH repro from the PR description, not the broader exact-round-trip invariant the change is meant to establish. Concretely: a Platform Payment balance of 9_007_199_254_739_994 credits formats as "90071.99254739994", but Double("90071.99254739994") * 1e11 rounds back to 9_007_199_254_739_995, so canSubmit will reject the full-balance prefill (the original bug) and submitPlatformPayment would try to spend one credit more than the account holds. The app already ships parseTokenAmount(_:decimals:) in Utils/TokenAmountFormatting.swift, which uses Decimal + strict grammar specifically for this class of bug — this view should reuse it instead of Double.

    private var parsedAmountCredits: UInt64? {
        parseTokenAmount(amountDash, decimals: 11)
    }

    /// Parse the amount text into Core duffs (1e8 per DASH). Used for
    /// the Core-funded identity path.
    private var parsedAmountDuffs: UInt64? {
        parseTokenAmount(amountDash, decimals: 8)
    }
suggestion: No unit test exercises the prefill formatter the fix depends on

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1288)

amountString(smallestUnits:decimalsPerWhole:) is now load-bearing for canSubmit and has several non-obvious branches (whole-only, padded fractional with leading zeros, trailing-zero trim, empty-fraction collapse). The PR verifies it once on testnet at 0.99984186 DASH. A small XCTest covering integer balances, fractional balances with leading zeros (e.g. 50_000 duffs → "0.0005"), trailing-zero trim, and the original repro value would prevent regressions in this helper without a manual testnet step. As-is, the only safety net is hand-testing.

nitpick: decimalPlaces computation silently assumes decimalsPerWhole is a power of 10

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1297)

let decimalPlaces = String(decimalsPerWhole).count - 1 derives the fractional-digit count from the string length of the divisor, which is only correct when decimalsPerWhole is a power of 10. Both current call sites (creditsPerDash = 10^11, duffsPerDash = 10^8) satisfy that, so this is not a live bug. But the helper signature is generic; a future caller passing e.g. 1_000_000_001 would silently produce a wrong fractional length with no compile- or runtime-time signal. A one-line precondition comment, or decimalPlaces = Int(log10(Double(decimalsPerWhole))) with a precondition that the divisor is a power of ten, would make the assumption explicit.

🤖 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.

- [SUGGESTION] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1313-1333: Exact prefill formatter still feeds a Double-based parser; round-trip breaks at large balances
  `defaultAmountString(for:)` / `amountString(smallestUnits:decimalsPerWhole:)` now emit an exact decimal string, but `parsedAmountCredits` and `parsedAmountDuffs` still parse that string via `Double(trimmed)` and rescale by `creditsPerDash`/`duffsPerDash`. That only fixes the specific 0.99984186 DASH repro from the PR description, not the broader exact-round-trip invariant the change is meant to establish. Concretely: a Platform Payment balance of 9_007_199_254_739_994 credits formats as `"90071.99254739994"`, but `Double("90071.99254739994") * 1e11` rounds back to 9_007_199_254_739_995, so `canSubmit` will reject the full-balance prefill (the original bug) and `submitPlatformPayment` would try to spend one credit more than the account holds. The app already ships `parseTokenAmount(_:decimals:)` in `Utils/TokenAmountFormatting.swift`, which uses `Decimal` + strict grammar specifically for this class of bug — this view should reuse it instead of `Double`.
- [SUGGESTION] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1288-1309: No unit test exercises the prefill formatter the fix depends on
  `amountString(smallestUnits:decimalsPerWhole:)` is now load-bearing for `canSubmit` and has several non-obvious branches (whole-only, padded fractional with leading zeros, trailing-zero trim, empty-fraction collapse). The PR verifies it once on testnet at 0.99984186 DASH. A small XCTest covering integer balances, fractional balances with leading zeros (e.g. 50_000 duffs → `"0.0005"`), trailing-zero trim, and the original repro value would prevent regressions in this helper without a manual testnet step. As-is, the only safety net is hand-testing.
- [NITPICK] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1297-1297: decimalPlaces computation silently assumes decimalsPerWhole is a power of 10
  `let decimalPlaces = String(decimalsPerWhole).count - 1` derives the fractional-digit count from the string length of the divisor, which is only correct when `decimalsPerWhole` is a power of 10. Both current call sites (`creditsPerDash` = 10^11, `duffsPerDash` = 10^8) satisfy that, so this is not a live bug. But the helper signature is generic; a future caller passing e.g. `1_000_000_001` would silently produce a wrong fractional length with no compile- or runtime-time signal. A one-line precondition comment, or `decimalPlaces = Int(log10(Double(decimalsPerWhole)))` with a `precondition` that the divisor is a power of ten, would make the assumption explicit.

Inline posting hit GitHub HTTP 422, so I posted the same verified findings as a top-level review body.

…cimal truncate

Replace the integer-arithmetic `amountString` helper with a four-line
truncate-to-4-decimals on the `Double` itself. Same effect for any
realistic balance — the truncate guarantees the prefilled value is
`<= balance` — at a fraction of the surface area.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@llbartekll llbartekll marked this pull request as draft May 20, 2026 16:03
@llbartekll llbartekll changed the title fix(swift-example): unhide Create Identity submit button and auto-dismiss sheet on success fix(swift-example-app): unhide Create Identity submit button and auto-dismiss sheet on success May 20, 2026
@llbartekll llbartekll marked this pull request as ready for review May 20, 2026 16:07
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

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

Code Review

Latest delta (truncate prefill to 4 decimals via Double, dismiss-on-success, remove createdIdentityId state) is clean and correctly fixes the original hide-submit repro. Two carried-forward concerns: (a) the amount parsers still use Double end-to-end before handing UInt64 funding units to Rust, which can drift one smallest-unit at extreme typed values (~90k+ DASH credits); (b) the prefill logic still has no XCTest coverage. One new latest-delta edge: positive balances strictly below 0.0001 DASH (or <10_000 duffs on Core) now prefill as the literal string "0", which the parsers reject and re-creates the disappearing-submit-button failure mode for that small range. No blockers.

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

3 additional finding(s)

suggestion: Sub-0.0001 DASH balances prefill as "0" and re-introduce the disappearing-submit-button failure mode

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1281)

The new prefill computes (dash * 10_000).rounded(.down) / 10_000, formats with %.4f, then strips trailing zeros and an optional trailing dot. For any positive balance strictly less than 0.0001 DASH the truncated value is 0.0, the format produces "0.0000", trimming yields "0." and then "0". parsedAmountCredits / parsedAmountDuffs parse "0" to Double 0 and reject it via guard dash > 0, so canSubmit stays false and the submit button is hidden — the exact failure mode the PR is trying to eliminate, just shifted to a smaller balance range. Reproduces for a Platform Payment account with balance in (0, 10_000_000) credits, or any Core funding account where available < 10_000 duffs (the min(available, defaultCoreFundingDuffs) collapses to available when available < 250_000). The if balance == 0 { return "" } early-out only protects the zero case, not small-nonzero. A if truncated == 0 guard that falls back to the full balance (or formats at higher precision and lets parseTokenAmount round-trip exactly) would close this.

suggestion: Amount parsers still go through `Double` before marshalling raw UInt64 funding units across the FFI

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1290)

Both parsedAmountCredits and parsedAmountDuffs still decode amountDash with Double and rescale to credits/duffs. The prefill-side truncation in this PR keeps the default value safe, but user-typed (or pasted) amounts at large balances still drift by one smallest unit because Double's 53-bit mantissa cannot exactly represent every integer above 2^53. For credits, that threshold is reached at ~90,071.99 DASH: "90071.99254739994" parses to 9_007_199_254_739_995 credits instead of 9_007_199_254_739_994, so canSubmit can reject a legitimate full-balance entry, and a successful submit can ask Rust to spend one credit more than the user typed. These integers cross the Swift→Rust boundary verbatim — submitPlatformPayment passes parsedAmountCredits into ManagedPlatformWallet.registerIdentityFromAddresses (IdentityAddressInput.creditsIdentityFundingInputFFI.credits), and the Core path hands parsedAmountDuffs directly to registerIdentityWithFunding as amountDuffs: UInt64. The app already ships parseTokenAmount(_:decimals:) in Utils/TokenAmountFormatting.swift, which uses Decimal with strict-grammar validation and truncation-rounding specifically to avoid this class of bug. Reusing it from this view also picks up locale-comma handling and rejects trailing junk like "5abc" that Double(_:) parses inconsistently.

    private var parsedAmountCredits: UInt64? {
        parseTokenAmount(amountDash, decimals: 11)
    }

    /// Parse the amount text into Core duffs (1e8 per DASH). Used for
    /// the Core-funded identity path.
    private var parsedAmountDuffs: UInt64? {
        parseTokenAmount(amountDash, decimals: 8)
    }
nitpick: Prefill/truncation logic has no XCTest coverage

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1258)

defaultAmountString(for:) is now load-bearing for whether the submit button appears, and this push rewrote it again (Double truncate to 4 decimals, format, strip trailing zeros). There is no matching XCTest in SwiftExampleAppTests exercising the key cases this prefill depends on: a Platform Payment balance near the original repro, balances that should collapse to an integer after trimming, sub-0.0001 DASH balances (see the related finding above), and Core defaults that must stay above the minimum funding floor. Regressions in this path will only be caught by manual simulator/testnet testing.

🤖 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.

- [SUGGESTION] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1281-1285: Sub-0.0001 DASH balances prefill as "0" and re-introduce the disappearing-submit-button failure mode
  The new prefill computes `(dash * 10_000).rounded(.down) / 10_000`, formats with `%.4f`, then strips trailing zeros and an optional trailing dot. For any positive balance strictly less than 0.0001 DASH the truncated value is 0.0, the format produces `"0.0000"`, trimming yields `"0."` and then `"0"`. `parsedAmountCredits` / `parsedAmountDuffs` parse "0" to Double 0 and reject it via `guard dash > 0`, so `canSubmit` stays false and the submit button is hidden — the exact failure mode the PR is trying to eliminate, just shifted to a smaller balance range. Reproduces for a Platform Payment account with balance in (0, 10_000_000) credits, or any Core funding account where `available < 10_000` duffs (the `min(available, defaultCoreFundingDuffs)` collapses to `available` when available < 250_000). The `if balance == 0 { return "" }` early-out only protects the zero case, not small-nonzero. A `if truncated == 0` guard that falls back to the full balance (or formats at higher precision and lets `parseTokenAmount` round-trip exactly) would close this.
- [SUGGESTION] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1290-1311: Amount parsers still go through `Double` before marshalling raw UInt64 funding units across the FFI
  Both `parsedAmountCredits` and `parsedAmountDuffs` still decode `amountDash` with `Double` and rescale to credits/duffs. The prefill-side truncation in this PR keeps the default value safe, but user-typed (or pasted) amounts at large balances still drift by one smallest unit because `Double`'s 53-bit mantissa cannot exactly represent every integer above 2^53. For credits, that threshold is reached at ~90,071.99 DASH: `"90071.99254739994"` parses to `9_007_199_254_739_995` credits instead of `9_007_199_254_739_994`, so `canSubmit` can reject a legitimate full-balance entry, and a successful submit can ask Rust to spend one credit more than the user typed. These integers cross the Swift→Rust boundary verbatim — `submitPlatformPayment` passes `parsedAmountCredits` into `ManagedPlatformWallet.registerIdentityFromAddresses` (`IdentityAddressInput.credits` → `IdentityFundingInputFFI.credits`), and the Core path hands `parsedAmountDuffs` directly to `registerIdentityWithFunding` as `amountDuffs: UInt64`. The app already ships `parseTokenAmount(_:decimals:)` in `Utils/TokenAmountFormatting.swift`, which uses `Decimal` with strict-grammar validation and truncation-rounding specifically to avoid this class of bug. Reusing it from this view also picks up locale-comma handling and rejects trailing junk like `"5abc"` that `Double(_:)` parses inconsistently.
- [NITPICK] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1258-1285: Prefill/truncation logic has no XCTest coverage
  `defaultAmountString(for:)` is now load-bearing for whether the submit button appears, and this push rewrote it again (Double truncate to 4 decimals, format, strip trailing zeros). There is no matching XCTest in `SwiftExampleAppTests` exercising the key cases this prefill depends on: a Platform Payment balance near the original repro, balances that should collapse to an integer after trimming, sub-0.0001 DASH balances (see the related finding above), and Core defaults that must stay above the minimum funding floor. Regressions in this path will only be caught by manual simulator/testnet testing.

Inline posting hit GitHub HTTP 422, so I posted the same verified findings as a top-level review body.

Comment thread packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift Outdated
@shumkov shumkov moved this to In review / testing in Platform team May 21, 2026
…constant

Replaces the inline `10_000` literals with a named
`prefillDecimalPlaces = 4` constant, addressing review feedback that
the magic number's intent wasn't obvious at the call site.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@llbartekll llbartekll requested a review from shumkov May 21, 2026 07:55
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

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

Code Review

Verified against SHA 50ef472: the latest commit only renamed magic numbers into prefillDecimalPlaces, so all three prior findings still apply unchanged. No new latest-delta defects beyond the carried-forward set: (1) sub-0.0001 DASH balances still prefill to literal "0" and re-trigger the disappearing-submit-button failure mode at a smaller balance band; (2) parsedAmountCredits/parsedAmountDuffs still route through Double before sending UInt64 funding units across the FFI, allowing one-smallest-unit drift; (3) the load-bearing prefill/parse helpers have no XCTest coverage (grep of SwiftExampleAppTests confirms zero hits).

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

3 additional finding(s)

suggestion: Positive balances below 0.0001 DASH still prefill as "0" and hide the submit button

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1287)

defaultAmountString(for:) truncates the selected balance to prefillDecimalPlaces (currently 4), formats with %.4f, then strips trailing zeros. For any positive Platform Payment balance below 10_000_000 credits, or any Core/CoinJoin default below 10_000 duffs (which happens when available < 10_000 since min(available, defaultCoreFundingDuffs) collapses), truncated becomes 0.0, the formatter yields "0.0000", and the trim loop collapses that to "0". Both parsedAmountCredits (line 1297) and parsedAmountDuffs (line 1310) then reject "0" via the dash > 0 guard and return nil, so canSubmit returns false at 677-683 and the submit button is hidden — the exact failure mode this PR set out to fix, shifted to a smaller balance band. The balance == 0 early-out at line 1277 only protects the exact-zero case. A if truncated == 0 guard that either returns "" (consistent with the empty-balance branch, so the field is treated as unset) or formats at higher precision so the parsers round-trip would close this.

suggestion: Amount parsers route through `Double` before marshalling UInt64 funding units across the FFI

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1297)

parsedAmountCredits and parsedAmountDuffs still parse amountDash with Double(_:) and then rescale to credits/duffs via floating-point multiplication. Double's 53-bit mantissa cannot exactly represent every integer above 2^53, so user-typed (or pasted) amounts at large balances drift by one smallest unit. For credits the threshold is reached around ~90,071.99 DASH: "90071.99254739994" parses to 9_007_199_254_739_995 credits instead of 9_007_199_254_739_994, which can cause canSubmit to reject a legitimate full-balance entry, or — worse — let a submit through that asks Rust to spend one more credit than the user typed. Those integers cross the boundary verbatim: parsedAmountCredits feeds registerIdentityFromAddresses via IdentityAddressInput.credits, and parsedAmountDuffs feeds registerIdentityWithFunding as amountDuffs: UInt64. The app already ships parseTokenAmount(_:decimals:) in Utils/TokenAmountFormatting.swift, which uses Decimal with strict-grammar validation and round-down truncation specifically to avoid this class of bug (and rejects trailing junk like "5abc" that Double(_:) accepts inconsistently). Reusing it from this view also picks up locale-comma handling.

    private var parsedAmountCredits: UInt64? {
        parseTokenAmount(amountDash, decimals: 11)
    }

    /// Parse the amount text into Core duffs (1e8 per DASH). Used for
    /// the Core-funded identity path.
    private var parsedAmountDuffs: UInt64? {
        parseTokenAmount(amountDash, decimals: 8)
    }
nitpick: Prefill/truncation logic has no XCTest coverage

packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift (line 1264)

defaultAmountString(for:) now controls whether the Create Identity submit button is visible for both Platform Payment and Core funding paths, and the logic has been rewritten twice across this PR's history (most recently parameterized via prefillDecimalPlaces). A grep of SwiftExampleAppTests/ finds no test referencing defaultAmountString, parsedAmountCredits, parsedAmountDuffs, or prefillDecimalPlaces. Cases worth covering: the original near-full-balance repro (e.g. 99_984_186 credits → "9.9984"), exact integer collapse (100_000_000_000 credits → "1"), the sub-0.0001 DASH branch (related finding above), and the Core default path when available < defaultCoreFundingDuffs. Without these, regressions in this load-bearing path will only be caught by manual simulator/testnet runs.

🤖 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.

- [SUGGESTION] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1287-1292: Positive balances below 0.0001 DASH still prefill as "0" and hide the submit button
  `defaultAmountString(for:)` truncates the selected balance to `prefillDecimalPlaces` (currently 4), formats with `%.4f`, then strips trailing zeros. For any positive Platform Payment balance below `10_000_000` credits, or any Core/CoinJoin default below `10_000` duffs (which happens when `available < 10_000` since `min(available, defaultCoreFundingDuffs)` collapses), `truncated` becomes `0.0`, the formatter yields `"0.0000"`, and the trim loop collapses that to `"0"`. Both `parsedAmountCredits` (line 1297) and `parsedAmountDuffs` (line 1310) then reject `"0"` via the `dash > 0` guard and return `nil`, so `canSubmit` returns false at 677-683 and the submit button is hidden — the exact failure mode this PR set out to fix, shifted to a smaller balance band. The `balance == 0` early-out at line 1277 only protects the exact-zero case. A `if truncated == 0` guard that either returns `""` (consistent with the empty-balance branch, so the field is treated as unset) or formats at higher precision so the parsers round-trip would close this.
- [SUGGESTION] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1297-1318: Amount parsers route through `Double` before marshalling UInt64 funding units across the FFI
  `parsedAmountCredits` and `parsedAmountDuffs` still parse `amountDash` with `Double(_:)` and then rescale to credits/duffs via floating-point multiplication. `Double`'s 53-bit mantissa cannot exactly represent every integer above 2^53, so user-typed (or pasted) amounts at large balances drift by one smallest unit. For credits the threshold is reached around ~90,071.99 DASH: `"90071.99254739994"` parses to `9_007_199_254_739_995` credits instead of `9_007_199_254_739_994`, which can cause `canSubmit` to reject a legitimate full-balance entry, or — worse — let a submit through that asks Rust to spend one more credit than the user typed. Those integers cross the boundary verbatim: `parsedAmountCredits` feeds `registerIdentityFromAddresses` via `IdentityAddressInput.credits`, and `parsedAmountDuffs` feeds `registerIdentityWithFunding` as `amountDuffs: UInt64`. The app already ships `parseTokenAmount(_:decimals:)` in `Utils/TokenAmountFormatting.swift`, which uses `Decimal` with strict-grammar validation and round-down truncation specifically to avoid this class of bug (and rejects trailing junk like `"5abc"` that `Double(_:)` accepts inconsistently). Reusing it from this view also picks up locale-comma handling.
- [NITPICK] In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift`:1264-1293: Prefill/truncation logic has no XCTest coverage
  `defaultAmountString(for:)` now controls whether the Create Identity submit button is visible for both Platform Payment and Core funding paths, and the logic has been rewritten twice across this PR's history (most recently parameterized via `prefillDecimalPlaces`). A grep of `SwiftExampleAppTests/` finds no test referencing `defaultAmountString`, `parsedAmountCredits`, `parsedAmountDuffs`, or `prefillDecimalPlaces`. Cases worth covering: the original near-full-balance repro (e.g. `99_984_186` credits → `"9.9984"`), exact integer collapse (`100_000_000_000` credits → `"1"`), the sub-0.0001 DASH branch (related finding above), and the Core default path when `available < defaultCoreFundingDuffs`. Without these, regressions in this load-bearing path will only be caught by manual simulator/testnet runs.

Inline posting hit GitHub HTTP 422, so I posted the same verified findings as a top-level review body.

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

Labels

None yet

Projects

Status: In review / testing

Development

Successfully merging this pull request may close these issues.

3 participants