wallet: Deprecate addwitnessaddress#12210
Conversation
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
Wouldn't it be better to suggest using the RPC parameters, rather than changing the config file and restarting?
There was a problem hiding this comment.
Yeah getnewaddress with the address_type parameter more closely resembles what this RPC was used for IMO
There was a problem hiding this comment.
best to document both, I'll mention that option as well.
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
Doesn't require a new wallet backup anymore, IIRC.
There was a problem hiding this comment.
I don't think it makes sense to update the documentation of a deprecated call, besides adding a DEPRECATED message and hiding it.
There was a problem hiding this comment.
Agreed, its not like backing up would actually be a bad thing anyway
|
utACK |
|
utACK, thanks! |
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
This still mentions estimatefee :)
There was a problem hiding this comment.
Also, if you deprecate it like this you'll have to add -deprecatedrpc=addwitnessaddress to the tests which use it (easier than reworking the tests at the same time)
There was a problem hiding this comment.
Gah, will update. Nothing is ever easy, is it? I assumed I was just updating the documentation hence I didn't bother running any tests.
|
Concept ACK |
|
The following tests are failing due to to the deprecation:
I'm starting to think this was a kind of bad idea, if we still rely so much on that call. |
|
We can fix the test, but lets move the deprecation to 0.17 to not block 0.16? |
|
I don't think anything needs to be blocked on that, just adding |
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
should say -deprecatedrpc=addwitnessaddress instead of -deprecatedrpc=estimatefee
|
utACK modulo flack's comment. |
|
utACK after comments. We can slowly remove |
Now that segwit is natively supported by the wallet, deprecate the hack `addwitnessaddress`.
laanwj
force-pushed
the
2018_01_deprecate_addwitnessaddress
branch
from
e849ae0 to
cdf3e03
Compare
cdf3e03 wallet: Deprecate addwitnessaddress (Wladimir J. van der Laan) Pull request description: Now that segwit is natively supported by the wallet, deprecate the hack `addwitnessaddress`. Tree-SHA512: f33b1c33d200fa8f1a0fba424b30e9c2a78147cde8bb0a3fd41194b77980454cddfb23da256cd6fe78726e87161deaa23357d0764e74c3eb83177cc518afa49c
f523c6b [qa] Use address type in addmultisigaddress to avoid addwitnessaddress (João Barbosa) 886a92f [rpc] Add address type option to addmultisigaddress (João Barbosa) Pull request description: Adds the option `address_type` to `addmultisigaddress` and `createmultisg` RPC. This also allows to avoid `addwitnessaddress` to obtain an `p2sh-segwit` or `bech32` multsig address. Related to #12210 as this reduces `addwitnessaddress` usage. Tree-SHA512: 8f8f85dfcff66bb6c7e1e9865e37c285dead1d6dadb9672a89b92fa209d03cc35817ca1d656588c6c2146b728daaf7540b851929b640294653c62836cbefe7ee
10 participants
Now that segwit is natively supported by the wallet, deprecate the hack
addwitnessaddress.