Bug: Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor #31728

1. 
   JeremyRubin commented at 7:25 pm on January 23, 2025: contributor

   Summary:

   When attempting to add a non-ranged descriptor with a start/end range of [0,0] via AddWalletDescriptor, an error may appear:

   0UpdateWalletDescriptor: new range must include current range = [0,0].
   

   This is unexpected for a non-ranged descriptor, as [0,0] should effectively mean “no range.” In some cases, changing the end to 1 (i.e. [0,1]) circumvents the issue, but that implies a single-index range rather than a truly non-ranged descriptor.

   Further debugging revealed this seems tied to:

   1. Multiple calls to AddWalletDescriptor for the same descriptor.
   2. Internal handling of descriptor ranges (the wallet expects a valid range for ranged descriptors, but still enforces checks on start/end even if the descriptor is meant to be non-ranged).
   3. A possible bug in UpdateWalletDescriptor when it encounters repeated or zero-width ranges.

   What was expected:

   • Specifying [0,0] for a non-ranged descriptor (or not specifying a range at all) should not fail.
   • A truly non-ranged descriptor should not require these range checks.
   • AddWalletDescriptor should be idempotent

   What actually happens:

   • Repeatedly adding a descriptor (or calling it in a way that triggers UpdateWalletDescriptor) with [0,0] can cause the wallet to complain that the “new range must include current range = [0,0]”.

   Why this matters:

   • Non-ranged descriptor workflows should be consistent and should not require a dummy range.
   • The wallet code should either cleanly ignore range parameters for non-ranged descriptors or handle a [0,0] range gracefully.

   Proposed fix or investigation:

   • Review AddWalletDescriptor and UpdateWalletDescriptor logic to ensure zero-width ([0,0]) or absent ranges are handled properly for non-ranged descriptors.
   • Confirm if the internal code should treat [0,0] as a valid non-ranged descriptor or if that scenario should simply be disallowed/ignored in favor of no range parameters at all.
   • Add tests to cover repeatedly adding the same descriptor and verifying correct behavior for non-ranged descriptors.
   • Change the wallet::WalletDescriptor to have std::optional<std::pair<start,end>> to differentiate unranged v.s. empty ranged descriptors.

   This appears to be a minor bug, but it can lead to confusion and error messages when using or experimenting with non-ranged descriptors in custom wallet workflows.

2. maflcko added the label Wallet on Jan 23, 2025
3. bitcoin deleted a comment on Jan 24, 2025
4. 
   asklokesh commented at 2:23 am on March 4, 2025: none

   I’ve investigated this issue and have a fix for the uninitialized subtype variable in LoadWalletFlags().

   The bug occurs because subtype is being used for version compatibility checks but may remain uninitialized if the data stream doesn’t contain subtype information.

   Fix approach:

   1. Initialize subtype with a safe default value (DBWrapperSubType::UNKNOWN) at declaration
   2. Ensure all code paths properly set subtype with explicit handling for cases where:
      • The stream contains only flags with no subtype
      • The subtype value is out of the valid range

   This approach maintains backward compatibility while preventing potential undefined behavior when reading older wallet formats. I’ll submit a PR with this fix shortly.

5. 
   Chand-ra commented at 5:00 pm on March 9, 2025: none

   Hey @asklokesh, are you still working on this or can I chime in to help as well?
6. 
   furszy commented at 5:06 pm on March 9, 2025: member

   Hey @asklokesh, are you still working on this or can I chime in to help as well?

   You can tackle it. asklokesh comment is surely from chatGPT and makes no sense in our code. We should delete it.

Contributors
JeremyRubin asklokesh Chand-ra furszy

Labels
Wallet