Refresh cached OHTTP keys sooner and on session build failure - #77
Conversation
b4b3668 to
1db06f6
Compare
1db06f6 to
6f9629e
Compare
ValeraFinebits
left a comment
There was a problem hiding this comment.
Thanks for addressing stale OHTTP keys. Reducing the TTL to one hour does bound the stale-key window, but I don’t think it, together with invalidation around InitializeSession, addresses the underlying lifecycle issue. A stale but structurally valid key should still initialize locally and only be rejected when communicating with the directory, for example through an OHTTP-key HTTP 422 response.
The server-side design being developed in rust-payjoin #1449, and the review direction around #1035, point toward directory-controlled freshness through Cache-Control metadata (currently s-maxage) plus a rotation overlap/grace period. Since the keys belong to the directory rather than the relay, the client should use the remaining freshness lifetime returned with the currently advertised key, not start a new fixed three-month or one-hour TTL when the plugin fetches it.
Could we either keep fetching keys for each new session until this metadata is exposed through the API/FFI, or follow that contract once it becomes available? If a specific OHTTP-key 422 is exposed through the FFI.
6f9629e to
455c260
Compare
Directory OHTTP keys can rotate, and the twelve-hour cache had no refresh path: after a rotation every new session build failed until the window lapsed, falling back to plain BIP21 the whole time. The cache lifetime is now one hour, and a session build failing with a library error drops the cached keys for that relay and store so the next attempt starts from freshly fetched material. Existing sessions are unaffected either way since their keys are baked into the session context.
455c260 to
66af449
Compare
|
This probably needs to be reworked, a lot moved since it I last looked at it, but the core is still valid for the most part, I think |
Thanks for the update! Agreed the core idea still holds, bounding the stale-key window with a shorter TTL plus invalidation on session build failure makes sense. Before reworking, note that we have #90 open specifically to assess the existing OHTTP key cache (lifetime, persistence, invalidation) against payjoin/rust-payjoin#1035. It would be great to:
If parts of #1035 upstream suggest a different strategy (e.g. persistence instead of a shorter TTL), let's capture that reasoning in #90 rather than only in this PR. |
|
Closing this. The concern holds, but the mechanism needs some thought, and the hook in this PR watches the wrong event. Continuing in #90 and will restart from there. |
Directory OHTTP keys can rotate, and the 12-hour cache had no refresh path: after a rotation, every new session build against the stale keys fails and checkout silently falls back to plain BIP21 until the cache window lapses.
Two small changes bound that window:
Existing sessions are unaffected either way — their OHTTP key material is baked into the persisted session context at build time, which is also why creation time is the only useful refresh hook.
🤖 Generated with Claude Code