💬 hebasto commented on issue "guix: Build fails for `x86_64-apple-darwin`":
(https://github.com/bitcoin/bitcoin/issues/30810#issuecomment-2336415272)
The problem was `env SDK_PATH=/home/hebasto`, which effectively shared my home directory with a Guix container.

✅ hebasto closed an issue: "guix: Build fails for `x86_64-apple-darwin`"
(https://github.com/bitcoin/bitcoin/issues/30810)

💬 hodlinator commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1748936794)
The error looks very much like what I ran into in my attempt to make `CScript` take a `span`, see my change to `prevector` in 05c16ae8649ee3b6bbcdbdf0541a65ccf4477537 (referenced in https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1715642251), I believe that might surprisingly enough fix the issue.

💬 l0rinc commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1748938887)
Wow, what a weird fix!
Thanks a lot, added it in a separate commit, let's see if this fixes it indeed: [`18870ac` (#30765)](https://github.com/bitcoin/bitcoin/pull/30765/commits/18870ac2eb419fcadd3f055c373f464a7319482e)

👍 hodlinator approved a pull request: "Remove unsafe uint256S() and test-only uint160S()"
(https://github.com/bitcoin/bitcoin/pull/30773#pullrequestreview-2288367006)
re-ACK 43cd83b0c7ba436b8ffc83d8a65e935714dffe70

`git range-diff 43cd83b~5 b60013 43cd83b`

- Added back slightly useless "Construct from hex string" section to make the PR more straight-forward to review (and commit history arguably cleaner as a consequence).
- Added half of my suggestion for bringing back testing against integer literals in `conversion` test. :+1:

(Passed `ctest` locally).

Agree with l0rinc's nits though (https://github.com/bitcoin/bitcoin/pull/30773#discussion_r174
...

💬 davidgumberg commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2336493024)
> @sipa No, it's exactly the same with `-blocksxor=0`.

This has uncovered a bug in #28052, `InitBlocksdirXorKey` does not return an empty `std::vector<std::byte>` when the `-blocksxor=0` argument is used, it instead [returns](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1185) a [vector with 8 bytes equal to zero](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1152):

`
...

🤔 tdb3 reviewed a pull request: "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD"
(https://github.com/bitcoin/bitcoin/pull/30807#pullrequestreview-2288434916)
Concept ACK

Brief code review.
Looks good. Planning to review in more detail.

💬 BenWestgate commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2336527820)
> ### `dbcache=43008`
> Time to reach block 860036: 6 hours 42 minutes Time to exit bitcoind (writing out): 110 minutes (cache was 26691.3MiB at the time) Significant write out seen only when exiting bitcoins.

The long writing out would go significantly faster on a higher performance SSD, it is also CPU-intensive as it's compressing the 27GB in memory. So the faster initial sync is a win.

💬 ajtowns commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2336545633)
> Motivation is still the same to limit the DoS exposure in face of some DoSy behaviors from tx-relay peers.

If this is just something a peer can opt-in to, I don't think it mitigates any DoS behaviours: spammers/attackers will just use the old version.

I'm pretty skeptical that anything here can really prevent *attacks* -- an attacker can run multiple or low-latency connections to end up with pretty much the same result in most cases, I think. As far as I can see the main benefit is to ma
...

👋 hebasto's pull request is ready for review: "ci: Post CMake-migration fixes and amendments"
(https://github.com/bitcoin/bitcoin/pull/30841)

📝 hebasto opened a pull request: "test: Drop no longer needed workarounds"
(https://github.com/bitcoin/bitcoin/pull/30847)
This PR deletes the workarounds introduced in https://github.com/bitcoin/bitcoin/pull/16564 and https://github.com/bitcoin/bitcoin/pull/15382, as `ctest` skips these cases gracefully: https://github.com/bitcoin/bitcoin/blob/5c80192ff6b982ee3a75be4142fe942b8206f2cd/src/test/CMakeLists.txt#L201-L203

💬 hebasto commented on pull request "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`":
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2336612503)
> > I am not measuring a difference in performance in the generation of the headers between master and this PR.
>
> Building on Windows benefits most.

On my Windows machine, the time to process `data/*.h` headers for the `test_bitcoin` target has been reduced from ~155s to ~105s.

@sipsorcery What do you think?

💬 laanwj commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2336616645)
ACK 6c9000cfbfab1cd3b48efebd8e0e90ae597cf561

Building on x86_64 gets different guix output for MacOS. But this is unrelated to this PR.
```
-129b365bba906f50926218d5b8bb76921f5f637549fe44941e01eb8ba5d5d5f0 guix-build-6c9000cfbfab/output/arm64-apple-darwin/bitcoin-6c9000cfbfab-arm64-apple-darwin-unsigned.zip
+f30b60f67b6415c6598a64aeb64f5653349e9dfdeacb901c4173a73fb725904f guix-build-6c9000cfbfab/output/arm64-apple-darwin/bitcoin-6c9000cfbfab-arm64-apple-darwin-unsigned.zip
-f836231d54d1
...

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1749166051)
> `BUILD_FOR_FUZZING` has nothing to do with fuzz engines. According to the documentation it is an option to "Build for fuzzing. Enabling this will disable all other targets and override BUILD_FUZZ_BINARY."

You're right, there's no need for a special case for MSVC.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1749166887)
Fixed in https://github.com/bitcoin/bitcoin/pull/30803.

💬 fjahr commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2336627101)
> This has uncovered a bug in https://github.com/bitcoin/bitcoin/pull/28052, InitBlocksdirXorKey does not return an empty std::vector<std::byte> when the -blocksxor=0 argument is used, it instead [returns](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1185) a [vector with 8 bytes equal to zero](https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src/node/blockstorage.cpp#L1152):

I may be missing somethi
...

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1749188455)
> Seems like a better reason to not do this is that it doesn't actually work properly with CTest? Did this work when you tested it previously? i.e:
>
> ```shell
> ctest --test-dir build -R mempool_tests --rerun-failed --output-on-failure
> Internal ctest changing into directory: /bitcoin/build
> Test project /bitcoin/build
> Start 53: mempool_tests
> 1/1 Test #53: mempool_tests ....................***Failed 0.01 sec
> /bitcoin/build/src/test/test_bitcoin.exe: 1: MZ????@???: not f
...

💬 fjahr commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1749192762)
Found this here because I saw #30833 and it got me to look into `blocksxor` for the first time. I see the same failures @glozow describes (also on macos) and I can confirm the findings here about the use of the temp folders because it does not happen when the test uses a clean chain, e.g. using a clean chain like this made the test robust for me: https://github.com/fjahr/bitcoin/commit/ef00b8eab48cf7ebcc8a0539f237175b8f5a0373 I didn't find anything new beyond that so far but I thought it should
...

💬 tdb3 commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2336682320)
Seems worth experimenting with:

https://github.com/bitcoin/bitcoin/blob/fa460884406b35b0dee75af23f42e8b4c4acbebc/src%2Fstreams.cpp#L76-L80

I wonder if the array creation, the copy, or the xoring here are to blame? Experimenting with the all 0s xor key can enable isolation.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1749231491)
On my Ubuntu 24.04 system all test pass.

<details>
<summary>Logs</summary>

```
$ make -C depends -j 16 HOST=x86_64-w64-mingw32 NO_QT=1
$ cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
$ cmake --build build -j 16
$ ctest --test-dir build -j 16
Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
Test project /home/hebasto/git/bitcoin/build
Start 1: util_test_runner
Start 2: util_rpcauth_test
Start 3: univalue_
...