perf(hdwallet): speed up ExportHDAddresses ~10-15x - #994
Open
praetoriansentry wants to merge 4 commits into
Open
perf(hdwallet): speed up ExportHDAddresses ~10-15x#994praetoriansentry wants to merge 4 commits into
praetoriansentry wants to merge 4 commits into
Conversation
Deriving N addresses re-derived the full BIP32 path from the master key for every index, costing ~7 slow big.Int EC multiplies per address in go-bip32, plus duplicate PublicKey() and uncompressed-pubkey computations. Now the shared parent key is derived once with a single NewChildKey per index, each key's public keys are computed once and reused, and the per-address export runs in parallel bounded by runtime.NumCPU() with deterministic output ordering. polycli wallet inspect --addresses 500: 6.6s -> 0.65s polycli wallet inspect --addresses 10000: minutes -> 8.4s Output is byte-identical to the previous implementation; a new test verifies all 500 exported keys against independent per-path derivation and golden values captured from the old code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CodeQL flagged the strconv.Atoi result flowing into uint32(i) without a bound check it could verify. Parse the derivation path address index with strconv.ParseUint(s, 10, 32) so the value is provably within the bip32 child index range at the source, and return an explicit error when the index overflows instead of failing later during derivation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CodeQL flagged the uint64 result of ParseUint(s, 10, 32) converted to int, which can overflow on 32-bit platforms. Non-hardened bip32 child indexes are bounded by 2^31-1 anyway (this code path does not support hardened addresses), so parse with bitSize 31, which also guarantees the value fits in an int everywhere. Co-Authored-By: Claude Fable 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
3 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.
Description
polycli wallet inspect --addresses 10000took several minutes. The bottleneck washdwallet.ExportHDAddresses: go-bip32 uses a puremath/bigsecp256k1 implementation (~1ms per EC multiply), and the old code paid ~10 EC multiplies per address:m/44'/60'/0'/0/i) from the master key, even though the first 4 levels are identical for all addresses;PublicKey()and the uncompressed public key were each computed twice per address;Changes (all in
hdwallet/):NewChildKey(i)per address (~7 EC multiplies → 2).exportAddresshelper that computes the compressed/uncompressed public keys once and reuses them;toBTCAddressnow takes the public key and the redundanttoETHAddresswrapper is removed.ExportRootAddressgets the same dedup.errgroup, bounded atruntime.NumCPU(), writing into an index-addressed slice so output ordering is deterministic. Passes-race.No breaking changes: output is byte-identical to the previous implementation (verified by diffing 500-address JSON output from the old and new binaries).
cmd/fundalso callsExportHDAddressesand inherits the speedup.Benchmarks (i9-14900KS):
Jira / Linear Tickets
Testing
TestExportHDAddressesMatchesPerPathDerivation: exports 500 addresses and verifies every field (private/public/full keys, ETH, BTC, WIF) against independent per-path derivation from the master key, plus golden values captured from the pre-optimization implementation.BenchmarkExportHDAddressesfor future regression measurement.go test ./hdwallet/), including BIP32 test vectors and the 34-caseTestDerivationPath.go test -raceon the new test.polycli wallet inspect --mnemonic "..." --addresses 500diffed between old and new binaries; also smoke-tested--root-only,--addresses 1, and custom--path.🤖 Generated with Claude Code