wallet, rpc: update listdescriptors response format #21063

pull S3RK wants to merge 1 commits into bitcoin:master from S3RK:listdescriptors_format changing 2 files +37 −21

S3RK commented at 8:36 AM on February 2, 2021: contributor

Update listdescriptors response format according to RPC interface guidelines.

This is a follow up for #20226

Before:

Result:
[                               (json array) Response is an array of descriptor objects
  {                             (json object)
    "desc" : "str",             (string) Descriptor string representation
    "timestamp" : n,            (numeric) The creation time of the descriptor
    "active" : true|false,      (boolean) Activeness flag
    "internal" : true|false,    (boolean, optional) Whether this is internal or external descriptor; defined only for active descriptors
    "range" : [                 (json array, optional) Defined only for ranged descriptors
      n,                        (numeric) Range start inclusive
      n                         (numeric) Range end inclusive
    ],
    "next" : n                  (numeric, optional) The next index to generate addresses from; defined only for ranged descriptors
  },
  ...
]

After:

Result:
{                                 (json object)
  "wallet_name" : "str",          (string) Name of wallet this operation was performed on
  "descriptors" : [               (json array) Array of descriptor objects
    {                             (json object)
      "desc" : "str",             (string) Descriptor string representation
      "timestamp" : n,            (numeric) The creation time of the descriptor
      "active" : true|false,      (boolean) Activeness flag
      "internal" : true|false,    (boolean, optional) Whether this is internal or external descriptor; defined only for active descriptors
      "range" : [                 (json array, optional) Defined only for ranged descriptors
        n,                        (numeric) Range start inclusive
        n                         (numeric) Range end inclusive
      ],
      "next" : n                  (numeric, optional) The next index to generate addresses from; defined only for ranged descriptors
    },
    ...
  ]
}

S3RK cross-referenced this on Feb 2, 2021 from issue wallet, rpc: add listdescriptors command by S3RK
fanquake added the label Wallet on Feb 2, 2021
S3RK cross-referenced this on Feb 2, 2021 from issue Add release notes for listdescriptors RPC by S3RK
meshcollider commented at 10:18 AM on February 2, 2021: contributor

utACK 6c6803cdfd4889cfb9edccaaf4b6aeb081db2f94

in src/wallet/rpcdump.cpp:1765 in 6c6803cdfd outdated

1761 | @@ -1761,8 +1762,8 @@ RPCHelpMan listdescriptors()
1762 |                      }},
1763 |                      {RPCResult::Type::NUM, "next", true, "The next index to generate addresses from; defined only for ranged descriptors"},
1764 |                  }},
1765 | -            }
1766 | -        },
1767 | +            }}

jonatack commented at 10:39 AM on February 2, 2021:

suggested fix line 1758

s/Whether this is internal or external descriptor/Whether this is an internal or external descriptor/

S3RK commented at 7:43 AM on February 9, 2021:

Fixed

S3RK force-pushed on Feb 9, 2021
S3RK commented at 7:45 AM on February 9, 2021: contributor

Fixed typo
jonatack commented at 10:38 AM on February 10, 2021: contributor

Tested ACK 94fe31eaf0dae8a970134bc0e6d7d69020a87daa
achow101 commented at 6:21 PM on February 20, 2021: member

Code Review ACK 94fe31eaf0dae8a970134bc0e6d7d69020a87daa
fanquake requested review from meshcollider on Feb 21, 2021
luke-jr commented at 1:25 AM on February 24, 2021: member

I'm not sure it makes sense to return the wallet_name here... And the "prefer an Object result" advice (I assume this is trying to satisfy) doesn't seem like it should be applicable to list* methods.

Don't really care to argue against it if others disagree tho.

Concept ~0
DrahtBot cross-referenced this on Mar 2, 2021 from issue rpc: replace wallet raw pointers with references (#18592 rebased) by fanquake
DrahtBot commented at 12:15 PM on March 2, 2021: 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:
- #21500 (wallet, rpc: add an option to list private descriptors by S3RK)
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.
meshcollider commented at 10:06 PM on March 8, 2021: contributor

This requires rebase after https://github.com/bitcoin/bitcoin/commit/a69c3b35f8974b378a87a3e42d331bd4147e07df (#21277) due to changes in wallet_listdescriptors.py
wallet, rpc: update listdescriptors response format 2e5f7def22
S3RK force-pushed on Mar 9, 2021
meshcollider commented at 11:16 PM on March 11, 2021: contributor

utACK 2e5f7def22e1b212fbd69e9147145d9d1f408aaf
S3RK cross-referenced this on Mar 22, 2021 from issue wallet, rpc: add an option to list private descriptors by S3RK
fanquake commented at 9:03 AM on April 1, 2021: member

Concept ACK. @achow101 @jonatack want to take another look here?
achow101 commented at 4:59 PM on April 1, 2021: member

re-ACK 2e5f7def22e1b212fbd69e9147145d9d1f408aaf

jonatack commented at 5:17 PM on April 1, 2021: contributor

re-ACK 2e5f7def22e1b212fbd69e9147145d9d1f408aaf

new help

listdescriptors

List descriptors imported into a descriptor-enabled wallet.
Result:
{                                 (json object)
  "wallet_name" : "str",          (string) Name of wallet this operation was performed on
  "descriptors" : [               (json array) Array of descriptor objects
    {                             (json object)
      "desc" : "str",             (string) Descriptor string representation
      "timestamp" : n,            (numeric) The creation time of the descriptor
      "active" : true|false,      (boolean) Activeness flag
      "internal" : true|false,    (boolean, optional) Whether this is an internal or external descriptor; defined only for active descriptors
      "range" : [                 (json array, optional) Defined only for ranged descriptors
        n,                        (numeric) Range start inclusive
        n                         (numeric) Range end inclusive
      ],
      "next" : n                  (numeric, optional) The next index to generate addresses from; defined only for ranged descriptors
    },
    ...
  ]
}

fanquake merged this on Apr 2, 2021
fanquake closed this on Apr 2, 2021
sidhujag referenced this in commit 993616e6e9 on Apr 2, 2021
bitcoin locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#20226 wallet, rpc: add listdescriptors command #21049 Add release notes for listdescriptors RPC #21500 wallet, rpc: add an option to list private descriptors

wallet, rpc: update listdescriptors response format #21063

Conflicts