-
Notifications
You must be signed in to change notification settings - Fork 76
Payjoin-cli should cache ohttp-keys for re-use #1035
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
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 19678524663Details
💛 - Coveralls |
27c936f to
8512a28
Compare
0b94117 to
3744b03
Compare
| relay_manager: Arc<Mutex<RelayManager>>, | ||
| ) -> Result<ValidatedOhttpKeys> { | ||
| println!("before first some"); | ||
| if let Some(ohttp_keys) = config.v2()?.ohttp_keys.clone() { |
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.
oops! , println statements i used for debugging got commited accidentally , cleaning it up
3744b03 to
0fe2229
Compare
|
@zealsham I am going to mark this as draft. Do you mind cleaning up the commit messages and history and re-opening. Thank you. |
d664164 to
971c8d3
Compare
This pr adds the functionality of ohttp-keys catching to payjoin-cli , ohttp-keys should not be fetched each time and a cached key should be use. Keys expire in 6 months .
971c8d3 to
2198e3f
Compare
|
@arminsabouri the PR is ready |
|
|
||
| // try cache for this selected relay first | ||
| if let Some(cached) = read_cached_ohttp_keys(&selected_relay) { | ||
| println!("using Cached keys for relay: {selected_relay}"); |
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.
Can we use the logger instead of println?
|
|
||
| // try cache for this selected relay first | ||
| if let Some(cached) = read_cached_ohttp_keys(&selected_relay) { | ||
| println!("using Cached keys for relay: {selected_relay}"); |
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.
| println!("using Cached keys for relay: {selected_relay}"); | |
| println!("using Cached keys for relay: {selected_relay}"); |
| // try cache for this selected relay first | ||
| if let Some(cached) = read_cached_ohttp_keys(&selected_relay) { | ||
| println!("using Cached keys for relay: {selected_relay}"); | ||
| if !is_expired(&cached) && cached.relay_url == selected_relay { |
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.
Why would read_cached_ohttp_keys return an expired or keys for a different relay? Perphaps read_cached_ohttp_keys should return ValidateOhttpKeys?
| fetched_at: u64, | ||
| } | ||
|
|
||
| fn get_cache_file(relay_url: &url::Url) -> PathBuf { |
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.
Why are cached keys not being persisted in the database?
This pr adds the functionality of ohttp-keys catching to payjoin-cli , ohttp-keys should not be fetched each time and a cached key should be use. Keys expire in 6 months .
To do
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.