Remove createwitnessaddress RPC command #8699
Conversation
| "It returns a json object with the address and witness script.\n" | ||
| "WARNING: It does NOT examine validity of the script in any way. Resulting address may be unspendable\n" | ||
| "WARNING: Sementics and network rules of witness and non-witness scripts may not be identical.\n" |
3339721 to
656c2ad
Compare
|
utACK 656c2ad |
| @@ -323,11 +323,13 @@ UniValue createmultisig(const UniValue& params, bool fHelp) | |||
|
|
|||
| UniValue createwitnessaddress(const UniValue& params, bool fHelp) | |||
| { | |||
| if (fHelp || params.size() < 1 || params.size() > 1) | |||
| if (fHelp || params.size() < 1 || params.size() > 1 || Params().NetworkIDString() == "main") | |||
There was a problem hiding this comment.
Please don't do a string test on the chain params. If you need a specific condition, add a ChainParams::AllowCreateWitness method or so.
There was a problem hiding this comment.
Also, is there any need to not just remove the RPC entirely?
There was a problem hiding this comment.
Just not sure if that's useful for testing. Should we keep it for test only or remove it entirely? I don't have strong opinion.
This RPC command is unsafe as it will return an address even if the script is invalid.
656c2ad to
86c3f8d
Compare
|
Revised to simply remove the command |
|
I'm a bit confused by the description. Is this only for 0.13.1? Should this be merged into master/0.14 at all?
Does it have value on testnet/regtest? |
|
@laanwj If this is merged, it is meant for all branches. |
|
So we want that RPC call to disappear forever, not just disable it on mainnet pending segwit rollout? |
|
I think it should never be allowed on mainnet. In principle we should not generate any address that may likely be unspendable. Also, I don't think it's very useful for testing. It is not used by any RPC test, and a witness address could be generated trivially with python. |
|
If it is never useful, sorry to ask, but why did this command make it into a release in the first place? Edit: obviously ACK on removing it |
|
It was part of the segwit pull request, and just added to mimick
createmultisig/addmultisigaddress. However, we only now realized that it's
pretty dangerous as it can't guarantee the returned address is valid. Maybe
it's still useful, but it at least needs a big warning.
|
86c3f8d Remove createwitnessaddress (Johnson Lau)
This RPC command is unsafe as it will return an address even if the script is invalid. Github-Pull: bitcoin#8699 Rebased-From: 86c3f8d
|
This is backported in #8815, removing tag |
For 0.13.1
createwitnessaddressdoes not examine script validity in any way and may return unspendable addresses. It should be disabled on mainnet.In longer term (maybe 0.14), we should disable all the sensitive commands (e.g. dumpprivkey, importprivkey) by default, and only be enabled with a command-line argument