d5800da5199527a366024bc80cad7fcca17d5c4a [test] Remove final references to mininode (John Newbery)
5e8df3312e47a73e747ee892face55ed9ababeea test: resort imports (John Newbery)
85165d4332b0f72d30e0c584b476249b542338e6 scripted-diff: Rename mininode to p2p (John Newbery)
9e2897d020b114a10c860f90c5405be029afddba scripted-diff: Rename mininode_lock to p2p_lock (John Newbery)
Pull request description:
New contributors are often confused by the terminology in the test framework, and what the difference between a _node_ and a _peer_ is. To summarize:
- a 'node' is a bitcoind instance. This is the thing whose behavior is being tested. Each bitcoind node is managed by a python `TestNode` object which is used to start/stop the node, manage the node's data directory, read state about the node (eg process status, log file), and interact with the node over different interfaces.
- one of the interfaces that we can use to interact with the node is the p2p interface. Each connection to a node using this interface is managed by a python `P2PInterface` or derived object (which is owned by the `TestNode` object). We can open zero, one or many p2p connections to each bitcoind node. The node sees these connections as 'peers'.
For historic reasons, the word 'mininode' has been used to refer to those p2p interface objects that we use to connect to the bitcoind node (the code was originally taken from the 'mini-node' branch of https://github.com/jgarzik/pynode/tree/mini-node). However that name has proved to be confusing for new contributors, so rename the remaining references.
ACKs for top commit:
amitiuttarwar:
ACK d5800da519
MarcoFalke:
ACK d5800da5199527a366024bc80cad7fcca17d5c4a 🚞
Tree-SHA512: 2c46c2ac3c4278b6e3c647cfd8108428a41e80788fc4f0e386e5b0c47675bc687d94779496c09a3e5ea1319617295be10c422adeeff2d2bd68378e00e0eeb5de
dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 refactor: test: use _ variable for unused loop counters (Sebastian Falbesoner)
Pull request description:
This tiny PR substitutes Python loops in the form of `for x in range(N): ...` by `for _ in range(N): ...` where applicable. The idea is indicating to the reader that a block (or statement, in list comprehensions) is just repeated N times, and that the loop counter is not used in the body, hence using the throwaway variable. This is already done quite often in the current tests (see e.g. `$ git grep "for _ in range("`). Another alternative would be using `itertools.repeat` (according to Python core developer Raymond Hettinger it's [even faster](https://twitter.com/raymondh/status/1144527183341375488)), but that doesn't seem to be widespread in use and I'm not sure about a readability increase.
The only drawback I see is that whenever one wants to debug loop iterations, one would need to introduce a loop variable again. Reviewing this is basically a no-brainer, since tests would fail immediately if a a substitution has taken place on a loop where the variable is used.
Instances to replace were found by `$ git grep "for.*in range("` and manually checked.
ACKs for top commit:
darosior:
ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64
instagibbs:
manual inspection ACK dac7a111bd
practicalswift:
ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 -- the updated code is easier to reason about since the throwaway nature of a variable is expressed explicitly (using the Pythonic `_` idiom) instead of implicitly. Explicit is better than implicit was we all know by now :)
Tree-SHA512: 5f43ded9ce14e5e00b3876ec445b90acda1842f813149ae7bafa93f3ac3d510bb778e2c701187fd2c73585e6b87797bb2d2987139bd1a9ba7d58775a59392406
8888bd43c100f9f0ca1122fcc896fb7b999d61c6 Remove redundant nLastTry check (MarcoFalke)
00001e57fe74c061aa9cbc72b07252335cb566e0 Remove redundant nTime checks (MarcoFalke)
Pull request description:
Split out from https://github.com/bitcoin/bitcoin/pull/24697 because it makes sense on its own.
ACKs for top commit:
dergoegge:
re-ACK 8888bd43c100f9f0ca1122fcc896fb7b999d61c6
naumenkogs:
utACK 8888bd43c100f9f0ca1122fcc896fb7b999d61c6
Tree-SHA512: 32c6cde1c71e943c76b7991c2c24caf29ae467ab4ea2d758483a0cee64625190d1a833b468e8eab1f834beeb2c365af96552c14b05270f08cf63790e0707581d
292828cd7744ec7eadede4ad54aa2117087c5435 [test] Test addr cache for multiple onion binds (dergoegge)
3382905befd23364989d941038bf7b1530fea0dc [net] Seed addr cache randomizer with port from binding address (dergoegge)
f10e80b6e4fbc151abbf1c20fbdcc3581d3688f0 [net] Use ConnectedThroughNetwork() instead of GetNetwork() to seed addr cache randomizer (dergoegge)
Pull request description:
The addr cache id randomizer is currently supposed to be seeded with the network of the inbound connection and the local socket (only the address is used not the port): a8098f2cef/src/net.cpp (L2800-L2804)
For inbound onion connections `CNode::addr.GetNetwork()` returns `NET_UNROUTABLE` and `CNode::addrBind` is set to `127.0.0.1:<onion bind port>`. This results in the same addr cache for all inbound connections on 127.0.0.1 binds.
To avoid the same addr cache across all onion and other 127.0.0.1 binds, we should seed the addr cache randomizer with the correct network for inbound onion connections (using `CNode::ConnectedThroughNetwork()`) as well as the port of `CNode::addrBind`.
ACKs for top commit:
sipa:
utACK 292828cd7744ec7eadede4ad54aa2117087c5435
mzumsande:
Code Review ACK 292828cd7744ec7eadede4ad54aa2117087c5435
naumenkogs:
utACK 292828cd7744ec7eadede4ad54aa2117087c5435
Tree-SHA512: d0be13bab6bc121c2926d4b168687f6c2ed4ce0c9dd19be71eb4886adeba8afc3daacdc4e232a0ba3b03a89d69b618abc5595b69abd1ad0c476d825bc6ea1f9f
52a797bfe5ced4329f2272be417c35730ec8839f doc: Avoid ADL for function calls (Hennadii Stepanov)
Pull request description:
It happened two times recently, when [ADL](https://en.cppreference.com/w/cpp/language/adl) popped up unexpectedly and brought some confusion:
- https://github.com/bitcoin/bitcoin/pull/24338/files#r805989994
> Any idea why this even compiles?
- https://www.erisian.com.au/bitcoin-core-dev/log-2022-02-18.html#l-51:
> 2022-02-18T03:24:14 \<dongcarl\> Does anyone know why this compiles? 6d3d2caa37
> 2022-02-18T03:24:14 \<dongcarl\> GetUTXOStatsWithHasher and MakeUTXOHasher are both in the `kernel::` namespace and I never added a `using` declaration on top...
> 2022-02-18T03:25:53 \<sipa\> https://en.cppreference.com/w/cpp/language/adl ?
Let's document our intention to avoid similar cases in the future.
ACKs for top commit:
laanwj:
Anyhow, ACK 52a797bfe5ced4329f2272be417c35730ec8839f, there is no need to hold merge up on this, documenting it is a step forward.
Tree-SHA512: f52688b5d8f6130302185206ec6ea4731b099a75294ea2d477901a52d6d58473e3427e658aea408c140c2824c37a0399ec7376aded2a91197895ea52d51f0018
ebfc308ea4b8851118e8194d837556bf443c329c test: add coverage for non-hex value to -minimumchainwork (brunoerg)
Pull request description:
This PR adds test coverage for the following init error:
b9ef5a10e2/src/init.cpp (L917-L919)
Passing a non-hex value to -minimumchainwork should throw an initial error.
ACKs for top commit:
laanwj:
Code review ACK ebfc308ea4b8851118e8194d837556bf443c329c
kristapsk:
re-ACK ebfc308ea4b8851118e8194d837556bf443c329c
Tree-SHA512: c665903757ae3b8b2480df97bb888e60ba4387b009fcb8031041822e87a155a0e4950ebe79873c1034f0826504521d82b1fdfdb5e8378b227d14ca545b8d4e11
151009cf76f3f1adc17630d5370bf019be127373 qt, wallet, refactor: Drop unused `WalletModel::PaymentRequestExpired` (Hennadii Stepanov)
Pull request description:
The `PaymentRequestExpired` value in the `WalletModel::StatusCode` enumeration has been unused since bitcoin/bitcoin#17165.
ACKs for top commit:
furszy:
ACK 151009cf, no usage for it.
kristapsk:
cr ACK 151009cf76f3f1adc17630d5370bf019be127373, checked that `PaymentRequestExpired` is not referenced anywhere else.
Tree-SHA512: c2ea3443af5d369ca294d79559869f688aaa806b91ffe0090f3b34638a8377ec2f11d6f5c09cc2d11ab55035850237e60e992acba671097a6642c6bb9e709273
fa27ee88edbf696b7eef2efbfcf1446ec522fd85 Get time less often in AddrManImpl::ResolveCollisions_() (MarcoFalke)
Pull request description:
First commit split out from https://github.com/bitcoin/bitcoin/pull/24697
ACKs for top commit:
sipa:
utACK fa27ee88edbf696b7eef2efbfcf1446ec522fd85
fanquake:
ACK fa27ee88edbf696b7eef2efbfcf1446ec522fd85
Tree-SHA512: 40c8594d2a5ce02a392ac5f9f120c24c6bcd495b0bcc901fd6064dde9f6123cd109504cee7b612a9555b70cfd7759cbd6cd496d007bb374c27610d01b464191c
7036cf52aa080d2a63993c2298555252d507dd2f Delete UpdatePackagesForAdded at beginning of addPackageTxs. (KevinMusgrave)
Pull request description:
In `CreateNewBlock` (in miner.cpp), `inBlock` is cleared before `addPackageTxs`, so `inBlock` will be empty in the first call to `UpdatePackagesForAdded`. I saw this brought up in these [PR review club logs](https://bitcoincore.reviews/24538) and there didn't seem to be a definitive answer for why the call is necessary. There's also an [old PR](https://github.com/bitcoin/bitcoin/pull/10200) where this change was going to be applied, but it got closed.
If `addPackageTxs` can be called when `inBlock` is not empty, then maybe a test should be added for that case. All the tests seem to pass with this deletion.
ACKs for top commit:
glozow:
utACK 7036cf52aa080d2a63993c2298555252d507dd2f
Tree-SHA512: 9e757b71b9035f68a0c6fef229b8cd83f1bdbe23f05bb02cc1bab8c3c177805b388bceb2bb1f0bce354791ccb29f351a6c51979b96ffe4d9fc6c978f83e36afc
ff4a38a32766942ce5c4be6d6510f800a9f8e0d9 build: Fix configuring depends with cmake (Hennadii Stepanov)
Pull request description:
This PR fixesbitcoin/bitcoin#24389.
On master (28aa0e3ca0a6cfeb5b2b63929d4bc58de6ee6f02) configuring of the `libmultiprocess` package for the `x86_64-w64-mingw32` target fails:
```
$ cd depends
$ make libmultiprocess_configured MULTIPROCESS=1 HOST=x86_64-w64-mingw32
Configuring libmultiprocess...
CMake Warning:
No source or binary directory provided. Both will be assumed to be the
same as the current working directory, but note that this warning will
become a fatal error in future CMake releases.
-- The CXX compiler identification is GNU 9.3.0
-- Check for working CXX compiler: /usr/bin/x86_64-w64-mingw32-g++-posix
-- Check for working CXX compiler: /usr/bin/x86_64-w64-mingw32-g++-posix -- broken
CMake Error at /usr/share/cmake-3.16/Modules/CMakeTestCXXCompiler.cmake:53 (message):
The C++ compiler
"/usr/bin/x86_64-w64-mingw32-g++-posix"
is not able to compile a simple test program.
It fails with the following output:
Change Dir: /home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/CMakeFiles/CMakeTmp
Run Build Command(s):/usr/bin/make cmTC_93273/fast && make[1]: Entering directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/CMakeFiles/CMakeTmp'
/usr/bin/make -f CMakeFiles/cmTC_93273.dir/build.make CMakeFiles/cmTC_93273.dir/build
make[2]: Entering directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/CMakeFiles/CMakeTmp'
Building CXX object CMakeFiles/cmTC_93273.dir/testCXXCompiler.cxx.o
/usr/bin/x86_64-w64-mingw32-g++-posix -I/home/hebasto/GitHub/bitcoin/depends/x86_64-w64-mingw32/include -pipe -O2 -o CMakeFiles/cmTC_93273.dir/testCXXCompiler.cxx.o -c /home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/CMakeFiles/CMakeTmp/testCXXCompiler.cxx
Linking CXX executable cmTC_93273
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_93273.dir/link.txt --verbose=1
/usr/bin/x86_64-w64-mingw32-g++-posix -I/home/hebasto/GitHub/bitcoin/depends/x86_64-w64-mingw32/include -pipe -O2 -L/home/hebasto/GitHub/bitcoin/depends/x86_64-w64-mingw32/lib -rdynamic CMakeFiles/cmTC_93273.dir/testCXXCompiler.cxx.o -o cmTC_93273
x86_64-w64-mingw32-g++-posix: error: unrecognized command line option ‘-rdynamic’
make[2]: *** [CMakeFiles/cmTC_93273.dir/build.make:87: cmTC_93273] Error 1
make[2]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/CMakeFiles/CMakeTmp'
make[1]: *** [Makefile:121: cmTC_93273/fast] Error 2
make[1]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/CMakeFiles/CMakeTmp'
CMake will not be able to correctly generate this project.
Call Stack (most recent call first):
CMakeLists.txt:6 (project)
-- Configuring incomplete, errors occurred!
See also "/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/CMakeFiles/CMakeOutput.log".
See also "/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/CMakeFiles/CMakeError.log".
make: *** [funcs.mk:283: /home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-w64-mingw32/libmultiprocess/d576d975debdc9090bd2582f83f49c76c0061698-f496b1e64cb/./.stamp_configured] Error 1
```
The reason of that failure is the unset `-DCMAKE_SYSTEM_NAME` flag:
```
$ make print-libmultiprocess_cmake MULTIPROCESS=1 HOST=x86_64-w64-mingw32
libmultiprocess_cmake=env CC="x86_64-w64-mingw32-gcc" CFLAGS=" -I/home/hebasto/GitHub/bitcoin/depends/x86_64-w64-mingw32/include -pipe -O2 " CXX="x86_64-w64-mingw32-g++-posix" CXXFLAGS=" -I/home/hebasto/GitHub/bitcoin/depends/x86_64-w64-mingw32/include -pipe -O2 " LDFLAGS=" -L/home/hebasto/GitHub/bitcoin/depends/x86_64-w64-mingw32/lib " cmake -DCMAKE_INSTALL_PREFIX:PATH="/home/hebasto/GitHub/bitcoin/depends/x86_64-w64-mingw32" -DCMAKE_SYSTEM_NAME= -DCMAKE_C_COMPILER_TARGET=x86_64-w64-mingw32 -DCMAKE_CXX_COMPILER_TARGET=x86_64-w64-mingw32
```
This PR fixes this error:
```
$ make libmultiprocess_configured MULTIPROCESS=1 HOST=x86_64-w64-mingw32
$ # no errors
```
ACKs for top commit:
fanquake:
ACK ff4a38a32766942ce5c4be6d6510f800a9f8e0d9 - going to merge this now, and we can follow up with more cmake improvements.
Tree-SHA512: bd8d8b2f4eedcc8c46cf995b9c39493ea4d0b13c224f77ef62985304ebd392f05119043a06f1401c64f962007a8faa4bb53715d99a408ee6c33bb49a2dd650ba
0000a63689036dc4368d04c0648a55fdf507932f Simplify GetTime (MarcoFalke)
Pull request description:
The implementation of `GetTime` is confusing:
* The value returned by `GetTime` is assumed to be equal to `GetTime<std::chrono::seconds>()`. Both are mockable and the only difference is return type, the value itself is equal. However, the implementation does not support this assumption.
* On some systems, `time_t` might be a signed 32-bit integer (https://en.cppreference.com/w/c/chrono/time), thus breaking in the year 2038, whereas `GetTime<std::chrono::seconds>` does not. Also, `time_t` might be `-1` "on error", where "error" is unspecified.
* `GetTime<std::chrono::seconds>` calls `GetTimeMicros`, which calls `GetSystemTime`, which calls `std::chrono::system_clock::now`, which doesn't have the above issues. See https://en.cppreference.com/w/cpp/chrono/system_clock/now
* `GetTimeMicros` and the internal-only `GetSystemTime` will likely be renamed (to clarify they are the non-mockable non-monotonic system time) or removed in the future to be replaced by appropriate `std::chrono::time_point<Clock>` getters.
Fix all issues by:
* making `GetTime()` an alias for `GetTime<std::chrono::seconds>().count()`.
* inlining the needed parts of `GetSystemTime` directly instead of needlessly increasing the function call stack with functions that are likely to be removed in the future.
ACKs for top commit:
martinus:
Code review, untested ACK 0000a63689036dc4368d04c0648a55fdf507932f. By the way strictly speaking `std::chrono::system_clock` is only guaranteed to be based on the unix epoch starting with C++20: https://en.cppreference.com/w/cpp/chrono/system_clock
theStack:
Code-review ACK 0000a63689036dc4368d04c0648a55fdf507932f
Tree-SHA512: f751ba740e0da65537be800e9414dd02282d9f04c0b0fb986a36546f257d0b888d8688653cdda5d355ec832c0e09d866922d9161b1ccd33485c1c92c5d1e802f
01e121d29087db047e4bc01bd64d054f83cfc5df depends: fix capnp's descriptor for make download (Cory Fields)
Pull request description:
The non-native capnp was trying to fetch the wrong file.
Without this, "make -C depends MULTIPROCESS=1 download" is broken.
Presumably it breaks with the download target because the dependency graph is flattened. It manages to work if native_capnp is encountered first because it will then be found in the cache.
ACKs for top commit:
gruve-p:
tACK 01e121d290
hebasto:
ACK 01e121d29087db047e4bc01bd64d054f83cfc5df, tested on Linux Mint 20.2 (x86_64).
Tree-SHA512: 2605d895f3799be5a311f6f7d36a5c13cdb715dc148915ad818f4afc7d5de92cd6b8ecd34ff2b21cef6743b090819bba1e3353096cfb5659c55f76113ce5adf3
6d7e46ce23217da53ff52f535879c393c02fa2b2 Remove `Warning:` (Prayank)
Pull request description:
Reason: I noticed that `Warning` is printed 2 times in `-getinfo` while reviewing https://github.com/bitcoin/bitcoin/pull/21832#issuecomment-851004943
Same string is used for GUI, log and stderr. If we need to add `Warning:` in GUI or other place we can always prepend to this string.
CLI:
```
Warnings: Unknown new rules activated (versionbit 28)
```
GUI:
ACKs for top commit:
MarcoFalke:
review ACK 6d7e46ce23217da53ff52f535879c393c02fa2b2
Tree-SHA512: 25760cf6d850f6c2216d651fa46574d2d21a9d58f4577ecb58f9566ea8cd07bfcd6679bb8a1f53575e441b02d295306f492c0c99a48c909e60e5d977ec1861f6
735610940c0cac72343ca247eb3062b2e09512a0 build: set --build when configuring packages in depends (fanquake)
Pull request description:
After reading [#Specifying-Target-Triplets](https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/autoconf.html#Specifying-Target-Triplets) in the autoconf manuel, my understanding is that this change should mostly be a no-op, as `--build` defaults to the output of [`config.guess`](https://github.com/bitcoin/bitcoin/blob/master/depends/config.guess), however, this may be slightly more correct
> For historical reasons, whenever you specify --host, be sure to specify --build too; this will be fixed in the future.
and will quell some warnings in depends (#16354), i.e:
> configure: WARNING: If you wanted to set the --build type, don't use --host.
If a cross compiler is detected then cross compile mode will be used.
If anything, this also explicitly enables cross-compilation mode when `--host` differs from `--build`. As for "fixed in the future", this is the case for Autoconf 2.70+.
If we don't make this change, I think we should consider closing #16354. We've now got a good handle on our depends configure flags, and we can just leave the last few warnings as they are. It's also unlikely that we are going to add the `xorg-macros` package to depends.
Guix builds at 735610940c0cac72343ca247eb3062b2e09512a0:
```bash
bash-5.1# find output -type f -name *$(git rev-parse --short HEAD)*.* -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
afabf99650d57abd3df2c7e6253a3281f9814ada77d01c28ffb57c740b14fd8e output/bitcoin-735610940c0c-aarch64-linux-gnu-debug.tar.gz
6e9dedced61489f7734c5387ee05823db68eb4b890fa05f11541c1b966576446 output/bitcoin-735610940c0c-aarch64-linux-gnu.tar.gz
5333a3c5978a61966251e56fedab8e059d9d8a1a7231a0908095bb373204a81f output/bitcoin-735610940c0c-arm-linux-gnueabihf-debug.tar.gz
d28e1c166b96e58cf40b3f9801c3050c2785b25eeb261d44eeb31a32a75e134c output/bitcoin-735610940c0c-arm-linux-gnueabihf.tar.gz
e00b0149c52e9207be1cfb56ec3b715b55f41691401095f7a7038ac124f08d80 output/bitcoin-735610940c0c-osx-unsigned.dmg
ae5052ad1a05916bc6656b034c169097471fd9bdba16b606e2bcb592c9395544 output/bitcoin-735610940c0c-osx-unsigned.tar.gz
8586a9b8b82a8832397fcd0756aa9649adc3e61ae579fb370b99fe406298199f output/bitcoin-735610940c0c-osx64.tar.gz
d560a8781bae856b6593e9b731fa99de0a4ad7bd3e03084166dc98904ece9e7b output/bitcoin-735610940c0c-powerpc64-linux-gnu-debug.tar.gz
a24cf28b7efd5ebdc892e7e8809a05b3405bb4222f7d2eaa32de265214c78f28 output/bitcoin-735610940c0c-powerpc64-linux-gnu.tar.gz
ad29421842dbbe96526eb597a7ff2970f0acae8c3e75dc6c2100716a2ba20606 output/bitcoin-735610940c0c-powerpc64le-linux-gnu-debug.tar.gz
ee7a8ccfd68cc2297bb96eaaacf7b3eb939a5e39cecc3c710dbe828518f1bfbd output/bitcoin-735610940c0c-powerpc64le-linux-gnu.tar.gz
185bf4ea3b35072a8dd54aedbafc0892ec1e486c8ebd311f05f0fb47a19058cf output/bitcoin-735610940c0c-riscv64-linux-gnu-debug.tar.gz
803078b15a9bbaa567176827639895e762edbe80844b80400f8a9581e5311d32 output/bitcoin-735610940c0c-riscv64-linux-gnu.tar.gz
d920327a32a42a5abadd247f0d3f3ee0ab0488a28892c11838d99bede6456723 output/bitcoin-735610940c0c-win-unsigned.tar.gz
9f1741a37e294af153d57ad4a6dad3e5f1cdf75f1588c86cb467abf0c177ae27 output/bitcoin-735610940c0c-win64-debug.zip
2d1a91552c0dd7bb400ce6c71d2e98de702743a97afe5f4b0945865f251f3aea output/bitcoin-735610940c0c-win64-setup-unsigned.exe
8a70020bb0cb68516acf4b2cc89832d72373f31611bd075826a30a817a071cd2 output/bitcoin-735610940c0c-win64.zip
93b03837679474905538e96c44ba81ee5653795ad333dc00569b92c82dc6ea31 output/bitcoin-735610940c0c-x86_64-linux-gnu-debug.tar.gz
6ccd046a83ef5089667ad426875319beb83e2b2a5fb4d4459d5c5ce8d75f1a31 output/bitcoin-735610940c0c-x86_64-linux-gnu.tar.gz
12771acb17de84cda8e81c59704c00b1d4725ff51fa84c9067dc61a8dc795bdc output/src/bitcoin-735610940c0c.tar.gz
```
Gitian builds:
```bash
# macOS:
7d087749f610e5e3eca91c730d41afd4d404808e276242d05cedb89403de247f bitcoin-735610940c0c-osx-unsigned.dmg
96d68b4f0042dc40f5e8e362fa84dcf8e122dcb566889feb48ad8a982847dd8b bitcoin-735610940c0c-osx-unsigned.tar.gz
2dea72cbe5c3e228babd88b516a6f43b0e040e9510ea95b62f07fc0815bf8d22 bitcoin-735610940c0c-osx64.tar.gz
12771acb17de84cda8e81c59704c00b1d4725ff51fa84c9067dc61a8dc795bdc src/bitcoin-735610940c0c.tar.gz
478dbda9d551a8bed985c61c625df3f543afab26989a52bc44d5a4745715af81 bitcoin-core-osx-22-res.yml
# Windows:
c31a5118dfa21259b9868e2c4858c635becb786c2ee8af92302da7811a1efe9b bitcoin-735610940c0c-win-unsigned.tar.gz
5c171ec3aee830abc8ae843d674d5fe7a57420e782ee765b6992914ae5b7671a bitcoin-735610940c0c-win64-debug.zip
ca010ced982f75d111e01f3759b961deb83e7bf511b1ee9c9041f4830570f2a0 bitcoin-735610940c0c-win64-setup-unsigned.exe
62514b01281f81af6057fb12cc5e3813c1d8f6532d0d8ac8f6f2909a595aa1dd bitcoin-735610940c0c-win64.zip
12771acb17de84cda8e81c59704c00b1d4725ff51fa84c9067dc61a8dc795bdc src/bitcoin-735610940c0c.tar.gz
e26b55d8302a419f594396db6ff441aaeb5e275e02fde7b1b4eb092270f902e7 bitcoin-core-win-22-res.yml
# Linux:
e5be4deb91cb372de484468c0575a04f907f982e35a9841659bb213540f770d0 bitcoin-735610940c0c-aarch64-linux-gnu-debug.tar.gz
6169e822679ece2bb6af7027362a6f373eb939eef9d89edc58c21190b4083c7d bitcoin-735610940c0c-aarch64-linux-gnu.tar.gz
aee5308767c5c930853a80ab94325b2a4ab3dbe9a7a20c7a9d60d57af2b5383f bitcoin-735610940c0c-arm-linux-gnueabihf-debug.tar.gz
a9cf6e56d859fffd5a2b5f1e6360515bc6b06b048776b93cc08954859457a251 bitcoin-735610940c0c-arm-linux-gnueabihf.tar.gz
7617d85c358e6614fd40026a8d346f9edba5b5139e3daade2ec2aec72aed3a1f bitcoin-735610940c0c-powerpc64-linux-gnu-debug.tar.gz
f76e46a8b130812541ebcaaba173b4cbe2ec1781e348579b44f96e9244cc2475 bitcoin-735610940c0c-powerpc64-linux-gnu.tar.gz
19ada7abb801422bd7772d7ee559004cab11a46141a747aabd27085e8c3d200d bitcoin-735610940c0c-powerpc64le-linux-gnu-debug.tar.gz
f3a6bbefe9f47b3960e3b9eeb3f740ecddf8773ae76961ced6a3cdf82503e37f bitcoin-735610940c0c-powerpc64le-linux-gnu.tar.gz
42ca333e8f82b4ec0b2354efc7c4f1c27ed32d60c35b88e33d26833419f19e69 bitcoin-735610940c0c-riscv64-linux-gnu-debug.tar.gz
dde930d9134fb5be1374014db3f6f503b614ae801a1d77f6f4b82a5f11eb2518 bitcoin-735610940c0c-riscv64-linux-gnu.tar.gz
82e1b4cc8aac309a99458cc27fd0285215a8af53780094c4990303ce8948cf0c bitcoin-735610940c0c-x86_64-linux-gnu-debug.tar.gz
ee8caad2ccddd8eaf8aa2d19040d859130168183355b09a2a0f77a59a97fcaee bitcoin-735610940c0c-x86_64-linux-gnu.tar.gz
12771acb17de84cda8e81c59704c00b1d4725ff51fa84c9067dc61a8dc795bdc src/bitcoin-735610940c0c.tar.gz
d90545b13226a338c994d9e538f468a6fafd694273bf71e14614feeaf5727ad4 bitcoin-core-linux-22-res.yml
```
ACKs for top commit:
dongcarl:
Code Review ACK 735610940c0cac72343ca247eb3062b2e09512a0
hebasto:
ACK 735610940c0cac72343ca247eb3062b2e09512a0, tested on Linux Mint 20.1 (x86_64).
Tree-SHA512: 13ce05269309a12d03b81a539ba5585003785f7b1bb7894a9353385a6e9c709f920cc4f79322f28d7a256809754f201e70be6b917915c7b14e1461530075781d
## Issue being fixed or feature implemented
After bitcoin-core/gui#204 is wrong column is stretched as mentioned in
#5829
It's not the last column that must be stretched, it's the one before it.
The size of "Address / Label" column should also be adjusted accordingly
on window resize. gui#204 broke this behaviour.
## What was done?
This PR uses QT internal features to aim same behaviour as before
gui-204
## How Has This Been Tested?
Run, resize window - seems as proper column changing size.
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
c133cdcdc3397a734d57e05494682bf9bf6f4c15 Cap listsinceblock target_confirmations param (Adam Stein)
Pull request description:
This addresses an issue brought up in #19587.
Currently, the `target_confirmations` parameter to `listsinceblock` is not checked for being too large. When `target_confirmations` is greater than one more than the current number of blocks, `listsinceblock` fails with error code -1. In comparison, when `target_confirmations` is less than 1, a -8 "Invalid parameter" error code is thrown.
This PR fixes the issue by returning a -8 "Invalid parameter" error if the `target_confirmations` value corresponds to a block with more confirmations than the genesis block. This happens if `target_confirmations` exceeds one more than the number of blocks.
ACKs for top commit:
laanwj:
Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15
ryanofsky:
Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15. Just suggested changes since last review. Thanks!
Tree-SHA512: 02680f4cb937d2c24d5019abd0ebfa188b8a50679a1e64e9c26bfe5c17eef6aea906832e6e2d492ba8a2ea160041bf185d66795ee691e340f6793db03c21b89a
01cd24c22606408d5c0ac74c9a2c5d85eff77846 doc: set CC_FOR_BUILD when building on OpenBSD (fanquake)
Pull request description:
Closes: #19559
While #19559 has been fixed upstream, it makes sense to not only
recommend using `CC_FOR_BUILD`here until the fix is pulled in as
part of our next libsecp update, but after discussing with Cory,
he suggested we should be setting this on OpenBSD (which still has
the an ancient GCC) regardless.
ACKs for top commit:
real-or-random:
ACK 01cd24c22606408d5c0ac74c9a2c5d85eff77846 I looked at the diff (but can't test the instructions on OpenBSD)
laanwj:
Code review ACK 01cd24c22606408d5c0ac74c9a2c5d85eff77846
Tree-SHA512: 322802b9303771f1be2ad9628f268dfa71dc7ee77948fa2a34f21eceb19b2d8efdd8876c8f0778adbfcde48fa0f88cd4e698ae425428159abca38e8c7980da1d
0a8aa626dd69a357e1b798b07b64cf4177a464a3 refactor: Make HexStr take a span (Wladimir J. van der Laan)
Pull request description:
Make `HexSt`r take a span of bytes, instead of an awkward pair of templated iterators. This simplifies most of the uses.
ACKs for top commit:
elichai:
Code review ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3
hebasto:
re-ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3
jonatack:
re-ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3
Tree-SHA512: 6e178ece5cbac62119c857a10299b1e85422938084c3f03063e17119a5129e0c28016e05a6fabaa4c271a7e0a37c7cd89fa47c435ee19b38a5acfe80d00de992
9f88ded82b2898ca63d44c08072f1ba52f0e18d7 test addition of unknown segwit spends to txid reject filter (Gregory Sanders)
7989901c7eb62ca28b3d1e5d5831041a7267e495 Add txids with non-standard inputs to reject filter (Suhas Daftuar)
Pull request description:
Our policy checks for non-standard inputs depend only on the non-witness
portion of a transaction: we look up the scriptPubKey of the input being
spent from our UTXO set (which is covered by the input txid), and the p2sh
checks only rely on the scriptSig portion of the input.
Consequently it's safe to add txids of transactions that fail these checks to
the reject filter, as the witness is irrelevant to the failure. This is helpful
for any situation where we might request the transaction again via txid (either
from txid-relay peers, or if we might fetch the transaction via txid due to
parent-fetching of orphans).
Further, in preparation for future witness versions being deployed on the
network, ensure that WITNESS_UNKNOWN transactions are rejected in
AreInputsStandard(), so that transactions spending v1 (or greater) witness
outputs will fall into this category of having their txid added to the reject
filter.
ACKs for top commit:
ajtowns:
ACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7 - code review
jnewbery:
Code review ACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7
ariard:
Code Review/Tested ACK 9f88ded
naumenkogs:
utACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7
jonatack:
ACK 9f88ded82b2
Tree-SHA512: 1e93c0a5b68cb432524780ffc0093db893911fdfed9e2ed17f888e59114cc75d2a07062aefad4e5ce2e87c9270886117a8abb3c78fb889c9b9f31967f1777148
308caf326db5619141f0c224fa48410293d59330 doc: remove version number from bips.md (fanquake)
Pull request description:
This always just needs "bumping" (see previous rc type pulls), and the version number is already whichever version of the code you acquired bips.md with.
ACKs for top commit:
MarcoFalke:
lgtm ACK 308caf326db5619141f0c224fa48410293d59330
achow101:
ACK 308caf326db5619141f0c224fa48410293d59330
theStack:
ACK 308caf326db5619141f0c224fa48410293d59330
hebasto:
ACK 308caf326db5619141f0c224fa48410293d59330
Tree-SHA512: fcb98e7cdc0c1f8960bfba86be09c2badb36b613060fae394a56e1561c69d28f433434f573c8b1ae1d71ae326277dea2a4841d5c08ad39f8e8848300743146e7
ae4958be95a1158de9992a8e43ce032d87c74f13 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
c251d710a4c2981c6d52362a9a89db84da3d4a67 p2p, refactoring: use CInv helpers in net_processing.cpp (Jon Atack)
4254cd9f8f2437a916b06db4d925ce4eff8c94b9 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 c251d710a4c2981c6d52362a9a89db84da3d4a67
laanwj:
Code review ACK c251d710a4c2981c6d52362a9a89db84da3d4a67
vasild:
ACK c251d71
theStack:
Code-Review ACK c251d710a4c2981c6d52362a9a89db84da3d4a67
hebasto:
ACK c251d710a4c2981c6d52362a9a89db84da3d4a67, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: ead034b3c9e438909b4c5010c570d7930e69063c114290b051b7cebfd9bd5b19f573218bebe8a521256d32e830797f997adad3d85b4539c64ac5762b698e656d
fae656144e9ec25a0b610cd5278c1b7c42880097 travis: Re-enable s390x (MarcoFalke)
Pull request description:
According to travis, the issue has been solved. Quote
> I would like to confirm that we have resolved this issue and most of our users are reported that this issue has been resolved on their end as well. Could you please re-check and see if that still exists for you?
ACKs for top commit:
theStack:
ACK fae656144e
Tree-SHA512: cf42f96d25474a9dcf0817a049e30e29714731d708f73c40a3042b0c70a71ff08f07dd96a89f0dcd5a50a63a355cf30b3511172a32b8af7d5a2e13ad222a4b49
80968cf scripted-diff: rename movie folder to animation (Peter Bushnell)
Pull request description:
Rename the movies directory and RES_MOVIES make variable to animation and RES_ANIMATION respectively. Movies is a bit of an unexpected term to be found.
ACKs for top commit:
MarcoFalke:
ACK 80968cf
hebasto:
ACK 80968cf, tested on Linux Mint 20 (Qt 5.12.8).
Tree-SHA512: 6bd31ce36e821f6a1bef8a7972086a2387d6258c48fc9df12d3ffdae07d0237036afbc2dec673384b78d9567b91d6e12eafa59fa2305aa79153dfd9b7c3a8655
784ef8be41c7e5130a6b063b359031ee1ce75aff gui: Show permissions instead of whitelisted (Wladimir J. van der Laan)
Pull request description:
Show detailed permissions instead of legacy "whitelisted" flag in the peer list details.
These are formatted with `&` in between just like services flags. It reuses the "N/A" translation message if there are no special permissions.
This removes the one-but-last use of `legacyWhitelisted`.
Top commit has no ACKs.
Tree-SHA512: 11982da4b9d408c74bc56bb3c540c0eb22506be6353aa4d4d6c64461d140f0587be194e2daad1612fddaa2618025a856b33928ad89041558f418f721f6abd407
## Issue being fixed or feature implemented
Running 3 nodes on RegTest as platform does uses do not let to create
`llmq_test_instantsend` quorum:
```
1. switch to `llmq_test_instantsend`:
+ self.extra_args = [["-llmqtestinstantsenddip0024=llmq_test_instantsend"]] * 5
2. removed cycle-quorum related code:
- self.move_to_next_cycle()
- self.log.info("Cycle H height:" + str(self.nodes[0].getblockcount()))
- self.move_to_next_cycle()
- self.log.info("Cycle H+C height:" + str(self.nodes[0].getblockcount()))
- self.move_to_next_cycle()
- self.log.info("Cycle H+2C height:" + str(self.nodes[0].getblockcount()))
-
- self.mine_cycle_quorum(llmq_type_name='llmq_test_dip0024', llmq_type=103)
3. added new quorum:
+ self.mine_quorum(llmq_type_name='llmq_test_instantsend', llmq_type=104)
and eventually it stucked, no quorum happens
2024-01-13T19:18:49.317000Z TestFramework (INFO): Expected quorum_0 at:984
2024-01-13T19:18:49.317000Z TestFramework (INFO): Expected quorum_0 hash:6788e18f0235a5c85f3d3c6233fe132a80e74a2912256db3ad876a8ebf026048
2024-01-13T19:18:49.317000Z TestFramework (INFO): quorumIndex 0: Waiting for phase 1 (init)
<frozen>
```
## What was done?
Updated condition to enable "llmq_test_instantsend":
- it is RegTest and DIP0024 is not active
- it is RegTest, DIP0024 is active, and specified as
`llmqTypeDIP0024InstantSend`
## How Has This Been Tested?
Run unit and functional tests.
Beside that functional test feature_asset_locks.py now uses this quorum
for instant send and that's an arrow that hit 2 birds: we have test for
command line option `-llmqtestinstantsenddip0024` and code of
feature_asset_locks.py is simplified.
## Breaking Changes
yes, that's a bugfix that fix quorum `llmq_test_instantsend` absentance
on regtest after dip-0024 activation.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
## Issue being fixed or feature implemented
keybase.pub isn't a thing anymore; instead use thepasta.org; also
validate all .asc files
## What was done?
## How Has This Been Tested?
Validated 20.0.4, 20.0.3 and 20.0.2 with the script
## Breaking Changes
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: Wladimir J. van der Laan <laanwj@protonmail.com>
Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
## Issue being fixed or feature implemented
`llmq/utils` has simple util code that used all over code base and also
have too heavy code for calculation quorums such as:
`GetAllQuorumMembers`, `EnsureQuorumConnections` and other.
These helpers for calculation quorums are used only by
evo/deterministicmns, evo/simplifiedmns and llmq/* modules, but
llmq/utils is included in many other modules for various trivial
helpers.
## What was done?
Prior work:
- https://github.com/dashpay/dash/pull/5753
- #5486
See also #4798
This PR remove all non-quorum calculation code from llmq/utils.
Eventually it happens that easier to take everything out rather than
move Quorum Calculation to new place atm:
- new module llmq/options have a code related to various params, command
line options, spork-related etc
- llmq/utils is not included in various files which do not use any
llmq/utils code
- helper `BuildCommitmentHash` goes to llmq/commitment
- helper `BuildSignHash` goes to llmq/signing
- helper `GetLLMQParam` inlined since it's trivial (it has not been
trivial when introduced ages ago)
- removed dependency of `IsQuorumEnabled` on CQuorumManager which means
`quorumManager` deglobalization is done for 90%
## How Has This Been Tested?
- Run unit functional tests
- updated circular dependencies
`test/lint/lint-circular-dependencies.sh`
- check that llmq/utils is not included without needs to calculate
Quorums Members
```
$ grep -r include src/ 2> /dev/null | grep -v .Po: | grep -vE 'llmq/utils.(h|cpp)': | grep llmq/utils
src/evo/mnauth.cpp:#include <llmq/utils.h>
src/evo/deterministicmns.cpp:#include <llmq/utils.h>
src/llmq/quorums.cpp:#include <llmq/utils.h>
src/llmq/blockprocessor.cpp:#include <llmq/utils.h>
src/llmq/commitment.cpp:#include <llmq/utils.h>
src/llmq/debug.cpp:#include <llmq/utils.h>
src/llmq/dkgsessionhandler.cpp:#include <llmq/utils.h>
src/llmq/dkgsession.cpp:#include <llmq/utils.h>
src/llmq/dkgsessionmgr.cpp:#include <llmq/utils.h>
src/rpc/quorums.cpp:#include <llmq/utils.h>
```
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
9a40cfc558b3f7fa4fff1270f969582af17479a5 [refactor] use waiting inside disconnect_p2ps (gzhao408)
aeb9fb414e2d000830287d9dd3fed7fc2eb570d2 [test] wait for disconnect_p2ps to be reflected in getpeerinfo (gzhao408)
e81942d2e1288367e8da94adb2b2a88be99e4751 [test] logging and style followups for bloomfilter tests (gzhao408)
Pull request description:
Followup to #19083 which adds bloomfilter-related tests.
1. Make test_node `disconnect_p2ps` wait until disconnection is complete to avoid race conditions (and not place the burden on tests) from MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/19083#discussion_r437383989). And clean up any redundant `wait_until`s in the functional tests.
2. Clean up style + logging in p2p_filter.py and p2p_nobloomfilter_messages.py and jonatack's other [comments](https://github.com/bitcoin/bitcoin/pull/19083#pullrequestreview-428955784)
ACKs for top commit:
jonatack:
Code review ACK 9a40cfc from re-reviewing the diff and `git range-diff 5cafb46 8386ad5 9a40cfc`
MarcoFalke:
ACK 9a40cfc558b3f7fa4fff1270f969582af17479a5 🐂
Tree-SHA512: 2e14b1c12fc08a355bd5ccad7a2a734a4ccda4bc7dc7bac171cb57359819fc1599d764290729af74832fac3e2be258c5d406c701e78ab6d7262835859b9a7d87
3a10d935ac8ebabdfd336569d943f042ff84b13e [p2p/refactor] move disconnect logic and remove misbehaving (gzhao408)
ff8c430c6589ea72b9e169455cf6437c8623cc52 [test] test disconnect for filterclear (gzhao408)
1c6b787e0319c44f0e0bede3f4a77ac7c2089db2 [netprocessing] disconnect node that sends filterclear (gzhao408)
Pull request description:
Nodes that don't have bloomfilters turned on (i.e. no `NODE_BLOOM` service) should disconnect peers that send them `filterclear` P2P messages.
Non-bloomfilter nodes already disconnect peers for [`filteradd` and `filterload`](19e919217e/src/net_processing.cpp (L2218)), but #8709 removed `filterclear` so it could be used to reset tx relay. This isn't needed now because using `feefilter` message is much better for this purpose (See #19204).
Also refactors existing disconnect logic for `filteradd` and `filterload` into respective message handlers and removes banning for them.
ACKs for top commit:
jnewbery:
Code review ACK 3a10d935ac8ebabdfd336569d943f042ff84b13e
naumenkogs:
utACK 3a10d93
gillichu:
tested ACK: quick test_runner on macOS [`3a10d93`](3a10d935ac)
MarcoFalke:
re-ACK 3a10d935ac only change is replacing false with true 🚝
Tree-SHA512: 7aad8b3c0b0e776a47ad52544f0c1250feb242320f9a2962542f5905042f77e297a1486f8cdc3bf0fb93cd00c1ab66a67b2ec426eb6da3fe4cda56b5e623620f
(BACKPORT NOTICE: p2p_filter.py has also fixes for d5fbd4a92a3e7c1f8266d4cb4b639a0fe4c4c61f:test/functional/p2p_filter.py)
+ Merge #18726: test: check misbehavior more independently in p2p_filter.py
---------------------
dca73941eb0f0a4c9b68efed3870b536f7dd6cfe scripted-diff: rename node to peer for mininodes (gzhao408)
0474ea25afc65546cbfe5f822c0212bf3e211023 [test] fix race conditions and test in p2p_filter (gzhao408)
4ef80f0827392a1310ca5a29cc1f8f5ca5d16f95 [test] sending invalid msgs to node with bloomfilters=0 causes disconnect (gzhao408)
497a619386008dfaec0db15ecaebcdfaf75f5011 [test] add BIP 37 test for node with fRelay=false (gzhao408)
e8acc6015695c8439fc971a12709468995b96dcf [test] add mempool msg test for node with bloomfilter enabled (gzhao408)
Pull request description:
This PR adds a few tests that are bloomfilter-related, including behavior for when bloomfilters are turned _off_:
1. Tests p2p message `msg_mempool`: a node that has `peerbloomfilters` enabled should send its mempool (disabled behavior already tested [here](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_mempool.py)).
2. Tests that bloomfilter peers with [`fRelay=False`](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki#extensions-to-existing-messages) in the `version` message should not receive any invs until they set the filter. The rest is the same as what’s already tested in `p2p_filter.py`.
3. Tests that peers get disconnected if they send `filterload` or `filteradd` p2p messages to a node with bloom filters disabled.
4. Refactor: renames p2p_mempool.py to p2p_nobloomfilter_messages.py.
5. Fixes race conditions in p2p_filter.py
ACKs for top commit:
MarcoFalke:
ACK dca73941eb only changes is restoring accidentally deleted test 🍮
jonatack:
ACK dca73941eb0f0a4c9b68efed3870b536f7dd6cfe modulo a few nits if you retouch, happy to re-ACK if you take any of them but don't feel obliged to.
Tree-SHA512: 442aeab0755cb8b830251ea170d1d5e6da8ac9029b3276d407a20ee3d588cc61b77b8842368de18c244056316b8c63b911776d6e106bc7c023439ab915b27ad3
af2a145e575f23c64909e6cf1fb323c603bda7ca Refactor resource exhaustion test (Troy Giorshev)
5c4648d17ba18e4194959963994cc6b37053f127 Fix "invalid message size" test (Troy Giorshev)
ff1e7b884447a5ba10553b2d964625f94e255bdc Move size limits to module-global (Troy Giorshev)
57890abf2c7919eddfec36178b1136cd44ffe883 Remove two unneeded tests (Troy Giorshev)
Pull request description:
This PR touches only the p2p_invalid_messages.py functional test module. There are two main goals accomplished here. First, it fixes the "invalid message size" test, which previously made a message that was invalid for multiple reasons. Second, it refactors the file into a single consistent style. This file appears to have originally had two authors, with different styles and some test duplication.
It should now be easier and quicker to understand this module, anticipating the upcoming [BIP324](https://github.com/bitcoin/bitcoin/pull/18242) and [AltNet](https://github.com/bitcoin/bitcoin/issues/18989) changes.
This should probably go in ahead of #19107, but the two are not strictly related.
ACKs for top commit:
jnewbery:
ACK af2a145e575f23c64909e6cf1fb323c603bda7ca
MarcoFalke:
re-ACK af2a145e57 🍦
Tree-SHA512: 9b57561e142c5eaefac5665f7355c8651670400b4db1a89525d2dfdd20e872d6873c4f6175c4222b6f5a8e5210cf5d6a52da69b925b673a2e2ac30a15d670d1c
9952242c03fe587b5dff46a9f770e319146103bf build: improve builtin_clz* detection (fanquake)
Pull request description:
Fixes#19402.
The way we currently test for `__builtin_clz*` support with `AC_CHECK_DECLS` does not work with Clang:
```bash
configure:21492: clang++-10 -std=c++11 -c -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS conftest.cpp >&5
conftest.cpp💯10: error: builtin functions must be directly called
(void) __builtin_clz;
^
1 error generated.
```
This also removes the `__builtin_clz()` check, as we don't actually use it anywhere, and it's trvial to re-add detection if we do start using it at some point. If this is controversial then I'll add a test for it as well.
ACKs for top commit:
sipa:
ACK 9952242c03fe587b5dff46a9f770e319146103bf
laanwj:
ACK 9952242c03fe587b5dff46a9f770e319146103bf
Tree-SHA512: 695abb1a694a01a25aaa483b4fffa7d598842f2ba4fe8630fbed9ce5450b915c33bf34bb16ad16a16b702dd7c91ebf49fe509a2498b9e28254fe0ec5177bbac0
0ac09c9793cd6d25ef6df14d74fb960529e1b4e3 qt: Do not truncate node flag strings in debugwindow.ui peers details tab. (saibato)
Pull request description:
Fix: When fiddling around with new node flags other than the usual.
I saw that not all possible node flag strings i.e. the UNKNOWN[..] where
visible in peers details tab.
Since v18.2 fixed size was set to 300 and sliding is thereby limited.
A fix on my old linux cruft and small screen was to set minimumSize width to -1 or 0.
Qt will then autosize the slider to the max string length.
Thereby i had full display of all flags inclusive sliding without to fullscreen the window.
Not sure if this is even an issue for those who can afford big screens or high res macs?
Feedback welcome.
BTW: nice side effect now again easy to scroll trough long version names of the node.
can't wait to see strings like /Satoshi:0.23.99/NOX2NOX4NOX32 or what ever fits in the version string.
ACKs for top commit:
hebasto:
ACK 0ac09c9793cd6d25ef6df14d74fb960529e1b4e3, tested on Linux Mint 20 (x86_64, Qt 5.12.8).
promag:
Tested ACK 0ac09c9793cd6d25ef6df14d74fb960529e1b4e3 on macos.
Tree-SHA512: a1601b5e35f10b1fd9407b28142ca00c1b985a822be5d23be4d7d3376211450f06e17f962c44b8b40977f8f8bbbb701cac1c5abb4afb3618da76385dfac848a3
4d7369125a82214ea42b808a32b71b315a5c3c72 Disallow automatic conversion between hash types (Ben Woosley)
fa9ef2cdbed32438bdb32623af6e06f13ecd35e4 Remove an apparently unnecessary conversion (Ben Woosley)
966a22d859db37b1775e2180e5be032fc4fdf483 Explicitly support conversion between equivalent hash types (Ben Woosley)
f32c1e07fd6c174ff3f6406a619550d2f6c19360 Use explicit conversion from WitnessV0KeyHash -> CKeyID (Ben Woosley)
2c54217f913967703b404747133be67cf2f4feac Use explicit conversion from PKHash -> CKeyID (Ben Woosley)
a9e451f144480d7b170e49087df162989d31cd20 Convert CPubKey to WitnessV0KeyHash directly (Ben Woosley)
3fcc46812334074d2c77a6233e8a961cd0785872 Prefer explicit CScriptID construction (Ben Woosley)
0a5ea32ce605984094c5552877cb99bc81654f2c Prefer explicit uint160 conversion (Ben Woosley)
Pull request description:
This bases the script/standard hash types, TxDestination-related and CScriptID on a base template which does not silently convert the underlying `uintN` type.
Inspired by and built on #17924. Commits are small and focused to ease review.
Note some of these changes may be relative to existing bugs of the same sort as #17924. See particularly "Convert CPubKey to WitnessV0KeyHash directly" and "Remove an apparently unnecessary conversion".
ACKs for top commit:
achow101:
ACK 4d7369125a82214ea42b808a32b71b315a5c3c72
meshcollider:
re-utACK 4d7369125a82214ea42b808a32b71b315a5c3c72
Tree-SHA512: f1b3284ddc6fb6c6e726f2c22668b6d732d45eb5418262ed2b9c728f60be7be43dfb414b6ddd9915025c8dcd7f360dc3b46e997a945a2feb95b0e5c4f05d6b54
25f3554351a99a0a695fbd2a6a0f293b3adc3d98 scripted-diff: Make SeparatorStyle a scoped enum (Hennadii Stepanov)
Pull request description:
This PR is [split](https://github.com/bitcoin/bitcoin/pull/17877#issuecomment-644751515) from https://github.com/bitcoin/bitcoin/pull/17877 and makes `BitcoinUnits::SeparatorStyle` a scoped enum.
ACKs for top commit:
MarcoFalke:
review ACK 25f3554351a99a0a695fbd2a6a0f293b3adc3d98 🚐
Tree-SHA512: 578f1340a476cf79faa109a83815d3c75e26d9c18873e653d7624b52428ccb2677293116db0a60ae14c949d63b64988fc5a39c7184c2352b87b00e8ddaaaf474
fa7166759789c1282609ff3ab2e80d4f70910a9f ci: Move travis workarounds to .travis.yml (MarcoFalke)
Pull request description:
It seems odd to have travis related workarounds in the general ci config files. Fix that oddity by moving the travis related workarounds to the travis yaml file.
For unexplained reasons, this should also work around and thus close#19171
ACKs for top commit:
hebasto:
ACK fa7166759789c1282609ff3ab2e80d4f70910a9f, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: b4419d38e2b41f6e4d6e6b7658f1d972c40c390a49fe78808f8640d28efd84cc6668ce292d45b7c539e65b9e2ecbad10e796cb8f9329a0f1e7d0132ce962d226
0f8f51544558d204a4e9d0607b0f375150529637 RPC: Rephrase generatetoaddress help, and use PACKAGE_NAME (Luke Dashjr)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 357c6e0bd1b144213ca6cf0bfd649c7a482c2d6d5e98a254d20c8365d228dc71ae1b78aca4918fdbf065f8894ef82f8a475902d605204275bb99fe77d4b42fae
d49612f98add29066817b7c808b76c2d728948e5 Make SetMiscWarning() accept bilingual_str argument (Hennadii Stepanov)
d1ae7c0355662481a7d181a0a458284936d53eb1 Make GetWarnings() return bilingual_str (Hennadii Stepanov)
38e33aa481cefbe12c50f344bae190c0d95fb489 refactor: Make GetWarnings() bilingual_str aware internally (Hennadii Stepanov)
Pull request description:
This is one more step for consistent usage of `bilingual_str`.
No new translation messages are defined.
ACKs for top commit:
laanwj:
Code review ACK d49612f98add29066817b7c808b76c2d728948e5
MarcoFalke:
ACK d49612f98add29066817b7c808b76c2d728948e5 🌂
Tree-SHA512: 7413cb94a85291209c182845f6873350bb9e9ce940647d416c462a136603832fec8a63d792341bf634f07629767c78bc206d3a318cf10c7e87241c114c2496e9
There's no actual multiprocess code yet, that's just part related to build system.
To test build add an option --enable-multiprocess to configure
So far as there's no real multiprocessing yet, let's save CPU money - last commit in this PR disables gitlab's CI temporary
206f74e88cfa343d228c1d6596d3846863824ca5 Support make src/bitcoin-node and src/bitcoin-gui (João Barbosa)
Pull request description:
This change adds the following configure output variables
```
dnl Multi Process
BITCOIN_MP_NODE_NAME=bitcoin-node
BITCOIN_MP_GUI_NAME=bitcoin-gui
```
and adds support for
```sh
make src/bitcoin-node src/bitcoin-gui
```
ACKs for top commit:
laanwj:
Code review ACK 206f74e88cfa343d228c1d6596d3846863824ca5
Tree-SHA512: 4d1a694b9010ecc267ee955f4475127a58e6da72f30179ec740285ee6fe03cd91dcb6847317a47460dbd548edb88b7da6c7a98eac10f0dabe3ce4e83e0aa8093
7d0271b5c30f86e4af175a5ab7df5e593cd85195 depends: Set CMAKE_INSTALL_RPATH for native packages (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
---
After #19685 started setting `LDFLAGS`, the `INSTALL_RPATH_USE_LINK_PATH` cmake option used in the libmultiprocess build no longer works, so it is neccessary to set `CMAKE_INSTALL_RPATH` as a fallback.
It's unclear currently whether the bad interaction between `INSTALL_RPATH_USE_LINK_PATH` and `LDFLAGS` is a bug, but the issue is reported:
- https://github.com/bitcoin/bitcoin/issues/19981#issuecomment-696680877
- https://discourse.cmake.org/t/install-rpath-use-link-path-not-working-when-cmake-exe-linker-flags-ldflags-is-set/1892Fixes#19981
ACKs for top commit:
fanquake:
ACK 7d0271b5c30f86e4af175a5ab7df5e593cd85195 - I haven't looked in depth, but I've re-read through #19981 and checked the failure by testing #19160 (with this reverted):
dongcarl:
ACK 7d0271b Looked into this a bit, it makes sense that for the things we build in depends, we want the library search to start in depends. It seems reasonable to expect this to happen automatically when `CMAKE_INSTALL_PREFIX` and `INSTALL_RPATH_USE_LINK_PATH` are set, but oh well...
Tree-SHA512: 97cc5801c3204c14cd33004423631456ca0701e2127ee5146810a76e2f4aac9de1f4b5437402a4329cda54e022dc99270fee7e38c2995765f36b3848215fa78e
b8936883573708059357a66f67fad9dc77a8bade depends: Specify LDFLAGS to cmake as well (Carl Dong)
b3f541f618fe1f3d44baf6a0dd4299173c81f752 depends: Prepend CPPFLAGS to C{,XX}FLAGS for CMake (Carl Dong)
8e121e550953711cd03d7b6c221afd065c325c5e depends: Cleanup CMake invocation (Carl Dong)
8c7cd0c6d9f295bcb6913e3c69c9dac4ce2b25ce depends: More robust cmake invocation (Carl Dong)
3ecf0eca634601da216b06f091f95456c047f39c depends: Use $($(package)_cmake) instead of cmake (Carl Dong)
Pull request description:
- Use `$($(package)_cmake)` instead of invoking `cmake` directly
- Use well-known env vars instead of overriding CMake variables
ACKs for top commit:
ryanofsky:
Code review ACK b8936883573708059357a66f67fad9dc77a8bade. Only changes since last review are new commits adding whitespace, cppflags and ldflags to cmake invocation
Tree-SHA512: cfcd8cc9dcd0b336cf48b82fca9fe4bbc7930ed397cb7a68a07066680eb4c1906a6a9b5bd2589b4b4999e8f16232fa30ee9b376b60f4456d0fff931fbf9cc19a
92bc268e4af4ebcbde08567ea00e019ac509a769 build: Detect missed pkg-config early (Hennadii Stepanov)
1739eb23d8a6d272e70f95342323b6fe48b8eb6c build: Drop unused use_pkgconfig variable (Hennadii Stepanov)
a661449a2eeaf88efda36b6a84084dcbfe5b24eb build: Drop use_pkgconfig check for libmultiprocess check (Hennadii Stepanov)
90b95e7929463d6127c1b24fe1bf457d750a045c build: Drop dead non-pkg-config code for libevent check (Hennadii Stepanov)
44a14afbb889633a6c9a322a5aeca2e1b2cbdbd8 build: Drop dead non-pkg-config code for qrencode check (Hennadii Stepanov)
10cbae0c399302b0f8b1aa847c4246ba60bf25ee build: Drop dead non-pkg-config code for ZMQ check (Hennadii Stepanov)
06cfc9cadf7c5dd43147e6525a348d5f2d299422 build: Fix indentation in UNIVALUE check (Hennadii Stepanov)
6fd2118e777d11cbc81a45313d1a7d6400e34f3f build: Drop dead non-pkg-config code for UNIVALUE check (Hennadii Stepanov)
e9edbe4dbd8c24a779de7d92e5f10c870aab5511 build: Always use pkg-config (Hennadii Stepanov)
9e2e753b0605c8cd826381a362f0c7de56eea81f build: Always define ZMQ_STATIC for MinGW (Hennadii Stepanov)
Pull request description:
This PR:
- is based on #18297 (already merged)
- drops all of the non-pkg-config paths from the `configure` script
Ref: #17768
ACKs for top commit:
fanquake:
ACK 92bc268e4af4ebcbde08567ea00e019ac509a769. I re-gitian-built. There are a couple follow-ups that I'll PR shortly. Thanks for addressing my feedback above. I took too long to get back to this.
laanwj:
ACK 92bc268e4af4ebcbde08567ea00e019ac509a769
Tree-SHA512: 83c2d9cf03518867a1ebf7e26a8fc5b6dd8962ef983fe0d84e0c7eb74717f4c36a834da02faf0e503ffd87167005351671cf040c0d4ddae57ee152a6ff84012b
