scripted-diff: Update DeriveType enum values to mention ranged derivations #32745

1. 
   rkrux commented at 10:56 am on June 13, 2025: contributor

   While reviewing the MuSig2 descriptors PR #31244, I realized that the enum DeriveType here logically refers to the derive type for ranged descriptors. This became evident to me while going through the implementations of IsRanged & IsHardened functions of BIP32PubkeyProvider, and the ParsePubkeyInner function. Initially I got confused by reading IsRange translating to != DeriveType::NO, but later realised it specifically referred to the presence of ranged derivations. I propose explicitly mentioning “ranged” in the values of the DeriveType enum would make it easier to parse the descriptors code.

   This enum is used in one file only - script/descriptors.cpp. That’s why I explicitly passed it as the argument in the sed command in the script below.

   -BEGIN VERIFY SCRIPT- sed -i ’s/HARDENED/HARDENED_RANGED/g’ src/script/descriptor.cpp sed -i ’s/NO,/NON_RANGED,/g’ src/script/descriptor.cpp sed -i ’s/DeriveType::NO/DeriveType::NON_RANGED/g’ src/script/descriptor.cpp -END VERIFY SCRIPT-

2. 
   DrahtBot commented at 10:56 am on June 13, 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/32745.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

3. DrahtBot added the label Refactoring on Jun 13, 2025
4. rkrux force-pushed on Jun 13, 2025
5. 
   PeterWrighten commented at 11:28 am on June 13, 2025: none

   Good catch! But I consider you should better check other codes which also use this DeriveType, to make sure it is only used for Ranged descriptors.
6. rkrux marked this as ready for review on Jun 13, 2025
7. scripted-diff: Update `DeriveType` enum values to mention ranged derivations

   While reviewing the MuSig2 descriptors PR 31244, I realized that the enum
   `DeriveType` here logically refers to the derive type for ranged descriptors.
   This became evident to me while going through the implementations of `IsRanged`
    & `IsHardened` functions of `BIP32PubkeyProvider`, and the `ParsePubkeyInner`
   function. Initially I got confused by reading `IsRange` translating to
   `!= DeriveType::NO`, but later realised it specifically referred to the presence
    of ranged derivations. I propose explicitly mentioning "ranged" in the values
   of the `DeriveType` enum would make it easier to parse the descriptors code.
   
   This enum is used in one file only - `script/descriptors.cpp`. That's why I
   explicitly passed it as the argument in the `sed` command in the script below.
   
   -BEGIN VERIFY SCRIPT-
   sed -i 's/HARDENED/HARDENED_RANGED/g' src/script/descriptor.cpp
   sed -i 's/NO,/NON_RANGED,/g' src/script/descriptor.cpp
   sed -i 's/DeriveType::NO/DeriveType::NON_RANGED/g' src/script/descriptor.cpp
   -END VERIFY SCRIPT-

   d60b0088e8
8. in src/script/descriptor.cpp:358 in f64880672b outdated

   351@@ -352,7 +352,7 @@ class ConstPubkeyProvider final : public PubkeyProvider
   352     }
   353 };
   354 
   355-enum class DeriveType {
   356+enum class RangedDeriveType {
   357     NO,
   358     UNHARDENED,
   359     HARDENED,
   

   ---

   ---

   

   hodlinator commented at 12:40 pm on June 13, 2025:

   Agree that based on the IsRange() implementation, this enum could be clearer. But would lean towards only changing the values, not the name in this case:

   0enum class DeriveType {
   1    SINGLE,
   2    UNHARDENED_RANGE,
   3    HARDENED_RANGE,
   

   ---

   

   rkrux commented at 11:46 am on June 14, 2025:

   Accepted the suggestion with some modification by using NON_RANGED instead of SINGLE, which felt incorrect to me. I do like reading DeriveType::NON_RANGED instead of the original DeriveType::NO, which made me feel like there was no child derivation at all and prompted me to raise the PR.

   I had to experiment with the sed command in the scripted diff a bit to get the right order working but it was fun.

   ---

   

   hodlinator commented at 9:20 pm on June 14, 2025:

   Happy with your variation. :+1:
9. rkrux force-pushed on Jun 14, 2025
10. rkrux renamed this:
    scripted-diff: Replace `DeriveType` with `RangedDeriveType`
    scripted-diff: Update DeriveType enum values to mention ranged derivations
    on Jun 14, 2025
11. in src/script/descriptor.cpp:1 in d60b0088e8

    ---

    ---

    

    hodlinator commented at 9:17 pm on June 14, 2025:

    Commit message nits:

    1. Could include # to more closely conform to https://github.com/bitcoin/bitcoin/blob/d60b0088e81e2ef81b9cfe2d4b808ed819a52962/CONTRIBUTING.md?plain=1#L127-L129
    2. Correct method name
    3. Adjust narrowness of script in both direction through relying more on word boundaries (\b), ending up with 2 lines instead of 3.

     0  scripted-diff: Update `DeriveType` enum values to mention ranged derivations
     1
     2- While reviewing the MuSig2 descriptors PR 31244, I realized that the enum
     3+ While reviewing the MuSig2 descriptors PR [#31244](/bitcoin-bitcoin/31244/), I realized that the enum
     4  `DeriveType` here logically refers to the derive type for ranged descriptors.
     5- This became evident to me while going through the implementations of `IsRanged`
     6+ This became evident to me while going through the implementations of `IsRange`
     7  & `IsHardened` functions of `BIP32PubkeyProvider`, and the `ParsePubkeyInner`
     8  function. Initially I got confused by reading `IsRange` translating to
     9  `!= DeriveType::NO`, but later realised it specifically referred to the presence
    10  of ranged derivations. I propose explicitly mentioning "ranged" in the values
    11  of the `DeriveType` enum would make it easier to parse the descriptors code.
    12
    13  This enum is used in one file only - `script/descriptors.cpp`. That's why I
    14  explicitly passed it as the argument in the `sed` command in the script below.
    15
    16  -BEGIN VERIFY SCRIPT-
    17- sed -i 's/HARDENED/HARDENED_RANGED/g' src/script/descriptor.cpp
    18+ sed -i 's/HARDENED\b/HARDENED_RANGED/g' src/script/descriptor.cpp
    19- sed -i 's/NO,/NON_RANGED,/g' src/script/descriptor.cpp
    20- sed -i 's/DeriveType::NO/DeriveType::NON_RANGED/g' src/script/descriptor.cpp
    21+ sed -i 's/\bNO\b/NON_RANGED/g' src/script/descriptor.cpp
    22  -END VERIFY SCRIPT-
    

12. 
    hodlinator commented at 9:22 pm on June 14, 2025: contributor

    Code reviewed d60b0088e81e2ef81b9cfe2d4b808ed819a52962

Contributors
rkrux DrahtBot PeterWrighten hodlinator

Labels
Refactoring