7b8e15728d rpc: Fix rpcRunLater race in walletpassphrase (João Barbosa)
Pull request description:
Release locks before calling `rpcRunLater`.
Quick explanation: `rpcRunLater` leads to `event_free` which calls `event_del` which can wait for the event callback to finish if it's already running and that callback will try to lock wallet mutex - which is already locked in http thread.
Fixes#14995 , fixes#18482. Best reviewed with whitespace changes hidden.
ACKs for top commit:
MarcoFalke:
ACK 7b8e15728d, only tested that this avoids the node freezing. Did not look at how libevent works or how the deadlock happens or if this breaks other stuff. 📞
ryanofsky:
Code review ACK 7b8e15728d. Just updated comment since last review
Tree-SHA512: 17874a2fa7b0e164fb0d7ee4cb7d59650275b8c03476fb291d60af8b758495457660d3912623fb26259fefe84aeba21c0a9e0c6467982ba511f19344ed5413ab
b224b4e7bd test: wallet_bumpfee assertion fixup (Jon Atack)
Pull request description:
Follow-up to #18516 to fix up an assertion as per suggested change in https://github.com/bitcoin/bitcoin/pull/18516#discussion_r404191587.
ACKs for top commit:
jnewbery:
ACK b224b4e7bd
Tree-SHA512: 4973bba73a67c1ffaf460921b3d454e9d66a40a67f73b7df742e24a0e389adba3946a3958a729391ee6bfa4ef844be759ebf71d14d788434c248e48a2bbe5bde
cd3b1569d9 Correctly compute redeemScript from witnessScript for signrawtransaction (Andrew Chow)
Pull request description:
`ParsePrevouts` uses `GetScriptForWitness` on the given witnessScript to find the corresponding redeemScript. This is incorrect when the witnessScript is either a P2PK or P2PKH script as it returns the corresponding P2WPK script instead of turning the witnessScript into a P2WSH script. Instead this should make the script a `WitnessV0ScriptHash` destination and get the script for that.
Test cases are also added. These will fail on master with a `redeemScript does not correspond to witnessScript`
Reported on [Bitcointalk](https://bitcointalk.org/index.php?topic=5236818.0)
ACKs for top commit:
MarcoFalke:
weak ACK cd3b1569d9, only checked that the test fails without the code change 🚰
instagibbs:
utACK cd3b1569d9
Tree-SHA512: afac671dbb52ce88bfb4a9ca3dd6065427ad52c9778d0549ad40e9286778f308adad24fb3b3c3089545d7f88c57c53d41224fd7a4bb207550eff2fe06600118f
25e03ba1ff test: relax bumpfee dust_to_fee txsize an extra vbyte (Jon Atack)
Pull request description:
Hopefully closes#18511 by allowing the transaction size to be 140-141 vbytes rather than strictly 141, and bumps with a slightly larger fee to ensure dust in the 140 vbyte case.
ACKs for top commit:
jnewbery:
utACK 25e03ba1ff
Tree-SHA512: 76a04e1ce090e48befe048ed6d412222d7f8bc951ff822850833061a0606b1bebc5289f7249737d3fb9aa26eb857f99543981037cea6babe3e578e2cfe8afcdb
d6815a2313 refactor: drop boost::signals2 in validationinterface (Russell Yanofsky)
Pull request description:
Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions.
Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: https://github.com/bitcoin/bitcoin/issues/18517, https://github.com/bitcoin/bitcoin/pull/18471
ACKs for top commit:
MarcoFalke:
ACK d6815a2313
laanwj:
ACK d6815a2313
hebasto:
re-ACK d6815a2313
promag:
ACK d6815a2313.
Tree-SHA512: 4fc0f14a8446e8616cc142af6c3d36815f3254525d30348ba8e4d4bc74c249a5a8c9bc119bdd1be7ebd7abe0b784bc0c5551a3e156a766890cb2fdd891a95919
c0c43ae147 test: skip backwards compat tests if not compiled with wallet (fanquake)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: d9975a1490e69134408b6b724cea26a6c1397d43f59850283b9e338ae38e00fefbcd868fb141e0a4bb55f02076690a99331f29cfa2d0fa66c165032b24a94081
0eeb0468e7 net: Hardcoded seeds update for 0.20 (Wladimir J. van der Laan)
Pull request description:
Update hardcoded seeds from http://bitcoin.sipa.be/seeds.txt.gz,
according to release process.
Output from makeseeds.py:
```
IPv4 IPv6 Onion Pass
1364173 244127 2454 Initial
1364173 244127 2454 Skip entries with invalid address
1129552 213117 2345 After removing duplicates
1129548 213117 2345 Skip entries from suspicious hosts
338216 191944 2249 Enforce minimal number of blocks
336851 188993 2189 Require service bit 1
6998 1520 150 Require minimum uptime
5682 1290 89 Require a known and recent user agent
5622 1279 89 Filter out hosts with multiple bitcoin ports
512 146 89 Look up ASNs and limit results per ASN and per net
```
Top commit has no ACKs.
Tree-SHA512: ce1c2cda18dd5bd22586a5283a0877f3bd890437cc29dc1d85452ba4a4d28032f591c8b37f3329e8e649556cf83750b6949a068fad76d1773853d93014609da0
9e071b0089 test: remove rapidcheck integration and tests (fanquake)
Pull request description:
Whilst the property tests are interesting, ultimately [rapidcheck](https://github.com/emil-e/rapidcheck) integration in this repository has not gained much traction. We have a limited number of tests, and they are rarely (if ever) run. Have discussed this with Chris Stewart.
ACKs for top commit:
practicalswift:
ACK 9e071b0089
Tree-SHA512: d0c12af3163382eee8413da420c63e39265a7b700709a05d518445832d45e049aed9508e32524db5228fe3ac114609a00b7bb890be047c07032e44a5ef4611e9
Remove inconsistency between functional and unit test environments and make it
possible to substitute bitcoin-qt and bitcoin-node in place of bitcoind in
python tests, or to link bitcoind against shared libraries.
autotools and automake changes to support multiprocess execution.
This adds a new --enable-multiprocess flag, and build configuration code to
detect libraries needed for multiprocess support. The --enable-multiprocess
flag builds new bitcoin-node and bitcoin-gui executables, which are updated in
https://github.com/bitcoin/bitcoin/pull/10102 to communicate across processes.
But for now they are functionally equivalent to existing bitcoind and
bitcoin-qt executables.
0ed2d8e07d test: add BIP37 remote crash bug [CVE-2013-5700] test to p2p_filter.py (Sebastian Falbesoner)
Pull request description:
Integrates the missing message type `filteradd` to the test framework and checks that the BIP37 implementation is not vulnerable to the "remote crash bug" [CVE-2013-5700](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-5700) anymore. Prior to v.0.8.4, it was possible to trigger a division-by-zero error on the following line in the function `CBloomFilter::Hash()`:
f0d6487e29/src/bloom.cpp (L45)
By setting a zero-length filter via `filterload`, `vData.size()` is 0, so the modulo operation above, called on any .insert() or .contains() operation then crashed the node. The test uses the approach of just sending an arbitrary `filteradd` message after, which calls `CBloomFilter::insert()` (and in turn `CBloomFilter::Hash()`) on the node. The vulnerability was fixed by commit 37c6389c5a (an intentional covert fix, [according to gmaxwell](https://github.com/bitcoin/bitcoin/issues/18483#issuecomment-608224095)), which introduced flags `isEmpty`/`isFull` that wouldn't call the `Hash()` member function if `isFull` is true (set to true by default constructor).
To validate that the test fails if the implementation is vulnerable, one can simply set the flags to false in the member function `UpdateEmptyFull()` (that is called after a filter received via `filterload` is constructed), which activates the vulnerable code path calling `Hash` in any case on adding or testing for data in the filter:
```diff
diff --git a/src/bloom.cpp b/src/bloom.cpp
index bd6069b..ef294a3 100644
--- a/src/bloom.cpp
+++ b/src/bloom.cpp
@@ -199,8 +199,8 @@ void CBloomFilter::UpdateEmptyFull()
full &= vData[i] == 0xff;
empty &= vData[i] == 0;
}
- isFull = full;
- isEmpty = empty;
+ isFull = false;
+ isEmpty = false;
}
```
Resulting in:
```
$ ./p2p_filter.py
[...]
2020-04-03T14:38:59.593000Z TestFramework (INFO): Check that division-by-zero remote crash bug [CVE-2013-5700] is fixed
2020-04-03T14:38:59.695000Z TestFramework (ERROR): Assertion failed
[...]
[... some exceptions following ...]
```
ACKs for top commit:
naumenkogs:
utACK 0ed2d8e07d
Tree-SHA512: 02d0253d13eab70c4bd007b0750c56a5a92d05d419d53033523eeb3ed80318bc95196ab90f7745ea3ac9ebae7caee3adbf2a055a40a4124e0915226e49018fe8
691e2a7af7 build: create test_fuzz library from src/test/fuzz/fuzz.cpp (Harris)
Pull request description:
This PR creates a static library **libtest_fuzz.a** to speed up the compilation of fuzz tests. It is functionally similar to https://github.com/bitcoin/bitcoin/pull/17542
Fixes https://github.com/bitcoin/bitcoin/issues/18527
ACKs for top commit:
MarcoFalke:
ACK 691e2a7af7🦁
Tree-SHA512: 39d7d2731ca4370db518dbb969eb17ddbf9c030c3fe0dec0d04ff6578f24a128563fe5aced78300c92ce296623a7079fea5aea70619819a20c56fb34191f00ef
4670006762 test: remove redundant sync_with_ping after add_p2p_connection (Jon Atack)
Pull request description:
Now that #18247 is merged, these calls are redundant.
ACKs for top commit:
vasild:
utACK 4670006
Tree-SHA512: bdbfe8bcf9dbdde0a8115e3a62bfe359910798d7a3010d920ffca07049cb5f97bf8fb9b6f70079b0607105192b61a6d665774e59a2b678597b47ad6237595ad5
3dc8c012f0 test: remaining replacements of (send_message+sync_with_ping) with send_and_ping (Sebastian Falbesoner)
Pull request description:
This is a tiny follow-up PR to #18494, substituting the remaining occurences of `send_message(...)`/`sync_with_ping(...)` pairs with `send_and_ping(...)`, as suggested in the comment https://github.com/bitcoin/bitcoin/pull/18494#pullrequestreview-386418913. Thanks to jonatack and [MarcoFalke](https://github.com/bitcoin/bitcoin/pull/18494#issuecomment-608496342) for giving me the hint to do this follow-up.
ACKs for top commit:
practicalswift:
ACK 3dc8c012f0
Tree-SHA512: 44d64332933c23a7f59c0415e008ce1b2b2e07177f81cb9473b7c71558188f1c698e8973de5cc940280e4697f9553af852d9a42841304f82469673d1c8162852
facc332dc5 fuzz: Avoid running over all inputs after merging them (MarcoFalke)
Pull request description:
This cuts the time it takes to merge inputs by half
ACKs for top commit:
practicalswift:
ACK facc332dc5
Tree-SHA512: bb22992c463dd985d3b1e9b8908c591d0c8e620c38eba0a932d880f87133bfe4ca2036b166c4f79b92ddf7940f56c044e9cb8cc50309c74204df122b369c167d
fab32557f2 rpc: Make rpc documentation not depend on rpc args (MarcoFalke)
Pull request description:
This is required to host the documentation on a static resource (like a website or pdf)
ACKs for top commit:
emilengler:
utACK fab32557f2
promag:
ACK fab32557f2.
Tree-SHA512: 3ca2691c7fbd5f17c75df2887753da152f66521dcb7dee4c29af6339fdea011cecdd51f825b96bde9c6aaf82f4d915cbd5aacb52e4eae3898d9dbc216f627171
f32ab443a9 Bugfix: RPC: JSON null is not "None" (Luke Dashjr)
26dcf39581 Bugfix: RPC: Don't use a continuation elipsis after an elision elipsis (Luke Dashjr)
eca65caadc Bugfix: RPC: Add missing commas and correct indentation of explicit ELISION (Luke Dashjr)
Pull request description:
1. listsinceblock had a double ellipsis (elision + continuation); this looks ugly, just one is needed.
2. Elision ellipsis wasn't getting a comma, so was truncated to `".."` by comma-removal code.
3. Elision ellipsis was indented incorrectly (as if it was a subitem).
4. Similarly, type "none" would get truncated to `"Non"`, when it should really be `"null"` anyway.
ACKs for top commit:
MarcoFalke:
ACK f32ab443a9🐰
Tree-SHA512: 34e1c72673790ed11cdee838d64ea5e0ac498de19258df99d54b5322e003060123c65ad27ac2fd4729a1dfe52066a0629602a132b1ef85d4154affd99a065a3f
Update hardcoded seeds from seeds_emzy.txt seeds_lukejr.txt
seeds_sipa.txt seeds_sjors.txt, according to release process.
Output from makeseeds.py:
```
IPv4 IPv6 Onion Pass
1364173 244127 2454 Initial
1364173 244127 2454 Skip entries with invalid address
1129552 213117 2345 After removing duplicates
1129548 213117 2345 Skip entries from suspicious hosts
338216 191944 2249 Enforce minimal number of blocks
336851 188993 2189 Require service bit 1
6998 1520 150 Require minimum uptime
5682 1290 89 Require a known and recent user agent
5622 1279 89 Filter out hosts with multiple bitcoin ports
512 146 89 Look up ASNs and limit results per ASN and per net
```
fad2f68353 init: Replace URL_WEBSITE with PACKAGE_URL (MarcoFalke)
Pull request description:
This is needed for rebranding efforts such as #18489
ACKs for top commit:
hebasto:
ACK fad2f68353, tested on Linux Mint 19.3:
fanquake:
ACK fad2f68353 - clicked a link.
Tree-SHA512: c26e18cd328d3dd3fd7e25413e1bab780026687a148f126b8673e5f6cc13249f6c16689e45eba9da1545915c6001f96cd33f4e656c08cda3eae1c3fd88da23ea
