334de75885 scripted-diff: Remove Reference Links (Robert)
Pull request description:
Removed all reference links.
Found this issue from #19582.
The decision to remove links instead of update them was made in #19584
The author of that PR was slow to resolve his commit to use scripted diff so I made this PR instead.
ACKs for top commit:
laanwj:
ACK 334de75885
MarcoFalke:
ACK 334de75885
Tree-SHA512: a337116379912b27974867bd86ec7799a1d41d67b51771885467fbe1be003b415cb37ce8e521568bf3eae190ab2f6af0d6e29fd3ea25f2689b8fb31def8fec96
05e2740d92 doc: Add historical release notes for 0.20.1 (Wladimir J. van der Laan)
Pull request description:
ACKs for top commit:
MarcoFalke:
ACK 05e2740d92
Tree-SHA512: 8f33403c51bcbaf97410642878bbc5ee4cbced2e079a61362a69c7eb9d682cf8d3d6ce16fe9aa99c2c89f21a57910032219312403b16d322d403b029f39c62ff
10b7a6d532 refactor: make txmempool interface use GenTxid (Pieter Wuille)
5c124e1740 refactor: make FindTxForGetData use GenTxid (Pieter Wuille)
a2bfac8935 refactor: use GenTxid in tx request functions (Pieter Wuille)
e65d115b72 test: request parents of orphan from wtxid relay peer (Anthony Towns)
900d7f6c07 p2p: enable fetching of orphans from wtxid peers (Pieter Wuille)
9efd86a908 refactor: add GenTxid (=txid or wtxid) type and use it for tx request logic (Pieter Wuille)
d362f19355 doc: list support for BIP 339 in doc/bips.md (Pieter Wuille)
Pull request description:
This is based on https://github.com/bitcoin/bitcoin/pull/18044#discussion_r450687076.
A new type `GenTxid` is added to protocol.h, which represents a tagged txid-or-wtxid. The tx request logic is updated to use these instead of uint256s, permitting per-announcement distinguishing of txid/wtxid (instead of assuming that everything we want to request from a wtxid peer is wtx). Then the restriction of orphan-parent requesting to non-wtxid peers is lifted.
Also document BIP339 in doc/bips.md.
ACKs for top commit:
jnewbery:
Code review ACK 10b7a6d532
jonatack:
ACK 10b7a6d532
ajtowns:
ACK 10b7a6d532 -- code review. Using gtxid to replace the is_txid_or_wtxid flag for the mempool functions is nice.
naumenkogs:
utACK 10b7a6d
Tree-SHA512: d518d13ffd71f8d2b3c175dc905362a7259689e6022a97a0b4f14f1f9fdd87475cf5af70cb12338d1e5d31b52c12e4faaea436114056a2ae9669cb506240758b
ae4958be95 rpc: RPCResult Type of MempoolEntryDescription should be OBJ. If multiple entries are possible, wrapping Type should be OBJ_DYN. fixes#19579 (Chris L)
Pull request description:
If multiple entries are possible, wrapping Type should be OBJ_DYN.
fixes#19579
Top commit has no ACKs.
Tree-SHA512: 59cf9f6e9729a69a867e924d8306e0cd6b70a3d702fc5a4111345874bb1224ee51ac3f70cea61b25cfe6bde7f65cb02528d52acc20dda4eda692eddf34f217e8
decimal.InvalidOperation is a special case of a float parsing error, which
presumably should be handled in the same way as a general parsing error,
rather than blow up.
Alternatives include: logging the error, or re-raising with more information.
Example log output:
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 603, in sync_all
self.sync_blocks(nodes)
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 568, in sync_blocks
best_hash = [x.getbestblockhash() for x in rpc_connections]
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 568, in <listcomp>
best_hash = [x.getbestblockhash() for x in rpc_connections]
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 571, in __call__
return self.cli.send_cli(self.command, *args, **kwargs)
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 639, in send_cli
return json.loads(cli_stdout, parse_float=decimal.Decimal)
File "/usr/lib64/python3.6/json/__init__.py", line 367, in loads
return cls(**kw).decode(s)
File "/usr/lib64/python3.6/json/decoder.py", line 339, in decode
obj, end = self.raw_decode(s, idx=_w(s, 0).end())
File "/usr/lib64/python3.6/json/decoder.py", line 355, in raw_decode
obj, end = self.scan_once(s, idx)
decimal.InvalidOperation: [<class 'decimal.InvalidOperation'>]
e60ef21b8a doc: Clang 8 or later is required with FORCE_USE_SYSTEM_CLANG (fanquake)
Pull request description:
The usage of pragmas within the macOS SDK requires LLVM Clang 8. This is
the same version as our prebuilt Clang, however the minimum is worth noting
here as they may diverge and/or expert users might expect they could use an
earlier version.
If you compile depends using `FORCE_USE_SYSTEM_CLANG=1` and Clang 7 you'll see output like:
```bash
In file included from kernel/qcore_mac_objc.mm:44:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSText.h:9:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSView.h:19:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSResponder.h:10:
/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSEvent.h:19:1: error:
expected 'push' or 'pop' after '#pragma clang attribute'
/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/os/availability.h:104:273: note: expanded from macro
'API_UNAVAILABLE_BEGIN'
...__API_UNAVAILABLE_BEGIN5, __API_UNAVAILABLE_BEGIN4, __API_UNAVAILABLE_BEGIN3, __API_UNAVAILABLE_BEGIN2, __API_UNAVAILABLE_BEGIN1, 0)(__VA_A...
^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
```
I've got a [godbolt here](https://godbolt.org/z/j6r987) that contains a demo of the issue (based off the macOS SDK). It will compile with Clang 8 but not with Clang 7.
ACKs for top commit:
laanwj:
ACK e60ef21b8a
Tree-SHA512: affc082a4b28d9cd102a844e10f63f55f0f20a9f840e5d8037a41470ad1ea83edc3075ce262b1d618c59c9b75445e52ddd9ba9f84c5ac3da9ce20681473c47b1
This is a more narrowly-construed wait which eliminates the possibility of the
wait being triggered by other messages.
Co-authored-by: Billy Garrison <billygarrison.btc@gmail.com>
This is in preparation for exposing a ::data member function.
-BEGIN VERIFY SCRIPT-
sed -i "s/\([^.]\|other.\)data/\1m_data/g" src/uint256.h src/uint256.cpp
-END VERIFY SCRIPT-
284a969cc0 Linter to check commit message formatting (Amir Ghorbanian)
Pull request description:
Write linter to check that commit messages have a new line before the body or no body at all. fixes issue #19091.
ACKs for top commit:
troygiorshev:
ACK 284a969cc0 Reviewed, manually tested. Works great!
fjahr:
tested ACK 284a969cc0
adamjonas:
utACK 284a969cc0
Tree-SHA512: fa278f090780b54e4fa6e2967a62b4c1a4da55d112ec1ad6dd7e1181ac490c5c1af0165524b5781b463fdd6d0f79fd3d95b5160184e6eca432ccff1189f77390
fae8c28dae Pass mempool pointer to GetCoinsCacheSizeState (MarcoFalke)
fac674db20 Pass mempool pointer to UnloadBlockIndex (MarcoFalke)
faec851b6e test: Simplify cs_main locks (MarcoFalke)
Pull request description:
Split out from #19556
Instead of relying on the implicit mempool global, pass a mempool pointer (which can be `0`). This helps with testing, code clarity and unlocks the features described in #19556.
ACKs for top commit:
jnewbery:
code review ACK fae8c28dae
fjahr:
Code review ACK fae8c28dae
darosior:
Tested ACK fae8c28dae
jamesob:
ACK fae8c28dae ([`jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to`](https://github.com/jamesob/bitcoin/tree/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to))
Tree-SHA512: fa687518c8cda4a095bdbdfe56e01fae2fb16c13d51efbb1312cd6dc007611fc47f53f475602e4a843e3973c9410e6af5a81d6847bd2399f8262ca7205975728
8ed9002cd1 refactor: use local argsmanager in CRegTestParams (Ivan Metlushko)
9b20f66828 scripted-diff: Replace gArgs with local argsman (Ivan Metlushko)
a316e9ce26 refactor: add unused ArgsManager to replace gArgs (Ivan Metlushko)
Pull request description:
Rationale: reduce use of gArgs to decouple code and simplify future maintenance and easier unit testing.
This PR is continuation of work started in #18926 and #18662
It covers only places that register args in ArgsManager with `AddArgs()` or `AddHiddenArgs()`.
Closes#19511
ACKs for top commit:
MarcoFalke:
ACK 8ed9002cd1👛
Tree-SHA512: 7e6ba8e8357a48833c71e9c3942a769acb3d93bdcc6748a8ef2b7c4461a2499419b60896abf1d8b6bf8e88ee2590284cdd5da64220243ac22375300bcb8fe3e8
0fcff547d5 walletdb: Ensure that having no database handle is a failure (Andrew Chow)
da039d2a91 Remove BDB dummy databases (Andrew Chow)
0103d6434e Introduce DummyDatabase and use it in the tests (Andrew Chow)
Pull request description:
In the unit tests, we use a dummy `WalletDatabase` which does nothing and always returns true. This is currently implemented by creating a `BerkeleyDatabase` in dummy mode. This PR instead adds a `DummyDatabase` class which does nothing and never fails for use in the tests. `CreateDummyWalletDatabase` is changed to return this `DummyDatabase` and `BerkeleyDatabase` is cleaned up to remove all of the checks for `IsDummy`.
Based on `WalletDatabase` abstract class introduced in #19334
ACKs for top commit:
instagibbs:
utACK 0fcff547d5
MarcoFalke:
crACK 0fcff547d5🚈
Tree-SHA512: 05fbf32e078753e9a55a05f4c080b6d365b909a2a3a8e571b7e64b59ebbe53da49394f70419cc793192ade79f312f5e0422ca7c261ba81bae5912671c5ff6402
c251d710a4 p2p, refactoring: use CInv helpers in net_processing.cpp (Jon Atack)
4254cd9f8f p2p: add CInv transaction message helper methods (Jon Atack)
Pull request description:
Following the merge of wtxid relay in #18044, this is the first of three refactoring PRs (this one, #19610, and #19611) with no change in behavior, tightly scoped to ease review, to simplify the net processing code and improve encapsulation:
- add `CInv` transaction message helper methods, defined in the class
- use the new helpers in `net_processing.cpp` to simplify the code and improve encapsulation
Test coverage is provided by the functional p2p tests, notably (from seeing which tests failed when breaking things to test coverage) `p2p_segwit`, `p2p_tx_download`, `p2p_feefilter`, and `p2p_permissions`.
ACKs for top commit:
fjahr:
Code review ACK c251d710a4
laanwj:
Code review ACK c251d710a4
vasild:
ACK c251d71
theStack:
Code-Review ACK c251d710a4
hebasto:
ACK c251d710a4, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: ead034b3c9e438909b4c5010c570d7930e69063c114290b051b7cebfd9bd5b19f573218bebe8a521256d32e830797f997adad3d85b4539c64ac5762b698e656d
78c312c983 Replace current benchmarking framework with nanobench (Martin Ankerl)
Pull request description:
Replace current benchmarking framework with nanobench
This replaces the current benchmarking framework with nanobench [1], an
MIT licensed single-header benchmarking library, of which I am the
autor. This has in my opinion several advantages, especially on Linux:
* fast: Running all benchmarks takes ~6 seconds instead of 4m13s on
an Intel i7-8700 CPU @ 3.20GHz.
* accurate: I ran e.g. the benchmark for SipHash_32b 10 times and
calculate standard deviation / mean = coefficient of variation:
* 0.57% CV for old benchmarking framework
* 0.20% CV for nanobench
So the benchmark results with nanobench seem to vary less than with
the old framework.
* It automatically determines runtime based on clock precision, no need
to specify number of evaluations.
* measure instructions, cycles, branches, instructions per cycle,
branch misses (only Linux, when performance counters are available)
* output in markdown table format.
* Warn about unstable environment (frequency scaling, turbo, ...)
* For better profiling, it is possible to set the environment variable
NANOBENCH_ENDLESS to force endless running of a particular benchmark
without the need to recompile. This makes it to e.g. run "perf top"
and look at hotspots.
Here is an example copy & pasted from the terminal output:
| ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 2.52 | 396,529,415.94 | 0.6% | 25.42 | 8.02 | 3.169 | 0.06 | 0.0% | 0.03 | `bench/crypto_hash.cpp RIPEMD160`
| 1.87 | 535,161,444.83 | 0.3% | 21.36 | 5.95 | 3.589 | 0.06 | 0.0% | 0.02 | `bench/crypto_hash.cpp SHA1`
| 3.22 | 310,344,174.79 | 1.1% | 36.80 | 10.22 | 3.601 | 0.09 | 0.0% | 0.04 | `bench/crypto_hash.cpp SHA256`
| 2.01 | 496,375,796.23 | 0.0% | 18.72 | 6.43 | 2.911 | 0.01 | 1.0% | 0.00 | `bench/crypto_hash.cpp SHA256D64_1024`
| 7.23 | 138,263,519.35 | 0.1% | 82.66 | 23.11 | 3.577 | 1.63 | 0.1% | 0.00 | `bench/crypto_hash.cpp SHA256_32b`
| 3.04 | 328,780,166.40 | 0.3% | 35.82 | 9.69 | 3.696 | 0.03 | 0.0% | 0.03 | `bench/crypto_hash.cpp SHA512`
[1] https://github.com/martinus/nanobench
ACKs for top commit:
laanwj:
ACK 78c312c983
Tree-SHA512: 9e18770b18b6f95a7d0105a4a5497d31cf4eb5efe6574f4482f6f1b4c88d7e0946b9a4a1e9e8e6ecbf41a3f2d7571240677dcb45af29a6f0584e89b25f32e49e
82dee87933 test: test decodepsbt fee calculation (count input value only once per UTXO) (Sebastian Falbesoner)
Pull request description:
Fixes#19523, adding a simple test to `rpc_psbt.py` that checks that the decodepsbt fee matches the one given by the wallet (`walletcreatefundedpsbt`). This is in particular important for PSBTs with segwit inputs that have both a witness- and a non-witness-UTXO type set.
Example test run after reverting commit 75122780e2 ("Increment input value sum only once per UTXO in decodepsbt"):
```
$ test/functional/rpc_psbt.py
2020-07-26T11:31:44.862000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test__sutcd4y
20.00007580
2020-07-26T11:31:47.073000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/honeybadger/buidl/bitcoin_thestack/test/functional/test_framework/test_framework.py", line 118, in main
self.run_test()
File "test/functional/rpc_psbt.py", line 166, in run_test
assert_equal(decoded['fee'], created_psbt['fee'])
File "/home/honeybadger/buidl/bitcoin_thestack/test/functional/test_framework/util.py", line 49, in assert_equal
raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(20.00007580 == 0.00007580)
2020-07-26T11:31:47.125000Z TestFramework (INFO): Stopping nodes
......
```
ACKs for top commit:
achow101:
ACK 82dee87933
Tree-SHA512: 296b8a701f851d482ef6200c6cbf0cf0257a79a828ac6dbc39b05d8c2d839c6fdb9d3f5a084015295cfa3eac7c11faa2f2d52e619c11627b04c75150eead8330
2c6a02e024 Clean message_count and last_message (Troy Giorshev)
Pull request description:
From #19580
This PR changes comments to clarify the intended usage of `message_count` and `last_message`. Additionally it changes the only usage of `message_count` to use `last_message` instead, bringing the code into alignment with the intended usage.
Note: Now `message_count` is completely unused. However, it is ready to be used (i.e. the supporting code works) and likely will be used in some test in the future.
ACKs for top commit:
jnewbery:
utACK 2c6a02e024
Tree-SHA512: 07c7684c9586de4f845e10d7aac36c1aab9fb56b409949c1c70d5ca705bc3971ca7d5943245a0472def4efd7b4e1c5dad2f713db5ead8fca08404daf4891e98b
Previously having no database handle could still be considered a success
when BerkeleyDatabase and BerkeleyBatch were used for dummy database
things. With dedicated DummyDatabase and DummyBatch classes now, these
should fail.
