Skip to content

fix: arm32 uniffi bindings - #150

Merged
ovitrif merged 15 commits into
masterfrom
codex/fix-arm32-uniffi-returns
Sep 2, 2026
Merged

fix: arm32 uniffi bindings#150
ovitrif merged 15 commits into
masterfrom
codex/fix-arm32-uniffi-returns

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes #149

Description

  • Pins Android binding generation to Gobley revision 9bf3be0cfc39c1cc7a3853cc9f0a0dee79eb0f75.
  • Regenerates the 0.5.14 mobile bindings and release artifacts.
  • Removes generated Android JNI libraries from source control and ignores bindings/android/lib/src/main/jniLibs/.
    • The release workflow deletes and rebuilds these libraries before publishing the AAR, so the tracked copies duplicated CI output and could differ based on who built them locally.
    • Ignoring the directory prevents future JNI binary updates from growing the Git history.
  • Documents that CI builds and publishes the Android bindings while keeping the local binding commands available.

Preview

Not applicable; this changes binding generation and release artifacts.

QA Notes

  • ./build.sh all
  • cd bindings/android && ./gradlew build
  • cargo clippy --all-targets
  • cargo test modules::activity: 196 passed
  • ARM32 release A/B on an API 24 armeabi-v7a AVD: bitkit-core 0.5.11 reproduced the exact UniFFI checksum crash; 0.5.14 passed three cold launches (validation details).

@ovitrif ovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-09-01T21:51:36.022721Z e65746e Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ovitrif ovitrif changed the title fix: correct ARM32 UniFFI return carriers fix: arm32 uniffi return carriers Sep 1, 2026
@ovitrif ovitrif changed the title fix: arm32 uniffi return carriers fix: arm32 uniffi bindings Sep 1, 2026
@ovitrif

This comment was marked as outdated.

@chatgpt-codex-connector

This comment was marked as outdated.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

ARM32 release A/B validation

I reproduced the original crash with an older binding and verified the fix with the AAR generated by this PR at 0974571.

Environment and method

  • AOSP Android 7.0 / API 24 AVD with ro.product.cpu.abi=armeabi-v7a and ro.product.cpu.abilist=armeabi-v7a,armeabi.
  • Android Emulator 28.0.25 (build 5395263), using its qemu-system-armel full ARM emulator with CPU acceleration disabled. This was used because current emulator versions no longer support ARM32 guests.
  • The same signed, arm-only test app for both runs, changing only the com.synonym:bitkit-core-android version resolved from Maven Local.
  • The test APK had R8 minification enabled, but minification was not the condition under test.
  • The app called getDefaultGapLimit(), which initializes the UniFFI binding and runs its API checksum validation.
  • Before each run, the installed package was force-compiled with cmd package compile -m speed -f com.example.bitkitcorearm32test; each result below is from a force-stopped cold launch.

Control: bitkit-core 0.5.11

  • AAR SHA-256: 23772ce69a69378046c7ef01225c138c1c8c41f6013d857b3f78b327fc5f07f7
  • APK SHA-256: c256ba510bf0e646f883bf2b26a5228804a84faff2d35812955a44d09322f161
  • Its generated binding uses Short for checksum externs and .toShort() for expected checksums; for example, uniffi_bitkitcore_checksum_func_activities_to_json() is compared with 63832.toShort().
  • Result: the process failed during IntegrityCheckingUniffiLib.uniffiCheckApiChecksums() and crashed on the main thread:
E BITKIT_ARM32_TEST: FAIL bitkit-core=0.5.11
Caused by: java.lang.RuntimeException: UniFFI API checksum mismatch: try cleaning and rebuilding your project
    at com.synonym.bitkitcore.IntegrityCheckingUniffiLib.uniffiCheckApiChecksums(...)
E AndroidRuntime: FATAL EXCEPTION: main

Fixed: bitkit-core 0.5.14

  • AAR SHA-256: a2b5b0d110443c02ee8bbcd2fe42208369ed4530fb61a7a19359262e089df04c
    • This exactly matches bindings/android/lib/build/outputs/aar/lib-release.aar at 0974571.
  • APK SHA-256: a56e314a585a6aa07545c4b837a56a0af24af49de5e77a27b3cd10c81e51dd93
  • Its generated binding uses Int for the checksum externs and for ffi_bitkitcore_rust_future_complete_u8.
  • Result: three consecutive cold launches completed initialization without a checksum exception or fatal runtime entry:
I BITKIT_ARM32_TEST: PASS bitkit-core=0.5.14 gapLimit=20
I BITKIT_ARM32_TEST: PASS bitkit-core=0.5.14 gapLimit=20
I BITKIT_ARM32_TEST: PASS bitkit-core=0.5.14 gapLimit=20

