Skip to content

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

Closed
yashbhutwala wants to merge 10 commits into
cashubtc:mainfrom
yashbhutwala:feat/p2pk-key-store-autosign
Closed

feat: P2PK key storage and auto-sign on receive#1053
yashbhutwala wants to merge 10 commits into
cashubtc:mainfrom
yashbhutwala:feat/p2pk-key-store-autosign

Conversation

@yashbhutwala
Copy link
Copy Markdown

@yashbhutwala yashbhutwala commented Sep 8, 2025 •
edited
Loading

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.

@yashbhutwala yashbhutwala changed the title P2PK key storage and auto-sign on receive feat: P2PK key storage and auto-sign on receive Sep 8, 2025
@davidcaseria
Copy link
Copy Markdown
Contributor

davidcaseria commented Sep 9, 2025

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

@yashbhutwala yashbhutwala force-pushed the feat/p2pk-key-store-autosign branch 3 times, most recently from edcae79 to acf5421 Compare September 10, 2025 01:18
@yashbhutwala
Copy link
Copy Markdown
Author

yashbhutwala commented Sep 10, 2025

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.

@yashbhutwala yashbhutwala force-pushed the feat/p2pk-key-store-autosign branch from acf5421 to 4681ce9 Compare September 10, 2025 12:56
thesimplekid
thesimplekid requested changes Sep 14, 2025
View reviewed changes
Comment thread crates/cdk-common/src/database/wallet.rs Outdated
Comment thread crates/cdk-integration-tests/tests/p2pk_autosign.rs Outdated
Comment thread crates/cdk/src/wallet/mod.rs Outdated
Comment thread crates/cdk/src/wallet/receive.rs
@yashbhutwala yashbhutwala force-pushed the feat/p2pk-key-store-autosign branch from 4681ce9 to b043747 Compare September 16, 2025 20:48
@thesimplekid thesimplekid force-pushed the feat/p2pk-key-store-autosign branch from c1aedcb to 1c63195 Compare September 29, 2025 10:41
@thesimplekid
Copy link
Copy Markdown
Collaborator

thesimplekid commented Sep 29, 2025

Fixed error conversion and rebased.

@thesimplekid
chore: remove refs to common
d80a97b
@thesimplekid
Merge main into p2pk-key-store-autosign
8fb735f 
- Resolved conflicts in database.rs by accepting modular structure
- Moved SQLite-specific implementation to sqlite.rs
- Added P2PK methods to sqlite.rs and postgres.rs
- Moved SecretKey and P2pkSigningKey types to types/keys.rs
- Removed duplicate SecretKey from types/wallet.rs
- Fixed imports to use modular types structure
@thesimplekid
feat(cdk-ffi): improve secret key handling with secure memory management
f223737 
- Convert SecretKey from uniffi::Record to uniffi::Object for better encapsulation
- Implement Drop trait to ensure secure cleanup via zeroize
- Wrap secret keys in Arc across FFI boundary for safe sharing
- Update WalletDatabase trait to use Arc<SecretKey> and Arc<P2pkSigningKey>
- Add zeroize to workspace dependencies
- Improve test coverage for secret key conversions and byte handling
@davidcaseria
Copy link
Copy Markdown
Contributor

davidcaseria commented Oct 7, 2025

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
Copy link
Copy Markdown
Collaborator

thesimplekid commented Oct 7, 2025

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.

@thesimplekid thesimplekid modified the milestone: 0.14.0 Nov 12, 2025
@thesimplekid thesimplekid mentioned this pull request Dec 27, 2025
Merged
2 tasks
@thesimplekid
Copy link
Copy Markdown
Collaborator

thesimplekid commented Jan 11, 2026

superseded by #1466

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thesimplekid thesimplekid Awaiting requested review from thesimplekid

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yashbhutwala @davidcaseria @thesimplekid