Add hdenabled flag in getwalletinfo #12042

pull jeromer wants to merge 3 commits into bitcoin:master from jeromer:getwalletinfo/hdenabledflag changing 1 files +6 −2

jeromer commented at 7:11 PM on December 28, 2017: none

When running bitcoin-cli getwalletinfo we can see a new flag called hdenabled which is either true or false. This replicates the behaviour we see in bitcoin-qt which displays a "HD Enabled" logo in BitcoinGUI::setHDStatus()

The help now look like this:

bitcoin-cli getwalletinfo help
error code: -1
error message:
getwalletinfo
Returns an object containing various wallet state info.

Result:
{
  "walletname": xxxxx,             (string) the wallet name
  "walletversion": xxxxx,          (numeric) the wallet version
  "balance": xxxxxxx,              (numeric) the total confirmed balance of the wallet in BTC
  "unconfirmed_balance": xxx,      (numeric) the total unconfirmed balance of the wallet in BTC
  "immature_balance": xxxxxx,      (numeric) the total immature balance of the wallet in BTC
  "txcount": xxxxxxx,              (numeric) the total number of transactions in the wallet
  "keypoololdest": xxxxxx,         (numeric) the timestamp (seconds since Unix epoch) of the oldest pre-generated key in the key pool
  "keypoolsize": xxxx,             (numeric) how many new keys are pre-generated (only counts external keys)
  "keypoolsize_hd_internal": xxxx, (numeric) how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)
  "unlocked_until": ttt,           (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked
  "paytxfee": x.xxxx,              (numeric) the transaction fee configuration, set in BTC/kB
  "hdmasterkeyid": "<hash160>",    (string) the Hash160 of the HD master pubkey
  "hdenabled": true|false          (boolean) true if HD is enabled, false otherwise
}

Examples:
> bitcoin-cli getwalletinfo
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "getwalletinfo", "params": [] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

And the command output:

bitcoin-cli getwalletinfo
{
  "walletname": "wallet.dat",
  "walletversion": 139900,
  "balance": 0.00999772,
  "unconfirmed_balance": 0.00000000,
  "immature_balance": 0.00000000,
  "txcount": 7,
  "keypoololdest": 1514388183,
  "keypoolsize": 1000,
  "keypoolsize_hd_internal": 1000,
  "paytxfee": 0.00000000,
  "hdmasterkeyid": "ad304ceaa7bf2bf0a3543526968ee958fb94c847",
  "hdenabled": true
}

Add hdenabled flag in getwalletinfo 3d5741c370
Fix getwalletinfo help 851c134b85
Fix flag name 2b6734cd17
jonasschnelli commented at 7:36 PM on December 28, 2017: contributor

It's kinda redundant. If hdmasterkeyid is present, hd is enabled. Maybe better to mention this in the RPC help instead of adding the redundant hdenabled?
fanquake added the label RPC/REST/ZMQ on Dec 28, 2017
fanquake added the label Wallet on Dec 28, 2017

in src/wallet/rpcwallet.cpp:2707 in 2b6734cd17

2703 | @@ -2704,7 +2704,8 @@ UniValue getwalletinfo(const JSONRPCRequest& request)
2704 |              "  \"keypoolsize_hd_internal\": xxxx, (numeric) how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)\n"
2705 |              "  \"unlocked_until\": ttt,           (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked\n"
2706 |              "  \"paytxfee\": x.xxxx,              (numeric) the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB\n"
2707 | -            "  \"hdmasterkeyid\": \"<hash160>\"     (string) the Hash160 of the HD master pubkey\n"
2708 | +            "  \"hdmasterkeyid\": \"<hash160>\",    (string) the Hash160 of the HD master pubkey\n"

promag commented at 1:01 AM on December 29, 2017:

Agree with @jonasschnelli, just improve this description like (only present when HD is enabled). Better open a new PR with this suggestion though.

promag commented at 2:09 AM on December 29, 2017:

Also add optional flag.

promag commented at 1:05 AM on December 29, 2017: member

NACK, avoid redundant fields.
jeromer commented at 5:45 AM on December 29, 2017: none

Updating the RPC docs is a good option indeed.
meshcollider commented at 9:53 AM on December 29, 2017: contributor

@jeromer concept ACK on updating the help text, thanks for contributing :)
MarcoFalke commented at 10:01 AM on December 29, 2017: member

Don't forget to squash your commits when you push the fixed-up version. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
promag commented at 1:30 AM on January 4, 2018: member

@jeromer do you plan to change the PR?
gmaxwell commented at 10:55 AM on January 8, 2018: contributor

The really important piece of information-- does the wallet contain any non-HD private keys-- appears to exist nowhere.
jonasschnelli commented at 6:16 PM on January 8, 2018: contributor

@gmaxwell point is not directly related to this PR, but an important one.
MarcoFalke added the label Up for grabs on Jan 13, 2018
fanquake commented at 1:03 PM on January 29, 2018: member

I've opened #12293 to finish this up.
fanquake closed this on Jan 29, 2018
MarcoFalke removed the label Up for grabs on Jan 29, 2018
MarcoFalke referenced this in commit 9d9c4185fa on Jan 29, 2018
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#12293 [rpc] Mention that HD is enabled if hdmasterkeyid is present