Skip to content

fix: stop panicking on errors in exported gomobile bindings#2

Merged
TaprootFreak merged 1 commit into
DFXswiss:feature/ios-ble-pairingfrom
joshuakrueger-dfx:fix/no-panic-on-error
May 8, 2026
Merged

fix: stop panicking on errors in exported gomobile bindings#2
TaprootFreak merged 1 commit into
DFXswiss:feature/ios-ble-pairingfrom
joshuakrueger-dfx:fix/no-panic-on-error

Conversation

@joshuakrueger-dfx
Copy link
Copy Markdown

@joshuakrueger-dfx joshuakrueger-dfx commented May 8, 2026

Summary

  • Replace every panic(err) in go/api/ with a logged early return of the function's zero value (empty string, nil bytes, false)
  • Add defer recoverPanic(name) to every exported function as a safety net for unexpected panics from the underlying SDK
  • Nil-check the bitbox device pointer before dereferencing it (GetChannelHash, ChannelHashVerify, InitDevice, SupportsETH, SupportsLTC, SupportsBluetooth, SupportsERC20, DeviceInfo)
  • DeviceInfo no longer dereferences a nil *info on a failed device query
  • GetDeviceWithInfo falls back to a sensible default semver instead of panicking on a malformed version string
  • iOS Api.xcframework rebuilt with gomobile bind -target=ios .

Why

Every exported function panicked on the first error. gomobile surfaces a Go panic as an Objective-C / JNI exception and the host engine terminates before any Dart code can react to it. realunit-app hit this on a routine ETHSignMessage NACK from the device — the whole Flutter app died before DfxAuthService could fall back to the unauthenticated path. getETHAddress on a freshly paired device, BTCSignPSBT on a rejected confirmation, DeviceInfo after a disconnect — all crashed the engine the same way.

The SDK now stays in process: callers receive an empty signature / address / fingerprint and can decide what to do with it. Existing checks in the Swift wrapper (if let sig = signature) and Dart layer already treat empty results as "not available".

Verified behaviour after the patch

On realunit-app (develop HEAD = 951ac75) with this branch wired in via dependency_overrides, pairing a BitBox02 Plus succeeds and the app no longer crashes when the BitBox returns NACK on ETHSignMessage. The DFX auth call surfaces a normal Dart exception ("invalid signature string, value=0x") instead of an engine death — caller-side code can now show that error, retry, or fall back.

Out of scope

  • Android AAR was not rebuilt in this PR; run_build_tool_android.sh should be re-run before merging into a release branch.
  • The exported signatures still return zero values rather than typed errors, which keeps the binding ABI stable. A follow-up could expose error info via a separate LastError() getter or split the API to return tuples, but that needs callers on both Android and iOS to be updated.
  • The underlying NACK on ETHSignMessage from BitBox02 Plus on m/44'/60'/0'/0/0 chainId 1 was observed during testing and is not addressed here. That is a device-side / firmware compatibility question (likely needs DFX backend to accept a different proof-of-ownership for BitBox-backed wallets, or a SDK upgrade to a newer bitbox02-api-go that aligns with current BitBox02 Plus firmware).

Test plan

  • go build ./... clean
  • iOS xcframework regenerated; consuming app builds and runs without engine crash on ETHSignMessage NACK
  • BitBox-paired wallet survives DfxAuthService.getAuthToken() (signature comes back empty, backend rejects, app shows the unauthenticated state instead of dying)
  • Android AAR rebuilt by maintainer

@joshuakrueger-dfx joshuakrueger-dfx force-pushed the fix/no-panic-on-error branch from fd312fb to 681a9dd Compare May 8, 2026 11:23
Copy link
Copy Markdown

@TaprootFreak TaprootFreak left a comment

Choose a reason for hiding this comment

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

Android AAR must be rebuilt before merge — shipping iOS-only panic fixes while Android still crashes on the same code paths is not acceptable for a release.

Please run run_build_tool_android.sh and include the updated AAR binary in this PR.

Every exported function in `go/api/` panicked on the first error,
which gomobile surfaces as an Objective-C/JNI exception and
terminates the host engine. realunit-app hit this on a routine
ETHSignMessage NACK from the device — the whole Flutter app died
before the caller could show a useful message.

- Replace `panic(err)` with logged early returns of the function's
  zero value (empty string, nil bytes, false).
- Add `defer recoverPanic(name)` to every exported function as a
  safety net for unexpected panics from the underlying SDK.
