🚀 achow101 merged a pull request: "net: make the list of known message types a compile time constant"
(https://github.com/bitcoin/bitcoin/pull/29421)
(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
(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
(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
💬 m3dwards commented on issue "ci: Enable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2123216806)
@maflcko WIP on moving ASAN task over to GHA: https://github.com/m3dwards/bitcoin/actions/runs/9178831587/job/25239590853
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2123216806)
@maflcko WIP on moving ASAN task over to GHA: https://github.com/m3dwards/bitcoin/actions/runs/9178831587/job/25239590853
💬 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.
(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)
(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
(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.
(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>
(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?
(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.
(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
(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
...
(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)
(https://github.com/bitcoin/bitcoin/issues/29952)
🚀 fanquake merged a pull request: "build: Remove `--enable-threadlocal`"
(https://github.com/bitcoin/bitcoin/pull/30137)
(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?
(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?
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2123377364)
> 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.
I think it might actually be unused at this point, but also have a vauge memory of a Qt related failure, if it's missing.. Have pushed up a change to have it set properly for now.
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2123377364)
> 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.
I think it might actually be unused at this point, but also have a vauge memory of a Qt related failure, if it's missing.. Have pushed up a change to have it set properly for now.
💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608938596)
Needs update?
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608938596)
Needs update?
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608939683)
In Guix this should just be dsymutil, not llvm-dsymutil.
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608939683)
In Guix this should just be dsymutil, not llvm-dsymutil.
💬 AngusP commented on pull request "contrib/signet/miner: increase miner search space":
(https://github.com/bitcoin/bitcoin/pull/30130#discussion_r1608940916)
Sorry style nit here and later on, should use `"` not `'` for strings and `%` formatting for logging
```suggestion
logging.debug("grinding headhex: %s", headhex)
```
(https://github.com/bitcoin/bitcoin/pull/30130#discussion_r1608940916)
Sorry style nit here and later on, should use `"` not `'` for strings and `%` formatting for logging
```suggestion
logging.debug("grinding headhex: %s", headhex)
```