-
Notifications
You must be signed in to change notification settings - Fork 11
Implement support for submitting presigned requests #64
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: main
Are you sure you want to change the base?
Conversation
6b0f5ba to
9a20ac1
Compare
Adds process_activity_with_stamp & process_request_with_stamp to TurnkeyClient that submit requests using a provided stamp
9a20ac1 to
325b79c
Compare
r-n-o
left a comment
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.
Thanks for the PR @luca992! Can you say more about why posting with a pre-stamped request is beneficial? The reason I'm asking is we might want to consider another interface potentially. Educated guesses:
- Right now the request signing is hardcoded to be done by the
TurnkeyClient's API key but maybe you envision signing being done by something else on the server? (different API key? Different curve?). If that's the case we already have theStamptrait which you can implement. Would be good to know if it's something you tried to do or not (but I might be off because it might not be possible; all depends on your use case!) - Maybe the issue is that you're not producing the requests or the stamp in the local process that posts them? (proxy?) If that's the case we might want to have a new struct (say
TurnkeyRequestProxy) or a new set of methods with the "proxy" name somewhere to make it clear that it's the intended use-case.
Another thought I'm having: we could have a proxy_request and proxy_activity method on the TurnkeyClient, and have these take a Request as input. The post_body parsing and validation can happen in a util function potentially? (advantage being: TurnkeyClient doesn't have to parse, interface is a bit more uniform)
|
@r-n-o yeah basically for proxy requests similar to this old example Or for example when we let a user import a private key, I want the request to go through our backend so we make sure we sync their updated keys on success immediately after the request is made. I prefer not proxying all turnkey requests with a custom turnkey endpoint on our backend. I'd rather just make apis that take a stamp a serialized body and relay the request only when we actually need it (we often need to include other data for our backendwith the turnkey request we want to relay). |
r-n-o
left a comment
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.
Now that I understand the use-case a bit better I think it all makes sense! Thanks for taking the time to explain!
2 things to address:
- naming:
process_activity_with_stamp=>proxy_activityandprocess_request_with_stamp=>proxy_requestjust to make it clear no signing is happening. I also like that these names contain "proxy" in them, to make the use-case clear. The "with_stamp" becomes implicit because it's required in the function params - I'd like to see this used "for real" in our e2e tests. Mind writing a new example in our
examplescrate? I'm thinking a newproxy.rsexample in https://github.com/tkhq/rust-sdk/tree/main/examples/src/bin
On that last point let me know if that's too much to take on: happy to work on that and add commits to your branch instead!
|
I can do naming quick. I can see if I can make some tests in the next week or two. I'll let you know when I start on it. If you feel like doing it before me just lmk so I don't do duplicate work |
|
Sounds good! Same on my end. I'll be at devconnect next week so unlikely I'll get to it. I'll post something here if I get started for whatever reason! 👍 |
Adds
process_activity_with_stamp&process_request_with_stampapis toTurnkeyClientthat submit requests using a provided stamp.