Skip to content

Conversation

@zealsham
Copy link
Collaborator

@zealsham zealsham commented Sep 3, 2025

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

  • it shoudnt still use cached key when the relay-url isnt even present in config or as command-line flag
  • should handle a case of non-expired key but key being rejected by relay (will be handled in different PR)
  • get rid of unwrap calls that can panic (unhandled)
  • should not use the cached keys if --ohttp-keys flag is present cli command
  • it should be possible to cache multiple ohttp-keys based on relay-url (maybe in subsequent pr ?)

Pull Request Checklist

Please confirm the following before requesting review:

@zealsham zealsham marked this pull request as draft September 3, 2025 00:10
@coveralls
Copy link
Collaborator

coveralls commented Sep 3, 2025

Pull Request Test Coverage Report for Build 19678524663

Details

  • 50 of 54 (92.59%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 83.578%

Changes Missing Coverage Covered Lines Changed/Added Lines %
payjoin-cli/src/app/v2/ohttp.rs 50 54 92.59%
Totals Coverage Status
Change from base Build 19649551708: 0.04%
Covered Lines: 9034
Relevant Lines: 10809

💛 - Coveralls

@DanGould DanGould added payjoin-cli enhancement New feature or request labels Sep 3, 2025
@zealsham zealsham changed the title Payjoin-cli should cache ohttp-keys for re-use WIP: Payjoin-cli should cache ohttp-keys for re-use Sep 14, 2025
@zealsham zealsham changed the title WIP: Payjoin-cli should cache ohttp-keys for re-use Payjoin-cli should cache ohttp-keys for re-use Sep 24, 2025
@zealsham zealsham marked this pull request as ready for review September 24, 2025 13:47
relay_manager: Arc<Mutex<RelayManager>>,
) -> Result<ValidatedOhttpKeys> {
println!("before first some");
if let Some(ohttp_keys) = config.v2()?.ohttp_keys.clone() {
Copy link
Collaborator Author

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

@arminsabouri
Copy link
Collaborator

@zealsham I am going to mark this as draft. Do you mind cleaning up the commit messages and history and re-opening. Thank you.

@arminsabouri arminsabouri marked this pull request as draft November 24, 2025 16:35
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 .
@zealsham zealsham marked this pull request as ready for review November 25, 2025 18:40
@zealsham
Copy link
Collaborator Author

@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}");
Copy link
Collaborator

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}");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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 {
Copy link
Collaborator

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

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?

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

Labels

enhancement New feature or request payjoin-cli

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants