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

💬 murchandamus commented on pull request "Fix waste calculation in SelectionResult":
(https://github.com/bitcoin/bitcoin/pull/28366#issuecomment-2040240531)
> Are you still working on this?

Thanks, rebased

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1553992489)
looks like peer id needs to be used again: https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1542964936

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1553996459)
> think you forgot to push?

Github published everything instead of adding my comment to the group -_- yes, pushing in a second.

🤔 mzumsande reviewed a pull request: "net_processing: make any misbehavior trigger immediate discouragement"
(https://github.com/bitcoin/bitcoin/pull/29575#pullrequestreview-1983711854)
Code Review ACK e7ccaa01e4e0ef502d5727367baa54bcdcf1fb83

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1554000244)
> we should have tests(in master?) for this I hope.

apparently not?

💬 theuni commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2040260622)
Trying to make sense of all of this, I found [this RFC](https://discourse.llvm.org/t/rfc-hardening-in-libc/73925) helpful:

> - LLVM 18: first release that supports hardening modes and ways to enable them as described in the RFC.
> - The safe mode (available since the LLVM 15 release) is still supported; the release notes will mention that projects using the safe mode have to transition to use the hardened mode or the debug-lite mode instead (debug-lite is the rough equivalent of the old sa
...

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1554004259)
Hm no, I think we should return pairs of tx and the `fromPeer`, and just attribute the error to the right peer. If we restrict it to same sender, then we can also block packages by sending the children fast and refusing to send the parent.

⚠️ achow101 opened an issue: "`WalletCreate{Encrypted/Plain}` benchmark crash on Windows"
(https://github.com/bitcoin/bitcoin/issues/29816)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

After modifying the guix build script for 27.0rc1 to produce the benchmark binary, when `bench_bitcoin` is run on Windows, it crashes on the `WalletCreateEncrypted` and `WalletCreatePlain` benchmarks with

```
terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'
what(): filesystem error: cannot remove all: Broken pipe [C:\Users\ava\AppData\Loca
...

💬 theuni commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2040274229)
Oh, heh, I missed that @maflcko had already linked that. See also [here for an RFC about the deprecation](https://discourse.llvm.org/t/rfc-removing-the-legacy-debug-mode-from-libc/71026).

I think I'm inclined to agree that it's not worth supporting the old mode. It seems it's old and janky and deprecated for good reason.

💬 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
...