This controlled A/B reproduces the exact ARM32 failure with the old generated carriers and verifies that the binding generated by this PR fixes it.

@coreyphillips

Copy link
Copy Markdown
Collaborator

Two independent reviews, nothing blocking a merge.

worth doing, does not block

  • Fix widens only unsigned narrow returns; bool returns keep the Byte carrier (bindings/android/lib/src/main/kotlin/com/synonym/bitkitcore/bitkitcore.android.kt:3522) [one reviewer only]. The pinned Gobley commit changes the JNA carrier for u8/u16 direct returns to Int, but leaves signed sub-word returns alone. At the JNA boundary those two cases are indistinguishable: before the fix, a u16 return and an i16 return both generated the identical declaration ): Short. Nothing in the Kotlin declaration or the native ABI tells them apart, so whatever makes a Short carrier read wrong on armeabi-v7a should make a Byte carrier read wrong too. UniFFI carries bool as i8, so these stay on the narrow carrier and are lifted with FfiConverterBoolean.lift(Byte) = value.toInt() != 0 (line 4488): uniffi_bitkitcore_fn_func_is_valid_bip39_word : Byte (3522) uniffi_bitkitcore_fn_func_is_address_used : Byte (3520) uniffi_bitkitcore_fn_func_delete_activity_by_id : Byte (3239) uniffi_bitkitcore_fn_func_delete_transaction_details: Byte (3251) uniffi_bitkitcore_fn_func_remove_closed_channel_by_id: Byte (3751) uniffi_bitkitcore_fn_func_trezor_is_ble_available : Byte (3852) ffi_bitkitcore_rust_future_complete_i8 : Byte (4047) // async bool If the residual defect is real it fails silently (a wrong true/false) rather than crashing at startup, which is worse than the checksum crash this PR fixes, and nobody would have noticed it on 0.5.11 because the checksum mismatch killed the process before any of these ran. I could not confirm it: I have no ARM32 device, and neither the Gobley commit message nor its CHANGELOG records the root cause, so the asymmetry may be deliberate. One plausible discriminator is that all 199 checksum functions take zero arguments while the bool functions take a RustBufferByValue. Since you already have the API 24 armeabi-v7a AVD set up, a cheap check is isValidBip39Word("abandon") vs isValidBip39Word("zzzz") on 0.5.14: if both come back true, the same widening is needed for Int8. What I did confirm: checksum name/value pairs are byte-identical to master (only the .toShort() suffix is gone), and the unmasked != 13556 comparison is safe because rustc zero-extends extern "C" u16 returns to 32 bits. I checked that against the committed dylib with ctypes (restype=c_uint32): activities_from_json -> 0x000034f4, activities_to_json -> 0x0000f958, check_sweepable_balances -> 0x0000fac9, all upper bits clear and matching the literals in the bindings.

nits

  • README still says jniLibs are only needed before a direct Gradle publish (bindings/android/README.md:93) [one reviewer only]. validateReleaseNativeLibraries in lib/build.gradle.kts:117 is wired to bundleReleaseAar as well as publish*, so with jniLibs untracked a plain ./gradlew build in a fresh clone now fails with "Android native library missing". It fails loudly, which is the right behaviour, but the README sentence reads as if only publishing needs the prior ./build_android.sh run.
  • build.sh target order swapped with no stated reason (build.sh:107) [one reviewer only]. all now runs android before ios. I traced the three scripts and could not find an ordering dependency: python runs last either way and rewrites crate_type itself, and build_android.sh restores Cargo.toml through its EXIT trap. If the intent was just to fail fast on the long Android leg, it is worth a line in the PR description; otherwise it is an unrelated change in a release-artifact PR.

@ovitrif

ovitrif commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

The unsigned-only widening is intentional. On ARM32, AAPCS zero-extends u8/u16 into r0, while JNA Byte/Short carriers are JNI jbyte/jshort and expect a sign-extended value. Those disagree only when the high bit is set (u8 >= 128, u16 >= 32768), which is the checksum case that crashed 0.5.11. Bool is UniFFI i8 0/1; sign-extension and zero-extension produce the same 32-bit r0 value, so the Byte carrier already matches the ABI and widening it would not change the result. That is also how the Gobley pin is scoped: unsigned 8-bit and 16-bit direct returns only.

Nits in d7ae6be:

  • README now says ./build_android.sh is required before ./gradlew build as well as publish, because validateReleaseNativeLibraries is wired to bundleReleaseAar.
  • Restored the previous all target order (ios then android then python). The swap was incidental and has no script dependency.

@ovitrif
ovitrif merged commit a9db5a5 into master Sep 2, 2026
1 check passed
@ovitrif
ovitrif deleted the codex/fix-arm32-uniffi-returns branch September 2, 2026 18:16
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.

fix: android arm32 uniffi return carriers

2 participants