mirror of
https://github.com/dashpay/dash.git
synced 2024-12-26 04:22:55 +01:00
6b71f274ae
e073f1dfda7a2a2cb2be9fe2a1d576f122596021 test: make sure keypool sizes do not change on `getrawchangeaddress`/`getnewaddress` failures (UdjinM6)
367bb7a80cc71130995672c853d4a6e0134721d6 wallet: Avoid updating `ReserveDestination::nIndex` when `GetReservedDestination` fails (UdjinM6)
Pull request description:
I think the expected behaviour of `getrawchangeaddress` and `getnewaddress` RPCs is that their failure should not affect keypool in any way. At least that's how legacy wallets work, you can confirm this behaviour by running `wallet_keypool.py --legacy-wallet` on master with e073f1dfda7a2a2cb2be9fe2a1d576f122596021 applied on top. However running `wallet_keypool.py --descriptors` on the same commit results in the following failure:
```
File "/path/to/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/path/to/bitcoin/test/functional/wallet_keypool.py", line 114, in run_test
assert_equal(kp_size_before, kp_size_after)
File "/path/to/bitcoin/test/functional/test_framework/util.py", line 57, in assert_equal
raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not([18, 24] == [19, 24])
```
This happens because we pass `nIndex` (which is a class member) into `GetReservedDestination` and since it's passed by reference we get an updated value back, so `nIndex` won't be equal `-1` anymore, no matter if the function failed or succeeded. This means that `ReturnDestination` (called by dtor of `ReserveDestination`) will try to return something we did not actually reserve.
The fix is to simply use a temporary variable instead of a class member and only update `nIndex` when `op_address` actually has value, basically do it the same way we do for other class members (`address` and `fInternal`) already.
ACKs for top commit:
achow101:
ACK e073f1dfda7a2a2cb2be9fe2a1d576f122596021
josibake:
ACK e073f1dfda
Tree-SHA512: 1128288a60dd4d8f306ef6f7ac66cdfeae3c9cc35c66ecada2d78fa61ac759f2a757b70fc3976ba8b5081200942b58dfabc184c01ccf911af40ba8c145344651
215 lines
9.6 KiB
Python
Executable File
215 lines
9.6 KiB
Python
Executable File
#!/usr/bin/env python3
|
|
# Copyright (c) 2014-2015 The Bitcoin Core developers
|
|
# Distributed under the MIT software license, see the accompanying
|
|
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
|
|
|
# Exercise the wallet keypool, and interaction with wallet encryption/locking
|
|
|
|
# Add python-bitcoinrpc to module search path:
|
|
|
|
import time
|
|
from decimal import Decimal
|
|
|
|
from test_framework.authproxy import JSONRPCException
|
|
from test_framework.test_framework import BitcoinTestFramework
|
|
from test_framework.util import assert_equal, assert_raises_rpc_error
|
|
|
|
class KeyPoolTest(BitcoinTestFramework):
|
|
|
|
def set_test_params(self):
|
|
self.num_nodes = 1
|
|
self.extra_args = [['-usehd=1']]
|
|
|
|
def skip_test_if_missing_module(self):
|
|
self.skip_if_no_wallet()
|
|
|
|
def run_test(self):
|
|
nodes = self.nodes
|
|
addr_before_encrypting = nodes[0].getnewaddress()
|
|
addr_before_encrypting_data = nodes[0].getaddressinfo(addr_before_encrypting)
|
|
wallet_info_old = nodes[0].getwalletinfo()
|
|
if not self.options.descriptors:
|
|
assert addr_before_encrypting_data['hdchainid'] == wallet_info_old['hdchainid']
|
|
|
|
# Encrypt wallet and wait to terminate
|
|
nodes[0].encryptwallet('test')
|
|
if self.options.descriptors:
|
|
# Import hardened derivation only descriptors
|
|
nodes[0].walletpassphrase('test', 10)
|
|
nodes[0].importdescriptors([
|
|
{
|
|
"desc": "pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1h/*h)#a0nyvl0k",
|
|
"timestamp": "now",
|
|
"range": [0,0],
|
|
"active": True
|
|
},
|
|
{
|
|
"desc": "sh(pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/2h/*h))#lmeu2axg",
|
|
"timestamp": "now",
|
|
"range": [0,0],
|
|
"active": True
|
|
},
|
|
{
|
|
"desc": "pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/4h/*h)#l3crwaus",
|
|
"timestamp": "now",
|
|
"range": [0,0],
|
|
"active": True,
|
|
"internal": True
|
|
},
|
|
{
|
|
"desc": "sh(pkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/5h/*h))#qg8wa75f",
|
|
"timestamp": "now",
|
|
"range": [0,0],
|
|
"active": True,
|
|
"internal": True
|
|
}
|
|
])
|
|
nodes[0].walletlock()
|
|
# Keep creating keys
|
|
addr = nodes[0].getnewaddress()
|
|
addr_data = nodes[0].getaddressinfo(addr)
|
|
wallet_info = nodes[0].getwalletinfo()
|
|
|
|
# TODO: enable this assert after bitcoin#17681 is backported
|
|
# assert addr_before_encrypting_data['hdmasterfingerprint'] != addr_data['hdmasterfingerprint']
|
|
if not self.options.descriptors:
|
|
assert addr_data['hdchainid'] == wallet_info['hdchainid']
|
|
|
|
try:
|
|
addr = nodes[0].getnewaddress()
|
|
raise AssertionError('Keypool should be exhausted after one address')
|
|
except JSONRPCException as e:
|
|
assert e.error['code']==-12
|
|
|
|
# put six (plus 2) new keys in the keypool (100% external-, +100% internal-keys, 1 in min)
|
|
nodes[0].walletpassphrase('test', 12000)
|
|
nodes[0].keypoolrefill(6)
|
|
nodes[0].walletlock()
|
|
wi = nodes[0].getwalletinfo()
|
|
if self.options.descriptors:
|
|
# this counters are zero, bitcoin have here 6 * 3 (3 different types)
|
|
assert_equal(wi['keypoolsize_hd_internal'], 6)
|
|
assert_equal(wi['keypoolsize'], 6)
|
|
else:
|
|
assert_equal(wi['keypoolsize_hd_internal'], 6)
|
|
assert_equal(wi['keypoolsize'], 6)
|
|
|
|
# drain the internal keys
|
|
nodes[0].getrawchangeaddress()
|
|
nodes[0].getrawchangeaddress()
|
|
nodes[0].getrawchangeaddress()
|
|
nodes[0].getrawchangeaddress()
|
|
nodes[0].getrawchangeaddress()
|
|
nodes[0].getrawchangeaddress()
|
|
# remember keypool sizes
|
|
wi = nodes[0].getwalletinfo()
|
|
kp_size_before = [wi['keypoolsize_hd_internal'], wi['keypoolsize']]
|
|
# the next one should fail
|
|
try:
|
|
nodes[0].getrawchangeaddress()
|
|
raise AssertionError('Keypool should be exhausted after six addresses')
|
|
except JSONRPCException as e:
|
|
assert e.error['code']==-12
|
|
# check that keypool sizes did not change
|
|
wi = nodes[0].getwalletinfo()
|
|
kp_size_after = [wi['keypoolsize_hd_internal'], wi['keypoolsize']]
|
|
assert_equal(kp_size_before, kp_size_after)
|
|
|
|
addr = set()
|
|
# drain the external keys
|
|
addr.add(nodes[0].getnewaddress())
|
|
addr.add(nodes[0].getnewaddress())
|
|
addr.add(nodes[0].getnewaddress())
|
|
addr.add(nodes[0].getnewaddress())
|
|
addr.add(nodes[0].getnewaddress())
|
|
addr.add(nodes[0].getnewaddress())
|
|
assert len(addr) == 6
|
|
# remember keypool sizes
|
|
wi = nodes[0].getwalletinfo()
|
|
kp_size_before = [wi['keypoolsize_hd_internal'], wi['keypoolsize']]
|
|
# the next one should fail
|
|
try:
|
|
addr = nodes[0].getnewaddress()
|
|
raise AssertionError('Keypool should be exhausted after six addresses')
|
|
except JSONRPCException as e:
|
|
assert e.error['code']==-12
|
|
# check that keypool sizes did not change
|
|
wi = nodes[0].getwalletinfo()
|
|
kp_size_after = [wi['keypoolsize_hd_internal'], wi['keypoolsize']]
|
|
assert_equal(kp_size_before, kp_size_after)
|
|
|
|
# refill keypool with three new addresses
|
|
nodes[0].walletpassphrase('test', 1)
|
|
nodes[0].keypoolrefill(3)
|
|
# test walletpassphrase timeout
|
|
time.sleep(1.1)
|
|
assert_equal(nodes[0].getwalletinfo()["unlocked_until"], 0)
|
|
|
|
# drain the keypool
|
|
for _ in range(3):
|
|
nodes[0].getnewaddress()
|
|
assert_raises_rpc_error(-12, "Keypool ran out", nodes[0].getnewaddress)
|
|
|
|
nodes[0].walletpassphrase('test', 100)
|
|
nodes[0].keypoolrefill(100)
|
|
wi = nodes[0].getwalletinfo()
|
|
if self.options.descriptors:
|
|
# dash has only 1 type of output addresses
|
|
assert_equal(wi['keypoolsize_hd_internal'], 100)
|
|
assert_equal(wi['keypoolsize'], 100)
|
|
else:
|
|
assert_equal(wi['keypoolsize_hd_internal'], 100)
|
|
assert_equal(wi['keypoolsize'], 100)
|
|
|
|
# create a blank wallet
|
|
nodes[0].createwallet(wallet_name='w2', blank=True, disable_private_keys=True)
|
|
w2 = nodes[0].get_wallet_rpc('w2')
|
|
|
|
# refer to initial wallet as w1
|
|
w1 = nodes[0].get_wallet_rpc(self.default_wallet_name)
|
|
|
|
# import private key and fund it
|
|
address = addr.pop()
|
|
desc = w1.getaddressinfo(address)['desc']
|
|
if self.options.descriptors:
|
|
res = w2.importdescriptors([{'desc': desc, 'timestamp': 'now'}])
|
|
else:
|
|
res = w2.importmulti([{'desc': desc, 'timestamp': 'now'}])
|
|
assert_equal(res[0]['success'], True)
|
|
w1.walletpassphrase('test', 100)
|
|
|
|
res = w1.sendtoaddress(address=address, amount=0.00010000)
|
|
nodes[0].generate(1)
|
|
destination = addr.pop()
|
|
|
|
# Using a fee rate (10 sat / byte) well above the minimum relay rate
|
|
# creating a 5,000 sat transaction with change should not be possible
|
|
assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it. Please call keypoolrefill first.", w2.walletcreatefundedpsbt, inputs=[], outputs=[{addr.pop(): 0.00005000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010})
|
|
|
|
# creating a 10,000 sat transaction without change, with a manual input, should still be possible
|
|
res = w2.walletcreatefundedpsbt(inputs=w2.listunspent(), outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010})
|
|
assert_equal("psbt" in res, True)
|
|
|
|
# creating a 10,000 sat transaction without change should still be possible
|
|
res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010})
|
|
assert_equal("psbt" in res, True)
|
|
# should work without subtractFeeFromOutputs if the exact fee is subtracted from the amount
|
|
res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008900}], options={"feeRate": 0.000010})
|
|
assert_equal("psbt" in res, True)
|
|
|
|
# dust change should be removed
|
|
res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008800}], options={"feeRate": 0.000010})
|
|
assert_equal("psbt" in res, True)
|
|
|
|
# create a transaction without change at the maximum fee rate, such that the output is still spendable:
|
|
res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.00008824})
|
|
assert_equal("psbt" in res, True)
|
|
assert_equal(res["fee"], Decimal("0.00001694"))
|
|
|
|
# creating a 10,000 sat transaction with a manual change address should be possible
|
|
res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.000010, "changeAddress": addr.pop()})
|
|
assert_equal("psbt" in res, True)
|
|
|
|
if __name__ == '__main__':
|
|
KeyPoolTest().main()
|