Skip to content
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

Replace Configuration with RequestBuilder to replace PjUri::create_pj_request #19

Closed
DanGould opened this issue Jan 13, 2023 · 2 comments · Fixed by #106
Closed

Replace Configuration with RequestBuilder to replace PjUri::create_pj_request #19

DanGould opened this issue Jan 13, 2023 · 2 comments · Fixed by #106
Labels
api receive receiving payjoin
Milestone

Comments

@DanGould
Copy link
Contributor

          If this configuration is going to hold this data then yes, I think it makes sense to be part of this. The idea of a builder to expose this interface in an accessible way rather than expose `optional_parameters::Params` as pub makes sense to me.

I'm starting to think of this Configuration struct more as a sender::RequestBuilder that can turn into a (Request, Context) when you call from_psbt_and_uri. Maybe the signature for that method should be

impl RequestBuilder {
    fn create_pj_request(psbt: Psbt, pj_url: Uri<PayJoin>) -> Result<(Request, Context)>;
}

instead of on the manually-validated Uri<PayJoinParams> how it is now: pj_url.create_pj_request(psbt, pj_params: Configuration) -> Result<(Request, Context)>.

Then create_pj_request could validate the endpoint check_pj_supported() during that call instead of requiring it be done by hand before the call, too.

Originally posted by @DanGould in #18 (comment)

@DanGould
Copy link
Contributor Author

DanGould commented Apr 15, 2023

Right now this crate re-exports bip21 types. I think these could be hidden by using a builder, accepting only a string and reporting an error for bip21 without payjoin support.

@DanGould DanGould added send sending payjoin receive receiving payjoin and removed send sending payjoin labels May 27, 2023
@DanGould DanGould added the api label Jul 7, 2023
@DanGould DanGould added this to the 1.0 milestone Jul 31, 2023
@DanGould
Copy link
Contributor Author

This work would also simplify the api for v2 in #101. I'd like to return impl send::Context to support both versions' contexts but can't within yet another trait impl for PjUri, but I could if this were an impl that behaved like RequestBuilder::build() -> (Request, impl send::Context)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api receive receiving payjoin
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant