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.
233a886b42 test: check that getblockfilter RPC fails without block filter index (Sebastian Falbesoner)
Pull request description:
If a node was started without compact block filter index (parameter `--blockfilterindex=0`), the `getblockfilter` RPC call should fail.
ACKs for top commit:
MarcoFalke:
review ACK 233a886b42
Tree-SHA512: c8824373fad7d1de2dcb43c1d9541d736b478235be243080d2b7479c2588eac0e5722337ec1307394b331e0002fbcabb368e4955c2dc98dd5fce76d8c089e8a1
The source we are currently using is from 2015. The upstream repo has
received a small number of bug fixes and improvements since then.
Including one that fixes an issue for Windows users:
https://github.com/miniupnp/libnatpmp/pull/13.
The source we are currently using is the most recent "official" release,
however I don't think it's worth waiting for a new one. The maintainer
was prompted to do so in Oct 2020, then again in Jan of this year, and
no release has eventuated. Given libnatpmp is a new inclusion into our
repository, I think we should be using this newer source.
This also cleans up a few warnings we currently see in depends builds:
```bash
Extracting libnatpmp...
/home/ubuntu/bitcoin/depends/sources/libnatpmp-20150609.tar.gz: OK
Preprocessing libnatpmp...
Configuring libnatpmp...
Building libnatpmp...
make[1]: Entering directory '/home/ubuntu/bitcoin/depends/work/build/x86_64-w64-mingw32/libnatpmp/20150609-13efa1beb87'
x86_64-w64-mingw32-gcc -Os -fPIC -Wall -DENABLE_STRNATPMPERR -c -o natpmp.o natpmp.c
x86_64-w64-mingw32-gcc -Os -fPIC -Wall -DENABLE_STRNATPMPERR -c -o getgateway.o getgateway.c
natpmp.c:42: warning: "EWOULDBLOCK" redefined
42 | #define EWOULDBLOCK WSAEWOULDBLOCK
|
In file included from natpmp.c:38:
/usr/share/mingw-w64/include/errno.h:166: note: this is the location of the previous definition
166 | #define EWOULDBLOCK 140
|
natpmp.c:43: warning: "ECONNREFUSED" redefined
43 | #define ECONNREFUSED WSAECONNREFUSED
|
In file included from natpmp.c:38:
/usr/share/mingw-w64/include/errno.h:110: note: this is the location of the previous definition
110 | #define ECONNREFUSED 107
|
natpmp.c:271:5: warning: ‘readnatpmpresponseorretry’ redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
271 | int readnatpmpresponseorretry(natpmp_t * p, natpmpresp_t * response)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
ar crs libnatpmp.a natpmp.o getgateway.o
make[1]: Leaving directory '/home/ubuntu/bitcoin/depends/work/build/x86_64-w64-mingw32/libnatpmp/20150609-13efa1beb87'
Staging libnatpmp...
Postprocessing libnatpmp...
Caching libnatpmp...
```
This pr makes the macOS build docs more informative and adds in the following information:
- Proper descriptions and delineation of required/optional dependencies
- walk-through of optional dependencies
- configuration walk-through
- various other tid-bits of information
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
bd49ac4168 build: fix libnatpmp macos cross compile (fanquake)
Pull request description:
Currently, our cross-compile of libnatpmp for macOS doesn't work at all.
The wrong archiver is used, which produces an archive the linker doesn't like.
This becomes clear when configuring:
```bash
configure:25722: checking for initnatpmp in -lnatpmp
configure:25747: env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH -u LIBRARY_PATH /home/ubuntu/bitcoin/depends/x86_64-apple-darwin18/native/bin/clang++ --target=x86_64-apple-darwin18 <trim> -Wl,-headerpad_max_install_names -Wl,-dead_strip -Wl,-dead_strip_dylibs conftest.cpp -lnatpmp >&5
ld: archive has no table of contents for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
Fix this by using the right `ar` (we do the same for upnp).
While we're at it, fix the build so that we are using our c/ppflags.
In practice this basically means building with `-O2` rather than `-Os`.
Note that this fixes an issue that is also fixed by #21209. However, given there are reservations about updating to use a newer libnatpmp source, we should just fix this for now.
ACKs for top commit:
hebasto:
ACK bd49ac4168, tested:
Tree-SHA512: 2efc2c788ef3ebebfbf564ef07b6cf63a72d8a0bccc22b0ba36537216aa575436b7e87088477e85f6d9191ad34f0b13f1c22cf88c90e1cb81641bfee5dc3058a
4899fa3abd doc: Update build instructions for Fedora (Hennadii Stepanov)
Pull request description:
This PR updates build instructions for Fedora, as Fedora 33 has no `libdb4-devel` and `libdb4-cxx-devel` packages in its default repos.
ACKs for top commit:
practicalswift:
ACK 4899fa3abd
Tree-SHA512: 0c23e2ce0ea690ed5eeaa56514d4246e0057a77b7c71f28af4ee1e480521d465122f81cea37cc773ce2db4fc189d5ab3c8f8ffdd65f150cc006390aa1e2a4ac8
10d4477dae tests: Add fuzzing harness for TorController (practicalswift)
64219c01dc torcontrol: Move TorControlReply, TorControlConnection and TorController to improve testability (practicalswift)
Pull request description:
Add fuzzing harness for `TorController`.
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:
laanwj:
ACK 10d4477dae
Tree-SHA512: 2da4b1000afe0e65a234636b8fbf6a26fe9e257852bd837168ca73aa3575959e9aded19054620439e4ed0b2787c70cad4541a8c2d210f5238d7f5e9e0545b734
bec7f2caf7 doc: install qt5 when building on macOS (fanquake)
Pull request description:
Brew has updated such that qt now refers to [Qt 6.0.1](https://github.com/Homebrew/homebrew-core/blob/master/Formula/qt.rb). If builders
install this, configure will not pick up qt. For now, install
[qt@5 (5.15.2)](https://github.com/Homebrew/homebrew-core/blob/master/Formula/qt@5.rb), until required build system and likely source changes
are made.
ACKs for top commit:
hebasto:
ACK bec7f2caf7, tested on Tested on macOS Big Sur 11.2.2 (20D80).
Tree-SHA512: 86663cfbc68c8c6f5d608d60cd59b37d3faf1e7f33ae17ec2e1a7c076e835eb8200181a17575f121929ea6ecded74b1619096fe5a763106f56de0bdbea9ae4fa
76f52e3da3 build: Fix Qt processing of configure script for depends with DEBUG=1 (Hennadii Stepanov)
Pull request description:
This PR:
- makes the `configure` script correctly pickup Qt if depends is built with `DEBUG=1`:
- for Windows -- fix#19266
- for macOS -- fix#16391
- is an alternative to #18117 (without downsides)
ACKs for top commit:
fanquake:
ACK 76f52e3da3. Tested native darwin, and darwin/win cross compile with `DEBUG=1`.
Tree-SHA512: 8fde99302b4b06faf109315bddba9e3063b156c50f8f9863c2bd51718538c719429a63fdced071730c18022f2e559d3b25c1dcec3efa81fe79f657253680956a
Brew has updated such that qt now refers to Qt 6.0.1. If builders
install this, configure will not work pick up qt. For now, install
qt@5 (5.15.2), until required build system and likely source changes
are made.
a4128138b4 build: Make AM_CONDITIONAL([ENABLE_EXTERNAL_SIGNER]) unconditional (Hennadii Stepanov)
9fef209945 build, refactor: Fix indentation for if..then..fi (Hennadii Stepanov)
Pull request description:
#16546 introduced a regression in the `configure`:
```
$ ./autogen.sh
$ ./configure --disable-wallet --without-utils --without-daemon --without-gui --disable-tests --disable-bench
...
checking whether to build test_bitcoin... no
checking whether to reduce exports... no
checking that generated files are newer than configure... done
configure: error: conditional "ENABLE_EXTERNAL_SIGNER" was never defined.
Usually this means the macro was only invoked conditionally.
```
This PR fixes this bug, and refactors indentation to make easier to spot similar bugs in the future.
ACKs for top commit:
Sjors:
utACK a4128138b4
fanquake:
ACK a4128138b4 - this fixes the bug described, and improves readability.
Tree-SHA512: 4469dcc006690f38f93c3cdf8d15b76f5fc8ea76e87a1b5db5ee891dc9851f6ec539f2a6fd02a361aa76baa4f4b2b9fe8289137f5d9734ee5984f265cb131ef5
9086e0dd3c build: set Unicode true for NSIS installer (fanquake)
Pull request description:
Now that we are using Focal for gitian builds, and have [NSIS 3.0+ available](https://packages.ubuntu.com/focal/nsis) (also in Guix), we can create installers that [support unicode](https://nsis.sourceforge.io/Docs/Chapter4.html#aunicodetarget).
Unicode is only becoming the NSIS default [beginning with the 3.07 release](https://nsis.sourceforge.io/Docs/AppendixF.html#v3.07-cl), so we need to set this attribute to get support.
Should close: #13817
Gitian builds:
```bash
b8553615b6b4be5e4459e03796e700b30b5d198a7f184f27be6983ff901b5592 bitcoin-9086e0dd3c92-win-unsigned.tar.gz
a6b024a5a68e0196e8e118168c918285e820f2d0ffe9c38db680580459da8bf3 bitcoin-9086e0dd3c92-win64-debug.zip
ff4003d4f61127c707e44b5235eaf924b30351f20cde27e775131982a1b4cf92 bitcoin-9086e0dd3c92-win64-setup-unsigned.exe
1876bee55fa9ea99b91203975c13d0ad8a046b4b58068bde41c977fd1d12de13 bitcoin-9086e0dd3c92-win64.zip
000f2778f8f166a89b4ab35f155156c1c34800be6e47d29b5308043c50128392 src/bitcoin-9086e0dd3c92.tar.gz
d650a9b8f2dd1df777bf42439dfcbcf6bc358e30ec148b9992a18b39f76b1ecf bitcoin-core-win-22-res.yml
```
ACKs for top commit:
laanwj:
ACK 9086e0dd3c
hebasto:
ACK 9086e0dd3c, tested on Windows 10 Pro (20H2, build 19042.804):
Tree-SHA512: cc7b7ca05877571d0a29a7d36e40279f54d886d8ab27facfa722c2ee95a1fc06c2bad8ef1eb1980d283ae981659d737021a46c8f4618e24d510b5ab384990e09
After the introduction of wtxidrelay and sendaddrv2 messages during
version handshake, extend p2p_leak.py test to reflect this.
Also, some minor fixes and doc improvements.
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
