feat: Upgrade to @solana/web3.js version 2.0.0
#15535
Conversation
steveluscher
force-pushed
the
solana-web3-js-two-dot-oh
branch
from
2e47a86
to
61dc248
Compare
| ...args: [ | ||
| signature: Signature, | ||
| config: Readonly<{ | ||
| encoding: 'jsonParsed'; |
There was a problem hiding this comment.
This is intense, but basically boils down to: choose the overload that has encoding: 'jsonParsed', infer its return type, and use that.
| computeUnitsConsumed: 100n, | ||
| preBalances: [200n, 100n], | ||
| postBalances: [180n, 110n], | ||
| fee: 10n, |
There was a problem hiding this comment.
By and large, the new API spits off the correct number types – bigint when the corresponding type on the server is u64 and number otherwise.
| before: lastSignature?.signature, | ||
| limit, | ||
| }) | ||
| .send(); |
There was a problem hiding this comment.
Here is one example of a callsite where you could introduce an AbortSignal if ever you wanted to make these RPC-related operations cancellable.
.send({ abortSignal });| await Promise.all( | ||
| signatures.map(signature => | ||
| api.rpc | ||
| .getTransaction(signature, { | ||
| encoding: 'jsonParsed', | ||
| maxSupportedTransactionVersion: 0, | ||
| commitment: 'confirmed', | ||
| }) | ||
| .send(), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
RPC providers generally hate it when we make request batches. It thwarts their ability to load balance, and it creates an all-or-nothing dynamic where one failed request invalidates the entire batch. Typically HTTP/2 transports give us all the performance of manual batching, and all of the benefits of routing on the backend. You may like to visit the call to createDefaultRpcTransport() and experiment with the dispatcher_NODE_ONLY config option.
| if (isDurableNonceTransaction(message)) { | ||
| // TODO: Handle durable nonce transactions. | ||
| throw new Error('Unimplemented: Confirming durable nonce transactions'); | ||
| } |
There was a problem hiding this comment.
If you ever want to support these, there is a version of sendAndConfirmTransactionFactory that can confirm durable nonce transactions.
| let NEXT_ACCOUNT_SUBSCRIPTION_ID = 0; | ||
| const ACCOUNT_SUBSCRIPTION_ABORT_CONTROLLERS = new Map<number, AbortController>(); | ||
| function abortSubscription(id: number) { | ||
| const abortController = ACCOUNT_SUBSCRIPTION_ABORT_CONTROLLERS.get(id); | ||
| ACCOUNT_SUBSCRIPTION_ABORT_CONTROLLERS.delete(id); | ||
| abortController?.abort(); | ||
| } |
There was a problem hiding this comment.
I would have preferred to put an unsubscribe() function on the SubscriptionAccountInfo type itself so that I didn't have to establish this correspondence between subscription ‘ids’ and abortcontrollers, but that type is in common so I didn't touch it.
| // TODO: Wrap this for/await loop in a try/catch and write code to recover in the event | ||
| // that the account subscription going down. |
There was a problem hiding this comment.
You can capture subscription transport errors now. Warn! Retry! Whatever!
| (async () => { | ||
| // TODO: Wrap this for/await loop in a try/catch and write code to recover in the event | ||
| // that the account subscription going down. | ||
| for await (const _ of accountNotifications) { |
There was a problem hiding this comment.
The subscriptions API vends AsyncIterables now. You can iterate over them, wrap the whole thing in try/catch and do optional cleanup in finally. When you call abortController.abort() the loop will gracefully exit and enter the finally block.
| @@ -198,7 +196,7 @@ export const fixtures = { | |||
| }, | |||
| }, | |||
| expectedOutput: | |||
| '01000609c80f8b50107e9f3e3c16a661b8c806df454a6deb293d5e8730a9d28f2f4998c6a99c9c4d0c7def9dd60a3a40dc5266faf41996310aa62ad6cbd9b64e1e2cca78ebaa24826cef9644c1ecf0dfcf955775b8438528e97820efc2b20ed46be1dc580000000000000000000000000000000000000000000000000000000000000000527706a12f3f7c3c852582f0f79b515c03c6ffbe6e3100044ba7c982eb5cf9f28c97258f4e2489f1bb3d1029148e0d830b5a1399daff1084048e7bd8dbe9f8590306466fe5211732ffecadba72c39be7bc8ce5bbc5f7126b2c439b3a40000000d27c181cb023db6239e22e49e4b67f7dd9ed13f3d7ed319f9e91b3bc64cec0a906ddf6e1d765a193d9cbe146ceeb79ac1cb485ed5f5b37913a8cf5857eff00a96772b7d36a2e66e52c817f385d7e94d3d4b6d47d7171c9f2dd86c6f1be1a93eb040600050250c3000006000903a0860100000000000506000207040308000804010402000a0c00e1f5050000000009', | |||
| '01000609c80f8b50107e9f3e3c16a661b8c806df454a6deb293d5e8730a9d28f2f4998c6a99c9c4d0c7def9dd60a3a40dc5266faf41996310aa62ad6cbd9b64e1e2cca78ebaa24826cef9644c1ecf0dfcf955775b8438528e97820efc2b20ed46be1dc580000000000000000000000000000000000000000000000000000000000000000527706a12f3f7c3c852582f0f79b515c03c6ffbe6e3100044ba7c982eb5cf9f28c97258f4e2489f1bb3d1029148e0d830b5a1399daff1084048e7bd8dbe9f8590306466fe5211732ffecadba72c39be7bc8ce5bbc5f7126b2c439b3a40000000d27c181cb023db6239e22e49e4b67f7dd9ed13f3d7ed319f9e91b3bc64cec0a906ddf6e1d765a193d9cbe146ceeb79ac1cb485ed5f5b37913a8cf5857eff00a96772b7d36a2e66e52c817f385d7e94d3d4b6d47d7171c9f2dd86c6f1be1a93eb040600050250c3000006000903a086010000000000050600020704030801000804010402000a0c00e1f5050000000009', | |||
There was a problem hiding this comment.
The old library was a little less pedantic than this one. The changed bytes here are at the end starting from here: 1000804010402000a0c00e1f5050000000009
| return { | ||
| async addSignature(privateKeyHex: string) { | ||
| const privateKeyBytes = getBase16Codec().encode(privateKeyHex); | ||
| const keypair = await createKeyPairFromPrivateKeyBytes(privateKeyBytes); | ||
| transaction = await signTransaction([keypair], transaction); | ||
| }, | ||
| serializeMessage() { | ||
| return getBase16Codec().decode(transaction.messageBytes); | ||
| }, | ||
| serialize() { | ||
| return pipe(transaction, getTransactionEncoder().encode, getBase16Codec().decode); | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Rather than to really thrash wallet-core I decided to make a shim that behaves mostly like the legacy API.
This change attempts to largely replicate the old account and transaction shapes, but also represents a breaking change as some of the old types (like `PublicKey`) no longer exist in `@solana/web3.js` version 2. Some type utilties were added so that we could source the _actual_ data shapes from the API calls themselves. For instance, we choose the return type that results from calling `getTransaction` with the config `"encoding": "jsonParsed"` _in particular_.
1. Added an RPC subscriptions instance to the `API` type
2. Added code to throw in the event that the transaction is a durable nonce transaction. A little extra work would have to be done to import and construct the durable nonce transaction confirmer.
3. Added a `FIXME` explaining why it would be better to save the `lastValidBlockhash` at the time the transaction was first given a blockhash lifetime, instead of fetching a too-new one in `pushTransaction`
4. Eliminated all of the custom transaction confirmation code, in favour of the new built-in routines.
5. Added a `FIXME` to note that the `fetch()` API typically doesn't allow you to override the `Origin` header. I didn't test whether your runtime actually allows it or not.
Note that all RPC methods are now cancellable via `AbortController`, if you supply an `AbortSignal` to the send/subscribe methods (ie. `.send({ abortSignal })`)
…`@solana/web3.js` version 2 1. Corrected a decimal amount in one of the fixtures. That `amount` field is denominated in basis points. 2. Deleted most of the custom transaction instruction formatting code, in favour of the new auto-generated program clients for the System, Compute Budget, and Token programs. 3. Added a note that `lastValidBlockHeight` is sometimes `undefined` in tests
steveluscher
force-pushed
the
solana-web3-js-two-dot-oh
branch
from
61dc248
to
4b67a19
Compare
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is network access?This module accesses the network. Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use. What are unpopular packages?This package is not very popular. Unpopular packages may have less maintenance and contain other problems. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
|
please @martykan if you could take part in the review process? I believe others (@tomasklim) will join too |
| @@ -188,16 +197,22 @@ export const extractAccountBalanceDiff = ( | |||
| const postBalance = transaction.meta?.postBalances[pubKeyIndex]; | |||
|
|
|||
| return { | |||
| preBalance: new BigNumber(preBalance ?? 0), | |||
| postBalance: new BigNumber(postBalance ?? 0), | |||
| preBalance: new BigNumber(preBalance?.toString() ?? 0), | |||
There was a problem hiding this comment.
I guess this should also be changed above in if (isTokenDiff)... (L190-191)
| .decimalPlaces(0, BigNumber.ROUND_UP) | ||
| .toNumber(); | ||
| .toString(); |
There was a problem hiding this comment.
| .toString(); | |
| .toString(10); |
This is to prevent BigNumber.toString outputting large numbers in scientific notation
| @@ -21,7 +21,7 @@ | |||
| }, | |||
| "dependencies": { | |||
| "@mobily/ts-belt": "^3.13.1", | |||
| "@solana/web3.js": "^1.95.4", | |||
| "@solana/web3.js": "^2.0.0", | |||
There was a problem hiding this comment.
| "@solana/web3.js": "^2.0.0", |
Yarn depcheck is telling me this is unused
| @@ -13,8 +13,11 @@ | |||
| }, | |||
| "dependencies": { | |||
| "@mobily/ts-belt": "^3.13.1", | |||
| "@solana-program/compute-budget": "^0.6.1", | |||
| "@solana-program/system": "^0.6.2", | |||
| "@solana-program/token": "^0.4.1", | |||
| "@solana/buffer-layout": "^4.0.1", | |||
There was a problem hiding this comment.
| "@solana/buffer-layout": "^4.0.1", |
Also seems to be currently unused
Description
The Trezor team noticed some problems with the subscriptions subsystem in
@solana/web3.js@1(see #12628). The new library (blog post) makes it much easier to catch errors when the subscriptions transport goes down. Rather than to try to hack a fix in, I offered to upgrade the entire app to version 2.0.0.Warning
I did not test this. All of the unit tests pass and there are no intra-monorepo type errors, but that says nothing of whether any of this works or not. Please do run it through your regular QA process.
How to review this PR
This PR represents a stack of commits, each dealing with a different concern/package. Step through the commits one at a time, reading the notes in each commit message.
Notes
crypto.subtleglobal, then install@solana/webcrypto-ed25519-polyfilland instantiate it at the entrypoint of the app, or at least as close to the cryptographic operation (egasync addSignature(privateKeyHex: string)) as possible.BREAKING CHANGE: To the extent that any non-Trezor application depends on the packages published from this monorepo, the interfaces may have changed.