💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2397918937)
> Also playing around with this and reading the [netdriver post](https://blog.swiecki.net/2018/01/fuzzing-tcp-servers.html) (i couldn't find any other docs on it) I'm not sure if this will ever even get past the version handshake? So I'm wondering how useful this tool really is for us.
Yes, I don't think it will pass the version handshake. I can't see what kind of bugs it would find that other fuzzers would not get.
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2397918937)
> Also playing around with this and reading the [netdriver post](https://blog.swiecki.net/2018/01/fuzzing-tcp-servers.html) (i couldn't find any other docs on it) I'm not sure if this will ever even get past the version handshake? So I'm wondering how useful this tool really is for us.
Yes, I don't think it will pass the version handshake. I can't see what kind of bugs it would find that other fuzzers would not get.
💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2397922175)
Re https://github.com/bitcoin/bitcoin/pull/30965#pullrequestreview-2352892509
Thank you for the questions @mzumsande!
> So can you explain a bit more in detail how this helps kernel?
I will try to give a thought-dump of what I am attempting to achieve here. As said in the title description, the kernel-motivated goal of this PR is to allow users of the library to instantiate the block tree db in the block manager in the same way we would currently in the chainstate load routines without
...
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2397922175)
Re https://github.com/bitcoin/bitcoin/pull/30965#pullrequestreview-2352892509
Thank you for the questions @mzumsande!
> So can you explain a bit more in detail how this helps kernel?
I will try to give a thought-dump of what I am attempting to achieve here. As said in the title description, the kernel-motivated goal of this PR is to allow users of the library to instantiate the block tree db in the block manager in the same way we would currently in the chainstate load routines without
...
📝 theuni opened a pull request: "RFC: build: support for pre-compiled headers."
(https://github.com/bitcoin/bitcoin/pull/31053)
This cuts off roughly a third of my total build time, either with `-j8` or `-j24`.
There are many different ways to go about adding pre-compiled header support with CMake. This is probably the simplest and most naive, but it has a substantial impact.
I used [ClangBuildAnalyzer](https://github.com/aras-p/ClangBuildAnalyzer) to measure the headers which took the most amount of time to parse while building the kernel library. I did this under the assumption that those are probably the most in
...
(https://github.com/bitcoin/bitcoin/pull/31053)
This cuts off roughly a third of my total build time, either with `-j8` or `-j24`.
There are many different ways to go about adding pre-compiled header support with CMake. This is probably the simplest and most naive, but it has a substantial impact.
I used [ClangBuildAnalyzer](https://github.com/aras-p/ClangBuildAnalyzer) to measure the headers which took the most amount of time to parse while building the kernel library. I did this under the assumption that those are probably the most in
...
💬 theuni commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2397943775)
Ping @hebasto, @fanquake
Also @willcl-ark, you might be interested in this as you've been benchmarking build times.
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2397943775)
Ping @hebasto, @fanquake
Also @willcl-ark, you might be interested in this as you've been benchmarking build times.
💬 TheCharlatan commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2397951718)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2397951718)
Concept ACK
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2397978598)
> Remaining (non nitty) things I think to be addressed:
>
> 1. [refactor: TxDownloadManager + fuzzing #30110 (comment)](https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391768053)
>
> 2. [refactor: TxDownloadManager + fuzzing #30110 (comment)](https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1787010637)
Yes 👍 and thanks for looking through what happened with the nonstandard input txns @marcofleon @instagibbs! Sorry for the delay, I still need to sit down and
...
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2397978598)
> Remaining (non nitty) things I think to be addressed:
>
> 1. [refactor: TxDownloadManager + fuzzing #30110 (comment)](https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391768053)
>
> 2. [refactor: TxDownloadManager + fuzzing #30110 (comment)](https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1787010637)
Yes 👍 and thanks for looking through what happened with the nonstandard input txns @marcofleon @instagibbs! Sorry for the delay, I still need to sit down and
...
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1790920762)
Yeah, need to rework this, it's confusing
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1790920762)
Yeah, need to rework this, it's confusing
💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2399035773)
Here's my depends/x86_64-apple-darwin22.6.0/toolchain.cmake
```
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.
# This file is expected to be highly volatile and may still change substantially.
# If CMAKE_SYSTEM_NAME is set within a toolchain file, CMake will also
# set CMAKE_CROSSCOMPILING to TRUE, even if CMAKE_SYSTEM_NAME matches
# CMAKE_HOST_SYSTEM_
...
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2399035773)
Here's my depends/x86_64-apple-darwin22.6.0/toolchain.cmake
```
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.
# This file is expected to be highly volatile and may still change substantially.
# If CMAKE_SYSTEM_NAME is set within a toolchain file, CMake will also
# set CMAKE_CROSSCOMPILING to TRUE, even if CMAKE_SYSTEM_NAME matches
# CMAKE_HOST_SYSTEM_
...
⚠️ Sjors opened an issue: "macOS 15.0.1 can't build Qt (Intel, without Xcode)"
(https://github.com/bitcoin/bitcoin/issues/31054)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
`make` in `depends` fails
This is very recent, will try to find an earlier commit where it works.
### Expected behaviour
To work.
### Steps to reproduce
```
cd depends
make NO_WALLET=1 NO_UPNP=1 NO_ZMQ=1
```
### Relevant log output
```
...
Configuring qt...
Creating qmake...
In file included from /Users/sjors/dev/bitcoin2/depends/work/build/x86_64-apple-darwin24.0.0/qt/5.1
...
(https://github.com/bitcoin/bitcoin/issues/31054)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
`make` in `depends` fails
This is very recent, will try to find an earlier commit where it works.
### Expected behaviour
To work.
### Steps to reproduce
```
cd depends
make NO_WALLET=1 NO_UPNP=1 NO_ZMQ=1
```
### Relevant log output
```
...
Configuring qt...
Creating qmake...
In file included from /Users/sjors/dev/bitcoin2/depends/work/build/x86_64-apple-darwin24.0.0/qt/5.1
...
💬 Sjors commented on issue "macOS 15.0.1 can't build Qt (Intel, without Xcode)":
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2399057050)
It happens even on d71ac768424333b65a6d88c9752cc9c7fdb276f3, so it's not a recent change in the code. Will try some variations with Xcode, etc.
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2399057050)
It happens even on d71ac768424333b65a6d88c9752cc9c7fdb276f3, so it's not a recent change in the code. Will try some variations with Xcode, etc.
💬 Sjors commented on issue "macOS 15.0.1 can't build Qt (Intel, without Xcode)":
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2399165977)
When I install XCode 16 and build depends for master @ 62e4516722115c2d5aeb6c197abc73ca7c078b23 it's happy.
`brew install llvm` (currently at 19.1.1) does not help. I used:
```
export PATH="$(brew --prefix llvm)/bin:$PATH"
export LDFLAGS="-L$(brew --prefix llvm)/lib"
export CPPFLAGS="-I$(brew --prefix llvm)/include"
```
Doesn't work on d71ac768424333b65a6d88c9752cc9c7fdb276f3 either.
I went back as far as ef20add4c98674183720d9631ac780f9a248b487 and still getting this error.
...
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2399165977)
When I install XCode 16 and build depends for master @ 62e4516722115c2d5aeb6c197abc73ca7c078b23 it's happy.
`brew install llvm` (currently at 19.1.1) does not help. I used:
```
export PATH="$(brew --prefix llvm)/bin:$PATH"
export LDFLAGS="-L$(brew --prefix llvm)/lib"
export CPPFLAGS="-I$(brew --prefix llvm)/include"
```
Doesn't work on d71ac768424333b65a6d88c9752cc9c7fdb276f3 either.
I went back as far as ef20add4c98674183720d9631ac780f9a248b487 and still getting this error.
...
💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2399173681)
Ditto on d71ac768424333b65a6d88c9752cc9c7fdb276f3.
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2399173681)
Ditto on d71ac768424333b65a6d88c9752cc9c7fdb276f3.
📝 TheCharlatan opened a pull request: "doc: Fix README functional test invocation"
(https://github.com/bitcoin/bitcoin/pull/31055)
Seems like this was missed during the cmake migration.
(https://github.com/bitcoin/bitcoin/pull/31055)
Seems like this was missed during the cmake migration.
💬 maflcko commented on pull request "doc: Fix README functional test invocation":
(https://github.com/bitcoin/bitcoin/pull/31055#issuecomment-2399195553)
See https://github.com/bitcoin/bitcoin/pull/30859 (doc: cmake: prepend "build" to functional/test_runner.py by LarryRuane)
(https://github.com/bitcoin/bitcoin/pull/31055#issuecomment-2399195553)
See https://github.com/bitcoin/bitcoin/pull/30859 (doc: cmake: prepend "build" to functional/test_runner.py by LarryRuane)
✅ TheCharlatan closed a pull request: "doc: Fix README functional test invocation"
(https://github.com/bitcoin/bitcoin/pull/31055)
(https://github.com/bitcoin/bitcoin/pull/31055)
💬 TheCharlatan commented on pull request "doc: Fix README functional test invocation":
(https://github.com/bitcoin/bitcoin/pull/31055#issuecomment-2399203281)
Ah, missed that pull request.
(https://github.com/bitcoin/bitcoin/pull/31055#issuecomment-2399203281)
Ah, missed that pull request.
📝 maflcko opened a pull request: "ci: Double ctest timeout"
(https://github.com/bitcoin/bitcoin/pull/31056)
It looks like msan sometimes hits the timeout. So double it, which should still be useful to catch real timeouts in the wine windows-cross unit tests.
Example: https://github.com/bitcoin/bitcoin/runs/31200880897
(https://github.com/bitcoin/bitcoin/pull/31056)
It looks like msan sometimes hits the timeout. So double it, which should still be useful to catch real timeouts in the wine windows-cross unit tests.
Example: https://github.com/bitcoin/bitcoin/runs/31200880897
💬 maflcko commented on issue "win64-cross CI timeout after 2h ":
(https://github.com/bitcoin/bitcoin/issues/30969#issuecomment-2399281998)
The addrman_tests normally take 14 seconds (https://cirrus-ci.com/task/5931376359243776?logs=ci#L2350), so I guess this issue is a duplicate of https://github.com/bitcoin/bitcoin/issues/23357, just that ctest turned the fail into a timeout.
(https://github.com/bitcoin/bitcoin/issues/30969#issuecomment-2399281998)
The addrman_tests normally take 14 seconds (https://cirrus-ci.com/task/5931376359243776?logs=ci#L2350), so I guess this issue is a duplicate of https://github.com/bitcoin/bitcoin/issues/23357, just that ctest turned the fail into a timeout.
💬 maflcko commented on pull request "ci: Add missing -DWERROR=ON to test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/31045#issuecomment-2399312390)
> you can do `alias nproc="sysctl -n hw.physicalcpu"` instead of installing all of `coreutils`.
No. It needs to be a real executable in `PATH`, otherwise, it won't be picked up in a new shell:
```
$ alias npr=nproc ; type npr ; bash -c 'type npr'
npr is aliased to `nproc'
bash: line 1: type: npr: not found
```
If you disagree, I am happy to push any other commit, which passes CI and achieves the same.
(https://github.com/bitcoin/bitcoin/pull/31045#issuecomment-2399312390)
> you can do `alias nproc="sysctl -n hw.physicalcpu"` instead of installing all of `coreutils`.
No. It needs to be a real executable in `PATH`, otherwise, it won't be picked up in a new shell:
```
$ alias npr=nproc ; type npr ; bash -c 'type npr'
npr is aliased to `nproc'
bash: line 1: type: npr: not found
```
If you disagree, I am happy to push any other commit, which passes CI and achieves the same.
👍 maflcko approved a pull request: "fuzz: Add fuzz-only build mode option for targets"
(https://github.com/bitcoin/bitcoin/pull/31028#pullrequestreview-2353962202)
lgtm
(https://github.com/bitcoin/bitcoin/pull/31028#pullrequestreview-2353962202)
lgtm