cashubtc/cdk

Fork 0

feat: P2PK key storage and auto-sign on receive #1053

Closed

yashbhutwala wants to merge 10 commits from feat/p2pk-key-store-autosign into main

yashbhutwala commented

2025-09-08 16:10:23 +00:00

(Migrated from github.com)

Implements #1029

Add wallet DB trait for P2PK key storage (add/get/list/remove)
SQL-backed storage + migrations (sqlite/postgres)
Auto-merge stored keys with options to sign proofs on receive
Wallet helpers to add/generate/list P2PK signing keys

Followed development guide: formatted, typos, clippy; validated pure tests.

Implements #1029 - Add wallet DB trait for P2PK key storage (add/get/list/remove) - SQL-backed storage + migrations (sqlite/postgres) - Auto-merge stored keys with options to sign proofs on receive - Wallet helpers to add/generate/list P2PK signing keys Followed development guide: formatted, typos, clippy; validated pure tests.

thesimplekid (Migrated from github.com) reviewed

2025-09-08 16:10:23 +00:00

davidcaseria commented

2025-09-09 14:53:14 +00:00

(Migrated from github.com)

Should the wallet use the seed to make deterministic secrets instead of randomly generated secret keys (e.g., using BIP32)?

yashbhutwala commented

2025-09-10 01:20:00 +00:00

(Migrated from github.com)

Should the wallet use the seed to make deterministic secrets instead of randomly generated secret keys (e.g., using BIP32)?

Great question — and I think we’re talking about two different classes of secrets:

Ephemeral blinding secrets (used when minting proofs): These are intentionally one-time, unlinkable values. Making them deterministic from a seed doesn’t buy us recoverability (Cashu is bearer; you need the proofs, not a seed, to restore funds) and introduces coordination/duplication risks across devices. For these, we should keep using high-entropy randomness.
Long‑lived keys (e.g., P2PK lock keys, auth keys, or anything we must be able to re-derive): These should indeed come from the wallet seed. Using hardened derivation via BIP32/SLIP‑0010 is appropriate here so users can restore keys across devices.

In this PR, the “secret keys” in question are of the first type, so I kept them random by design.

> Should the wallet use the seed to make deterministic secrets instead of randomly generated secret keys (e.g., using BIP32)? Great question — and I think we’re talking about two different classes of secrets: - Ephemeral blinding secrets (used when minting proofs): These are intentionally one-time, unlinkable values. Making them deterministic from a seed doesn’t buy us recoverability (Cashu is bearer; you need the proofs, not a seed, to restore funds) and introduces coordination/duplication risks across devices. For these, we should keep using high-entropy randomness. - Long‑lived keys (e.g., P2PK lock keys, auth keys, or anything we must be able to re-derive): These should indeed come from the wallet seed. Using hardened derivation via BIP32/SLIP‑0010 is appropriate here so users can restore keys across devices. In this PR, the “secret keys” in question are of the first type, so I kept them random by design.

thesimplekid (Migrated from github.com) requested changes

2025-09-14 19:43:25 +00:00

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

I don't think these should be no ops especially ones that do not return errors we should force db backends to support this.

crates/cdk/src/wallet/mod.rs

We store the pubkey and secret key, why don't we return both?

crates/cdk/src/wallet/receive.rs

It seems to me the effect of this is for each receive even ones where no proof needs a signature we get all secret keys from the db and create a list of them, this seem unnecessary. Why don't we iter over the proofs get all pubkeys for those and then get the secret keys for only those pubkeys.

yashbhutwala (Migrated from github.com) reviewed

2025-09-16 20:49:47 +00:00

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

Updated the trait so these methods are mandatory (no default no-ops) and wired them through every wallet backend. The SQL store keeps its existing table, Redb has a v5 migration to create p2pk_signing_keys, and the FFI bridge now exposes the concrete impls, so all backends persist and surface P2PK keys consistently.

yashbhutwala (Migrated from github.com) reviewed

2025-09-16 20:50:11 +00:00

yashbhutwala (Migrated from github.com) reviewed

2025-09-16 20:50:34 +00:00

crates/cdk/src/wallet/mod.rs

Switched list_p2pk_signing_keys (and the underlying db APIs) to return (pubkey, secret) pairs. Callers now get the stored public key alongside the secret so they can correlate entries without re-deriving.

yashbhutwala (Migrated from github.com) reviewed

2025-09-16 20:51:11 +00:00

crates/cdk/src/wallet/receive.rs

Reworked the merge logic: we seed the map with any keys provided in ReceiveOptions, then, as we inspect each proof, we only call get_p2pk_key for the pubkeys actually referenced (track misses to avoid repeat queries). No more list_p2pk_keys() scan on every receive.

thesimplekid commented

2025-09-29 10:41:58 +00:00

(Migrated from github.com)

Fixed error conversion and rebased.

davidcaseria commented

2025-10-07 20:47:19 +00:00

(Migrated from github.com)

Should there be a wallet function to create a p2pk payment request that generates and saves the key as a convenience method so users don't have to remember to save it themselves?

thesimplekid commented

2025-10-07 21:51:55 +00:00

(Migrated from github.com)

Should there be a wallet function to create a p2pk payment request that generates and saves the key as a convenience method so users don't have to remember to save it themselves?

Yes that's why I haven't merged. I don't think user should have to generate a secret key like they do now; should call a fn as you suggest.

> Should there be a wallet function to create a p2pk payment request that generates and saves the key as a convenience method so users don't have to remember to save it themselves? Yes that's why I haven't merged. I don't think user should have to generate a secret key like they do now; should call a fn as you suggest.

👍 1

thesimplekid commented

2026-01-11 12:18:56 +00:00

(Migrated from github.com)

superseded by https://github.com/cashubtc/cdk/pull/1466