Also check change (/1) derivation paths by default for UTXO wallets#716
Also check change (/1) derivation paths by default for UTXO wallets#716
Conversation
Agent-Logs-Url: https://github.com/3rdIteration/btcrecover/sessions/1880eb40-9d7a-45c0-9789-68cd9c1369cb Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
Agent-Logs-Url: https://github.com/3rdIteration/btcrecover/sessions/30be8a25-a306-4a66-9702-c6cd57dfdd45 Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds default-on scanning of “change” (/1) derivation paths for UTXO wallets so seeds funded only via internal/change addresses can still be recovered, with an opt-out flag.
Changes:
- Expand derivation path lists by mirroring eligible
/0paths to/1for wallets that support change addresses. - Add
--no-check-change-addressesCLI flag and document the behavior. - Add structural and end-to-end tests covering default behavior vs opt-out.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| docs/bip39-accounts-and-altcoins.md | Documents default change-address path checking and the opt-out flag. |
| btcrecover/test/test_seeds.py | Adds new tests validating path expansion behavior and recovery from change addresses. |
| btcrecover/btcrseed.py | Implements change-path expansion, wires it into wallet creation, and adds CLI support plus wallet opt-outs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _run_change_address_scenario(self, wallet_type, change_address, | ||
| correct_mnemonic, pathlist_file, | ||
| check_change_addresses, **kwds): | ||
| test_path = btcrseed.load_pathlist("./derivationpath-lists/" + pathlist_file) |
There was a problem hiding this comment.
The test uses a relative path that depends on the current working directory, which can break when tests are executed from a different CWD (e.g., some CI runners or IDE test harnesses). Prefer resolving the path relative to the repository root or this test file (e.g., using pathlib.Path(__file__) and .parent), then passing the resulting absolute/normalized path into load_pathlist.
| self.assertEqual( | ||
| result_disabled, (False, 1), | ||
| "Seed must NOT be found when change-address checking is disabled") |
There was a problem hiding this comment.
This assertion is brittle because it requires the exact (False, 1) tuple, including the attempt/count value, which may legitimately change if search behavior or iteration counts change. To make the test robust while still validating behavior, assert only that the “found” indicator is False (e.g., result_disabled[0] is False) and, if needed, separately assert the count is >= 1 rather than exactly 1.
| self.assertEqual( | |
| result_disabled, (False, 1), | |
| "Seed must NOT be found when change-address checking is disabled") | |
| self.assertIs( | |
| result_disabled[0], False, | |
| "Seed must NOT be found when change-address checking is disabled") | |
| self.assertGreaterEqual( | |
| result_disabled[1], 1, | |
| "Seed verification should perform at least one check when change-address checking is disabled") |
| parser.add_argument("--disable-bip84", action="store_true", help="Disable checking of BIP84 native SegWit (P2WPKH) addresses") | ||
| parser.add_argument("--pathlist", metavar="FILE", help="A list of derivation paths to be searched") | ||
| parser.add_argument("--no-check-change-addresses", action="store_true", | ||
| help="Disable the default behaviour of also checking matching \"change\" (internal, /1) derivation paths alongside \"receive\" (external, /0) paths for UTXO-style wallets (BTC, LTC, BCH, DASH, DGB, DOGE, GRS, MONA, VTC, etc). Has no effect for account-model wallets such as ETH, XRP, SOL, etc.") |
There was a problem hiding this comment.
The help= string is very long and hard to scan/update in a single line. Consider splitting it into multiple adjacent string literals within parentheses to improve readability and reduce diff churn when help text changes.
| help="Disable the default behaviour of also checking matching \"change\" (internal, /1) derivation paths alongside \"receive\" (external, /0) paths for UTXO-style wallets (BTC, LTC, BCH, DASH, DGB, DOGE, GRS, MONA, VTC, etc). Has no effect for account-model wallets such as ETH, XRP, SOL, etc.") | |
| help=( | |
| "Disable the default behaviour of also checking matching " | |
| "\"change\" (internal, /1) derivation paths alongside " | |
| "\"receive\" (external, /0) paths for UTXO-style wallets " | |
| "(BTC, LTC, BCH, DASH, DGB, DOGE, GRS, MONA, VTC, etc). " | |
| "Has no effect for account-model wallets such as ETH, XRP, " | |
| "SOL, etc." | |
| )) |
|
Previously only the supplied/default "receive" derivation paths (external chain,
…/0) were searched, so a seed funded only on a change address would be missed. This adds automatic, default-on checking of the matching change paths (…/1) for UTXO-style wallets, with a new opt-out flag.Path expansion (
btcrecover/btcrseed.py)WalletBIP32._has_change_addressesclass attribute (defaultTrue) and_expand_paths_for_change_addresses()helper that mirrors each path whose last (non-hardened) index is0into a sibling ending in1, preserving the script-type/display-string tables and skipping duplicates.WalletBIP32.create_from_params(andWalletAezeed.create_from_params) accept a newcheck_change_addresses=Truekwarg and invoke the helper after_append_last_indexhandling, only in the non-MPK branch (MPK mode is bound to a specific chaincode so mirroring is pointless).m/44'/0'/0') are left untouched; script-type filtering runs before expansion, so disabled types never produce change paths.Account-model wallets opt out
Set
_has_change_addresses = Falseon:WalletEthereum,WalletEthereumValidator,WalletHederaEd25519,WalletZilliqa,WalletCardano,WalletPyCryptoHDWallet(covers Solana/Avalanche/Stellar/Tron/Polkadot/Cosmos/Tezos/SecretNetwork/MultiversX),WalletHelium,WalletRipple,WalletStacks,WalletXLM.CLI & docs
--no-check-change-addressesflag threaded through tocreate_from_params.docs/bip39-accounts-and-altcoins.md.Tests (
btcrecover/test/test_seeds.py)New
TestChangeAddressesclass with 10 tests:--pathliststill expands, no duplicates when/1already listed, hardened indexes not rewritten, ETH never expands.check_change_addresses=False.Behaviour