Age | Commit message (Collapse) | Author |
|
|
|
18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd ci, msvc: Add "Run fuzz binaries" step (Hennadii Stepanov)
52933d7283736fe3ae15e7ac44c02ca3bd95fe6d fuzz: Pass `SystemRoot` environment variable to subprocess (Hennadii Stepanov)
23cb8207cdd6c674480840b76626039cdfe7cb13 ci, msvc: Add "Clone fuzz corpus" step (Hennadii Stepanov)
19dceddf4bcdb74e84cf27229039a239b873d41b build, msvc: Build `fuzz.exe` binary (Hennadii Stepanov)
4c078d7bd278fa8b4db6e1da7b9b747f49a8ac4c build, msvc: Enable preprocessor conformance mode (Hennadii Stepanov)
09f5a74198c328c80539c17d951a70558e6b361e fuzz: Re-implement `read_stdin` in portable way (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/29760.
Suggested in https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2025593572.
ACKs for top commit:
maflcko:
lgtm ACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd π
sipsorcery:
tACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd
sipa:
utACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd
Tree-SHA512: 672ed6926ee9091f68f13780e77b60fc1d48731f16e847d849374f8426ffe1dafd9bcab06a27af62e8052ba345bb57f20f40579d6be8540c12ef85c23a6eec8b
|
|
It's very hard to randomly construct a transaction that would be the
parent of an existing orphanage tx. For functions like
AddChildrenToWorkSet and GetChildren that take orphan parents, use a tx
that was previously constructed.
|
|
992c714451676cee33d3dff49f36329423270c1c common: Don't terminate on null character in UrlDecode (Fabian Jahr)
099fa571511f113e0056d4bc27b3153a42f9dc65 scripted-diff: Modernize name of urlDecode function and param (Fabian Jahr)
8f39aaae417c33490e0e41fb97620eb23ced3d05 refactor: Remove hooking code for urlDecode (Fabian Jahr)
650d43ec15f7a3ae38126f65ef8fa0b1fd3ee936 refactor: Replace libevent use in urlDecode with our own code (Fabian Jahr)
46bc6c2aaa613eef526b21a06bf21e8edde31a88 test: Add unit tests for urlDecode (Fabian Jahr)
Pull request description:
Fixes #29654 (as a side-effect)
Removing dependencies is a general goal of the project and the xz backdoor has been an additional wake up call recently. Libevent shows many of the same symptoms, few maintainers and slow releases. While libevent can not be removed completely over night we should start removing itβs usage where it's possible, ideally with the end goal to removing it completely.
This is a pretty easy win in that direction. The [`evhttp_uridecode` function from libevent](https://github.com/libevent/libevent/blob/e0a4574ba2cbcdb64bb2b593e72be7f7f4010746/http.c#L3542) we were using in `urlDecode` could be easily emulated in fewer LOC. This also ports the [applicable test vectors over from libevent](https://github.com/libevent/libevent/blob/master/test/regress_http.c#L3430).
ACKs for top commit:
achow101:
ACK 992c714451676cee33d3dff49f36329423270c1c
theStack:
Code-review ACK 992c714451676cee33d3dff49f36329423270c1c
maflcko:
ACK 992c714451676cee33d3dff49f36329423270c1c π
stickies-v:
ACK 992c714451676cee33d3dff49f36329423270c1c
Tree-SHA512: 78f76ae7ab3b6710eab2aaac20f55eb0da7803e057eaa6220e865f328666a5399ef1a479702aaf630b2f974ad3aa15e2b6adac9c11bc8c3d4be21e8af1667fea
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/urlDecode/UrlDecode/g' $(git grep -l 'urlDecode' ./src)
sed -i 's/urlEncoded/url_encoded/g' $(git grep -l 'urlEncoded' ./src)
-END VERIFY SCRIPT-
|
|
|
|
diagram checks
016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85 fuzz: explicitly cap the vsize of RBFs for diagram checks (Greg Sanders)
Pull request description:
In master we are hitting a case where vsize transactions much larger than max standard size are causing an overflow in not-yet-exposed RBF diagram checking code: https://github.com/bitcoin/bitcoin/pull/29757#issuecomment-2049220195
`ConsumeTxMemPoolEntry` is creating entries with tens of thousands of sigops cost, causing the resulting RBFs to be "overly large".
To fix this I cause the fuzz test to stop adding transactions to the mempool when we reach a potential overflow of `int32_t`.
ACKs for top commit:
glozow:
ACK 016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85
marcofleon:
ACK 016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85. I ran libFuzzer on `package_rbf` on the current master branch until the overflow was encountered. Then I built the PR branch and ran the fuzzer using the crash input.
Tree-SHA512: b3ffc98d2c4598eb3010edd58b9370aab1441aafbb1044c83b2b90c17dfe9135b8de9dba475dd0108863c1ffedede443cd978e95231a41cf1f0715629197fa51
|
|
|
|
|
|
|
|
being set as client_maxfeerate failure
4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 fuzz: Add coverage for client_maxfeerate (Greg Sanders)
91d7d8f22a1c528db14fa743c66cd861ea00e84b AcceptMultipleTransactions: Fix workspace client_maxfeerate (Greg Sanders)
f3aa5bd5eb6d1088f98a4dc7daaab0e17a7d5529 fill_mempool: assertions and docsctring update (Greg Sanders)
a3da63e8febe475f2250f6432bca237d31fa9107 Move fill_mempool to util function (Greg Sanders)
73b68bd8b4f9447e30091c7f8c3dc91a086bd93b fill_mempool: remove subtest-specific comment (Greg Sanders)
Pull request description:
Bug causes an `Assume()` failure due to the expectation that the individual result should be invalid when done over `submitpackage` via rpc.
Bug introduced by https://github.com/bitcoin/bitcoin/pull/28950 , and I discovered it rebasing https://github.com/bitcoin/bitcoin/pull/28984 since it's easier to hit in that test scenario.
Tests in place were only checking `AcceptSingleTransaction`-level checks due to package evaluation only triggering when minfee is too high for the parent transaction.
Added test along with fix, moving the fill_mempool utility into a common area for re-use.
ACKs for top commit:
glozow:
reACK 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46
theStack:
ACK 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46
ismaelsadeeq:
re-ACK https://github.com/bitcoin/bitcoin/commit/4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 via [diff](https://github.com/bitcoin/bitcoin/compare/4fe7d150eb3c85a6597d8fc910fe1490358197ad..4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46)
Tree-SHA512: 3729bdf7f25d04e232f173ccee04ddbb2afdaafa3d04292a01cecf58fb11b3b2bc133e8490277f1a67622b62d17929c242dc980f9bb647896beea4332ee35306
|
|
With the introduction and usage of TimeOffsets, there is no more need
for this file. Remove it.
|
|
This helper class is an alternative to CMedianFilter, but without a
lot of the special logic and exceptions that we needed while it was
still used for consensus.
|
|
`UCharCast` calls
56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889 refactor, bench, fuzz: Drop unneeded `UCharCast` calls (Hennadii Stepanov)
Pull request description:
The `CKey::Set()` template function handles `std::byte` just fine: https://github.com/bitcoin/bitcoin/blob/b5d21182e5a66110ce2796c2c99da39c8ebf0d72/src/key.h#L105
Noticed in https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1546288181.
ACKs for top commit:
maflcko:
lgtm ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
laanwj:
Seems fine, code review ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
hernanmarino:
ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
Tree-SHA512: 0f6b6e66692e70e083c7768aa4859c7db11aa034f555d19df0e5d33b18c0367ba1c886bcb6be3fdea78248a3cf8285576120812da55b995ef5e6c94a9dbd9f7c
|
|
|
|
encodings
a8203e94123b6ea6e4f4a6320e3ad20457f44a28 refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair<Wtxid, CTransactionRef> (AngusP)
c3c18433ae1d5b024d4cb92c762f5ca0ec7849c8 refactor: Use typesafe Wtxid in compact block encoding message, instead of ambiguous uint256. (AngusP)
Pull request description:
The first commit replaces `uint256` with typesafe `Wtxid` (or `Txid`) types introduced in #28107.
The second commit then simplifies the extra tx `vector` to just be of `CTransactionRef`s instead of a `std::pair<Wtxid, CTransactionRef>`, as it's easy to get a `Wtxid` from a transaction ref.
ACKs for top commit:
glozow:
ACK a8203e94123b6ea6e4f4a6320e3ad20457f44a28
dergoegge:
ACK a8203e94123b6ea6e4f4a6320e3ad20457f44a28
Tree-SHA512: b4ba1423a8059f9fc118782bd8a668213d229c822f22b01267de74d6ea97fe4f2aad46b5da7c0178ecc9905293e9a0eafba1d75330297c055e27fd53c8c8ebfd
|
|
vec of pair<Wtxid, CTransactionRef>
All `CTransactionRef` have `.GetWitnessHash()` that returns a cached `const Wtxid` (since fac1223a568fa1ad6dd602350598eed278d115e8),
so we don't need to pass transaction refs around with their IDs as they're easy to get from a ref.
|
|
The `CKey::Set()` template function handles `std::byte` just fine.
|
|
80f8b92f4f2311b9e9a25361c9dd973244e6f95c remove libbitcoinconsensus (fanquake)
Pull request description:
This was deprecated in `v27.0`, for removal in `v28.0`. See discussion in PR #29189.
ACKs for top commit:
theuni:
Concept ACK and light review ACK 80f8b92f4f2311b9e9a25361c9dd973244e6f95c. My only hesitation here is that (afaics?) there's now nothing keeping undesired features like threading or globals from working their way into the interpreter in future commits.
m3dwards:
Concept ACK https://github.com/bitcoin/bitcoin/pull/29648/commits/80f8b92f4f2311b9e9a25361c9dd973244e6f95c
TheCharlatan:
ACK 80f8b92f4f2311b9e9a25361c9dd973244e6f95c
hebasto:
ACK 80f8b92f4f2311b9e9a25361c9dd973244e6f95c, I have reviewed the code and it looks OK.
Tree-SHA512: 17a62118aeb088f2695c892bb32794dfea3061e3cb7d9e8e9f1c06c3ff6f63a7587fa532e37edbb91fbc5a19b12c9a0f8e05fa9e8864aa07f92665375d847e80
|
|
ambiguous uint256.
Wtxid/Txid types introduced in #28107
|
|
|
|
|
|
|
|
7b29119d79efbc8c4148f350cc86531fde8b7251 use const ref for client_maxfeerate (Greg Sanders)
f10fd07320da302e8d038213c85e2b16e77d5dc2 scripted-diff: Rename max_sane_feerate to client_maxfeerate (Greg Sanders)
Pull request description:
Follow-ups to https://github.com/bitcoin/bitcoin/pull/28950
ACKs for top commit:
glozow:
utACK 7b29119d79efbc8c4148f350cc86531fde8b7251
stickies-v:
ACK 7b29119d79efbc8c4148f350cc86531fde8b7251
Tree-SHA512: b9e13509c6e9d7c08aa9d4e759f9707004c1c7b8f3e521fe2ec0037160b87c7fb02528966b9f26eaca6291621df9300e84b5aec66dbc2e97d13bf2f3cd7f979c
|
|
chunks against clusters of size 2
72959867784098137a50c34f86deca8235eef4f8 Unit tests for CalculateFeerateDiagramsForRBF (Greg Sanders)
b767e6bd47cb0fb8f7aea3fb10c597e59a35bf74 test: unit test for ImprovesFeerateDiagram (Greg Sanders)
7e89b659e1ddd0c04fa2bddba9706b5d1a1daec3 Add fuzz test for FeeFrac (Greg Sanders)
4d6528a3d6bf3821c216c68f99170e2faab5d63c fuzz: fuzz diagram creation and comparison (Greg Sanders)
e9c5aeb11d641b8cae373452339760809625021d test: Add tests for CompareFeerateDiagram and CheckConflictTopology (Greg Sanders)
588a98dccc5dbb6e331f28d83a4a10a13d70eb31 fuzz: Add fuzz target for ImprovesFeerateDiagram (Greg Sanders)
2079b80854e2595f6f696e7c13a56c7f2a7da9f4 Implement ImprovesFeerateDiagram (Greg Sanders)
66d966dcfaad3638f84654e710f403cb0a0a2ac7 Add FeeFrac unit tests (Greg Sanders)
ce8e22542ed0b4fa5794d3203207146418d59473 Add FeeFrac utils (Greg Sanders)
Pull request description:
This is a smaller piece of https://github.com/bitcoin/bitcoin/pull/28984 broken off for easier review.
Up to date explanation of diagram checks are here: https://delvingbitcoin.org/t/mempool-incentive-compatibility/553
This infrastructure has two near term applications prior to cluster mempool:
1) Limited Package RBF(https://github.com/bitcoin/bitcoin/pull/28984): We want to allow package RBF only when we know it improves the mempool. This narrowly scoped functionality allows use with v3-like topologies, and will be expanded at some point post-cluster mempool when diagram checks can be done efficiently against bounded cluster sizes.
2) Replacement for single tx RBF(in a cluster size of up to two) against conflicts of up to cluster size two. `ImprovesFeerateDiagram` interface will have to change for this use-case, which is a future direction to solve certain pins and improve mempool incentive compatibility: https://delvingbitcoin.org/t/ephemeral-anchors-and-mev/383#diagram-checks-fix-this-3
And longer-term, this would be the proposed way we would compute incentive compatibility for all conflicts, post-cluster mempool.
ACKs for top commit:
sipa:
utACK 72959867784098137a50c34f86deca8235eef4f8
glozow:
code review ACK 72959867784098137a50c34f86deca8235eef4f8
murchandamus:
utACK 72959867784098137a50c34f86deca8235eef4f8
ismaelsadeeq:
Re-ACK https://github.com/bitcoin/bitcoin/commit/72959867784098137a50c34f86deca8235eef4f8
willcl-ark:
crACK 72959867784098137a50c34f86deca8235eef4f8
sdaftuar:
ACK 72959867784098137a50c34f86deca8235eef4f8
Tree-SHA512: 79593e5a087801c06f06cc8b73aa3e7b96ab938d3b90f5d229c4e4bfca887a77b447605c49aa5eb7ddcead85706c534ac5eb6146ae2396af678f4beaaa5bea8e
|
|
-BEGIN VERIFY SCRIPT-
git grep -l 'max_sane_feerate' | xargs sed -i 's/max_sane_feerate/client_maxfeerate/g'
-END VERIFY SCRIPT-
|
|
transport test
626f8e398e219b84907ccaad036f69177d39284c fuzz: actually test garbage >64b in p2p transport test (Pieter Wuille)
Pull request description:
This fixes an oversight from #28196: in the `p2p_transport_bidirectional_v2` fuzz test, when the desired garbage length is over 64 bytes, the code would actually use garbage length 0. Fix this.
ACKs for top commit:
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/29667/commits/626f8e398e219b84907ccaad036f69177d39284c
brunoerg:
crACK 626f8e398e219b84907ccaad036f69177d39284c
Tree-SHA512: f6346367adb10464b6c9d20aef43625531d2a4d8110887ad03214b8c1907b83560f2dd5b5415e2180a40b4cd276d51881b32b60c740471b5c6bb218aa19848d8
|
|
submitpackage
38f70ba6ac86fb96c60571d2e1f316315c1c73cc RPC: Add maxfeerate and maxburnamount args to submitpackage (Greg Sanders)
Pull request description:
Resolves https://github.com/bitcoin/bitcoin/issues/28949
I couldn't manage to do it very cleanly outside of (sub)package evaluation itself, since it would change the current interface very heavily. Instead I threaded through the max fee argument and used that directly via ATMPArgs. From that perspective, this is somewhat a reversion from https://github.com/bitcoin/bitcoin/pull/19339. In a post-cluster mempool world, these checks could be consolidated to right after the given (ancestor) package is linearized/chunked, by just checking the feerate of the top chunk and rejecting the submission entirely if the top chunk is too high.
The implication here is that subpackages can be submitted to the mempool prior to hitting this new fee-based error condition.
ACKs for top commit:
ismaelsadeeq:
Re-ACK https://github.com/bitcoin/bitcoin/commit/38f70ba6ac86fb96c60571d2e1f316315c1c73cc ππΎ
glozow:
ACK 38f70ba6ac with some non-blocking nits
murchandamus:
LGTM, code review ACK 38f70ba6ac86fb96c60571d2e1f316315c1c73cc
Tree-SHA512: 38212aa9de25730944cee58b0806a3d37097e42719af8dd7de91ce86bb5d9770b6f7c37354bf418bd8ba571c52947da1dcdbb968bf429dd1dbdf8715315af18f
|
|
This was deprecated in v27.0, for removal in v28.0.
See discussion in PR #29189.
|
|
|
|
Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
|
|
Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
|
|
|
|
And thread the feerate value through ProcessNewPackage to
reject individual transactions that exceed the given
feerate. This allows subpackage processing, and is
compatible with future package RBF work.
|
|
-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
|
|
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
|
|
|
|
Turn the `std::vector` to `std::array` because it is cheaper and
allows us to have the number of the messages as a compile time
constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in
future code to build other `std::array`s with that size.
|
|
connections
|
|
bit_width is a drop-in replacement with an exact meaning in c++, so there is
no need to continue testing/fuzzing/benchmarking.
|
|
|
|
`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
|