💬 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?

^{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)}

💬 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?

💬 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.

💬 theuni commented on pull request "build: LLD based macOS toolchain":
(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.

💬 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)
```

🤔 AngusP reviewed a pull request: "contrib/signet/miner: increase miner search space"
(https://github.com/bitcoin/bitcoin/pull/30130#pullrequestreview-2069607288)
LGTM apart from style nit

💬 AngusP commented on pull request "contrib/signet/miner: increase miner search space":
(https://github.com/bitcoin/bitcoin/pull/30130#discussion_r1608941385)
```suggestion
logging.debug("grinder exhausted the nonce search space, retrying with new block header")
```

💬 AngusP commented on pull request "contrib/signet/miner: increase miner search space":
(https://github.com/bitcoin/bitcoin/pull/30130#discussion_r1608941525)
```suggestion
logging.debug("mining aborted by the user")
```

💬 theuni commented on pull request "cli: restrict multiple exclusive argument usage in bitcoin-cli":
(https://github.com/bitcoin/bitcoin/pull/30148#issuecomment-2123435878)
Concept ACK.

This is very close to @Brotcrunsher's original commit. At the very least they should be a co-author.