New Turnkey signer: @turnkey/solana
#195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
andrewkmin
reviewed
Jan 18, 2024
| * @param tx Transaction object (native @solana/web3.js type) | ||
| * @param fromAddress Solana address (base58 encoded) | ||
| */ | ||
| public async addSignature(tx: Transaction, fromAddress: string) { |
There was a problem hiding this comment.
supporting just addSignature makes sense given this is all a user needs support for, at least for now. (when we implement a signer interface, then things might change as we support sign and partialSign, etc)
There was a problem hiding this comment.
Yeah definitely agree. We can extend the interface as needed in the future. There's also the "sign message" functionality; this can be added fairly easily but I've left it out for now since it's not been requested just yet.
andrewkmin
reviewed
Jan 18, 2024
andrewkmin
approved these changes
Jan 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
This is an attempt at wrapping signing logic with Solana.
The example code for signing with Solana (
examples/with-solana/src/createSolanaTransfer.ts) is now a lot shorter / cleaner, I think that's a win:Open to feedback on naming and structure of course; unfortunately
@solana/web3.jsdoesn't have ready-to-use abstract signer interfaces so this is a wrapper rather than a concrete implementation of a supported abstraction.I've briefly looked at https://github.com/solana-labs/solana-web3.js/tree/master/packages/signers but this is experimental (only committed 2 months ago) so unlikely to be useful right away. We don't want to force people to upgrade all the way to experimental releases . FWIW
@solana/signerstakes the same approach than what I have here and creates wrappers. If we were to go this route we'd choose to implement theTransactionPartialSignerandMessagePartialSignerinterfaces. Pretty easy to add down the road if we want to!I've also seen https://github.com/solana-labs/wallet-adapter: seems like a useful library used by many projects. I think we should try to implement our own "wallet" in https://github.com/solana-labs/wallet-adapter/tree/master/packages/wallets? Not sure if that'd be useful?
Oh and: this branch also updates the Solana demo to work with wallets rather than private keys!
How I Tested These Changes
with-solanademo manually