Skip to content

fix: support 16 KB Android pages#1107

Open
ovitrif wants to merge 10 commits into
masterfrom
ovi/fix-16kb-dependencies
Open

fix: support 16 KB Android pages#1107
ovitrif wants to merge 10 commits into
masterfrom
ovi/fix-16kb-dependencies

Conversation

@ovitrif

@ovitrif ovitrif commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1103

This PR:

  1. Updates CameraX, DataStore, JNA, Graphics Path, and ML Kit barcode scanning to versions that support Android devices using 16 KB memory pages.
  2. Builds the CameraX and Graphics Path JNI libraries from their pinned upstream sources with 16 KB ELF alignment and packages the AndroidX Apache 2.0, libyuv BSD, and libyuv patent notices with the application.
  3. Gates camera and gallery QR analysis on Play Services barcode model readiness, initializes CameraX after the first permission grant, owns gallery scanner resources for the screen lifecycle, reports gallery URI access failures through the scanner error path, and retries camera and gallery model installation independently.
  4. Validates final APK and AAB alignment, every packaged native library's PT_LOAD segments, and GNU_RELRO alignment across all packaged ABIs.
  5. Provisions a pinned, checksum-verified Bundletool, preserves the rebuilt application and producer DWARF entries in the merged native-symbol archive, and runs the native compatibility gate in internal releases, public releases, and the local release path.

QA Notes

Manual Tests

  • 1. Launch a fresh mainnetDebug installation on an Android 17 emulator using a true 16 KB page size → Confirm Android shows no page-size compatibility alert.
  • 2. Accept the terms → Skip the introduction → Create a new wallet → Relaunch Bitkit → Confirm the wallet home restores and Lightning and VSS startup complete.
  • 3. Open the scanner → Grant camera permission → Present a BIP21 QR code → Confirm model readiness completes before analysis and the scanner parses the address and amount.

Automated Checks

  • just compile
  • just test
  • just lint
  • Final universal APK and AAB validation across all packaged ABIs and native libraries
  • Scanner lifecycle and retry-ownership tests plus byte-identical AndroidX and libyuv notice assets in the APK and AAB
  • Gallery URI failure/success coverage and a two-ABI merged native-symbol archive containing all required DWARF entries
  • Negative-control validation rejects the current master APK
  • actionlint, shellcheck, bash -n for the native validation and Bundletool provisioning scripts, and git diff --check
  • GitHub-hosted internal and public release artifact validation (after merge)

@ovitrif
ovitrif marked this pull request as ready for review July 24, 2026 08:30
@greptile-apps

This comment has been minimized.

greptile-apps[bot]

This comment was marked as resolved.

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CI is green here, but green CI never exercises the release path. Four blockers, plus a question that may let most of this PR be dropped.

Blockers

1. create-native-debug-symbols.sh fails on every release

$output is this script's own output name, but this PR turns it into a required input:

output="app/build/outputs/native-debug-symbols/$variant/native-debug-symbols-$build_number.zip"
...
if [ ! -f "$output" ]; then
    echo "AGP native debug symbols archive not found at '$output'." >&2

AGP 8.13.2 writes native-debug-symbols.zip with no version suffix (MergeNativeDebugMetadataTask -> withName("native-debug-symbols.zip"), confirmed against the pinned jar). The only file that has ever existed at the build-numbered name is this script's own prior output, and the Justfile runs rm -f "$symbols_dir"/native-debug-symbols*.zip before the build, so no stale copy survives either. Result: just release and both release workflows abort after a full build.

agp_archive="$output_dir/native-debug-symbols.zip"
if [ ! -f "$agp_archive" ]; then
    echo "AGP native debug symbols archive not found at '$agp_archive'." >&2
    exit 1
fi
copy_archive_symbols "$agp_archive" "$tmp_dir" "$application_required_libs"

2. check-16kb-compat.sh aborts on macOS with no error message

validate_apk and validate_aab both do:

unzip -q "${artifact}" -d "${extracted_root}"

Release APKs contain hundreds of AAPT2 entries that differ only by case (res/-b.xml vs res/-B.xml), 348 in the current release APK. On case-insensitive APFS these collide, unzip drops into its interactive replace ...? prompt and exits 1. Under set -euo pipefail the script dies there, so fail() never runs and there is no diagnostic at all: just release just stops, having validated nothing.

unzip -q -o "${artifact}" 'lib/*/*.so' 'base/lib/*/*.so' -d "${extracted_root}"

That also matches what the existing "Verify native libraries are stripped" step already does.

3. The 16 KB gate is applied to 32-bit ABIs, and that is the only thing forcing the ML Kit swap

