💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1247342599)
Thank you, Luke, updated. Is that with GCC? I don't see the warning with Clang 16.0.6.

```diff
BOOST_FIXTURE_TEST_CASE(logging_IsNoneCategory, LogSetup)
{
- for (const std::string& s : {"none", "0"}) {
+ for (const char* const& s : {"none", "0"}) {
BOOST_CHECK(LogInstance().IsNoneCategory(s));
}
- for (const std::string& s : {"", "NONE", "net", "all", "1"}) {
+ for (const char* const& s : {"", "NONE", "net", "all", "1"}) {
BOOST_CHECK(!LogInstance
...

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1247357530)
Because `-debugexclude=none"` is passed. It looks like the passed extra args are concatenated to `self.args` before starting the process, i.e. `self.process = subprocess.Popen(self.args + extra_args...` in `test_framework/test_node.py#L219`, so the "none" value cancels the -debugexclude={libevent,leveldb,rand} args in the same file.

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1247357713)
Good question, though. I'll add a comment.

💬 MarcoFalke commented on pull request "refactor: Open file in FileCommit":
(https://github.com/bitcoin/bitcoin/pull/28006#issuecomment-1614159773)
Closing for now, until there is a higher need for it.

✅ MarcoFalke closed a pull request: "refactor: Open file in FileCommit"
(https://github.com/bitcoin/bitcoin/pull/28006)

💬 MarcoFalke commented on pull request "fix: delay in TimeOffset applied to AdjustedTime caused by send/receive message queues, correct pointer alignment issue":
(https://github.com/bitcoin/bitcoin/pull/28010#issuecomment-1614167569)
No need to open several pulls for the same change. Let's discuss the concept in #28007 first, which you can then reopen and (force) push to.

✅ MarcoFalke closed a pull request: "fix: delay in TimeOffset applied to AdjustedTime caused by send/receive message queues, correct pointer alignment issue"
(https://github.com/bitcoin/bitcoin/pull/28010)

🤔 MarcoFalke reviewed a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-1506562478)
(nit)

💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1247469065)
```suggestion
throw std::ios_base::failure("AutoFile::seek: file handle is nullptr");
```

📝 MarcoFalke opened a pull request: "test: Rename EncodeDecimal to serialization_fallback"
(https://github.com/bitcoin/bitcoin/pull/28011)
The new name better explains that the function handles fallbacks, without listing all in the function name.

👍 MarcoFalke approved a pull request: "util: Show descriptive error messages when FileCommit fails"
(https://github.com/bitcoin/bitcoin/pull/26654#pullrequestreview-1506634475)
nice. lgtm, but I didn't look at the windows stuff

💬 MarcoFalke commented on pull request "util: Show descriptive error messages when FileCommit fails":
(https://github.com/bitcoin/bitcoin/pull/26654#discussion_r1247515634)
```suggestion
LogPrintf("fsync failed: %s\n", SysErrorString(errno));
```

nits:

* Can remove the manual func logging, which isn't needed, because all strings in this function are already unique to the codebase. Also, users can enable the setting which enabled function logging.
* Can change `%d` to `%s`, since it is a string. (No behavior difference though)

💬 FelixWeis commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1614300636)
reverted to compiling 6ee3881551f2cd411c4e4d8b0ccedf0f0416d8c2 (tag: v25.0). same cpu, os, compiler version. runs smooth since 3 days

💬 MarcoFalke commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1614306854)
> runs smooth since 3 days

This implies it is not an upstream bug, or the issue fixed itself in the meantime. This may take some time, but you could try bisecting?

💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1247595124)
I can't think of a good reason why this shouldn't be `[[nodiscard]]` too. However, as I said [here](https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1246307365) I am trying to keep this PR's scope limited to just the flush functions and their immediate call sites. I added the `[[nodiscard]]` to `FindBlockPos`, because it reports on the flush error now too. `FindUndoPos` on the other hand is not immediately affected by the flush errors. There are many such examples where we ignore error
...

🚀 fanquake merged a pull request: "refactor: remove in-code warning suppression"
(https://github.com/bitcoin/bitcoin/pull/28002)

💬 hebasto commented on pull request "contrib: add macOS test for fixup_chains usage":
(https://github.com/bitcoin/bitcoin/pull/27999#issuecomment-1614340227)
Guix builds:
```
0e17d462808f86aa7157e27a957da88fd1adeb491ad6c01138aca93e5ad1d018 guix-build-7f96638723a0/output/arm64-apple-darwin/SHA256SUMS.part
ceb208e6374f5d7367b73128e90ca6eaeea15d50c69e49c8cf75b47212525ad7 guix-build-7f96638723a0/output/arm64-apple-darwin/bitcoin-7f96638723a0-arm64-apple-darwin-unsigned.dmg
e31663554cfde8a37a9f3438c9c895dde94b90ff87e28f12f78be71ef6421d93 guix-build-7f96638723a0/output/arm64-apple-darwin/bitcoin-7f96638723a0-arm64-apple-darwin-unsigned.tar.gz
68a7b
...

✅ fanquake closed an issue: "index: ThreadSanitizer: data race on vptr "
(https://github.com/bitcoin/bitcoin/issues/27355)

🚀 fanquake merged a pull request: "test: Use same timeout for all index sync"
(https://github.com/bitcoin/bitcoin/pull/27988)

👍 fanquake approved a pull request: "doc: i2p documentation updates"
(https://github.com/bitcoin/bitcoin/pull/27937#pullrequestreview-1506823009)
ACK 11900e5a8aa4647e50e8d97fc9a8b35e9e20772b