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

authenticated remote signer requests #1985

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

e-asphyx
Copy link
Collaborator

Thank you for your contribution to Taquito.

Before submitting this PR, please make sure:

  • Your code builds cleanly without any errors or warnings
  • You have added unit tests
  • You have added integration tests (if relevant/appropriate)
  • All public methods or types have TypeDoc coverage with a complete description, and ideally an @example
  • You have added or updated corresponding documentation
  • If relevant, you have written a first draft summary describing the change for inclusion in Release Notes.

Release Note Draft Snippet

If relevant, please write a summary of your change that will be suitable for
inclusion in the Release Notes for the next Taquito release.

@netlify
Copy link

netlify bot commented Sep 30, 2022

Deploy Preview for taquito-test-dapp canceled.

Name Link
🔨 Latest commit 5521bcc
🔍 Latest deploy log https://app.netlify.com/sites/taquito-test-dapp/deploys/633ef3d84190e10007a49b53

@jevonearth
Copy link
Collaborator

Nice work @e-asphyx !

Can you revert/remove the updates to package-lock files, please?
Can you add an example of use into the README.md in packages/taquito-remote-signer

How trivial/cumbersome would it be to get unit tests added for this? Vs integration tests. I guess integration tests would either require mocking a signer server in typescript, or relying on a signatory instance?

@github-actions
Copy link

github-actions bot commented Sep 30, 2022

A new deploy preview is available on Netlify at https://c9702f9d--tezostaquito.netlify.app

Copy link
Collaborator

@roxaneletourneau roxaneletourneau left a comment

Choose a reason for hiding this comment

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

@e-asphyx Can you add tests to the PR?

@@ -32,6 +32,16 @@ await Tezos.contract.transfer({ to: publicKeyHash, amount: 2 });

The constructor of the `RemoteSigner` class requires the public key hash and the URL of the remote signer as parameters. It also takes optional headers (i.e., Authorization) and an optional `HttpBackend` to override the default one if needed.

### Authenticated requests
`RemoteSigner` can use an authenticated protocol. All you need is to pass a secret key in the Tezos Base58 format as `RemoteSignerOptions` property:
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could specify "The secret key shouldn't be associated with an onchain account"

packages/taquito-remote-signer/src/auth.ts Outdated Show resolved Hide resolved
@BearCooder
Copy link

Hello @e-asphyx Can you add tests to the PR? Let me know, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants