rpc: add ability to export/import descriptor files in `listdescriptors` and `importdescriptors` #25747

pull w0xlt wants to merge 5 commits into bitcoin:master from w0xlt:desc_file changing 6 files +201 −27

w0xlt commented at 5:30 AM on July 30, 2022: contributor
This PR adds a new parameter save_to_file to listdescriptors RPC, allowing the user to save the descriptors directly to a file and also adds a new parameter desriptor_file to importdescriptors RPC, allowing the user to import the descriptors directly from a file.

The first and second commits add the options parameter in a way that maintains compatibility with the previous private parameter.

With this PR, exporting / importing descriptors and creating watch-only wallet is as simple as:
```
$ bitcoin-cli -rpcwallet="cold_wallet"  listdescriptors "{\"save_to_file\": \"/home/user/Downloads/desc.json\"}"
$ bitcoin-cli -rpcwallet="watch_only_wallet"  importdescriptors "{\"descriptor_file\": \"/home/user/Downloads/desc.json\"}"
```
Without this PR, the user can manually create a file from the listdescriptors output or use jq to extract the descriptor field from listdescriptors but the idea here is to make things more automated and standardized.

And this also opens the way to add the functionality of importing descriptors from files in the GUI,
RPC: Support specifying different types for param aliases
author: Luke Dashjr <luke-jr+git@utopios.org>
f7337ff6e6

rpc: change `listdescriptors` to accept `options` parameter

This changes `listdescriptors` to accept `options` parameter
but retains compatibility with the previous `private` parameter.

9569ff9273

DrahtBot added the label RPC/REST/ZMQ on Jul 30, 2022
kristapsk commented at 6:45 AM on July 30, 2022: contributor

What's the point? Why can't you just redirect bitcoin-cli output to the file?
DrahtBot commented at 2:03 PM on July 30, 2022: contributor


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #26156 (test: check that listdescriptors descriptor strings are sorted by theStack)
- #26074 (refactor: Set RPCArg options with designated initializers by MarcoFalke)
- #22838 (descriptors: Be able to specify change and receiving in a single descriptor string 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.
w0xlt force-pushed on Jul 31, 2022
w0xlt commented at 5:44 AM on July 31, 2022: contributor
@kristapsk with #25757, perhaps the purpose of this PR has become clearer.

From the #25757 description: With these PRs, exporting / importing descriptors and creating watch-only wallet is as simple as:
```
$ bitcoin-cli -rpcwallet="cold_wallet"  listdescriptors "{\"save_to_file\": \"/home/user/Downloads/desc.json\"}"
$ bitcoin-cli -rpcwallet="watch_only_wallet"  importdescriptors "{\"descriptor_file\": \"/home/user/Downloads/desc.json\"}"
```
Without these PRs, the user can manually create a file from the listdescriptors output or use jq to extract the descriptor field from listdescriptors but the idea here is to make things more automated and standardized.

And this also opens the way to add the functionality of importing descriptors from files in the GUI,
aureleoules commented at 9:01 AM on August 2, 2022: member

Concept ACK considering #25757 exists. I think you should add the other PR's commits to this one. I believe this makes exporting and importing descriptors easier.

in test/functional/wallet_listdescriptors.py:90 in c8840eb855 outdated

  85 | +            'descriptor_file': desc_file
  86 | +            }, wallet.listdescriptors({'save_to_file': desc_file}))
  87 | +
  88 | +        with open(desc_file, encoding="utf8") as json_file:
  89 | +            assert_equal(expected['descriptors'], json.load(json_file))
  90 |

aureleoules commented at 9:10 AM on August 2, 2022:

maybe add a case when the file already exists?

w0xlt commented at 7:08 PM on August 2, 2022:

Done in https://github.com/bitcoin/bitcoin/pull/25747/commits/dd96b3d96b061c05732226aff5bc4570da1284af.

aureleoules commented at 9:27 AM on August 2, 2022: member

I was able to save the descriptor file with ./src/bitcoin-cli -regtest -rpcwallet=test listdescriptors '{"save_to_file": "my_desc"}' But I feel like the syntax is a bit much, is there a way to simplify it?

In this case, it would be simplier to just do ./src/bitcoin-cli -regtest -rpcwallet=test listdescriptors > my_desc
rpc: add `save_to_file` option to `listdescriptors` dd96b3d96b

rpc: change `importdescriptors` to accept `options` parameter

This changes `importdescriptors` to accept `options` parameter
but maintains compatibility with the previous `requests` parameter.

9c129d1ff8

rpc: add `descriptor_file` option to `importdescriptors` 28b26c4687
w0xlt force-pushed on Aug 2, 2022
w0xlt renamed this:
~~rpc: add a new parameter `save_to_file` to `listdescriptors` RPC~~
rpc: add ability to export/import descriptor files in `listdescriptors` and `importdescriptors`
on Aug 2, 2022
w0xlt commented at 7:20 PM on August 2, 2022: contributor

@aureleoules As suggested, I merged the other PR's commit to this one.

Note that ./src/bitcoin-cli -regtest -rpcwallet=test listdescriptors > my_desc does not create a descriptor file, but a file whose content is the RPC response. You cannot directly import the content of this file into importdescriptors.

The save_to_file option writes in the file only the descriptors attribute of the response.
S3RK commented at 7:52 AM on August 8, 2022: contributor

Concept NACK. Using shell redirection is enough for CLI. Probably this change is better fitting with the GUI.
luke-jr commented at 11:30 PM on August 9, 2022: member

Concept NACK. I think we're trying to eliminate file-based RPC methods? At the very least, we shouldn't make RPC methods that do both file-based and non-file-based access...
w0xlt commented at 2:28 AM on August 10, 2022: contributor

This PR assumes that using descriptor files is a less error-prone and less manual way of creating watch-only wallets than the current way.

If it is not the case, what is the current workflow to create a watch-only wallet ?
luke-jr commented at 8:09 PM on August 10, 2022: member

For that use case, I would suggest a RPC method that makes a watch-only copy of a loaded wallet.
DrahtBot added the label Needs rebase on Sep 27, 2022
DrahtBot commented at 10:02 AM on September 27, 2022: contributor



🐙 This pull request conflicts with the target branch and needs rebase.

<sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>
achow101 commented at 7:54 PM on October 12, 2022: member

This PR does not seem to have conceptual support. Please leave a comment if you would like this to be reopened.
achow101 closed this on Oct 12, 2022
bitcoin locked this on Oct 12, 2023

Contributors

Labels

RPC/REST/ZMQ Needs rebase

Linked (view graph)

#25757 rpc: add a new parameter `desriptor_file` to `importdescriptors` RPC #25758 ci: `AssertionError: 1759238429.9936483` <= 1759238430 in `wallet_encryption.py --descriptors`

rpc: add ability to export/import descriptor files in `listdescriptors` and `importdescriptors` #25747

Conflicts