Refactoring and Code Reuse#522
Merged
Merged
Conversation
Updated checksums for multiple pods in Podfile.lock, reflecting changes in dependency versions or sources. No source code changes included.
…tion Critical security fix for address validation: - Replace basic prefix/length checks with proper cryptographic validation - Validate Bech32 checksums for bc1 addresses (P2WPKH, P2WSH, P2TR) - Validate Base58Check checksums for legacy (P2PKH) and P2SH addresses - Add support for Taproot (P2TR/bc1p) address validation - Fix derivePath vs derive method usage for BIP32 path derivation This prevents users from accidentally sending funds to malformed addresses that passed the previous basic validation. https://claude.ai/code/session_01VfafSJryJaC3SeCBcTi4ES
Extract duplicated ECC validation, transaction size estimation, and address derivation utilities into a new services/ecc-utils.ts module. This consolidation: - Creates ECCLibrary interface for type-safe ECC library handling - Centralizes validateECCLibrary() and validateECCLibraryFull() functions - Unifies estimateTransactionSize() with documented constants - Consolidates address derivation utilities (deriveAddressIndexAndChainFromAddress, deriveAddressIndexFromAddress, findNextUnusedAddressIndex) - Centralizes generateChangeAddress() and generateCancellationAddress() - Provides single shared address index cache The same validation and derivation logic was previously duplicated across bitcoin-service.ts, rbf-service.ts, and cpfp-service.ts (~985 lines removed). https://claude.ai/code/session_01ExDrbBrFwxPbWtZEumt1hD
…ation-DDzUc Refactor: Extract shared ECC utilities into dedicated module
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+10
to
+19
| import { | ||
| validateECCLibrary, | ||
| validateECCLibraryFull, | ||
| estimateTransactionSize, | ||
| deriveAddressIndexAndChainFromAddress, | ||
| deriveAddressIndexFromAddress, | ||
| findNextUnusedAddressIndex, | ||
| generateCancellationAddress, | ||
| clearAddressIndexCache, | ||
| } from './ecc-utils'; |
Contributor
Author
There was a problem hiding this comment.
@claude can you check these are not required
validateECCLibrary
deriveAddressIndexFromAddress
findNextUnusedAddressIndex
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
2 participants
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.
This pull request refactors Bitcoin and CPFP service modules to centralize cryptographic and address utility functions into a shared
ecc-utilsmodule, improving code maintainability and reducing duplication. It also enhances Bitcoin address validation logic to be more robust and accurate, and streamlines ECC library validation and initialization.The most important changes are:
Refactoring and Code Reuse:
Moved utility functions such as
estimateTransactionSize,generateChangeAddress, andderiveAddressIndexAndChainFromAddressfrombitcoin-service.tsandcpfp-service.tsinto a sharedecc-utilsmodule, and updated imports accordingly. This reduces code duplication and centralizes cryptographic/address logic. [1] [2] [3] [4]Removed local cache and cache-clearing logic from
cpfp-service.ts, instead re-exporting cache clearing fromecc-utilsfor backward compatibility.Validation and Initialization Improvements:
validateECCLibraryFullfunction, simplifying and unifying the validation process. [1] [2] [3] [4]Bitcoin Address Validation:
isValidBitcoinAddressto usebitcoinjs-libfor checksum and format validation, supporting all major Bitcoin address types (P2WPKH, P2WSH, P2TR, P2PKH, P2SH) with proper error handling.BIP32 Derivation Consistency:
derivePathinstead ofderive, ensuring compatibility and correctness.