isscript
is unknown for unknown witness versions, so it should be marked optional in the docs
doc: getaddressinfo[isscript] is optional #30457
pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2407-doc changing 2 files +6 −3-
maflcko commented at 5:10 pm on July 16, 2024: member
-
DrahtBot commented at 5:10 pm on July 16, 2024: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage
For detailed information about the code coverage, see the test coverage report.
Reviews
See the guideline for information on the review process.
Type Reviewers ACK stickies-v, tdb3 If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
-
DrahtBot added the label Docs on Jul 16, 2024
-
in test/functional/rpc_invalid_address_message.py:114 in fa63ecc720 outdated
110@@ -109,6 +111,7 @@ def test_getaddressinfo(self): 111 assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, BECH32_INVALID_PREFIX) 112 assert_raises_rpc_error(-5, "Invalid or unsupported Base58-encoded address.", node.getaddressinfo, BASE58_INVALID_PREFIX) 113 assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, INVALID_ADDRESS) 114+ "isscript" not in node.getaddressinfo(BECH32_VALID_UNKNOWN_WITNESS)
stickies-v commented at 6:05 pm on July 16, 2024:This is a no-op, I think you meant to assert?
0 assert "isscript" not in node.getaddressinfo(BECH32_VALID_UNKNOWN_WITNESS)
maflcko commented at 4:52 am on July 17, 2024:thanks, fixedin src/wallet/rpc/addresses.cpp:537 in fa63ecc720 outdated
533@@ -534,7 +534,7 @@ RPCHelpMan getaddressinfo() 534 {RPCResult::Type::BOOL, "solvable", "If we know how to spend coins sent to this address, ignoring the possible lack of private keys."}, 535 {RPCResult::Type::STR, "desc", /*optional=*/true, "A descriptor for spending coins sent to this address (only when solvable)."}, 536 {RPCResult::Type::STR, "parent_desc", /*optional=*/true, "The descriptor used to derive this address if this is a descriptor wallet"}, 537- {RPCResult::Type::BOOL, "isscript", "If the key is a script."}, 538+ {RPCResult::Type::BOOL, "isscript", /*optional=*/true, "If the key is a script."},
stickies-v commented at 6:16 pm on July 16, 2024:unrelated: that is quite the field description 😳 Time to start thinking about deprecatingisscript
andiswitness
, perhaps?
instagibbs commented at 2:09 pm on July 17, 2024:yeah I was having a real hard time understanding what a lot of these fields actually mean…
maflcko commented at 2:15 pm on July 17, 2024:I think it means “some kind of pay to script hash”, that is SH or WSH.
I guess it would be clearer to just return an enum as string, rather than a collection of boolean fields.
stickies-v commented at 6:17 pm on July 16, 2024: contributorcode LGTM fa63ecc72045ffa771448941e1fe066f7421f640 but I think the test needs to be fixedin test/functional/rpc_invalid_address_message.py:15 in fa63ecc720 outdated
11@@ -12,6 +12,7 @@ 12 ) 13 14 BECH32_VALID = 'bcrt1qtmp74ayg7p24uslctssvjm06q5phz4yrxucgnv' 15+BECH32_VALID_UNKNOWN_WITNESS = 'bcrt1p424qxxyd0r'
tdb3 commented at 0:59 am on July 17, 2024:Maybe it’s worth adding a line for segwit v0 as well? (e.g. witness program not 20 or 32 bytes long)?
also
nit: might increase clarity
0- BECH32_VALID_UNKNOWN_WITNESS = 'bcrt1p424qxxyd0r' 1+ BECH32_VALID_UNKNOWN_WITNESS_PROG = 'bcrt1p424qxxyd0r'
maflcko commented at 4:53 am on July 17, 2024:Left the nit for now.tdb3 commented at 1:06 am on July 17, 2024: contributorApproach ACK Thanks for increasing clarity in RPC. Looks like there’s a missingassert
intest_getaddressinfo()
. Left a nit.doc: getaddressinfo[isscript] is optional fa6390df20maflcko force-pushed on Jul 17, 2024DrahtBot added the label CI failed on Jul 17, 2024DrahtBot removed the label CI failed on Jul 17, 2024stickies-v approvedstickies-v commented at 10:19 am on July 17, 2024: contributorACK fa6390df205513319f28e35e3e17c40ecaa7d731DrahtBot requested review from tdb3 on Jul 17, 2024tdb3 approvedtdb3 commented at 11:31 am on July 17, 2024: contributorACK fa6390df205513319f28e35e3e17c40ecaa7d731fanquake merged this on Jul 17, 2024fanquake closed this on Jul 17, 2024
maflcko deleted the branch on Jul 17, 2024
maflcko
DrahtBot
stickies-v
instagibbs
tdb3
Labels
Docs
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-08 01:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me