💬 achow101 commented on pull request "validation: Disable CheckForkWarningConditions for background chainstate":
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2374863380)
ACK c0a0c72b4d68a4f0c53c2c4b95f4d6e399f8e4ee

✅ achow101 closed an issue: "`invalidateblock` during background validation"
(https://github.com/bitcoin/bitcoin/issues/30958)

🚀 achow101 merged a pull request: "validation: Disable CheckForkWarningConditions for background chainstate"
(https://github.com/bitcoin/bitcoin/pull/30962)

💬 Oly6 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/39219fe145e5e6e6f079b591e3f4b5fea8e71804#commitcomment-147202742)
good

💬 achow101 commented on pull request "[28.x] backports and finalize (or rc3)":
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2374879207)
Added #30962

I think it's trivial enough to not require rc3 as well.

💬 achow101 commented on pull request "validation: Disable CheckForkWarningConditions for background chainstate":
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2374879640)
Backport in #30959

💬 fjahr commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2374881703)
> Is there no way to query AS Maps directly from the BGP tables?

Who's BGP table? Can you be a bit more specific what you are thinking of in terms of architecture and trust assumptions?

🤔 ryanofsky reviewed a pull request: "refactor: Implement missing error checking for ArgsManager flags"
(https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2329185957)
Updated 87b6d4cb5a5c04f6c8542c633c3bfa5f76901d43 -> 5a945600451037693a032e6df94f99a666dd581f ([`pr/argcheck.41`](https://github.com/ryanofsky/bitcoin/commits/pr/argcheck.41) -> [`pr/argcheck.42`](https://github.com/ryanofsky/bitcoin/commits/pr/argcheck.42), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/argcheck.41..pr/argcheck.42)) with suggested formatting & grammar tweaks.

re: https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2325050760

Thanks for the review. To
...

💬 ryanofsky commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1775793996)
re: https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1773221823

> nit: you're using both `can not` and `cannot` in the errors

Seems like cannot is better so switched to that, thanks!

💬 ryanofsky commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1775792589)
re: https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1773293766

Happy to change, but I'm not really sure what to do here. I always just look at plaintext comments, not generated documentation, and I do think having breaks between list items makes the plaintext comment more readable since most of the items are paragraphs. I could get rid of the break before the first item while keeping the breaks before the other items, though, if that works and would be preferred.

💬 ryanofsky commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1775766244)
re: https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1773291395

Thanks, applied suggestion

👍 ryanofsky approved a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968#pullrequestreview-2329376198)
Code review ACK e9d60af9889c12b4d427adefa53fd234e417f8f6. Nice change to make AppInitMain shorter and more understandable.

💬 ryanofsky commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#discussion_r1775897574)
Not important, for but both of the InitAndLoadChainstate calls in this PR, I'm surprised someone would prefer newline separated arguments over space separated:

```c++
InitAndLoadChainstate(node, do_reindex, do_reindex_chainstate, cache_sizes, args);
```

clang-format has a nice "BinPack" and "Align" options for packing arguments neatly without having to take up large amounts of space

💬 TheCharlatan commented on pull request "build: Add missing USDT header dependency to kernel":
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2375104990)
Concept ACK

Good find, I think just including it like you've done here is ok for now.

💬 TheCharlatan commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#discussion_r1775916783)
Mmh, I think this must have been something I did out of old habit. It would be good to have these stricter clang format options documented in the dev notes so we can at least all try to write similarly formatted new code.

💬 mzumsande commented on issue "assumeutxo: not syncing the snapshot chainstate":
(https://github.com/bitcoin/bitcoin/issues/30971#issuecomment-2375136490)
That's strange. I just tried to reproduce syncing this snapshot with current master (39219fe145e5e6e6f079b591e3f4b5fea8e71804) and didn't experience this (although I do get get similar "Cache size" log messages).

Are you syncing from random peers or do you have some local setup?

👍 ryanofsky approved a pull request: "test: add test for specifying custom pidfile via `-pid`"
(https://github.com/bitcoin/bitcoin/pull/30724#pullrequestreview-2329424579)
Code review ACK 04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a

🤔 jarolrod reviewed a pull request: "doc: Add `nproc` support for Mac through `coreutils`"
(https://github.com/bitcoin/bitcoin/pull/30936#pullrequestreview-2329456514)
Don't agree with having this in the build doc, no need to have a user install such a package that is not needed to build bitcoin. Docs should only have users install required dependencies. If it really helps, productivity notes could point a user as to how they can have `nproc` support.

A mac user can also just `alias nproc="sysctl -n hw.logicalcpu"`, no need for us to be opinionated on installing a dependency for this in this doc.

💬 theuni commented on pull request "ci: add `LLVM_SYMBOLIZER_PATH` to Valgrind fuzz job":
(https://github.com/bitcoin/bitcoin/pull/30961#issuecomment-2375175185)
Makes sense to me. I assume this could also be solved with a symlink or update-alternatives, but even if so, using the env var seems simpler.

💬 l0rinc commented on pull request "doc: Add `nproc` support for Mac through `coreutils`":
(https://github.com/bitcoin/bitcoin/pull/30936#issuecomment-2375187987)
> A mac user can also just alias nproc="sysctl -n hw.logicalcpu"

Absolutely, but let's document these somewhere, since we're using `-j$(nproc)` in a few other places in the code, without mentioning that Mac users will need to do a few extra steps