P2pk receive wallet #1466

2025-12-27T10:17:59Z

lescuer97 commented

2025-12-27 10:17:59 +00:00

(Migrated from github.com)

Description

Add public key generation with counter for wallet.

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

I followed the code style guidelines
I ran just final-check before committing

### Description Add public key generation with counter for wallet. ----- ### Notes to the reviewers  ----- ### Suggested [CHANGELOG](https://github.com/cashubtc/cdk/blob/main/CHANGELOG.md) Updates  #### CHANGED #### ADDED #### REMOVED #### FIXED ---- ### Checklist * [ ] I followed the [code style guidelines](https://github.com/cashubtc/cdk/blob/main/CODE_STYLE.md) * [ ] I ran `just final-check` before committing

thesimplekid (Migrated from github.com) reviewed

2025-12-27 10:17:59 +00:00

thesimplekid commented

2025-12-27 22:17:06 +00:00

(Migrated from github.com)

This replaces this right? https://github.com/cashubtc/cdk/pull/1053

thesimplekid (Migrated from github.com) reviewed

2025-12-27 22:19:14 +00:00

crates/cdk-common/src/database/wallet/mod.rs

I think we need to have a discussion and have some guidelines on when we should add new fns to the db traits and when we should take advantage of the KVStore. I think there is a balance between over using the store and making our db traits too complicated.

lescuer97 commented

2025-12-29 09:17:59 +00:00

(Migrated from github.com)

This replaces this right? #1053

yes

> This replaces this right? #1053 yes

lescuer97 (Migrated from github.com) reviewed

2025-12-29 09:37:14 +00:00

crates/cdk-common/src/database/wallet/mod.rs

I think we need to have a discussion and have some guidelines on when we should add new fns to the db traits and when we should take advantage of the KVStore. I think there is a balance between over using the store and making our db traits too complicated.

could you explain the difference between KVStore and the traits? what do you think it should be the parameters on choosing one or the other?

> I think we need to have a discussion and have some guidelines on when we should add new fns to the db traits and when we should take advantage of the KVStore. I think there is a balance between over using the store and making our db traits too complicated. could you explain the difference between KVStore and the traits? what do you think it should be the parameters on choosing one or the other?

thesimplekid (Migrated from github.com) reviewed

2025-12-29 20:12:00 +00:00

crates/cdk-common/src/database/wallet/mod.rs

The kvstore is general so we can just store arbitrary data in it without having to add fns on the trait. With the drawback of a different migration pattern and less structured data. So I'm not necessarily saying we should do it here but there is a discussion worth having about when to use the Kv and when to add new fns and db tables.

lescuer97 (Migrated from github.com) reviewed

2025-12-30 22:09:18 +00:00

crates/cdk-common/src/database/wallet/mod.rs

yeah, I understand. I would use the KVstore when you have data that is clearly independent. EX: Information about a specific npub that you sent money to comes up in my mind

lescuer97 commented

2025-12-30 22:21:39 +00:00

(Migrated from github.com)

@thesimplekid @crodas I think this is know in a decent state. I know I'm lacking tests. I want to get your opinion on the implementation before I do that.

thesimplekid (Migrated from github.com) requested changes

2025-12-31 11:21:08 +00:00

crates/cdk-cli/Cargo.toml

We shouldn't need this should be able to get what we need from cdk

crates/cdk-cli/src/sub_commands/get_public_keys.rs

					
				@ -0,0 +1,70 @@

				use std::str::FromStr;

Lets remove the emojis

crates/cdk-common/src/wallet/mod.rs

This should need doc comments not sure why clippy hasn't caught it

crates/cdk-ffi/src/database.rs

Out of date

crates/cdk-ffi/src/database.rs

out of date

crates/cdk/src/wallet/mod.rs

Outdated

crates/cdk/src/wallet/multi_mint_wallet.rs

Think we should rename this get private key?

These fns should also have doc comments

I wonder if we should have a fn to get the most recent pubkey especially with p2bk the user might want to just reuse the pubkey.

lescuer97 (Migrated from github.com) reviewed

2026-01-01 10:07:19 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

this does not give a private key, so no

lescuer97 (Migrated from github.com) reviewed

2026-01-01 10:07:57 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

I don't think it's a bad idea.

lescuer97 (Migrated from github.com) reviewed

2026-01-01 11:57:45 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

I made a function for getting the latest public key

lescuer97 (Migrated from github.com) reviewed

2026-01-01 11:58:18 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

I don't expose right now a way to get the secret key. this is a private method.

I'll take suggestions on naming

