Skip to content

Conversation

@ajaysehwal
Copy link
Contributor

@ajaysehwal ajaysehwal commented Jan 28, 2026

Description

Relates to issue: ZEUS-3614 & #3614 (review)

Please enter a description and screenshots, if appropriate, of the work covered in this PR

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Configuration change
  • Locales update
  • Quality assurance
  • Other

Checklist

  • I’ve run yarn run tsc and made sure my code compiles correctly
  • I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
  • I’ve run yarn run prettier and made sure my code is formatted correctly
  • I’ve run yarn run test and made sure all of the tests pass

Testing

If you modified or added a utility file, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms (please specify OS version and phone model/VM):

  • Android
  • iOS

I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):

  • Embedded LND
  • LND (REST)
  • LND (Lightning Node Connect)
  • Core Lightning (CLNRest)
  • Nostr Wallet Connect
  • LndHub

Locales

  • I’ve added new locale text that requires translations
  • I’m aware that new translations should be made on the ZEUS Transfix page and not directly to this repo

Third Party Dependencies and Packages

  • Contributors will need to run yarn after this PR is merged in
  • 3rd party dependencies have been modified:
    • verify that package.json and yarn.lock have been properly updated
    • verify that dependencies are installed for both iOS and Android platforms

Other:

  • Changes were made that require an update to the README
  • Changes were made that require an update to onboarding

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @ajaysehwal, 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 enhances the Cashu Dev Kit (CDK) by adding support for P2PK spending conditions, allowing users to lock tokens to a specific public key. This adds a layer of security and control over token spending. The changes include updates to the Android and iOS modules to handle the new conditions, as well as improvements to error handling to provide more informative error messages to the user.

Highlights

  • P2PK Condition Handling: This PR introduces the ability to apply P2PK (Pay to Public Key) spending conditions when sending Cashu tokens. This allows for more control over how tokens can be spent, such as requiring a signature from a specific public key.
  • Android and iOS Updates: The changes include updates to both the Android (CashuDevKitModule.kt) and iOS (CashuDevKitModule.swift) modules to handle the new P2PK spending conditions.
  • Error Handling: The PR adds improved error handling in the CashuStore to provide a more informative error message when a user attempts to claim tokens locked to a specific wallet.
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 correctly implements P2PK spending conditions for Cashu sends, ensuring consistency across both Android and iOS platforms. The changes also include necessary updates to the TypeScript store to prepare the condition data and handle a new error case related to P2PK-locked tokens. My review includes a couple of suggestions to improve code clarity and robustness in the Kotlin implementation and to simplify an error-checking condition in the TypeScript code.

Comment on lines +869 to +906
var includeFee = false
var conditions: SpendingConditions? = null
if (optionsJson != null) {
val parsed = JSONObject(optionsJson)
includeFee = parsed.optBoolean("include_fee", false)

// Parse spending conditions (P2PK) if provided
if (parsed.has("conditions") && !parsed.isNull("conditions")) {
val cond = parsed.getJSONObject("conditions")
val kind = cond.optString("kind")
if (kind == "P2PK" && cond.has("data")) {
val data = cond.getJSONObject("data")
val pubkeyHex = data.getString("pubkey")
val locktime =
if (data.has("locktime") && !data.isNull("locktime"))
data.getLong("locktime").toULong()
else null
val refundKeysJson = data.optJSONArray("refund_keys")
val refundKeys = refundKeysJson?.let { arr ->
(0 until arr.length()).mapNotNull { i ->
try { arr.getString(i) } catch (e: Exception) { null }
}
}

conditions = SpendingConditions.P2pk(
pubkey = pubkeyHex,
conditions = Conditions(
locktime = locktime ?: 0UL,
pubkeys = emptyList(),
refundKeys = refundKeys ?: emptyList(),
numSigs = 0UL,
sigFlag = 0.toUByte(),
numSigsRefund = 0UL
)
)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logic for parsing spending conditions is quite nested and could be made more robust. Using optJSONObject with let blocks would flatten the structure, improve readability, and prevent potential JSONExceptions from malformed optionsJson. This would also align the implementation more closely with the safer, optional-chaining approach used in the Swift counterpart.

                var includeFee = false
                var conditions: SpendingConditions? = null
                optionsJson?.let {
                    val parsed = JSONObject(it)
                    includeFee = parsed.optBoolean("include_fee", false)

                    // Parse spending conditions (P2PK) if provided
                    parsed.optJSONObject("conditions")?.let { cond ->
                        if (cond.optString("kind") != "P2PK") return@let

                        cond.optJSONObject("data")?.let { data ->
                            val pubkeyHex = data.optString("pubkey").takeIf { it.isNotEmpty() } ?: return@let

                            val locktime = if (data.has("locktime") && !data.isNull("locktime")) {
                                data.optLong("locktime").toULong()
                            } else {
                                null
                            }

                            val refundKeys = data.optJSONArray("refund_keys")?.let { arr ->
                                (0 until arr.length()).mapNotNull { i ->
                                    arr.optString(i).takeIf { it.isNotEmpty() }
                                }
                            }

                            conditions = SpendingConditions.P2pk(
                                pubkey = pubkeyHex,
                                conditions = Conditions(
                                    locktime = locktime ?: 0UL,
                                    pubkeys = emptyList(),
                                    refundKeys = refundKeys ?: emptyList(),
                                    numSigs = 0UL,
                                    sigFlag = 0.toUByte(),
                                    numSigsRefund = 0UL
                                )
                            )
                        }
                    }
                }

Copy link
Contributor

Choose a reason for hiding this comment

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

solid suggestion

Comment on lines +2296 to +2251
if (
e?.message?.includes('Witness is missing for p2pk signature') ||
(typeof e?.message === 'string' &&
e.message.toLowerCase().includes('witness is missing') &&
e.message.toLowerCase().includes('p2pk'))
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The condition to check for the 'witness is missing' error is a bit verbose. The second part of the || condition is a more robust, case-insensitive check that makes the first part redundant. Simplifying this will make the code cleaner and easier to maintain.

            if (
                typeof e?.message === 'string' &&
                e.message.toLowerCase().includes('witness is missing') &&
                e.message.toLowerCase().includes('p2pk')
            ) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Solid suggestion

@ajaysehwal ajaysehwal marked this pull request as draft January 28, 2026 09:51
@ajaysehwal ajaysehwal force-pushed the cashu-token-locking branch 4 times, most recently from d3459e1 to 4f7e530 Compare January 28, 2026 10:18
@ajaysehwal ajaysehwal marked this pull request as ready for review January 28, 2026 10:19
@ajaysehwal ajaysehwal mentioned this pull request Jan 28, 2026
28 tasks
@ajaysehwal ajaysehwal force-pushed the cashu-token-locking branch 2 times, most recently from 7003395 to 7ba4b33 Compare January 28, 2026 11:12
this.error_msg = undefined;
});

console.log('mintToken', memo, value, pubkey, lockTime);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this log

data: conditionData
};
}
console.log('conditions', conditions);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove log

const mintUrl = this.selectedMintUrl;

try {
await this.syncCDKBalances();
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to sync balances here?

@kaloudis
Copy link
Contributor

After changes above, be sure to test on Android again at the very least after the native code changes

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