out of net_tests, because the eviction tests:
- are a different domain of test coverage, with different dependencies
- run more slowly than the net tests
- will be growing in size, in this PR branch and in the future, as eviction
test coverage is improved
to allow deterministic unit testing of the ratio-based peer eviction protection
logic, which protects peers having longer connection times and those connected
via higher-latency networks.
Add documentation.
ea76f4ac7d Doc: Tell howto install clang-format on Debian/Ubuntu (wodry)
Pull request description:
Because only macOS wasy mentioned, I was unsure if this would be a macOS specific tool. I guess Linux is more used than Mac, so Linux guide should be there, too.
ACKs for top commit:
hebasto:
ACK ea76f4ac7d, every system upgrade via clean installation I do the same.
Tree-SHA512: 75c28540e8815cb41f4cf92784b6349978988b679e4deef9ae77ede951f93516ca13ec7b313ab72865b01273e115b49ed2b67cdcd68015af1b643a6186b190dd
Because only macOS wasy mentioned, I was unsure if this would be a macOS specific tool. I guess Linux is more used than Mac, so Linux guide should be there, too.
fa81773243 style-only: Remove whitespace (MarcoFalke)
fae77b9e6d net: Simplify ProcessGetBlockData execution by removing send flag. (Patrick Strateman)
fae7c0429f log: Clarify that block request below NODE_NETWORK_LIMITED_MIN_BLOCKS disconnects (MarcoFalke)
Pull request description:
* Clarify that "ignoring" really means "disconnect" in the log
* Revive a refactor I took from #13670
ACKs for top commit:
jnewbery:
utACK fa81773243
sipa:
utACK fa81773243
Tree-SHA512: 0a4fcb979cb82c4e26012881eeaf903c38dfbb85d461476c01e35294760744746a79c48ffad827fe31c1b830f40c6e4240529c71e375146e4d0313c3b7d784ca
0fc0c00f7a test: Drop unused get_machine function (Hennadii Stepanov)
61a0f8f9cc test: Cleanup test files in test-{security,symbol}-check.py (Hennadii Stepanov)
Pull request description:
1) Test source and executable files are neither ignored by `.gitignore` nor removed by `make clean` and `make distclean`.
2) The `get_machine` function is no longer used since #21255.
ACKs for top commit:
fanquake:
ACK 0fc0c00f7a
Tree-SHA512: ef3fcf22d4a04b6e4f37f748bd4be57e09696d2a77982e26292843cb2a1297789c8325f5c4bdad37d8094fce7765c4cc9ab19809e07471487943361b2b1a252c
fa2a80bf12 refactor: Pass PeerManagerImpl members only once (MarcoFalke)
Pull request description:
Member variables are already passed to methods via `this`, so no need to pass them another time as function parameter.
ACKs for top commit:
jnewbery:
utACK fa2a80bf12
amitiuttarwar:
utACK fa2a80bf12
Tree-SHA512: 1743825c7560cc748235e3db03e4cea02ad1f670f1b898d7757da644f12693ba9bb2d3eb09b64b3d10dd2e68f52dea31e26d5e97bdc013759baa0515d3c7055c
680eb56d82 [net processing] Don't pass CConnman to RelayTransactions (John Newbery)
a38a4e8f03 [net processing] Move RelayTransaction into PeerManager (John Newbery)
Pull request description:
This is the first part of #21160. It moves the RelayTransaction() function to be a member function of the PeerManager class. This is required in order to move the transaction inventory data into the Peer object, since Peer objects are only accessible from within PeerManager.
ACKs for top commit:
ajtowns:
ACK 680eb56d82
Tree-SHA512: 8c93491a4392b6369bb7f090de326a63cd62a088de59026e202f226f64ded50a0cf1a95ed703328860f02a9d2f64d3a87ca1bca9a6075b978bd111d384766235
bb3f79fbdc doc: Update libnatpmp info in dependencies.md (Hennadii Stepanov)
1a01a5dc8d doc: Update zlib info in dependencies.md (Hennadii Stepanov)
Pull request description:
Update docs according to the recent changes in the code:
- #21209 (zlib)
- #21376 (libnatpmp)
ACKs for top commit:
fanquake:
ACK bb3f79fbdc - thanks for keeping this updated.
Tree-SHA512: 48350ad07700aa071ad6c34e4c161aaadc050488fc068cf478e9781d632828187962a4384c1b67c2344145a2c00c3e16cddd09259130af8e9e86cd76cd32900d
c180c911b8 doc: revamp macOS build doc (Jarol Rodriguez)
Pull request description:
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 tidbits of information
This is a part of the efforts done in https://github.com/bitcoin/bitcoin/pull/20601 and https://github.com/bitcoin/bitcoin/pull/20610 to update the docs and introduce some consistency between them.
This update does not add instructions for arm-based M1 Macbooks as I do not have one to test with. It would be nice to have someone follow up with an update containing instructions for arm-based Macs.
**Before/Master:** [render](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md)
**After/PR:** [render](c180c911b8/doc/build-osx.md)
ACKs for top commit:
fanquake:
ACK c180c911b8 - I still think these are getting too verbose and we're duplicating information all over the place; dependencies, configure options, combinations of options etc. However if people are happy to maintain them, I guess it's fine for now, and this revamping has already happened for some of the other build READMEs.
Tree-SHA512: 1440046c723fe80d4158e4a429e3aa8bd93570acb84ad202d5d24c749ab9a89a3aca8b61b49e75e042a4bf4317acd632d3906e1b5808a9052e74209256528b45
ebc4ab721b refactor: post Optional<> removal cleanups (fanquake)
57e980d13c scripted-diff: remove Optional & nullopt (fanquake)
Pull request description:
Same rationale & motivation as #21404, which turned out to be quite low in the number of potential conflicts. Lets see what the bot has to say here.
ACKs for top commit:
practicalswift:
cr ACK ebc4ab721b: patch looks correct
jnewbery:
utACK ebc4ab721b
laanwj:
Code review ACK ebc4ab721b
Tree-SHA512: 550fbeef09b9d35ddefaa805d1755c18c8fd499c4b0f77ebfece8c20296a7abd1cf6c699e2261f92fe3552deeb7555ec2a2287ffe3ab9e98bb9f8612a4d43be3
57ff5a42ab doc: specify minimum HWI version (Sjors Provoost)
03308b2bfa rpc: don't require wallet for enumeratesigners (Sjors Provoost)
Pull request description:
HWI just released 2.0. See https://github.com/bitcoin-core/HWI/releases/tag/2.0.0
As of #16546 we already rely on features that are in 2.0 and not in the previous 1.* releases:
* `--chain` param
This shouldn't be a problem, because HWI 2.0 has been released before we release v22.
Misc improvements:
* document that HWI 2.0 is required
* drop wallet requirement for `enumeratesigners`
ACKs for top commit:
achow101:
Code Review ACK 57ff5a42ab
Tree-SHA512: 3fb6ba20894e52a116f89525a5f5a1f61d363ccd904e1cffd0e6d095640fc6d2edf0388cd6ae20f83bbc31e5f458255ec090b6e823798d426eba3e45b4336bf9
f9cd2bfbcc Rename CoinSelectionParams::effective_fee to m_effective_feerate (Andrew Chow)
bdd0c2934b wallet: Move discard feerate fetching to CreateTransaction (Andrew Chow)
448d04b931 wallet: Move long term feerate setting to CreateTransaction (Andrew Chow)
e2f429e6bb wallet: Replace nFeeRateNeeded with effective_fee (Andrew Chow)
1a6a0b0dfb wallet: Use existing feerate instead of getting a new one (Andrew Chow)
Pull request description:
During coin selection, there are various places where we need to have a feerate. We need the feerate for the transaction itself, the discard fee rate, and long term feerate. Fetching these each time we need them can lead to a race condition where two feerates that should be the same are actually different. One particular instance where this can happen is during the loop in `CreateTransactionInternal`. After inputs are chosen, the expected transaction fee is calculated using a newly fetched feerate. If `pick_new_inputs == false`, the loop will go again with the assumption that the fee for the transaction remains the same. However because the feerate is fetched again, it is possible that it actually isn't and this causes coin selection to fail.
Instead of fetching the feerate each time it is needed, we fetch them all at once at the top of `CreateTransactionInternal`, store them in `CoinSelectionParams`, and use them where needed.
While some of these fee rates probably don't need this caching, I've done it for consistency and the guarantee that they remain the same.
Fixes#19229
ACKs for top commit:
glozow:
reACK f9cd2bfbcc
fjahr:
Code review re-ACK f9cd2bfbcc
Xekyo:
tACK f9cd2bfbcc
meshcollider:
Code review + test run ACK f9cd2bfbcc
Tree-SHA512: be83ff64ba473c3cdd3469c812e214659b6e2a9584c22ed2b1595618fce0d4b35d0901e61068cd1069fc1a8fb911db01dd7312d05c3b8cbafbe2504ab7a3e863
Instead of fetching the discard feerate for each SelectCoinsMinConf
iteration, fetch and cache it once during CreateTransaction so that it
is shared for each SelectCoinsMinConf through
coin_selection_params.m_discard_feerate.
Does not change behavior.
Instead of setting the long term feerate for each SelectCoinsMinConf
iteration, set it once during CreateTransaction and let it be shared
with each SelectCoinsMinConf through
coin_selection_params.m_long_term_feerate.
Does not change behavior.
d09120b7d1 test: give fundraw more time for test_transaction_too_large (Jon Atack)
Pull request description:
to hopefully fix timeouts from a new test added in 48a0319bab of #20536 merged March 8, 2021
seen locally when running via the test runner
```
File "/home/jon/projects/bitcoin/bitcoin/test/functional/rpc_fundrawtransaction.py", line 927, in test_transaction_too_large
raise JSONRPCException({
test_framework.authproxy.JSONRPCException: 'generatetoaddress' RPC took longer than 30.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)
```
and in the CI like https://bitcoinbuilds.org/index.php?ansilog=28537952-2c92-46f2-9871-8918e5ba2738.log#l2398
```
File "/home/ubuntu/src/test/functional/rpc_fundrawtransaction.py", line 927, in test_transaction_too_large
test_framework.authproxy.JSONRPCException: 'generatetoaddress' RPC took longer than 240.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)
```
Top commit has no ACKs.
Tree-SHA512: f11c923439014fe12420f986c640fd301a26282eb41516957d73b9c751087cbae3d0e316f9ccb49bcb424f488540266f70d3f97948633e77c62bd7935df90452
Make sure that all fee calculations use the same feerate.
coin_selection_params.effective_fee is the variable we use for all fee
calculations, so get rid of remaining nFeeRateNeeded usages and just
directly set coin_selection_params.effective_fee.
Does not change behavior.
During each loop of CreateTransaction, instead of constantly getting a
new feerate, use the feerate that we have already fetched for all
fee calculations. Thix fixes a race condition where the feerate required
changes during each iteration of the loop.
This commit changes behavior as the "Fee estimation failed" error will
now take priority over "Signing transaction failed".
4d008f908e Always add -daemonwait to known command line arguments (Hennadii Stepanov)
Pull request description:
This is a follow up of #21007.
When `AC_CHECK_DECLS([fork])` fails:
- on master (8e6532053f):
```
$ src/bitcoind -daemonwait
Error: Error parsing command line arguments: Invalid parameter -daemonwait
```
- with this PR:
```
$ src/bitcoind -daemonwait
Error: -daemon is not supported on this operating system
```
ACKs for top commit:
laanwj:
Code review ACK 4d008f908e
Tree-SHA512: 7fcb5e9d76958adcf57e04fa74bd2a98d62459d81a3c57a97bd74c346cbf47c53e560a15455fb024e912c3b44e8487a83499e993b282871ba069953e665d88a9
Add a regression test for https://github.com/bitcoin/bitcoin/pull/21407.
The test creates a socket that, upon read, returns some data, but never
the expected terminator `\n`, injects that socket into the I2P code and
expects `i2p::sam::Session::Connect()` to fail, printing a specific
error message to the log.
Change the types of `i2p::Connection::sock` and
`i2p::sam::Session::m_control_sock` from `Sock` to
`std::unique_ptr<Sock>`.
Using pointers would allow us to sneak `FuzzedSock` instead of `Sock`
and have the methods of the former called.
After this change a test only needs to replace `CreateSock()` with
a function that returns `FuzzedSock`.
Change `ConnectSocketDirectly()` to take a `Sock` argument instead of a
bare `SOCKET`. With this, use the `Sock`'s (possibly mocked) methods
`Connect()`, `Wait()` and `GetSockOpt()` instead of calling the OS
functions directly.
If `recv(2)` returns an error (`-1`) and sets `errno` to a temporary
error like `EAGAIN` a proper application code is expected to retry the
operation.
If the fuzz data is exhausted, then `FuzzedSock::Recv()` will keep
returning `-1` and setting `errno` to the first element of
`recv_errnos[]` which happened to be `EAGAIN`. This may continue forever
or cause the fuzz test to run for a long time before some higher level
application "receive timeout" is triggered.
Thus, put `ECONNREFUSED` as first element of `recv_errnos[]`.
A conforming `recv(2)` call is supposed to return the same data on a
call following `recv(..., MSG_PEEK)`. Extend `FuzzedSock::Recv()` to do
that.
For simplicity we only return 1 byte when `MSG_PEEK` is used. If we
would return a buffer of N bytes, then we would have to keep track how
many of them were consumed on subsequent non-`MSG_PEEK` calls.
We want `Get()` to always return the same value, otherwise it will look
like the `FuzzedSock` implementation itself is broken. So assign
`m_socket` a random number in the `FuzzedSock` constructor.
There is nothing to fuzz about the `Get()` and `Release()` methods, so
use the ones from the base class `Sock`.
`Reset()` is just setting our socket to `INVALID_SOCKET`. We don't want
to use the base `Reset()` because it will close `m_socket` and given
that our `m_socket` is just a random number it may end up closing a real
opened file descriptor if it coincides with our random `m_socket`.
7059e6d822 test: add a test to ensure RecvUntilTerminator() limit works (Vasil Dimov)
80a5a8ea2b i2p: limit the size of incoming messages (Vasil Dimov)
Pull request description:
Put a limit on the amount of data `Sock::RecvUntilTerminator()` can read
if no terminator is received.
In the case of I2P this avoids a runaway (or malicious) I2P proxy
sending us tons of data without a terminator before a timeout is
triggered.
ACKs for top commit:
laanwj:
Re-ACK 7059e6d822
Tree-SHA512: 21f3f3468c765c726cdc877eae847cdb4dbe225d94c5bd1849bd752c9761fac881c554f16ea7a685ad40312159d554e126c481e21c5fd83a6d947060b920373d
88df300f20 qt: Do not translate file extensions (Hennadii Stepanov)
Pull request description:
File extensions are untranslatable by their nature.
ACKs for top commit:
laanwj:
Concept and code review ACK 88df300f20
Talkless:
tACK 88df300f20, tested on Debian Sid with Qt 5.15.2. Tested all filters except for .psbt.
jarolrod:
re-ACK 88df300f20
Tree-SHA512: 104d383543edcee8fb825f98d3b6669a7aaae2c74b6602f9bc407bf1c88be121ec535f2f9be87afa6ca775dc79865165f620553f6f6ab1d31a3f9ea93f7f9593
76782e560b refactor: reuse test string with embedded null char in util_tests (Jon Atack)
24c6546946 test: add ParseUInt8() unit and fuzz test coverage (Jon Atack)
Pull request description:
We have unit test and fuzzer coverage for
- `ParseInt64()`
- `ParseInt32()`
- `ParseUInt64()`
- `ParseUInt32()`
but not `ParseUInt8()`, so this pull adds it.
I was tempted to add a commit that applies clang formatting to the file, or one that updates the C-style casts to named casts, but resisted the temptation unless reviewers request it.
ACKs for top commit:
laanwj:
Code review ACK 76782e560b
MarcoFalke:
cr ACK 76782e560b
Tree-SHA512: 1d7948b3385632094a3b0f0e38f87dccddabf74002e68aa055a51408866b057828ffa15c4b22aa9adde458155fbb5e443b66a9dbf3d7713358fc98a14d64bdcf
52f0be3a93 compat: remove memcpy -> memmove backwards compatibility alias (fanquake)
Pull request description:
In glib 2.13 memcpy was changed such that the way it copied bytes was reversed.
This caused all sorts of issues for existing software, which depended on the
existing behavior (when they should have been using memmove). See:
https://sourceware.org/bugzilla/show_bug.cgi?id=12518https://bugzilla.redhat.com/show_bug.cgi?id=638477
Now that we require glibc 2.17+ (#17538), we should be well clear of having to
maintain our memcpy -> memmove aliasing, which was introduced in #4339.
Gitian builds:
```bash
# Linux:
52dee59c8c7d5620ac9b140b79fcaf3d2f15a219293140190f9283ba871f5391 bitcoin-52f0be3a9332-aarch64-linux-gnu-debug.tar.gz
8963473b8791c5c6033a992d7dd761832fe1fb5732be790a6e9f8c11d67ad8ae bitcoin-52f0be3a9332-aarch64-linux-gnu.tar.gz
1fb3365c1ef60ecd1eb2d18f671f8f1e8cde0585de7de74aa0c5121093100c26 bitcoin-52f0be3a9332-arm-linux-gnueabihf-debug.tar.gz
305c5b032d51ba97459715211112204a09d119edd6ec2a12b796559ad3fde761 bitcoin-52f0be3a9332-arm-linux-gnueabihf.tar.gz
1f950a3e3979a4e1a67696b3fddc3090a0489a43b49e2b58a348d4b02ada2aa8 bitcoin-52f0be3a9332-powerpc64-linux-gnu-debug.tar.gz
0b9731dba768b30c91dadec4cd7a98c86e06fbf6354555f798b46b7c4fab7b5f bitcoin-52f0be3a9332-powerpc64-linux-gnu.tar.gz
c4a37aae56cc023964f8d9e82d1b66913079cab559cbfc1c9127969aa968a06f bitcoin-52f0be3a9332-powerpc64le-linux-gnu-debug.tar.gz
dfbaa4f3bf12988a0a7f82c4b10162e5e7a63382a7e29d0170bc32ce344c97c3 bitcoin-52f0be3a9332-powerpc64le-linux-gnu.tar.gz
3a0280d2c06516e50b0841d6f42d9589355dc9a1f8bb9a0b123554cd91b08004 bitcoin-52f0be3a9332-riscv64-linux-gnu-debug.tar.gz
cc199a0f254b2366e80a6a884120ec3ea442983990ba1a5eb993c36060686eba bitcoin-52f0be3a9332-riscv64-linux-gnu.tar.gz
eb8e7ca673cc06c167ab082fe457a41f73758ecd5b34941300e3cd378c29b197 bitcoin-52f0be3a9332-x86_64-linux-gnu-debug.tar.gz
dad19226c0e4c54b78ca2fa85fc28c5bfd1e1178e3f765472bd2f895a1d57145 bitcoin-52f0be3a9332-x86_64-linux-gnu.tar.gz
ef89be95b84bb7c6fef055634cd20caf2fa5b42441502918dbfbf758bb2daab6 src/bitcoin-52f0be3a9332.tar.gz
dc61f5ca33330c1609bc56b23f39fef3c1ff5ec6a1799d5b7a18f3c3b3acc9f9 bitcoin-core-linux-22-res.yml
```
Guix builds:
```bash
b50d6399cb59e5e4a9247b12a3eda61de6e51bd87ef1f27b388b75b71dfccf92 output/bitcoin-52f0be3a9332-aarch64-linux-gnu-debug.tar.gz
23d845dc13e60a581ebdfbaa6063f559a56cce06734e1b50790d2fc13e257793 output/bitcoin-52f0be3a9332-aarch64-linux-gnu.tar.gz
79094406fe00939bbce17a6d65de5a2686625e871432350c69e674cc80b1491c output/bitcoin-52f0be3a9332-arm-linux-gnueabihf-debug.tar.gz
65a91913249a743015eceea5a56c497d606af17270cb7e8a3df10cf729b757ec output/bitcoin-52f0be3a9332-arm-linux-gnueabihf.tar.gz
5e75ca5e8cf6934ba5a5a1b4d26c1b361b118e10ef34b73845d038035ddb9b85 output/bitcoin-52f0be3a9332-osx-unsigned.dmg
774b372696cde8ceab40f6909dadea3fc87b375b495fcfb4ee8a963afd7fbd3a output/bitcoin-52f0be3a9332-osx-unsigned.tar.gz
dc4bdfb7b32dcc0b6e876d6d7ab3cb8d1472f21f66546ab70515f96262292e21 output/bitcoin-52f0be3a9332-osx64.tar.gz
ea178ff9e28439f80129445cf260215c74eea2e610f62ff045061f287675d3ff output/bitcoin-52f0be3a9332-powerpc64-linux-gnu-debug.tar.gz
0390687a7aaa3f0a8a78be2deab21116599e5b332f00a2d1fdce97a5bd30e3eb output/bitcoin-52f0be3a9332-powerpc64-linux-gnu.tar.gz
52c948719a27f252f5969558abc2718c1e365ea85496322cb4ec97eab8a234cc output/bitcoin-52f0be3a9332-powerpc64le-linux-gnu-debug.tar.gz
5a4a8748dffe7e6a5bd07f3f564b1f2052440c4199fe25aaa41675bfb69e61db output/bitcoin-52f0be3a9332-powerpc64le-linux-gnu.tar.gz
ba521bd2b4e73aea317821a9e08da9a326c0be3b38d923b35ba14bc68ee6c814 output/bitcoin-52f0be3a9332-riscv64-linux-gnu-debug.tar.gz
783ea81ab2f6b642b13ebf7882aa822d12f95936574a8848a74b1b8978e6801d output/bitcoin-52f0be3a9332-riscv64-linux-gnu.tar.gz
376706fc12e58d7d559a87e1ce64be22eaac3fc32d95c60d603ad893d9128cc1 output/bitcoin-52f0be3a9332-win-unsigned.tar.gz
7aa48242fb71e29b00992b2be8677f1ea49f2ca82c5355bf0c1d4c8d14635596 output/bitcoin-52f0be3a9332-win64-debug.zip
41e6461ab573fa8f6ac0f198193e72a4a047bb7a4193f743b937e81739c929cc output/bitcoin-52f0be3a9332-win64-setup-unsigned.exe
e2c4ecb05f24577da12f722d848bf6ac89f3f549d6d2bfd30d65676099c0725b output/bitcoin-52f0be3a9332-win64.zip
60ed63b3b562fa2141f18f1556a03c2474b75797088cd68fdb3e7d057a6983a3 output/bitcoin-52f0be3a9332-x86_64-linux-gnu-debug.tar.gz
adb0bb62dc8b99d025a863e921b8e670f4c8f4b5600cd6d79eb552ede10bc8b8 output/bitcoin-52f0be3a9332-x86_64-linux-gnu.tar.gz
ef89be95b84bb7c6fef055634cd20caf2fa5b42441502918dbfbf758bb2daab6 output/src/bitcoin-52f0be3a9332.tar.gz
```
ACKs for top commit:
laanwj:
Concept and code review ACK 52f0be3a93
Tree-SHA512: 851634a633cc7d27b10f11436768f3695a7615d5850166c3718028c36d3a7dd56baa2dd1028f47802891703e9f5a1d382f559e388ecef2249e2004edc62d97bf
8348a3742b net: fix hSocket param in netbase.h::ConnectSocketDirectly() (Jon Atack)
e6bd74b2e5 net: move Doxygen docs from netbase.cpp to netbase.h (Jon Atack)
12cc5704db net: update incorrect Doxygen documentation in netbase.cpp (Jon Atack)
Pull request description:
While doing #21328, I noticed docs that were out-of-date or in the wrong file.
The second commit is essentially move-only and is best reviewed with `colorMoved = dimmed-zebra` and `colorMovedWs = allow-indentation-change`.
ACKs for top commit:
laanwj:
Code review ACK 8348a3742b
Tree-SHA512: 13dae4abd3009fc43dfffc98e0f7eebcd6ad02afdd6050a7685a2ad4e6aaad93480d93886a2d1bd2375c2439d426494e4a8bc0c60e0e3104bfaa1830831ca663
