3b19100303 depends: remove PYTHONPATH from config.site (fanquake)
Pull request description:
We no-longer need this, as we no-longer build python packages.
ACKs for top commit:
hebasto:
ACK 3b19100303, this PR effectively reverts no longer needed de619a37fd.
Tree-SHA512: 775354773f83fc98922f1d4ee84d8f1e866fb6fb2a59a3eaf06a7a5f0d846f7dc1b84862c58195dfb91ddfb02b2dc86bee78b51459f91c65a5b1464df9f3c53c
6a917918b7 fuzz: allow fake and duplicate inputs in tx_package_eval target (Greg Sanders)
a0626ccdad fuzz: allow reaching MempoolAcceptResult::ResultType::DIFFERENT_WITNESS in tx_package_eval target (Greg Sanders)
Pull request description:
Exercises `DIFFERENT_WITNESS` by using "blank" WSH() and allowing witness to determine wtxid, and attempts to make invalid/duplicate inputs.
ACKs for top commit:
dergoegge:
Coverage looks good to me ACK 6a917918b7
Tree-SHA512: db894f5f5b81c6b454874baf11f296462832285f41ccb09f23c0db92b9abc98f8ecacd72fc8f60dc92cb7947f543a2e55bed2fd210b0e8ca7c7d5389d90b14af
821a8a1125 doc: remove x86_64 build assumption from depends doc (fanquake)
Pull request description:
This dates from the introduction of depends, and has not been the case for some time now.
ACKs for top commit:
maflcko:
lgtm ACK 821a8a1125
hebasto:
ACK 821a8a1125.
theuni:
ACK 821a8a1125
Tree-SHA512: 640967a3e6dfab495fd733d3379aa916ac7f67e89a92ef6a94c3bea0494dc7921a9d7485e1b90a1beab00548b575cdab8fb08eb9267dcc5e890cc796ae1b6875
30bd4b1e4a doc: remove mention of missing bdb being a configure error (fanquake)
Pull request description:
This is no-longer the case, unless you're passing additional flags, which is not the case in this example.
ACKs for top commit:
maflcko:
lgtm ACK 30bd4b1e4a
TheCharlatan:
ACK 30bd4b1e4a
hebasto:
ACK 30bd4b1e4a.
Tree-SHA512: b3730546d7ff1f49854b88e710c72c4f6e4b6d238147599d4c4e4adeeb256424c2096635f6c51dcfe2e5a9c1155c1c9915fe03a09c5c38605bee2722756c8f6e
defdf67765 contrib: use a raw string for a regular expression literal that contains backslashes in signet/miner (muxator)
Pull request description:
Running `contrib/signet/miner` under python >= 3.12 causes a `SyntaxWarning`. The problem was already present in previous versions, but it only triggered a `DeprecationWarning`, which was not shown by default.
The change is useful for future-proofing the code base, since future python versions will start to exit with a runtime exception (see the reference given later).
Command to see the warning at runtime under python3.11 (`DeprecationWarning`, needs "-Walways"):
```
$ python3.11 -Walways ./contrib/signet/miner
<BASE>/contrib/signet/miner:33: DeprecationWarning: invalid escape sequence '\d'
RE_MULTIMINER = re.compile("^(\d+)(-(\d+))?/(\d+)$")
2023-11-15 16:02:49 ERROR Must specify command
```
Command to see the warning at runtime under python3.12 (`SyntaxWarning`, no modifiers needed):
```
$ python3.12 ./contrib/signet/miner
<BASE>/contrib/signet/miner:33: SyntaxWarning: invalid escape sequence '\d'
RE_MULTIMINER = re.compile("^(\d+)(-(\d+))?/(\d+)$")
2023-11-15 16:03:00 ERROR Must specify command
```
Reference (https://docs.python.org/3.8/library/re.html):
> Regular expressions use the backslash character ('\') [...]. This collides with Python’s usage of the same character for the same purpose in string literals; [...]
>
> Also, please note that any invalid escape sequences in Python’s usage of the backslash in string literals now generate a DeprecationWarning and in the future this will become a SyntaxError.
>
> The solution is to use Python’s raw string notation for regular expression patterns;
ACKs for top commit:
maflcko:
lgtm ACK defdf67765
ajtowns:
utACK defdf67765
Tree-SHA512: 81bd4892938e7d40a226ca20b5b61ff2470ad763743528da290271faefc535167b56f44665e2d03ed2607c4f7bc8a3200e7931f98fe28dbaf0d2a842c96549f5
f718a74b12 guix: remove python-macholib (fanquake)
d3cbff16c2 guix: update signapple (fanquake)
Pull request description:
Update to the latest signapple, which includes https://github.com/achow101/signapple/pull/13.
Drop python-macholib and python-altgraph.
ACKs for top commit:
Sjors:
ACK f718a74b12
Tree-SHA512: 199b2108f2f063b6b0fb5354ac79a30b46e848c923ebe7d02f7d7d3f08749817a1f6b4c14d21658fd2f2d68f8be1698e1999edf7e2366b1cae3bf2709a665e30
a0c254c13a Drop CHashWriter (Anthony Towns)
c94f7e5b1c Drop OverrideStream (Anthony Towns)
6e9e4e6130 Use ParamsWrapper for witness serialization (Anthony Towns)
Pull request description:
Choose whether witness is included in transaction serialization via serialization parameter rather than the stream version. See #25284 and #19477 for previous context.
ACKs for top commit:
maflcko:
re-ACK a0c254c13a🐜
theuni:
ACK a0c254c13a
Tree-SHA512: 8fd5cadfd84c5128e36c34a51fb94fdccd956280e7f65b7d73c512d6a9cdb53cdd3649de99ffab5322bd34be26cb95ab4eb05932b3b9de9c11d85743f50dcb13
Running the miner under python >= 3.12 causes a SyntaxWarning. The problem was
already present in previous versions, but it only triggered a
DeprecationWarning, which was not shown by default.
The change is useful for future-proofing the code base, since future python
versions will start to exit with a runtime exception (see the reference given
later).
Command to see the warning at runtime under python3.11 (DeprecationWarning,
needs "-Walways"):
$ python3.11 -Walways ./contrib/signet/miner
<BASE>/contrib/signet/miner:33: DeprecationWarning: invalid escape sequence '\d'
RE_MULTIMINER = re.compile("^(\d+)(-(\d+))?/(\d+)$")
2023-11-15 16:02:49 ERROR Must specify command
Command to see the warning at runtime under python3.12 (SyntaxWarning, no
modifiers needed):
$ python3.12 ./contrib/signet/miner
<BASE>/contrib/signet/miner:33: SyntaxWarning: invalid escape sequence '\d'
RE_MULTIMINER = re.compile("^(\d+)(-(\d+))?/(\d+)$")
2023-11-15 16:03:00 ERROR Must specify command
Reference ( https://docs.python.org/3.8/library/re.html ):
Regular expressions use the backslash character ('\') [...]. This collides
with Python’s usage of the same character for the same purpose in string
literals; [...]
Also, please note that any invalid escape sequences in Python’s usage of the
backslash in string literals now generate a DeprecationWarning and in the
future this will become a SyntaxError.
The solution is to use Python’s raw string notation for regular expression
patterns;
fd30e9688e test: migrate to some per-symbol ubsan suppressions (fanquake)
Pull request description:
Now that the symbolizer should be hanging around (#28814), migrate some file-wide suppressions to be symbol specific. Should assist in catching new issues that may otherwise go unnoticed due to file-wide suppression.
Only tested (so far) on aarch64 using the native ASAN & FUZZ CI.
ACKs for top commit:
maflcko:
lgtm ACK fd30e9688e
dergoegge:
utACK fd30e9688e (if CI is green)
Tree-SHA512: fbc44464d22813969dd4d1cdeab00042fa45f0af9bf1aed4fd3b688dc7b3c377a7c0f5f0c0a37ba65b649cfb5c7ff8ab2774500fe182d702c4340ca19f08479f
d799ea26ed doc: rewrite explanation for -par= (fanquake)
Pull request description:
The negative bound for script threads comes from the machine which generates the man pages, so may only be correct for that machine. Any other placeholder value will also be wrong for some machines. Fix this be removing the value. This also fixes help2man incorrectly bolding the value, as if it were a paramater.
Closes#28850.
ACKs for top commit:
maflcko:
lgtm ACK d799ea26ed
theStack:
ACK d799ea26ed
Tree-SHA512: 2eec0086faf4cc64bbf46b22949662f84d8546d2322c3d507fc44a4e1f64d228a2901af4fa4535c0771e3e14600be8308fc5dbd407b66ae6ae4f8878d8372c0a
1e5b86171e test: Add test for array serialization (TheCharlatan)
d49d198840 refactor: Initialize magic bytes in constructor initializer (TheCharlatan)
Pull request description:
This is a followup-PR for #28423
* Initialize magic bytes in constructor
* Add a small unit test for serializing arrays.
ACKs for top commit:
sipa:
utACK 1e5b86171e
maflcko:
lgtm ACK 1e5b86171e
Tree-SHA512: 0f58d2332dc501ca9fd419f40ed4f977c83dce0169e9a0eee1ffc9f8daa2d2ef7e7df18205ba076f55d90ae6c4a20d2b51ab303150d38470a962bcc58a66f6e7
3c61c60b90 build: Add an old hack to remove bind_at_load from libtool. (Cory Fields)
45257601da build: remove -bind_at_load usage (fanquake)
Pull request description:
This is deprecated on macOS:
```bash
ld: warning: -bind_at_load is deprecated on macOS
```
and likely redundant anyways, given the behaviour of dyld3.
Unfortunately libtool is still injecting a `-bind_at_load`, because it's version check is broken:
```bash
# Don't allow lazy linking, it breaks C++ global constructors
# But is supposedly fixed on 10.4 or later (yay!).
if test CXX = "$tagname"; then
case ${MACOSX_DEPLOYMENT_TARGET-10.0} in
10.[0123])
func_append compile_command " $wl-bind_at_load"
func_append finalize_command " $wl-bind_at_load"
;;
esac
fi
```
so this adds another change to strip them out at the end of configure.
Note that anywhere the ld64 warnings are being emitted, we are already not adding this flag to our hardened ldflags, because of `-Wl,-fatal_warnings`.
ACKs for top commit:
theuni:
utACK 3c61c60b90.
hebasto:
ACK 3c61c60b90, tested on macOS Sonoma 14.1.1 (23B81, Apple M1) and Ubuntu 23.10 (cross-compiling for macOS). Also I've verified the actual diff in the `libtool` script.
Tree-SHA512: 98e6a095dc2d2409f8ec3b9d462e0db3643d7873d7903a12f8acd664829e7e84e797638556fa42ca8ebc1003f13a38fe9bb8a2a50cecfa991155da818574bf08
49a92579c7 build: latest config.sub in depends (fanquake)
ced0435a71 build: latest config.guess in depends (fanquake)
Pull request description:
Before we make any local modifications (i.e #28733) pull the latest files from upstream.
ACKs for top commit:
TheCharlatan:
ACK 49a92579c7
Tree-SHA512: fbbe0d6ef72a196a652467af0550b38da23b932fe68da4965a9b0dc4795db9c869969db98f660cd360f6af3a7659b46c25e3fd398e0ef127dae71726b9a915a6
fa6b053b5c mempool: persist with XOR (MarcoFalke)
Pull request description:
Currently the `mempool.dat` file stores data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan the file and move it into quarantine, or delete it, or corrupt it.
While the local wallet is expected to re-submit any pending transactions, unrelated transactions may be missing from the mempool after a restart. This may cause fee estimates to be off, or may cause block relay to be slower.
Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a random XOR pattern over the dat file when writing or reading it.
Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat file. Any program that intentionally wants to mess with the dat file can still trivially do so.
ACKs for top commit:
achow101:
re-ACK fa6b053b5c
glozow:
reACK fa6b053b5c
ismaelsadeeq:
ACK fa6b053b5c
Tree-SHA512: ded2ce3d81bc944b828263534e3178a1e45a914fe8e024f4a14c6561a73e301820944ecc75dd704b3d4221a7a3a5c0597ccab79546250c1197609ee981fe324e
bbbbdb0cd5 ci: Add filesystem lint check (MarcoFalke)
fada2f9110 refactor: Replace <filesystem> with <util/fs.h> (MarcoFalke)
Pull request description:
Using `std::filesystem` is problematic:
* There is a `fs` namespace wrapper for it. So having two ways to achieve the same is confusing.
* Not using the `fs` wrapper is dangerous and buggy, because it disables known bugs by deleting problematic functions.
Fix all issues by removing use of it and adding a linter to avoid using it again in the future.
ACKs for top commit:
TheCharlatan:
ACK bbbbdb0cd5
fanquake:
ACK bbbbdb0cd5🦀
Tree-SHA512: 0e2d49742b08eb2635e6fce41485277cb9c40fe20b81017c391d3472a43787db1278a236825714ca1e41c9d2f59913865cfb0c649e3c8ab1fb598c849f80c660
3b70f7b615 doc: fix broken doc/design/multiprocess.md links after #24352 (Ryan Ofsky)
6d43aad742 span: Make Span template deduction guides work in SFINAE context (Ryan Ofsky)
8062c3bdb9 util: Add ArgsManager SetConfigFilePath method (Ryan Ofsky)
441d00c60f interfaces: Rename CalculateBumpFees methods to be compatible with capn'proto (Ryan Ofsky)
156f49d682 interfaces: Change getUnspentOutput return type to avoid multiprocess segfault (Ryan Ofsky)
4978754c00 interfaces: Add schedulerMockForward method so mockscheduler RPC can work across processes (Ryan Ofsky)
924327eaf3 interfaces: Fix const virtual method that breaks multiprocess support (Ryan Ofsky)
82a379eca8 streams: Add SpanReader ignore method (Russell Yanofsky)
Pull request description:
This is a collection of small changes to interfaces and code which were needed as part of multiprocess PR #10102, but have been moved here to make that PR smaller.
All of these changes are refactoring changes which do not affect behavior of current code
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
achow101:
ACK 3b70f7b615
naumenkogs:
ACK 3b70f7b615
maflcko:
re-ACK 3b70f7b615🎆
Tree-SHA512: 2368772b887056ad8a9f84c299cfde76ba45943770e3b5353130580900afa9611302195b899ced7b6e303b11f053ff204cae7c28ff4e12c55562fcc81119ba4c
The negative bound for script threads comes from the machine which
generates the man pages, so may only be correct for that machine. Any
other placeholder value will also be wrong for some machines. Fix this
be removing the value. This also fixes help2man incorrectly bolding the
value, as if it were a paramater.
Closes#28850.
fca0a8938e ci: remove "--exclude banman" for fuzzing in mac (brunoerg)
f9b286353f fuzz: call lookup functions before calling `Ban` (brunoerg)
Pull request description:
Fixes#27924
To not have any discrepancy, it's required to call lookup functions before calling `Ban`. If we don't do it, the assertion `assert(banmap == banmap_read);` may fail because `BanMapFromJson` will call `LookupSubNet` and cause the discrepancy between the banned and the loaded one. It happens especially in MacOS (#27924).
Also, calling lookup functions before banning is what RPC `setban` does.
ACKs for top commit:
maflcko:
lgtm ACK fca0a8938e
dergoegge:
ACK fca0a8938e
Tree-SHA512: a3d635088a556df4507e65542157f10b41d4f87dce42927b58c3b812f262f4544b6b57f3384eef1097ffdd7c32b8dd1556aae201254960cbfbf48d45551200f7
4dd94ca18f [refactor] remove access to mapTx in validation_block_tests (TheCharlatan)
d0cd2e804e [refactor] rewrite BlockAssembler inBlock and failedTx as sets of txids (glozow)
55b0939cab scripted-diff: rename vTxHashes to txns_randomized (TheCharlatan)
a03aef9cec [refactor] rewrite vTxHashes as a vector of CTransactionRef (glozow)
938643c3b2 [refactor] remove access to mapTx in validation.cpp (glozow)
333367a940 [txmempool] make CTxMemPoolEntry::lockPoints mutable (glozow)
1bf4855016 [refactor] use CheckPackageLimits for checkChainLimits (glozow)
dbc5bdbf59 [refactor] remove access to mapTx.find in mempool_tests.cpp (glozow)
f80909e7a3 [refactor] remove access to mapTx in blockencodings_tests.cpp (glozow)
8892d6b744 [refactor] remove access to mapTx from rpc/mempool.cpp (glozow)
fad61aa561 [refactor] get wtxid from entry instead of vTxHashes (glozow)
9cd8cafb77 [refactor] use exists() instead of mapTx.find() (glozow)
14804699e5 [refactor] remove access to mapTx from policy/rbf.cpp (glozow)
1c6a73abbd [refactor] Add helper for retrieving mempool entry (TheCharlatan)
453b4813eb [refactor] Add helper for iterating through mempool entries (stickies-v)
Pull request description:
Motivation
* It seems preferable to use stdlib data structures instead of boost if they can achieve close to the same thing.
* Code external to mempool should ideally use its public helper methods instead of accessing `mapTx` or its iterators directly.
* Reduce the number of complex boost multi index type interactions
* Also see #28335 for further context/motivation. This PR together with #28385 simplifies that one.
Overview of things done in this PR:
* Make `vTxHashes` a vector of transaction references instead of a pair of transaction hash and iterator. The trade off here is that the data is retrieved on the fly with `GetEntry` instead of being cached in `vTxHashes`.
* Introduce `GetEntry` helper method to replace the more involved `GetIter` where applicable
* Replace `mapTx` access with `CTxMemPool` helper methods
* Simplify `checkChainLimits` call in `node/interfaces.cpp`
* Make `CTxMemPoolEntry`s `lockPoints`mutable such that they can be changed with a const iterator directly instead of going through `mapTx`
* Make `BlockAssembler`'s `inBlock` and `failedTx` sets of transaction hashes.
ACKs for top commit:
glozow:
reACK 4dd94ca
maflcko:
re-ACK 4dd94ca18f👝
stickies-v:
re-ACK 4dd94ca18f
Tree-SHA512: c4d043f2186e4fde337591883fac66cade3058173987b49502bd65cecf69207a3df1077f6626809652ab63230013167b7f39a2b39f1c5166959e5495df57065f
44445ae8f1 test: Avoid intermittent failures in feature_init (MarcoFalke)
Pull request description:
The code not only modifies block dat files, but also leveldb files, which may be of smaller size. Such corruption may not force leveldb to abort, according to the intermittent test failures.
Fix the intermittent test failures by reverting 5ab6419f38 .
ACKs for top commit:
kevkevinpal:
lgtm ACK [44445ae](44445ae8f1)
fjahr:
ACK 44445ae8f1
theStack:
ACK 44445ae8f1
Tree-SHA512: 8084e4aeb8a976c1706a1898d7854c55d0c4ec7b5a08f65f97ffc173c935f9b0e0c1caef7be1538a458e4c018f7bd1948173349ec76ca48bc4013a63f284bb0f
ebc7063c80 doc: update docs for CHECK_ATOMIC macro (fanquake)
Pull request description:
Clarify that supported versions of GCC are not affected, and that Clang
prior to version 15 still requires the explicit `-latomic` linking, when
compiling for 32-bit.
ACKs for top commit:
hebasto:
ACK ebc7063c80.
Tree-SHA512: 6044dc28547431cfde7e89b663b5f9a86a4cb801212a21c3dbb18a1c41a53640480c3e4e944050dc3ec4cded9bc4c1f8eae8dbb60596289fef49bb13a8b53b76
380e365563 guix: switch to 6.1 kernel headers over 5.15 (fanquake)
Pull request description:
6.1 is the current longterm release: https://kernel.org/.
Note that using an older version of the kernel headers inside Guix, is not a "hack" for compatibility, and is explicitly recommended against by glibc:
https://sourceware.org/glibc/wiki/FAQ#What_version_of_the_Linux_kernel_headers_should_be_used.3F.
so using the latest version of the longterm headers seems appropriate.
The last time we changed this was when we consolidated all builds to 5.15, in #25006.
ACKs for top commit:
TheCharlatan:
ACK 380e365563
Tree-SHA512: 78eb601e10261d99afd030dd7d039d962c106c48a57f16deb1c65b68fee4831e1070e4c35201f567fd24bbdab30a2b00804ddd118e1fee1dc8cdac7a3fb32ac5
22e38080ea test: fix node index bug when comparing peerinfo (Kashif Smith)
Pull request description:
fix node index bug when comparing peerinfo in test/functional/p2p_v2_transport.py
ACKs for top commit:
theStack:
ACK 22e38080ea
mzumsande:
ACK 22e38080ea, good find!
Tree-SHA512: 9ee336eea999c61fb9f8704cc6361cf289fd3a361ab636c97695121ca3bcb8b38fbbfb55484311c17faa76d02065d91d190c489e1f3defd628216bf80a93f1fe
Allows calling UpdateLockPoints() with a (const) txiter. Note that this
was already possible for caller using mapTx.modify(txiter). The point
here is to not be accessing mapTx when doing so.
The behavior is the same as CalculateMemPoolAncestors. The only
difference is the string returned, and the string is discarded anyway
since checkChainLimits only cares about pass/fail.
