ffff9dcdc3 test: Explain why test logging should be used (MarcoFalke)
Pull request description:
Background is that some tests don't have any `self.log` call at all. Thus there are no "anchor points" and those tests are hard to debug because the logs can't easily be parsed by a human.
ACKs for top commit:
jonatack:
ACK ffff9dcdc3
instagibbs:
ACK ffff9dcdc3
fanquake:
re-ACK ffff9dcdc3
Tree-SHA512: 08d962e85c4892c2a0c58feb5dc697c680a9d68e41a79417da6fcd415e0c5c735c4533a985cf225bb89deb5ca717d9bedf990657958079185804caa512b10f5a
cbf2d75d8f qa: Add getdescriptorinfo functional test (João Barbosa)
Pull request description:
The `getdescriptorinfo` RPC was added in #15368, this PR adds some tests.
Top commit has no ACKs.
Tree-SHA512: 5bf3fb5842b975089821c7ac52202ecb23df255f655862646eb532e38e335ff963f8973bcf5b8bba386183281dc9bfe7279ba1cf25fd518c9a45fb45a9243e4d
d484279a46 test: add logging to wallet_listsinceblock.py (Jon Atack)
Pull request description:
This is the first commit from #17535.
Top commit has no ACKs.
Tree-SHA512: bb4f527a41bca3ffbf69e910311ce7f85dcc7a2be41350b3c653a27f4044f392b7e528f330e9691f497212469f6b16ce263230bb7a919548dd4e3e21cc72142f
d2774c09cf Clear any input_errors for an input after it is signed (Andrew Chow)
dc174881ad Replace GetSigningProvider with GetSolvingProvider (Andrew Chow)
6a9c429084 Move direct calls to MessageSign into new SignMessage functions in CWallet and ScriptPubKeyMan (Andrew Chow)
82a30fade7 Move key and script filling and signing from CWallet::FillPSBT to ScriptPubKeyMan::FillPSBT (Andrew Chow)
3d70dd99f9 Move FillPSBT to be a member of CWallet (Andrew Chow)
a4af324d15 Use CWallet::SignTransaction in CreateTransaction and signrawtransactionwithwallet (Andrew Chow)
f37de92744 Implement CWallet::SignTransaction using ScriptPubKeyMan::SignTransaction (Andrew Chow)
d999dd588c Add SignTransaction function to ScriptPubKeyMan and LegacyScriptPubKeyMan (Andrew Chow)
2c52b59d0a Refactor rawtransaction's SignTransaction into generic SignTransaction function (Andrew Chow)
Pull request description:
Following #17261, the way to sign transactions, PSBTs, and messages was to use `GetSigningProvider()` and get a `SigningProvider` containing the private keys. However this may not be feasible for future `ScriptPubKeyMan`s, such as for hardware wallets. Instead of exporting a `SigningProvider` containing private keys, we need to pass these things into the `ScriptPubKeyMan` (via `CWallet`) so that they can do whatever is needed internally to sign them. This is largely a refactor as the logic of processing transactions, PSBTs, and messages for is moved into `LegacyScriptPubKeyMan` and `CWallet` instead of being handled by the caller (e.g. `signrawtransaction`).
To help with this, I've refactored the 3(!) implementations of a `SignTransaction()` function into one generic one. This function will be called by `signrawtransactionwithkey` and `LegacyScriptPubKeyMan::SignTransaction()`. `CWallet::CreateTransaction()` is changed to call `CWallet::SignTransaction()` which in turn, calls `LegacyScriptPubKeyMan::SignTransaction()`. Other `ScriptPubKeyMan`s may implement `SignTransaction()` differently.
`FillPSBT()` is moved to be a member function of `CWallet` and the `psbtwallet.cpp/h` files removed. It is further split so that `CWallet` handles filling the UTXOs while the `ScriptPubKeyMan` handles adding keys, derivation paths, scripts, and signatures. In the end `LegacyScriptPubKeyMan::FillPSBT` still calls `SignPSBTInput`, but the `SigningProvider` is internal to `LegacyScriptPubKeyMan`. Other `ScriptPubKeyMan`s may do something different.
A new `SignMessage()` function is added to both `CWallet` and `ScriptPubKeyMan`. Instead of having the caller (i.e. `signmessage` or the sign message dialog) get the private key, hash the message, and sign, `ScriptPubKeyMan` will now handle that (`CWallet` passes through to the `ScriptPubKeyMan`s as it does for many functions). This signing code is thus consolidated into `LegacyScriptPubKeyMan::SignMessage()`, though other `ScriptPubKeyMan`s may implement it differently. Additionally, a `SigningError` enum is introduced for the different errors that we expect to see from `SignMessage()`.
Lastly, `GetSigningProvider()` is renamed to `GetPublicSigningProvider()`. It will now only provide pubkeys, key origins, and scripts. `LegacySigningProvider` has it's `GetKey` and `HaveKey` functions changed to only return false. Future implementations should return `HidingSigningProvider`s where private keys are hidden.
Other things like `dumpprivkey` and `dumpwallet` are not changed because they directly need and access the `LegacyScriptPubKeyMan` so are not relevant to future changes.
ACKs for top commit:
instagibbs:
reACK d2774c09cf
Sjors:
re-utACK d2774c09cf
meshcollider:
re-utACK d2774c09cf
Tree-SHA512: 89c83e7e7e9315e283fae145a2264648a9d7f7ace8f3281cb3f44f0b013c988d67ba4fa9726e50c643c0ed921bdd269adaec984840d11acf4a681f3e8a582cc1
46a1ee7732 doc: Add historical release notes for 0.19.1 (Wladimir J. van der Laan)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 9ff578fbaec5694d63256d65cbe7e9d0ba99ac50803c8192f9b5e90a614dc89778bdb17b03f5f7725397f820e085e7d704deb1d6a58b18cd4e4fc91ea413b6ae
6590395f60 tests: Remove FUZZERS_MISSING_CORPORA (practicalswift)
815c7a6793 tests: Add basic fuzzing harness for CNetAddr/CService/CSubNet related functions (netaddress.h) (practicalswift)
Pull request description:
Add basic fuzzing harness for `CNetAddr`/`CService`/`CSubNet` related functions (`netaddress.h`).
To test this PR:
```
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/netaddress
…
```
Top commit has no ACKs.
Tree-SHA512: 69dc0e391d56d5e9cdb818ac0ac4b69445d0195f714442a06cf662998e38b6e0bbaa635dce78df37ba797feed633e94abba4764b946c1716d392756e7809112d
Make sure that there are no errors set for an input after it is signed.
This is useful for when there are multiple ScriptPubKeyMans. Some may
fail to sign, but one may be able to sign, and after it does, we don't
want there to be any more errors there.
Not all ScriptPubKeyMans will be able to provide private keys,
but pubkeys and scripts should be. So only provide public-only
SigningProviders, i.e. ones that can help with Solving.
Instead of fetching a SigningProvider from ScriptPubKeyMan in order
to fill and sign the keys and scripts for a PSBT, just pass that
PSBT to a new FillPSBT function that does all that for us.
bf06641819 tests: Reset FUZZERS_MISSING_CORPORA to enable regression fuzzing for more harnesses (practicalswift)
Pull request description:
Update `FUZZERS_MISSING_CORPORA` to enable regression fuzzing for all harnesses in `master`.
Note that this PR is expected to fail in Travis until https://github.com/bitcoin-core/qa-assets/pull/4 is merged :)
Top commit has no ACKs.
Tree-SHA512: 6b4d23726ca99c7ab9454087f0354e35d307e7e16b90741cdf7838265a07a631cf54d9533b4f1bf9a0fd900c5ed909756a35cab738284bf39b09ab1c2ed54368
a652ba6293 rpc/wallet: initialize nFeeRequired to avoid using garbage value on failure (Karl-Johan Alm)
Pull request description:
Initialize the `nFeeRequired` variable to avoid using an uninitialized value for errors happening before it is set to 0.
Note: this originally fixed `nFeeRet` in `wallet.cpp`.
ACKs for top commit:
promag:
ACK a652ba6293.
Sjors:
utACK a652ba6293
practicalswift:
ACK a652ba6293 -- patch looks correct
meshcollider:
utACK a652ba6293
Tree-SHA512: 0d12f1ffd0851ed5ce6d109d2c87f55e8b1d57da297e684feeabb57229200c4078f029c55ca5aa5712bd18e26dda3ce538443dfe68a7a6d504428068f81fded0
0d0bc3b5c1 build: Add locale fuzzer to FUZZERS_MISSING_CORPORA (practicalswift)
Pull request description:
Add `locale` fuzzer to `FUZZERS_MISSING_CORPORA`.
This is a follow-up to #18126 which broke Travis. Sorry about that :)
ACKs for top commit:
fanquake:
ACK 0d0bc3b5c1
Tree-SHA512: c0968dc798839f87c891d1dfccf5541883ac56b51a29f52244e78c221c9c087d2dea0a959612d907d53b29fca1f486b340227b17653227ecbf6ca5ab0e85b0d3
79facb11e9 wallet: use constant CWallets in rpcwallet.cpp (Karl-Johan Alm)
d9b0ebc1da wallet: make ReserveDestination pwallet ivar const (Karl-Johan Alm)
57c569e4d9 wallet: make BackupWallet() const (Karl-Johan Alm)
df3a818d2a wallet: make getters const (Karl-Johan Alm)
227b9dd2d6 wallet/spkm: make GetOldestKeyPoolTime() const (Karl-Johan Alm)
22d329ad0e wallet: use constant CWallets in rpcdump.cpp (Karl-Johan Alm)
7b3587b29d wallet/db: make IsDummy() const (Karl-Johan Alm)
d366795d18 wallet/db: make Backup() const (Karl-Johan Alm)
8cd0b86340 wallet: make CanGetAddresses() const (Karl-Johan Alm)
037fa770eb wallet: make KeypoolCountExternalKeys() const (Karl-Johan Alm)
ddc93557ad wallet: make CanGenerateKeys() const (Karl-Johan Alm)
dc2d0650fd make BlockUntilSyncedToCurrentChain() const (Karl-Johan Alm)
Pull request description:
A lot of places refer to `CWallet*`'s as `CWallet * const`, which translates to *"an immutable pointer to a mutable `CWallet` instance"*; this is
1. often not what the author meant, especially as a lot of these places do not at all modify the wallet object, and
2. confusing, as it tends to suggest that this is a proper way to refer to a constant `CWallet` instance.
This PR changes references to wallets to `const CWallet* const` whenever immutability is expected. This should result in no behavioral changes at all, and improved compile-time error checking.
Note from irc:
> <sipa> sounds good to me; this is the sort of change that as long as it compiles, the behavior shouldn't change
> <sipa> though in general it may lead to introducing automatic copying of objects sometimes (e.g. trying to std::move a const object will work, but generally result in a copy rather than an efficient move)
> <sipa> CWallet objects aren't copied or moved though
ACKs for top commit:
laanwj:
ACK 79facb11e9
Empact:
ACK 79facb11e9
promag:
ACK 79facb11e9.
fjahr:
ACK 79facb11e9
Tree-SHA512: 80a80c1a52f0f788d0ccb268b53bc0f46c796643a3c5a22b55bbbde4ffa6c7e347784e5e53b1e488a3b4e14399e31d5be9417ad5b6319c74a462609e9b1a98e8
70a6b529f3 lint-cppcheck: Remove -DHAVE_WORKING_BOOST_SLEEP_FOR (Anthony Towns)
294937b39d scheduler_tests: re-enable mockforward test (Anthony Towns)
cea19f6859 Drop unused reverselock.h (Anthony Towns)
d0ebd93270 scheduler: switch from boost to std (Anthony Towns)
b9c4260127 sync.h: add REVERSE_LOCK (Anthony Towns)
306f71b4eb scheduler: don't rely on boost interrupt on shutdown (Anthony Towns)
Pull request description:
Replacing boost functionality with C++11 stuff.
Motivated by #18227, but should stand alone. Changing from `boost::condition_var` to `std::condition_var` means `threadGroup.interrupt_all` isn't enough to interrupt `serviceQueue` anymore, so that means calling `stop()` before `join_all()` is needed. And the existing reverselock.h code doesn't work with sync.h's DebugLock code (because the reversed lock won't be removed from `g_lockstack` which then leads to incorrect potential deadlock warnings), so I've replaced that with a dedicated class and macro that's aware of our debug lock behaviour.
Fixes#16027, Fixes#14200, Fixes#18227
ACKs for top commit:
laanwj:
ACK 70a6b529f3
Tree-SHA512: d1da13adeabcf9186d114e2dad9a4fdbe2e440f7afbccde0c13dfbaf464efcd850b69d3371c5bf8b179d7ceb9d81f4af3cc22960b90834e41eaaf6d52ef7d331
259e290db8 tests: Add fuzzing harness for locale independence testing (practicalswift)
Pull request description:
Context: [C and C++ locale assumptions in bitcoind and bitcoin-qt](https://github.com/bitcoin/bitcoin/pull/18124)
Add fuzzing harness for locale independence testing of functions in `strencodings.h` and `tinyformat.h`.
Test this PR using:
```
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/locale
…
```
The tested functions (`ParseInt32(…)`, `ParseInt64(…)`, `atoi(const std::string&)`, `atoi64(const std::string& str)`, `i64tostr(const char*)`, `itostr(…)`, `strprintf(…)`) all call locale dependent functions (such as `strtol(…)`, `strtoll(…)`, `atoi(const char*)`, etc.) but are assumed to do so in a way that the tested functions return same results regardless of the chosen C locale (`setlocale`).
This fuzzer aims to test that those assumptions hold up also in practice now and over time.
Top commit has no ACKs.
Tree-SHA512: d108d2f85aa6f482839dafbc7579465ffd4bacf7bc52835ad0fbaa1c71aed9b3870c83447b3d453a03b9ce307e76a3cfdd350a0c77024ab094c93c7d62c8a527
