💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1752256496)
I wasn't using depends, so had to update multiprocess on my system.

💬 fjahr 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_r1752254628)
nit: I think this would be the same, just shorter?

```suggestion
if (tx->IsCoinBase() || !blockindex || WITH_LOCK(::cs_main, return !(blockindex->nStatus & BLOCK_HAVE_MASK)) ||
```

💬 fjahr 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_r1752259940)
Also it seems like the `!blockindex` can be taken out safely since it's checked a few lines above and if the check fails we throw an error.

💬 fjahr 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_r1752270427)
> I think the suggestion doesn't work because we still need to feed the result into have_undo so that if UndoReadFromDisk fails for an unexpected reason like disk errors, we don't attempt to access blockUndo below and segfault.

I see, I think in that scenario it would be best to throw an error right there instead of ignoring it silently and giving the user an incomplete response? I mean, we have all indication that we should have the undo data in that scenario and then we can not read it, it
...

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2341427105)
I'm going to hold off reviewing this until I see the TxGraph PR to better understand its intended usage. :+1:

⚠️ mcelrath opened an issue: "Race condition between ZMQ UpdateTip and getblocktemplate"
(https://github.com/bitcoin/bitcoin/issues/30862)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

When ZMQ announces a new chain tip via "sequence" "C" message, if `getblocktemplate` is called immediately, it can return a block template that does not reflect the new chain tip.

This is important for Braidpool, which needs to switch to a new chain tip as quickly as possible when a new block comes in.

### Expected behaviour

`getblocktemplate` should return a result consistent with th
...

💬 ryanofsky commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2341480416)
Are there release notes for cmake transation? Seems like the autoconf -> cmake mapping table https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e would be especially helpful to have in release notes to help users and packagers convert previous configurations to new ones.

🤔 stickies-v reviewed a pull request: "util: Use consteval checked format string in FatalErrorf"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2291996189)
I'm still a strong concept ACK on this, but I wonder if the incremental appoach currently taken is the best approach. We'll have to (I think quite likely non-trivially) update the logic based on flags, widths, precisions, conversion, ... actually used in the codebase, as seen with e.g. the precision and zero-padding comments?

So perhaps it'd be sensible to first create a branch that forces `ConstevalFormatString` everywhere, informing our requirements and adding full unit test coverage, and t
...

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1751671633)
Note: it seems we are explicitly preventing `*` dynamic width/precision specifiers here, which was previously allowed (and checked) with the linter. That's not necessarily bad, it seems we're not currently using them (quick non-exhaustive search) and it can of course be added back in later, and we're throwing instead of silently ignoring it.

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1752314431)
We do have quite a few use cases of `\.[\d+]f` precision that would throw here. No issue in the current code, but would like have to be overhauled in the next PR?

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1751669308)
I think having a separate `CountNumberFormatSpecifiers` would have made testing this function a bit easier and with better error feedback, while simultaneously also reducing template instantiation. Neither are overwhelmingly strong arguments, but I don't think there are really any downsides either?

E.g. sample, pretty rough (but passes tests) diff:

<details>
<summary>git diff on faa32adbcf</summary>

```diff
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp

...

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2341522716)
@instagibbs Sure.

To give a bit more background, my idea to store things in `Cluster` form and convert to `DepGraph`, was due to a (mistaken) belief that incremental changes to a `DepGraph` would be $\mathcal{O}(n)$ per-dependency, meaning $\mathcal{O(n^2)$ per-transaction, and thus potentially $\mathcal{O}(n^3)$ for a full $n$ transactions.

@sdaftuar pointed out the possibility of an $\mathcal{O(n)}$ per-transaction algorithm, or $\mathcal{O(n^2)}$ for $n$ transactions, implemented in thi
...

✅ ryanofsky closed an issue: "Improve minimum file descriptor accounting and documentation"
(https://github.com/bitcoin/bitcoin/issues/18911)

🚀 ryanofsky merged a pull request: "init: fixes file descriptor accounting"
(https://github.com/bitcoin/bitcoin/pull/30065)

⚠️ dergoegge opened an issue: "fuzz: `scriptpubkeyman`: heap-buffer-overflow miniscript.cpp in CScript BuildScript"
(https://github.com/bitcoin/bitcoin/issues/30864)
```
$ echo "dHIoJTE3LzwyOzM+LGw6cGsoJTA4KSk=" | base64 --decode > scriptpubkeyman.crash
$ FUZZ=scriptpubkeyman src/test/fuzz/fuzz scriptpubkeyman.crash
...
SUMMARY: AddressSanitizer: heap-buffer-overflow miniscript.cpp in CScript BuildScript<opcodetype, CScript&, opcodetype, CScript&, opcodetype>(opcodetype&&, CScript&, opcodetype&&, CScript&, opcodetype&&)
...
```

💬 QBlockQ commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2341564709)
Just a suggestion (you may send me a BIG thanks after a decade), why don't you consider integration of post-quantum cryptography (PQC) in this time for the next release of Bitcoin Core 27.1. PQC integration code is ready!

💬 achow101 commented on issue "fuzz: `scriptpubkeyman`: heap-buffer-overflow miniscript.cpp in CScript BuildScript":
(https://github.com/bitcoin/bitcoin/issues/30864#issuecomment-2341582360)
I believe this is only an issue with the fuzzer as I can't trigger this crash outside of it. However, it does reveal an actual issue in the handling of multipath key expressions with miniscript.

The issue appears to be because `MiniscriptDescriptor`'s `m_node` is shallow copied, and when cloned fragments belonging to the multipath components are destroyed later, various shared_ptrs end up also being destroyed.

The following diff fixes this particular crash, but I think it is insufficient:
...

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1752469255)
Thanks, fixed.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1752472646)
Thanks, fixed.

💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1752485188)
This error was reported [many](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107852) [times](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109569) for GCC 12.2, managed to reproduce the error via [godbolt](https://godbolt.org/z/8r9TKKoxv) (it's fixed in GCC 12.3) and on a local Dockerfile:
<details>
<summary>Dockerfile</summary>

```dockerfile
FROM docker.io/amd64/debian:bookworm

# Update and install dependencies
RUN apt update && apt install -y \
build-essential \
cmake \

...