Skip to content

fix(swift-sdk): build the shippable profile by default - #4371

Open
romchornyi wants to merge 1 commit into
v4.2-devfrom
fix/ship-release-profile-by-default
Open

fix(swift-sdk): build the shippable profile by default#4371
romchornyi wants to merge 1 commit into
v4.2-devfrom
fix/ship-release-profile-by-default

Conversation

@romchornyi

@romchornyi romchornyi commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

build_ios.sh defaults to --profile dev, which maps to the dev-ios profile: Cargo's dev defaults (debug assertions and overflow checks on, opt-level = 0) plus panic = "abort". So in such a build every debug_assert! in dash-spv, key-wallet and platform-wallet is live, and aborts the host process instead of degrading.

That is exactly what a dev build is for locally. It is also what shipped.

A TestFlight build of the wallet — dashpay 9.0.0 (25), iPhone17,2, iOS 26.6 — died with SIGABRT:

Thread 9 Crashed:
  abort  ←  rust_panic  ←  core::panicking::panic_fmt
  ←  dash_spv::sync::filters::manager::FiltersManager::start_download::{{closure}}
        (manager.rs:182)
  ←  dash_spv::sync::filters::sync_manager::…::start_sync   (sync_manager.rs:89)
  ←  dash_spv::sync::sync_manager::SyncManager::handle_network_event  (sync_manager.rs:193)

manager.rs:182 at the pinned rev is debug_assert!(self.is_idle(), "manager should have no in-flight state on start"); — a line a release build compiles out entirely.

Two independent confirmations that the shipped binary had debug assertions on:

  1. The assert executed at all. debug_assert! is a no-op under cfg!(debug_assertions) == false, so its firing is first-hand proof.
  2. The crash symbolicates to exact source lines. release-ios never sets debug, so it emits no line tables; dev-ios's debug = "line-tables-only" is what makes per-line frames possible.

A session log from a comparable build carries platform_wallet_ffi::metrics lines, which this script enables only under dev-ios — the same story from a second angle.

So the invariant violation is a real bug worth fixing on its own (upstream, in dash-spv), but the profile is what turned it into a user-visible crash rather than a log line.

What was done?

  • Default profile is now release.
  • dev remains available and now prints a warning naming what it implies, rather than being the silent default.
  • --help spells out the difference instead of listing dev (default) with no consequences attached.
  • The quick-build snippet in AGENTS.md shows both forms.

Both CI callers already pass --profile explicitly — release-swift-sdk.yml uses release, swift-example-app-ui-smoke.yml uses dev — so neither changes behaviour.

The trade-off, stated plainly

An unqualified ./build_ios.sh is now slow: release-ios is fat LTO, one codegen unit, opt-level = 3. It used to be fast. A developer who wants the fast build asks for it by name; the alternative is that people keep shipping the one whose cost only shows up weeks later on someone else's device.

If reviewers prefer to keep dev as the default, the warning alone still helps — but it is a warning in a build log, which is precisely where this went unnoticed the first time.

How Has This Been Tested?

bash -n packages/swift-sdk/build_ios.sh      # syntax clean
./build_ios.sh --help                        # profile section reads correctly

Verified by reading, not by running a full build of each profile: the change is to a default and a message, and both CI callers pin the profile explicitly so their behaviour is unchanged by construction.

Breaking Changes

No API or artifact-format change. A behavioural change for anyone invoking build_ios.sh without --profile: they now get an optimized build, which takes considerably longer and produces a framework without debug assertions. That is the intent.

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

Summary by CodeRabbit

  • Documentation

    • Updated iOS framework build instructions to target simulators and use release builds by default.
    • Documented the optional development profile and its debug limitations.
  • Build Improvements

    • Added clearer build profile guidance and warnings for development builds.
    • Release builds are now the default for improved distribution readiness.

`build_ios.sh` defaulted to `--profile dev`, which maps to `dev-ios`:
Cargo's `dev` defaults (debug assertions and overflow checks on,
opt-level 0) plus `panic = "abort"`. Every `debug_assert!` in dash-spv,
key-wallet and platform-wallet is therefore live in such a build, and
aborts the host process instead of degrading.

That is correct for local iteration and wrong for anything a tester
touches — and it has already happened. A TestFlight build of the wallet
(9.0.0/25) died with SIGABRT inside
`FiltersManager::start_download` at `filters/manager.rs:182`, which is
`debug_assert!(self.is_idle(), ...)`. A release build would have compiled
that line out entirely. The same build's logs carry
`platform_wallet_ffi::metrics` lines, which `build_ios.sh` only enables
under `dev-ios` — so the profile, not the assert, is what made an
internal invariant a user-visible crash.

The default is now `release`; `dev` stays available and prints a warning
naming what it implies. Both CI callers pass `--profile` explicitly
(`release-swift-sdk.yml` release, `swift-example-app-ui-smoke.yml` dev),
so neither changes behaviour.

The trade-off is deliberate: an unqualified `./build_ios.sh` is now slow
(fat LTO, opt-level 3) where it used to be fast. A developer who wants
the fast one asks for it by name; nobody ships the slow-to-notice one by
forgetting to.
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 794ff007-1a25-4723-adea-7565a05520d4

📥 Commits

Reviewing files that changed from the base of the PR and between c6eedde and 109e5c9.

📒 Files selected for processing (2)
  • AGENTS.md
  • packages/swift-sdk/build_ios.sh

📝 Walkthrough

Walkthrough

The iOS build script now defaults to release builds, documents release and dev profiles, and warns about dev build limitations. The framework build instructions specify simulator targeting and profile selection.

Changes

iOS build profiles

Layer / File(s) Summary
Profile defaults and warnings
packages/swift-sdk/build_ios.sh, AGENTS.md
The script defaults to the release profile, adds colored warning output, documents profile behavior, and warns that dev-ios may abort the host app. The build instructions specify release and optional development simulator builds.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: quantumexplorer, zocolini

🚥 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 concisely describes the main change: using the shippable release profile by default for Swift SDK builds.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ 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/ship-release-profile-by-default

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

@thepastaclaw

thepastaclaw commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit 109e5c9)

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Final validation — Codex/Sol only (Phase 2 disabled)

At exact head 109e5c9, the script correctly maps an omitted profile to the existing release-ios Cargo profile while retaining explicit dev-ios builds, their tokio-metrics feature, and a clear distribution warning. The documentation matches the new behavior, explicit CI profile selection remains unchanged, and no in-scope correctness or FFI issues were found.
Source: reviewer backends codex/gpt-5.6-sol (general) and codex/gpt-5.6-sol (ffi-engineer); final verifier backend codex/gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and not reviewer evidence.

Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.

Review provenance

  • Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — ffi-engineer (completed)
  • Verifier: gpt-5.6-sol — verifier
  • Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
  • Secondary pass: disabled (temporary_phase2_sonnet_disable)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants