gui
gui copied to clipboard
[WIP/Draft] Add Address tab
This PR proposes a new tab called "Addresses" which shows some wallet addresses and also adds the listaddress
RPC.
This is a common feature that many wallets have, but not Bitcoin Core clients (GUI and bitcoin-cli).
Most of commits were picked from https://github.com/bitcoin/bitcoin/pull/23019, but they were considerably modified.
As stated in the title, it is still a draft. I would like to check if reviewers find this feature interesting before proceeding (since Bitcoin Core has a different security model and architecture than most wallets).
Some caveats:
. This PR, at moment (91717d3d0181ef3b211c38dde72d313acd25b5ea), seems to work fine for descriptor wallets as m_map_script_pub_keys
keeps all the keys, even after they have been generated (for receiving or change).
. But for legacy wallet, it has been showing inconsistent behavior. setInternalKeyPool
and setExternalKeyPool
remove the generated keys. I retrieved them from m_address_book
but apparently this only works for the receiving keys.
. To extract the index of a key in the HD path, this PR uses a regex. Although it seems reliable, may have performance impact. Ideally, regex should not be used. If this feature is implemented for descriptor wallets only, this is not necessary as ScriptPubKey Map
stores the index.
. The Address table shows m_hd_chain.m_next_{external,internal}_index + 5
addresses for legacy wallets and m_wallet_descriptor.next_index + 5
for descriptor. It means it shows up to the last generated address + 5.
. Just out of curiosity, I tested loading all addresses available in keypool (by default, 1000 keys for each keypool / descriptor) and this has a significant negative impact on wallet loading. I wonder if it would be possible to do this with good performance.
. pre-HD wallets have not been tested.
Feedback and suggestions on how to handle the issues mentioned above are welcome.
I thought this was deliberate, because addresses are intended to be single-use.
Concept NACK. This is just a more complicated/confusing address book.
Also, addresses do not have balances. They only receive, never send.
🐙 This pull request conflicts with the target branch and needs rebase.
Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
- Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
- Is it no longer relevant? ➡️ Please close.
- Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.