Issue Details

Number

30686

Title

wallet: lastprocessedblock can be inconsistent with internal best block

Description

### Is there an existing issue for this? - [X] I have searched the existing issues ### Current behaviour `m_last_block_processed` and `m_last_block_processed_height`, which the users can see in their `getwalletinfo()` result as `lastprocessedblock` > `height`, are not guaranteed to match the block locator stored in the wallet. In practice this means that if the user checks this height and then creates a backup of the wallet they might expect that the wallet will only need to continue syncing from the `lastprocessedblock` when the backup is loaded later. However, this may or may not be the case. Where the wallet starts syncing depends on when the block locator was last written to disk and that is not clear to the user. An example of how this created confusion in the `wallet_assumeutxo.py` case can be seen here: https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1719951882 ### Expected behaviour The preferred way of dealing with this seems to be that the block locator is updated either every time the `lastprocessedblock` is updated or at least on "pull basis" every time the user interacts with these values. I.e. when the user might create a backup the block locator should be updated to be in sync with the current `lastprocessedblock`. Another more minimalistic approach could be to show users the relevant block locator. That may be in `getwalletinfo()` or the return value of `backupwallet()`. I do think that this would still require extra documentation effort to explain to users how these values are different and how both of them relevant. ### Steps to reproduce This commit demonstrates the problem well though it does require understanding of assumeutxo: https://github.com/bitcoin/bitcoin/pull/30678/commits/4ffdc9b70711b2bec0e6e86a4f2cc0a87de406d2 The backup `backup_w.dat` is created at height 299. Loading it fails and the test says that it fails because it is part of the background sync (the IBD chain) and at this point the background sync has not finished meaning that the blocks that are needed are not available for scanning. However, height 299 is actually not part of the background sync. It is the snapshot height. The test only works on master because the block locator is not updated before the backup, so a block previously of 299 is where the wallet is trying to start syncing and that block is then indeed part of the background sync. The commit above changes this by writing the best block locator before the backup is created. This makes the test fail in the form it is on master. The commit then changes the test by creating a backup that is older (at height 199). This height is part of the background sync so the test now works again because it does not depend on the best block locator behavior anymore. While the test does not make it explicit, when trying to change the test @alfonsoromanz observed this behavior and relied on `lastprocessedblock` to tell him what the start point for the sync would be when the backup was loaded (same link as above: https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1719951882) ### Relevant log output _No response_ ### How did you obtain Bitcoin Core Compiled from source ### What version of Bitcoin Core are you using? master ### Operating system and version macos ### Machine specifications _No response_

URL

https://github.com/bitcoin/bitcoin/issue/30686

Closed by

Notes

Attention of

Kill Factor (1-10)

Save

Back to List