I don't expose right now a way to get the secret key. this is a private method. I'll take suggestions on naming

thesimplekid (Migrated from github.com) reviewed

2026-01-01 12:21:05 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

this does not give a private key, so no

get_public_key i meant

> this does not give a private key, so no get_public_key i meant

thesimplekid (Migrated from github.com) reviewed

2026-01-01 19:22:28 +00:00

crates/cdk-redb/src/wallet/mod.rs

We need to set the time here. we can use unix_time()

We need to set the time here. we can use `unix_time()`

crates/cdk/src/wallet/receive.rs

We don't error if we do not have the required keys to sign. If we cannot get enough sigs for a proof we should not send it to the mint.

lescuer97 (Migrated from github.com) reviewed

2026-01-01 22:09:36 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

changed the named functions plus added missing ones to ffi

lescuer97 (Migrated from github.com) reviewed

2026-01-01 22:13:36 +00:00

crates/cdk/src/wallet/receive.rs

I added an enough_signatures function to the Proof struct that will check. Needed to add a new error to the Nut 10 file.

I'm getting a typo error from a meeting agenda. I assume this is a mistake

I added an `enough_signatures` function to the Proof struct that will check. Needed to add a new error to the Nut 10 file. I'm getting a typo error from a `meeting agenda`. I assume this is a mistake

thesimplekid (Migrated from github.com) reviewed

2026-01-06 13:45:26 +00:00

crates/cdk/src/wallet/receive.rs

I'm getting a typo error from a meeting agenda. I assume this is a mistake

Rebase will solve this

> I'm getting a typo error from a meeting agenda. I assume this is a mistake Rebase will solve this

thesimplekid (Migrated from github.com) reviewed

2026-01-08 22:19:23 +00:00

thesimplekid (Migrated from github.com) left a comment

Thanks just a few comments.

crates/cashu/src/nuts/nut00/mod.rs

We already have a verify_p2pk() fn that we use for the mint. I think we can just reuse that and remove this fn?

We already have a `verify_p2pk()` fn that we use for the mint. I think we can just reuse that and remove this fn?

crates/cdk-cli/src/sub_commands/get_public_keys.rs

					
				@ -0,0 +1,70 @@

				use std::str::FromStr;

There is a get_latest_pubke fn on the MultiMintWallet we should use that if the latest arg is passed.

There is a `get_latest_pubke` fn on the MultiMintWallet we should use that if the latest arg is passed.

We don't need the Ok import here.

We don't need the `Ok` import here.

crates/cdk-cli/src/sub_commands/get_public_keys.rs

					
				@ -0,0 +62,4 @@

				        println!("\npublic not found!\n");

				    }

				    println!("\npublic keys found:\n");

				    for public_key in list_public_keys {

We should check if the list is empty here and print no keys found if so.

crates/cdk-redb/src/wallet/mod.rs

```suggestion ```

crates/cdk-sql-common/src/wallet/mod.rs

This is missing the created time.

We should add some db tests for the new functions that would have caught this.

This is missing the created time. We should add some db tests for the new functions that would have caught this.

crates/cdk/src/wallet/multi_mint_wallet.rs

We should have a purpose identifier. We could reuse the cashu purpose of 129372' and then use 1 for the next where on secrets we use 0.

Why is this only here on the multi-mint wallet. Shouldn't it be on the Wallet and then the Multi-mint wallet can call that. Generally this is how we use the multi-mint wallet where it is just a dumb wrapper around the Wallet.

Why is this only here on the multi-mint wallet. Shouldn't it be on the `Wallet` and then the Multi-mint wallet can call that. Generally this is how we use the multi-mint wallet where it is just a dumb wrapper around the Wallet.

These fns should be on the Wallet as well. The `Wallet~ needs to be able to usable without the multi mint wallet.

These fns should be on the `Wallet` as well. The `Wallet~ needs to be able to usable without the multi mint wallet.

thesimplekid (Migrated from github.com) reviewed

2026-01-09 10:00:48 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

I thought about this a bit more and we should not reuse the cashu purpose. We should only use that where its spec'd in the nuts which this is not.

lescuer97 (Migrated from github.com) reviewed

2026-01-09 10:32:38 +00:00

crates/cdk-sql-common/src/wallet/mod.rs

yeah, I had a default param in the migration, that is why I had not caught it. I will add some tests

lescuer97 (Migrated from github.com) reviewed

2026-01-09 10:33:10 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

I used the new 1 as we said with the cashu purpose.

thesimplekid (Migrated from github.com) reviewed

2026-01-09 10:43:24 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

I used the new 1 as we said with the cashu purpose.

I think was a bad idea from me. What if something else gets added to the spec using that purpose, we would then conflict.

> I used the new 1 as we said with the cashu purpose. I think was a bad idea from me. What if something else gets added to the spec using that purpose, we would then conflict.

lescuer97 (Migrated from github.com) reviewed

2026-01-09 10:58:06 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

I might have a miss understanding on how this works. I didn't want to put the functions inside the Wallet mostly because it would mean that the keys are dependent of each mint.
Should I just allow for both?

I might have a miss understanding on how this works. I didn't want to put the functions inside the Wallet mostly because it would mean that the keys are dependent of each mint. Should I just allow for both?

thesimplekid (Migrated from github.com) reviewed

2026-01-09 11:30:16 +00:00

crates/cdk/src/wallet/multi_mint_wallet.rs

The keys won't really rely on the wallet since there is nothing in the derivation path linking them to a mint. But yeah we need the fns on the Wallet so can just make this on both the Wallet and MultimintWallet. The logic can be in a shared helper fn.

The keys won't really rely on the wallet since there is nothing in the derivation path linking them to a mint. But yeah we need the fns on the `Wallet` so can just make this on both the `Wallet` and `MultimintWallet`. The logic can be in a shared helper fn.

lescuer97 commented

2026-01-14 13:08:44 +00:00

(Migrated from github.com)

@thesimplekid rebased

thesimplekid commented

2026-01-15 10:02:22 +00:00

(Migrated from github.com)

ACK 4093b6c014de5612b2d2497d909d74ef47c3b8bc

thesimplekid (Migrated from github.com) approved these changes

2026-01-15 17:54:27 +00:00

thesimplekid (Migrated from github.com) requested changes

2026-01-15 17:56:11 +00:00

crates/cdk/src/wallet/p2pk.rs

					
				@ -0,0 +33,4 @@

				        ChildNumber::from_hardened_idx(0)?,

				        ChildNumber::from_hardened_idx(0)?,

				        ChildNumber::from_normal_idx(last_derivation_index)?,

				    ]);

