feat: reuse signed request when reading state #584
Conversation
|
@krpeacock could you review these changes? I would love to remove the |
lmuntaner
changed the title
lmuntaner
changed the title
| @@ -386,8 +385,19 @@ export class HttpAgent implements Agent { | |||
| }); | |||
|
|
|||
| // Apply transform for identity. | |||
| transformedRequest = await id?.transformRequest(transformedRequest); | |||
| return id?.transformRequest(transformedRequest); | |||
There was a problem hiding this comment.
@lmuntaner is removing this await intentional?
There was a problem hiding this comment.
No, just because I am returning the value, I don't think it's needed.
If we had a try/catch then yes, but since we don't, then I don't think so.
lmuntaner
force-pushed
the
feat_LM_reuse-signed-request
branch
from
90b744e
to
bce0d41
Compare
|
@krpeacock can you review again please? |
|
@lmuntaner I think the approach here is good, and I see the usefulness for hardware wallets where each request requires another prompt. It's unfortunate to expand the use of I'm ready to approve, if we add a new e2e test and document the new feature in the changelog |
|
Yes, I agree the use of The test and docs sound good, when I have some time I will get to it. Thanks. |
lmuntaner
force-pushed
the
feat_LM_reuse-signed-request
branch
from
cfdc19e
to
d996dac
Compare
|
@krpeacock ready for another review I added an e2e tests to read_state to make sure it also works when the request is passed. I also added a few points to the changelog. Let me know what you think. |
|
@krpeacock is the e2e test and the docs that I added what you needed?? |
Description
Enable the
readStateof the Agent to reuse the request.Motivation
Signing with the hardware wallet requires the user to accept from the device. Reusing the signed request will improve the user experience for hardware wallet users.
How Has This Been Tested?
I linked a branch of nns-dapp to the changes in this PR and validated that everything worked, not just the new feature.
Checklist: