Skip to content

Conversation

@niteshbalusu11
Copy link
Member

  • add JNI wrappers for wallet ops + extra methods (maintenanceRefresh, tryClaimLightningReceive, offboardAll, peakKeyPair, verifyMessage, bolt11Invoice) and log config use - add Kotlin facade (NitroArkNative) and example RN module to surface these native calls - wire Android demo buttons in example app to exercise the JNI path with config passthrough and logging

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @niteshbalusu11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the Android integration of the NitroArk library by providing direct access to its core C++/Rust functionalities via JNI and a Kotlin facade. This allows Android developers to interact with critical wallet operations natively, potentially improving performance and offering a more direct integration path. The changes also include a comprehensive demonstration within the example application, showcasing how to utilize these new native capabilities.

Highlights

  • JNI Wrappers for Core Wallet Operations: New JNI (Java Native Interface) wrappers have been added in C++ to expose several core wallet operations from the underlying bark_cxx library. These include maintenanceRefresh, tryClaimLightningReceive, offboardAll, peakKeyPair, verifyMessage, and bolt11Invoice, along with logging for configuration usage.
  • Kotlin Facade for Android Integration: A new Kotlin facade, NitroArkNative, has been introduced. This object provides a clean, idiomatic Kotlin interface to interact with the C++ JNI helpers, allowing Android-only consumers to bypass the JavaScript layer for direct native calls.
  • React Native Demo Module: An example React Native module (NitroArkDemoModule) has been created to surface these new native Kotlin calls to the JavaScript environment. This module acts as a bridge, enabling the React Native application to invoke the newly exposed JNI functionalities.
  • Android Example App Integration: The Android example application has been updated with new UI buttons. These buttons are wired to exercise the new JNI path, demonstrating the direct interaction with the native wallet operations, including configuration passthrough and logging.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully exposes core JNI helpers to Kotlin and provides a demonstration through an Android example. The new C++ JNI layer, Kotlin facade, and React Native demo module are well-structured. My review focuses on improving the robustness of the JNI layer and the maintainability of the example app. Specifically, I've identified a critical issue with JSON serialization that could lead to invalid data, a performance optimization for JNI calls, and an opportunity to reduce code duplication in the TypeScript example. Addressing these points will enhance the quality and reliability of the implementation.

@niteshbalusu11
Copy link
Member Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully exposes core JNI helpers to Kotlin and adds a demonstration in the Android example app. The Kotlin and TypeScript changes are well-implemented. However, the C++ JNI code introduces several critical issues related to memory safety and resource management. These include potential dangling pointers, a null pointer dereference vulnerability, and JNI local reference leaks that could lead to application crashes. These critical and high-severity issues in NitroArkJni.cpp must be addressed before merging.

@hsjoberg hsjoberg self-assigned this Nov 20, 2025
- add JNI wrappers for wallet ops + extra methods (maintenanceRefresh,
tryClaimLightningReceive, offboardAll, peakKeyPair, verifyMessage,
bolt11Invoice) and log config use - add Kotlin facade (NitroArkNative)
and example RN module to surface these native calls - wire Android demo
buttons in example app to exercise the JNI path with config passthrough
and logging
@niteshbalusu11 niteshbalusu11 force-pushed the expose-core-methods-to-kotlin branch from 1e2d492 to 0043168 Compare November 22, 2025 14:48
@niteshbalusu11
Copy link
Member Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully exposes core JNI helpers to Kotlin and demonstrates their use in the Android example app. The new C++ JNI layer, Kotlin facade, and React Native module are well-structured. My review focuses on critical JNI correctness issues, including potential memory leaks and dangling pointers that could lead to crashes. I've also included suggestions for performance optimization by caching JNI lookups, and improving code maintainability by reducing duplication and ensuring consistent naming. Addressing these points will significantly improve the robustness and performance of the native integration.

@niteshbalusu11
Copy link
Member Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully exposes core JNI helpers to Kotlin and demonstrates their use via an Android example. The changes are comprehensive, including updates to the Android build script, a new C++ JNI layer, a Kotlin facade, and an example React Native module. My review focuses on improving the maintainability of the build script and addressing several correctness, performance, and resource management issues within the new C++ JNI implementation. Key feedback includes refactoring duplicated logic in the build script, correcting potential JNI local reference leaks, suggesting performance optimizations for JNI calls, and flagging a critical issue with the use of thread_local that could lead to bugs.

@niteshbalusu11
Copy link
Member Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully exposes core JNI helpers to Kotlin, providing a native Android interface for wallet operations, and includes a comprehensive example app to demonstrate this new functionality. The changes are well-structured, adding support for the armeabi-v7a architecture and creating a clean Kotlin facade.

My review focuses on improving the robustness and maintainability of the new code. I've identified a critical issue in the C++ JNI layer related to a dangling pointer in logging, and a few high-severity issues concerning risky pointer usage and improper JNI exception handling. Additionally, I've suggested refactoring opportunities in the Android build script to reduce code duplication. These changes will enhance the stability and long-term maintainability of the native code.

Comment on lines 282 to 288
__android_log_print(ANDROID_LOG_INFO, LOG_TAG,
"load_wallet(native) datadir=%s regtest=%s signet=%s bitcoin=%s birthday_height=%s ark=%s "
"esplora=%s bitcoind=%s",
datadir.c_str(), opts.regtest ? "true" : "false", opts.signet ? "true" : "false",
opts.bitcoin ? "true" : "false",
opts.birthday_height != nullptr ? std::to_string(*opts.birthday_height).c_str() : "null",
config.ark.c_str(), config.esplora.c_str(), config.bitcoind.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

There is a potential dangling pointer issue here. std::to_string(*opts.birthday_height) creates a temporary std::string object. Calling .c_str() on it gives a pointer to its internal buffer. This temporary string is destroyed at the end of the statement (at the semicolon), leaving a dangling pointer that __android_log_print might try to access, leading to undefined behavior.

To fix this, you should store the result of std::to_string in a variable that lives until after __android_log_print is called.

    const std::string birthday_height_str =
        opts.birthday_height != nullptr ? std::to_string(*opts.birthday_height) : "null";
    __android_log_print(ANDROID_LOG_INFO, LOG_TAG,
                        "load_wallet(native) datadir=%s regtest=%s signet=%s bitcoin=%s birthday_height=%s ark=%s "
                        "esplora=%s bitcoind=%s",
                        datadir.c_str(), opts.regtest ? "true" : "false", opts.signet ? "true" : "false",
                        opts.bitcoin ? "true" : "false", birthday_height_str.c_str(),
                        config.ark.c_str(), config.esplora.c_str(), config.bitcoind.c_str());

Copy link
Member Author

Choose a reason for hiding this comment

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

LOL

Screenshot 2025-11-24 at 9 48 45 PM

Copy link
Member

@hsjoberg hsjoberg left a comment

Choose a reason for hiding this comment

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

Looks good. Too much JNI stuff to review accurate behavior but I guess we will see in practice how it goes

@niteshbalusu11 niteshbalusu11 merged commit 286aaa0 into master Nov 26, 2025
4 checks passed
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