💬 fanquake commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2040280405)
Dropped the older define.

💬 hebasto commented on issue "`WalletCreate{Encrypted/Plain}` benchmark crash on Windows":
(https://github.com/bitcoin/bitcoin/issues/29816#issuecomment-2040280595)
Does your Windows user have a OS permission to create symbolic links?

🤔 tdb3 reviewed a pull request: "test: Handle functional test disk-full error"
(https://github.com/bitcoin/bitcoin/pull/29335#pullrequestreview-1983747381)
ACK for 2f7fcd5fe7289f7ad3efd566dbbd725264654f8f.
Left a nit inline.

Used a small ramdisk (1.2GB) with high jobs count (16) to force space exhaustion. As expected, received the initial warning `Running the test suite with fewer than 2626.0 MB of free space might cause tests to fail.` and also received the exiting error `Early exiting after test failure due to disk running out of space...`

💬 tdb3 commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1554019884)
nit: The directory is left full upon exit. The user would need to clean it up before re-running tests. Having the user manually clean up the directory seems reasonable to me (the user may want to see actions leading to the failure). An enhanced error message adds value.

```diff
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index dd3ea370bd..f08eb63b21 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -668,7 +668,9 @@ de
...

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1554024570)
Last push should fix this with the above approach. Still need to write a test case for the peers being different.

💬 achow101 commented on issue "`WalletCreate{Encrypted/Plain}` benchmark crash on Windows":
(https://github.com/bitcoin/bitcoin/issues/29816#issuecomment-2040289538)
> Does your Windows user have an OS permission to create symbolic links?

No, but even when run as admin which does, the error still occurs.

💬 theuni commented on pull request "build: replace custom `MAC_OSX` macro with standard `__APPLE__` for consistent macOS detection":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2040323463)
I recently did the same thing in one of my branches while working to remove the remaining autoconf vars from crypto/: https://github.com/theuni/bitcoin/commit/3646f76688e289774cce45e8f7a9967ee570ddda .

I think this is the right thing to do. I just checked locally and this applies to iOS as well:
`clang -target arm64-apple-ios test.cpp --sysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS16.4.sdk -o testing`

Where test.cpp is:
```c++
#in
...

💬 theuni commented on pull request "build: replace custom `MAC_OSX` macro with standard `__APPLE__` for consistent macOS detection":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2040325415)
Whoops, sorry. I spoke too fast. This is the right thing to do for `sha256.cpp`. I'd have to look into the rest in detail, but I'd ACK that small change.

💬 hebasto commented on pull request "crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc's":
(https://github.com/bitcoin/bitcoin/pull/29815#issuecomment-2040326708)
Concept ACK.

💬 itornaza commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2040330763)
approach ACK 49c0b8b2288e60ae22fcac5d03811cf36ecec058

It is indeed frustrating to watch some functional tests failing by just using 12-16 jobs in parallel. This PR addresses the issue with minimal footprint in the test code. However, the timeout of some tests are even greater on my machine without me understanding the reason yet. Until this issue is resolved, running the extended tests with the default number of jobs seems the best approach to me.

## On Master branch

Replicated the erra
...

💬 theuni commented on pull request "Update libsecp256k1 subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/29803#issuecomment-2040434183)
Subtree merge looks clean and not backdoored:
```
$ test/lint/git-subtree-check.sh -r src/secp256k1
src/secp256k1 in HEAD currently refers to tree c3c4db9c0e4636135963272b005b11a451303feb
src/secp256k1 in HEAD was last updated in commit 53eec53dca1cb677d11564b055d3b8581ddd6747 (tree c3c4db9c0e4636135963272b005b11a451303feb)
src/secp256k1 in HEAD was last updated to upstream commit d8311688bd383d3a923a1b11789cded3cc8e5e03 (tree c3c4db9c0e4636135963272b005b11a451303feb)
GOOD
```
secp chang
...

💬 TheCharlatan commented on pull request "depends: build libnatpmp with CMake":
(https://github.com/bitcoin/bitcoin/pull/29708#issuecomment-2040551060)
Guix build (x86_64):
```
a134241bc9ff9823ce078ea10dbac544ff63536ee3f66fb557414ca191b1d62d guix-build-3c1ae3ee33d4/output/aarch64-linux-gnu/SHA256SUMS.part
f5a46ab7a5faae9ce50b2d084cf65d14372b354b9f8369178f1348b70a3b675b guix-build-3c1ae3ee33d4/output/aarch64-linux-gnu/bitcoin-3c1ae3ee33d4-aarch64-linux-gnu-debug.tar.gz
e2353ed4f0738026fe16cd90b503ef2f02f31e0b68ba42ea80924997225ae374 guix-build-3c1ae3ee33d4/output/aarch64-linux-gnu/bitcoin-3c1ae3ee33d4-aarch64-linux-gnu.tar.gz
78060899627
...

💬 cbergqvist commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2040552035)
Thanks for trying it out!

> and again the same test that is not a subject of this PR:
>
> ```
> wallet_transactiontime_rescan.py --legacy-wallet | ✖ Failed | 159 s (not covered by this PR)
> ```
>
> Note: All test run on Apple M2 Max chip and macos 14.4.1 (23E224)

Searched closed issues and it seems like others running on Mac are also seeing `wallet_transactiontime_rescan.py --legacy-wallet` fail: #28221. Hopefully that specific failure & fix can be addressed in that issue.

💬 theuni commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2040590662)
utACK. Should darwin get this too though since it also uses libc++ ?

💬 dergoegge commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2040596926)
Changed the dev note. Ready for review.

📝 TheCharlatan opened a pull request: "kernel: De-globalize fReindex"
(https://github.com/bitcoin/bitcoin/pull/29817)
fReindex is one of the last remaining globals exposed by the kernel library, so move it into the blockstorage class to reduce the amount of global mutable state and make the kernel library a bit less awkward to use.

---

This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587).

💬 cbergqvist commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2040608592)
@itornaza if you have the inclination, it might be helpful if you could provide combined logs for the `wallet_importdescriptors.py --descriptors` test when it fails even when using this PR's commit. Then I/we could compare it with the combined logs produced on my machine (uploaded and linked in the first comment) and maybe figure out what lies behind the performance difference.

👍 TheCharlatan approved a pull request: "clang-tidy: Enable misc-no-recursion"
(https://github.com/bitcoin/bitcoin/pull/29690#pullrequestreview-1984165501)
ACK d9536e6e423ef7384a1869f3ac8a9cf7f503ed6f

👋 TheCharlatan's pull request is ready for review: "kernel: De-globalize fReindex"
(https://github.com/bitcoin/bitcoin/pull/29817)

💬 sipsorcery commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2040660307)
> I believe this is unrelated to this PR, see #29051. `make clean` and rebuilding should fix it.

I'm on Windows and using msvc but I did do a clean build but it didn't help.

I suspect it's down to msvc being less forgiving than gcc, probably due to a recent msvc change although I couldn't find anything in the release notes.

Apolgies for dumping a screenshot but it does show a succinct summary of the issue. This is from an attempt to compile (no linking involved) the single blockfilter_t
...