📝 theStack opened a pull request: "depends: remove no longer needed patch for Boost::Process"
(https://github.com/bitcoin/bitcoin/pull/29844)
As Boost::Process has been replaced by cpp-subprocess (PR #28981), this patch touches an unused code part and is hence not needed anymore.

👍 rkrux approved a pull request: "test: Refactor fee calculation to remove satoshi_round function"
(https://github.com/bitcoin/bitcoin/pull/29566#pullrequestreview-1991321508)
tACK [b7a4a81](https://github.com/bitcoin/bitcoin/pull/29566/commits/b7a4a8118b43ebd2ab1b64247990ff49d6f19d26)

Make successful, all functional tests successful.

> To address this, the satoshi_round function is removed, and Decimal values are utilized in its place.
1. As per the second commit, the `satoshi_round` is back, let's update the PR description to reflect that because it will be picked up by the bot on merge?
2. Should we consider squashing the first 2 commits into one because af
...

💬 rkrux commented on pull request "test: Refactor fee calculation to remove satoshi_round function":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1559202803)
We can't use `satoshi_round()` here because it will by default round down and that's not what's needed here? Wondering if that function can also provide an option to not round at all.

👍 hebasto approved a pull request: "depends: remove no longer needed patch for Boost::Process"
(https://github.com/bitcoin/bitcoin/pull/29844#pullrequestreview-1991326457)
ACK 95c594f4e9ea3cb57aa03b75d4d70fe0e1742065, I have reviewed the code and it looks OK.

💬 sr-gi commented on pull request "test: Extends wait_for_getheaders so a specific block hash can be checked":
(https://github.com/bitcoin/bitcoin/pull/29736#discussion_r1559208410)
Sure, because "getheaders" is in `last_message` for that peer, but only for that one as the assert after the loop is checking.

The later `wait_for_getheaders` (the one on `expected_peer`) is checked on the other node only. I guess that is not too clear based on how `expected_peer` is initialized. We could just set `expected_peer` as follows to make it clearer:

```python
for p in [peer2, peer3]:
with p2p_lock:
if "getheaders" in p.last_message:
count += 1

...

👍 fanquake approved a pull request: "depends: remove no longer needed patch for Boost::Process"
(https://github.com/bitcoin/bitcoin/pull/29844#pullrequestreview-1991335394)
ACK 95c594f4e9ea3cb57aa03b75d4d70fe0e1742065

📝 stickies-v opened a pull request: "rpc: return warnings as an array instead of just a single one"
(https://github.com/bitcoin/bitcoin/pull/29845)
The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning is returned.

Fix that by returning all warnings as an array.

As a side benefit, clean up the GetWarnings() logic.

Since this PR changes the RPC result schema, I've added release notes. Adding a `-deprecatedrpc` option for a change this simple seems overkill in my view and I'd rather not do, but I'm ha
...

🚀 fanquake merged a pull request: "depends: remove no longer needed patch for Boost::Process"
(https://github.com/bitcoin/bitcoin/pull/29844)

💬 maflcko commented on pull request "refactor, bench, fuzz: Drop unneeded `UCharCast` calls":
(https://github.com/bitcoin/bitcoin/pull/29820#issuecomment-2047172096)
Just for reference, the reason this is safe, is not because of the line linked to in the pull request description, but due to the existing `UCharCast` in the function. Error when trying to pass the wrong type:


```
./key.h:103:26: error: no matching function for call to 'UCharCast'
103 | } else if (Check(UCharCast(&pbegin[0]))) {
| ^~~~~~~~~
note: in instantiation of function template specialization 'CKey::Set<__gnu_cxx::__normal_iterator<int *, s
...

💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1559223172)
I remember trying a few combinations of this and in all cases the compiler/liker failed to find the semaphore. I just retried with the following patch which I think is similar to what you describe.

```patch
diff --git a/src/util/trace.h b/src/util/trace.h
index a4f0ea1ca4..8d7f6bdb74 100644
--- a/src/util/trace.h
+++ b/src/util/trace.h
@@ -27,8 +27,8 @@
// automatically incremented by tracing frameworks (bpftrace, bcc, libbpf, ...)
// upon attaching to the tracepoint and decremented
...

🤔 BrandonOdiwuor reviewed a pull request: "rpc: method removeprunedfunds should take an array of txids"
(https://github.com/bitcoin/bitcoin/pull/29468#pullrequestreview-1991364802)
Concept ACK

💬 fanquake commented on pull request "util/system: Close non-std fds when execing slave processes":
(https://github.com/bitcoin/bitcoin/pull/22417#issuecomment-2047265597)
What is the status of this now that Boost Process is removed (given that this currently adds Boost Process code)?

💬 maflcko commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1559292092)
Thanks for the explanation. I don't see another solution right now.

💬 ajtowns commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#discussion_r1557976175)
I guess the hash160 will better guarantee uniqueness, but a 40 character string is kind-of long? Would it make sense to use the 8 character `HexStr(Params().MessageStart())` value instead, special-casing for when the message start magic number matches the default signet's `0a03cf40`?

💬 maflcko commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#discussion_r1559309031)
nit in 0737f4ea62dd85fe7e8914bb82086a2628ae8204: Would be nice to add the error message to the commit message

📝 fanquake opened a pull request: "guix: replace GCC unaligned VMOV patch with binutils patch"
(https://github.com/bitcoin/bitcoin/pull/29846)
Rather than invasively patching GCC, given we have binutils 2.38 available, we can patch it to flip the default for
`-muse-unaligned-vector-move`.

A 1 line binutils patch, is much more maintainable than the ~300 line patch into GCC. It's also a slight inprovement in regards to patching out ualigned instructions in the release binaries. For comparison:
Master:
```bash
objdump -D bin/*.exe | rg "vmova|vmovdqa|vmovaps|vmovapd|vmovdqa64|vmovdqa32"
141b8be20: c5 f8 28 1a vmo
...

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1559354990)
Woops yes will fix

💬 torkelrogstad commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-2047432612)
It's a fair point to not want to introduce new instances of undesirable behavior. But I **definitely** don't think we should change existing behavior. That would break ALL users that use Go for interfacing with Bitcoin Core, and maybe other languages as well

📝 paplorinc opened a pull request: "refactor: Simplify base32/64 padding calculations"
(https://github.com/bitcoin/bitcoin/pull/29847)
Instead of modifying the string view by removing the suffixes manually, we trim the `ConvertBits` iteration instead by counting the trailing "=" chars.

Also added roundtrip tests for safety.

💬 willcl-ark commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2047459040)
Will this close #28413 ?