Shouldn't we still have a purpose here even though we said we shouldnt reuse the cashu one?

lescuer97 (Migrated from github.com) reviewed

2026-01-16 13:43:26 +00:00

crates/cdk/src/wallet/p2pk.rs

					
				@ -0,0 +33,4 @@

				        ChildNumber::from_hardened_idx(0)?,

				        ChildNumber::from_hardened_idx(0)?,

				        ChildNumber::from_normal_idx(last_derivation_index)?,

				    ]);

yeah we probably should

lescuer97 (Migrated from github.com) reviewed

2026-01-16 14:15:46 +00:00

crates/cdk/src/wallet/p2pk.rs

					
				@ -0,0 +33,4 @@

				        ChildNumber::from_hardened_idx(0)?,

				        ChildNumber::from_hardened_idx(0)?,

				        ChildNumber::from_normal_idx(last_derivation_index)?,

				    ]);

Added a purpose and solved conflicts

lescuer97 commented

2026-01-19 14:48:18 +00:00

(Migrated from github.com)

this commit: https://github.com/cashubtc/cdk/pull/1466/commits/57a072b4d6b3bc468cc7c8f9078929ebadce3359 changes the derivation to in accordance to https://github.com/cashubtc/nuts/pull/331.

plus adds tests

this commit: https://github.com/cashubtc/cdk/pull/1466/commits/57a072b4d6b3bc468cc7c8f9078929ebadce3359 changes the derivation to in accordance to https://github.com/cashubtc/nuts/pull/331. plus adds tests

👍 1

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin lescuer97/p2pk_receive_wallet:lescuer97/p2pk_receive_wallet

git switch lescuer97/p2pk_receive_wallet

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff lescuer97/p2pk_receive_wallet

git switch lescuer97/p2pk_receive_wallet

git rebase main

git switch main

git merge --ff-only lescuer97/p2pk_receive_wallet

git switch lescuer97/p2pk_receive_wallet

git rebase main

git switch main

git merge --no-ff lescuer97/p2pk_receive_wallet

git switch main

git merge --squash lescuer97/p2pk_receive_wallet

git switch main

git merge --ff-only lescuer97/p2pk_receive_wallet

git switch main

git merge lescuer97/p2pk_receive_wallet

git push origin main

Sign in to join this conversation.

No reviewers

thesimplekid

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

cashubtc/cdk!1466

				`@ -0,0 +1,70 @@`
				`use std::str::FromStr;`

				`@ -0,0 +1,70 @@`
				`use std::str::FromStr;`

Rows
Columns