👍 TheCharlatan approved a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2293613874)
ACK 9c98c42a0166e9e201f6e9d32a0692fad5a185f0

👍 ryanofsky approved a pull request: "Remove unsafe uint256S() and test-only uint160S()"
(https://github.com/bitcoin/bitcoin/pull/30773#pullrequestreview-2293623641)
Code review ACK 43cd83b0c7ba436b8ffc83d8a65e935714dffe70. Only code change is a small refactoring which looks good. The rest of the PR is all test changes, which I only lightly reviewed, but seem to be positive and do what's described

🚀 ryanofsky merged a pull request: "Remove unsafe uint256S() and test-only uint160S()"
(https://github.com/bitcoin/bitcoin/pull/30773)

📝 achow101 opened a pull request: "descriptor: Add proper Clone function to miniscript::Node"
(https://github.com/bitcoin/bitcoin/pull/30866)
Multipath descriptors requires performing a deep copy, so a Clone function that does that is added to miniscript::Node instead of the current shallow copy.

Fixes #30864

💬 achow101 commented on issue "fuzz: `scriptpubkeyman`: heap-buffer-overflow miniscript.cpp in CScript BuildScript":
(https://github.com/bitcoin/bitcoin/issues/30864#issuecomment-2341894445)
#30866

💬 achow101 commented on pull request "test: Add explicit onion bind to p2p_permissions":
(https://github.com/bitcoin/bitcoin/pull/30805#discussion_r1752653030)
Done

💬 maflcko commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#issuecomment-2341907938)
review ACK 8fb81c21cfaaee4ea68e5023c31d58b01f53fa4f

👍 tdb3 approved a pull request: "test: Add explicit onion bind to p2p_permissions"
(https://github.com/bitcoin/bitcoin/pull/30805#pullrequestreview-2293665953)
ACK a1e737ef089d407256428ea29be20f46bbea71fe

👍 ryanofsky approved a pull request: "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors"
(https://github.com/bitcoin/bitcoin/pull/30765#pullrequestreview-2293686068)
Code review ACK 48b67d8294eb62c5371e7dbc30b7cf811fb7e51c. This looks great! PR is much simpler now, and it is nice to see the GCC workaround expanded and documented so well.

Would suggest a few changes, implemented in diff below:

- Replacing `PushDataSize` `size_t` parameter with `uint32_t` parameter to document fact that it can't accept larger sizes, and to call `WriteLE32` without a type conversion.
- Replacing `PushData` `const value_type* data, size_t size` parameters with `std::span<
...

💬 furszy commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#discussion_r1752677734)
Done. Updated.

💬 furszy commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#discussion_r1752677951)
Done. Updated.

💬 furszy commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#discussion_r1752678000)
Done. Updated.

💬 fjahr commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2341951885)
> > How about adding an instruction for testing assumeutxo mainnet parameters?
>
> Not related to the testing guide, but it could also be mentioned in the release notes that assumeutxo mainnet params are available now.

I have added it to the [release notes draft](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft).

👍 tdb3 approved a pull request: "test: Add explicit onion bind to p2p_permissions"
(https://github.com/bitcoin/bitcoin/pull/30805#pullrequestreview-2293721430)
ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1752696755)
> making implementation details verbose to use is a feature to avoid developers from accidentally using them in real code

I can't immediately see a use case for this function, but is there a reason we'd want to actively discourage this function from being used? It seems harmless to me, but if there is, perhaps it'd be good to add a brief docstring to the function to describe that?

> The verbosity in tests is a bit annoying, but seems acceptable to me.

I agree.

💬 stickies-v commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1752698600)
> Given that they are not used,

Yup, I agree with the simplified approach. I didn't like the half/half of quite a of bit of complexity without being complete, this makes more sense. Can be marked as resolved, thanks!

🤔 TheCharlatan reviewed a pull request: "build: Use CMake's default permissions in macOS `deploy` target"
(https://github.com/bitcoin/bitcoin/pull/30838#pullrequestreview-2293751960)
Guix build (aarch64):
```
8eeb195889318631934b31746dcbd04b26c2312e2aa29dbd7be9de9581a8b805 guix-build-5ba03e7d35e1/output/arm64-apple-darwin/SHA256SUMS.part
05c10866f6972aa58a379935dbc4eb39a8e2abc54a27aa4fa2ce06a7443593ba guix-build-5ba03e7d35e1/output/arm64-apple-darwin/bitcoin-5ba03e7d35e1-arm64-apple-darwin-unsigned.tar.gz
f81ec9368bf068a373ec88428becdef08533e8984ad95a3b017f7f1a8cbbb5b1 guix-build-5ba03e7d35e1/output/arm64-apple-darwin/bitcoin-5ba03e7d35e1-arm64-apple-darwin-unsigned.z
...

🤔 furszy reviewed a pull request: "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD"
(https://github.com/bitcoin/bitcoin/pull/30807#pullrequestreview-2293758551)
> > AssumeUTXO nodes prioritize tip synchronization first, meaning they advertise
> > knowledge of all blocks posterior to the snapshot base block hash.
>
> This is confusing to me - nodes advertise `NODE_NETWORK` (all historical blocks prior to tip) or `NODE_NETWORK_LIMITED` (last 288 from tip). They also communicate their current tip with peers, but it's not part of the advertisement. So this includes (amongst others) blocks **prior** to the snapshot block hash, not posterior.

Yeah ok.
...

💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2342010162)
ACK 59028de422b

👍 theStack approved a pull request: "test: Add explicit onion bind to p2p_permissions"
(https://github.com/bitcoin/bitcoin/pull/30805#pullrequestreview-2293772589)
re-ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c