validate_archive_libraries walks every .so regardless of ABI. Android's 16 KB requirement is 64-bit only: https://developer.android.com/guide/practices/page-sizes states the Play requirement as "must support 16 KB page sizes on 64-bit devices", and its verification step inspects "arm64-v8a or x86_64" shared libraries.

I pulled barcode-scanning-17.3.0.aar from dl.google.com and read the ELF program headers. On arm64-v8a, libbarhopper_v3.so has PT_LOAD p_align = 0x4000 and PT_GNU_RELRO end 0x4b0000, which is 16 KB aligned. It passes this script's own gate on the ABI that matters. Only the 32-bit slices are 0x1000. 17.3.0 is still <latest> in Google's maven-metadata, so there is no newer bundled build pending.

Suggest skipping non arm64-v8a / x86_64 ABIs in validate_archive_libraries (abi_for_library is already there) while still requiring at least one 64-bit library to be found. zipalign -P 16 on the whole APK is correct and ABI agnostic, keep that.

4. The PT_GNU_RELRO end-alignment rule is not a 16 KB requirement

relro_end=$((virtual_address + memory_size))
if (( relro_end % PAGE_SIZE != 0 )); then

Google's criterion is PT_LOAD p_align >= 16384. Bionic's _phdr_table_set_gnu_relro_prot page-rounds RELRO deliberately ("we're going to be over-protective here and put every page touched by the segment as read-only"), so a non-page-aligned RELRO end is not a load or runtime failure.

As written this rule rejects Google's own current prebuilts. Measured with NDK 28 llvm-readelf -W -l, arm64-v8a:

library PT_LOAD p_align RELRO end 16 KB aligned
camera-core 1.5.2 (what master already ships) 0x4000 pass libsurface_util_jni fails
camera-core 1.6.1 0x4000 pass libsurface_util_jni fails
graphics-path 1.0.0 / 1.0.1 / 1.1.0 0x4000 pass fails
jna 5.18.1 (already on master) 0x4000 pass pass
ldk-node / vss-client / paykit / bitkit-core 0x4000 pass pass

The cause is just lld padding RELRO to -z common-page-size (4096 by default), so any future AAR built with stock NDK defaults will block a release for an incompatibility that does not exist.

Neither library is actually at risk, which I checked in the program headers: for libsurface_util_jni.so the RW LOAD segment is exactly the RELRO region, so there is nothing writable to over-protect; for graphics-path the second RW segment (0x9ed8) sits in a different 16 KB page from the RELRO region (0x5b40).

Suggest dropping the relro_end % PAGE_SIZE assertion and the relro_count > 0 requirement, keeping PT_LOAD p_align >= 0x4000, zipalign -v -c -P 16 4, and the PAGE_ALIGNMENT_16K bundle check. If RELRO padding is wanted as project policy, apply it to the ndkBuild outputs only, not to every .so in the APK.

The question this raises

With the gate corrected to Google's actual criterion, every native library in master's dependency set already passes on arm64-v8a, including bundled ML Kit. That would make both app/src/main/cpp/ (48k lines) and the ML Kit swap unnecessary.

Which library and which criterion actually produced the page-size alert on the 16 KB emulator? If it was RELRO or 32-bit only, most of this PR can be dropped.

Product impact of the ML Kit swap

This is not a version bump, it is a change of distribution model, and the PR body item 1 ("Updates ... ML Kit barcode scanning to versions that support ... 16 KB memory pages") understates it.

  • barcode-scanning-17.3.0.aar is 9.9 MB and ships libbarhopper_v3.so for four ABIs plus three .tflite models. Decodes fully on device, no Play services.
  • play-services-mlkit-barcode-scanning-18.3.1.aar is 519 KB and ships classes.jar and a manifest. No native code, no model. All detection is delegated to Play services at runtime.

On a device without Play services, areModulesAvailable fails, isScannerReady never becomes true, analyze() drops every frame, the gallery path never reaches processImage, and the new Retry button can never succeed. There is no fallback: com.google.zxing:core is a dependency but is only used for encoding in QrCodeImage.kt, and there is no GoogleApiAvailability handling anywhere in the repo. We distribute sideloadable APKs on GitHub releases, so this is not only a Play Store audience.

There is also an offline regression for Play Store users' first scan: the com.google.mlkit.vision.DEPENDENCIES manifest pre-download only applies to Play Store installs, so a sideloaded build has to fetch the module from Google over the network before the first scan works.

If the swap stays, it needs a zxing decode fallback for the unavailable case and explicit product sign-off. Either way the changelog fragment needs to say it: "Improves native library compatibility on Android devices using 16 KB memory pages" gives a user whose scanner stopped working nothing to go on.

Other findings

