Help text for importaddress referred to the first parameter as script, when in fact it's address. Calling with a script argument fails:
→ bcli -named importaddress script=2N3qhMpHK8WNo7wv87W9eHMgvGyJU1593Ei
error code: -8
error message:
Unknown named parameter script
→ bcli -named importaddress address=2N3qhMpHK8WNo7wv87W9eHMgvGyJU1593Ei
# success!
This seems to revert 5c17059872c9b63a1e05c7aa8aea32a03c3ec73a of #6415, no?
This seems to revert 5c17059 of #6415
Hmmm - yes. I hadn't noticed that. That commit updated part of the help text to refer to the first argument as script:
"1. \"script\" (string, required) The hex-encoded script (or address)\n"
but not the usage summary:
"importaddress \"address\" ( \"label\" rescan p2sh )\n"
When named arguments were introduced (https://github.com/bitcoin/bitcoin/commit/37a166f1465c91be3e700c0175f34dd0c3698df8), the usage summary was used to name the first argument:
+ { "wallet", "importaddress", &importaddress, true, {"address","label","rescan","p2sh"} },
Since the argument name is now part of the API, I think we should update the help text to match the argument name rather than the other way round.
Isn't it misleading to import a script over a key called address? I know it would be correct but still not clear.
What about renaming the parameter to address_or_script or to importdata (or similar) to avoid confusion?
What about renaming the parameter to address_or_script or to importdata (or similar) to avoid confusion?
I'd prefer to update the text to match the API, than change the API (which could be a compatibility break for clients).
An alternative to this PR would be to add script as an alias for address (and change the usage summary so that script is the canonical name for the arg). Thoughts?
utACK 2c71edc2fc52cfc3a0b401f8d5d051c8938c150e. This makes the doc reflect the actual name of the named parameter, which can no longer be changed at this point.
utACK 2c71edc.