Skip to content

fix(kotlin-sdk): address Android JNI review issues #4002

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
QuantumExplorer merged 1 commit into from
Jul 4, 2026
Merged

fix(kotlin-sdk): address Android JNI review issues #4002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/kotlin-sdk-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@ on:
pull_request:
paths:
- 'packages/kotlin-sdk/**'
- 'Cargo.toml'
- 'Cargo.lock'
- 'packages/rs-*/**'
- 'packages/rs-unified-sdk-jni/**'
- 'packages/rs-sdk-ffi/**'
- 'packages/rs-platform-wallet-ffi/**'
Comment on lines +11 to 14

@thepastaclaw thepastaclaw Jul 4, 2026

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.

💬 Nitpick: packages/rs-*/** already subsumes the three specific rs-* paths

The umbrella glob at line 11 (packages/rs-*/**) already matches packages/rs-unified-sdk-jni/**, packages/rs-sdk-ffi/**, and packages/rs-platform-wallet-ffi/**. GitHub Actions unions the patterns so this is harmless, but the redundancy suggests the three specific entries are load-bearing when they aren't. Drop lines 12–14.

Suggested change
- 'packages/rs-*/**'
- 'packages/rs-unified-sdk-jni/**'
- 'packages/rs-sdk-ffi/**'
- 'packages/rs-platform-wallet-ffi/**'
- 'packages/rs-*/**'

source: ['claude']

- 'packages/dapi-grpc/**'
- 'packages/data-contracts/**'
- 'packages/*-contract/**'
- 'packages/simple-signer/**'
- '.github/workflows/kotlin-sdk-build.yml'

concurrency:
Expand Down
6 changes: 5 additions & 1 deletion packages/kotlin-sdk/build_android.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ FEATURES=""
if [[ $SHIELDED -eq 1 ]]; then
FEATURES="shielded"
fi
FEATURE_ARGS=()
if [[ -n "$FEATURES" ]]; then
FEATURE_ARGS+=(--features "$FEATURES")
fi

# --- Toolchain checks -------------------------------------------------------

Expand Down Expand Up @@ -148,7 +152,7 @@ done

cargo ndk "${CARGO_NDK_ARGS[@]}" -o "$JNILIBS_DIR" -P "$MIN_SDK" \
build -p "$PACKAGE" --profile "$CARGO_PROFILE" \
${FEATURES:+--features "$FEATURES"} --no-default-features
"${FEATURE_ARGS[@]}" --no-default-features

