fix(swift-sdk): freeze failed registration step at the failure instant#3854
Conversation
The Registration Progress sheet always rendered the asset-lock pipeline labels (asset-lock tx / broadcast / InstantSend / ChainLock) even when the identity was funded from the shielded pool via a Type-20 IdentityCreateFromShieldedPool, where none of those stages exist. Plumb a FundingMode from CreateIdentityView through RegistrationCoordinator onto IdentityRegistrationController, and render a 4-step shielded list (selecting notes, building the Halo 2 proof, broadcasting the state transition, registering the identity). The Type-20 FFI call is opaque, so the active step is a display heuristic anchored to the ~30s proof duration; a terminal-at timestamp freezes the failed step so it doesn't drift while a failed row sits in Pending Registrations. The asset-lock path is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIdentityRegistrationController now records the instant of terminal transition ( ChangesTerminal-Time Anchoring for Shielded Progress
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
✅ Review complete (commit 5eec7dc) |
…3f3579 # Conflicts: # packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift # packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift # packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift # packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift
The shielded step heuristic derived the .failed step from live wall clock, so a registration failing during note selection showed the red icon on step 1 and then drifted it to step 2 once the 2s selection window lapsed — failed rows are retained until dismissed, so the drift is always visible. Record terminalAt on the terminal phase transition and anchor the .failed derivation there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift`:
- Around line 213-257: shieldedStep(elapsedTo:) currently only returns 1 or 2 so
the in-flight path never maps to steps 3 or 4; update shieldedStep(elapsedTo:)
to return 1..4 by splitting the in-flight timeline into multiple thresholds
(e.g. shieldedNoteSelectionWindow → step 1, next window → step 2 (Halo2 proof),
next window → step 3 (broadcast), else step 4 (registering identity)), using
existing Self.* window constants or adding new constants (e.g.
shieldedProofGenerationWindow, shieldedBroadcastWindow) as needed; keep
shieldedCurrentStep’s use of controller.phase/.terminalAt as-is so .inFlight and
.failed continue to call the revised shieldedStep(elapsedTo:) to determine the
correct step.
🪄 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: 14c5a609-fe11-4f60-bd31-e325c55c8391
📒 Files selected for processing (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift`:
- Around line 213-257: shieldedStep(elapsedTo:) currently only returns 1 or 2 so
the in-flight path never maps to steps 3 or 4; update shieldedStep(elapsedTo:)
to return 1..4 by splitting the in-flight timeline into multiple thresholds
(e.g. shieldedNoteSelectionWindow → step 1, next window → step 2 (Halo2 proof),
next window → step 3 (broadcast), else step 4 (registering identity)), using
existing Self.* window constants or adding new constants (e.g.
shieldedProofGenerationWindow, shieldedBroadcastWindow) as needed; keep
shieldedCurrentStep’s use of controller.phase/.terminalAt as-is so .inFlight and
.failed continue to call the revised shieldedStep(elapsedTo:) to determine the
correct step.
🪄 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: 14c5a609-fe11-4f60-bd31-e325c55c8391
📒 Files selected for processing (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift
🛑 Comments failed to post (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift (1)
213-257:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winShielded step mapping never reaches steps 3/4 before completion.
shieldedStep(elapsedTo:)only returns step 1 or 2, so the shielded flow cannot progress to “Broadcasting transition” or “Registering identity” while in-flight, despite rendering a 4-step model.Suggested fix
+ // Tune these to your existing heuristic targets for shielded flow. + private static let shieldedProofWindowEnd: TimeInterval = 36.0 + private static let shieldedBroadcastWindowEnd: TimeInterval = 40.0 + private func shieldedStep(elapsedTo anchor: Date) -> Int { guard let submittedAt = controller.lastSubmittedAt else { return 1 } let elapsed = anchor.timeIntervalSince(submittedAt) - return elapsed < Self.shieldedNoteSelectionWindow ? 1 : 2 + if elapsed < Self.shieldedNoteSelectionWindow { return 1 } + if elapsed < Self.shieldedProofWindowEnd { return 2 } + if elapsed < Self.shieldedBroadcastWindowEnd { return 3 } + return 4 }🤖 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/RegistrationProgressView.swift` around lines 213 - 257, shieldedStep(elapsedTo:) currently only returns 1 or 2 so the in-flight path never maps to steps 3 or 4; update shieldedStep(elapsedTo:) to return 1..4 by splitting the in-flight timeline into multiple thresholds (e.g. shieldedNoteSelectionWindow → step 1, next window → step 2 (Halo2 proof), next window → step 3 (broadcast), else step 4 (registering identity)), using existing Self.* window constants or adding new constants (e.g. shieldedProofGenerationWindow, shieldedBroadcastWindow) as needed; keep shieldedCurrentStep’s use of controller.phase/.terminalAt as-is so .inFlight and .failed continue to call the revised shieldedStep(elapsedTo:) to determine the correct step.
|
Re CodeRabbit's finding that |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Prior PastaClaw findings: none. Latest-delta findings (5eec7dc): none. Cumulative current-head findings: none. The two-file fix adds a terminalAt timestamp to anchor the shielded .failed step display, with correct write ordering (reset on submit, set before phase inside the same MainActor.run), a safe ?? now fallback for pre-submit failure shapes, and a clean extraction of the step heuristic into a shared helper.
|
✅ 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: "8011b89f6cbc707c13c6e571e5b84e0848c72a7a48496f817b846eb86ae5bb72"
)Xcode manual integration:
|
Resolves conflicts with #3854 (freeze failed registration step at the failure instant): the .unconfirmed and .failed transitions now also stamp terminalAt, and the shielded failed-step heuristic combines the broadcast-rejected stage attribution with the frozen terminalAt anchor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 participants
Issue being fixed or feature implemented
RegistrationProgressSection.shieldedCurrentStepderives the.failedstep from live wall-clock elapsed time sincelastSubmittedAt. Failed registrations are retained in Pending Registrations until the user dismisses them, so a registration that fails during note selection (< 2 s) first shows the red failure icon on step 1 ("Selecting shielded notes") and then silently moves it to step 2 ("Generating Halo 2 proof") once the 2-second note-selection window lapses on the wall clock — the failure marker drifts to a stage that never ran.What was done?
IdentityRegistrationControllerrecords aterminalAttimestamp when the phase reaches.completed/.failed, reset on eachsubmitso retries re-measure from the new attempt.shieldedCurrentStep's.failedbranch anchors the elapsed-time mapping atterminalAtinstead of livenow, freezing the failed icon on the step that was actually displayed when the failure happened. The.inFlightbranch still uses livenow; the shared window mapping is factored into a smallshieldedStep(elapsedTo:)helper.How Has This Been Tested?
./build_ios.sh --target simand built SwiftExampleApp —BUILD SUCCEEDED.nowderivation drifts.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit