wallet: Prepare for future upgrades by recording versions of last client to open and decrypt #32895

1. 
   achow101 commented at 8:13 pm on July 7, 2025: member

   When a wallet is automatically upgraded, we always do so in a way that allows the user to load their wallet in an older version. When the user loads their wallet into an upgraded version again, the wallet may not perform the automatic upgrade and end up with upgraded and non-upgraded material in the wallet.

   If we write some information about the last client to open the wallet, we would be able to detect these upgrade-downgrade-upgrade situations and perform another upgrade if so. However, in order for this to work, we need to have implemented writing such information into versions prior to the ones which introduce new automatic upgrades.

   We are already writing the CLIENT_VERSION into all wallets in a “version” record, although this record is not updated in the same way across all previous versions. Since v24.0, we are always updating the record when the client version differs, but prior to that, the record would only be updated when the client version is newer. Even so, we can use this record to store the last client version.

   The first commit decouples the written client version from the node version. This aids in development of new features. The version is entirely decoupled from previous versions by requiring all versions to have bit 30 set, and the initial version being 0 | (1 << 30). The versions just need to be at least 299900, and forcing bit 30 to be set was an easy way to achieve this.

   However, using version numbers is not as flexible as feature flags, so the second commit introduces the concept of Wallet Client Features. These flags are distinct from the existing Wallet Feature Flags since they are conceptually different. These are written in a separate record, and the wallet client version is incremented to (1 << 0) | (1 << 30).

   Lastly, since some upgrades may need private keys and can only occur after the wallet is decrypted, the third commit also adds a last decrypted features flags record to record the features of the last client to decrypt the wallet.

   The result is that we will now be able to detect a downgrade down to v24.0 and be able to implement in parallel multiple features that require automatic upgrades.

2. DrahtBot added the label Wallet on Jul 7, 2025
3. 
   DrahtBot commented at 8:13 pm on July 7, 2025: contributor

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

   Code Coverage & Benchmarks

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	ryanofsky

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
   • #31650 (refactor: Avoid copies by using const references or by move-construction by maflcko)
   • #27865 (wallet: Track no-longer-spendable TXOs separately by achow101)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

   LLM Linter (✨ experimental)

   Possible typos and grammar issues:

   • “might had been created” -> “might have been created” [incorrect past perfect modal construction]
   • “when a upgrade-downgrade-upgrade was performed” -> “when an upgrade-downgrade-upgrade was performed” [incorrect indefinite article before a vowel sound]

   drahtbot_id_4_m

4. 
   maflcko commented at 9:24 am on July 8, 2025: member

   I wonder if this can be uncoupled from CLIENT_VERSION, because the coupling is confusing and makes testing harder:

   • It is confusing, because switching between versions that kept the wallet code the same will pretend that there was a relevant upgrade/downgrade, even though the wallet behavior is exactly identical.
   • CLIENT_VERSION on the development branch is changed every 6 months, not when a new wallet feature was added that needs to deal with un-upgraded material. I know that running from the master branch is not supported and devs will have to deal with un-upgraded material themselves, but it seems easy to fix (and cleaner) to just introduce a new variable in the wallet that is equal to the current CLIENT_VERSION and then handle it separately and explicitly from now on.
5. 
   ryanofsky commented at 2:42 pm on July 8, 2025: contributor

   Concept ACK. It shouldn’t hurt to add more metadata to help detect when there might be a mix of old data in a newer format and new data in an older format in the same wallet.

   Conceptually, I think we could always detect these cases by just scanning the entire wallet for data that needs to be upgraded on loading. But having this extra metadata is nice because it can avoid slowing down wallet loading by avoiding unnecessary upgrade rescans.

   On the approach in e94ef51837daa5ec9552eb14e8ccc38d1b82b65b, I think it is ok, but would suggest some ways it might be improved:

   • I agree with Marco’s comment that it would be great to move away from CLIENT_VERSION and use something independent of the node version.
     • A downside of this suggestion is that the PR might not be able to reuse the existing VERSION field and might need to introduce 2 new fields instead of 1. I think the tradeoff could be worth it though.
   • It would be nice to avoid version numbers altogether and use feature flags instead, defining new feature flags when new data fields are introduced, and writing supported feature flags to LAST_OPENED_FEATURES & LAST_DECRYPTED_FEATURES fields on wallet loading.
     • This would not require introducing a new data type since we already have a WalletFlags enum. New internal and external features could just be new (nonmandatory) WalletFlags values.
     • IMO, flags are better than version numbers because they decouple code that implements new features from the merge and release process. They also make code more readable and maintainable because checks like if (last_opened_supported_specific_feature_x) are more obvious than if (last_opened_version > some_version_number_with_collection_of_features)
   • I assume we don’t care about detecting upgrades & downgrades to versions before v24.0 which wouldn’t overwrite the VERSION field on opening. But if we did, it’d be possible to detect this by writing last_opened and last_decrypted metadata to a file alongside the wallet database instead of inside the wallet database, and touching the file whenever the database is flushed. This way, if the database file modification time was greater than metadata file modification time, you could infer that the database had been loaded by an older version in the meantime and needs to be upgraded. Wouldn’t recommend this approach because it is more complicated, but wanted to mention it as an alternative to introducing new database fields.
6. 
   achow101 commented at 5:52 pm on July 8, 2025: member

   I wonder if this can be uncoupled from CLIENT_VERSION, because the coupling is confusing and makes testing harder:

   Definitely, however I’d like to be able to reuse the existing “version” record which mostly has the desired behavior since v24.0. The detection only works because of older versions overwriting the field with their CLIENT_VERSION. Introducing a new field now means that we cannot detect when an upgraded wallet was opened in any version older than v30.0 (assuming this is merged for the next release). The timeline would be too soon for any newly introduced features in say v31.0 or v32.0 (and I’ve got a few things in mind already that would utilize this). I’m already not particularly thrilled by the fact that the current implementation can only detect down to 24.0 as I expect that users doing upgrade-downgrade-upgrade with even older versions is commonplace.

   Conceptually, I think we could always detect these cases by just scanning the entire wallet for data that needs to be upgraded on loading.

   I don’t think that is actually viable as it would largely end up performing the upgrade on all loads regardless of whether the upgrade has already been done.

7. 
   maflcko commented at 6:28 pm on July 8, 2025: member

   Maybe I am missing something obvious, but my suggestion was not to introduce a new field/record. It was to re-use the existing field (and value) and manually handle the value going forward only when needed.

   That is, old wallets will (obviously) continue to write their CLIENT_VERSION to the version record. Also, any new wallet will continue to write a value of the current CLIENT_VERSION to it, just like on the current development branch (this is unchanged as well). The only change is that for the next release, the wallet will continue to write the same value as today, unless a feature was added that requires bumping the version, in which case the version is bumped in the pull request introducing the feature.

8. 
   achow101 commented at 6:34 pm on July 8, 2025: member

   Maybe I am missing something obvious, but my suggestion was not to introduce a new field/record. It was to re-use the existing field (and value) and manually handle the value going forward only when needed.

   Sorry, I was responding to @ryanofsky’s comment at the same time since they were very similar.

   I think that may be workable.

9. walletdb: Decouple last client record from CLIENT_VERSION

   Instead of tying the last client record with the node version, introduce
   a separate wallet client version that will be increased as necessary
   when new automatic upgrades are introduced.

   0edbe1f543
10. wallet: Introduce LastClientFeatures flags and LAST_OPENED_FEATURES record

    LastClientFeatures are feature flags indicating what automatic upgrade
    features supported by the last client that opened the wallet.
    
    The LAST_OPENED_FEATURES record stores these flags and must be
    set to match the exact features supported by the client that opens a
    wallet.

    3eae71b2c6
11. wallet: Record the version of the last client to decrypt a wallet f747394b62
12. achow101 force-pushed on Jul 8, 2025
13. 
    achow101 commented at 8:53 pm on July 8, 2025: member

    I’ve updated the PR to do a combination of @maflcko’s and @ryanofsky’s suggestions.

    The new approach decouples version from CLIENT_VERSION. It also writes out a set of client specific feature flags into two new records: lastopenedfeatures and lastdecryptedfeatures. However, these feature flags are separate from the existing wallet feature flags.

    I believe this preserves the behavior of detecting downgrades down to v24.0, while also giving us the flexibility of a new field and feature flags.

Contributors
achow101 DrahtBot maflcko ryanofsky

Labels
Wallet