-
Notifications
You must be signed in to change notification settings - Fork 27
feat: pulls out btc package #696
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
base: dev
Are you sure you want to change the base?
Conversation
…utilities Adds a new `btc` namespace to the SDK with the following modules: - btc.slip132: SLIP-132 xpub format conversion (normalize, format, getPrefix, inferPurpose) - btc.network: Network inference from xpub (inferFromXpub, getCoinType, isTestnet) - btc.getXpub/getXpubs/getAllXpubs: Flexible xpub fetching with purpose/coinType support - btc.tx: Transaction building (buildTxReq, estimateFee) with UTXO selection - btc.wallet: Wallet utilities (getSummary, getSnapshot) for balance/UTXO aggregation - btc.provider: Blockbook provider abstraction for chain data Key design decisions: - No breaking changes: existing fetchBtcXpub/signBtcTx APIs unchanged - Uses Uint8Array/DataView instead of Node.js Buffer for browser compatibility - Provider interface allows swapping Blockbook for other backends - Types reuse existing PreviousOutput for Lattice signing compatibility Includes 41 new tests across unit, integration, and e2e test suites.
Move BTC utilities from packages/sdk/src/btc/ to new packages/btc/ package for independent versioning and reuse. Changes: - Create @gridplus/btc package with slip132, network, tx, wallet utilities - SDK now depends on @gridplus/btc via workspace:* - SDK re-exports @gridplus/btc for backward compatibility - Keep xpub.ts in SDK (has Lattice-specific dependencies) - Add type guards and safeParseInt for robustness - Use Uint8Array instead of Buffer for browser compatibility
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e309972391
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return [ | ||
| purpose + HARDENED_OFFSET, | ||
| coinType + HARDENED_OFFSET, | ||
| HARDENED_OFFSET, | ||
| 1, |
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.
Use the correct account index for change path
The change derivation path hard-codes the account component to 0' (HARDENED_OFFSET) instead of using the account that the UTXOs/xpub belong to. If a caller builds a transaction for a non‑zero account (e.g., they fetched an account-1 xpub via getXpub({ account: 1 }) and pass its UTXOs here), the change output will be sent to account 0, which is not covered by the account-1 xpub and will appear “missing” to the wallet. This is a functional regression for multi‑account use; the change path should be derived from the UTXO path or be provided explicitly.
Useful? React with 👍 / 👎.
| const hasSegwit = inputType !== 'p2pkh'; | ||
| const segwitOverhead = hasSegwit ? VBYTE_SIZES.SEGWIT_MARKER : 0; | ||
|
|
||
| const outputSize = VBYTE_SIZES.P2WPKH_OUTPUT; | ||
|
|
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.
Estimate output size by script type, not fixed P2WPKH
Fee estimation always treats outputs as P2WPKH (outputSize = P2WPKH_OUTPUT), even when the wallet purpose is legacy or wrapped segwit. For purpose 44/49, the change output is P2PKH or P2SH‑P2WPKH, which are larger; this underestimates vbytes and can select too few inputs or produce too-low fees for non‑segwit outputs. This shows up when building legacy/wrapped transactions, where fee and change calculations rely on this estimate.
Useful? React with 👍 / 👎.
The cleanup steps (show logs, stop simulator) used working-directory which fails if the simulator checkout didn't happen. Now checks if directory exists before accessing files.
The SDK depends on @gridplus/btc via workspace:*, so btc must be built first. Updated root package.json scripts to build/test btc package before SDK.
commit: |
📝 Summary
🔧 Context / Implementation
🧪 Test Plan
🖼️ Screenshots (if applicable)