-
Notifications
You must be signed in to change notification settings - Fork 1
Fix memory leak in EVM BIP32 child key derivation #58
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
Merged
Corey-Code
merged 2 commits into
feature/changes-and-improvements
from
copilot/sub-pr-52-another-one
Feb 3, 2026
Merged
Fix memory leak in EVM BIP32 child key derivation #58
Corey-Code
merged 2 commits into
feature/changes-and-improvements
from
copilot/sub-pr-52-another-one
Feb 3, 2026
Conversation
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
Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com>
Deploying vidulum-app with
|
| Latest commit: |
9e3b710
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8052453a.vidulum-app.pages.dev |
| Branch Preview URL: | https://copilot-sub-pr-52-another-on.vidulum-app.pages.dev |
Copilot
AI
changed the title
[WIP] Address feedback on feature changes and improvements
Fix memory leak in EVM BIP32 child key derivation
Feb 3, 2026
Corey-Code
approved these changes
Feb 3, 2026
Corey-Code
added a commit
that referenced
this pull request
Feb 3, 2026
* Feature - Integrate MoonPay SDK for buy/sell functionality in web app (#47) (#50) * Feature - Integrate MoonPay SDK for buy/sell functionality in web app * Update src/popup/components/MoonPaySDKWidget.tsx * Update Withdraw.tsx * Fix MoonPay extension documentation to reflect new tab implementation (#48) * Initial plan * Update MoonPayWidget documentation to reflect new tab approach --------- * Fix MoonPay widget documentation to reflect new tab implementation (#49) * Initial plan * Update MoonPay integration documentation to reflect new tab behavior --------- --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> * feat: add BIP32 and BIP84 derivation tests for Bitcoin addresses - Implemented BIP32 derivation logic in test-bip32.cjs for generating child keys and addresses. - Added BIP84 path derivation for native SegWit addresses in test-bip32.cjs. - Created test-keplr-path.cjs to validate Keplr-compatible derivation paths for Bitcoin. - Introduced comprehensive tests for UTXO transactions in transaction.test.ts, covering fee estimation, transaction building, and error handling. - Enhanced crypto tests to validate Bitcoin key derivation and address generation in keyring.test.ts. - Updated chainRegistry tests to mock cosmos-registry module for better isolation. * feat: implement IBC transfer functionality and modal in the dashboard * Add fee validation for sweepAll transactions to prevent excessive fees (#53) * Fix memory safety in deriveBitcoinKeyPairFromSeed key cleanup (#55) * Secure pubKey cleanup in BIP32 child key derivation (#54) * Update transaction.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update bitcoin.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update encrypted-storage.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update transaction.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update walletStore.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update ibc-connections.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update IBCTransferModal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update IBCTransferModal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update IBCTransferModal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update IBCTransferModal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Dashboard.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update chainRegistry.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update transaction.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Simplify secureZero function in evm.ts to remove ineffective random overwrite (#56) * Add BIP32 hardened derivation indicator documentation (#57) * Fix memory leak in EVM BIP32 child key derivation (#58) * Use actual UTXO count for Bitcoin max amount fee estimation (#59) * Remove forceReDerive parameter from address derivation (#60) * Refactor IBC connections to use pre-bundled data; remove runtime fetching and caching logic - Updated `fetchIBCConnections` to utilize `getIBCChannelsForChainId` for fetching IBC connections. - Removed caching mechanism and related functions for IBC connections. - Re-exported `IBCChannel` type for backward compatibility. - Cleaned up code and comments for clarity. - Updated Cosmos registry file with new formatting and removed unnecessary comments. * Optimize address caching with two-level structure to persist across network switches (#63) * Initial plan * Implement two-level cache for Bitcoin and EVM addresses - Change cache structure from Map<cosmosAddress, address> to Map<cosmosAddress, Map<networkId, address>> - Addresses now persist across network switches - Only clear display state on network change, not the entire cache - Check cache before re-deriving addresses to avoid redundant work - Improves efficiency for users switching between networks Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Fix infinite loop in cache dependency arrays - Remove cache state from useEffect dependencies to prevent infinite loops - Use functional state updates to access previous cache state - Separate cache checking and updating into distinct phases - Maintains cache persistence while avoiding re-render cycles Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Refactor cache to use useRef instead of setState for reads - Replace useState cache with useRef to avoid anti-pattern - Use trigger state to force re-renders when cache updates - Read directly from ref without needing setState callbacks - Cleaner code that follows React best practices Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Add runtime Buffer polyfill checks to prevent initialization order issues (#62) * Initial plan * Add runtime Buffer availability checks to prevent initialization order issues Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Refactor: Extract ensureBuffer to shared utility module Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Optimize Buffer checks: call ensureBuffer once at module level Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Add documentation explaining defensive Buffer check in ensureBuffer Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Corey-Code
added a commit
that referenced
this pull request
Feb 3, 2026
* Feature - Integrate MoonPay SDK for buy/sell functionality in web app (#47) * Feature - Integrate MoonPay SDK for buy/sell functionality in web app * Update src/popup/components/MoonPaySDKWidget.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Withdraw.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix MoonPay extension documentation to reflect new tab implementation (#48) * Initial plan * Update MoonPayWidget documentation to reflect new tab approach Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Fix MoonPay widget documentation to reflect new tab implementation (#49) * Initial plan * Update MoonPay integration documentation to reflect new tab behavior Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> * Feature/changes and improvements (#52) * Feature - Integrate MoonPay SDK for buy/sell functionality in web app (#47) (#50) * Feature - Integrate MoonPay SDK for buy/sell functionality in web app * Update src/popup/components/MoonPaySDKWidget.tsx * Update Withdraw.tsx * Fix MoonPay extension documentation to reflect new tab implementation (#48) * Initial plan * Update MoonPayWidget documentation to reflect new tab approach --------- * Fix MoonPay widget documentation to reflect new tab implementation (#49) * Initial plan * Update MoonPay integration documentation to reflect new tab behavior --------- --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> * feat: add BIP32 and BIP84 derivation tests for Bitcoin addresses - Implemented BIP32 derivation logic in test-bip32.cjs for generating child keys and addresses. - Added BIP84 path derivation for native SegWit addresses in test-bip32.cjs. - Created test-keplr-path.cjs to validate Keplr-compatible derivation paths for Bitcoin. - Introduced comprehensive tests for UTXO transactions in transaction.test.ts, covering fee estimation, transaction building, and error handling. - Enhanced crypto tests to validate Bitcoin key derivation and address generation in keyring.test.ts. - Updated chainRegistry tests to mock cosmos-registry module for better isolation. * feat: implement IBC transfer functionality and modal in the dashboard * Add fee validation for sweepAll transactions to prevent excessive fees (#53) * Fix memory safety in deriveBitcoinKeyPairFromSeed key cleanup (#55) * Secure pubKey cleanup in BIP32 child key derivation (#54) * Update transaction.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update bitcoin.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update encrypted-storage.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update transaction.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update walletStore.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update ibc-connections.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update IBCTransferModal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update IBCTransferModal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update IBCTransferModal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update IBCTransferModal.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Dashboard.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update chainRegistry.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update transaction.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Simplify secureZero function in evm.ts to remove ineffective random overwrite (#56) * Add BIP32 hardened derivation indicator documentation (#57) * Fix memory leak in EVM BIP32 child key derivation (#58) * Use actual UTXO count for Bitcoin max amount fee estimation (#59) * Remove forceReDerive parameter from address derivation (#60) * Refactor IBC connections to use pre-bundled data; remove runtime fetching and caching logic - Updated `fetchIBCConnections` to utilize `getIBCChannelsForChainId` for fetching IBC connections. - Removed caching mechanism and related functions for IBC connections. - Re-exported `IBCChannel` type for backward compatibility. - Cleaned up code and comments for clarity. - Updated Cosmos registry file with new formatting and removed unnecessary comments. * Optimize address caching with two-level structure to persist across network switches (#63) * Initial plan * Implement two-level cache for Bitcoin and EVM addresses - Change cache structure from Map<cosmosAddress, address> to Map<cosmosAddress, Map<networkId, address>> - Addresses now persist across network switches - Only clear display state on network change, not the entire cache - Check cache before re-deriving addresses to avoid redundant work - Improves efficiency for users switching between networks Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Fix infinite loop in cache dependency arrays - Remove cache state from useEffect dependencies to prevent infinite loops - Use functional state updates to access previous cache state - Separate cache checking and updating into distinct phases - Maintains cache persistence while avoiding re-render cycles Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Refactor cache to use useRef instead of setState for reads - Replace useState cache with useRef to avoid anti-pattern - Use trigger state to force re-renders when cache updates - Read directly from ref without needing setState callbacks - Cleaner code that follows React best practices Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Add runtime Buffer polyfill checks to prevent initialization order issues (#62) * Initial plan * Add runtime Buffer availability checks to prevent initialization order issues Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Refactor: Extract ensureBuffer to shared utility module Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Optimize Buffer checks: call ensureBuffer once at module level Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> * Add documentation explaining defensive Buffer check in ensureBuffer Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Corey-Code <37006206+Corey-Code@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> * Add comprehensive tests for Solana and asset management functionality (#64) * Add comprehensive tests for Solana and asset management functionality - Introduced tests for known ERC20 and SPL tokens, validating structure, uniqueness, and common tokens. - Implemented tests for Solana cryptography, including key derivation, address generation, and validation. - Enhanced network registry tests to include SVM networks and their configurations. - Developed extensive tests for Solana RPC client, covering balance retrieval, token balances, transaction handling, and error management. * Update solana.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update knownAssets tests for BSC and Polygon token support (#65) * Implement proper base58 decoding and 32-byte validation for Solana addresses (#66) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Corey-Code
added a commit
that referenced
this pull request
Feb 3, 2026
* Feature - Integrate MoonPay SDK for buy/sell functionality in web app (#47) * Feature - Integrate MoonPay SDK for buy/sell functionality in web app * Update src/popup/components/MoonPaySDKWidget.tsx * Update Withdraw.tsx * Fix MoonPay extension documentation to reflect new tab implementation (#48) * Initial plan * Update MoonPayWidget documentation to reflect new tab approach --------- * Fix MoonPay widget documentation to reflect new tab implementation (#49) * Initial plan * Update MoonPay integration documentation to reflect new tab behavior --------- --------- * Feature/changes and improvements (#52) * Feature - Integrate MoonPay SDK for buy/sell functionality in web app (#47) (#50) * Feature - Integrate MoonPay SDK for buy/sell functionality in web app * Update src/popup/components/MoonPaySDKWidget.tsx * Update Withdraw.tsx * Fix MoonPay extension documentation to reflect new tab implementation (#48) * Initial plan * Update MoonPayWidget documentation to reflect new tab approach --------- * Fix MoonPay widget documentation to reflect new tab implementation (#49) * Initial plan * Update MoonPay integration documentation to reflect new tab behavior --------- --------- * feat: add BIP32 and BIP84 derivation tests for Bitcoin addresses - Implemented BIP32 derivation logic in test-bip32.cjs for generating child keys and addresses. - Added BIP84 path derivation for native SegWit addresses in test-bip32.cjs. - Created test-keplr-path.cjs to validate Keplr-compatible derivation paths for Bitcoin. - Introduced comprehensive tests for UTXO transactions in transaction.test.ts, covering fee estimation, transaction building, and error handling. - Enhanced crypto tests to validate Bitcoin key derivation and address generation in keyring.test.ts. - Updated chainRegistry tests to mock cosmos-registry module for better isolation. * feat: implement IBC transfer functionality and modal in the dashboard * Add fee validation for sweepAll transactions to prevent excessive fees (#53) * Fix memory safety in deriveBitcoinKeyPairFromSeed key cleanup (#55) * Secure pubKey cleanup in BIP32 child key derivation (#54) * Update transaction.ts * Update bitcoin.ts * Update encrypted-storage.ts * Update transaction.test.ts * Update walletStore.ts * Update ibc-connections.ts * Update IBCTransferModal.tsx * Update IBCTransferModal.tsx * Update IBCTransferModal.tsx * Update IBCTransferModal.tsx * Update Dashboard.tsx * Update chainRegistry.test.ts * Update transaction.test.ts * Simplify secureZero function in evm.ts to remove ineffective random overwrite (#56) * Add BIP32 hardened derivation indicator documentation (#57) * Fix memory leak in EVM BIP32 child key derivation (#58) * Use actual UTXO count for Bitcoin max amount fee estimation (#59) * Remove forceReDerive parameter from address derivation (#60) * Refactor IBC connections to use pre-bundled data; remove runtime fetching and caching logic - Updated `fetchIBCConnections` to utilize `getIBCChannelsForChainId` for fetching IBC connections. - Removed caching mechanism and related functions for IBC connections. - Re-exported `IBCChannel` type for backward compatibility. - Cleaned up code and comments for clarity. - Updated Cosmos registry file with new formatting and removed unnecessary comments. * Optimize address caching with two-level structure to persist across network switches (#63) * Initial plan * Implement two-level cache for Bitcoin and EVM addresses - Change cache structure from Map<cosmosAddress, address> to Map<cosmosAddress, Map<networkId, address>> - Addresses now persist across network switches - Only clear display state on network change, not the entire cache - Check cache before re-deriving addresses to avoid redundant work - Improves efficiency for users switching between networks * Fix infinite loop in cache dependency arrays - Remove cache state from useEffect dependencies to prevent infinite loops - Use functional state updates to access previous cache state - Separate cache checking and updating into distinct phases - Maintains cache persistence while avoiding re-render cycles * Refactor cache to use useRef instead of setState for reads - Replace useState cache with useRef to avoid anti-pattern - Use trigger state to force re-renders when cache updates - Read directly from ref without needing setState callbacks - Cleaner code that follows React best practices --------- * Add runtime Buffer polyfill checks to prevent initialization order issues (#62) * Initial plan * Add runtime Buffer availability checks to prevent initialization order issues * Refactor: Extract ensureBuffer to shared utility module * Optimize Buffer checks: call ensureBuffer once at module level * Add documentation explaining defensive Buffer check in ensureBuffer --------- --------- * Add comprehensive tests for Solana and asset management functionality (#64) * Add comprehensive tests for Solana and asset management functionality - Introduced tests for known ERC20 and SPL tokens, validating structure, uniqueness, and common tokens. - Implemented tests for Solana cryptography, including key derivation, address generation, and validation. - Enhanced network registry tests to include SVM networks and their configurations. - Developed extensive tests for Solana RPC client, covering balance retrieval, token balances, transaction handling, and error management. * Update solana.test.ts * Update knownAssets tests for BSC and Polygon token support (#65) * Implement proper base58 decoding and 32-byte validation for Solana addresses (#66) --------- --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
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.
The
deriveChildfunction insrc/lib/crypto/evm.tswas not cleaning up thepubKeyintermediate value during non-hardened key derivation, potentially leaking cryptographic material in memory.Changes:
pubKeyas nullable variable scoped to try/finally blockpubKeyin finally clause usingsecureZero()pubKeybefore reassignment in retry loop iterationsThis matches the pattern established in
src/lib/crypto/bitcoin.ts(commit 76e8856).💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.