| Age | Commit message (Collapse) | Author |
|---|
|
This was deprecated in v27.0, for removal in v28.0.
See discussion in PR #29189.
|
|
not-on-curve P2PK outputs
28287cfbe18b6c468f76389e9f66bdcf2d00f8d1 test: add script compression coverage for not-on-curve P2PK outputs (Sebastian Falbesoner)
Pull request description:
This PR adds unit test coverage for the script compression functions `{Compress,Decompress}Script` in the special case of uncompressed P2PK outputs (scriptPubKey: OP_PUSH65 <0x04 ....> OP_CHECKSIG) with [pubkeys that are not fully valid](https://github.com/bitcoin/bitcoin/blob/44b05bf3fef2468783dcebf651654fdd30717e7e/src/pubkey.cpp#L297-L302), i.e. where the encoded point is not on the secp256k1 curve. For those outputs, script compression is not possible, as the y coordinate of the pubkey can't be recovered (see also call-site of `IsToPubKey`):
https://github.com/bitcoin/bitcoin/blob/44b05bf3fef2468783dcebf651654fdd30717e7e/src/compressor.cpp#L49-L50
Likewise, for a compressed script of an uncompressed P2PK script (i.e. compression ids 4 and 5) where the x coordinate is not on the curve, decompression fails:
https://github.com/bitcoin/bitcoin/blob/44b05bf3fef2468783dcebf651654fdd30717e7e/src/compressor.cpp#L122-L129
Note that the term "compression" is used here in two different meanings (though they are related), which might be a little confusing. The encoding of a pubkey can either be compressed (33-bytes with 0x02/0x03 prefixes) or uncompressed (65-bytes with 0x04 prefix). On the other hand there is also compression for whole output scripts, which is used for storing scriptPubKeys in the UTXO set in a compact way (and also for the `dumptxoutset` result, accordingly). P2PK output scripts with uncompressed pubkeys get compressed by storing only the x-coordinate and the sign as a prefix (0x04 = even, 0x05 = odd). Was diving deeper into the subject while working on https://github.com/bitcoin/bitcoin/pull/27432, where the script decompression of uncompressed P2PK needed special handling (see also https://github.com/bitcoin/bitcoin/issues/24628#issuecomment-1108798536).
Trivia: as of now (block 801066), there are 13 uncompressed P2PK outputs in the UTXO set with a pubkey not on the curve (which obviously means they are unspendable).
ACKs for top commit:
achow101:
ACK 28287cfbe18b6c468f76389e9f66bdcf2d00f8d1
tdb3:
ACK for 28287cfbe18b6c468f76389e9f66bdcf2d00f8d1.
cbergqvist:
ACK 28287cf!
marcofleon:
Nicely done, ACK 28287cfbe18b6c468f76389e9f66bdcf2d00f8d1. Built the PR branch, ran the unit and functional tests, everything passed.
Tree-SHA512: 777b6c3065654fbfa1ce94926f4cadb91a9ca9dc4dd4af6008ad77bd1da5416f156ad0dfa880d26faab2e168bf9b27e0a068abc9a2be2534d82bee61ee055c65
|
|
-onion
567cec9a05e1261e955535f734826b12341684b6 doc: add release notes and help text for unix sockets (Matthew Zipkin)
bfe51928911daf484ae07deb52a7ff0bcb2526ae test: cover UNIX sockets in feature_proxy.py (Matthew Zipkin)
c65c0d01630b44fa71321ea7ad68d5f9fbb7aefb init: allow UNIX socket path for -proxy and -onion (Matthew Zipkin)
c3bd43142eba77dcf1acd4984e437759f65e237a gui: accomodate unix socket Proxy in updateDefaultProxyNets() (Matthew Zipkin)
a88bf9dedd1d8c1db0a9c8b663dab3e3c2f0f030 i2p: construct Session with Proxy instead of CService (Matthew Zipkin)
d9318a37ec09fe0b002815a7e48710e530620ae2 net: split ConnectToSocket() from ConnectDirectly() for unix sockets (Matthew Zipkin)
ac2ecf3182fb5ad9bcd41540b19382376114d6ee proxy: rename randomize_credentials to m_randomize_credentials (Matthew Zipkin)
a89c3f59dc44eaf4f59912c1accfc0ce5d61933a netbase: extend Proxy class to wrap UNIX socket as well as TCP (Matthew Zipkin)
3a7d6548effa6cd9a4a5413b690c2fd85da4ef65 net: move CreateSock() calls from ConnectNode() to netbase methods (Matthew Zipkin)
74f568cb6fd5c74b7b9bf0ce69876430746a53b1 netbase: allow CreateSock() to create UNIX sockets if supported (Matthew Zipkin)
bae86c8d318d06818aa75a9ebe3db864197f0bc6 netbase: refactor CreateSock() to accept sa_family_t (Matthew Zipkin)
adb3a3e51de205cc69b1a58647c65c04fa6c6362 configure: test for unix domain sockets (Matthew Zipkin)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/27252
UNIX domain sockets are a mechanism for inter-process communication that are faster than local TCP ports (because there is no need for TCP overhead) and potentially more secure because access is managed by the filesystem instead of serving an open port on the system.
There has been work on [unix domain sockets before](https://github.com/bitcoin/bitcoin/pull/9979) but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side, specifically connecting to a local Tor proxy (Tor can listen on unix sockets and even enforces strict curent-user-only access permission before binding) configured by `-onion=` or `-proxy=`
I copied the prefix `unix:` usage from Tor. With this patch built locally you can test with your own filesystem path (example):
`tor --SocksPort unix:/Users/matthewzipkin/torsocket/x`
`bitcoind -proxy=unix:/Users/matthewzipkin/torsocket/x`
Prep work for this feature includes:
- Moving where and how we create `sockaddr` and `Sock` to accommodate `AF_UNIX` without disturbing `CService`
- Expanding `Proxy` class to represent either a `CService` or a UNIX socket (by its file path)
Future work:
- Enable UNIX sockets for ZMQ (https://github.com/bitcoin/bitcoin/pull/27679)
- Enable UNIX sockets for I2P SAM proxy (some code is included in this PR but not tested or exposed to user options yet)
- Enable UNIX sockets on windows where supported
- Update Network Proxies dialog in GUI to support UNIX sockets
ACKs for top commit:
Sjors:
re-ACK 567cec9a05e1261e955535f734826b12341684b6
tdb3:
re ACK for 567cec9a05e1261e955535f734826b12341684b6.
achow101:
ACK 567cec9a05e1261e955535f734826b12341684b6
vasild:
ACK 567cec9a05e1261e955535f734826b12341684b6
Tree-SHA512: de81860e56d5de83217a18df4c35297732b4ad491e293a0153d2d02a0bde1d022700a1131279b187ef219651487537354b9d06d10fde56225500c7e257df92c1
|
|
0a533613fb44207053796fd01a9f4b523a3153d4 docs: add release notes for #27114 (brunoerg)
e6b8f19de9a6d1c477d0bbda18d17794cd81a6f4 test: add coverage for whitelisting manual connections (brunoerg)
c985eb854cc86deb747caea5283c17cf51b6a983 test: add option to speed up tx relay/mempool sync (brunoerg)
66bc6e2d1749f43d7b314aa2784a06af78440170 Accept "in" and "out" flags to -whitelist to allow whitelisting manual connections (Luke Dashjr)
8e06be347c5e14cbe75256eba170e0867f95f360 net_processing: Move extra service flag into InitializeNode (Luke Dashjr)
9133fd69a5cc9a0ab1a06a60d09f1b7e1039018e net: Move `NetPermissionFlags::Implicit` verification to `AddWhitelistPermissionFlags` (Luke Dashjr)
2863d7dddb62d987b3e1c3b8bfad7083f0f774b2 net: store `-whitelist{force}relay` values in `CConnman` (brunoerg)
Pull request description:
Revives #17167. It allows whitelisting manual connections. Fixes #9923
Since there are some PRs/issues around this topic, I'll list some motivations/comments for whitelisting outbound connections from them:
- Speed-up tx relay/mempool sync for testing purposes (my personal motivation for this) - In #26970, theStack pointed out that we whitelist peers to speed up tx relay for fast mempool synchronization, however, since it applies only for inbound connections and considering the topology `node0 <--- node1 <---- node2 <--- ... <-- nodeN`, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long.
- https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1865155764 - "Before enabling -v2transport by default (which I'd image may happen after https://github.com/bitcoin/bitcoin/pull/24748) we could consider a way to force manual connections to be only-v1 or even only-v2 (disabling reconnect-with-v1). A possibility could be through a net permission flag, if https://github.com/bitcoin/bitcoin/pull/27114 makes it in."
- https://github.com/bitcoin/bitcoin/pull/17167#issuecomment-1168606032 - "This would allow us to use https://github.com/bitcoin/bitcoin/pull/25355 when making outgoing connections to all nodes, except to whitelisted ones for which we would use our persistent I2P address."
- Force-relay/mempool permissions for a node you intentionally connected to.
ACKs for top commit:
achow101:
ACK 0a533613fb44207053796fd01a9f4b523a3153d4
sr-gi:
re-ACK [0a53361](https://github.com/bitcoin/bitcoin/pull/27114/commits/0a533613fb44207053796fd01a9f4b523a3153d4)
pinheadmz:
ACK 0a533613fb44207053796fd01a9f4b523a3153d4
Tree-SHA512: 97a79bb854110da04540897d2619eda409d829016aafdf1825ab5515334b0b42ef82f33cd41587af235b3af6ddcec3f2905ca038b5ab22e4c8a03d34f27aebe1
|
|
1342a31f3ab61d964b9a24825bee24f53ba4e1cc [functional test] sibling eviction (glozow)
5fbab378597126eb1d0c2b2addb0859f79e508f4 [unit test] sibling not returned from SingleV3Checks if 1p2c or 3gen (glozow)
170306728aa23a4d5fcc383ddabd97f66fed5119 [policy] sibling eviction for v3 transactions (glozow)
b5d15f764fed0b30c9113429163700dea907c2b1 [refactor] return pair from SingleV3Checks (glozow)
Pull request description:
When we receive a v3 transaction that would bust a mempool transaction's descendant limit, instead of rejecting the new tx, consider replacing the other descendant if it is much higher feerate (using existing RBF criteria to assess that it's more incentive compatible and to avoid DoS).
Delving post with more background and motivation: https://delvingbitcoin.org/t/sibling-eviction-for-v3-transactions/472
ACKs for top commit:
sdaftuar:
ACK 1342a31f3ab61d964b9a24825bee24f53ba4e1cc
achow101:
ACK 1342a31f3ab61d964b9a24825bee24f53ba4e1cc
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/29306/commits/1342a31f3ab61d964b9a24825bee24f53ba4e1cc
Tree-SHA512: dd957d49e51db78758f566c49bddc579b72478e371275c592d3d5ba097d20de47a6c81952045021b99d82a787f5b799baf16dd0ee0e6de90ba12e21e275352be
|
|
d27e2d87b95b7982c05b4c88e463cc9626ab9f0a test: test_bitcoin: allow -testdatadir=<datadir> (Larry Ruane)
Pull request description:
This backward-compatible change would help with code review, testing, and debugging. When `test_bitcoin` runs, it creates a working or data directory within `/tmp/test_common_Bitcoin\ Core/`, named as a long random (hex) string.
This small patch does three things:
- If the (new) argument `-testdatadir=<datadir>` is given, use `<datadir>/test_temp/<test-name>/datadir` as the working directory
- When the test starts, remove `<datadir>/test_temp/<test-name>/datadir` if it exists from an earlier run (currently, it's presumed not to exist due to the long random string)
- Don't delete the working directory at the end of the test if a custom data directory is being used
Example usage, which will remove, create, use `/somewhere/test_temp/getarg_tests/boolarg`, and leave it afterward:
```
$ test_bitcoin --run_test=getarg_tests/boolarg -- -testdatadir=/somewhere
Running 1 test case...
Test directory (will not be deleted): "/somewhere/test_temp/getarg_tests/boolarg/datadir"
*** No errors detected
$ ls -l /somewhere/test_temp/getarg_tests/boolarg/datadir
total 8
drwxrwxr-x 2 larry larry 4096 Feb 22 10:28 blocks
-rw-rw-r-- 1 larry larry 1273 Feb 22 10:28 debug.log
```
(A relative pathname also works.)
This change affects only `test_bitcoin`; it could also be applied to `test_bitcoin-qt` but that's slightly more involved so I'm skipping that for now.
The rationale for this change is that, when running the test using the debugger, it's often useful to watch `debug.log` as the test runs and inspect some of the other files (I've looked at the generated `blknnnn.dat` files for example). Currently, that requires figuring out where the test's working directory is since it changes on every test run. Tests can be run with `-printtoconsole=1` to show debug logging to the terminal, but it's nice to keep `debug.log` continuously open in an editor, for example.
Even if not using a debugger, it's sometimes helpful to see `debug.log` and other artifacts after the test completes.
Similar functionality is already possible with the functional tests using the `--tmpdir=` and `--nocleanup` arguments.
ACKs for top commit:
davidgumberg:
ACK https://github.com/bitcoin/bitcoin/pull/26564/commits/d27e2d87b95b7982c05b4c88e463cc9626ab9f0a
tdb3:
re-ACK for d27e2d87b95b7982c05b4c88e463cc9626ab9f0a
achow101:
ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a
cbergqvist:
ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a! (Already did some testing with `fs::remove()` to make sure it was compatible with the `util::Lock/UnlockDirectory` implementation).
marcofleon:
ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a. I ran all the tests with my previous open file limit and no errors were detected. Also ran some individual tests with no, relative, and absolute paths and everything looks good.
furszy:
ACK d27e2d8
Tree-SHA512: a8f535f34a48b6699cb440f97f5562ec643f3bfba4ea685768980b871fc8b6e1135f70fc05dbe19aa2c8bacb1ddeaff212d63473605a7422ff76332b3a6b1f68
|
|
CalculateClaimedHeadersWork
eb7cc9fd2140df77acc6eb42004cf45b260bc629 Rename CalculateHeadersWork to CalculateClaimedHeadersWork (Greg Sanders)
Pull request description:
And clean up some comments. Confusion about what this is doing seems to be a running theme:
https://github.com/bitcoin/bitcoin/pull/29549#discussion_r1511113344
https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1141510303
ACKs for top commit:
achow101:
ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629
pablomartin4btc:
ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629
0xB10C:
ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629
dergoegge:
ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629
BrandonOdiwuor:
ACK eb7cc9fd2140df77acc6eb42004cf45b260bc629
Tree-SHA512: 6ccbc5e417155516487bb220753d189b5341dec05366db88a3fa5b1932eace21fbfaf23408c639bb54b36169a8d0a7536a1ee5e63b4ce5a3b70f2ff8407b6e07
|
|
d5228efb5391b31a9a0673019e43e7fa2cd4ac07 kernel: Remove dependency on CScheduler (TheCharlatan)
06069b3913dda048f5d640a662b0852f86346ace scripted-diff: Rename MainSignals to ValidationSignals (TheCharlatan)
0d6d2b650d1017691f48c9109a6cd020ab46aa73 scripted-diff: Rename SingleThreadedSchedulerClient to SerialTaskRunner (TheCharlatan)
4abde2c4e3fd9b66394b79874583bdc2a9132c36 [refactor] Make MainSignals RAII styled (TheCharlatan)
84f5c135b8118cbe15b8bfb4db80d61237987f64 refactor: De-globalize g_signals (TheCharlatan)
473dd4b97ae40e43e1a1a97fdbeb40be4855e9bc [refactor] Prepare for g_signals de-globalization (TheCharlatan)
3fba3d5deec6d7bae33823b8da7682f9b03d9deb [refactor] Make signals optional in mempool and chainman (TheCharlatan)
Pull request description:
By defining a virtual interface class for the scheduler client, users of the kernel can now define their own event consuming infrastructure, without having to spawn threads or rely on the scheduler design.
Removing `CScheduler` also allows removing the thread and exception modules from the kernel library.
To make the `CMainSignals` class easier to use from a kernel library perspective, remove its global instantiation and adopt RAII practices.
Renames `CMainSignals` to `ValidationSignals`, which more accurately describes its purpose and scope.
Also make the `ValidationSignals` in the `ChainstateManager` and CTxMemPool` optional. This could be useful in the future for using or testing these classes without having to instantiate any form of signal handling.
---
This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). It improves the kernel API and removes two modules from the kernel library.
ACKs for top commit:
maflcko:
re-ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07 π
ryanofsky:
Code review ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07. Just comment change since last review.
vasild:
ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07
furszy:
diff ACK d5228ef
Tree-SHA512: e93a5f10eb6182effb84bb981859a7ce750e466efd8171045d8d9e7fe46e4065631d9f6f533c5967c4d34c9bb7d7a67e9f4593bd4c5b30cd7b3bbad7be7b331b
|
|
Specifying this argument overrides the path location for test_bitcoin;
it becomes <datadir>/test_common_Bitcoin Core/<testname>/datadir. Also,
this directory isn't removed after the test completes. This can make it
easier for developers to study the results of a test (see the state of
the data directory after the test runs), and also (for example) have an
editor open on debug.log to monitor it across multiple test runs instead
of having to re-open a different pathname each time.
Example usage (note the "--" is needed):
test_bitcoin --run_test=getarg_tests/boolarg -- \
-testdatadir=/somewhere/mydatadir
This will create (if necessary) and use the data directory:
/somewhere/mydatadir/test_common_Bitcoin Core/getarg_tests/boolarg/datadir
Co-authored-by: furszy <mfurszy@protonmail.com>
|
|
Should fix the GCC compilation portion of #29517:
https://github.com/bitcoin/bitcoin/issues/29517#issuecomment-1973573314.
See also:
https://www.gnu.org/software/gnulib/manual/html_node/fopencookie.html
but note that FreeBSD has supported this function since 11.x.
|
|
|
|
|
|
Also implement CService::GetSAFamily() to provide sa_family_t
|
|
modernization
86b7f28d6c507155a9d3a15487ee883989b88943 serialization: use internal endian conversion functions (Cory Fields)
432b18ca8d0654318a8d882b28b20af2cb2d2e5d serialization: detect byteswap builtins without autoconf tests (Cory Fields)
297367b3bb062c57142747719ac9bf2e12717ce9 crypto: replace CountBits with std::bit_width (Cory Fields)
52f9bba889fd9b50a0543fd9fedc389592cdc7e5 crypto: replace non-standard CLZ builtins with c++20's bit_width (Cory Fields)
Pull request description:
This replaces #28674, #29036, and #29057. Now ready for testing and review.
Replaces platform-specific endian and byteswap functions. This is especially useful for kernel, as it means that our deep serialization code no longer requires bitcoin-config.h.
I apologize for the size of the last commit, but it's hard to avoid making those changes at once.
All platforms now use our internal functions rather than libc or platform-specific ones, with the exception of MSVC.
Sadly, benchmarking showed that not all compilers are capable of detecting and optimizing byteswap functions, so compiler builtins are instead used where possible. However, they're now detected via macros rather than autoconf checks.
This[ matches how libc++ implements std::byteswap for c++23](https://github.com/llvm/llvm-project/blob/main/libcxx/include/__bit/byteswap.h#L26).
I suggest we move/rename `compat/endian.h`, but I left that out of this PR to avoid bikeshedding.
#29057 pointed out some irregularities in benchmarks. After messing with various compilers and configs for a few weeks with these changes, I'm of the opinion that we can't win on every platform every time, so we should take the code that makes sense going forward. That said, if any real-world slowdowns are caused here, we should obviously investigate.
ACKs for top commit:
maflcko:
ACK 86b7f28d6c507155a9d3a15487ee883989b88943 π
fanquake:
ACK 86b7f28d6c507155a9d3a15487ee883989b88943 - we can finish pruning out the __builtin_clz* checks/usage once the minisketch code has been updated. This is more good cleanup pre-CMake & for the kernal.
Tree-SHA512: 715a32ec190c70505ffbce70bfe81fc7b6aa33e376b60292e801f60cf17025aabfcab4e8c53ebb2e28ffc5cf4c20b74fe3dd8548371ad772085c13aec8b7970e
|
|
|
|
|
|
25eab523897e790f4f4d7b49cdbf19d13e3b0fcc fuzz: add target for local addresses (brunoerg)
Pull request description:
This PR adds fuzz target for local address functions - (`AddLocal`, `RemoveLocal`, `SeenLocal`, `IsLocal`)
ACKs for top commit:
dergoegge:
ACK 25eab523897e790f4f4d7b49cdbf19d13e3b0fcc
vasild:
ACK 25eab523897e790f4f4d7b49cdbf19d13e3b0fcc
Tree-SHA512: 24faaab86dcd8835ba0e2d81fb6322a39a9266c7edf66415dbc4421754054f47efb6e0de4efdc7ea026b0686792658e86a526f7cf27cbc6cf9ed0c4aed376f97
|
|
|
|
An unnecessary check was added to the block mutation tests
in #29412 where IsBlockMutated is returning true for the invalid
reasons: we try to check mutation via transaction duplication,
but the merkle root is not updated before the check, therefore
the check fails because the provided root and the computed root
differ, but not because the block contains the same transaction twice.
The check is meaningless so it can be removed.
|
|
d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc [test] IsBlockMutated unit tests (dergoegge)
1ed2c9829700054526156297552bb47230406098 Add transaction_identifier::size to allow Span conversion (dergoegge)
1ec6bbeb8d27d31647d1433ccb87b362f6d81f90 [validation] Cache merkle root and witness commitment checks (dergoegge)
5bf4f5ba32da4627f152b54d266df9b2aa930457 [test] Add regression test for #27608 (dergoegge)
49257c0304828a185c273fcb99742c54bbef0c8e [net processing] Don't process mutated blocks (dergoegge)
2d8495e0800f5332748cd50eaad801ff77671bba [validation] Merkle root malleation should be caught by IsBlockMutated (dergoegge)
66abce1d98115e41f394bc4f4f52341960ddc839 [validation] Introduce IsBlockMutated (dergoegge)
e7669e1343aec4298fd30d539847963e6fa5619c [refactor] Cleanup merkle root checks (dergoegge)
95bddb930aa72edd40fdff52cf447202995b0dce [validation] Isolate merkle root checks (dergoegge)
Pull request description:
This PR proposes to check for mutated blocks early as a defense-in-depth mitigation against attacks leveraging mutated blocks.
We introduce `IsBlockMutated` which catches all known forms of block malleation and use it to do an early mutation check whenever we receive a `block` message.
We have observed attacks that abused mutated blocks in the past, which could have been prevented by simply not processing mutated blocks (e.g. https://github.com/bitcoin/bitcoin/pull/27608 for which a regression test is included in this PR).
ACKs for top commit:
achow101:
ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc
maflcko:
ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc π
fjahr:
Code review ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc
sr-gi:
Code review ACK https://github.com/bitcoin/bitcoin/commit/d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc
Tree-SHA512: 618ff4ea7f168e10f07504d3651290efbb1bb2ab3b838ffff3527c028caf6c52dedad18d04d3dbc627977479710930e200f2dfae18a08f627efe7e64a57e535f
|
|
Rather than a complicated set of tests to decide which bswap functions to
use, always prefer the compiler built-ins when available.
These builtins and fallbacks can all be removed once we're using c++23, which
adds std::byteswap.
|
|
connections
|
|
This code has been dead since https://github.com/bitcoin/bitcoin/pull/28967.
Required as a precondition for replacing Boost.Process with
cpp-subprocess to make diff for this code meaningful and reviewable.
The plan is to reintroduce Windows-specific code in this test
simultaneously with enabling Windows support in cpp-subprocess.
|
|
|
|
|
|
call for MinGW builds
d2fe90571e98e02617682561ea82acb1e2647827 test: Drop `x` modifier in `fsbridge::fopen` call for mingw builds (Hennadii Stepanov)
Pull request description:
The MinGW-w64 toolchain links executables to the old msvcrt C Runtime Library that does not support the `x` modifier for the [`_wfopen()`](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-170) function.
Fixes https://github.com/bitcoin/bitcoin/issues/29014.
ACKs for top commit:
maflcko:
ACK d2fe90571e98e02617682561ea82acb1e2647827
fanquake:
ACK d2fe90571e98e02617682561ea82acb1e2647827 - the plan here should still be to migrate to the newer windows runtime.
Tree-SHA512: 0269b66531e58c093ecda3a3e355a20ee8274e165d7e010f8f125881b3c8d4cfe801abdca4605d81efd3b2dbe9a81896968971f6f53da7f6c6093b76b47c5bc9
|
|
bit_width is a drop-in replacement with an exact meaning in c++, so there is
no need to continue testing/fuzzing/benchmarking.
|
|
The MinGW-w64 toolchain links executables to the old msvcrt C Runtime
Library that does not support the `x` modifier for the _wfopen()
function.
|
|
|
|
|
|
9d1dbbd4ceb8c04340927f5127195dc306adf3fc scripted-diff: Fix bitcoin_config_h includes (TheCharlatan)
Pull request description:
As mentioned in https://github.com/bitcoin/bitcoin/pull/26924#issuecomment-1403449932 and https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399, it is currently not safe to remove `bitcoin-config.h` includes from headers because some unrelated file might be depending on it.
See also #26972 for discussion.
Solve this by including the file directly everywhere it's required, regardless of whether or not it's already included by another header.
There should be no functional change here, but it will allow us to safely remove includes from headers in the future.
~I'm afraid it's a bit tedious to reproduce these commits, but it's reasonably straightforward:~
Edit: See note below
```bash
# All commands executed from the src/ subdir.
# Collect all tokens from bitcoin-config.h.in
# Isolate the tokens and remove blank lines
# Replace newlines with | and remove the last trailing one
# Collect all files which use these tokens
# Filter out subprojects (proper forwarding can be verified from Makefiles)
# Filter out .rc files
# Save to a text file
git grep -E -l `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-with-config-include.txt
# Find all files from the above list which don't include bitcoin-config.h
git grep -L -E "config/bitcoin-config.h" -- `cat files-with-config-include.txt`
# Include them manually with the exception of some files in crypto:
# crypto/sha256_arm_shani.cpp crypto/sha256_avx2.cpp crypto/sha256_sse41.cpp crypto/sha256_x86_shani.cpp
# These are exceptions which don't use bitcoin-config.h, rather the Makefile.am adds these cppflags manually.
# Commit changes. This should match the first commit of this PR.
# Use the same search as above to find all files which DON'T use any config tokens
git grep -E -L `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-without-config-include.txt
# Manually remove the includes and commit changes. This should match the second commit of this PR.
```
Edit: I'll keep this old description for posterity, but the manual approach has been replaced with a scripted diff from TheCharlatan
ACKs for top commit:
maflcko:
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3f πͺ
TheCharlatan:
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc
hebasto:
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc, I have reviewed the code and it looks OK.
fanquake:
ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc
Tree-SHA512: f11ddc4ae6a887f96b954a6b77f310558ddb271088a3fda3edc833669c4251b7f392515224bbb8e5f67eb2c799b4ffed3b07d96454e82ec635c686d0df545872
|
|
By defining a virtual interface class for the scheduler client, users of
the kernel can now define their own event consuming infrastructure,
without having to spawn threads or rely on the scheduler design.
Removing CScheduler also allows removing the thread and
exception modules from the kernel library.
|
|
-BEGIN VERIFY SCRIPT-
s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }
s 'CMainSignals' 'ValidationSignals'
s 'MainSignalsImpl' 'ValidationSignalsImpl'
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
s() { git grep -l "$1" src | (grep -v "$3" || cat;) | xargs sed -i "s/$1/$2/g"; }
s 'SingleThreadedSchedulerClient' 'SerialTaskRunner' ''
s 'SinglethreadedSchedulerClient' 'SerialTaskRunner' ''
s 'm_schedulerClient' 'm_task_runner' ''
s 'AddToProcessQueue' 'insert' ''
s 'EmptyQueue' 'flush' ''
s 'CallbacksPending' 'size' 'validation'
sed -i '109s/CallbacksPending/size/' src/validationinterface.cpp
-END VERIFY SCRIPT-
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
|
|
|
|
|
|
This is done in preparation for the next two commits, where the
CMainSignals are de-globalized.
This avoids adding new constructor arguments to the ChainstateManager
and CTxMemPool classes over the next two commits.
This could also allow future tests that are only interested in the
internal behaviour of the classes to forgo instantiating the signals.
|
|
-BEGIN VERIFY SCRIPT-
regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)'
exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp"
git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done;
git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done;
for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done;
-END VERIFY SCRIPT-
The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with:
./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -)
The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile.
The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing.
The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing.
The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
|
|
`NetWhitelist{bind}Permissions::TryParse`
864e2e9097de8f1fda63137f803687dd5cc96c03 fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse` (brunoerg)
Pull request description:
The string `s` represents the value from `-whitelist`/`-whitebind` (e.g. "bloom,forcerelay,noban@1.2.3.4:32") and it is used in `NetWhitelistPermissions::TryParse` and `NetWhitebindPermissions::TryParse`. However, a max length of 32 is not enough to cover a lot of cases. Even disconsidering the permissions, 32 would not be enough to cover a lot of addresses. This PR fixes it.
ACKs for top commit:
maflcko:
lgtm ACK 864e2e9097de8f1fda63137f803687dd5cc96c03
epiccurious:
utACK 864e2e9097de8f1fda63137f803687dd5cc96c03.
vasild:
ACK 864e2e9097de8f1fda63137f803687dd5cc96c03
Tree-SHA512: 2b89031b9f2ea92d636f05fd167b1e5ac726742a7e7c1af8ddaeaf90236e659731aaa6b7c23f65ec16ce52ac1b9e68e7b16e23c59e355312d057e001976d172a
|
|
|
|
|
|
29029df5c700e6940c712028303761d91ae15847 [doc] v3 signaling in mempool-replacements.md (glozow)
e643ea795e4b6fea4a6bbb3d72870ee6a4c836b1 [fuzz] v3 transactions and sigop-adjusted vsize (glozow)
1fd16b5c62f54c7f4c60122acd65d852f63d1e8b [functional test] v3 transaction submission (glozow)
27c8786ba918a42c860e6a50eaee9fdf56d7c646 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke)
9a1fea55b29fe025355b06b45e3d77d192acc635 [policy/validation] allow v3 transactions with certain restrictions (glozow)
eb8d5a2e7d939dd3ee683486e98702079e0dfcc0 [policy] add v3 policy rules (glozow)
9a29d470fbb62bbb27d517efeafe46ff03c25f54 [rpc] return full string for package_msg and package-error (glozow)
158623b8e0726dff7eae4288138f1710e727db9c [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow)
Pull request description:
See #27463 for overall package relay tracking.
Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340
Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418
Rationale:
- There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see #28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2]
- Switching to a cluster-based mempool (see #27677 and #28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution.
V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2.
Immediate benefits:
- You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later.
- Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction.
This also enables some other cool things (again see #27463 for overall roadmap):
- Ephemeral Anchors
- Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees.
- We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use.
- We can switch to a cluster-based mempool [5] (#27677 #28676), which removes CPFP carve out [6].
[1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html
[2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward.
[3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html
[4]: Original PR #25038 also contains a lot of the discussion
[5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7
[6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12
ACKs for top commit:
sdaftuar:
ACK 29029df5c700e6940c712028303761d91ae15847
achow101:
ACK 29029df5c700e6940c712028303761d91ae15847
instagibbs:
ACK 29029df5c700e6940c712028303761d91ae15847 modulo that
Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
|
|
Ensure we are checking sigop-adjusted virtual size by creating setups
and packages where sigop cost is larger than bip141 vsize.
Co-authored-by: Gregory Sanders <gsanders87@gmail.com>
|
|
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
|
|
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
|
|
`NetWhitelist{bind}Permissions::TryParse`
|
|
make check runs a bunch of other subtree tests that exercise code that
is hardly ever changed and have a comparatively long runtime. There
seems to be no target for running just the unit tests, so add one.
|
|
|
|
CJDNS addresses
b851c5385d0a0acec4493be1561cea285065d5dc fuzz: extend ConsumeNetAddr() to return I2P and CJDNS addresses (Vasil Dimov)
Pull request description:
In the process of doing so, refactor `ConsumeNetAddr()` to generate the addresses from IPv4, IPv6, Tor, I2P and CJDNS networks in the same way - by preparing some random stream and deserializing from it. Similar code was already found in `RandAddr()`.
ACKs for top commit:
achow101:
ACK b851c5385d0a0acec4493be1561cea285065d5dc
mzumsande:
ACK b851c5385d0a0acec4493be1561cea285065d5dc
brunoerg:
utACK b851c5385d0a0acec4493be1561cea285065d5dc
Tree-SHA512: 9905acff0e996f30ddac0c14e5ee9e1db926c7751472c06d6441111304242b563f7c942b162b209d80e8fb65a97249792eef9ae0a96100419565bf7f59f59676
|
|
ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Remove GetAdjustedTime (dergoegge)
Pull request description:
This picks up parts of #25908.
The use of adjusted time is removed from validation code while the warning to users if their clock is out of sync with the rest of the network remains.
ACKs for top commit:
naumenkogs:
ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2
achow101:
ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2
maflcko:
lgtm ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 π€½
stickies-v:
ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2
Tree-SHA512: d1f6b9445c236915503fd2ea828f0d3b92285a5dbc677b168453276115e349972edbad37194d8becd9136d8e7219b576af64ec51c72bdb1923e57e405c0483fc
|
