Skip to content

Commit

Permalink
Create a tr() descriptor bech32m DescriptorScriptPubKeyMan by default
Browse files Browse the repository at this point in the history
  • Loading branch information
achow101 committed Nov 16, 2021
1 parent 54b3699 commit 8fb5784
Show file tree
Hide file tree
Showing 13 changed files with 80 additions and 54 deletions.
11 changes: 4 additions & 7 deletions src/wallet/scriptpubkeyman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1876,12 +1876,6 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const

bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal)
{
if (addr_type == OutputType::BECH32M) {
// Don't allow setting up taproot descriptors yet
// TODO: Allow setting up taproot descriptors
return false;
}

LOCK(cs_desc_man);
assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));

Expand Down Expand Up @@ -1911,7 +1905,10 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_
desc_prefix = "wpkh(" + xpub + "/84'";
break;
}
case OutputType::BECH32M: assert(false); // TODO: Setup taproot descriptor
case OutputType::BECH32M: {
desc_prefix = "tr(" + xpub + "/86'";
break;
}
} // no default case, so the compiler can warn about missing cases
assert(!desc_prefix.empty());

Expand Down
3 changes: 0 additions & 3 deletions src/wallet/test/fuzz/notifications.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ struct FuzzedWallet {
CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider)
{
auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)};
if (type == OutputType::BECH32M) {
type = OutputType::BECH32; // TODO: Setup taproot descriptor and remove this line
}
CTxDestination dest;
bilingual_str error;
if (fuzzed_data_provider.ConsumeBool()) {
Expand Down
5 changes: 0 additions & 5 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3164,11 +3164,6 @@ void CWallet::SetupDescriptorScriptPubKeyMans()

for (bool internal : {false, true}) {
for (OutputType t : OUTPUT_TYPES) {
if (t == OutputType::BECH32M) {
// Skip taproot (bech32m) for now
// TODO: Setup taproot (bech32m) descriptors by default
continue;
}
auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this));
if (IsCrypted()) {
if (IsLocked()) {
Expand Down
23 changes: 15 additions & 8 deletions test/functional/rpc_fundrawtransaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -571,12 +571,12 @@ def test_locked_wallet(self):
if self.options.descriptors:
self.nodes[1].walletpassphrase('test', 10)
self.nodes[1].importdescriptors([{
'desc': descsum_create('wpkh(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/0h/*h)'),
'desc': descsum_create('tr(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/0h/*h)'),
'timestamp': 'now',
'active': True
},
{
'desc': descsum_create('wpkh(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/1h/*h)'),
'desc': descsum_create('tr(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/1h/*h)'),
'timestamp': 'now',
'active': True,
'internal': True
Expand Down Expand Up @@ -778,11 +778,18 @@ def test_option_feerate(self):
for param, zero_value in product(["fee_rate", "feeRate"], [0, 0.000, 0.00000000, "0", "0.000", "0.00000000"]):
assert_equal(self.nodes[3].fundrawtransaction(rawtx, {param: zero_value})["fee"], 0)

# With no arguments passed, expect fee of 141 satoshis.
assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000141, vspan=0.00000001)
# Expect fee to be 10,000x higher when an explicit fee rate 10,000x greater is specified.
result = node.fundrawtransaction(rawtx, {"fee_rate": 10000})
assert_approx(result["fee"], vexp=0.0141, vspan=0.0001)
if self.options.descriptors:
# With no arguments passed, expect fee of 153 satoshis as descriptor wallets now have a taproot output.
assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000153, vspan=0.00000001)
# Expect fee to be 10,000x higher when an explicit fee rate 10,000x greater is specified.
result = node.fundrawtransaction(rawtx, {"fee_rate": 10000})
assert_approx(result["fee"], vexp=0.0153, vspan=0.0001)
else:
# With no arguments passed, expect fee of 141 satoshis as legacy wallets only support up to segwit v0.
assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000141, vspan=0.00000001)
# Expect fee to be 10,000x higher when an explicit fee rate 10,000x greater is specified.
result = node.fundrawtransaction(rawtx, {"fee_rate": 10000})
assert_approx(result["fee"], vexp=0.0141, vspan=0.0001)

self.log.info("Test fundrawtxn with invalid estimate_mode settings")
for k, v in {"number": 42, "object": {"foo": "bar"}}.items():
Expand Down Expand Up @@ -1073,7 +1080,7 @@ def test_22670(self):
# Make sure the default wallet will not be loaded when restarted with a high minrelaytxfee
self.nodes[0].unloadwallet(self.default_wallet_name, False)
feerate = Decimal("0.1")
self.restart_node(0, [f"-minrelaytxfee={feerate}", "-discardfee=0"]) # Set high minrelayfee, set discardfee to 0 for easier calculation
self.restart_node(0, [f"-minrelaytxfee={feerate}", "-discardfee=0", "-changetype=bech32", "-addresstype=bech32"]) # Set high minrelayfee, set discardfee to 0 for easier calculation

self.nodes[0].loadwallet(self.default_wallet_name, True)
funds = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
Expand Down
2 changes: 1 addition & 1 deletion test/functional/rpc_psbt.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class PSBTTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 3
self.extra_args = [
["-walletrbf=1"],
["-walletrbf=1", "-addresstype=bech32", "-changetype=bech32"], #TODO: Remove address type restrictions once taproot has psbt extensions
["-walletrbf=0", "-changetype=legacy"],
[]
]
Expand Down
7 changes: 4 additions & 3 deletions test/functional/tool_wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ def log_wallet_timestamp_comparison(self, old, new):

def get_expected_info_output(self, name="", transactions=0, keypool=2, address=0):
wallet_name = self.default_wallet_name if name == "" else name
output_types = 3 # p2pkh, p2sh, segwit
if self.options.descriptors:
output_types = 4 # p2pkh, p2sh, segwit, bech32m
return textwrap.dedent('''\
Wallet info
===========
Expand All @@ -85,6 +85,7 @@ def get_expected_info_output(self, name="", transactions=0, keypool=2, address=0
Address Book: %d
''' % (wallet_name, keypool * output_types, transactions, address))
else:
output_types = 3 # p2pkh, p2sh, segwit. Legacy wallets do not support bech32m.
return textwrap.dedent('''\
Wallet info
===========
Expand Down Expand Up @@ -298,8 +299,8 @@ def test_getwalletinfo_on_different_wallet(self):
assert_equal(1000, out['keypoolsize_hd_internal'])
assert_equal(True, 'hdseedid' in out)
else:
assert_equal(3000, out['keypoolsize'])
assert_equal(3000, out['keypoolsize_hd_internal'])
assert_equal(4000, out['keypoolsize'])
assert_equal(4000, out['keypoolsize_hd_internal'])

self.log_wallet_timestamp_comparison(timestamp_before, timestamp_after)
assert_equal(timestamp_before, timestamp_after)
Expand Down
41 changes: 29 additions & 12 deletions test/functional/wallet_address_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ def test_address(self, node, address, multisig, typ):
assert_equal(info['witness_version'], 0)
assert_equal(len(info['witness_program']), 40)
assert 'pubkey' in info
elif not multisig and typ == "bech32m":
# P2TR single sig
assert info["isscript"]
assert info["iswitness"]
assert_equal(info["witness_version"], 1)
assert_equal(len(info["witness_program"]), 64)
elif typ == 'legacy':
# P2SH-multisig
assert info['isscript']
Expand Down Expand Up @@ -339,19 +345,31 @@ def run_test(self):
self.log.info("Nodes with addresstype=legacy never use a P2WPKH change output (unless changetype is set otherwise):")
self.test_change_output_type(0, [to_address_bech32_1], 'legacy')

self.log.info("Nodes with addresstype=p2sh-segwit only use a P2WPKH change output if any destination address is bech32:")
self.test_change_output_type(1, [to_address_p2sh], 'p2sh-segwit')
self.test_change_output_type(1, [to_address_bech32_1], 'bech32')
self.test_change_output_type(1, [to_address_p2sh, to_address_bech32_1], 'bech32')
self.test_change_output_type(1, [to_address_bech32_1, to_address_bech32_2], 'bech32')
if self.options.descriptors:
self.log.info("Nodes with addresstype=p2sh-segwit only use a bech32m change output if any destination address is bech32:")
self.test_change_output_type(1, [to_address_p2sh], 'p2sh-segwit')
self.test_change_output_type(1, [to_address_bech32_1], 'bech32m')
self.test_change_output_type(1, [to_address_p2sh, to_address_bech32_1], 'bech32m')
self.test_change_output_type(1, [to_address_bech32_1, to_address_bech32_2], 'bech32m')
else:
self.log.info("Nodes with addresstype=p2sh-segwit only use a P2WPKH change output if any destination address is bech32:")
self.test_change_output_type(1, [to_address_p2sh], 'p2sh-segwit')
self.test_change_output_type(1, [to_address_bech32_1], 'bech32')
self.test_change_output_type(1, [to_address_p2sh, to_address_bech32_1], 'bech32')
self.test_change_output_type(1, [to_address_bech32_1, to_address_bech32_2], 'bech32')

self.log.info("Nodes with change_type=bech32 always use a P2WPKH change output:")
self.test_change_output_type(2, [to_address_bech32_1], 'bech32')
self.test_change_output_type(2, [to_address_p2sh], 'bech32')

self.log.info("Nodes with addresstype=bech32 always use a P2WPKH change output (unless changetype is set otherwise):")
self.test_change_output_type(3, [to_address_bech32_1], 'bech32')
self.test_change_output_type(3, [to_address_p2sh], 'bech32')
if self.options.descriptors:
self.log.info("Nodes with addresstype=bech32 always use either a bech32 or bech32m change output (unless changetype is set otherwise):")
self.test_change_output_type(3, [to_address_bech32_1], 'bech32m')
self.test_change_output_type(3, [to_address_p2sh], 'bech32')
else:
self.log.info("Nodes with addresstype=bech32 always use a P2WPKH change output (unless changetype is set otherwise):")
self.test_change_output_type(3, [to_address_bech32_1], 'bech32')
self.test_change_output_type(3, [to_address_p2sh], 'bech32')

self.log.info('getrawchangeaddress defaults to addresstype if -changetype is not set and argument is absent')
self.test_address(3, self.nodes[3].getrawchangeaddress(), multisig=False, typ='bech32')
Expand All @@ -370,10 +388,9 @@ def run_test(self):
self.test_address(4, self.nodes[4].getrawchangeaddress('bech32'), multisig=False, typ='bech32')

if self.options.descriptors:
self.log.info("Descriptor wallets do not have bech32m addresses by default yet")
# TODO: Remove this when they do
assert_raises_rpc_error(-12, "Error: No bech32m addresses available", self.nodes[0].getnewaddress, "", "bech32m")
assert_raises_rpc_error(-12, "Error: No bech32m addresses available", self.nodes[0].getrawchangeaddress, "bech32m")
self.log.info("Descriptor wallets have bech32m addresses")
self.test_address(4, self.nodes[4].getnewaddress("", "bech32m"), multisig=False, typ="bech32m")
self.test_address(4, self.nodes[4].getrawchangeaddress("bech32m"), multisig=False, typ="bech32m")
else:
self.log.info("Legacy wallets cannot make bech32m addresses")
assert_raises_rpc_error(-8, "Legacy wallets cannot provide bech32m addresses", self.nodes[0].getnewaddress, "", "bech32m")
Expand Down
2 changes: 1 addition & 1 deletion test/functional/wallet_createwallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def run_test(self):
w6.keypoolrefill(1)
# There should only be 1 key for legacy, 3 for descriptors
walletinfo = w6.getwalletinfo()
keys = 3 if self.options.descriptors else 1
keys = 4 if self.options.descriptors else 1
assert_equal(walletinfo['keypoolsize'], keys)
assert_equal(walletinfo['keypoolsize_hd_internal'], keys)
# Allow empty passphrase, but there should be a warning
Expand Down
6 changes: 3 additions & 3 deletions test/functional/wallet_descriptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ def run_test(self):
self.log.info("Making a descriptor wallet")
self.nodes[0].createwallet(wallet_name="desc1", descriptors=True)

# A descriptor wallet should have 100 addresses * 3 types = 300 keys
# A descriptor wallet should have 100 addresses * 4 types = 400 keys
self.log.info("Checking wallet info")
wallet_info = self.nodes[0].getwalletinfo()
assert_equal(wallet_info['format'], 'sqlite')
assert_equal(wallet_info['keypoolsize'], 300)
assert_equal(wallet_info['keypoolsize_hd_internal'], 300)
assert_equal(wallet_info['keypoolsize'], 400)
assert_equal(wallet_info['keypoolsize_hd_internal'], 400)
assert 'keypoololdest' not in wallet_info

# Check that getnewaddress works
Expand Down
18 changes: 15 additions & 3 deletions test/functional/wallet_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,24 @@ def run_test(self):
assert_equal(input_addrs[0], input_addrs[1])
# Node 2 enforces avoidpartialspends so needs no checking here

if self.options.descriptors:
# Descriptor wallets will use Taproot change by default which has different fees
tx4_ungrouped_fee = 3060
tx4_grouped_fee = 4400
tx5_6_ungrouped_fee = 5760
tx5_6_grouped_fee = 8480
else:
tx4_ungrouped_fee = 2820
tx4_grouped_fee = 4160
tx5_6_ungrouped_fee = 5520
tx5_6_grouped_fee = 8240

self.log.info("Test wallet option maxapsfee")
addr_aps = self.nodes[3].getnewaddress()
self.nodes[0].sendtoaddress(addr_aps, 1.0)
self.nodes[0].sendtoaddress(addr_aps, 1.0)
self.generate(self.nodes[0], 1)
with self.nodes[3].assert_debug_log(['Fee non-grouped = 2820, grouped = 4160, using grouped']):
with self.nodes[3].assert_debug_log([f'Fee non-grouped = {tx4_ungrouped_fee}, grouped = {tx4_grouped_fee}, using grouped']):
txid4 = self.nodes[3].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
tx4 = self.nodes[3].getrawtransaction(txid4, True)
# tx4 should have 2 inputs and 2 outputs although one output would
Expand All @@ -124,7 +136,7 @@ def run_test(self):
addr_aps2 = self.nodes[3].getnewaddress()
[self.nodes[0].sendtoaddress(addr_aps2, 1.0) for _ in range(5)]
self.generate(self.nodes[0], 1)
with self.nodes[3].assert_debug_log(['Fee non-grouped = 5520, grouped = 8240, using non-grouped']):
with self.nodes[3].assert_debug_log([f'Fee non-grouped = {tx5_6_ungrouped_fee}, grouped = {tx5_6_grouped_fee}, using non-grouped']):
txid5 = self.nodes[3].sendtoaddress(self.nodes[0].getnewaddress(), 2.95)
tx5 = self.nodes[3].getrawtransaction(txid5, True)
# tx5 should have 3 inputs (1.0, 1.0, 1.0) and 2 outputs
Expand All @@ -137,7 +149,7 @@ def run_test(self):
addr_aps3 = self.nodes[4].getnewaddress()
[self.nodes[0].sendtoaddress(addr_aps3, 1.0) for _ in range(5)]
self.generate(self.nodes[0], 1)
with self.nodes[4].assert_debug_log(['Fee non-grouped = 5520, grouped = 8240, using grouped']):
with self.nodes[4].assert_debug_log([f'Fee non-grouped = {tx5_6_ungrouped_fee}, grouped = {tx5_6_grouped_fee}, using grouped']):
txid6 = self.nodes[4].sendtoaddress(self.nodes[0].getnewaddress(), 2.95)
tx6 = self.nodes[4].getrawtransaction(txid6, True)
# tx6 should have 5 inputs and 2 outputs
Expand Down
2 changes: 1 addition & 1 deletion test/functional/wallet_hd.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def run_test(self):
keypath = self.nodes[1].getaddressinfo(out['scriptPubKey']['address'])['hdkeypath']

if self.options.descriptors:
assert_equal(keypath[0:14], "m/84'/1'/0'/1/")
assert_equal(keypath[0:14], "m/86'/1'/0'/1/")
else:
assert_equal(keypath[0:7], "m/0'/1'")

Expand Down
8 changes: 4 additions & 4 deletions test/functional/wallet_keypool.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ def run_test(self):
nodes[0].walletlock()
wi = nodes[0].getwalletinfo()
if self.options.descriptors:
assert_equal(wi['keypoolsize_hd_internal'], 18)
assert_equal(wi['keypoolsize'], 18)
assert_equal(wi['keypoolsize_hd_internal'], 24)
assert_equal(wi['keypoolsize'], 24)
else:
assert_equal(wi['keypoolsize_hd_internal'], 6)
assert_equal(wi['keypoolsize'], 6)
Expand Down Expand Up @@ -132,8 +132,8 @@ def run_test(self):
nodes[0].keypoolrefill(100)
wi = nodes[0].getwalletinfo()
if self.options.descriptors:
assert_equal(wi['keypoolsize_hd_internal'], 300)
assert_equal(wi['keypoolsize'], 300)
assert_equal(wi['keypoolsize_hd_internal'], 400)
assert_equal(wi['keypoolsize'], 400)
else:
assert_equal(wi['keypoolsize_hd_internal'], 100)
assert_equal(wi['keypoolsize'], 100)
Expand Down
6 changes: 3 additions & 3 deletions test/functional/wallet_listdescriptors.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ def run_test(self):
node.createwallet(wallet_name='w3', descriptors=True)
result = node.get_wallet_rpc('w3').listdescriptors()
assert_equal("w3", result['wallet_name'])
assert_equal(6, len(result['descriptors']))
assert_equal(6, len([d for d in result['descriptors'] if d['active']]))
assert_equal(3, len([d for d in result['descriptors'] if d['internal']]))
assert_equal(8, len(result['descriptors']))
assert_equal(8, len([d for d in result['descriptors'] if d['active']]))
assert_equal(4, len([d for d in result['descriptors'] if d['internal']]))
for item in result['descriptors']:
assert item['desc'] != ''
assert item['next'] == 0
Expand Down

0 comments on commit 8fb5784

Please sign in to comment.