-
Notifications
You must be signed in to change notification settings - Fork 1
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
PE-6052 -- feat(web): implement walletAdapter for SOL and ETH web signing support #188
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #188 +/- ##
==========================================
+ Coverage 93.31% 93.34% +0.02%
==========================================
Files 25 25
Lines 3097 3245 +148
Branches 144 147 +3
==========================================
+ Hits 2890 3029 +139
- Misses 207 216 +9 ☔ View full report in Codecov by Sentry. |
protected getAuthenticatedTurbo({ | ||
privateKey, | ||
signer: providedSigner, | ||
paymentServiceConfig = {}, | ||
uploadServiceConfig = {}, | ||
token, | ||
gatewayUrl, | ||
tokenMap, | ||
tokenTools, | ||
logger, | ||
walletAdapter, | ||
}: TurboAuthenticatedConfiguration & { logger: TurboWinstonLogger }) { | ||
token = token === 'pol' ? 'matic' : token; | ||
|
||
if (!token) { |
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.
this common
level factory de-duplicates the code from the node
and web
factories. we defer to the child classes for importing node and web specific uploaders and signers via abstract methods to keep the imports clean
if (this.walletAdapter) { | ||
if (!isEthereumWalletAdapter(this.walletAdapter)) { | ||
throw new Error( | ||
'Unsupported wallet adapter -- must implement getSigner', | ||
); | ||
} | ||
const signer = this.walletAdapter.getSigner(); | ||
if (signer.sendTransaction === undefined) { | ||
throw new Error( | ||
'Unsupported wallet adapter -- getSigner must return a signer with sendTransaction API for crypto funds transfer', | ||
); | ||
} | ||
|
||
const { hash } = await signer.sendTransaction({ | ||
to: target, | ||
value: parseEther(amount.toFixed(18)), | ||
}); | ||
return hash; | ||
} |
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.
here we defer to ethereum wallet adapter when available which allows web wallets to use the crypto-fund flow
17ee976
to
088457c
Compare
token = token === 'pol' ? 'matic' : token; | ||
|
||
token ??= 'arweave'; // default to arweave if token is not provided |
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.
could this be:
token = token === 'pol' ? 'matic' : token; | |
token ??= 'arweave'; // default to arweave if token is not provided | |
token = token === 'pol' ? 'matic' : token ??= 'arweave'; // default to arweave if token is not provided |
more so curious, not necessary to change
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.
it works with parenthesis around (token ??= 'arweave'), but doesn't appear to provide type safety on token being defined
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.
the pol=>matic
convenience part I think could be expanded to include an "acceptableTokenStringMap" that converts the tickers as well like
sol => solana
eth => ethereum
ar => arweave
it would be a littel easier to use the token input parameter on the sdk, but no need for yet
🎉 This PR is included in version 1.19.0-alpha.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 1.19.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
this PR implements ETH and SOL web signing for upload and crypto fund flows