Add dev-listaddrs command - #940
Conversation
rustyrussell
left a comment
rustyrussell
left a comment
There was a problem hiding this comment.
Hi! Thanks for the great patch!
I always go through the first patch from someone and make every single tiny comment I can, so I've done that :) Mostly little minor things. Feel free to push changes on the end; I'll squash them into one patch to apply.
Thanks!
| struct json_result *response = new_json_result(cmd); | ||
| jsmntok_t *bip32tok; | ||
| u64 bip32_max_index; | ||
| const tal_t *tmpctx = tal_tmpctx(NULL); |
There was a problem hiding this comment.
OK, here's a trick: if you use tmpctx = tal_tmpctx(cmd) then it will be freed along with cmd (eg. on failure paths) and make your life a little easier.
Or, just use 'cmd' instead of tmpctx altogether, since its lifetime is only this function anyway?
| json_array_start(response, "addresses"); | ||
|
|
||
| s64 keyidx; | ||
| for (keyidx = 0; keyidx < bip32_max_index; keyidx++) { |
There was a problem hiding this comment.
Minor nit: just use for (s64 keyidx = 0 for local vars. It's the one thing about C99 declare-anywhere I like :)
|
|
||
| { | ||
| char *hex = pubkey_to_hexstr(tmpctx, &pubkey); | ||
| json_add_string(response, "pubkey", hex); |
There was a problem hiding this comment.
You can use json_add_pubkey() here...
There was a problem hiding this comment.
But except for deposit addresses, we generally don't use p2sh, we use straight segwit. You'll probably want the bech32 variants as well?
There was a problem hiding this comment.
Do you think it is better to show both bech32 & p2sh address for the same derivation like that:
$ lightning-cli dev-listaddrs
{ "addresses" : [
{
"keyidx": 0,
"pubkey": "...",
"p2sh": "...",
"p2sh_redeemscript": "...",
"bech32": "...",
"bech32_redeemscript": "..."
} ] }
or use address_type option (like get_newaddr) to show only one kind of derivation address?
$ lightning-cli dev-listaddrs bech32
{ "addresses" : [
{
"keyidx" : 0,
"address" : "...",
"pubkey" : "...",
"redeemscript" : "..."
} ] }
| int len = sizeof(struct ripemd160) + 1; | ||
| char *hex = tal_arr(NULL, char, hex_str_size(len)); | ||
| hex_encode(redeemscript, len, hex, hex_str_size(len)); | ||
| json_add_string(response, "redeemscript", hex); |
| } | ||
|
|
||
| if (!bip32tok || !json_tok_u64(buffer, bip32tok, &bip32_max_index)) { | ||
| bip32_max_index = db_get_intvar(cmd->ld->wallet->db, "bip32_max_index", 0); |
There was a problem hiding this comment.
Minor nit: inconsistent use of spaces vs. tabs
|
|
Thanks for updating this @lvaccaro, if you enable edits by owners when creating a PR you'll allow us to amend and add fixes to your PR btw. I tried cleaning the PR up further but then was unable to push to your branch. For reference my cleanup looks like this: master...cdecker:listaddr Please don't create merge requests, but rather rebase on top of |
|
Sorry, I close this pr and I will open a new pr from master. |
3 participants
Add dev-listaddrs command to show bip32 addresses list (up to bip32_max_index).
help
$ lightning-cli help ... command=dev-listaddrs description=Show addresses list up to derivation {index} (default is the last bip32 index) ...output
$ lightning-cli dev-listaddrs { "addresses" : [ { "keyidx" : 0, "address" : "...", "pubkey" : "...", "redeemscript" : "..." }, .... ] }Set optional value {index} to show also future derivations up to {index}. This could be useful for testing and recovery.