Repository navigation
fix(btc): read the vault's scriptType so dApps get the segwit address - #161
Open
BitHighlander wants to merge 1 commit into
Open
BitHighlander wants to merge 1 commit into
BitHighlander wants to merge 1 commit into
Conversation
KeepKey Desktop's /api/pubkeys/batch returns rows with `scriptType` (camelCase) and an empty address. The extension reads `script_type` everywhere, so every UTXO row looked script-less and derived as p2pkh. A native-segwit account was handed to dApps as the legacy encoding of the same key: swaps.pro showed recipient 1JaUQDVN...c54Hso for bc1qcr8te4..., and BTC swapped there didn't show in Desktop's accounts (recoverable only through Desktop's sweeper). Normalize rows at ingestion (fresh batch and cached) so script_type is set, and let utxoAccountFromPubkeyRow accept either name. Regression tests use the vault's real row shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
KeepKey Desktop's
/api/pubkeys/batchreturns UTXO rows as{ pubkey: '<xpub>', address: '', scriptType: 'p2wpkh' }(camelCase;projects/keepkey-vault/src/bun/rest-api.ts, the batch handler). The SDK passes them through unchanged. The extension readsscript_type(snake_case) everywhere, so every UTXO row looked like it had no script type:utxoAccountFromPubkeyRowfell back top2pkh, sorequest_accountshanded dApps the legacy encoding of the native-segwit key. On swaps.pro, a USDC → BTC swap's recipient was1JaUQDVN…c54Hso(the legacy encoding ofbc1qcr8te4…for the BIP-39 test wallet). BTC sent there doesn't show in KeepKey Desktop's accounts; only Desktop's sweeper recovers it.pk.script_typelookups inbackground/index.ts(preferred-account choice, send context, receive address) compared againstundefinedfor the same reason.The existing tests built rows with
script_type, which isn't the shape the vault sends, so this never showed up in CI.Fix
normalizePubkeyRows(inutxoDerive.ts) copiesscriptTypeintoscript_typewhen it's missing.wallet.tsapplies it to the fresh batch and to cached rows (caches written before this fix lackscript_type). Rows that already havescript_type, and non-UTXO rows, are left as they are.utxoAccountFromPubkeyRowalso accepts either name.bc1qcr8te4kr609gcawutmrza0j4xv80jy8z306fyu.Verified
make type-check✅ ·make test✅ (146 pass) ·make lint✅ (no new warnings) · prettier ✅1JaUQDVN…c54Hsobefore and isbc1qcr8t…306fyuafter. Nothing signed or broadcast. Not yet exercised against a real device and Desktop.Follow-up worth considering
Anyone who swapped into BTC through the extension with a SegWit account may hold BTC on the legacy encoding of their key. Desktop's sweeper handles it; a support note could point people there.
🤖 Generated with Claude Code