💬 m3dwards commented on pull request "test: fix exclude parsing for functional runner":
(https://github.com/bitcoin/bitcoin/pull/30872#discussion_r1756773854)
I'll take it

💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1756777516)
> Would be good to add a comment explaining `if(CMAKE_GENERATOR MATCHES "Ninja|Makefiles")` saying that COMPILER_LAUNCHER variable only works for these backends not other backends. Otherwise the check seems arbitrary and doesn't seem to make sense.

Thanks! Done.

🤔 maflcko reviewed a pull request: "test: Drop no longer needed workarounds"
(https://github.com/bitcoin/bitcoin/pull/30847#pullrequestreview-2300178606)
lgtm

💬 maflcko commented on pull request "test: Drop no longer needed workarounds":
(https://github.com/bitcoin/bitcoin/pull/30847#discussion_r1756777352)
Just for reference. This never worked in the first place, because "Skipping" was never picked up, because logging was disabled until it was re-added in commit f03c9420958de31fdfecec5fa3e23134aac61803

💬 fanquake commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756779260)
When `-DBUILD_FOR_FUZZING=ON` is passed.

💬 maflcko commented on pull request "test: fix exclude parsing for functional runner":
(https://github.com/bitcoin/bitcoin/pull/30872#issuecomment-2346184415)
review ACK 72b46f28bf8d37a166961b5aa2a22302b932b756

👍 stickies-v approved a pull request: "util: Use consteval checked format string in FatalErrorf, LogConnectFailure"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2300018207)
ACK fa37d9b0fd0b8f1b23163887618b2fa1a817d596

I like the new test structure and added documentation, thanks! Left some non-blocking nits, only if you need to force-push.

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756687137)
nit: in this new new structure, `err_mix` makes more sense than `check_mix`

<details>
<summary>`s/check_/err_/g`</summary>

```diff
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 4c61632fce..f398404d75 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -62,25 +62,25 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
PassFmt<2>("%2$*3$d");
PassFmt<1>("%.*f");

- auto check_mix{"Format specifiers
...

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756671121)
nit: `%8$` is not a valid specifier
```suggestion
// Positional specifier, like %8$s
```

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756690672)
+1 for focusing on run-time errors, formatting errors aren't worth spending much time on imo.

💬 fanquake commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2346192841)
Guix Build
```bash
d4baa1d2e9ee7abeb60d115dd52c6bc7d02e8116fe4b94c0f040371321e37b80 guix-build-253a691441dd/output/aarch64-linux-gnu/SHA256SUMS.part
7c38410b9468799b74371ba195f34121a4789e917dcf611f2ccc4b81f8a63eee guix-build-253a691441dd/output/aarch64-linux-gnu/bitcoin-253a691441dd-aarch64-linux-gnu-debug.tar.gz
4a6ec8df80822e5938b1a55f2e08a945b12a9d4f8b2f1bad05ea8be90a3b6173 guix-build-253a691441dd/output/aarch64-linux-gnu/bitcoin-253a691441dd-aarch64-linux-gnu.tar.gz
48de6fea5e2d35be4
...

💬 hebasto commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756794139)
Could you please suggest your alternative?

💬 stickies-v commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#issuecomment-2346210098)
re-ACK 1eac96a503b6bac3eaf5d0eb3d23ffde3bfbf9aa

CI timeout issues seem unrelated.

📝 theStack opened a pull request: "code style: update .editorconfig file"
(https://github.com/bitcoin/bitcoin/pull/30877)
Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d57) w.r.t. following changes:
- consider Rust .rs files (relevant since #28076, commit bbbbdb0c)
- reflect build system change to CMake (#30454, #30664)
- add setting for bare Makefile still used for depends builds

Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim). The PR is made under the assumption that the file is still considered useful, especia
...

💬 hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-2346225294)
I put some time into implementing 3 variations on different personal branches:

### [unchanged_key_CDiskTxPos](https://github.com/bitcoin/bitcoin/compare/20b259332ff1d2e6ca6b291ffeda00b51497bbed...hodlinator:bitcoin:pr/24539_unchanged_key_CDiskTxPos) / baf57fb3a45c97d6cfb531b9252c97d4f27ad7ae

`uint256` value -> `CDiskTxPos`

Still have key of `COutPoint`: 32 + 4 = 36 bytes
old value - `uint256`: 32 bytes
new value - `CDiskTxPos`: 4 + 4 + 4 = 12 bytes

36+32 -> 36+12 => 29% theoretical
...

🤔 stickies-v reviewed a pull request: "http: Use 'starts_with' for matching URI prefix"
(https://github.com/bitcoin/bitcoin/pull/30868#pullrequestreview-2300252485)
Code LGTM eb43872b84bfc4d09fe38ce0cb74722f6f9842da. Commit message and PR title need updating, this is a `tidy` change now affecting multiple modules, not an `http` change.

💬 furszy commented on pull request "interfaces: #30697 follow ups":
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1756828713)
A bit delayed but here.

> So we both agree it is bad for overwrite function to reject null values and update function to accept them, but I would prefer both functions to accept them and you want both functions to reject them.
>
> Here are the reason I think it is better to accept them:
>
> * I think it is generally bad practice to accept nullable values and then handle them with runtime errors. If you want to write API's that don't accept null values I think you should use non-nullable
...

💬 Zk2u commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2346241399)
@starius could you possibly rebase this onto latest?

💬 fanquake commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756842485)
Something like:
```bash
# Always skip the ctime tests, if we are building no other tests.
# Otherwise, they are built if Valgrind is available. See SECP256K1_VALGRIND.
```
?

💬 petertodd commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2346256796)
> In general. if we have 0-value dust, I find it unlikely to hope that miners will clean it up for no reason?

Anyone can pay fees to clean up the dust. Doesn't have to be miners specifically.