If -changetype is not explicitly set, then regardless of -addresstype, the wallet will use a ~bech32 change address~ P2WPKH change output if any destination is P2WPKH or P2WSH.
This seems more intuitive to me and more in line with the spirit of BIP-69.
When combined with #11991 a QT user could opt to use bech32 exclusively without having to figure out how to launch with -changetype=bech32, although so would #11937.
@promag for change addresses yes, so that could be an objection.
More conservative approaches could be:
have another UI check box to send change to a bech32 address; this could be checked by default if all destinations are bech32. This won’t work for RPC users, but those can be expected to use the -changeaddress flag.
add an additional constraint that at least one of the the from addresses is bech32.
Seems to me if the user is explicitly setting a change type, we should respect that.
Instead, I suggest turning OUTPUT_TYPE_DEFAULT into a real value, and handling it specially for change here. Eg, https://github.com/luke-jr/bitcoin/commits/bech32-change
It seems a bit pointless to stand on its own in a PR. It would be better to just include it as a separate commit here (and in another PR I’m about to open).
To get it in your own branch as-is, switch to it and do:
0git fetch git://github.com/luke-jr/bitcoin bech32-change && git reset --hard 5efd54745c34ff329d9c75f2f5ee852930b21231
2666+ // Check if the destination doesn't contain a witness program, in which
2667+ // case the addresstype for this destination is not bech32:
2668+ int witnessversion = 0;
2669+ std::vector<unsigned char> witnessprogram;
2670+ if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) {
2671+ anyDestinationBech32 = true;
any_destination_bech32?
2752- scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, g_change_type));
2753+ // If g_change_type is p2sh_segwit, use bech32 for the change
2754+ // address if any destination is bech32.
2755+ OutputType change_type = g_change_type;
2756+ if (g_change_type == OUTPUT_TYPE_P2SH_SEGWIT && anyDestinationBech32) {
2757+ change_type = OUTPUT_TYPE_BECH32;
Should check inputs instead?
Or even check if any input or any output is bech32?
By legacy I mean p2pkh. We should not create native segwit change if the user has specifically asked for non-segwit change. If a user is setting legacy then it’s likely that they really don’t want segwit outputs for some reason (for example, compatibility with older software).
That argument doesn’t apply for p2sh-segwit. The only reason to ever use p2sh-segwit change is for change distinguishability privacy, which, as you note– doesn’t apply when there are segwit outputs.
2661@@ -2661,6 +2662,13 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2662
2663 if (recipient.fSubtractFeeFromAmount)
2664 nSubtractFeeFromAmount++;
2665+
2666+ // Check if any destination contains a witness program:
2667+ int witnessversion = 0;
2668+ std::vector<unsigned char> witnessprogram;
2669+ if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) {
It might be assertophobia, but I’m not sure what you mean by that example. If you send coins to a future v1 segwit address, the change has to go somewhere right? So the change would just go to a v0 witness derived from the next available key in the pool.
Note that IsWitnessProgram writes to witnessversion, not the other way around. And witnessversion isn’t used when the change address is created.
It depends on what the intention of v1 is whether change can safely go to a v1 address. But I don’t see how it would be unsafe to send to a v0 address. Of course maybe there is a reason, or future us will find a reason. So an assert would force them to think about that. Is that what you mean?
Not unsafe, but defeats the privacy logic that this enables, I think.
Maybe I’m making too much hay of this.
@instagibbs since we don’t support v1 there is no ‘private’ option possible for us, so given that privacy is impossible, we should at least choose the most efficient!
The only reason we initially defaulted to do anything but produce native change is because under the assumption that most payments would be non-native that would be pretty bad for privacy. But that privacy argument only holds when the decision results in us matching.
Perhaps in the future if we support both v0 and v1 and have no particular reason to choose between them (e.g. they’re both about equally efficient, and our wallet’s policy doesn’t demand one vs the other), we would just match whatever we’re paying… but we can’t do that now.
any_destination_native_segwit is needed.
2746@@ -2739,8 +2747,16 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2747 return false;
2748 }
2749
2750- LearnRelatedScripts(vchPubKey, g_change_type);
2751- scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, g_change_type));
2752+ // If -changetype is not specified and g_address_type is not legacy,
2753+ // and any destination is (P2SH-)P2WPKH, use P2WPKH for the change
and any destination is (P2SH-)P2WPKH, I don’t think this is correct? should just be P2WPKH
If this is merged, I think it ought to be paired with #12146. It’s a good, off-by-default option to have available for strange edge cases that want to run the latest full node software but have what would otherwise be an infuriating need to remain compatible with legacy systems.
Just my 2 bits.
I must still misunderstand their relation, then. Sorry for piping up.
Does -changetype=bech32 also prevent the behavior this PR adds?
241+ self.test_change_output_type(2, [to_address_bech32_1], 'bech32')
242+ self.test_change_output_type(2, [to_address_p2sh], 'bech32')
243+
244+ self.log.debug("Nodes with addresstype=bech32 always use a P2WPKH change output:")
245+ self.test_change_output_type(3, [to_address_bech32_1], 'bech32')
246+ self.test_change_output_type(3, [to_address_p2sh], 'bech32')
addresstype implicitly sets changetype:
g_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), g_address_type)
If the user overrides changetype it will use that, so I could change the word “always” in the comment to reflect that better.
2661@@ -2661,6 +2662,13 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2662
2663 if (recipient.fSubtractFeeFromAmount)
2664 nSubtractFeeFromAmount++;
2665+
2666+ // Check if any destination contains a witness program:
2667+ int witnessversion = 0;
2668+ std::vector<unsigned char> witnessprogram;
2669+ if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) {
2670+ any_destination_native_segwit = true;
net.py fails on Travis, but passes locally. I’ll rebase because there are some new test related fixes on master.
2751- scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, g_change_type));
2752+ // If -changetype is not specified and g_address_type is not legacy,
2753+ // and any destination is (P2SH-)P2WPKH, use P2WPKH for the change
2754+ // output.
2755+ OutputType change_type = g_change_type;
2756+ if (!gArgs.IsArgSet("-changetype") && g_address_type != OUTPUT_TYPE_LEGACY && any_destination_native_segwit) {
Suggestion: test g_address_type against OUTPUT_TYPE_DEFAULT, instead of touching gArgs here.
If that is not possible, please move the gArgs.IsArgSet("-changetype") to wallet/init.cpp and set a global boolean g_changetype_is_default or such - don’t spread the handling for one option over two separate places.
g_change_type is no longer based on -addresstype. Functions that use g_change_type need to handle OUTPUT_TYPE_NONE, which I’ve done everywhere with: g_change_type ? g_change_type : g_address_type.
utACK 327634f97e5921815c30e50d076cde0f6ff51aaf
I think the code can be made a bit cleaner by introducing an explicit P2SH_SEGWIT_BUT_NATIVE_WHEN_NATIVE_DESTINATION or so type in OutputType, but let’s do that later.
2746@@ -2739,8 +2747,16 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2747 return false;
2748 }
2749
2750- LearnRelatedScripts(vchPubKey, g_change_type);
2751- scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, g_change_type));
2752+ // If -changetype is not specified and g_address_type is not legacy,
2753+ // and any destination is (P2SH-)P2WPKH, use P2WPKH for the change
shouldn’t this read:
// and any destination is P2WPKH, use P2WPKH for the change
642@@ -643,8 +643,9 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, const SendCoinsRecipient& r
643 // use for change. Despite an actual payment and not change, this is a close match:
644 // it's the output type we use subject to privacy issues, but not restricted by what
645 // other software supports.
646- wallet->LearnRelatedScripts(newKey, g_change_type);
647- CTxDestination dest = GetDestinationForKey(newKey, g_change_type);
648+ OutputType change_type = g_change_type ? g_change_type : g_address_type;
change_type = g_change_type != OUTPUT_TYPE_NONE ? g_change_type : g_address_type
125+ # Make sure the destinations are included, and remove it:
126+ for destination in destinations:
127+ output_addresses.pop(output_addresses.index(destination))
128+
129+ assert_equal(len(output_addresses), 1)
130+
130+
131+ change_address = output_addresses[0]
132+
133+ self.log.debug("Check if change address " + change_address + " is " + expected_type)
134+
135+ self.test_address(node_sender, change_address, False, expected_type)
181@@ -182,9 +182,11 @@ bool WalletParameterInteraction()
182 return InitError(strprintf(_("Unknown address type '%s'"), gArgs.GetArg("-addresstype", "")));
183 }
184
185- g_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), g_address_type);
Suggest writing this as:
0g_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), OUTPUT_TYPE_NONE);
1if (g_change_type == OUTPUT_TYPE_NONE && gArgs.IsArgSet("-changetype")) {
2 return InitError(strprintf(_("Unknown change type '%s'"), gArgs.GetArg("-changetype", "")));
3}
Reasons:
g_address_type code aboveg_change_type set to indeterminate value if !IsArgSet.You may additionally want to replace gArgs.IsArgSet("-changetype") with !gArgs.GetArg("-changetype", "").empty(), so passing the empty string is the same as not passing a value, and will not trigger an error. This would be more consistent with addresstype parsing above, and might be practically useful, e.g. if somebody has an initial value set in a config file but wants to reset it on the command line.
changetype=... in the config file makes gArgs.IsArgSet("-changetype" evaluate to true. The config file value is ignored if a parameter overrides it. I still like your shorter notation, so using that now.
@ryanofsky setting changetype=… in the config file makes gArgs.IsArgSet("-changetype" evaluate to true
Yes, that’s the reason I also suggested replacing gArgs.IsArgSet("-changetype") with !gArgs.GetArg("-changetype", "").empty()
changetype=bech32 the user could unset it with -changetype=. Fixed.
255@@ -256,9 +256,9 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request)
256 pwallet->TopUpKeyPool();
257 }
258
259- OutputType output_type = g_change_type;
260+ OutputType output_type = g_change_type ? g_change_type : g_address_type;
261 if (!request.params[0].isNull()) {
262- output_type = ParseOutputType(request.params[0].get_str(), g_change_type);
263+ output_type = ParseOutputType(request.params[0].get_str(), OUTPUT_TYPE_NONE);
getnewaddress and getrawchangeaddress. Former will continue treating empty string like null, and latter will now treat the empty string as an error. Would suggest either passing output_type here to continue treating empty string like null both places, or passing OUTPUT_TYPE_NONE in getnewaddress to now raise an error both places.
OutputType output_type = g_change_type ? g_change_type : g_address_type above is overridden in this line. I added some tests for getrawchangeaddress.
116+
117+ # Make sure the transaction has change:
118+ assert_equal(len(tx["vout"]), len(destinations) + 1)
119+
120+ # Make sure the destinations are included, and remove them:
121+ output_addresses = list(map(lambda vout: vout['scriptPubKey']['addresses'][0], tx["vout"]))
May be easier to read as
0output_addresses = [vout['scriptPubKey']['addresses'][0] for vout in tx["vout"]]
118+ assert_equal(len(tx["vout"]), len(destinations) + 1)
119+
120+ # Make sure the destinations are included, and remove them:
121+ output_addresses = list(map(lambda vout: vout['scriptPubKey']['addresses'][0], tx["vout"]))
122+ for destination in destinations:
123+ output_addresses.pop(output_addresses.index(destination))
Could simplify to output_addresses.remove(destination). Another alternative could be
0change_addresses = [d for d in output_addresses if d not in destinations]
That’s what happens if you write javascript for a few years… :-)
Fixed
2753+ // and any destination is P2WPKH or P2WSH, use P2WPKH for the change
2754+ // output.
2755+ OutputType change_type = g_change_type ? g_change_type : g_address_type;
2756+ if (g_change_type == OUTPUT_TYPE_NONE && g_address_type != OUTPUT_TYPE_LEGACY && any_destination_native_segwit) {
2757+ change_type = OUTPUT_TYPE_BECH32;
2758+ }
Just a suggestion, but perhaps this would be simpler as
0OutputType change_type =
1 g_change_type != OUTPUT_TYPE_NONE ? g_change_type :
2 g_address_type != OUTPUT_TYPE_LEGACY && any_destination_native_segwit ? OUTPUT_TYPE_BECH32 :
3 g_address_type;
~utACK 0ee841a21aaea880e3af1e93b53d373295d1f829~ (EDIT: see #12119 (comment)). Changes since last review were various suggested changes and new tests. Thanks for the new tests!
@sipa but -addresstype could have that value?
Presumeably enum type could be added and used as a default value without having a corresponding string that would be parsed by -addresstype. Anyway, this does seem like a good idea.
I just realized this PR doesn’t currently update the -changetype help text, so I withdraw my ACK until that is done. (The current text says default is same as -addresstype, which is no longer accurate.)
Conditional utACK 0ee841a21aaea880e3af1e93b53d373295d1f829 if help is updated.
current text says
default is same as -addresstype, which is no longer accurate
Maybe I can change it to: “Default is same as -addresstype, except when sending to a native segwit address and -addresstype is p2sh-segwit”?
2755+ OutputType change_type =
2756+ g_change_type != OUTPUT_TYPE_NONE ? g_change_type :
2757+ g_address_type != OUTPUT_TYPE_LEGACY && any_destination_native_segwit ? OUTPUT_TYPE_BECH32 :
2758+ g_address_type;
2759+
2760+ LearnRelatedScripts(vchPubKey, change_type);
2746@@ -2739,8 +2747,16 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2747 return false;
2748 }
2749
2750- LearnRelatedScripts(vchPubKey, g_change_type);
2751- scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, g_change_type));
2752+ // If -changetype is not specified and g_address_type is not legacy,
2753+ // and any destination is P2WPKH or P2WSH, use P2WPKH for the change
2754+ // output.
2755+ OutputType change_type =
IMO should avoid long expressions. Suggestion:
0OutputType change_type = g_change_type;
1if (change_type == OUTPUT_TYPE_NONE) {
2 if (any_destination_native_segwit && g_address_type != OUTPUT_TYPE_LEGAC) {
3 change_type = OUTPUT_TYPE_BECH32;
4 } else {
5 change_type = g_address_type;
6 }
7}
Also, CreateTransaction is already a rather long function, could you factor out this to a new function? Then add tests for that in src/wallet/test/wallet_tests.cpp.
Switched to your version.
I extracted the change type stuff to a new function. I made a note to add tests later.
Updated help text:
@promag added the const you suggested, as well as explicit == OUTPUT_TYPE_NONE in both places (@ajtowns).
This can be avoided/skipped if there is a custom address in coin control. Move it below to where any_destination_native_segwit is needed.
Done (which involved another loop over vecSend, but I assume that’s trivially fast).
2654+ // Check if any destination contains a witness program:
2655+ int witnessversion = 0;
2656+ std::vector<unsigned char> witnessprogram;
2657+ if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) {
2658+ any_destination_native_segwit = true;
2659+ }
Performance improvement:
0 any_destination_native_segwit = true;
1 break;
2643@@ -2644,6 +2644,31 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
2644 return true;
2645 }
2646
2647+OutputType CWallet::TransactionChangeType(const std::vector<CRecipient>& vecSend)
Suggestions, for now, keep static and not a CWallet member:
0static OutputType GetChangeTypeForRecipients(const std::vector<CRecipient>& recipients)
1{
And below:
0const OutputType change_type = GetChangeTypeForRecipients(vecSend);
don’t mind the function name.
g_change_type with a CWallet member).
CWallet. Feel free to leave as it is.
This is perhaps just personal taste, but I prefer early function return if we already know what change address we’ll use without having to look at the outputs. That also means we can remove the local variables any_destination_native_segwit and change_type. It’d look something like:
0 // If -changetype is specified, always use that change type.
1 if (g_change_type != OUTPUT_TYPE_NONE) {
2 return g_change_type;
3 }
4
5 // if g_address_type is legacy, use legacy address as change (even
6 // if some of the outputs are P2WPKH or P2WSH).
7 if (g_address_type == OUTPUT_TYPE_LEGACY) {
8 return OUTPUT_TYPE_LEGACY;
9 }
10
11 // if any destination is P2WPKH or P2WSH, use P2WPKH for the change
12 // output.
13 for (const auto& recipient : vecSend) {
14 // Check if any destination contains a witness program:
15 int witnessversion = 0;
16 std::vector<unsigned char> witnessprogram;
17 if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) {
18 return OUTPUT_TYPE_BECH32;
19 }
20 }
21
22 // else use g_address_type for change
23 return g_address_type;
utACK dc2812f.
The only thing that bothers me is the fact that g_address_type = OUTPUT_TYPE_NONE is an error and now g_change_type = OUTPUT_TYPE_NONE is not.
9@@ -10,7 +10,7 @@
10 - node2 uses p2sh/segwit addresses and bech32 addresses for change
11 - node3 uses bech32 addresses
12
13-node4 exists to generate new blocks.
14+node5 exists to generate new blocks.
131+
132+ self.log.debug("Check if change address " + change_addresses[0] + " is " + expected_type)
133+ self.test_address(node_sender, change_addresses[0], multisig=False, typ=expected_type)
134+
135 def run_test(self):
136 # Mine 101 blocks on node4 to bring nodes out of IBD and make sure that
on node5
210@@ -183,7 +211,7 @@ def run_test(self):
211 assert_equal(unconf_balances[to_node], to_send * 10 * (2 + n))
212
213 # node4 collects fee and block subsidy to keep accounting simple
115@@ -104,10 +116,26 @@ def test_address(self, node, address, multisig, typ):
116 # Unknown type
117 assert(False)
118
119+ def test_change_output_type(self, node_sender, destinations, expected_type):
120+ txid = self.nodes[node_sender].sendmany("", dict.fromkeys(destinations, 0.001))
"" is for.
127+ # Make sure the destinations are included, and remove them:
128+ output_addresses = [vout['scriptPubKey']['addresses'][0] for vout in tx["vout"]]
129+ change_addresses = [d for d in output_addresses if d not in destinations]
130+ assert_equal(len(change_addresses), 1)
131+
132+ self.log.debug("Check if change address " + change_addresses[0] + " is " + expected_type)
28+this also verifies that spending coins sent to all these address types works.
29+
30+Test that node1 uses a bech32 addresses for change if any destination address is
31+bech32. Test that node0 always uses a legacy change address. Test that node4 uses
32+p2sh/segwit output for change.
33+"""
I think the whole docstring could be updated to reflect better what the test is actually doing:
0"""Test that the wallet can send and receive using all combinations of address types.
1
2There are 5 nodes-under-test:
3 - node0 uses legacy addresses
4 - node1 uses p2sh/segwit addresses
5 - node2 uses p2sh/segwit addresses and bech32 addresses for change
6 - node3 uses bech32 addresses
7 - node4 uses p2sh/segwit addresses for change
8
9node5 exists to generate new blocks.
10
11## Multisig address test
12
13Test that adding a multisig address with:
14 - an uncompressed pubkey always gives a legacy address
15 - only compressed pubkeys gives the an `-addresstype` address
16
17## Sending to address types test
18
19A series of tests, iterating over node0-node4. In each iteration of the test, one node sends:
20 - 10/101th of its balance to itself (using getrawchangeaddress for single key addresses)
21 - 20/101th to the next node
22 - 30/101th to the node after that
23 - 40/101th to the remaining node
24 - 1/101th remains as fee+change
25
26Iterate over each node for single key addresses, and then over each node for
27multisig addresses.
28
29Repeat test, but with explicit address_type parameters passed to getnewaddress
30and getrawchangeaddress:
31 - node0 and node3 send to p2sh.
32 - node1 sends to bech32.
33 - node2 sends to legacy.
34
35As every node sends coins after receiving, this also
36verifies that spending coins sent to all these address types works.
37
38## Change type test
39
40Test that the nodes generate the correct change address type:
41 - node0 always uses a legacy change address.
42 - node1 uses a bech32 addresses for change if any destination address is bech32.
43 - node4 always uses p2sh/segwit output for change.
44"""
232+ self.nodes[5].sendtoaddress(self.nodes[4].getnewaddress(), Decimal("1"))
233+ self.nodes[5].generate(1)
234+ sync_blocks(self.nodes)
235+ assert_equal(self.nodes[4].getbalance(), 1)
236+
237+ self.log.debug("Nodes with change_type=bech32 always use a P2WPKH change output:")
INFO level
49 class AddressTypeTest(BitcoinTestFramework):
50 def set_test_params(self):
51- self.num_nodes = 5
52- self.extra_args = [["-addresstype=legacy"], ["-addresstype=p2sh-segwit"], ["-addresstype=p2sh-segwit", "-changetype=bech32"], ["-addresstype=bech32"], []]
53+ self.num_nodes = 6
54+ self.extra_args = [["-addresstype=legacy"], ["-addresstype=p2sh-segwit"], ["-addresstype=p2sh-segwit", "-changetype=bech32"], ["-addresstype=bech32"], ["-changetype=p2sh-segwit"], []]
Perhaps clearer as:
0 self.extra_args = [["-addresstype=legacy"], # node0
1 ["-addresstype=p2sh-segwit"], # node1
2 ["-addresstype=p2sh-segwit", "-changetype=bech32"], # node2
3 ["-addresstype=bech32"], # node3
4 ["-changetype=p2sh-segwit"], # node4
5 []] # node5
247+
248+ self.log.debug('getrawchangeaddress fails with invalid changetype argument')
249+ assert_raises_rpc_error(-5, "Unknown address type 'bech23'", self.nodes[3].getrawchangeaddress, 'bech23')
250+
251+ self.log.debug("Nodes with addresstype=p2sh-segwit only use a P2WPKH change output " +
252+ "if any destination address is bech32:")
Tested ACK dc2812f3e40dbbc7533a2d2e15cf4dfb3ae079af.
A bunch of style nits inline. Otherwise looks great.
Only if -changetype is not set and -addresstype is not "legacy".
2643@@ -2644,6 +2644,34 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
2644 return true;
2645 }
2646
2647+OutputType CWallet::TransactionChangeType(const std::vector<CRecipient>& vecSend)
TransactionChangeType() doesn’t really need to be a class member function on CWallet, and could be a utility function in walletutil.cpp. In general, it’d be nice to start making the wallet class and wallet.cpp a bit smaller, but that’s probably a tidy-up for another day.
2643@@ -2644,6 +2644,34 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
2644 return true;
2645 }
2646
2647+OutputType CWallet::TransactionChangeType(const std::vector<CRecipient>& vecSend)
2648+{
2649+ // If -changetype is specified, always use that change type.
2650+ if (g_change_type != OUTPUT_TYPE_NONE) {
For the future, it might be a good idea add change_type and address_type fields to CoinControl class, and have this method accept a CoinControl argument, instead of relying on the globals. This would make it easier to ensure that when an RPC is being called that has address type options, that they will take precedence over the globals.
Pieter’s suggestion of adding a separate P2SH_SEGWIT_BUT_NATIVE_WHEN_NATIVE_DESTINATION enum value instead of assigning multiple meanings to OUTPUT_TYPE_NONE is also a good idea #12119 (comment).
Other cleanups are also possible. I think a nice refactoring PR that didn’t change behavior could be submitted that:
DEFAULT enum value and just set P2SH_SEGWIT as default during initialization.NONE enum value, replacing it with PARSE_ERROR and P2SH_SEGWIT_BUT_NATIVE_WHEN_NATIVE_DESTINATION (or something shorter like MATCH_OTHER_OUTPUTS).
Labels
Wallet
Milestone
0.16.0