e11b649650 validation: CVerifyDB::VerifyDB: Use locking annotation (Carl Dong)
03f75c42e1 validation: Use existing chain member in CChainState::LoadGenesisBlock (Carl Dong)
5e4af77380 validation: Use existing chain member in CChainState::AcceptBlock (Carl Dong)
fee73347c0 validation: Pass in chain to FindBlockPos+SaveBlockToDisk (Carl Dong)
a9d28bcd8d validation: Use *this in CChainState::ActivateBestChainStep (Carl Dong)
4744efc9ba validation: Pass in chainstate to CTxMemPool::check (Carl Dong)
1fb7b2c595 validation: Use *this in CChainState::InvalidateBlock (Carl Dong)
8cdb2f7e58 validation: Move LoadBlockIndexDB to CChainState (Carl Dong)
8b99efbcc0 validation: Move invalid block handling to CChainState (Carl Dong)
2bdf37fe18 validation: Pass in chainstate to CVerifyDB::VerifyDB (Carl Dong)
31eac50c72 validation: Remove global ::VersionBitsTip{State,SinceHeight,Statistics} (Carl Dong)
63e4c7316a validation: Pass in chainstate to ::PruneBlockFilesManual (Carl Dong)
4bada76237 validation: Pass in chainstate to UpdateTip (Carl Dong)
a3ba08ba7d validation: Remove global ::{{Precious,Invalidate}Block,ResetBlockFailureFlags} (Carl Dong)
4927c9e699 validation: Remove global ::LoadGenesisBlock (Carl Dong)
9da106be4d validation: Check chain tip is non-null in CheckFinalTx (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
Based on:
- [x] #20750 | [Bundle 2/n] Prune g_chainman usage in mempool-related validation functions
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
Note to self:
- [x] Address: https://github.com/bitcoin/bitcoin/pull/20750#discussion_r579400663
ACKs for top commit:
laanwj:
Code review ACK e11b649650
Tree-SHA512: 205a451a741e32f17d5966de289f2f5a3f0817738c0087b70ff4755ddd217b53d01050ed396669bda2b1d216a88d927b9778777f9ff95ab1fe20e59c5f341776
fa59ad5130 fuzz: Add missing include (test/util/setup_common.h) (MarcoFalke)
Pull request description:
`src/test/fuzz/socks5.cpp` is using the symbol `BasicTestingSetup`, which is defined in `src/test/util/setup_common.h`.
Currently compilation happens to succeed because the needed dependency is indirectly included. Compilation will break as soon as the indirect dependency is broken. According to the dev notes, everything that is used must be included.
Fix the issue by including the missing include.
ACKs for top commit:
fanquake:
ACK fa59ad5130
Tree-SHA512: 9359d5d288ebc5a53d753ebed1ee8d49ddcfe12aeb56054ea43654c0d915337bb0dce7c8a7178e94711ff8dacd1b3ea0a2871b21b1709cd9786efc0c1ef532b3
0eaea66e8b Make tx relay data structure use std::chrono types (Pieter Wuille)
55e82881a1 Make all Poisson delays use std::chrono types (Pieter Wuille)
c733ac4d8a Convert block/header sync timeouts to std::chrono types (Pieter Wuille)
4d98b401fb Change all ping times to std::chrono types (Pieter Wuille)
Pull request description:
(Picking up #20044. Rebased against master.)
This changes various uses of integers to represent timestamps and durations to `std::chrono` duration types with type-safe conversions, getting rid of various `.count()`, constructors, and conversion factors.
ACKs for top commit:
jnewbery:
utACK 0eaea66e8b
vasild:
ACK 0eaea66e8b
MarcoFalke:
re-ACK 0eaea66e8b, only changes: minor rename, using C++11 member initializer, using 2min chrono literal, rebase 🤚
ajtowns:
utACK 0eaea66e8b
Tree-SHA512: 2dbd8d53bf82e98f9b4611e61dc14c448e8957d1a02575b837fadfd59f80e98614d0ccf890fc351f960ade76a6fb8051b282e252e81675a8ee753dba8b1d7f57
We don't mark RelayTransaction as const. Even though it doesn't mutate
PeerManagerImpl state, it _is_ mutating the internal state of a CNode
object, by updating setInventoryTxToSend. In a subsequent commit, that
field will be moved to the Peer object, which is owned by
PeerMangerImpl.
This requires PeerManagerImpl::ReattemptInitialBroadcast() to no longer
be const.
1f05dbd06d util: Avoid invalid integer negation in ValueFromAmount: make ValueFromAmount(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() (practicalswift)
7cc75c9ba3 util: Avoid invalid integer negation in FormatMoney: make FormatMoney(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() (practicalswift)
Pull request description:
Avoid invalid integer negation in `FormatMoney` and `ValueFromAmount`.
Fixes#20402.
Before this patch:
```
$ CC=clang CXX=clang++ ./configure --with-sanitizers=undefined
$ make -C src/ test/test_bitcoin
$ src/test/test_bitcoin -t rpc_tests/rpc_format_monetary_values -t util_tests/util_FormatMoney
core_write.cpp:21:29: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount'
(aka 'long'); cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior core_write.cpp:21:29 in
test/rpc_tests.cpp(186): error: in "rpc_tests/rpc_format_monetary_values":
check ValueFromAmount(std::numeric_limits<CAmount>::min()).write() == "-92233720368.54775808" has failed
[--92233720368.-54775808 != -92233720368.54775808]
util/moneystr.cpp:16:34: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount'
(aka 'long'); cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior util/moneystr.cpp:16:34 in
test/util_tests.cpp(1188): error: in "util_tests/util_FormatMoney":
check FormatMoney(std::numeric_limits<CAmount>::min()) == "-92233720368.54775808" has failed
[--92233720368.-54775808 != -92233720368.54775808]
```
After this patch:
```
$ CC=clang CXX=clang++ ./configure --with-sanitizers=undefined
$ make -C src/ test/test_bitcoin
$ src/test/test_bitcoin -t rpc_tests/rpc_format_monetary_values -t util_tests/util_FormatMoney
```
ACKs for top commit:
laanwj:
re-ACK 1f05dbd06d
Tree-SHA512: 5aaeb8e2178f1597921f53c12bdfc2f3d5993d10c41658dcd25943e54e8cc2116a411bc71d928f890b33bc0b3761a8ee4449b0532bce41125b6c60692808c8c3
7d3343fb8e cli: update -netinfo help doc following the merge of 882ce251 (Jon Atack)
ef614bb408 cli: small -netinfo simplification and performance improvement (Jon Atack)
6b45ef3233 cli: improve -netinfo invalid argument error message (Jon Atack)
3732404afa cli: warn in help that -netinfo is not intended to be a stable API (Jon Atack)
7afdd72258 cli: enable -netinfo help to run without a remote server (Jon Atack)
Pull request description:
A few updates, some per IRC discussion today at http://www.erisian.com.au/bitcoin-core-dev/log-2021-01-07.html#l-87 with respect to -netinfo:
- enable `-netinfo help` to run without a remote server
- warn in `-netinfo help` that -netinfo is not intended to be a stable API
- improve the -netinfo invalid argument error message
- make a performance improvement and simplification I noticed after the merge of #20764
- update the -netinfo help doc following the merge of #21192
-----
How to test manually: 🔬🧪📈
1. check out and build this branch locally; if you need help, don't hesitate to refer to https://jonatack.github.io/articles/how-to-review-pull-requests-in-bitcoin-core#pull-down-the-code-locally or https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests
2. while it is compiling, look at the code changes
3. stop signet (if it is running) with `./src/bitcoin-cli -signet stop`
4. once the build is completed, run `./src/bitcoin-cli -signet -netinfo help`
5. the help should be printed even though the signet server is not running
6. near the top you should see the new warning, "This human-readable interface will change regularly and is not intended to be a stable API" as well as a bit more description about the integer argument values.
7. start signet with `./src/bitcoind -signet`
8. test the improved invalid argument error message if you run `./src/bitcoin-cli -signet -netinfo 256` or `./src/bitcoin-cli -signet -netinfo a` (valid values are from 0 to 255)
9. leave review feedback or `ACK <commit hash>` -- done 🍻
ACKs for top commit:
michaelfolkson:
Re-ACK 7d3343fb8e
pinheadmz:
RE-ACK 7d3343fb8e
Tree-SHA512: 28c5e9f295ffccba5c2a70faac4987d45f35d4758cf8f10daa767e83212316c4cfc65930e4066f7ad627e9d15b92d43439d1ba9c2f755dfde61885c6a70aa155
366e3e1f89 fuzz: Add FUZZED_SOCKET_FAKE_LATENCY mode to FuzzedSock to allow for fuzzing timeout logic (practicalswift)
b22d4c1607 fuzz: Add fuzzing harness for Socks5(...) (practicalswift)
Pull request description:
Add [regression fuzz harness](https://twitter.com/kayseesee/status/1205287895923212289) for CVE-2017-18350. This fuzzing harness would have found CVE-2017-18350 within a minute of fuzzing :)
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:
vasild:
ACK 366e3e1f89
Tree-SHA512: 5d8e1863b635efd10ccb11678b71472ba1523c3ef16affa7f9cd638635c1a9c307e28f432d5b87eb0c9cd1c3c1aeafbb24fa7ae86fe4e5090fda2e20d542b6ca
a701fcf01f net: Do not skip the I2P network from GetNetworkNames() (Vasil Dimov)
0181e24439 net: recognize I2P from ParseNetwork() so that -onlynet=i2p works (Vasil Dimov)
b905363fa8 net: accept incoming I2P connections from CConnman (Vasil Dimov)
0635233a1e net: make outgoing I2P connections from CConnman (Vasil Dimov)
9559bd1404 net: add I2P to the reachability map (Vasil Dimov)
76c35c60f3 init: introduce I2P connectivity options (Vasil Dimov)
c22daa2ecf net: implement the necessary parts of the I2P SAM protocol (Vasil Dimov)
5bac7e45e1 net: extend Sock with a method to check whether connected (Vasil Dimov)
42c779f503 net: extend Sock with methods for robust send & read until terminator (Vasil Dimov)
ea1845315a net: extend Sock::Wait() to report a timeout (Vasil Dimov)
78fdfbea66 net: dedup MSG_NOSIGNAL and MSG_DONTWAIT definitions (Vasil Dimov)
34bcfab562 net: move the constant maxWait out of InterruptibleRecv() (Vasil Dimov)
cff65c4a27 net: extend CNetAddr::SetSpecial() to support I2P (Vasil Dimov)
f6c267db3b net: avoid unnecessary GetBindAddress() call (Vasil Dimov)
7c224fdac4 net: isolate the protocol-agnostic part of CConnman::AcceptConnection() (Vasil Dimov)
1f75a653dd net: get the bind address earlier in CConnman::AcceptConnection() (Vasil Dimov)
25605895af net: check for invalid socket earlier in CConnman::AcceptConnection() (Vasil Dimov)
545bc5f81d util: fix WriteBinaryFile() claiming success even if error occurred (Vasil Dimov)
8b6e4b3b23 util: fix ReadBinaryFile() returning partial contents (Vasil Dimov)
4cba2fdafa util: extract {Read,Write}BinaryFile() to its own files (Vasil Dimov)
Pull request description:
Add I2P support by using the [I2P SAM](https://geti2p.net/en/docs/api/samv3) protocol. Unlike Tor, for incoming connections we get the I2P address of the peer (and they also receive ours when we are the connection initiator).
Two new options are added:
```
-i2psam=<ip:port>
I2P SAM proxy to reach I2P peers and accept I2P connections (default:
none)
-i2pacceptincoming
If set and -i2psam is also set then incoming I2P connections are
accepted via the SAM proxy. If this is not set but -i2psam is set
then only outgoing connections will be made to the I2P network.
Ignored if -i2psam is not set. Notice that listening for incoming
I2P connections is done through the SAM proxy, not by binding to
a local address and port (default: true)
```
# Overview of the changes
## Make `ReadBinary()` and `WriteBinary()` reusable
We would need to dump the I2P private key to a file and read it back later. Move those two functions out of `torcontrol.cpp`.
```
util: extract {Read,Write}BinaryFile() to its own files
util: fix ReadBinaryFile() returning partial contents
util: fix WriteBinaryFile() claiming success even if error occurred
```
## Split `CConnman::AcceptConnection()`
Most of `CConnman::AcceptConnection()` is agnostic of how the socket was accepted. The other part of it deals with the details of the `accept(2)` system call. Split those so that the protocol-agnostic part can be reused if we accept a socket by other means.
```
net: check for invalid socket earlier in CConnman::AcceptConnection()
net: get the bind address earlier in CConnman::AcceptConnection()
net: isolate the protocol-agnostic part of CConnman::AcceptConnection()
net: avoid unnecessary GetBindAddress() call
```
## Implement the I2P [SAM](https://geti2p.net/en/docs/api/samv3) protocol (not all of it)
Just the parts that would enable us to make outgoing and accept incoming I2P connections.
```
net: extend CNetAddr::SetSpecial() to support I2P
net: move the constant maxWait out of InterruptibleRecv()
net: dedup MSG_NOSIGNAL and MSG_DONTWAIT definitions
net: extend Sock::Wait() to report a timeout
net: extend Sock with methods for robust send & read until terminator
net: extend Sock with a method to check whether connected
net: implement the necessary parts of the I2P SAM protocol
```
## Use I2P SAM to connect to and accept connections from I2P peers
Profit from all of the preceding commits.
```
init: introduce I2P connectivity options
net: add I2P to the reachability map
net: make outgoing I2P connections from CConnman
net: accept incoming I2P connections from CConnman
net: recognize I2P from ParseNetwork() so that -onlynet=i2p works
net: Do not skip the I2P network from GetNetworkNames()
```
ACKs for top commit:
laanwj:
re-ACK a701fcf01f
jonatack:
re-ACK a701fcf01f reviewed diff per `git range-diff ad89812 2a7bb34 a701fcf`, debug built and launched bitcoind with i2pd v2.35 running a dual I2P+Torv3 service with the I2P config settings listed below (did not test `onlynet=i2p`); operation appears nominal (same as it has been these past weeks), and tested the bitcoind help outputs grepping for `-i i2p` and the rpc getpeerinfo and getnetworkinfo helps
Tree-SHA512: de42090c9c0bf23b43b5839f5b4fc4b3a2657bde1e45c796b5f3c7bf83cb8ec6ca4278f8a89e45108ece92f9b573cafea3b42a06bc09076b40a196c909b6610e
e3e0a2432c Add benchmark to write JSON into a string (Martin Ankerl)
Pull request description:
The benchmark `BlockToJsonVerbose` only tests generating (and destroying)
the JSON data structure, but serializing into a string is also a
performance critical aspect of the RPC calls.
Extracts test setup into a `struct TestBlockAndIndex`, and uses it in
both `BlockToJsonVerbose` and `BlockToJsonVerboseWrite`.
Also, use `ankerl::nanobench::doNotOptimizeAway` to make sure the compiler
can't optimize the result of the calls away.
Here are benchmark results on my Intel i7-8700:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 71,807,017.00 | 13.93 | 0.4% | 555,782,961.00 | 220,788,645.00 | 2.517 | 102,279,341.00 | 0.4% | 0.80 | `BlockToJsonVerbose`
| 27,916,835.00 | 35.82 | 0.1% | 235,084,034.00 | 89,033,525.00 | 2.640 | 42,911,139.00 | 0.3% | 0.32 | `BlockToJsonVerboseWrite`
ACKs for top commit:
laanwj:
Code review ACK e3e0a2432c
Tree-SHA512: bc4d6d1588d47d4bd7af8e7908e44b8561bc391a2d73eccd7c0aa37fc40e8a9ce1fa1f3c29b416eef24a73c6bce3036839c0bbfe1b8dbd6d1bba3718b7ca5383
So that help texts include "i2p" in:
* `./bitcoind -help` (in `-onlynet` description)
* `getpeerinfo` RPC
* `getnetworkinfo` RPC
Co-authored-by: Jon Atack <jon@atack.com>
Introduce two new options to reach the I2P network:
* `-i2psam=<ip:port>` point to the I2P SAM proxy. If this is set then
the I2P network is considered reachable and we can make outgoing
connections to I2P peers via that proxy. We listen for and accept
incoming connections from I2P peers if the below is set in addition to
`-i2psam=<ip:port>`
* `-i2pacceptincoming` if this is set together with `-i2psam=<ip:port>`
then we accept incoming I2P connections via the I2P SAM proxy.
Implement the following commands from the I2P SAM protocol:
* HELLO: needed for all of the remaining ones
* DEST GENERATE: to generate our private key and destination
* NAMING LOOKUP: to convert .i2p addresses to destinations
* SESSION CREATE: needed for STREAM CONNECT and STREAM ACCEPT
* STREAM CONNECT: to make outgoing connections
* STREAM ACCEPT: to accept incoming connections