- Nil-check the `bitbox` device pointer before dereferencing it in
  GetChannelHash, ChannelHashVerify, InitDevice, SupportsETH,
  SupportsLTC, SupportsBluetooth, SupportsERC20 and DeviceInfo.
- DeviceInfo no longer dereferences a nil `*info` when the device
  query fails.
- GetDeviceWithInfo falls back to a sensible default semver instead
  of panicking on a malformed version string.

The iOS xcframework was rebuilt with `gomobile bind -target=ios .`
so the host can pick up the new behaviour. Android still needs to
be rebuilt separately via the existing run_build_tool_android.sh.
@joshuakrueger-dfx joshuakrueger-dfx force-pushed the fix/no-panic-on-error branch from 681a9dd to d919b27 Compare May 8, 2026 11:58
@joshuakrueger-dfx
Copy link
Copy Markdown
Author

@TaprootFreak Android AAR jetzt mit drin (android/libs/api.aar + api-sources.jar).

Build-Setup:

  • gomobile @ go1.26
  • Android NDK r28 (28.0.13004108) via android-commandlinetools
  • `gomobile bind -target=android -androidapi 24 -o api.aar .`

Beide Binaries (iOS xcframework + Android AAR) sind aus identischer Go-Quelle gebaut und enthalten dieselben Panic-zu-Error-Konvertierungen.

@TaprootFreak TaprootFreak merged commit c0cbca3 into DFXswiss:feature/ios-ble-pairing May 8, 2026
TaprootFreak added a commit that referenced this pull request May 8, 2026
* refactor: replace Go binding script with Gradle task and new helper script

* refactor: adjust Gradle build configuration and update Go binding task

* refactor: update Gradle task to handle API AAR relocation

* refactor: simplify Gradle build script and adjust Go binding task paths

* feat: add iOS BLE support for BitBox02 Nova

Implement CoreBluetooth communication layer for BitBox02 Nova (Plus)
hardware wallet on iOS:

- Add Bluetooth.swift with BLE service/characteristic discovery,
  read/write operations, and u2fhid packet handling
- Add BitboxFlutterPlugin.swift methods for iOS: initBitBox,
  getChannelHash, channelHashVerify, getETHAddress (all on background
  threads to avoid blocking UI)
- Fix product mapping: bb02p-* now correctly maps to
  ProductBitBox02PlusMulti/PlusBTCOnly (not the non-Plus variants)
- Fix BLE stale buffer issue: clear read buffer + drain semaphore
  before each new u2fhid init frame to prevent hwwRspBusy panics
- Fix race condition: semaphore.signal() moved inside readBufferLock
- Add 10s timeout on readBlocking to prevent hangs on BLE disconnect
- Change InitDevice() from panic to bool return on error
- Add pre-built Api.xcframework for iOS (gomobile bind)
- Add channelHash and channelHashVerify to Dart platform interface

* fix: run signETHMessage on background thread

Prevents blocking the main thread during DFX auth signing
when BitBox device communication is required.

* fix: run all signing methods on background thread

Move signETHRLPTransaction and signETHTypedMessage to
DispatchQueue.global().async to prevent blocking the main thread
during device communication.

* fix: ANR.

* fix: stop panicking on errors in exported gomobile bindings (#2)

Every exported function in `go/api/` panicked on the first error,
which gomobile surfaces as an Objective-C/JNI exception and
terminates the host engine. realunit-app hit this on a routine
ETHSignMessage NACK from the device — the whole Flutter app died
before the caller could show a useful message.

- Replace `panic(err)` with logged early returns of the function's
  zero value (empty string, nil bytes, false).
- Add `defer recoverPanic(name)` to every exported function as a
  safety net for unexpected panics from the underlying SDK.
- Nil-check the `bitbox` device pointer before dereferencing it in
  GetChannelHash, ChannelHashVerify, InitDevice, SupportsETH,
  SupportsLTC, SupportsBluetooth, SupportsERC20 and DeviceInfo.
- DeviceInfo no longer dereferences a nil `*info` when the device
  query fails.
- GetDeviceWithInfo falls back to a sensible default semver instead
  of panicking on a malformed version string.

The iOS xcframework was rebuilt with `gomobile bind -target=ios .`
so the host can pick up the new behaviour. Android still needs to
be rebuilt separately via the existing run_build_tool_android.sh.

---------

Co-authored-by: Konstantin Ullrich <konstantinullrich12@gmail.com>
Co-authored-by: TuanLamNguyen <xnguyen.lam@gmail.com>
Co-authored-by: joshuakrueger-dfx <joshua.krueger@dfx.swiss>
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.

2 participants