✅ 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_
...

💬 ismaelsadeeq commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#discussion_r1749233222)
Ah, I see.
For users, it will be surprising to see that passing a convertible value like -1 is treated the same as 1.

To prevent confusion, this should be documented elsewhere if it isn't already, or the error message should be modified to indicate this behavior "that boolean convertible value that will result in true is also accepted".