Repository navigation
fix(solana): never return an unsigned or wrongly signed transaction - #1056
Closed
bryzettler wants to merge 4 commits into
Closed
bryzettler wants to merge 4 commits into
bryzettler wants to merge 4 commits into
Conversation
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>
Contributor
Author
|
Folded into #1055 so 2.15.13 ships as one PR. |
This file contains hidden or 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
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.
Problem
signTxnreturned the transaction unsigned when a software account had no private key on the device. This happens when accounts sync through iCloud but the keychain entry does not, for example on a new phone or after a reinstall. The app then submitted the unsigned bytes, and blockchain-api rejected them withTransaction is not signed by its fee payer. The user saw a server error, not the existing "Private Key Not Found" prompt. Hotspot transfers, claims, votes, swaps and dApp signing were all affected.Fix
checkSecureAccountrestore prompt and throwsrestoreAccount.missingAlert.message.solanaAddress. Otherwise the app reloads it from SecureStore, which closes the stale window after an account switch. A stored key for another account throws.assertSignedBychecks with ed25519 that the user's slot verifies over the message bytes. A missing, zero or wrong-key signature now fails in the app, not at the server.onMessagecatches a signing error and postssignatureDeclined, so the dApp promise does not hang.signAsAccountinsrc/solana/signing.ts, with device and storage calls passed in.signTxnkeeps thesolanaAddresscheck and theisRequiredSignerearly return.Tests
src/solana/__tests__/signing.test.tsadds 19 tests on real web3.js transactions and real keys. They cover: versioned and legacy txs, a missing key, a mismatched key, a stale cached key, a Ledger with no account index, Ledger and Keystone signatures by the user and by another key, an empty device signature, a message changed after signing, and a malformed signature.yarn lint: 0 errors.yarn test: 131/131 pass.jest.config.jsnow transforms@noble/ed25519, because it ships ESM only.Not tested on device
I did not run iOS or Android, and I did not run Ledger or Keystone end to end. Their path changes only by the added verify step.
Not in this PR
signMsgchecks!accountIndex, so message signing on a Ledger at index 0 takes the software path.signAndSendTransactionsigns withPromise.all. A missing key can show one alert per transaction.