wallet: `NotifyCanGetAddressesChanged` when advancing `next_index` #34993

Even though TopUp() notifies, advancing next_index after can deplete available addresses, so make sure to notify any time it's changed.

This would manifest as users seeing a clickable Receive button in the GUI when in fact no address can be generated in some edge cases, e.g. when a user has a watch only wallet with a hardened derivation path and runs out of keys.

This feels like it's begging for:

1. a refactor to make it impossible to modify next_index or range_end without firing CanGetAddressesChanged
2. a test

I banged my head against the keyboard for a bit but I couldn't get either of these to fall out, I also tried massaging a few clankers into doing it but I couldn't get any results that seemed reasonable to me, still seems like a worthwhile fix so opening PR anyway.

I also included a moveonly commit to pair code that can change the result of CanGetAddresses() with the notification firing

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34993.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

This would manifest as users seeing a clickable Receive button in the GUI when in fact no address can be generated in some edge cases.

I don't think it is actually possible for this to occur.

The edge case that could cause this is if the wallet is encrypted and locked, and the active descriptor is one that has hardened child derivation. Once next_index exceeds the end of the range and TopUp is needed, the Receive button stops being able to give out new addresses. However, what we do in the GUI currently is that clicking "Receive" will actually show the passphrase dialog.

This seems like a better UX to me rather than disabling the button, so I'm unconvinced that this change is actually necessary.

This change won't impact how encrypted wallets + hardened xpubs are handled since: CanGetAddresses() returns true for wallets where HavePrivateKeys() == true:

https://github.com/bitcoin/bitcoin/blob/e9dd8631c32ae835f7afb8e0fd2b7caafa9314d9/src/wallet/scriptpubkeyman.cpp#L1175

This is relevant for watch-only wallets with hardened descriptors, e.g. #32489

This is relevant for watch-only wallets with hardened descriptors

Ah, right.

ACK 88bbce40fe8241b51174f0920a3ef036cdc3b4e1

Could encapsulate the range changes into a function, so we don't have to add this signal call everywhere?

Also, the flow currently is the following one (splitting it in two, even though it is the same flow):

1. User interacts with the GUI -> model -> interface -> wallet -> spkm -> signals NotifyCanGetAddressesChanged.

2. NotifyCanGetAddressesChanged -> wallet -> model -> interface -> wallet -> spkm -> CanGetAddresses().

Instead of doing this back and forth, it would be simpler to just pass the value directly: NotifyCanGetAddressesChanged(bool can_get_addresses).