Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 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
...
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1791789613)
There's probably enough GUI-only stuff here, i.e `bison, ninja-build, python3, xz-utils`, that this could be moved to it's own `#### Gui` section.
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1791790452)
Could you better explain this patch? The description says that using the `QT_NEEDS_QMAIN` macro is required, but using it breaks things, so you're just getting rid of it? So how do things continue to work, if you're removing the required macro?
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1791797379)
Given that this is introducing a second copy of a patch we already have, it'd be good if there was someone we could avoid having to maintain two different copies of the same thing.
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1791799818)
I guess same thought here, given that this PR is introducing two copies of this same patch.
🤔 vasild reviewed a pull request: "net, net_processing: additional and consistent disconnect logging"
(https://github.com/bitcoin/bitcoin/pull/28521#pullrequestreview-2354306033)
Approach ACK 9967e8feb6

I noticed the repeating pattern `fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : ""` while working on https://github.com/bitcoin/bitcoin/pull/29415 and I repeated it a lot of times in the new messages I added :disappointed:. It has been on my TODO to introduce some function to deduplicate that. Thanks for the `CNode::LogIP()` method!
💬 vasild commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791797716)
_(suggestion at random place in the code)_

Consider to also adjust the `CConnman::DisconnectNode()` functions.
💬 vasild commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791761963)
```suggestion
LogDebug(BCLog::NET,
"socket receive timeout: %is, %s\n", count_seconds(now - last_recv),
node.DisconnectMsg(fLogIPs)
```
💬 vasild commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791784114)
This log shouldn't contain "disconnecting" because we do not disconnect.
💬 vasild commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791764812)
This `%d %d`, `last_recv.count() != 0, last_send.count() != 0` is super obscure. It was like that before, so it is fine wrt this PR.

But would be nice to fix it, since this is changing the same log message already. I am not sure how though. Maybe change that `%d` to something like "has never received from the peer" + "has never sent to the peer".
⚠️ dergoegge opened an issue: "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`"
(https://github.com/bitcoin/bitcoin/issues/31057)
Without `-DBUILD_FOR_FUZZING` the fuzz binary is less suited for testing puporses, because it won't crash on `Assume` and it won't work around fuzz blockers with `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`. Afaict, the only place where the fuzz binary is currently used without `-DBUILD_FOR_FUZZING` is in the macOS and windows CI jobs.

I would like to propose that we split the CI jobs into separate fuzz-only jobs that use `-DBUILD_FOR_FUZZING` and that we remove support for building the fuzz bi
...
💬 dergoegge commented on issue "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`":
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2399758280)
cc @maflcko @marcofleon