wallet, rpc: update listdescriptors response format #21063

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

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
    },
    ...
  ]
}

fanquake added the label Wallet on Feb 2, 2021

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: member

Fixed typo

jonatack commented at 10:38 AM on February 10, 2021: member

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 commented at 12:15 PM on March 2, 2021: member

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

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: member

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

DrahtBot locked this on Aug 16, 2022