Age | Commit message (Collapse) | Author |
|
85885919656a3f606f3d7f208378aabe95f3f62d guix: ignore additioanl failing certvalidator test (fanquake)
e8a6c888e46ec6afd408feac92cd81e2be6af490 build: point to latest commit on the master branch (fanquake)
Pull request description:
Guix recently force pushed to their `version-1.4.0` branch, causing #24040. At the time #24042 pointed to the newest commit on that branch, fixing #24040, but it didn't actually build. Guix have now [temporarily removed](https://lists.gnu.org/archive/html/guix-devel/2022-01/msg00313.html) their `version-1.4.0` branch, while they incorporate additional fixes.
This PR updates our guix time-machine to point to a recent commit on the guix master branch, so that builds can resume for all hosts apart from Windows. Windows builds are broken due to what looks like an upstream issue, which results in e failure to build the [`mingw-w64-x86_64-winpthreads package`](https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/mingw.scm#n137). The build fails with:
```bash
/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash ./libtool --tag=CC --mode=link x86_64-w64-mingw32-gcc -Wall -DWIN32_LEAN_AND_MEAN -g -O2 -no-undefined -version-info 1:0:0 -L./fakelib -Wc,-no-pthread -o libwinpthread.la -rpath /gnu/store/2bglap5gk4db8lajaahg4d7361myw5qw-mingw-w64-x86_64-winpthreads-8.0.0/lib src/libwinpthread_la-barrier.lo src/libwinpthread_la-cond.lo src/libwinpthread_la-misc.lo src/libwinpthread_la-mutex.lo src/libwinpthread_la-rwlock.lo src/libwinpthread_la-spinlock.lo src/libwinpthread_la-thread.lo src/libwinpthread_la-ref.lo src/libwinpthread_la-sem.lo src/libwinpthread_la-sched.lo src/libwinpthread_la-clock.lo src/libwinpthread_la-nanosleep.lo src/version.lo
libtool: link: x86_64-w64-mingw32-gcc -shared src/.libs/libwinpthread_la-barrier.o src/.libs/libwinpthread_la-cond.o src/.libs/libwinpthread_la-misc.o src/.libs/libwinpthread_la-mutex.o src/.libs/libwinpthread_la-rwlock.o src/.libs/libwinpthread_la-spinlock.o src/.libs/libwinpthread_la-thread.o src/.libs/libwinpthread_la-ref.o src/.libs/libwinpthread_la-sem.o src/.libs/libwinpthread_la-sched.o src/.libs/libwinpthread_la-clock.o src/.libs/libwinpthread_la-nanosleep.o src/.libs/version.o -L./fakelib -g -O2 -no-pthread -o .libs/libwinpthread-1.dll -Wl,--enable-auto-image-base -Xlinker --out-implib -Xlinker .libs/libwinpthread.dll.a
x86_64-w64-mingw32-ld: final link failed: bad value
collect2: error: ld returned 1 exit status
```
This info will be sent upstream. You can likely recreate the failure with `guix build mingw-w64-x86_64-winpthreads`.
Once Guix is at a point of tagging a 1.4.0 release commit (after re-branching and RCs), we can shift our time-machine to point to it. If a change is made upstream in relation to the Windows issue we are seeing, we will adjust our time-machine commit earlier.
Guix builds:
```bash
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
9bbaba7be551f871d940346dd8896908968e5403cdbaa9a990b98a5ef5d8a2c8 guix-build-85885919656a/output/aarch64-linux-gnu/SHA256SUMS.part
4da3c6589afb383e689c1569d9d0379dba3e6e18039f331c91da180b937b6b68 guix-build-85885919656a/output/aarch64-linux-gnu/bitcoin-85885919656a-aarch64-linux-gnu-debug.tar.gz
089c83b739149ea22809a6594aaec5b1df648d770e1086f17ab901e998b54dfd guix-build-85885919656a/output/aarch64-linux-gnu/bitcoin-85885919656a-aarch64-linux-gnu.tar.gz
71db39d3e84f50a4146300271077bf9687ede8a15ee450d1b62270f362c6fa98 guix-build-85885919656a/output/arm-linux-gnueabihf/SHA256SUMS.part
6b6528e6077f403a53a12199a5dc98d5ea829380295aa768232d619417615465 guix-build-85885919656a/output/arm-linux-gnueabihf/bitcoin-85885919656a-arm-linux-gnueabihf-debug.tar.gz
4ce4d0c28d09bfd46492cd14f8d0a93c58ca827c2e5dcb7aadf74147b8aff7ea guix-build-85885919656a/output/arm-linux-gnueabihf/bitcoin-85885919656a-arm-linux-gnueabihf.tar.gz
971b85090756ec9557792bfc7f47a013434d30ef41a19b6284c6efc425ce36bf guix-build-85885919656a/output/dist-archive/bitcoin-85885919656a.tar.gz
64b9757d597f4665dc7b71f6ff3e52671ca2783a8834ef212dd795ff98350919 guix-build-85885919656a/output/powerpc64-linux-gnu/SHA256SUMS.part
5009332c4b7a23263a99e5f39a635d1193c7300672916abd6528112ba56c71c5 guix-build-85885919656a/output/powerpc64-linux-gnu/bitcoin-85885919656a-powerpc64-linux-gnu-debug.tar.gz
84f0e74abe3cf499480c16b371522c6ab958e5c234514c720e7b8915be4dc62d guix-build-85885919656a/output/powerpc64-linux-gnu/bitcoin-85885919656a-powerpc64-linux-gnu.tar.gz
f73d0fe614caaa617aa2e65f59ccf689ddd2c484878ebd0649dd2e14b31b0329 guix-build-85885919656a/output/powerpc64le-linux-gnu/SHA256SUMS.part
0985d12c3aa1b7625a7369a4d4a8c7f92e1eaf4276c4457610f90d3c057a843e guix-build-85885919656a/output/powerpc64le-linux-gnu/bitcoin-85885919656a-powerpc64le-linux-gnu-debug.tar.gz
e35a34bc9bf9bc8b9e760b7b9da897ed9bf669dc2a36cd3d5395dbb25683d057 guix-build-85885919656a/output/powerpc64le-linux-gnu/bitcoin-85885919656a-powerpc64le-linux-gnu.tar.gz
c1800ea2353feb984f05ea6f6faa6421b5f3764a036d335fbb18c6d313176e8b guix-build-85885919656a/output/riscv64-linux-gnu/SHA256SUMS.part
17012337bfc124970dcb26fe0bef0eb0ec57eabaafdd0533828732e407199941 guix-build-85885919656a/output/riscv64-linux-gnu/bitcoin-85885919656a-riscv64-linux-gnu-debug.tar.gz
c9528178e2266ef7d2f49b6b6f65233c58c8f71d196d9347421f988ba4b662bd guix-build-85885919656a/output/riscv64-linux-gnu/bitcoin-85885919656a-riscv64-linux-gnu.tar.gz
2e6c4fc18b866648e35ebece7c7c8f625833c4f37c1aae8b7d0bc0d5caca6f69 guix-build-85885919656a/output/x86_64-apple-darwin/SHA256SUMS.part
dcee189ad0b8dad71df9053db9c0e2f308c1742cb1638d60d515dafcf4cca8be guix-build-85885919656a/output/x86_64-apple-darwin/bitcoin-85885919656a-osx-unsigned.dmg
d9366875d36c1993831548ca94de859cbdecca26b77877012456b63bf144558e guix-build-85885919656a/output/x86_64-apple-darwin/bitcoin-85885919656a-osx-unsigned.tar.gz
6ddd041d2b359f1be5272e79d80deb7ed233289394cbc3436b4cc43853c9690f guix-build-85885919656a/output/x86_64-apple-darwin/bitcoin-85885919656a-osx64.tar.gz
ef8ce6b349d886f23341dd034463b18d0c5298cf6b2d95632ae24ca98b1b51c8 guix-build-85885919656a/output/x86_64-linux-gnu/SHA256SUMS.part
ce5c2d681617ab1f2f20487ab0b73c48a436a3d313e7441d594a68f28a39a5d3 guix-build-85885919656a/output/x86_64-linux-gnu/bitcoin-85885919656a-x86_64-linux-gnu-debug.tar.gz
b364641aff6a1cde8b1bd950a50c74e9c49a09669d5597365de4da250b41aea6 guix-build-85885919656a/output/x86_64-linux-gnu/bitcoin-85885919656a-x86_64-linux-gnu.tar.gz
```
ACKs for top commit:
hebasto:
ACK 85885919656a3f606f3d7f208378aabe95f3f62d
dunxen:
ACK 8588591
Tree-SHA512: ee75c630366200e2a93a3a82fb5f689c6f69e3a39a00e8bf61609a9f21283b6511e9e2ceb44c52550f8f231b38f5966d2ef2669ada5588272cc424cbd25a1ed2
|
|
fa2bcc4e42e7fed61727b3de4019e9702d4090ce Run coin.IsSpent only once in a row (MarcoFalke)
Pull request description:
Follow-up to commit 64e4963c635ec3a73a5fa3f32f6ec08e70609f60 and https://github.com/bitcoin/bitcoin/pull/23976#discussion_r787758193
ACKs for top commit:
glozow:
utACK fa2bcc4e42e7fed61727b3de4019e9702d4090ce, agree the assertion is sufficient
theStack:
Code-review ACK fa2bcc4e42e7fed61727b3de4019e9702d4090ce
w0xlt:
crACK https://github.com/bitcoin/bitcoin/pull/24102/commits/fa2bcc4e42e7fed61727b3de4019e9702d4090ce
shaavan:
Code Review ACK fa2bcc4e42e7fed61727b3de4019e9702d4090ce
brunoerg:
crACK fa2bcc4e42e7fed61727b3de4019e9702d4090ce
Tree-SHA512: 3be9d6b313bf6bb835f031826c81777b4659118d839001d084e72462391cb64ba81d06a5e07fd21fcfb709a71b08892b23212a98604ce8481da489476b72f072
|
|
Mutex, and rename it
dec787d8ac2e8fb42db87431dd622bf44897bc4e refactor: replace RecursiveMutex `m_addr_local_mutex` with Mutex (w0xlt)
93609c1dfad70961697d0d12bf01cd34b8ceb6c8 p2p: add assertions and negative TS annotations for m_addr_local_mutex (w0xlt)
c4a31ca267f74bff76a43878177d05d22825a203 scripted-diff: rename cs_addrLocal -> m_addr_local_mutex (w0xlt)
Pull request description:
This PR is related to #19303 and gets rid of the `RecursiveMutex cs_addrLocal`.
ACKs for top commit:
hebasto:
ACK dec787d8ac2e8fb42db87431dd622bf44897bc4e, I have reviewed the code and it looks OK, I agree it can be merged.
shaavan:
reACK dec787d8ac2e8fb42db87431dd622bf44897bc4e
Tree-SHA512: b7a043bfd4e2ccbe313bff21ad815169db6ad215ca96daf358ce960c496a548b4a9e90be9e4357430ca59652b96df87c097450118996c6d4703cbaabde2072d0
|
|
scripted-diffs
d8dfc403f74858b79c87f650b5d609aa60a4dc3b script: redirecting stderr to stdout before pipelining into grep (anouar kappitou)
30df5c3dd4d39d9027b0341d01d3233400104893 script: preventing non-compatible sed binary. (anouar kappitou)
Pull request description:
This Pull request improve scripted diff by checking for non-compatible sed binary. Fixes #19815
ACKs for top commit:
shaavan:
reACK d8dfc403f74858b79c87f650b5d609aa60a4dc3b
theStack:
Tested ACK d8dfc403f74858b79c87f650b5d609aa60a4dc3b
Tree-SHA512: 4813abb02195e04b8953662e51cba4599789e6185a6d384f56255e2d37c2b16be46c7c8861e44ba1a13d42573a58eb708901472925cb308dba2f602020e9f0f6
|
|
with CNetMessage::m_type
224d87855ec38cc15866d9673e1b19942a82c1cd net, refactor: Drop tautological local variables (Hennadii Stepanov)
3073a9917b31d15ba958ea8148585633ba905f8b scripted-diff: Rename CNetMessage::m_command with CNetMessage::m_type (Hennadii Stepanov)
Pull request description:
https://github.com/bitcoin/bitcoin/pull/18533#issue-594592488:
> a message is not a command, but simply a message of some type
Continuation of bitcoin/bitcoin#18533 and bitcoin/bitcoin#18937.
ACKs for top commit:
theStack:
Concept and code-review ACK 224d87855ec38cc15866d9673e1b19942a82c1cd
shaavan:
Code Review ACK 224d87855ec38cc15866d9673e1b19942a82c1cd
w0xlt:
crACK 224d878
Tree-SHA512: 898cafb44708dae1413fcc1533d809d75878891354f1b5edaaec1287f4921c31adc9330f4d42d82544a39689886bc17fee71ea587f9199fd5cc849d376f82176
|
|
9b8dcb25b57ad31b77c9f37d9a1f5b07dc6378b4 [net processing] Rename PoissonNextSendInbound to NextInvToInbounds (John Newbery)
ea99f5d01e56ab0192d211da1034ffb299876937 [net processing] Move PoissonNextSendInbound to PeerManager (John Newbery)
bb060746df22c956b8f44e5b8cd1ae4ed73faddc scripted-diff: replace PoissonNextSend with GetExponentialRand (John Newbery)
03cfa1b6035dbcf6a414f9bc432bd9e612801ebb [refactor] Use uint64_t and std namespace in PoissonNextSend (John Newbery)
9e64d69bf74c8a381fb59841519cc3736bce14d4 [move] Move PoissonNextSend to src/random and update comment (John Newbery)
Pull request description:
`PoissonNextSend` and `PoissonNextSendInbound` are used in the p2p code to obfuscate various regularly occurring processes, in order to make it harder for others to get timing-based information deterministically.
The naming of these functions has been confusing to several people (including myself, see also #23347) because the resulting random timestamps don't follow a Poisson distribution but an exponential distribution (related to events in a Poisson process, hence the name). This PR
- moves `PoissonNextSend()` out of `net` to `random` and renames it to `GetExponentialRand()`
- moves `PoissonNextSendInbound()` out of `CConnman` to `PeerManager` and renames it to `NextInvToInbounds()`
- adds documentation for these functions
This is work by jnewbery - due to him being less active currently, I opened the PR and will address feedback.
ACKs for top commit:
jnewbery:
ACK 9b8dcb25b5
hebasto:
ACK 9b8dcb25b57ad31b77c9f37d9a1f5b07dc6378b4, I have reviewed the code and it looks OK, I agree it can be merged.
theStack:
ACK 9b8dcb25b57ad31b77c9f37d9a1f5b07dc6378b4 📊
Tree-SHA512: 85c366c994e7147f9981fe863fb9838502643fa61ffd32d55a43feef96a38b79a5daa2c4d38ce01074897cc95fa40c76779816edad53f5265b81b05c3a1f4f50
|
|
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
|
|
|
|
|
|
pubkeys
36012ef143917f97179d3ba6599ef36a26a9a014 qa: test descriptors with mixed xpubs and const pubkeys (Antoine Poinsot)
Pull request description:
Writing unit tests for Miniscript descriptors i noticed that `test/descriptor_tests`'s `DoCheck()` assumes that a descriptor would either contain only extended keys or only const pubkeys: if it detects an xpub in the descriptor it would assert the number of cached keys is equal to the number of keys in the descriptor, which does not hold if the descriptor also contains const (raw?) public keys since we only cache parent xpubs.
ACKs for top commit:
achow101:
ACK 36012ef143917f97179d3ba6599ef36a26a9a014
Tree-SHA512: 2ede67a6dff726bcad3e260f3deb25c9b77542ed1880eb4ad136730b741014ce950396c69c7027225de1ef27108d609bafd055188b88538ace0beb13c7e34b0b
|
|
faedb111d2df78eabddaeccaba42f47572388bc7 refactor tests to fix ubsan suppressions (MarcoFalke)
Pull request description:
The ubsan suppressions for test files have several issues:
* They bloat the suppressions file, distracting from real bugs
* They are file-wide, thus suppressing any other (newly introduced) issues in the same file
* Some of them are causing compile issues with -Wimplicit-int-conversion
Fix all issues by making the integer truncations or sign changes explicit.
This is a refactor that shouldn't change the test binary unless compiled with sanitizers.
ACKs for top commit:
jonatack:
ACK faedb111d2df78eabddaeccaba42f47572388bc7 per `git range-diff 1824644a fa9bab0 faedb11` and clang 13 debug build sanity check
Tree-SHA512: a1a6aaf2cf52562a49220375deb10062b388c1ccb7ddcd9ef2a8e6b9873961c5b285704799589b7dd5579f3db8692ba72a9d622172b4e4d2423534cd550e8859
|
|
|
|
Mutex, and rename it
5e7e4c9f6e57f5333bd17a20b0c85a78d032998e refactor: replace RecursiveMutex g_maplocalhost_mutex with Mutex (w0xlt)
a7da1409bc9f614009f76c1bfc55f029ff1265e4 scripted-diff: rename cs_mapLocalHost -> g_maplocalhost_mutex (w0xlt)
Pull request description:
This PR is related to #19303 and gets rid of the `RecursiveMutex cs_mapLocalHost`.
ACKs for top commit:
shaavan:
ACK 5e7e4c9f6e57f5333bd17a20b0c85a78d032998e
theStack:
ACK 5e7e4c9f6e57f5333bd17a20b0c85a78d032998e
hebasto:
ACK 5e7e4c9f6e57f5333bd17a20b0c85a78d032998e, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 961171e346fe385e16db9830115a8096f4ca2499bbea11a08c02ca808638dfb63c434ab9d66392c71e85be6352c8a2b6a0054b5a61aaabd28d71581fed5beae7
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/cs_addrLocal/m_addr_local_mutex/g' -- $(git grep --files-with-matches 'cs_addrLocal')
-END VERIFY SCRIPT-
|
|
======================================================================
ERROR: test_revocation_mode_soft (tests.test_validate.ValidateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/test_validate.py", line 85, in test_revocation_mode_soft
validate_path(context, path)
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/../certvalidator/validate.py", line 50, in validate_path
return _validate_path(validation_context, path)
File "/tmp/guix-build-python-certvalidator-0.1-1.e5bdb4b.drv-0/source/tests/../certvalidator/validate.py", line 358, in _validate_path
raise PathValidationError(pretty_message(
certvalidator.errors.PathValidationError: The path could not be validated because the end-entity certificate expired 2022-01-14 12:00:00Z
|
|
The version-1.4.0 branch no-longer exists, and will be branched off
master again shortly.
|
|
signing on Windows
e2ab9f83f8655bf09ea392beeee36b2bbe29769b build: disable external signer on Windows (fanquake)
Pull request description:
This change explicitly disables support for external signing when targeting Windows and OpenBSD. The driver for this is that Boost Process uses boost::filesystem internally, when targeting Windows, which gets in the way of removing our usage of it (#20744). While we could adjust #20744 to still link against the Boost libs when building for Windows, that would be disappointing, as we wouldn't have cleanly removed the Boost usage we're trying too (including the build infrastructure), and, we'd be in a position where we would be building releases differently depending on the platform, which is something I want to avoid.
After discussion with Sjors, Achow and Hebasto, this seemed like a reasonable step to move #20744 forward (as-is). Note that support for external signing ([while already being experimental](https://github.com/bitcoin/bitcoin/blob/master/doc/external-signer.md#example-usage)), could be considered even more experimental on Windows. Also, oddly, we have external-signing [explicitly disabled in our Windows (cross-compile) CI](https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/ci/test/00_setup_env_win64.sh#L16), it's not clear why this is the case, as, if it's a feature being built into releases, it should be being built and tested in the CI which is most-like the release process.
There is an [issue open upstream](https://github.com/boostorg/process/issues/207), in regards to migrating Boost Process to std::filesystem, or having an option to use it. However there hasn't been much discussion since it was opened ~9 months ago. There is another related issue here: https://github.com/klemens-morgenstern/boost-process/issues/164.
Resolves #24036.
ACKs for top commit:
Sjors:
utACK e2ab9f8
achow101:
ACK e2ab9f83f8655bf09ea392beeee36b2bbe29769b
kallewoof:
utACK e2ab9f83f8655bf09ea392beeee36b2bbe29769b
hebasto:
ACK e2ab9f83f8655bf09ea392beeee36b2bbe29769b, tested on Linux Mint 20.2 (x86_64).
Tree-SHA512: 36fcfc0e1a008a8271dc76b8e12e93d3e1d1e528bf668e95a559e9f6fd7d5f031bd7a6a6bc8b9fa9d057b2cd56f9ec8838c7f74e87899bf9a6aeb787afbd112c
|
|
dc5d6b0d4793ca978f71f69ef7d6b818794676c2 fs: Make compatible with boost 1.78 (Andrew Chow)
Pull request description:
Boost 1.78 removed `operator+` in a way that breaks our usage of it in a subclass. A [proposed workaround](https://github.com/boostorg/filesystem/issues/223#issuecomment-1000230207) for this is to cast the argument to `boost::filesystem::path`, and this is backwards compatible with older versions of boost.
Additionally, it appears that `fs::canonical` no longer removes trailing slashes. This was causing a test to fail. The solution is to explicitly remove the trailing separator in the one place that `fs::canonical` is used.
Lastly, `fs::create_directories` now has an error message saying `create_directories` instead of `create_directory`. This caused wallet_multiwallet.py to fail. The error message check has been updated to be able accept either string.
Fixes #23846
ACKs for top commit:
ryanofsky:
Code review ACK dc5d6b0d4793ca978f71f69ef7d6b818794676c2
vincenzopalazzo:
ACK https://github.com/bitcoin/bitcoin/pull/24104/commits/dc5d6b0d4793ca978f71f69ef7d6b818794676c2
Tree-SHA512: d4d8e7b49b8dfbf0ced9bfe9a2b3827841227fc755fc799f19159076b0ccf882432cc8b6ad93cdeda98fb58b942b9ba50a9e0a6b4f6b1e0097e80f1074ae5682
|
|
10 build
6897c4bdf51a4aa74320ebfffa9467db14670109 build: patch depends zeromq to fix building on NetBSD Current (fanquake)
ce6dd2f1a2e2c56d86d00e0eeec34c9982017416 zeromq 4.3.4 (fanquake)
Pull request description:
This is a prerequisite for #23955. It updates zeromq to the latest available version, and adds a patch, [that I've sent upstream](https://github.com/zeromq/libzmq/pull/4326), to fix building on NetBSD Current (10).
ACKs for top commit:
hebasto:
ACK 6897c4bdf51a4aa74320ebfffa9467db14670109, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: d05d9753630faebe842e1ca70c8c4af660a38e7331a9d95e84df3a3b14564c5118ca41c4fc49fb71dfee563b63e1014e5a3f8874d652e26de59e8e188a12970e
|
|
|
|
Follow-up to commit 64e4963c635ec3a73a5fa3f32f6ec08e70609f60
|
|
e177fcab3831b6d259da5164cabedcc9e78f6957 Replace `struct update_lock_points` with lambda (glozow)
c7cd98c7176800a51e6a6b3634a26b508aa33ff2 document and clean up MaybeUpdateMempoolForReorg (glozow)
Pull request description:
followup to #23683, addressing https://github.com/bitcoin/bitcoin/pull/23683#issuecomment-989741186
ACKs for top commit:
hebasto:
ACK e177fcab3831b6d259da5164cabedcc9e78f6957, I have reviewed the code and it looks OK, I agree it can be merged.
instagibbs:
ACK e177fcab3831b6d259da5164cabedcc9e78f6957
MarcoFalke:
Approach ACK e177fcab3831b6d259da5164cabedcc9e78f6957 😶
Tree-SHA512: 8c2709dd5cab73cde41f3e5655d5f237bacfb341f78eac026169be579528695ca628c8777b7d89760d8677a4e6786913293681cfe16ab702b30c909703e1824c
|
|
bf044ef9ecc93a69619cbaa9fa2b874d5fc06932 build: specify hosts for qrencode package (fanquake)
Pull request description:
Similar to how we specify the OS's we build Qt for, specify which OS's
we will build qrencode for (a qt dependency). This commit alone doesn't
change anything, but when we start supporting other OS's, i.e #23948,
where we wont support qt (or at least initially), it'll skip building
the qrencode package, which would be unused.
I'll rebase the other *BSD changes on top of this.
ACKs for top commit:
hebasto:
ACK bf044ef9ecc93a69619cbaa9fa2b874d5fc06932, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 3f5f11f122704a664dd77d8da0b7e9b95d44b2f1514d0199deed9b8b8ad0d8883a1de1f444b796c5f4681f423a380c3905fce720d7d2b788130162c907c2ce3b
|
|
|
|
-BEGIN VERIFY SCRIPT-
s() { sed -i 's/cs_mapLocalHost/g_maplocalhost_mutex/g' $1; }
s src/net.cpp
s src/net.h
s src/rpc/net.cpp
s src/test/net_tests.cpp
-END VERIFY SCRIPT-
|
|
bd52684508ca2964e6a3af503d21ff99675380c7 test: rest /tx with an invalid/unknown txid (brunoerg)
Pull request description:
This PR adds test coverage to the endpoint `/tx` (rest) passing an invalid and an unknown txid to test its return.
Invalid -> should return status code 400 (bad request)
Unknown -> should return status code 404 (not found)
ACKs for top commit:
kallewoof:
ACK bd52684508ca2964e6a3af503d21ff99675380c7
Tree-SHA512: a7fbb63f30d06fc0855133a36e8317c7930ba13aa2b4a2dd1fc35079d59eacace72e1ffe7ae1b3e067066fe51792415940d72d923e83a659a0d5965e4110b32a
|
|
86a4a15bdcc96eb565ab80166642d71d542061a9 Highlight DNS request part (Prayank)
Pull request description:
_What?_
Highlight DNS requests part in Proxy section
_Why?_
1. DNS requests are very important while considering privacy
2. Lot of users might skip reading it because of the way it is mixed with everything else in the doc right now
3. I have seen lot of users ignoring DNS requests or unaware of such things while using privacy tools
_How?_
Initially I had tried keeping these lines separate from code block but [Jonatack didn't agree with the changes](https://github.com/bitcoin/bitcoin/pull/21157#discussion_r618177116). Harding suggested using [bold/italic in `<pre></pre>`](https://github.com/bitcoin/bitcoin/pull/21157#discussion_r644364143). I have used the suggestions from previous PR and added `---`
This is a part of alternative described in https://github.com/bitcoin/bitcoin/pull/22316
ACKs for top commit:
jonatack:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
Rspigler:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
achow101:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
RiccardoMasutti:
ACK 86a4a15
lsilva01:
ACK https://github.com/bitcoin/bitcoin/pull/22317/commits/86a4a15bdcc96eb565ab80166642d71d542061a9
kristapsk:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
theStack:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
Tree-SHA512: a4fe0e8c08df330e5ca78ce19ce74be7034c653f4374469d928908847a6debf385283e3a6da66de600566c7bab6290ccd35df26864aef94cbb3f294123391437
|
|
`indentLevel` arguments
f25114148320c3fe07af3abc6e2f150d0a2c10f4 util: Fix mis-swapped `prettyIndent` and `indentLevel` arguments (Hennadii Stepanov)
Pull request description:
On master d0bf9bb6a539f151ec92725d20a2b6c22cb095a5:
```
$ cat settings.json
{
"wallet": [
"210803d"
]
}
```
With this PR:
```
$ cat settings.json
{
"wallet": [
"210803d"
]
}
```
ACKs for top commit:
ryanofsky:
Code review ACK f25114148320c3fe07af3abc6e2f150d0a2c10f4. Nice catch!
Tree-SHA512: 1c315c807a070039c05a77f4e8855011d468b9aeb141e4fa3b1e1aaaab252e3831e8bdfe0caddf7704a00492799022f761f6d21a047c5bf75cdbcc96f04fc04e
|
|
|
|
No behavior change.
This code was introduced in 5add7a7 before we required C++11, which is
why the struct was needed. As we are now using more modern C++ and this
is the only place where lockpoints are updated for mempool entries, it
is more idiomatic to call `modify` with a lambda.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
Co-authored-by: John Newbery <john@johnnewbery.com>
|
|
Similar to how we specify the OS's we build Qt for, specify which OS's
we will build qrencode for (a qt dependency). This commit alone doesn't
change anything, but when we start supporting other OS's, i.e #23948,
where we wont support qt (or at least initially), it'll skip building
the qrencode package, which would be unused.
|
|
and fuzz tests, make addrman consistency check ratio easier to change
7f122a4188af7130be9251611e41136a17c814f1 fuzz: non-addrman fuzz tests: override-able check ratio (Vasil Dimov)
3bd83e273d104e9474af8f1bdf4f969163e33ade fuzz: addrman fuzz tests: override-able check ratio (Vasil Dimov)
46b0fe78298c8f416a91dec9d4e0f3f4cb1e68b0 test: non-addrman unit tests: override-able check ratio (Vasil Dimov)
81e4d54d3a95f7bffeb353217a6c32eb2aca8b5c test: addrman unit tests: override-able check ratio (Vasil Dimov)
6dff6214be768a3fab6d5201daf5ef6071764746 bench: put addrman check ratio in a variable (Vasil Dimov)
6f7c7567c578b5a41f8e90ce4491e40f7faeaa56 fuzz: parse the command line arguments in fuzz tests (Vasil Dimov)
92a0f7e58d4b6323d21f1c45d4c20266c35df030 test: parse the command line arguments in unit tests (Vasil Dimov)
Pull request description:
Previously command line arguments passed to unit and fuzz tests would be ignored by the tests themselves. They would be used by the boost test framework (e.g. `--run_test="addrman_tests/*"`) or by the fuzzer (e.g. `-runs=1`). However both provide ways to pass down the extra arguments to the test itself. Use that, parse the arguments and make them available to the tests via `gArgs`.
This makes the tests more flexible as they can be run with any bitcoind config option specified on the command line.
When creating `AddrMan` objects in tests, use `-checkaddrman=` (if provided) instead of hardcoding the check ratio in many different places. See https://github.com/bitcoin/bitcoin/pull/20233#issuecomment-889813074 for further motivation for this.
ACKs for top commit:
mzumsande:
re-ACK 7f122a4188af7130be9251611e41136a17c814f1
josibake:
reACK https://github.com/bitcoin/bitcoin/pull/23373/commits/7f122a4188af7130be9251611e41136a17c814f1
Tree-SHA512: 3a05e61e4d70a0569bb67594bcce3aad8fdef63cdcc54e2823a3bc9f18679571985004412b6c332a210f67849bab32d8467b4115fbff8f5fac9834982e60dcf3
|
|
`m_cs_callbacks_pending` with Mutex (and rename)
5574e6ed52d6effd3b7beff0f09b44449202a585 refactor: replace RecursiveMutex `m_callbacks_mutex` with Mutex (Sebastian Falbesoner)
3aa258109e3f3e0b1bfc4f811cbadfd6d516208c scripted-diff: rename `m_cs_callbacks_pending` -> `m_callbacks_mutex` (Sebastian Falbesoner)
Pull request description:
This PR is related to #19303 and gets rid of the RecursiveMutex `m_cs_callbacks_pending`. All of the critical sections (6 in total) only directly access the guarded elements, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex:
https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/src/scheduler.cpp#L138-L145
https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/src/scheduler.cpp#L152-L160
https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/src/scheduler.cpp#L169-L172
https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/src/scheduler.cpp#L184-L187
https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/src/scheduler.cpp#L197-L199
https://github.com/bitcoin/bitcoin/blob/807169e10b4a18324356ed6ee4d69587b96a7c70/src/scheduler.cpp#L203-L206
Also, it is renamed to adapt to the (unwritten) naming convention to use the `_mutex` suffix rather than the `cs_` prefix.
ACKs for top commit:
hebasto:
ACK 5574e6ed52d6effd3b7beff0f09b44449202a585, I have reviewed the code and it looks OK, I agree it can be merged.
w0xlt:
crACK 5574e6e
Tree-SHA512: ba4b151d956582f4c7183a1d51702b269158fc5e2902c51e6a242aaeb1c72cfcdf398f9ffa42e3072f5aba21a8c493086a5fe7c450c271322da69bd54c37ed1f
|
|
with Mutex (and rename)
30927cb5306d01da2716786c2d9457c49ec49d0f refactor: replace RecursiveMutex `m_subver_mutex` with Mutex (Sebastian Falbesoner)
0639aba42ad8449fe43eb2aad0ffbe3390aabd37 scripted-diff: rename `cs_SubVer` -> `m_subver_mutex` (Sebastian Falbesoner)
Pull request description:
This PR is related to #19303 and gets rid of the RecursiveMutex `cs_SubVer`. Both of the critical sections only directly access the guarded variable, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex.
ACKs for top commit:
hebasto:
ACK 30927cb5306d01da2716786c2d9457c49ec49d0f, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 2faead792ea0b2f79f9f7fe99acde5cf2bfcd2f15c51fbb6cb1099d4f81276001a492f7d46a5139055f4366c2d58a36a8ba19f21d56df20e0ed93af3141dbe11
|
|
fa99e108e778b5169b3de2ce557af68f1fe0ac0b Fix implicit-integer-sign-change in arith_uint256 (MarcoFalke)
Pull request description:
This refactor doesn't change behaviour, but clarifies that the numbers being dealt with aren't supposed to be negative. This helps when reading the code and allows to remove a sanitizer suppression for the whole file.
ACKs for top commit:
PastaPastaPasta:
utACK fa99e108e778b5169b3de2ce557af68f1fe0ac0b
shaavan:
ACK fa99e108e778b5169b3de2ce557af68f1fe0ac0b
Tree-SHA512: f227e2fd22021e39f0445ec041f4a299d13477c23cef0fc06c53fb3313cbe550cec329336224a7e8775d9045b8009423052b394e83d42a1e40772085dfcdd471
|
|
progress estimation
fac22fd36b2d9f55dada31cc0da55520431b972a log: Remove GetAdjustedTime from IBD header progress estimation (MarcoFalke)
Pull request description:
This is a "refactor" that shouldn't change behaviour, because the two times are most likely equal. A minimum of 5 outbound peers are needed to adjust the time. And if the time is adjusted, it will be by at most 70 minutes (`DEFAULT_MAX_TIME_ADJUSTMENT`). Thus, the progress estimate should differ by at most 7 blocks.
ACKs for top commit:
laanwj:
Code review ACK fac22fd36b2d9f55dada31cc0da55520431b972a
vincenzopalazzo:
ACK https://github.com/bitcoin/bitcoin/pull/24033/commits/fac22fd36b2d9f55dada31cc0da55520431b972a
Tree-SHA512: bf9f5eef66db0110dd268cf6dbfab64b9c11ba776924f5b386ceae3f2d005272cceb87ebcc96e0c8b854c051514854a2a5af39ae43bad008fac685b5aafaabd0
|
|
fa7238300c18938cdf627cacfc58d4b81602417f fuzz: Limit fuzzed time to years 2000-2100 (MarcoFalke)
Pull request description:
It doesn't make sense to fuzz times in the past, as Bitcoin Core will refuse to start in the past.
Fix that and also remove a sanitizer suppression, which would be hit in net_processing in `ProcessMessage`:
```cpp
if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60)
addr.nTime = nNow - 5 * 24 * 60 * 60; // <-- Here
```
This changes the format of fuzz inputs. Previously a time value was (de)serialized as 40 bytes, now it is 32 bytes.
ACKs for top commit:
mzumsande:
Code Review ACK fa7238300c18938cdf627cacfc58d4b81602417f
Tree-SHA512: ca6e7233beec2d9ef9fd481d8f1331942a4d2c8fe518b857629bebcc53a4f42ae123b994cf5d359384a0a8022098ff5a9c146600bc2593c6d88734e25bc240ad
|
|
fa4c72e706e7a7bb3b271bd95f0398952daa9051 doc: Rework 14707 release notes (MarcoFalke)
fa9377c7464ed37ae1dc007250cffb9dc9d19a78 doc: move-only release note snippets (MarcoFalke)
Pull request description:
Requested by myself in https://github.com/bitcoin/bitcoin/pull/14707/files#r764313750
ACKs for top commit:
andrewtoth:
ACK fa4c72e706e7a7bb3b271bd95f0398952daa9051
Tree-SHA512: 091b9cb511ed3bef1e3d48ba5917ee23dd4bbb0f1daf999ca2b429fcff57b8876375b8579ac0a8ec438dc5c75d536d6d27074f325f51ffd38b0edc284d0c0155
|
|
In each of the critical sections, only the the guarded variable is
accessed, without any chance that within one section another one is
called. Hence, we can use an ordinary Mutex instead of RecursiveMutex.
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/cs_SubVer/m_subver_mutex/g' ./src/net.h ./src/net.cpp ./src/net_processing.cpp
-END VERIFY SCRIPT-
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/std::string m_command;/std::string m_type;/g' ./src/net.h
sed -i 's/* command and size./* type and size./g' ./src/net.h
sed -i 's/msg.m_command/msg.m_type/g' ./src/net.cpp ./src/net_processing.cpp ./src/test/fuzz/p2p_transport_serialization.cpp
-END VERIFY SCRIPT-
|
|
|
|
In each of the critical sections, only the the guarded variables are
accessed, without any chance that within one section another one is
called. Hence, we can use an ordinary Mutex instead of RecursiveMutex.
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/m_cs_callbacks_pending/m_callbacks_mutex/g' ./src/scheduler.h ./src/scheduler.cpp
-END VERIFY SCRIPT
|
|
|
|
Can be reviewed with
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
|
|
aa8a65e4a88bfbd83ac756a87bfb8faf52fb675d test: use MiniWallet for mempool_accept.py (Sebastian Falbesoner)
b24f6c6855bdd09bf445faeebe9d54c3d07a46d9 test: MiniWallet: support default `from_node` for creating txs (Sebastian Falbesoner)
f30041c9143d0added18105c9f0c4ae3f340efbc test: create txs with current `nVersion` (2) by default (Sebastian Falbesoner)
2f7978682245ada40e7f72c6f28bf2379fdc8ce2 test: refactor: add constant for sequence number `SEQUENCE_FINAL` (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (mempool_accept.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in #20078.
It also includes some other minor changes that came up while working on the replacement:
* [commit 1/4] replace magic number 0xffffffff for a tx's nSequence with a new constant `SEQUENCE_FINAL`
* [commit 2/4] create `CTransaction` instances with the current nVersion=2 by default, in order to use BIP68 for tests
* [commit 3/4] support default `from_node` parameter for creating txs (this is a stripped down version of PR #24025)
ACKs for top commit:
MarcoFalke:
re-ACK aa8a65e4a88bfbd83ac756a87bfb8faf52fb675d 📊
Tree-SHA512: 34cd085ea4147ad5bd3f3321c84204064ceb95f382664c7fe29062c1bbc79d9d9465c5e46d35e11c416f2f3cd46030c90a09b518c829c73ae40d060be5e4c9cb
|