Support for SegWit Addresses in RPC calls and change addresses #11177

rawodb commented at 10:17 AM on August 28, 2017: none

This is the approach I've taken to enabling segwit addresses for rpc calls on some bitcoin nodes I run.

-defaultwitnessaddress startup parameters has been added to return segwitaddresses by defaults
Witnessifier visitor has been moved to the top of the file for use in getaccountaddress/getnewaddress
getaccountaddress has been added the optional witness flag which will return Witnessified addresses
getnewaddress has been added the optional witness flag which will return Witnessified addresses
addwitnessaddress has been added an optional label parameter
Added check for witness used destination scripts in GetAccountPubkey
Change addresses default to P2SH wrapped segwit addresses when -defaultwitnessaddress is used

laanwj added the label RPC/REST/ZMQ on Aug 28, 2017

laanwj added the label Wallet on Aug 28, 2017

in src/init.cpp:456 in d9489cf3a1 outdated

 452 | @@ -453,6 +453,7 @@ std::string HelpMessage(HelpMessageMode mode)
 453 |          strUsage += HelpMessageOpt("-limitdescendantcount=<n>", strprintf("Do not accept transactions if any ancestor would have <n> or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT));
 454 |          strUsage += HelpMessageOpt("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT));
 455 |          strUsage += HelpMessageOpt("-vbparams=deployment:start:end", "Use given start/end times for specified version bits deployment (regtest-only)");
 456 | +        strUsage += HelpMessageOpt("-defaultwitnessaddress", "Return witness addresses by default in rpc calls");

promag commented at 11:24 AM on August 28, 2017:

Upper case RPC. Add default value.

in src/rpc/client.cpp:123 in d9489cf3a1 outdated

 119 | @@ -120,6 +120,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
 120 |      { "estimaterawfee", 1, "threshold" },
 121 |      { "prioritisetransaction", 1, "dummy" },
 122 |      { "prioritisetransaction", 2, "fee_delta" },
 123 | +    { "getaccountaddress", 1, "witness" },

promag commented at 11:27 AM on August 28, 2017:

This call is deprecated. Don't change it.

in src/wallet/rpcwallet.cpp:231 in 1e82c8aef4 outdated

 223 | @@ -221,10 +224,25 @@ UniValue getnewaddress(const JSONRPCRequest& request)
 224 |          throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
 225 |      }
 226 |      CKeyID keyID = newKey.GetID();
 227 | +    CBitcoinAddress addr = CBitcoinAddress(keyID);
 228 |  
 229 |      pwallet->SetAddressBook(keyID, strAccount, "receive");
 230 | +    bool witnessify = gArgs.GetBoolArg("-defaultwitnessaddress", false);
 231 | +    if (request.params.size() >= 2) {

promag commented at 11:33 AM on August 28, 2017:

Remove this test, isNull() below is enough because request.params[] returns null UniValue for out of bounds elements.

in src/wallet/rpcwallet.cpp:236 in 1e82c8aef4 outdated

 234 | +        }
 235 | +    }
 236 |  
 237 | -    return CBitcoinAddress(keyID).ToString();
 238 | +    if (witnessify) {
 239 | +		Witnessifier w(pwallet);

promag commented at 11:33 AM on August 28, 2017:

Wrong indentation.

in src/wallet/rpcwallet.cpp:1266 in 1498c55dd3 outdated

1265 | @@ -1265,7 +1266,11 @@ UniValue addwitnessaddress(const JSONRPCRequest& request)
1266 |          throw JSONRPCError(RPC_WALLET_ERROR, "Public key or redeemscript not known to wallet, or the key is uncompressed");
1267 |      }
1268 |  
1269 | -    pwallet->SetAddressBook(w.result, "", "receive");
1270 | +    std::string strLabel = "";
1271 | +    if (request.params.size() >= 2) {

promag commented at 11:34 AM on August 28, 2017:

if (!request.params[1].isNull()) instead. See developer notes.

in src/wallet/rpcwallet.cpp:1269 in 1498c55dd3 outdated

1265 | @@ -1265,7 +1266,11 @@ UniValue addwitnessaddress(const JSONRPCRequest& request)
1266 |          throw JSONRPCError(RPC_WALLET_ERROR, "Public key or redeemscript not known to wallet, or the key is uncompressed");
1267 |      }
1268 |  
1269 | -    pwallet->SetAddressBook(w.result, "", "receive");
1270 | +    std::string strLabel = "";

promag commented at 11:36 AM on August 28, 2017:

Just std::string label;

rawodb force-pushed on Aug 28, 2017

promag commented at 2:19 PM on August 28, 2017: member

Please don't mention users in commit messages. You also have to fixup some commits, for instance, you should add -defaultwitnessaddress help message in the first commit you use the argument.

sipa commented at 5:51 PM on August 28, 2017: member

I don't think we should be making witness addresses configurable on a per-address basis. Because of automatic keypool topic during rescan in HD mode, we need to automatically witnessify newly created keys anyway, as soon as there is a possibility of a single witness address been given out.

I'd prefer a wallet-wide setting that turns all addresses (including change) into witnesses (which will require a new backup of the wallet file, so it can't be done automatically).

rawodb force-pushed on Aug 28, 2017

[MOVEONLY] Move local Witnessifier to the top of the file a500384613

Add -defaultwitnessaddress startup parameter to enable default witnessification 42e6a7c7dd

Add GetWitnessScriptFromAddress / WitnessifyBitcoinAddress accessors d3347f722d

Add support for witness/-defaultwitnessaddress flag to getnewaddress 0e9c9403ae

Add support for witness/-defaultwitnessaddress flag to getaccountaddress and witness destination check to GetAccountPubkey fc3759292a

Add label support to addwitnessaddress e9959ec358

Use P2SH SegWit change addresses when -defaultwitnessaddress is used 4aa503d552

rawodb force-pushed on Sep 1, 2017

rawodb commented at 7:02 AM on September 1, 2017: none

Rebased off master, added support for P2SH wrapped change addresses when using -defaultwitnessaddress

rawodb renamed this:
~~Support for SegWit Addresses in RPC calls~~
Support for SegWit Addresses in RPC calls and change addresses
on Sep 1, 2017

rawodb closed this on Sep 7, 2017

MarcoFalke added this to the milestone 0.16.0 on Nov 10, 2017

rawodb deleted the branch on Dec 26, 2017

DrahtBot locked this on Sep 8, 2021