💬 maflcko commented on pull request "doc: Fix README functional test invocation":
(https://github.com/bitcoin/bitcoin/pull/31055#issuecomment-2399195553)
See https://github.com/bitcoin/bitcoin/pull/30859 (doc: cmake: prepend "build" to functional/test_runner.py by LarryRuane)

✅ TheCharlatan closed a pull request: "doc: Fix README functional test invocation"
(https://github.com/bitcoin/bitcoin/pull/31055)

💬 TheCharlatan commented on pull request "doc: Fix README functional test invocation":
(https://github.com/bitcoin/bitcoin/pull/31055#issuecomment-2399203281)
Ah, missed that pull request.

📝 maflcko opened a pull request: "ci: Double ctest timeout"
(https://github.com/bitcoin/bitcoin/pull/31056)
It looks like msan sometimes hits the timeout. So double it, which should still be useful to catch real timeouts in the wine windows-cross unit tests.

Example: https://github.com/bitcoin/bitcoin/runs/31200880897

💬 maflcko commented on issue "win64-cross CI timeout after 2h ":
(https://github.com/bitcoin/bitcoin/issues/30969#issuecomment-2399281998)
The addrman_tests normally take 14 seconds (https://cirrus-ci.com/task/5931376359243776?logs=ci#L2350), so I guess this issue is a duplicate of https://github.com/bitcoin/bitcoin/issues/23357, just that ctest turned the fail into a timeout.

💬 maflcko commented on pull request "ci: Add missing -DWERROR=ON to test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/31045#issuecomment-2399312390)
> you can do `alias nproc="sysctl -n hw.physicalcpu"` instead of installing all of `coreutils`.

No. It needs to be a real executable in `PATH`, otherwise, it won't be picked up in a new shell:

```
$ alias npr=nproc ; type npr ; bash -c 'type npr'
npr is aliased to `nproc'
bash: line 1: type: npr: not found
```

If you disagree, I am happy to push any other commit, which passes CI and achieves the same.

👍 maflcko approved a pull request: "fuzz: Add fuzz-only build mode option for targets"
(https://github.com/bitcoin/bitcoin/pull/31028#pullrequestreview-2353962202)
lgtm

💬 maflcko commented on pull request "fuzz: Add fuzz-only build mode option for targets":
(https://github.com/bitcoin/bitcoin/pull/31028#discussion_r1791549320)
```suggestion
constexpr bool build_for_fuzzing{
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
{true};
#else
{false};
#endif
```

nit: Could rename, so that it can be re-used in the future for other stuff, if needed?

💬 maflcko commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1791551628)
Will change if I have to re-touch

💬 maflcko commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1791551736)
Will change if I have to re-touch

💬 maflcko commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2399436238)
> > It would be good to check this. If you have a completely separate machine, you could try to install the "anti"-virus software there and see if the issue happens?
>
> I don't have another windows machine, so unfortunately I'm not able to check this. I'm not sure if it would do much, because I've now gotten to IBD several times without issues, whereas before when antivirus was still active, the issues came up persistently around block height 350,000-450,000 or so.

Bitcoin Core 28.0 was j
...

💬 maflcko commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2399472819)
I still could not reproduce. @apulsifer Do you see any unusual metrics in the monitoring? Do the graphs look different when the corruption happens for you?

Also, now that 28.0 is released, you may want to test and try it.

💬 maflcko commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2399475386)
Did you have any progress testing the hardware?

Also, now that 28.0 is released, you may want to test and try it.

🚀 fanquake merged a pull request: "depends: Print ready-to-use `--toolchain` option for CMake invocation"
(https://github.com/bitcoin/bitcoin/pull/31008)

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1791688265)
Continuing at https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1790649950

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1791692868)
Right. To resolve this and the issue at https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1790666758 I introduced a maximum lifetime of a private broadcast connection, after which time, we disconnect the peer regardless of the state the connection is in.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1791695089)
I went for the second one being simpler and also easier to reason about - if we received something other than what we broadcasted, then keep broadcasting (but not infinitely).

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2399582453)
`f9b2eaf96c...a51c2cdda5`: address suggestions and further try to silence the bogus std::optional warning.

🤔 stickies-v reviewed a pull request: "init: Some small chainstate load improvements"
(https://github.com/bitcoin/bitcoin/pull/31046#pullrequestreview-2354159584)
Approach ACK 1bec418e77482a4c53c8f1e7c89a151fdeae0f8e

💬 stickies-v commented on pull request "init: Some small chainstate load improvements":
(https://github.com/bitcoin/bitcoin/pull/31046#discussion_r1791672987)
Note: instantiating a `CDBWrapper` can throw other errors such as a `fs::filesystem_error` from [here](https://github.com/bitcoin/bitcoin/blob/62e4516722115c2d5aeb6c197abc73ca7c078b23/src/dbwrapper.cpp#L240), but I suppose that is not something the user can recover from with a reindex so not catching that here seems correct.

I think it logging the `dbwrapper_error` would be useful though?

<details>
<summary>git diff on 1bec418e77</summary>

```diff
diff --git a/src/node/chainstate.cpp
...