# cargo-ndk copies every built .so, including the standalone cdylibs the FFI
# dependency crates also produce (librs_sdk_ffi.so, libplatform_wallet_ffi.so,
Expand Down
80 changes: 43 additions & 37 deletions packages/rs-unified-sdk-jni/src/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ where
let ctx = &*(context as *const KotlinEventCtx);
let bridge = ctx.bridge.as_obj();
let env: &mut JNIEnv = &mut env;
if f(env, bridge).is_err() {
if env.with_local_frame(32, |env| f(env, bridge)).is_err() {
let _ = env.exception_clear();
}
}));
Expand Down Expand Up @@ -173,24 +173,27 @@ unsafe extern "C" fn tramp_platform_address_sync_completed(
) {
with_bridge(context, |env, bridge| {
for r in slice_or_empty(results, count) {
let wid = id32(env, &r.wallet_id)?;
let err = cstr_opt(env, r.error_message)?;
env.call_method(
bridge,
"onPlatformAddressSyncCompleted",
"([BZJJJJJJLjava/lang/String;)V",
&[
(&wid).into(),
JValue::Bool(r.success as u8),
JValue::Long(r.found_count as i64),
JValue::Long(r.absent_count as i64),
JValue::Long(r.checkpoint_height as i64),
JValue::Long(r.new_sync_height as i64),
JValue::Long(r.new_sync_timestamp as i64),
JValue::Long(r.last_known_recent_block as i64),
(&err).into(),
],
)?;
env.with_local_frame(16, |env| -> Result<(), jni::errors::Error> {
let wid = id32(env, &r.wallet_id)?;
let err = cstr_opt(env, r.error_message)?;
env.call_method(
bridge,
"onPlatformAddressSyncCompleted",
"([BZJJJJJJLjava/lang/String;)V",
&[
(&wid).into(),
JValue::Bool(r.success as u8),
JValue::Long(r.found_count as i64),
JValue::Long(r.absent_count as i64),
JValue::Long(r.checkpoint_height as i64),
JValue::Long(r.new_sync_height as i64),
JValue::Long(r.new_sync_timestamp as i64),
JValue::Long(r.last_known_recent_block as i64),
(&err).into(),
],
)?;
Ok(())
})?;
}
env.call_method(
bridge,
Expand All @@ -215,24 +218,27 @@ unsafe extern "C" fn tramp_shielded_sync_completed(
) {
with_bridge(context, |env, bridge| {
for r in slice_or_empty(results, count) {
let wid = id32(env, &r.wallet_id)?;
let err = cstr_opt(env, r.error_message)?;
env.call_method(
bridge,
"onShieldedSyncCompleted",
"([BZZZIJIJLjava/lang/String;)V",
&[
(&wid).into(),
JValue::Bool(r.success as u8),
JValue::Bool(r.skipped as u8),
JValue::Bool(r.cooldown_skip as u8),
JValue::Int(r.new_notes as i32),
JValue::Long(r.total_scanned as i64),
JValue::Int(r.newly_spent as i32),
JValue::Long(r.balance as i64),
(&err).into(),
],
)?;
env.with_local_frame(16, |env| -> Result<(), jni::errors::Error> {
let wid = id32(env, &r.wallet_id)?;
let err = cstr_opt(env, r.error_message)?;
env.call_method(
bridge,
"onShieldedSyncCompleted",
"([BZZZIJIJLjava/lang/String;)V",
&[
(&wid).into(),
JValue::Bool(r.success as u8),
JValue::Bool(r.skipped as u8),
JValue::Bool(r.cooldown_skip as u8),
JValue::Int(r.new_notes as i32),
JValue::Long(r.total_scanned as i64),
JValue::Int(r.newly_spent as i32),
JValue::Long(r.balance as i64),
(&err).into(),
],
)?;
Ok(())
})?;
}
env.call_method(
bridge,
Expand Down
25 changes: 14 additions & 11 deletions packages/rs-unified-sdk-jni/src/funding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,17 +395,20 @@ unsafe extern "C" fn seed_pool_progress_trampoline(
};
// SAFETY: `context` is a live GlobalRef pinned for the call duration.
let bridge = &*(context as *const GlobalRef);
let call = env.call_method(
bridge.as_obj(),
"onProgress",
"(JJJJ)V",
&[
jni::objects::JValue::Long(batch_index as i64),
jni::objects::JValue::Long(batches_total_estimate as i64),
jni::objects::JValue::Long(pool_notes_now as i64),
jni::objects::JValue::Long(target as i64),
],
);
let call = env.with_local_frame(8, |env| -> Result<(), jni::errors::Error> {
env.call_method(
bridge.as_obj(),
"onProgress",
"(JJJJ)V",
&[
jni::objects::JValue::Long(batch_index as i64),
jni::objects::JValue::Long(batches_total_estimate as i64),
jni::objects::JValue::Long(pool_notes_now as i64),
jni::objects::JValue::Long(target as i64),
],
)?;
Ok(())
});
if call.is_err() {
let _ = env.exception_clear();
}
Expand Down
84 changes: 40 additions & 44 deletions packages/rs-unified-sdk-jni/src/mnemonic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,55 +51,51 @@ unsafe extern "C" fn resolve_trampoline(
return RESULT_OTHER;
};

let wallet_id = std::slice::from_raw_parts(wallet_id_bytes, 32);
let Ok(jwallet_id) = env.byte_array_from_slice(wallet_id) else {
let _ = env.exception_clear();
return RESULT_OTHER;
};
match env.with_local_frame(16, |env| -> Result<i32, jni::errors::Error> {
let wallet_id = std::slice::from_raw_parts(wallet_id_bytes, 32);
let jwallet_id = env.byte_array_from_slice(wallet_id)?;
let value = env
.call_method(
ctx.bridge.as_obj(),
"resolveMnemonic",
"([B)Ljava/lang/String;",
&[(&jwallet_id).into()],
)?
.l()?;
if value.is_null() {
return Ok(RESULT_NOT_FOUND);
}

let jstr = JString::from(value);
let java_str = env.get_string(&jstr)?;
let mut bytes = java_str.to_bytes().to_vec();
drop(java_str);

let code = if bytes.len() + 1 > out_capacity {
RESULT_BUFFER_TOO_SMALL
} else {
std::ptr::copy_nonoverlapping(
bytes.as_ptr(),
out_mnemonic_utf8 as *mut u8,
bytes.len(),
);
*(out_mnemonic_utf8.add(bytes.len())) = 0;
*out_len = bytes.len();
RESULT_OK
};

let call = env.call_method(
ctx.bridge.as_obj(),
"resolveMnemonic",
"([B)Ljava/lang/String;",
&[(&jwallet_id).into()],
);
let value = match call.and_then(|v| v.l()) {
Ok(obj) => obj,
// Zero the intermediate Rust copy of the phrase. (The JVM String
// itself is garbage-collected — same residual exposure the iOS
// Swift String has.)
bytes.iter_mut().for_each(|b| *b = 0);
Ok(code)
}) {
Ok(code) => code,
Err(_) => {
let _ = env.exception_clear();
return RESULT_OTHER;
RESULT_OTHER
}
};
if value.is_null() {
return RESULT_NOT_FOUND;
}

let jstr = JString::from(value);
let Ok(java_str) = env.get_string(&jstr) else {
let _ = env.exception_clear();
return RESULT_OTHER;
};
let mut bytes = java_str.to_bytes().to_vec();
drop(java_str);

let code = if bytes.len() + 1 > out_capacity {
RESULT_BUFFER_TOO_SMALL
} else {
std::ptr::copy_nonoverlapping(
bytes.as_ptr(),
out_mnemonic_utf8 as *mut u8,
bytes.len(),
);
*(out_mnemonic_utf8.add(bytes.len())) = 0;
*out_len = bytes.len();
RESULT_OK
};

// Zero the intermediate Rust copy of the phrase. (The JVM String
// itself is garbage-collected — same residual exposure the iOS
// Swift String has.)
bytes.iter_mut().for_each(|b| *b = 0);
code
}));
result.unwrap_or(RESULT_OTHER)
}
Expand Down
Loading
Loading