💬 rustaceanrob commented on pull request "Implementation of SwiftSync":
(https://github.com/bitcoin/bitcoin/pull/34004#issuecomment-3664604170)
> found essentially no difference in the bitset encoding and using the run-length

This turned out to not be true. Using bit-packing as shown in the current state of the PR, the encoding was far worse. `0/1` encoding up to 928000 resulted in a hintsfile of 438Mb, whereas the run-lengths for height came in at 173Mb. I think 438Mb is not acceptable, so a hybrid approach is likely required. The construction of both files was around the same duration. I will do a more thorough analysis of chain hi
...

💬 rustaceanrob commented on pull request "Implementation of SwiftSync":
(https://github.com/bitcoin/bitcoin/pull/34004#discussion_r2626401193)
See comment, I will investigate this in the coming weeks.

🤔 hodlinator reviewed a pull request: "qa: Avoid knock-on exception in assert_start_raises_init_error"
(https://github.com/bitcoin/bitcoin/pull/32929#pullrequestreview-3586968227)
Latest push (6b0eac750f3aecb29446d70c039559143e43a011 -> 356883f0e48be59bcb154096cef82cbf3f0dd9d8) was largely motivated by https://github.com/bitcoin/bitcoin/pull/32929#pullrequestreview-3584856747 and contains changes:
* Moves the main fix commit first, and merges in the tiny doc-commit that was updating a comment in the same function.
* Renames `TestInitErrorTimeout` -> `TestMissingInitErrorTimeout` and clarifies log (https://github.com/bitcoin/bitcoin/pull/32929#discussion_r2624666464).
*
...

💬 hodlinator commented on pull request "qa: Avoid knock-on exception in assert_start_raises_init_error":
(https://github.com/bitcoin/bitcoin/pull/32929#discussion_r2626416752)
Good points, worth distinguishing it more from the pre-existing `TestInitErrorStartupFailure`.

💬 frankomosh commented on something "":
(https://github.com/bitcoin/bitcoin/commit/0190ac6622ff017aeead385289918d388a85218a#r173026481)
I think that in the case of `ImmediateTaskRunner`, the validation callbacks run synchronously rather than through the scheduler, which alters the ordering compared to production. Just curious if it would make sense to document that this execution mode is not production equivalent, so that fuzz coverage is interpreted accordingly?(and also for anyone who will use this as a refence in future)

🤔 frankomosh reviewed a pull request: "fuzz: compact block harness"
(https://github.com/bitcoin/bitcoin/pull/33300#pullrequestreview-3586886869)
Concept ACK. fuzzing direction look reasonable.

💬 frankomosh commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2626463355)
Seems like `txns_randomized` reorders between runs, could this cause the same fuzz seed to pick different transactions? If so, would it make sense to snapshot the vector first for stable indexing?

💬 frankomosh commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2626340174)
Since this helper appears to be used only by fuzz test code, is there any downside to keeping it in a test only location(i.e under `src/test/util`) instead of `src/util/fs.h`? Maybe that could help keep util’s interface focused on production use, but happy to defer if there’s a reason it needs to live here specifically.

🤔 stickies-v reviewed a pull request: "log: Use `__func__` for -logsourcelocations"
(https://github.com/bitcoin/bitcoin/pull/34088#pullrequestreview-3587361950)
Concept ACK. I agree that for some functions (such as your example), the verbosity is a downside.

Have you considered just capping the `function_name()` to, say, 20 characters, when printing? (could even make it fixed length to make logs a bit more uniform for readability) I think that'd be about as helpful without having to introduce our own `SourceLocation`?

💬 maflcko commented on pull request "log: Use `__func__` for -logsourcelocations":
(https://github.com/bitcoin/bitcoin/pull/34088#issuecomment-3665069468)
> Have you considered just capping the `function_name()` to, say, 20 characters, when printing? (could even make it fixed length to make logs a bit more uniform for readability) I think that'd be about as helpful without having to introduce our own `SourceLocation`?

I don't think this works, because the return type is included in the function signature. I think printing something short but wrong is worse than printing the full thing.

Some examples:

```
std::variant<int, double, float,
...

⚠️ rkrux opened an issue: "GUI crashes in regtest"
(https://github.com/bitcoin-core/gui/issues/918)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

The GUI crashes in regtest upon loading. It did work once initially but has been crashing since, tried quite a few times.

### Expected behaviour

The GUI should not crash in regtest, just like it doesn't in main or test environments.

### Steps to reproduce

Following is the command I have used. Also, tried a few variants of the `-datadir, -walletdir` args by adding and removing `/regtest
...

🤔 stickies-v reviewed a pull request: "scripted-diff: [doc] Unify stale copyright headers"
(https://github.com/bitcoin/bitcoin/pull/34084#pullrequestreview-3587528249)
Approach ACK. I've moved to a dateless copyright notice for new code, personally. The scripted-diff keeping the initial publication dates seems like the approach with the least friction.

🤔 maflcko reviewed a pull request: "fuzz: Add fuzz target for block index tree and related validation events"
(https://github.com/bitcoin/bitcoin/pull/31533#pullrequestreview-3587318193)
lgtm, but I haven't reviewed the ABC part and I wonder why it is needed.

review ACK db2d39f642979f929261e5f1cd67f0c2f2ca045f 🍶

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU
...

💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2626777661)
nit: Could use `!bm.IsBlockPruned(*index)` for a stricter check?

💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2626714452)
nit: Could use `IsValid(BLOCK_VALID_TREE)` for a stricter check? Also, to avoid a (silent) conflict with https://github.com/bitcoin/bitcoin/pull/32950 ?

💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2626894721)
nit: Could use the stricter `!bm.IsBlockPruned(prune_block)` check?

💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2626748203)
nit: Could use `IsValid(BLOCK_VALID_TRANSACTIONS)` for a stricter check?

💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2626858638)
q: Maybe I am missing something, but what is the benefit of re-implementing this. `ConnectTip` is already `protected`, so it may be possible to mock it to inject invalidity?

💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2626711907)
nit: Could use `IsValid(BLOCK_VALID_TREE)` for a stricter check?

💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2626902739)
same nit here for the asserts in the third commit.