faaa4f2b6af4ce249fc4809a030240afa45b1a33 refactor: Remove nMyStartingHeight from CNode/Connman (MarcoFalke)
Pull request description:
CNode and CConnman keep track of the active chain height when CNodes have been created, but apart from serializing the int once (when sending a version message), it is unused. So it can simply be removed in favor of a single int in PeerMan that can do the same.
ACKs for top commit:
jnewbery:
utACK faaa4f2b6af4ce249fc4809a030240afa45b1a33
theStack:
re-ACK faaa4f2b6af4ce249fc4809a030240afa45b1a33 ⚓
Tree-SHA512: 713919e49e0f799c4a5a2e5ab07d1ca1c92d4049829680c4f436f9b3b545c548ddbee78ff80611de50cc91a116a416bcc4602e80ee64953597dc04ca15f9806d
86c495223f048e5ca2cf0d8730af7db3b76f7aba net: add CNode::IsInboundOnion() public getter and unit tests (Jon Atack)
6609eb8cb50fe92c7317b5db9e72d4333b3aab1b net: assert CNode::m_inbound_onion is inbound in ctor (Jon Atack)
993d1ecd191a7d9161082d4026f020cbf00835bb test, fuzz: fix constructing CNode with invalid inbound_onion (Jon Atack)
Pull request description:
The goal of this PR is to be able to depend on `m_inbound_onion` in AttemptToEvictConnection in #20197:
- asserts `CNode::m_inbound_onion` is inbound in the CNode ctor to have a validity check at the class boundary
- fixes a unit test and a fuzz utility that were passing invalid inbound onion values to the CNode ctor
- drops an unneeded check in `CNode::ConnectedThroughNetwork()` for its inbound status
- adds a public getter `IsInboundOnion()` that also allows unit testing it
- adds unit test coverage
ACKs for top commit:
sipa:
utACK 86c495223f048e5ca2cf0d8730af7db3b76f7aba
LarryRuane:
ACK 86c495223f048e5ca2cf0d8730af7db3b76f7aba
vasild:
ACK 86c495223f048e5ca2cf0d8730af7db3b76f7aba
MarcoFalke:
review ACK 86c495223f048e5ca2cf0d8730af7db3b76f7aba 🐍
Tree-SHA512: 21109105bc4e5e03076fadd489204be00eac710c9de0127708ca2d0a10a048ff81f640f589a7429967ac3eb51d35fe24bb2b12e53e7aa3efbc47aaff6396d204
ccc8d5513fb2228eabc25b105f7a0498f8453885 doc: Update license year range to 2021 (Emil Engler)
Pull request description:
See #17801, #15061
The same procedure as every year. Happy new year to all of you :)
Top commit has no ACKs.
Tree-SHA512: 59ca9ba43d3722d5e2ada04fe6c361058a72c84b8437f6331f45e361d7ebf0f3586ccb02dc72af12b9d94f766897f0f041ce3a90efe95eeff22217f9f964ffc6
fa0074e2d82928016a43ca408717154a1c70a4db scripted-diff: Bump copyright headers (MarcoFalke)
Pull request description:
Needs to be done because no one has removed the years yet
ACKs for top commit:
practicalswift:
ACK fa0074e2d82928016a43ca408717154a1c70a4db
Tree-SHA512: 210e92acd7d400b556cf8259c3ec9967797420cfd19f0c2a4fa54cb2b3d32ad9ae27e771269201e7d554c0f4cd73a8b1c1a42c9f65d8685ca4d52e5134b071a3
4ddbcd0d9abe40cd387e63d8c4817e0fe36004dc fuzz: Add coverage for CDataStream consumer (practicalswift)
546a0764f3b701ee07f5a8d168e2a58fed6b46d5 fuzz: Fill various small fuzzing gaps (practicalswift)
Pull request description:
Fill various small fuzzing gaps.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
MarcoFalke:
review ACK 4ddbcd0d9abe40cd387e63d8c4817e0fe36004dc
Tree-SHA512: d20f2cc0172f39948673846d088121782f39b4556df8b38fa14859cfa062c1519d18ee9601d4503ef1ba9613976cc5349c1fc0f0b9601a3d68127ffce1b1854e
454a4088a87eac5878070b26d13d5574da891877 [doc] Add release notes for removed getpeerinfo fields. (Amiti Uttarwar)
b1a936d4ae7dd9030b0720ef05579a90ce2894f1 [rpc] Remove deprecated "whitelisted" field from getpeerinfo (Amiti Uttarwar)
094c3beaa47c909070607e94f2544ed1472ddb17 [rpc] Remove deprecated "banscore" field from getpeerinfo (Amiti Uttarwar)
537053336fbc1b633e7c99286c3e3492eaca1e9d [rpc] Remove deprecated "addnode" field from getpeerinfo (Amiti Uttarwar)
Pull request description:
This PR removes support for 3 fields on the `getpeerinfo` RPC that were deprecated in v0.21- `addnode`, `banscore` & `whitelisted`.
ACKs for top commit:
sipa:
utACK 454a4088a87eac5878070b26d13d5574da891877
jnewbery:
ACK 454a4088a87eac5878070b26d13d5574da891877.
Tree-SHA512: ccc0e90c0763eeb8529cf0c46162dbaca3f7773981b3b52d9925166ea7421aed086795d56b320e16c9340f68862388785f52a9b78314865070917b33180d7cd6
1ef2138c0db3bd4f9332c777fa3fb2770dc1b08c lint: run mypy over contrib/devtools (fanquake)
Pull request description:
wumpus mentioned on IRC that we don't currently run `mypy` over the `contrib/devtools` directory, and that it would likely be worthwhile given #20434. This just adds that dir to the linter, as well as some missing annotations to fix existing errors. Note that now we require Python 3.6 we can make use of variable annotations.
master (patched to check contrib devtools):
```bash
test/lint/lint-python.sh
contrib/devtools/symbol-check.py:154: error: Incompatible types in assignment (expression has type "List[str]", variable has type "str")
contrib/devtools/circular-dependencies.py:35: error: Need type annotation for 'deps' (hint: "deps: Dict[<type>, <type>] = ...")
contrib/devtools/circular-dependencies.py:67: error: Need type annotation for 'closure' (hint: "closure: Dict[<type>, <type>] = ...")
Found 4 errors in 3 files (checked 187 source files)
```
I haven't quite gone as far as to add annotations like
```python
CHECKS: Dict[str, List[Tuple[str, Callable[[Any], bool]]]] = {...
```
to `symbol-check.py`.
ACKs for top commit:
laanwj:
ACK 1ef2138c0db3bd4f9332c777fa3fb2770dc1b08c
Tree-SHA512: a58c2ece588c640289dc1d35dad5b1b8732788272daa0965d6bf44ee8a7f7c8e8585f94d233ac41c84b9ffcfc97841a00fe2c9acba41f58fd164f01de4b6512b
5e0dedb111ed9db03914b9dac36cf081974c2918 build: Define .INTERMEDIATE target once only (Hennadii Stepanov)
Pull request description:
A new warning was introduced in 22437fc72e (#20470):
```
$ ./autogen.sh
...
Makefile.am:335: warning: .INTERMEDIATE was already defined in condition !BUILD_DARWIN, which is included in condition TRUE ...
Makefile.am:139: ... '.INTERMEDIATE' previously defined here
...
```
Fixed in this PR.
ACKs for top commit:
jonatack:
Tested and very light review ACK 5e0dedb111ed9db03914b9dac36cf081974c2918
Tree-SHA512: ecf8de79ba394c36ee84e0b8d3ba78587e0f856259e9731e6bbb38d0baebfd083eb44d7ef6a386dd9e4508dd64fec1c2b9a007e175fbd4d986e845b1c300a649
faccf8b1e1af293dfe9158d732718e7798a2fd89 refactor: Enable -Wswitch for FeeEstimateHorizon (MarcoFalke)
Pull request description:
This enables the `-Wswitch` compiler warning for `FeeEstimateHorizon` by removing the `default` case in `switch` statements.
ACKs for top commit:
practicalswift:
cr ACK faccf8b1e1af293dfe9158d732718e7798a2fd89
jonatack:
ACK faccf8b1e1af293dfe9158d732718e7798a2fd89
hebasto:
ACK faccf8b1e1af293dfe9158d732718e7798a2fd89, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 63a8dff6e8dead149ec2fa8319e7ff41022c9534d423d3086fd8f22be073dc4915f74c7fe9139ee681a8204730cf58c80ef40c93fb33032d586e68b4f78f557d
40fdb2a212d0a0e775114e4766d065e6d234c155 test: Fix Comment Typo in BitcoinTestFramework (Joel Klabo)
Pull request description:
Missing "override" in comment describing use of set_test_params
ACKs for top commit:
michaelfolkson:
ACK 40fdb2a212d0a0e775114e4766d065e6d234c155
Tree-SHA512: bf893a0d5f8dc86a3ec2eaf48cd7c0f0f832f3b3d254b3d99953336db7e294571b1d2c8686030bf8a27cbe67b1a85a54e53ebefb2e57d6d8d6ac864a15dce4e7
667d203687708390bc0f43f2dd3f4ab427b88338 [doc] Add permissions to the getpeerinfo help. (Amiti Uttarwar)
Pull request description:
This field was previously being returned, but missing from the RPCHelpMan. This PR uses the existing `NET_PERMISSIONS_DOC` to inform RPC users about this field.
```
"permissions" : [ (json array) Any special permissions that have been granted to this peer
"str", (string) bloomfilter (allow requesting BIP37 filtered blocks and transactions),
noban (do not ban for misbehavior; implies download),
forcerelay (relay transactions that are already in the mempool; implies relay),
relay (relay even in -blocksonly mode, and unlimited transaction announcements),
mempool (allow requesting BIP35 mempool contents),
download (allow getheaders during IBD, no disconnect after maxuploadtarget limit),
addr (responses to GETADDR avoid hitting the cache and contain random records with the most up-to-date info).
...
],
```
ACKs for top commit:
Sjors:
tACK 667d203687708390bc0f43f2dd3f4ab427b88338
Tree-SHA512: 973631b41d35d6333e3cb06b35277de869110f6ad6498c7e74f00c75202e8de1788a48755c21ac964903e5e6050a5e769a63866211aec9004cd665a727a54a3c
5a9ee0869b0b722ebfcdaabaefba6376522b2eeb tests: Add fuzzing harness for node eviction logic (practicalswift)
Pull request description:
Add fuzzing harness for node eviction logic.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
MarcoFalke:
cr ACK 5a9ee0869b0b722ebfcdaabaefba6376522b2eeb
Tree-SHA512: c2401d22134867e23dab1ba94ae7ef36fdf52aa0588fdc4705d9cb765ddf979fd775fdf153ce2359f1bc1787cf60bf0ebcd47c7aa29c672e6a253fa58cac292d
c119ba3c9b321a7f4418860741b3f69173e9c891 [doc] clarify getdata limit after #14897 (Michael Polzer)
Pull request description:
GETDATA is limited to blocks and transactions now and can't be used for other non-block data
ACKs for top commit:
laanwj:
ACK c119ba3c9b321a7f4418860741b3f69173e9c891
theStack:
ACK c119ba3c9b
benthecarman:
ACK c119ba3c9b321a7f4418860741b3f69173e9c891
Tree-SHA512: d6e9c109bcce4ef004ec83a9ec591163279476524dec97ed5f5c34e322dca35af66a168f0878ff972bbcec79d81623903f3619fedf8f88cdced3f3f66a779173
fa511042b0bbec02016761bcd0d30f57e0386550 doc: [test] Remove outdated comment in fuzz runner (MarcoFalke)
Pull request description:
All folders are soft-created with `os.makedirs`
ACKs for top commit:
RiccardoMasutti:
ACK fa51104
Tree-SHA512: 4051688946a205a981bbb005300fe3263495ead26591042b38ae44f4297c7689a613b560052fb5405a62054734d2599cfb0554a37c7b7369fb3a3636743d04a8
fa2630328687645fbc7dd1ea46aac32514025715 fuzz: Check that NULL_DATA is unspendable (MarcoFalke)
Pull request description:
* Every script of type NULL_DATA must be unspendable
* The only know types of unspendable scripts are NULL_DATA and certain NONSTANDARD scripts
ACKs for top commit:
sipa:
utACK fa2630328687645fbc7dd1ea46aac32514025715
Tree-SHA512: 8297fbacf32b4868b12accc1c052d352d02d96540a1fc883de9d04a3df8734116deecc33046495c9a3af6d79fec7f8d63afbfa5e401a2ca8d7c70f0f13735c0d
87fe104537eab5ccd8728321fe1c9ba39f7dda78 depends: Use more legible qmake commands in qt package (Hennadii Stepanov)
bf35a8da6ec2791dedf36f459add69ac67b11ff9 depends: Do not set build_subdir for qt package (Hennadii Stepanov)
Pull request description:
Rather than using `cd` to jump all over the place, perform all `(q)make` commands from the top level directory.
Looking at bash like `cd ../../../..` gives me a headache.
Credits to **fanquake**.
This PR is an alternative to #20504 that works without any additional [non-trivial hack](https://github.com/bitcoin/bitcoin/pull/20504#issuecomment-734730336).
ACKs for top commit:
promag:
Tested ACK 87fe104537eab5ccd8728321fe1c9ba39f7dda78.
fanquake:
ACK 87fe104537eab5ccd8728321fe1c9ba39f7dda78
Tree-SHA512: 1d2a13b5358fc7406c5363ddd62fd363dbc0ec5ace68946e4d3e6e8620419afaa64ef2837488aaed226174e01e8897495085540f7126b80f8b2372d21b5b29f9
66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e test: RPC: getblock fee calculations (Elliott Jin)
bf7d6e31b1062ab5f90e14e83c56309f499fa2e9 RPC: getblock: tx fee calculation for verbosity 2 via Undo data (Elliott Jin)
Pull request description:
This change is progress towards #18771 . It adapts the fee calculation part of #16083 and addresses some feedback. The additional "verbosity level 3" features are planned for a future PR.
**Original PR description:**
> Using block undo data (like in #14802) we can now show fee information for each transaction in a block without the need for additional -txindex and/or a ton of costly lookups. For a start we'll add transaction fee information to getblock verbosity level 2. This comes at a negligible speed penalty (<1%).
ACKs for top commit:
luke-jr:
tACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e
fjahr:
tACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e
MarcoFalke:
review ACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e 🗜
Tree-SHA512: be1fe4b866946a8dc36427f7dc72a20e10860e320a28fa49bc85bd2a93a0d699768179be29fa52e18b2ed8505d3ec272e586753ef2239b4230e0aefd233acaa2
812baaa1f804a8c848284759c75e5f9d19e942f8 Switch to BIP341's suggested scheme for outputs without script (Pieter Wuille)
Pull request description:
BIP341 suggests using Hash<sub>TapTweak</sub>(pubkey) to derive the tweak in case of key-only outputs. The functional test framework currently uses Hash<sub>TapTweak</sub>(pubkey || 0x00...00) instead. Change this.
There is no technical reason to prefer one over the other, but in case someone looks at it for inspiration, it's better to be consistent with the BIP.
ACKs for top commit:
laanwj:
ACK 812baaa1f804a8c848284759c75e5f9d19e942f8
instagibbs:
ACK 812baaa1f8
Tree-SHA512: 02576c38776ec786255f49d7edecdb1ed8a9dcf0f547d58c23099588b4c3296edf279b103a6eb80e0f07d3c5ee9743f67d152f5244fd63adc6613b004f6969ed
176325a5a47befe32d480b3dc206dd0e64e04b21 [net processing] Remove dropmessagestest (John Newbery)
Pull request description:
-dropmessagestest is a command line option that causes 1 in n received
messages to be dropped. The Bitcoin P2P protocol is stateful and in
general cannot handle messages being dropped. Dropped
version/verack/ping/pong messages will cause the connection to time out
and be torn down. Other dropped messages may also cause the peer to
believe that the peer has stalled and tear down the connection.
It seems difficult to uncover any actual issues with -dropmessagestest,
and any coverage that could be generated would probably be easier to
trigger with fuzz testing.
ACKs for top commit:
MarcoFalke:
cr ACK 176325a5a47befe32d480b3dc206dd0e64e04b21
practicalswift:
cr ACK 176325a5a47befe32d480b3dc206dd0e64e04b21
dhruv:
cr ACK 176325a
amitiuttarwar:
ACK 176325a5a47befe32d480b3dc206dd0e64e04b21
Tree-SHA512: bd582e5e8c9eb272a5d8ec01ff07c36c0033fbb84c30d1c72c87a7a6c7290021dcaf7bf549179a8b95aeb4f7243158d5593bc7fcf1ec16213782e470fe36bb89
fa957f8dc9990e4479e4d2af46a63ceae89cd39b test: Add race:SendZmqMessage tsan suppression (MarcoFalke)
Pull request description:
Add suppression for `race:SendZmqMessage`, which isn't covered by the existing `zmq::*` suppression
Fixes#20618
ACKs for top commit:
hebasto:
re-ACK fa957f8dc9990e4479e4d2af46a63ceae89cd39b, as my previous comment is not directly related to this pull changes.
Tree-SHA512: 8642a8b79bbfa4bee89042b66e528f27fd78c5e84a33023df440662e9114e31445fd7b04940f44b11fa4ab7438d346385a21816289c818cce9958a9b16730452
3e1571285f4a0edf59d51bbdeee028be3038b6dc Update TSan suppressions (Hennadii Stepanov)
Pull request description:
It seems possible now to drop some TSan suppressions.
Top commit has no ACKs.
Tree-SHA512: 94518fd2f3a7168b2989424de0696e42c8f509b833aafbc7e75f4c1180a0b8d9a47f43c50d06b03b26a924643afe86274b2062c9d456c17a68576d19566ed66f
