P2pk receive wallet #1466
P2pk receive wallet #1466
Conversation
|
This replaces this right? #1053 |
| // stores p2pk signing keys for the wallet | ||
| async fn add_p2pk_key(&self, pubkey: &PublicKey, derivation_path: String, derivation_index: u32) -> Result<(), Self::Err>; | ||
| /// Get a stored P2PK signing key by pubkey. | ||
| async fn get_p2pk_key(&self, pubkey: &PublicKey) -> Result<Option<wallet::P2PKSigningKey>, Self::Err>; | ||
| /// List all stored P2PK signing keys. | ||
| async fn list_p2pk_keys(&self) -> Result<Vec<wallet::P2PKSigningKey>, Self::Err>; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
yes |
|
@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. |
| pub async fn p2pk_public_key( | ||
| &self, | ||
| pubkey: &PublicKey, | ||
| ) -> Result<Option<P2PKSigningKey>, database::Error> { | ||
| return self.localstore.get_p2pk_key(pubkey).await; | ||
| } |
There was a problem hiding this comment.
Think we should rename this get private key?
There was a problem hiding this comment.
this does not give a private key, so no
There was a problem hiding this comment.
I don't expose right now a way to get the secret key. this is a private method.
I'll take suggestions on naming
There was a problem hiding this comment.
this does not give a private key, so no
get_public_key i meant
There was a problem hiding this comment.
changed the named functions plus added missing ones to ffi
|
|
||
| wallet.melt_human_readable_quote(address, amount_msat).await | ||
| } | ||
| pub async fn generate_public_key(&self) -> Result<PublicKey, Error> { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I don't think it's a bad idea.
There was a problem hiding this comment.
I made a function for getting the latest public key
|
|
||
| proof.sign_p2pk(secret_key.to_owned().clone())?; | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
2 participants
Description
Add public key generation with counter for wallet.
Notes to the reviewers
Suggested CHANGELOG Updates
CHANGED
ADDED
REMOVED
FIXED
Checklist
just final-checkbefore committing