Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Xcode project configuration, primarily around code signing for development builds.
Changes:
- Adjusts
Zhiptarget Debug code signing settings (identity/style) and clears the provisioning profile specifier. - Reorders/relocates several
PBXBuildFile,PBXFileReference, andPBXGroupentries (project file serialization churn). - Adds extra blank entries in shell script build phase
shellScriptarrays.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
2641
to
2645
| shellScript = ( | ||
| "\"${BUILD_DIR%Build/*}SourcePackages/checkouts/firebase-ios-sdk/Crashlytics/run\"", | ||
| "", | ||
| "", | ||
| ); |
There was a problem hiding this comment.
These PBXShellScriptBuildPhase shellScript arrays contain two consecutive empty-string entries, which adds unnecessary churn to the project file and makes diffs noisier without changing behavior. Consider trimming redundant empty entries so the serialized project stays stable.
Adds DocC-style /// docs to every type/function/method/property, plus inline comments on nontrivial reactive transforms (validation pulses, flatMapLatest cancellation, eagerValidLazyErrorTurnedToEmptyOnEdit rationale, persist-then-emit ordering in the coordinator). Also bundles small staged refactors that already lived on the branch: - rename populate(with:) parameter viewModel -> output - named tuple destructuring in two closures Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ImageOrText, Wallet (+ Origin), WalletEncryptionPassword (+ Mode/Error), TransactionIntent (+ retroactive Address Codable), Weak. Adds intent/why notes on the QR-payload fallback chain, the wallet-origin → password-mode mapping, and why retroactive Codable on Address lowercases on decode. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds DocC + inline why-comments across:
- AbstractController (bar-button subjects + AbstractTarget bridge)
- SceneController (lifecycle, opt-in protocol detection, bar layout
application ladder, view↔view-model binding pipeline)
- Scene typealias, ContentView, TitledScene
- NavigationBarLayoutingNavigationController (per-scene nav-bar layout
application at every transition)
- BarButtonContent + BarButton predefined library
- Left/RightBarButton(Content)Making opt-in protocols
- BackButtonHiding marker
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Protocol files (WalletUseCase, OnboardingUseCase, PincodeUseCase,
TransactionsUseCase) and most implementations were already well-documented.
This pass adds short docstrings on the previously-bare init() {} and
@injected dependencies in the wallet implementations, plus inline notes on
DefaultRestoreWalletUseCase explaining why origin tagging matters for the
later password mode resolution.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…pers
Combine:
- Publisher+Extras: doc every method (mapToVoid, filterNil, orEmpty,
flatMapLatest, withLatestFrom, ifEmpty); explain why withLatestFrom
is hand-rolled (combineLatest has wrong trigger semantics) and
annotate the inner Subscription state machine
- Publisher+Helpers: doc every combineLatest/merge factory; note the
free-function family is RxSwift-style sugar for the project's
transform(input:) bodies
- Subject+BarButtonContent: doc the BarButton ergonomic shortcut
- UIControl+Publisher: doc the Publisher/Subscription pair, weak control
capture, and demand-ignoring rationale
Foundation:
- Array (mapToVoid, += element)
- Bundle.Key (CFBundle prefix composition; CFBundleVersion vs
CFBundleShortVersionString naming inversion)
- CharacterSet (hex w/ 0x, Bech32 w/ prefix, decimal w/ locale separator)
- Dictionary.compactMapValues, Encodable.dictionaryRepresentation
- Error+CustomStringConvertible auto-derived via enum case name
- JSONEncoder convenience init
- NSAttributedString+HTML: explain UTF-16 encoding requirement and the
family-with-symbolic-traits font merging
- String inserting(string:every:), droppingLast, sizing helpers
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Top-level UIKit:
- UIButton (1x1 colour image trick for per-state backgrounds)
- UIColor (brand Hex enum + named static colours; private hex init)
- UIFont (typography tokens, Barlow weight roles, double-struck digit
sizes, FontNameExpressible default)
- UILayoutPriority (literal conformances + .medium midpoint)
- UINavigationController, UIScrollView, UITabBarItem, UIWindow, WKWebView
UIView/:
- UIView+MotionEffect (parallax: layered images, motion strengths)
- UIView+ShakeAnimation (validation-failure shake; damping rationale)
- UIView+Spacer (.medium content priorities, fixed/flex variants)
UIViews+Publishers/:
- UIControl+Publishers, UITextField+Publishers, UIView+Publishers
(binders + reactive event streams; notification vs control-event
rationale; near-bottom scroll publisher used by ToS scenes)
Views+Styleable/:
- ContentViewProvider, Mergeable (override vs yield-to merge modes)
- UIView+Border (Border value type + AnyValidation mapping)
- Styling/UILabel (canonical doc walkthrough of the Style/Apply/
Customizing/Presets/Mergeable pattern)
- Styling/UIButton, UIStackView, UIImageView, UITextView,
FloatingLabelTextField (per-file presets, plus header notes
cross-referencing UILabel for the canonical pattern)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Protocols/:
- ContentViewProvider (notes the Extensions-side duplicate),
EmptyInitializable, PullToRefreshCapable, ScrollViewOwner,
StackViewStyling, TableViewOwner
Components/:
- CheckboxWithLabel + native CheckboxView (BEMCheckBox replacement,
layer drawing, beginTracking → toggle pattern)
- GradientView + GradientLayer + 8-direction GradientPoint
- RefreshControl (UIKit default-spinner alpha hack)
- TitledValueView (two-phase init contract; UITextView vs UILabel
inset compensation)
Delegates/TextFieldDelegate (per-keystroke gating, locale separator
normalisation in defer)
SceneViews/:
- AbstractSceneView (PullToRefreshCapable opt-in, defer { setup() }
chain, refresh-control binders/publishers)
- BaseScrollableStackViewOwner (ContentViewProvider runtime cast,
edge-to-superview helpers)
- BaseTableViewOwner (TableViewSceneView/HeaderlessTableViewSceneView
aliases; generic Header+Cell pairing)
SpecificViews/:
- ButtonWithSpinner (replaceText vs nextToText modes)
- SpinnerView (4-stage stroke animation breakdown)
- InputPincodeView/* (invisible UITextField + DigitView presentation
layer; secure entry; removeDuplicates anti-loop pattern; magnifier
suppression; setDigits string parser invariants)
Subclasses/:
- FloatingLabelTextField + RightLeftView, TypeOfInput, Validation
(validation circle, line/placeholder/title colour mapping,
AnyValidation routing)
- ImageAboveLabelButton (custom views + accessibility forwarding)
- ScrollableContentSizedTextView (intrinsic-size invalidation pattern)
TableView/:
- CellConfigurable, ClassIdentifiable + Identifiable + UITableViewCell
auto-conformance, AbstractTableViewCell + TableViewCell<Model>
- FooterView, SingleCellTypeTableView (typed dataSource/delegate,
selection publisher, deselection-mode policy, SectionModel)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Most files in Audio/, Auth/, DI/, ViewModel/, and OutputFormatting/ were
already well-documented (every Container factory has a docstring; protocol/
concrete pairs across DI explain the inject-side-effects philosophy).
This pass adds richer DocC + inline comments where they were missing:
- AbstractTarget (UIKit target/action → Combine subject bridge)
- ErrorTracker (dual of ActivityIndicator for the error path; trackError
operator; asInputErrors / asInputValidationErrors funnels)
- AmountFormatter (Zesame.Unit display name rules; thousands separator
only on integer part; rationale for max-5-decimal cap)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds DocC + inline rationale across the Preferences/Keychain/UserDefaults
trio:
- AnyKeyValueStoring (the type-erased base) and KeyValueStoring (typed
layer) — explains *why* there are two protocols and how the typed
layer's default impls delegate to the string-keyed primitives.
- KeyValueStore<KeyType> — explains the type-erasure motive and the
closure-captured thunks.
- KeychainSwift conformance — clarifies the load-order priority
(Data first so Codable blobs aren't mistaken for strings) and the
accessibleWhenPasscodeSetThisDeviceOnly access option.
- KeyValueStore+KeychainKey — documents the reinstall-detection trick
that hangs off Preferences.hasRunAppBefore (Keychain survives
uninstall, UserDefaults does not — so the missing flag means we
must wipe stale Keychain state).
- PreferencesKey / KeychainKey enum cases all gain per-case docs
(verbose pincode case calls out that it's app-lock only, not crypto).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds DocC + inline rationale across the Application/Utils trinkets:
- AppAppearance + UINavigationBar default tokens — clarifies which
tokens are styling defaults vs feature flags (defaultIsTranslucent,
defaultBarStyle), what `shadow` actually does, and why `attributeText`
has both mutating and UIAppearance variants.
- Bootstrap — documents the launch-time ordering and the reasoning
behind crash-reporting being torn down vs left configured.
- ConfigurationDetection — explains the assert-side-effect trick.
- Enum+Reflection / ReflectionNameOfCaseInNestedEnum — documents the
intended use (peeling associated values off coordinator steps,
analytics labels for enum cases).
- NavigationBarLayoutOwner — explains the per-screen layout protocol
and the four-metrics appearance stamp.
- Never — calls out interfaceBuilderSucks/incorrectImplementation/abstract
intent at the call sites where they fire.
- QRCoding — documents the JSON-over-QR interop choice and the brand
color baked into the generator.
- ImageConvertible — short identity-conformance note.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds DocC + inline rationale for the navigation subsystem:
- BaseCoordinator + Coordinating protocol + extensions explain the
division of labor (coordinators own UIKit transitions, view-models
declare intent via Navigator).
- CoordinatorTransition + start(coordinator:) extension call out the
.replace vs .append distinction and the wipe-then-start ordering
in setRootViewControllerIfEmptyElsePush.
- modallyPresent/push/replaceAllScenes — each documents how it
bridges view-model NavigationStep to a coordinator handler closure.
- removeAllViewControllers / setRootViewControllerIfEmptyElsePush —
document the modal-dismiss-first ordering and the runloop-hop
completion fallback.
- DeepLink/DeepLinkGenerator/DeepLinkHandler — explain the
universal-link parsing path mapping, the lock-buffer-replay
contract, and the belt-and-suspenders filter on `navigation`.
- OpenUrl/UrlOpener — call out the test-flake reason for the
indirection (workspace round-trip in simulator).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tors
Adds DocC + inline rationale across the validator subsystem:
- InputValidator + Validation/AnyValidation — explain the typed/erased
layering and why the view layer always projects through AnyValidation.
- Publisher+Validation — documents the eager-valid/lazy-error pulse
pattern (suppress errors mid-typing, surface them on field-blur,
*always* surface tracked async errors immediately).
- ValidationRule + ValidationRuleSet + Validatable — clarifies the
Validator-package replacement and the closure-thunk evaluator design.
- Per-validator files (Address, Amount, Encryption, GasLimit, GasPrice,
Keystore, PrivateKey, SufficientFunds) — each gets:
* Top-level intent: which screen, what it gates.
* Per-Error-case docs explaining what triggers the case.
* Inline rationale on tricky bits (locale-aware decimal separator
normalization, "endsWithDecimalSeparator" mid-typing fallback,
unit-aware error message rendering, length-check short-circuit
before crypto check).
- Switches the SufficientFundsValidator's swiftlint:disable:next to
a same-line :this directive so it doesn't conflict with the doc
block immediately above the declaration.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ator OnboardingCoordinator: explains the linear gate-decision tree (toNextStep) — each guard maps to a persisted onboarding fact, so a partially-completed onboarding resumes at the first unsatisfied step on next launch. Welcome scene: documents the three-layer parallax spaceship setup, the hidden-bar layout reasoning, and the single-input start-button bridge. TermsOfService scene: calls out the dual presentation context (hidden- bar onboarding step vs translucent-bar Settings modal) and the "scroll-to-bottom unlocks accept" gate. View model docs explain the isDismissible branch that swaps an accept CTA for a "Done" bar-button. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Both scenes share the dual-presentation pattern with TermsOfService (hidden bar in onboarding, translucent bar with Done button in the Settings modal). Docs explain the merged accept/decline triggers, the checkbox-gated buttons (CrashReporting), and the selectable-text re-enable that keeps the bug-bounty hyperlink tappable (ECC warning). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…inator - ChooseWallet scene: planet parallax + dual-CTA chooser, no UI state. - EnsureThatYouAreNotBeingWatched scene: privacy gate that gates the create flow; cancel suppresses the back arrow to avoid leaking partial state. - CreateNewWalletCoordinator: linear three-step flow (privacy gate -> password -> backup hand-off), with cancel-anywhere short-circuiting to .cancel and a Just-wrapped wallet handed to the reactive backup coordinator. - CreateNewWallet controller: title + cancel bar-button glue. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- BackupWalletCoordinator: documents the dual entry points (post-create vs Settings revisit), the wallet-source override pattern (avoid a storage round-trip for fresh wallets), and the two reveal sub-flows. - DecryptKeystoreCoordinator: same wallet-source pattern, two-step password-gate then reveal. - BackupWallet hub (View/ViewModel): explains the two presentation modes, the withLatestFrom-pull-current-keystore-at-click pattern, and the checkbox-gated done CTA. - BackUpKeystore modal: pretty-printed JSON + copy-to-pasteboard. - DecryptKeystoreToRevealKeyPair (View/VM): documents the validation pipeline (eager-valid/lazy-error + tracked-error pass-through), the flatMapLatest cancellation, and the InputValidator nested struct. - BackUpRevealedKeyPair (View/VM): renders the KeyPair as hex strings, routes copy taps to pasteboard + toast. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… screen - RestoreWalletCoordinator: linear two-step flow (privacy gate -> restore screen) with cancel-anywhere short-circuit. - RestoreWallet (View/VM): documents the segmented chooser pattern with two overlaid sub-views, the prepend-current-segment trick, and the composite "wrong password" binder that flips field state, force-redirects the segment, and disables the restore button. - RestoreUsingKeystoreView + ViewModel: explains the looser password mode, owned view-model + re-exported output pattern, and the validation-border binder for UITextView. - RestoreUsingPrivateKeyView + ViewModel: explains the strict new-wallet password mode, show/hide secure-entry toggle (with accumulator scan), and the dual encryption-password validators (single-field length check vs cross-field confirmation check). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- SetPincodeCoordinator: linear two-step flow (chooser -> confirm) with skip-collapse to a single .setPincode outcome. Refuses to start if a pincode is already configured (change-pincode goes via Settings -> remove). - ChoosePincode (View/VM): N-digit input + auto-focus on viewWillAppear, done-tap gated on pincode-completeness via withLatestFrom + filterNil. - ConfirmNewPincode (View/VM): re-entry input that preserves text on mismatch, (matches && checkbox-checked) gate for the CTA, persistence before nav step. - PincodeValidator + Error: documents the settingNew flag (kept around for future copy divergence between confirm-new and unlock screens). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…screen - MainCoordinator: post-onboarding hub coordinator with three modal sub-flows (Send/Receive/Settings) plus deep-link handling that opens the Send flow with a pre-filled transaction. Skips deep-link routing if a modal is already up. - LockAppScene: privacy-cover shown over Main when the app backgrounds, to mask the snapshot in the multitasking carousel. - UnlockAppWithPincode (View/VM): pincode-match auto-unlock + automatic Face/Touch ID prompt on viewDidAppear (not willAppear, to avoid competing with the presentation animation). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Main hub (View/VM): documents the three balance-fetch triggers, the cached-balance-prepend trick (no zero flash on launch), and the pre-warming of the gas-price cache for Send. - SendCoordinator: linear four-step flow with the QR/deeplink intent merge into a single transactionIntent stream, plus topmost-scene gating to avoid stale prefills. - Step 1 PrepareTransaction (View/VM): documents the four parallel concerns (balance fetch, per-field validation, sufficient-funds check, prefill). Switches the type_body_length disable to a same-line :this directive so it doesn't conflict with the docstring. - Step 1.1 ScanQRCode (View/VM): zilliqa:// prefix stripping + Combine bridge from the third-party reader. - Step 2 ReviewTransactionBeforeSigning (View/VM): read-only summary + formatted amount/fee/total + accept-checkbox gating. - Step 3 SignTransaction (View/VM): wallet-keystore signing pipeline, errorTracker for wrong-password, flatMapLatest cancellation. - Step 4 PollTransactionStatus (View/VM): polling with linear backoff, haptic + sound on receipt, four exit paths (skip/dismiss/view/timeout). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…modals - ReceiveCoordinator: single-screen flow with system share-sheet helper using DeepLinkGenerator; popover anchored on the right-bar item for iPad. - Receive (View/VM): QR-code rendering of the wallet's bech32 address, optional amount-to-request field, copy-to-pasteboard side effect. - SettingsCoordinator: documents the 11+ row destinations (URLs, re-presented onboarding scenes, sub-coordinators, destructive wipe). Switches the cyclomatic_complexity disable to a same-line :this directive so it doesn't conflict with the docstring. - Settings (View/VM/Item/TableViewCell): grouped table view with per-section row matrix; pincode row swaps based on hasConfiguredPincode; footer shows app version + network. NavigatingCellModel typed by Destination so the Settings step enum is the navigation target. - RemovePincode (View/VM): auto-removes on first matching pincode entry. - ConfirmWalletRemoval (View/VM): "are you sure" + checkbox-gated CTA; destructive wipe deferred to coordinator so dismiss animation completes first. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- MainCoordinator + SendCoordinator: [unowned self] → [weak self] in publisher chains whose source (deeplinkedTransaction) outlives the coordinator. Previously: hard crash if a deep link emitted after the coordinator was torn down (e.g. wallet removal during pending deep link). - ReviewTransactionBeforeSigningViewModel.totalCostInZil: drop the `try!` and return Optional; the view shows "—" instead of crashing if Amount(qa:) ever rejects. The crashing path was on the funds-display step of Send — the worst-tier UX in a wallet app. - SendCoordinator.openInBrowserDetailsForTransaction: route through the injected UrlOpener instead of UIApplication.shared.open. Restores DI consistency with the rest of the codebase. - PrepareTransactionViewModel: fix the misleading comment claiming the recipient address is "not auto-checksummed". The payment construction silently checksums via toChecksummedLegacyAddress() — comment now describes actual behaviour and notes how to surface it as a real UX. - KeyChainSwift+KeyValueStoring: document the load-time ordering invariant (each KeychainKey is read as the same type it was written; KeychainSwift stores Bool/String as raw bytes so getData would otherwise alias them). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- KeyValueStore<KeychainKey>.wallet: pure read, no side effects. The destructive reinstall logic moves to wipeStaleKeychainOnReinstallIfNeeded() in Bootstrap, called once at launch, routed through the injected Preferences/SecurePersistence (no more Preferences.default hardcoded reference). Tests can now control this path via Container registrations. - UnlockAppWithPincodeViewModel: .prefix(1) on the biometric viewDidAppear trigger (so the prompt doesn't reappear on rotation/app-switcher return) and .first() on the merged unlock signal (so a biometric-success during mid-pincode entry can't fire userDid(.unlockApp) twice). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previously these three coordinators/view-models hit `incorrectImplementation`
(a `Never`-returning fatal) when the wallet was missing at scene-init time.
Reachable via races (wallet removed in Settings while a Send modal was open),
keychain access failures (biometric-protected access flag fires), or any
future feature that touches wallet on the same flow.
Wallet-app crashes inside Send / Backup / Decrypt-Keystore are the worst-tier
UX, so all three now bail out gracefully:
- SignTransactionViewModel adds a new `.walletUnavailable` navigation step;
on missing wallet, schedules the pulse on next viewDidLoad and returns an
inert Output. SendCoordinator handles `.walletUnavailable` by finishing
the whole Send flow.
- BackupWalletCoordinator and DecryptKeystoreCoordinator: replace the
`walletStorageUseCase.wallet.map { guard … else { incorrectImplementation(…) } }`
pattern with a `.compactMap { $0 }`. Downstream waits for a wallet that
never arrives and the user can dismiss the modal cleanly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pasteboard.copy(_:) now takes an optional `expiringAfter:` interval. Sites that copy security-sensitive material (private key, public key on the same screen, both keystore-copy paths) pass `SensitivePasteboard.expirationSeconds` (60s). The system pasteboard auto-clears the entry — preventing indefinite residency for clipboard managers, Universal Clipboard sync to the user's Mac, or other apps reading pasteboard later. Non-sensitive copies (receive address, transaction id) keep the legacy no-expiration behaviour via the protocol-extension overload. MockPasteboard updated to record (string, expiringAfter) pairs so tests can assert on the auto-clear policy. Existing copiedString assertions are unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ssion
WalletStorageUseCase.wallet was a reactive shape that did Keychain I/O +
JSON-decode on every single subscription. Many ViewModel transform(_:)
methods subscribe several times per scene (PrepareTransaction, Main,
Receive, …), so a single Send-screen presentation could trigger 4-5
Keychain RPCs for a value that hasn't changed.
DefaultWalletStorageUseCase now caches the wallet in a CurrentValueSubject
that:
- Lazily seeds from securePersistence.wallet on first access
(one Keychain read per app session).
- Is updated synchronously by save(wallet:) and deleteWallet() so the
cache and underlying store stay in lockstep.
- Is read by loadWallet() and hasConfiguredWallet so the synchronous
accessors are also cache-backed (zero further I/O).
Adds `var wallet` to the protocol so concrete implementations can override
the default extension; no-state mocks (MockWalletUseCase) keep the default
which still calls loadWallet() — they don't pay the Keychain cost anyway.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Drop `prepend(0)` on the nonce publisher in PrepareTransactionViewModel. Previously: Review button enabled on a stale nonce=0 before the network balance/nonce fetch had completed. User submits → network rejects with "nonce too low" → confusing error mid-Send. Now: payment stays nil (Review disabled) until the first network response lands. Disabled state also doubles as a "no network" gate which is the right UX anyway — sending with no balance/nonce is never sensible. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
SignTransactionViewModel previously enabled the Sign button as soon as the password met the structural minimum-length policy. The actual decryption happened during sendTransaction, which then bounced back through errorTracker and flipped the field red — wasting a network round-trip on a wrong password and surfacing a confusing post-broadcast-attempt error. Now the Sign button gates on BOTH structural validity AND a debounced local keystore-decrypt check via verifyEncryptionPasswordUseCase. Wrong password → button stays disabled, no network round-trip wasted, error state is immediate. Debounced 250ms + removeDuplicates so we don't fire fresh KDF runs on every keystroke. flatMapLatest cancels any in-flight check when a newer password lands. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previously: a wallet generated in CreateNewWallet was held in memory only
until the user confirmed "I have backed up" in BackupWalletCoordinator.
An app kill anywhere in between would lose the freshly-derived private key
forever (the key is randomly generated, so password alone can't recover it).
Now:
- CreateNewWalletCoordinator persists the wallet to Keychain immediately
after derivation, in the .createWallet branch of toCreateWallet.
- A new preferences key `hasConfirmedNewWalletBackup` is set to false
at persist time and flipped true when BackupWalletCoordinator returns
`.backUp`. Future work can gate Send behind this flag and surface a
"back up your wallet" banner until it's true.
- ChooseWalletCoordinator no longer double-saves on the create branch
(the create coordinator already did it). The restore branch keeps the
save, since that flow doesn't write the wallet itself. Renamed the
helper from `userFinishedChoosing(wallet:)` to
`finishChoosing(wallet:persistFirst:)` so the call sites are explicit
about which branch owns persistence.
Tests:
- ChooseWalletCoordinatorTests: drop the now-incorrect save assertion on
the create branch (responsibility moved); restore-branch save assertion
stays.
- CreateNewWalletCoordinatorTests: register an in-memory `Preferences`
so the new flag write doesn't leak into real UserDefaults during the
test, plus add two tests covering the new persist-on-create + flag-flip
behaviour.
Also clean up TestStoreFactory.makeSecurePersistence: the prior
`Preferences.default.save(value: true, for: .hasRunAppBefore)` workaround
is no longer needed (the destructive reinstall side effect was moved out
of the wallet getter into Bootstrap).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AppCoordinator + Coordinating+Scene{Present,Replace} +
Coordinating+Child+Start + DeepLinkHandler: replace [unowned] capture
lists with [weak] and adjust call sites for the optional unwrap. Same
rationale as the earlier MainCoordinator/SendCoordinator fix — these
publishers (deeplinks, navigator pulses) can outlive the
coordinator/scene during teardown, so [unowned] is a latent crash
trigger. [weak] is the safer default.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
sendTransaction's flatMapLatest captured self unowned to call zilliqaService.sendTransaction. The use case is held by Container, but the publisher chain can be retained by a subscriber (the Send flow's ViewModel) past the use case's lifetime in tests/teardown. Switch to [weak self] returning Empty if self is gone — drops the in-flight send silently rather than crashing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Coordinators (Onboarding, ChooseWallet, CreateNewWallet, BackupWallet,
DecryptKeystore, RestoreWallet, SetPincode) and ViewModels
(TermsOfService, AskForCrashReportingPermissions, WarningCustomECC,
DecryptKeystoreToRevealKeyPair, RestoreWallet, ConfirmNewPincode) and
the RestoreWalletView segment-switch sink.
All conversions follow the pattern in the code-review fix: replace
[unowned self] with [weak self], unwrap with `self?.` for void calls or
`guard let self else { return }` for multi-step bodies, and return
Empty for flatMapLatest closures whose self is gone. ChooseWallet
coordinator's doc comment about the unowned rationale updated to
describe the [weak self] rationale.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
MainCoordinator + MainViewModel + SendCoordinator + Send sub-scenes (PrepareTransaction VM, ScanQRCode View+VM, PollTransactionStatus VM) + ReceiveCoordinator + SettingsCoordinator + SettingsViewModel + RemovePincodeViewModel: replace all remaining [unowned self] (and one [unowned useCase]) with [weak] equivalents. Same crash-safety rationale as the navigation-infra and onboarding fixes. MainViewModel gains an explicit Foundation import so the new `-> Date?` annotation on the balanceWasUpdatedAt map closure resolves. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 252 out of 252 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
34
to
42
| /// Protocol exposing a stable string identifier — used as the cell-reuse | ||
| /// identifier in `SingleCellTypeTableView`. | ||
| /// | ||
| /// Note: deliberately not named `Swift.Identifiable` because we don't want to | ||
| /// conform to the system protocol (which has different semantics). | ||
| protocol Identifiable { | ||
| /// Stable string identifier. | ||
| static var identifier: String { get } | ||
| } |
There was a problem hiding this comment.
protocol Identifiable introduced here shadows Swift’s standard-library Identifiable within this module. Even if the app doesn’t currently use Swift.Identifiable, this makes future uses ambiguous/confusing (especially if SwiftUI or other code brings Identifiable constraints in). Consider renaming this protocol to something domain-specific (e.g. ReuseIdentifierProviding/ReuseIdentifiable) or explicitly qualifying stdlib uses as Swift.Identifiable to avoid collisions.
Three actionable Copilot review comments on PR #136: 1. Rename `protocol Identifiable` → `ReuseIdentifiable` in Sources/Views/TableView/ClassIdentifiable.swift to avoid shadowing Swift's stdlib `Identifiable` protocol. The two protocols are unrelated — Swift.Identifiable is a SwiftUI/diffing identity, this one is a UIKit cell-reuse identifier — but the name collision would make any future use of stdlib Identifiable ambiguous. Updated the one stale doc reference in SingleCellTypeTableView.swift to match. 2. Trim duplicate trailing empty entry in the Crashlytics PBXShellScriptBuildPhase.shellScript array (Zhip.xcodeproj/project.pbxproj:2643-2644). 3. Trim duplicate trailing empty entry in the SwiftLint PBXShellScriptBuildPhase.shellScript array (same file). Skipping the fourth comment (Debug code-signing override clearing PROVISIONING_PROFILE_SPECIFIER) per the user's reply: "this is a one-off… for an otherwise obsolete repo". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 252 out of 252 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| COV_XML: .build/coverage.xml | ||
| SCHEME: Zhip | ||
| PROJECT: Zhip.xcodeproj | ||
| SIM_DEVICE: iPhone 17 |
There was a problem hiding this comment.
SIM_OS is referenced later in this workflow (e.g. simulator validation and the xcodebuild -destination), but the env var was removed. This will make those steps fail or behave unpredictably. Define SIM_OS again (matching the selected Xcode runtime) or update the steps to not depend on it.
Suggested change
| SIM_DEVICE: iPhone 17 | |
| SIM_DEVICE: iPhone 17 | |
| SIM_OS: latest |
Comment on lines
16
to
20
| COV_XML: .build/coverage.xml | ||
| SCHEME: Zhip | ||
| PROJECT: Zhip.xcodeproj | ||
| SIM_DEVICE: iPhone 17 | ||
| SIM_OS: "26.1" | ||
|
|
There was a problem hiding this comment.
PR title/description mention only documentation + new dev certs, but this PR also introduces substantial behavioral changes (wallet persistence flow, pasteboard expiration, deep links, coordinator ownership changes, etc.). Please update the PR description to reflect the real scope/risk so reviewers know what they’re approving.
Comment on lines
+145
to
+146
| func test_createWalletCreateWallet_persistsImmediatelyAndMarksNotBackedUp() { | ||
| // The wallet must be persisted on derivation so an app kill before |
There was a problem hiding this comment.
Test name contains a duplicated word (CreateWalletCreateWallet), which makes it harder to scan and search for the intended scenario. Rename to something like test_createWalletCreate_persistsImmediatelyAndMarksNotBackedUp (or similar) so it matches the actual branch under test.
Six wasted erase calls — chain operators don't need the type-erased shape
because the next operator is itself a Publisher extension that accepts any
Publisher. Removing keeps the same behavior with one fewer allocation per
subscription.
PrepareTransactionViewModel:
- amountFormatted: drop erase between map and ifEmpty.
- gasLimitPlaceholder + gasPricePlaceholder: drop erase between Just and
map (the trailing erase satisfies the AnyPublisher target type).
- costOfTransaction inner Just chain: drop erase between Just and
compactMap.
RestoreWalletUsingPrivateKeyViewModel.confirmEncryptionPasswordValidationValue:
drop erase between combineLatest and map.
SignTransactionViewModel.encryptionPasswordValidation:
drop erase between merge(...) and withLatestFrom — withLatestFrom is a
Publisher extension that doesn't require AnyPublisher input.
ConfirmNewPincodeViewModel.confirmedTrigger.withLatestFrom(...):
drop erase between map(\.value) and filterNil — filterNil is a Publisher
extension.
Other Just(...).eraseToAnyPublisher() sites in Output struct fields are
KEPT — those satisfy the field's AnyPublisher type and aren't superfluous.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
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.
Uh oh!
There was an error while loading. Please reload this page.