QrCodeAnalyzer.kt, spurious "Scan failed" toast after a successful scan. analyzer.close() in onDispose calls scanner.close(), which cancels in-flight ML Kit tasks. analyze() registers addOnCompleteListener with no executor and no closed guard, and a canceled Task reports isSuccessful=false with a null exception, so it.exception ?: AppError("Scan failed") fires on the main thread after dispose. Scan a valid QR in the Send sheet and the error toast lands on the Amount screen. QrImageScanOperation already gets this right with success/failure listeners plus closed/completed guards. Suggest a @Volatile private var closed, set it and isScannerReady = false in close(), return early from analyze() and from the listener, and skip it.isCanceled explicitly.

QrScanningScreen.kt / SendRecipientScreen.kt, raw exception text shown to users. toastQrScanError's non-model branch is toast(error), which renders error.message as the toast description. This replaces the localized other__qr_error_header / other__qr_error_text, so a camera scan failure now shows raw English ML Kit strings ("This detector is already closed!", "Scan failed"). Keep the localized copy in that branch and log the throwable via Logger.error(..., context = TAG).

No version lockstep between the vendored sources and the AARs. pickFirsts makes the locally built .so permanently shadow the AAR copy, but the camera = "1.6.1" / graphics-path = "1.1.0" pairing is recorded only in prose in third_party/androidx/README.md. graphics-path is a soft require and Compose already pulls it transitively at 1.0.1, so a routine compose-bom bump can resolve a newer AAR against a .so built from 1.1.0 sources. pathway.cpp does RegisterNatives with hardcoded descriptors, so a signature change there is an UnsatisfiedLinkError on API 28 to 33 with every build-time check still green. Suggest asserting in NativeReleaseConfigTest.kt that the versions in libs.versions.toml match the README, and/or strictly(...) pins so a transitive upgrade fails resolution instead. (Provenance itself is exact: the vendored files are byte-identical to upstream at the pinned SHAs.)

Nits

  • org.mockito.kotlin.any() is written inline fully qualified 11 times (9 in BarcodeModelInstallerTest.kt, 2 in QrImageScanOperationTest.kt) while five other symbols from the same package are imported. No other test file in the repo does this. Add import org.mockito.kotlin.any.
  • Trailing comma after the last modifier = ... argument at both new PrimaryButton calls (QrScanningScreen.kt:351, SendRecipientScreen.kt:455). Repo convention is no comma there: 242 modifier chains under ui/ end without one, 10 with.
  • BarcodeModelUnavailableException extends Exception rather than AppError. Related, QrScanningScreen still does onError(Exception("No QR code found in the image")) where SendRecipientScreen correctly uses AppError(context.getString(R.string.other__qr_error_text)).

Conflicts with #1106

git merge-tree gives conflicts in three files: scripts/create-native-debug-symbols.sh, app/src/test/java/to/bitkit/build/NativeReleaseConfigTest.kt, gradle/libs.versions.toml. release-internal.yml and app/build.gradle.kts auto-merge cleanly.

The one to watch: #1106 adds libpaykit.so to required_libs, this PR splits the same line into dependency_required_libs plus application_required_libs. Taking this PR's side of the hunk silently drops libpaykit.so from the release symbol gate even though both artifacts ship it, and then NativeReleaseConfigTest fails either way, where the least-effort green fix cements the gap. Correct resolution:

dependency_symbols_dir=${NATIVE_SYMBOLS_DEPENDENCY_DIR:-app/build/intermediates/native-debug-symbol-artifacts}
dependency_required_libs="libbitkitcore.so libldk_node.so libpaykit.so libvss_rust_client_ffi.so"
application_required_libs="libimage_processing_util_jni.so libsurface_util_jni.so libandroidx.graphics.path.so"
required_libs="$dependency_required_libs $application_required_libs"

Also, after both land, release-internal.yml builds an AAB that no step in that workflow inspects: release.yml passes both directories to the 16 KB check, the internal workflow passes only the APK dir and has no Setup bundletool step.

Verified as fine, no action needed

pickFirsts ordering is deterministic (AGP visits project native libs before external ones, with isReproducibleFileOrder()); APP_STL=c++_static with -nostdlib++ is sound and a missing libc++ symbol would be a link error, not a runtime one; the vendored trees are byte-identical to upstream at the pinned SHAs and the JNI signatures match camera-core 1.6.1 exactly; license packaging is adequate (no upstream NOTICE files exist at those commits, and the sources are unmodified); the libyuv subset resolves all referenced symbols; the ELF field parsing in check-16kb-compat.sh is correct for both 32- and 64-bit layouts; the bundletool pin and its SHA-256 are correct; strings.xml ordering and the values/-only placement match repo convention; NativeReleaseConfigTest asserting on generated assets is safe because preBuild carries the dependency; and ndkVersion matches the Justfile and both workflows.

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: resolve third-party 16 KB native compatibility

2 participants