-
Notifications
You must be signed in to change notification settings - Fork 77
Add receiver pk to session metadata #995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,26 +104,31 @@ impl AppTrait for App { | |
| Ok(()) | ||
| } | ||
| PjParam::V2(pj_param) => { | ||
| // TODO: perhaps we should store pj uri in the session wrapper as to not replay the event log for each session | ||
| let receiver_pubkey = pj_param.receiver_pubkey(); | ||
| let sender_state = | ||
| self.db.get_send_session_ids()?.into_iter().find_map(|session_id| { | ||
| let sender_persister = | ||
| SenderPersister::from_id(self.db.clone(), session_id).ok()?; | ||
| let (send_session, session_history) = | ||
| replay_sender_event_log(&sender_persister) | ||
| let session_receiver_pubkey = self | ||
| .db | ||
| .get_send_session_receiver_pk(&session_id) | ||
| .expect("Receiver pubkey should exist if session id exists"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This expect seems OK since it's looking for exactly one session to send. The other one seems less ok because the context does not restrict it to exactly one use. |
||
| if session_receiver_pubkey == *receiver_pubkey { | ||
| let sender_persister = | ||
| SenderPersister::from_id(self.db.clone(), session_id).ok()?; | ||
| let (send_session, _) = replay_sender_event_log(&sender_persister) | ||
| .map_err(|e| anyhow!("Failed to replay sender event log: {:?}", e)) | ||
| .ok()?; | ||
|
|
||
| let pj_uri = session_history.pj_param().map(|pj_param| pj_param.endpoint()); | ||
| let sender_state = | ||
| pj_uri.filter(|uri| uri == &pj_param.endpoint()).map(|_| send_session); | ||
| sender_state.map(|sender_state| (sender_state, sender_persister)) | ||
| Some((send_session, sender_persister)) | ||
| } else { | ||
| None | ||
| } | ||
| }); | ||
|
|
||
| let (sender_state, persister) = match sender_state { | ||
| Some((sender_state, persister)) => (sender_state, persister), | ||
| None => { | ||
| let persister = SenderPersister::new(self.db.clone())?; | ||
| let persister = | ||
| SenderPersister::new(self.db.clone(), receiver_pubkey.clone())?; | ||
| let psbt = self.create_original_psbt(&address, amount, fee_rate)?; | ||
| let sender = | ||
| SenderBuilder::from_parts(psbt, pj_param, &address, Some(amount)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ use std::sync::Arc; | |
| use payjoin::persist::SessionPersister; | ||
| use payjoin::receive::v2::SessionEvent as ReceiverSessionEvent; | ||
| use payjoin::send::v2::SessionEvent as SenderSessionEvent; | ||
| use payjoin::HpkePublicKey; | ||
| use rusqlite::params; | ||
|
|
||
| use super::*; | ||
|
|
@@ -22,13 +23,13 @@ pub(crate) struct SenderPersister { | |
| } | ||
|
|
||
| impl SenderPersister { | ||
| pub fn new(db: Arc<Database>) -> crate::db::Result<Self> { | ||
| pub fn new(db: Arc<Database>, receiver_pubkey: HpkePublicKey) -> crate::db::Result<Self> { | ||
| let conn = db.get_connection()?; | ||
|
|
||
| // Create a new session in send_sessions and get its ID | ||
| let session_id: i64 = conn.query_row( | ||
| "INSERT INTO send_sessions (session_id) VALUES (NULL) RETURNING session_id", | ||
| [], | ||
| "INSERT INTO send_sessions (session_id, receiver_pubkey) VALUES (NULL, ?1) RETURNING session_id", | ||
| params![receiver_pubkey.to_compressed_bytes()], | ||
| |row| row.get(0), | ||
| )?; | ||
|
|
||
|
|
@@ -217,4 +218,15 @@ impl Database { | |
|
|
||
| Ok(session_ids) | ||
| } | ||
|
|
||
| pub(crate) fn get_send_session_receiver_pk( | ||
| &self, | ||
| session_id: &SessionId, | ||
| ) -> Result<HpkePublicKey> { | ||
| let conn = self.get_connection()?; | ||
| let mut stmt = | ||
| conn.prepare("SELECT receiver_pubkey FROM send_sessions WHERE session_id = ?1")?; | ||
| let receiver_pubkey: Vec<u8> = stmt.query_row(params![session_id.0], |row| row.get(0))?; | ||
| Ok(HpkePublicKey::from_compressed_bytes(&receiver_pubkey).expect("Valid receiver pubkey")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a Or do we just need a new variant? I can see how this wouldn't be the end of the world to panic because there's nothing else to do as long as get_send_session_receiver_pk is only being used in the one place it currently is
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I was kinda ambivalent about this expect. If its truly undeserializable you got bigger problems and a panic is appropriate. But we don't really do that anywhere else in db.rs. db::Error::Deserialize makes the most sense if rust is not mad at the internal type
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ack and merging this is tiny techdebt outside of payjoin crate. |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't it be simpler to add a variant of
get_sens_session_idsthat can use the expected RK in a WHERE clause?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it would
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR was written with sled in mind and I totally forgot I could do this after I rebased.