scripted-diff: test: remove ‘descriptors=True’ argument for createwallet calls #32544

1. 
   theStack commented at 4:37 pm on May 17, 2025: contributor

   Descriptor wallets are already created by default since v23.0, but since the recent legacy wallet removal the descriptors parameter must be True for the createwallet RPC (see commit 9f04e02ffaee0fe64027dc56c7bea3885254321a), i.e. still passing it wouldn’t contain any information for test readers anymore. So simply drop them in the functional tests in order to reduce code bloat. The only exception is calls to older versions, which happens in wallet_backwards_compatibility.py and is explicitly excluded in the scripted diff.
2. scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls

   Descriptor wallets are already created by default since v23.0, but
   since the recent legacy wallet removal this parameter *must* be True
   (see commit 9f04e02ffaee0fe64027dc56c7bea3885254321a), i.e. still
   passing it wouldn't contain any information for test readers
   anymore. So simply drop them in the functional tests in order to
   reduce code bloat.
   
   -BEGIN VERIFY SCRIPT-
   sed -i 's/, descriptors=True//g' $(git ls-files -- 'test/functional' ':(exclude)test/functional/wallet_backwards_compatibility.py')
   sed -i '/descriptors=True,/d' ./test/functional/mempool_persist.py
   -END VERIFY SCRIPT-

   86de8c1668
3. 
   DrahtBot commented at 4:37 pm on May 17, 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/32544.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	Sjors, maflcko
   Concept ACK	1440000bytes

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

4. DrahtBot added the label Refactoring on May 17, 2025
5. 
   1440000bytes commented at 4:47 pm on May 17, 2025: none

   Concept ACK

   The argument descriptors could also be removed from createwallet RPC.

6. DrahtBot added the label CI failed on May 17, 2025
7. 
   DrahtBot commented at 5:23 pm on May 17, 2025: contributor

   🚧 At least one of the CI tasks failed. Task CentOS, depends, gui: https://github.com/bitcoin/bitcoin/runs/42411027184 LLM reason (✨ experimental): The CI failed because the AddressBookTests::addressBookTests() test failed, resulting in an assertion mismatch.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

8. 
   Sjors commented at 8:02 am on May 19, 2025: member

   ACK 86de8c1668005304b2c630ca2ad4a8ca8e348e90

   The argument descriptors could also be removed from createwallet RPC.

   Unfortunately not, as long as we support positional arguments. We can ignore it though.

9. 
   maflcko commented at 8:13 am on May 19, 2025: member

   lgtm ACK 86de8c1668005304b2c630ca2ad4a8ca8e348e90

   ci failure is obviously intermittent and unrelated

10. fanquake merged this on May 19, 2025
11. fanquake closed this on May 19, 2025

    ---

12. theStack deleted the branch on May 19, 2025

Contributors
theStack DrahtBot 1440000bytes Sjors maflcko

Labels
Refactoring CI failed