Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 theuni commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#issuecomment-2123125755)
This PR originally contained a commit which purposely added a violation in init.cpp:
```c++
thread_local std::string bad_var;
```
From the tidy c-i task:

> init.cpp:723:5: error: Variable with non-trivial destructor cannot be thread_local. [bitcoin-nontrivial-threadlocal,-warnings-as-errors]
> 723 | thread_local std::string bad_var;

Since it was correctly detected, I'll now remove that commit.
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608709082)
Why are we setting this, or why are we dropping it for Darwin?

We no-longer need it given we aren't building Darwin tools. It's been disabled for all other hosts since Guix was introduced. I can dig up the history and post it here.
💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608712706)
Ah, sorry, I misread. Had it backwards. It looked like it was being applied only to darwin before, but it was applied to everything _but_ darwin before. Now darwin is no longer an exception.

Thanks.
💬 achow101 commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#issuecomment-2123141284)
ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef
🚀 achow101 merged a pull request: "net: make the list of known message types a compile time constant"
(https://github.com/bitcoin/bitcoin/pull/29421)
👍 AngusP approved a pull request: "test: improve robustness of connect_nodes()"
(https://github.com/bitcoin/bitcoin/pull/30118#pullrequestreview-2069197384)
re-ACK 6629d1d0f8285d1bf2d87341a856abe903f26c13
💬 AngusP commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1608680140)
Style nit: Mixture of `'` and `"` strings here and in a few places, though it was already mixed before -- `"` seems to be the style in this file
💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2123226341)
`DSYMUTIL` does not get properly set in the config.site after these changes, and the wrong one ends up being found by our configure.

It's not clear to me if dsymutil ends up being used as part of our build at all. If so we should fix depends, if not we should remove it.
💬 laanwj commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#discussion_r1608823197)
Might as well make these `std::string`? Or is there still a good reason that these are `char*`? (it used to be that the message-making functions took char*, but i don't think that's the case anymore)
👍 theuni approved a pull request: "refactor: Model the bech32 charlimit as an Enum"
(https://github.com/bitcoin/bitcoin/pull/30047#pullrequestreview-2069439297)
utACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd
💬 hebasto commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#issuecomment-2123282028)
Concept ACK.
💬 Geremia commented on issue "LevelDB error: Corruption: block checksum mismatch didn't trigger reindex.":
(https://github.com/bitcoin/bitcoin/issues/30138#issuecomment-2123313968)
If I delete the blocks, it triggers a reindex when I restart the app. Perhaps the blocks were corrupted?

<sup>Also, I have a backup of the chainstate from May 11. [I wish I could re-download only the blocks since then, not have to re-download the entire chain again.](https://bitcoin.stackexchange.com/q/123059/4334)</sup>
💬 willcl-ark commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#discussion_r1608865277)
IIRC this was left because `constexpr std::string` was not available, but perhaps it is now in c++20?
📝 naiyoma opened a pull request: "cli: restrict multiple exclusive argument usage in bitcoin-cli"
(https://github.com/bitcoin/bitcoin/pull/30148)

This PR is a continuation of the validation suggested [here](https://github.com/bitcoin/bitcoin/pull/27815) to ensure that only one Request Handler can be specified at a time.
💬 laanwj commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#discussion_r1608884687)
i mean it's defining an array of `std::string` constexpr below it, so i would expect single std::strings to be possible too, i haven't tried it though
💬 luke-jr commented on issue "LevelDB error: Corruption: block checksum mismatch didn't trigger reindex.":
(https://github.com/bitcoin/bitcoin/issues/30138#issuecomment-2123345373)
I guess we should save a "reindex needed" flag somewhere when LevelDB corruption causes the node to shutdown?

> [I wish I could re-download only the blocks since then, not have to re-download the entire chain again.](https://bitcoin.stackexchange.com/q/123059/4334)

LevelDB corruption means you have a bad chainstate. It's not really possible to rebuild it without a full reindex from genesis, short of us doing something like utreexo commitments (even without a softfork, we could set a flag i
...
fanquake closed an issue: "Rethink thread_local (take 2)"
(https://github.com/bitcoin/bitcoin/issues/29952)
🚀 fanquake merged a pull request: "build: Remove `--enable-threadlocal`"
(https://github.com/bitcoin/bitcoin/pull/30137)
💬 rustyrussell commented on pull request "doc: note that you can assume C++20.":
(https://github.com/bitcoin/bitcoin/pull/30136#discussion_r1608904930)
Ah, I see what you're saying. I was prefer "you can assume c++20" (which was my issue, using <bit>, not "you must meet c++20" which implies you understand all nuances of C++ standards?