🤔 hebasto reviewed a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2318027774)
My Guix build:
```
aarch64
f65f7839cb8ce4b194ab55a698b9840dd335d790a5499d3303cf79885405bedb guix-build-7025942687fd/output/aarch64-linux-gnu/SHA256SUMS.part
7953c080586b3500c73edd93731b8b9bcfd79ac4f5fe7953d51dc54643899255 guix-build-7025942687fd/output/aarch64-linux-gnu/bitcoin-7025942687fd-aarch64-linux-gnu-debug.tar.gz
c82339ac1813727df996deda765087d430080a62d5fff7e16f46727be703a68c guix-build-7025942687fd/output/aarch64-linux-gnu/bitcoin-7025942687fd-aarch64-linux-gnu.tar.gz
ffe2b02e
...

🤔 hebasto reviewed a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2318031732)
Post-merge re-ACK 7025942687fd5e91d0a10ce5b2ac673b67a63491.

💬 maflcko commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1768466480)
Sure, done!

💬 maflcko commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2363634967)
Thanks! However, I think there was some kind of issue with the fuzz inputs, so that the macOS 14 CI ran into timeouts.

I've removed them temporarily again in https://github.com/bitcoin-core/qa-assets/commit/84cea7068728bc2c765b9da680eedcb85f9f3c1b . This should allow for some time to investigate, while unbreaking the CI.

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2363637034)
Sorry, wrong alarm. The timeout may be real. It may be `p2p_headers_presync` from https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2363634967

💬 maflcko commented on pull request "ci: Approximate MAKEJOBS in image build phase":
(https://github.com/bitcoin/bitcoin/pull/30935#issuecomment-2363643874)
(This is a follow-up to https://github.com/bitcoin/bitcoin/pull/28504)

💬 stickies-v commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1768582575)
I renamed it to `try_format` to be clear about the different behaviour compared to `tfm::format`, but agreed that avoiding merge conflicts is useful to coordinate on.

> My preference would be to leave the name as-is and instead just delete the unchecked one

Could you elaborate on this a bit more? We have these 4 other `format` overloads, I don't think you want to just remove them all? At least 2) and 4) would imply touching quite a bit of code to refactor, and it seems not all call sites o
...

💬 maflcko commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1768605114)
IIUC the compiler will lookup `format(const char*, ...)` for `format("%s, 1)`, so `format(const char*, ...)` would have to be deleted for the compiler to pick `format(ConstevalFormatString, ...)`. (My recommendation for this pull would be to do at least this)

IIUC the remaining `format` taking a `std::string` can be left as-is for now, as they are less common anyway than the ones taking a compile-time string literal. (Possibly something can be done for them as well, but it seems fine to leave
...

🤔 pablomartin4btc reviewed a pull request: "refactor: move `SignSignature` helpers to test utils"
(https://github.com/bitcoin/bitcoin/pull/30561#pullrequestreview-2318311398)
ACK 58499b00d0ad1c83e433caa8fcc0e5d3fd3f2070

The helpers are already used exclusively within the test code; this is a suitable cleanup.

💬 stickies-v commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1768664595)
> so `format(const char*, ...)` would have to be deleted for the compiler to pick `format(ConstevalFormatString, ...)`

I think just deleting `format(const char*, ...)` leads to ambiguous overloading?

```
../../../../src/checkqueue.h:136:36: error: call to 'format' is ambiguous
util::ThreadRename(strprintf("scriptch.%i", n));
^~~~~~~~~
../../../../src/tinyformat.h:1151:19: note: expanded from macro 'strprintf'
#define strprintf tfm::fo
...

💬 maflcko commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1768680022)
> I think just deleting `format(const char*, ...)` leads to ambiguous overloading?

Thanks for actually checking. I guess this means that the `std::string` overload would have to be renamed to avoid the overload problem. Hopefully only the fuzz tests and a few lines in real code use the string overload?



> you also asked to remove `./test/lint/lint-format-strings.py` in this pull which I think we shouldn't do before all `tfm::format` overloads are compile-time checked? So that's why I'm
...

💬 marcofleon commented on pull request "fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work":
(https://github.com/bitcoin/bitcoin/pull/30918#discussion_r1768704392)
That's true, good note.

💬 marcofleon commented on pull request "fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work":
(https://github.com/bitcoin/bitcoin/pull/30918#issuecomment-2363837325)
I also updated the PR description to include some additional changes.

⚠️ bitcoin-tools opened an issue: "`test_bitcoin` from pre-built 28.0rc2 tarball is failing for a JSON parsing error"
(https://github.com/bitcoin/bitcoin/issues/30938)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Tested the 28.0rc2 packages from [bitcoincore.org/bin](https://bitcoincore.org/bin/bitcoin-core-28.0/test.rc2/) for Linux x86_64, Darwin x86_64, and Darwin arm64.

Seeing consistent failures in the rc2 `test_bitcoin` binary running:
- Docker with base image of Arch ([1](https://github.com/bitcoin-tools/nodebuilder/actions/runs/10958688270/job/30429843131), [2](https://github.com/bitcoin
...

💬 instagibbs commented on pull request "refactor: move `SignSignature` helpers to test utils":
(https://github.com/bitcoin/bitcoin/pull/30561#issuecomment-2363854460)
ACK 58499b00d0ad1c83e433caa8fcc0e5d3fd3f2070

thanks for the cleanup!

💬 maflcko commented on issue "`test_bitcoin` from pre-built 28.0rc2 tarball is failing for JSON parsing":
(https://github.com/bitcoin/bitcoin/issues/30938#issuecomment-2363865740)
This can be worked around by installing python3. An alternative would be something like done here: https://github.com/bitcoin/bitcoin/pull/29868/files#r1747435819 (but that diff currently does not compile, as can be seen in the CI output)

💬 instagibbs commented on pull request "fuzz: Add check in `p2p_headers_presync` that chain work never exceeds minimum work":
(https://github.com/bitcoin/bitcoin/pull/30918#issuecomment-2363866498)
reACK 284bd17309ab3b124d9dcddfec62f5506383343b

💬 maflcko commented on issue "`test_bitcoin` from pre-built 28.0rc2 tarball is failing for JSON parsing":
(https://github.com/bitcoin/bitcoin/issues/30938#issuecomment-2363873370)
Another alternative would be to drop 199bb09d88e28d951c5068eb65643390dbedd066 from the 28.x branch. It fixed one bug, but introduced another.

💬 instagibbs commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2363876329)
> I'd suspect we probably wouldn't want that, given that the additional peers introduced here are also likely to be inbound peers (if full), so attacker controlling many of our inbound slots could stall us effectively.

If we were to go that direction, I suspect we would force the "last slot" to always be an outbound, just like the parallel compact block download attempts do now. The parallel cb attempts could instead be a bit more courteous to outbound connections in almost every case.

Jus
...

💬 instagibbs commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#discussion_r1768742804)
`MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK` seems like a variable we could rename to be more general and use here, especially if both mechanisms could fire