Repository navigation
fix: read Solana v1 transactions and reject v1 dApp signing (2.15.13) - #1055
Conversation
Mainnet enabled transaction v1 at epoch 1035. With web3.js 1.91.4 and maxSupportedTransactionVersion 0, any v1 tx in a wallet's history fails the RPC fetch. - Bump @solana/web3.js 1.91.4 -> 1.99.0 (first 1.x that decodes v1) - Set maxSupportedTransactionVersion: 1 on getTxn, getTransactions and the activity history fetch - dApp browser: web3.js 1.x cannot serialize v1 for signing, so reject v1 requests up front and pass the reason to the dApp instead of leaving its promise hanging Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
madninja
left a comment
There was a problem hiding this comment.
Confirmed the premise independently before reviewing the fix. That signature really is version 1 on mainnet at slot 449486272, and the same request with maxSupportedTransactionVersion: 0 returns exactly the documented -32015. So the problem is real and the three flag changes in solanaUtils.ts are right.
Coverage of the dApp path is complete: both handleSignAndSendTransaction and handleSignTransaction get the guard, and handleSignMessage has no transaction to check. Threading the reason through parsedData.error in walletStandard.js is the right shape, and it fixes a genuinely hung promise.
Three things to fix or answer before this ships.
1. The guard blocklists v1 instead of allowlisting what the wallet can sign. hasV1Transaction tests version === 1, so the first transaction version after this one decodes, passes the check, reaches serialize() and throws, and the dApp promise hangs again. That is the exact bug this PR fixes, reintroduced by the shape of the fix. Inverting it to reject anything that is not 'legacy' or 0 costs nothing and does not need revisiting when the next version lands. Worth renaming the constant to match, since the message would then be version-specific.
2. A transaction that decodes as neither still hangs the dApp. In deserializeTransactionInputs, a failed VersionedTransaction.deserialize falls through to Transaction.from(tx), and if that throws, the rejection propagates out of Promise.all with no postMessage, so nothing ever answers the dApp. That is pre-existing, but it is the same hung-promise failure this PR exists to close, and it sits three lines from the new guard. A try/catch that posts signatureDeclined with a reason would close the class rather than the instance.
3. @helium/spl-utils is pinned at 0.13.0, which still asks for max version 0. I checked the published tarballs rather than the changelog: 0.13.0, 0.13.1 and 0.13.2 all ship four maxSupportedTransactionVersion: 0 call sites, and 0.13.3 is the first with : 1 (0.13.4 is current). The call sites are in sendAndConfirmWithRetry, bulkSendRawTransactions and getAllTxns. This is latent rather than breaking today, since those read back transactions the wallet itself sent and web3.js 1.x cannot build a v1, so the app is not hitting it. But it means "the wallet reads v1" is true of solanaUtils.ts and not of the SDK underneath it, and the fix is a one-line bump to 0.13.4 in the same PR that claims v1 read support.
Two smaller notes.
The cast in hasV1Transaction is carrying helium-js#385's problem. Once 5.0.5 is published and picked up, the cast and its comment should go; worth a TODO tied to that PR so it does not outlive the cause.
The bump from 1.91.4 to 1.99.0 is not only web3.js. The lockfile takes rpc-websockets from 7.x to 9.x and superstruct from 0.14 to 2.0, both major, and rpc-websockets is the transport behind every Connection subscription in a React Native bundle. Combined with "not yet tested on a device", that is the risk in this PR rather than the flag changes. Worth exercising account and signature subscriptions on a real build before release, not just yarn test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
07b7892 answers this review: 3 fixed, 1 declined, 1 answered.
1
The guard tested
One note: web3.js 1.99.0 does not decode a version above 1 at all. Today that input goes to the catch in item 2, not to this guard. The allowlist starts to matter when web3.js learns the next version. 2
A failed decode rejected out of The decode step and the guard moved to Unproven: no test runs the handlers. The path from the catch through the WebView bridge to the dApp's promise was not run on a device. 3
Declined, because the direct dependency is not at 0.13.0. The 0.13.0 that you saw is real, but it is a nested copy. 4
07b7892 adds a TODO above the cast in 5
The two majors are not new to the lockfile. The runtime risk that you name is real. The app's own Also in 07b7892
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Two more commits landed after the review summary above. Both are in the dApp browser's sign path, and both come from a second pass over the fix for item 2. 2deaf07: send batches in sequence Item 2's catch made
9efacae: decode the same way as the sign sheet
Checks
The PR description has both changes. |
signTxn returned the transaction unsigned when the software wallet's private key was missing from the keychain. The app then submitted it, and blockchain-api rejected it with "Transaction is not signed by its fee payer". This happens when the account list syncs to a new device through iCloud but the keychain entry does not. - Throw on a missing key and show the existing "Private Key Not Found" restore prompt. - Reload the key when the cached keypair is unset or belongs to another account, and throw if the key does not match the wallet. - After any signer (software, Ledger, Keystone), verify the user's signature slot against the message before returning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
signTxn now throws on a missing or wrong key. Catch it in onMessage and post signatureDeclined so the dApp promise does not hang. Also drop the dead undefined-keypair branch in signWithKeypair and pin the verifyEd25519 catch with a malformed-signature test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move the wallet-type branch, key reload, missing-key path, device signature checks and assertSignedBy out of the SolanaProvider callback into signing.ts. Device and storage calls come in as callbacks, so the tests use real transactions and keys with no React Native harness. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Anchor 0.31.2 ships Solana v1 transaction parsing in AnchorProvider and fetches failed txs with maxSupportedTransactionVersion 1 so program logs come back instead of a parse error. It is published only as @anchor-lang/core. - Replace the direct dependency and all src imports - Alias @coral-xyz/anchor@^0.31.0 (used by the @helium/* SDKs) to @anchor-lang/core so the app keeps a single anchor copy and the AnchorProvider/Program types line up across SDK boundaries - Leave Pyth's @coral-xyz/anchor@^0.29.0 alone (older IDL format) - Bump engines.node to >=20.18.0 per the release requirement Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Bump the 21 HPL packages and the four HPL resolutions to the published versions (spl-utils 0.14.1, idls 0.12.1, blockchain-api 0.16.1) - Allow @solana/web3.js ^1.99.0 and @anchor-lang/core ^0.31.2 - Regenerate yarn.lock from the base branch Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- metro: send @coral-xyz/anchor (aliased to @anchor-lang/core) and @anchor-lang/core to one module path; Pyth keeps its anchor 0.29 copy - AddGatewayBle: deserialize onboarding transactions with VersionedTransaction.deserialize in place of bufferToTransaction Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Match the exact-pin style the repo used before the bump, in dependencies and resolutions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Node 18 is end of life, and the Agave 4.2 dependency bump sets engines.node to >=20.18.0. Run CI on the lowest version that range allows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pin CI to Node 20.18.0, the engines floor, and update the README setup steps to Node 20.18. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- AddGatewayBle: pass unsigned onboarding transactions to toTransactionData so the wallet signs once. - useLedger: drop the duplicate bs58 import and use base58. - metro.config.js: update the dedup comment and add a resolver test for the anchor alias. - Pin @helium/cron-sdk, @helium/tuktuk-idls and @helium/tuktuk-sdk to 0.1.1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HNT to DC fell through to Jupiter when the oracle price was unset, which failed with "Swap routes missing". The price was fetched once per connection, so one failed read left it unset. Route HNT to DC to the mint-DC path every time and show the oracle error instead. Poll the price every 5 minutes, retry after 30 seconds on error, and drop a price older than 10 minutes. Disable the swap button for HNT to DC while the price is missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Read the oracle without the persisted account cache, time out a hung request, expire a price older than 10 minutes, keep the price object when it is unchanged, and show a hint while the first price loads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ng' into consolidate-tmp
madninja
left a comment
There was a problem hiding this comment.
Items 1, 2 and 4 from my earlier review are fixed, and the new tests in dappTransactions.test.ts cover the allowlist, the undecodable-input rejection and the sequential stop-on-failure. On item 3, the reply was right when written. After the HPL bump no 0.13.x spl-utils remains: the copies are 0.14.1 and a nested 0.11.14 under @helium/onboarding 5.0.4 and @helium/react-native-sdk 4.0.4. That 0.11.14 copy still passes maxSupportedTransactionVersion: 0, and its helpers are called by those SDKs' submit paths, which the app does not use today. No change needed for it here.
Keeping changes requested for the folded-in work. I couldn't run the suite locally (no node_modules in my checkout), so the test-coverage points come from reading the code.
Before merge
- Onboarding can throw while the HNT price is missing. The new polling in
Balance.tsxsets the price toundefinedwhen it expires and when a refresh fails after it went stale, not only before the first fetch.networkTokensToDcreturnsundefinedwithout a price, andsrc/hooks/useOnboardingBalances.ts:87callsnetworkTokensToDc(...)!.div(...)during render, so onboarding throws for as long as the price is missing. Handleundefinedthere, and check the othernetworkTokensToDccallers (PaymentItem,BurnScreen,useImplicitBurn) for the same non-null assertion. - The swap button ignores
routeNotFound(inline). The screen shows the "route not found" text, but the button stays enabled, so a pair with no route (for example a non-HNT token into DC on mainnet) can still reachgetSwapTxwith whatever quote was cached last. This predates the PR, but the PR rewrites this condition and it is one line. - No test puts the wallet in a slot other than 0 (inline on
signing.ts:41). Every case insigning.test.tshas the user as fee payer, so replacing thefindIndexlookup with index 0 (or the legacy.findwithsignatures[0]) stays green. Add a case where the wallet signs but is not the fee payer, for both versioned and legacy transactions.
Here or in a tracked follow-up
signMsg(SolanaProvider.tsx:110) still signs withsecureAcctwithout checking that it belongs to the current account, which is the stale-key case the newsignAsAccounthandles for transactions (its own comment says the cached keypair "can be unset or stale after an account switch"). It also tests!accountIndexwheresignAsAccounttests forundefined. Routing messages through the same wallet-type and key-match logic would close both.- The dApp bridge matches replies by message type only, and the sign sheet's
show()replaces the pending resolver. With two requests in flight, the first never gets its own reply and both dApp promises settle on the second. A request id on each message, or refusing a request while one is pending, fixes it. Predates this PR.
Smaller
- When the expiry timer clears the price, nothing fetches a new one until the next 5-minute poll, so the button can sit disabled with "Fetching HNT price" and no request in flight. Fetch on expiry, or schedule the next poll at the earlier of the poll interval and the expiry. Related: the UI price only feeds the preview, since
mintsendsdcAmountand the program prices the burn from its own oracle, so worth saying in the PR that the gate is about the displayed figure. - If the wallet is not a required signer,
signTxnreturns the transaction unchanged, and the browser then answers the dApp with "transactionSigned". No key is used, but declining in the browser path would be more accurate. DeserializedTransaction.isVersionedis always true now that decoding usesVersionedTransaction.deserializeonly, so the legacy branches and casts indappTransactions.tsandBrowserWebViewScreen.tsxare dead. The comment atdappTransactions.ts:60says a send "landed", but withskipPreflightit means the RPC accepted it. The route decision lives inswapRoute,getOutputAmountandgetInputAmount; ingetInputAmountthe devnet check comes first, so entering a DC amount on devnet sets the HNT input to 0.
- Swap button is disabled when no route is found. - On devnet, a DC amount in "you receive" no longer zeroes the HNT input. - Onboarding balances no longer throw while the HNT price is missing; the HNT part is left out until a price arrives. - An expired HNT price triggers a fetch at once, unless one is in flight. - The dApp browser declines a sign request when the wallet is not a required signer of every transaction. - isRequiredSigner moves to src/solana/signing.ts, with tests. - DeserializedTransaction holds a VersionedTransaction only; the dead legacy branches are gone. The sendInSequence comment says "accepted by the RPC" instead of "landed". - signWithKeypair tests cover a wallet that signs but does not pay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Commit eb4df6f addresses the review: 6 fixed, 2 deferred.
1 — "Onboarding can throw while the HNT price is missing." 4 — " 5 — dApp bridge replies are not matched to their requests. 6 — Price expiry clears the price but does not fetch a new one. 7 — The browser posts 8 — Dead legacy branches, a wrong comment, and devnet input order.
|
madninja
left a comment
There was a problem hiding this comment.
Approving at eb4df6f. The three must-fix items are in: the onboarding non-null assertion is gone, routeNotFound is in the button's disabled, and the new tests sign as a wallet another account pays for, versioned and legacy. The browser now declines before the sign sheet when the wallet is not a required signer, and the dead legacy branches, the "landed" comment and the devnet input order are fixed. The two deferred items are listed under Follow-ups.
Notes, none blocking:
- The expiry refetch depends on two 10-minute constants agreeing. It is bounded today:
ORACLE_MAX_AGE(10 min) equalsMAX_PRICE_AGE_SECSin@helium/currency-utils0.12.1, so a successful fetch always leaves a positive expiry, and a stale feed is rejected and goes to the 30 s retry. If the library's limit is ever raised aboveORACLE_MAX_AGE, a feed aged between the two would fetch successfully with an expiry of 0 and refetch in a tight loop. Scheduling the expiry refetch withORACLE_RETRY_INTERVALinstead of 0 would bound it whatever the library does. - Onboarding undercounts while the price is missing.
useOnboardingBalancesnow counts the DC balance alone until a price arrives, so a wallet that can pay with HNT can briefly read as short. Returningundefined(still loading) while the price is missing would avoid that. The reply's "stop reading a missing price as 0 DC" reads the other way from the code, which does count the HNT part as 0 in that window. - Follow-ups as issues. The
signMsgkey match and the bridge request ids are only in the PR description, which stops being read once this merges. Opening an issue for each keeps them visible. - Device test before release. The signing and dApp bridge changes are covered by jest only. One pass on a device (a dApp sign, a multi-transaction
signAndSendTransaction, and a decline) before 2.15.13 ships.
- Onboarding: report DC+HNT as loading while the HNT price is missing; screens wait unless DC alone covers the cost. - signMsg: use the wallet-type and key-match logic of signAsAccount; refuse bytes that decode as a transaction message. - dApp bridge: request ids on every request and reply; decline a second request while one is open; read every message shape. - Sign sheet: settle the previous request before a new one opens. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed in d210564: 2 fixed, 1 declined. The device test (item 4) stays open until a device pass is done.
1
The loop cannot happen with the installed library. 2
Fixed. When the wallet holds HNT and no price is available, 3
Both are fixed in this PR instead.
The remaining follow-ups are listed in the PR description. |
Restore replaced the account record with alias and address only, so it dropped derivationPath. Its fallback scan also skipped the app's own default path (m/44'/501'/0'/0') and sub-account paths. After one restore, the next restore of a default-path account failed with "words do not match". Restore now scans the stored paths plus the Import scan set, keeps the record, and saves the path it found. One set of words restores every listed software account it holds a key for. Hardware wallet accounts never get a software key. The Solana provider cached the keypair by address. A restore keeps the address, so the cache kept the missing key until relaunch. A key-version counter now reloads it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Mainnet has enabled transaction v1 since epoch 1035 (2026-09-15), and v1 transactions are already landing in blocks. With web3.js 1.91.4 and
maxSupportedTransactionVersion: 0, any v1 transaction in a wallet's history makes the RPC fetch fail.dApp sign request deserializeTransactionInputs # VersionedTransaction.deserialize only hasUnsupportedTransaction # reject anything not legacy or v0 + isRequiredSigner (every tx) # reject when the wallet does not sign sign sheet sendInSequence # one at a time, stop at first failureChanges
@solana/web3.jsfrom 1.91.4 to 1.99.0, the first 1.x release that can decode v1.maxSupportedTransactionVersion: 1ingetTxn,getTransactionsand the activity history fetch (src/utils/solanaUtils.ts), and in thegetParsedTransactioncall insrc/features/txnDelegation/useSolTxns.ts, which passed no version before. This must ship with the bump: 1.91.4's response schema only accepts0 | 'legacy'.walletStandard.jspasses the reason to the dApp. Before this change, the dApp's promise hung.signAndSendTransactionnow signs and sends a batch one transaction at a time. Before this change, it sent the batch in parallel. If a send fails after the RPC accepted an earlier one, the error names the signatures already submitted, and the rest of the batch is not sent.VersionedTransaction.deserializeonly, the same way as the sign sheet. That call reads legacy transactions too. A request that the sign sheet cannot decode is rejected.transactionSigned(or the batch was sent) with no wallet signature.src/features/browser/dappTransactions.ts, with a test that runs legacy, v0, v1 and undecodable inputs through them. The sequential send is in the same file and has its own tests.Also folded in
These PRs are now part of this branch, so the 2.15.13 release gets one review:
signTxnnever returns a transaction that the wallet should have signed but did not. A missing keychain key now throws instead of a silent no-op ("not signed by its fee payer"). A transaction where the wallet is not a required signer is still returned unchanged; the dApp browser declines those up front.@helium/cron-sdk,@helium/tuktuk-idlsand@helium/tuktuk-sdkare pinned at exactly 0.1.1.Review fixes
useOnboardingBalances). When the wallet holds HNT and no price is available, the DC+HNT total reports loading (loadingMyDcWithHnt). Hotspot onboarding and location asserts wait instead of showing "insufficient funds", unless the DC balance alone covers the cost (waitingOnHntPriceinsrc/utils/dcWithHnt.ts). The location assert compares against the higher of the IOT and MOBILE costs while it waits, because the network is picked after the press.signMsgnow uses the same wallet-type and key-match logic assignAsAccount(signMessageAsAccountinsrc/solana/signing.ts). A software key that does not match the account, or a missing key, is refused. Ledger and Keystone accounts take the device path.isTransactionMessage). The dApp browser declines such a request before the sign sheet opens.signMessagereads the message bytes from every shape the bridge delivers (messageBytesindappTransactions.ts). Before this change, aUint8Arraymessage failed to decode and the request was declined.walletStandard.jscarries an id, and each reply carries the same id. A listener ignores replies for other requests. A second request while one is open is declined with "Request pending" (runOneRequestindappRequestGate.ts). A request that arrives before the wallet is ready is declined with its id.falsebefore it opens a new one, so no awaitedshow()hangs (signRequestResolver.ts).DeserializedTransactionholds aVersionedTransactiononly; the dead legacy branches are gone.isRequiredSignermoved tosrc/solana/signing.ts, with tests for a non-payer signer, an absent wallet, and a wallet that is only a read-only account.Private key restore
A user who restored their private key from the "Private Key Not Found" alert still got "not signed by its fee payer". This section fixes three defects in that flow.
AccountStorageProvidernow makes the provider load the key again after each key write.derivationPath. When the record had no path, the fallback scan did not try the app's default path (m/44'/501'/0'/0') or sub-account paths. So after one restore, the next restore of a default-path account failed with "The words you entered do not match", even with the correct words. Restore now tries every stored path plus the same scan set that Import uses (matchRestoredAccountsinsrc/features/onboarding/import/accountRestore.ts). It keeps the record and saves the path where it found the key.src/utils/derivationPaths.ts, so jest can load them.useDerivationAccountsre-exports them.Known limitation
The wallet still cannot sign v1 transactions. Signing needs web3.js 2.x or @solana/kit. HPL keeps wallet-signed paths on v0 until then.
Type-check note
@helium/onboarding@5.0.4has.d.tsfiles that referencejito-ts/node_modules/@solana/web3.js, which is web3.js 1.77.4. Its ambient declarations hide the installed web3.js types, sotscchecks against 1.77.4 types.dappTransactions.tshas one cast as a workaround, with a TODO that points at helium/helium-js#385. The real fix belongs in@helium/onboarding.Evidence
yarn lint: 0 errorsyarn test: 23 suites, 199/199 pass (all folded PRs merged).matchRestoredAccountstests: when the default and sub-account paths are removed from the scan set, the default-path, sub-account, wrong-stored-path, sibling and hardware-wallet cases fail. New:signWithKeypairsigns the wallet's slot of a versioned and a legacy transaction paid by another account; each test fails when the slot lookup is replaced with slot 0.isRequiredSignertests fail when the check reads every account key instead of the signer keys.expo export:embed) build5hyG1Lbu5bL2FV13DdshJua5RK5Vj7nqTJZuVV8YwN6A2Awq2WdbcFb8DgPV87xCSWsJPPzHVYnfMKVTgNkLAxYC:getTransactionandgetParsedTransactionsreturn version 1serialize()throws, which confirms the browser must reject v1 up frontMerge Danger
Door: two-way
No storage, migration or on-chain change. A revert restores the old fetch flags and browser behaviour.
Blast Radius: wide
Every activity fetch, every dApp sign request, message signing for every wallet type, every private key restore and every reader of the provider's cached keypair, the dApp connect flow, the bridge protocol between
walletStandard.jsand the app, and the HNT→DC swap path change. A dApp that sends transactions the wallet does not sign now gets a decline instead of a silent pass-through.walletStandard.jsships in the app bundle, so the script and the app change together.Follow-ups