✅ maflcko closed a pull request: "style: Fix spacing in script.h for OP_TRUE definition"
(https://github.com/bitcoin/bitcoin/pull/32077)

💬 luke-jr commented on issue "LevelDB error: Corruption: block checksum mismatch didn't trigger reindex.":
(https://github.com/bitcoin/bitcoin/issues/30138#issuecomment-2727002702)
>This should really be looked into, seeing this a lot more with our hardware now that blocks are full

If the chainstate is getting corrupt, that points to a hardware problem, and that is where it should be fixed.

> re-indexing entire blockchain for a corrupt chainstate a few hundred blocks from tip is horrible UX.

The issue here is that it *isn't* reindexing.

>Should only require reindex from last good block db.

Bitcoin Core does not store old block dbs. It has a single chainstate as of the
...

👍 hodlinator approved a pull request: "test: Update coverage.cpp to drop linux restriction"
(https://github.com/bitcoin/bitcoin/pull/32059#pullrequestreview-2686973172)
ACK ecdbe72916c1cabbf810e892d25fe6eed30b1306

Tested on Linux by running the [recommended](https://github.com/bitcoin/bitcoin/pull/32059#issuecomment-2722068136) Rust deterministic unit test coverage utility both without and with coverage support, with expected results.

💬 hodlinator commented on pull request "test: Update coverage.cpp to drop linux restriction":
(https://github.com/bitcoin/bitcoin/pull/32059#discussion_r1996374525)
nit: The PR title / description conforms fairly well to the contributing guidelines, but it would be nice if you also came a bit closer with the commit message.

https://github.com/bitcoin/bitcoin/blob/ecdbe72916c1cabbf810e892d25fe6eed30b1306/CONTRIBUTING.md?plain=1#L119-L124 \+ conventions on non-merge commits make me suggest something closer to:
```diff
- Extended ResetCoverageCounters in coverage.cpp with fallback implementation for non-linux linker and removed linux macro limitation.
+
...

💬 hodlinator commented on pull request "test: Update coverage.cpp to drop linux restriction":
(https://github.com/bitcoin/bitcoin/pull/32059#discussion_r1997328341)
Thanks for the thorough explanation, it was helpful!

nit:
This seems to build fine on Linux with/without coverage:
```C++
extern "C" void __llvm_profile_reset_counters(void);
extern "C" void __gcov_reset(void);
```
My guess is that what is important now is that the fallback is `weak`, not the prototype, could that be? Mac CI seems to accept that, but I'm not sure if our CI tests both non-coverage and coverage builds on Mac.

---
Iff the above is a problem on Mac/other platforms thoug
...

💬 hodlinator commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1997333029)
(The buggy reconnection attempts are handled in new PR #32073).

💬 hodlinator commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#issuecomment-2727012030)
Thanks for testing!

> I could not recreate a race condition but I have

Did you try the recommended approach in the commit message of 3301d2cbe8c3b76c97285d75fa59637cb6952d0b? It is a bit terse.

* Make sure you don't have the added `sync_txindex` fix on the Python side.
* Add the `sleep_for()` in the recommended place in the C++ side, (build).
* You should be able to get the individual tests to fail (simplest is to run the test_runner.py suite so the `--args` get sent in correctly).

...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2727013588)
Updated 21f6a3de77a9eedcca5d47f694d540d42b3ddbcc -> 5991a69ee0000de551955846d7d21733c326a748 ([kernelApi_28](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_28) -> [kernelApi_29](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_29), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_28..kernelApi_29))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1989793614), removed outdated comment about the validation interface i
...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1997340094)
Done.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1997340229)
I'm not quite sure what you meant with "Perhaps an alternative approach would be to keep the integer values between kernel_LogCategory the same as BCLog::LogFlags and just define a kernel-specific bitfield that defines which BCLog flags are valid". Does the current approach work for you?

💬 hodlinator commented on pull request "fuzz: More accurate coverage reports":
(https://github.com/bitcoin/bitcoin/pull/30156#discussion_r1997340468)
(Worked on in #32059).

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997476939)
The function `AddI2PSessionsIfNeeded` is called without checking if I2P is enabled, potentially leading to unnecessary session creation.

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997476940)
The `AddI2PSessionsIfNeeded` function is declared with `EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex)`, which is incorrect. This annotation suggests that the function requires the mutex to be unlocked, but it should require the mutex to be locked to ensure thread safety when modifying `m_unused_i2p_sessions`.

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997476976)
The function `AddI2PSessionsIfNeeded` is called without checking if I2P is enabled, which may lead to unnecessary session creation.

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997476977)
The `AddI2PSessionsIfNeeded` function is declared with `EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex)`, which is incorrect. The `!` operator is not valid in this context and should be removed to ensure proper lock requirements.

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997477829)
The function `AddI2PSessionsIfNeeded` is called without checking if I2P is enabled, potentially leading to unnecessary session creation.

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997477830)
The `AddI2PSessionsIfNeeded` function is declared with `EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex)`, which indicates that the function should not be called while holding the `m_unused_i2p_sessions_mutex`. However, the function is called within `CConnman::OpenNetworkConnection`, which is marked with the same lock requirement, potentially leading to a deadlock if the lock is held elsewhere in the call stack.

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997481576)
The function `AddI2PSessionsIfNeeded` is called without checking if `m_i2p_sam_session` is null, potentially leading to unnecessary session creation.

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997481577)
The `AddI2PSessionsIfNeeded` function is declared with `EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex)`, which is incorrect. The `!` operator is not valid in this context and should be removed to ensure proper locking behavior.

💬 aiswaryasankar commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#discussion_r1997486622)
The function `AddI2PSessionsIfNeeded` is called without checking if I2P is enabled, potentially leading to unnecessary session creation.