aaaa3aefbd test: Use TestNode *_path properties where possible (MarcoFalke)
dddd89962b test: Allow pathlib.Path as RPC argument via authproxy (MarcoFalke)
fa41614a0a scripted-diff: Use wallets_path and chain_path where possible (MarcoFalke)
fa493fadfb test: Use wallet_dir lambda in wallet_multiwallet test where possible (MarcoFalke)
Pull request description:
It seems inconsistent, fragile and verbose to:
* Call `get_datadir_path` to recreate the path that already exists as field in TestNode
* Call `os.path.join` with the hardcoded chain name or `self.chain` to recreate the TestNode `chain_path` property
* Sometimes even use the hardcoded node dir name (`"node0"`)
Fix all issues by using the TestNode properties.
ACKs for top commit:
willcl-ark:
re-ACK aaaa3aefbd
theStack:
Code-review ACK aaaa3aefbd🌊
Tree-SHA512: e4720278085beb8164e1fe6c1aa18f601558a9263494ce69a83764c1487007de63ebb51d1b1151862dc4d5b49ded6162a5c1553cd30ea1c28627d447db4d8e72
In functional tests it is a quite common scenario to generate fresh
elliptic curve keypairs, which is currently a bit cumbersome as it
involves multiple steps, e.g.:
privkey = ECKey()
privkey.generate()
privkey_wif = bytes_to_wif(privkey.get_bytes())
pubkey = privkey.get_pubkey().get_bytes()
Simplify this by providing a new `generate_keypair` helper function that
returns the private key either as `ECKey` object or as WIF-string
(depending on the boolean `wif` parameter) and the public key as
byte-string; these formats are what we mostly need (currently we don't
use `ECPubKey` objects from generated keypairs anywhere).
With this, most of the affected code blocks following the pattern above
can be replaced by one-liners, e.g.:
privkey, pubkey = generate_keypair(wif=True)
Note that after this commit, the only direct uses of `ECKey` remain in
situations where we want to set the private key explicitly, e.g. in
MiniWallet (test/functional/test_framework/wallet.py) or the test for
the signet miner script (test/functional/tool_signet_miner.py).
This commit updates the code by replacing the RPC call used to
decode an address and retrieve its corresponding scriptpubkey
with the address_to_scriptpubkey function. address_to_scriptpubkey
function can now decode all addresses formats, which makes
it more efficient to use.
Review note: The changes are complete, because self.options.descriptors
is set to None in parse_args (test_framework.py).
A value of None implies -disablewallet, see the previous commit.
So if a call to add_wallet_options is missing, it will lead to a test
failure when the wallet is compiled in.
The bool is only used to call a public helper, which some tests already
do. So use the public helper in all tests consistently and make the
confusingly named bool private.
The previous diff touched most files in ./test/, so bump the headers to
avoid having to touch them again for a bump later.
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./test/
-END VERIFY SCRIPT-
We don't want the legacy wallet to ever have bech32m addresses so don't
allow importing them. This includes addmultisigaddress as that is a
legacy wallet only RPC
Additionally, bech32m multisigs are not available yet, so disallow them
in createmultisig.
1) add a new sane "address" field (for outputs that have an
identifiable address, which doesn't include bare multisig)
2) with -deprecatedrpc: leave "reqSigs" and "addresses" intact
(with all weird/wrong behavior they have now)
3) without -deprecatedrpc: drop "reqSigs" and "addresses" entirely,
always.
Adds a --descriptors option globally to the test framework. This will
make the test create and use descriptor wallets. However some tests may
not work with this.
Some tests are modified to work with --descriptors and run with that
option in test_runer:
* wallet_basic.py
* wallet_encryption.py
* wallet_keypool.py
* wallet_keypool_topup.py
* wallet_labels.py
* wallet_avoidreuse.py
This adds checks to ensure the redeemScript/witnessScript actually
correspond to the provided scriptPubKey, and, if both are provided,
that they are sensibly related to each other.
Thanks to github user passionofvc for raising this issue.
01174596e6 signrawtransactionwithkey: report error when missing redeemScript/witnessScript param (Anthony Towns)
Pull request description:
Adding support for "witnessScript" as an alternative to "redeemScript" when using "signrawtransactionwithkey" meant that the `RPCTypeCheckObj()` call in `SignTransaction` can't error out just because either parameter is missing -- it's only a problem if both are missing, which isn't a state `RPCTypeCheckObj()` tests for. This results in the regression described in #16249. This patch adds some code to test for this case and give a similar error, namely:
error code: -8
error message:
Missing redeemScript/witnessScript
Fixes: #16249
ACKs for top commit:
meshcollider:
utACK 01174596e6
promag:
ACK 01174596e. Could also write test without `dict`/`del`:
Tree-SHA512: cf51346b7dea551b7f18f2a93c2a336a293b2535c62c03a5263cd2be8c58cf0cc302891da659c167e88ad1a68a756472c3c07e99f71627c61d32886fc5a3a353
Instead of creating a redeemScript with CreateMultisigRedeemscript and
then getting the destination with AddAndGetDestinationForScript, do
both in the same function.
CreateMultisigRedeemscript is changed to AddAndGetMultisigDestination.
It creates the redeemScript and returns it via an output parameter. Then
it calls AddAndGetDestinationForScript to add the destination to the
keystore and get the proper destination.
This allows us to inspect the public keys in the redeemScript before creating
the destination so that the correct destination is used when uncompressed
pubkeys are in the multisig.
