💬 glozow commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349136997)
(This is a repost, sorry about that)

> It's a regression fuzz test for [#26355](https://github.com/bitcoin/bitcoin/pull/26355)

Should this fuzzer fail if I make this line return the result from IsContinuationOfLowWorkHeadersSync?
https://github.com/bitcoin/bitcoin/blob/cf0120ff024aa73a56f2975c832fda6aa8146dfa/src/net_processing.cpp#L2900

I briefly fuzzed after making the change and didn't hit anything. Likely I just didn't fuzz enough, but posting just in case somebody has a fuzz outpu
...

👍 ryanofsky approved a pull request: "log: Use ConstevalFormatString"
(https://github.com/bitcoin/bitcoin/pull/30889#pullrequestreview-2303344498)
Code review ACK fabd78e2e30e557e04739e29c9c221ad47245df1. This is a simple change that should be useful for developers.

---

Do we know if this change affects compile time in any noticeable way? Might be worth checking since there are a lot of print statements, and compiler is now doing a little more work for each of them.

---

It might also be worth updating description to give a sense of what errors look like. For me they look like:

```c++
src/util/string.h: In function ‘int main
...

💬 jonatack commented on pull request "doc: unit test runner help fixup and improvements":
(https://github.com/bitcoin/bitcoin/pull/30890#issuecomment-2349148563)
Thanks everyone, updated to take all feedback.

👍 tdb3 approved a pull request: "doc: unit test runner help fixup and improvements"
(https://github.com/bitcoin/bitcoin/pull/30890#pullrequestreview-2303356646)
re ACK 44caadd83dbc612f41c70baaea3055c52f27df90

💬 jonatack commented on pull request "doc: unit test runner help fixup and improvements":
(https://github.com/bitcoin/bitcoin/pull/30890#issuecomment-2349155516)
(Thanks @tdb3, repushed to put the text suggested by @pablomartin4btc in the right place.)

👍 itornaza approved a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2303370741)
ACK b95bb2179610183d9398d50d8c8fd24b1450ad4d

The changes are on `src/ipc/capnp/common-types.h` and greatly enhance code reuse, readability and clarity through the use of concepts. As per the standard review procedure, I rebuilt and run all the tests locally and all of them pass.

💬 mzumsande commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1759034089)
I agree that it shouldn't be possible to have undo data but no block data.
On the other hand, callers of this function may not even be interested in the block data, and I'm not sure if a RPC call is the right place to check for this inconsistency. At least it's checked in `CheckBlockIndex()` [here](https://github.com/bitcoin/bitcoin/blob/06a9f7789e359131abad2489a221888f711bdc3b/src/validation.cpp#L5326), so I think I'll leave this as is for now.

💬 mzumsande commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1759034840)
fixed

💬 mzumsande commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2349173516)
[a5df908 ](https://github.com/bitcoin/bitcoin/commit/a5df908d981138f066f5f963a1813d453483814a)to [6a1aa51](https://github.com/bitcoin/bitcoin/commit/6a1aa510e31e8b77793341473aa5afc9d023a6e3): Fixed typos.

👍 pablomartin4btc approved a pull request: "doc: unit test runner help fixup and improvements"
(https://github.com/bitcoin/bitcoin/pull/30890#pullrequestreview-2303389366)
re-ACK 282f0e92559da23e356504a564a0322b9888e50b

Thanks!

💬 sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#discussion_r1759046001)
We were not checking it before, nor are we doing so later when emplacing it in `m_peer_map` either, so I'd lean towards no for consistency, but I don't have a strong opinion about it

👍 tdb3 approved a pull request: "doc: unit test runner help fixup and improvements"
(https://github.com/bitcoin/bitcoin/pull/30890#pullrequestreview-2303400654)
re ACK 282f0e92559da23e356504a564a0322b9888e50b

Latest change is moving the line about `--list-content`

💬 fanquake commented on issue "cmake: Installed static kernel library is unusable":
(https://github.com/bitcoin/bitcoin/issues/30801#issuecomment-2349209374)
@TheCharlatan what is the status of this now?

👍 andrewtoth approved a pull request: "rpc, rest: Improve block rpc error handling, check header before attempting to read block data."
(https://github.com/bitcoin/bitcoin/pull/30410#pullrequestreview-2303417074)
re-ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3

💬 andrewtoth commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1759063676)
nit: For consistency, we might want to update this function too to take `hash` as the first parameter, even if not needed.

💬 andrewtoth commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1759057303)
nit: why remove the extra context that we are testing with verbosity 2 and 3 specifically?

👍 itornaza approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2303431473)
Code review ACK a93c171faa7b4426823466e972c8f24260918bbf

Offering an option for the Template Provider to get block header data as soon as possible is a great addition.

Code changes look straight forward. Also, built the pr and successfully run all tests on it locally.

💬 fanquake commented on pull request "guix: Pointer Authentication and Branch Target Identification for aarch64 Linux":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-2349221011)
Rebased for #30433, and updated to add an export allowance for `__libc_single_threaded`.

💬 maflcko commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#issuecomment-2349225738)
ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5 🗞

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 07f4cebe5780f1039541d989e6
...

💬 rkrux commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2349229940)
@monlovesmango Thanks for sharing your feedback. I've gone through them and these points are correct, I've updated the guide.