💬 maflcko commented on pull request "RPC: access RPC arguments by name":
(https://github.com/bitcoin/bitcoin/pull/29277#issuecomment-2047093492)
Does it work with `OBJ_NAMED_PARAMS`?

💬 dergoegge commented on pull request "minisketch: pull subtree + #81":
(https://github.com/bitcoin/bitcoin/pull/29823#issuecomment-2047098733)
Fuzzed for 2000 CPU hours, no more UB was found

✅ fanquake closed an issue: "RFC: Replacing Boost Process"
(https://github.com/bitcoin/bitcoin/issues/24907)

🚀 fanquake merged a pull request: "Replace Boost.Process with cpp-subprocess"
(https://github.com/bitcoin/bitcoin/pull/28981)

👍 dergoegge approved a pull request: "minisketch: pull subtree + #81"
(https://github.com/bitcoin/bitcoin/pull/29823#pullrequestreview-1991284585)
ACK c03fd4e58bfd1ce7f4e67283e64152690cf25c21

💬 brunoerg commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1559180575)
> I think that usually it shouldn't substantially change the observations of a user, because 10 is a pretty high number and if all of those fail, we either have rally bad fixed seeds, or have a connectivity problem.

I agree. I've tested it following `contrib/seeds/README.md` to generate the seeds and the worst scenario was connecting with 5 of the 10.

💬 maflcko commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-2047125355)
Looks like this also fixed the Windows issue (https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-1523507720), so I guess there may have been a bug in the previous implementation.

💬 maflcko commented on pull request "doc: update NeedsRedownload() and nStatus comment":
(https://github.com/bitcoin/bitcoin/pull/29624#issuecomment-2047139574)
lgtm ACK 6cfac2c9094cdb2cc35efa72adef60063693fbfe

I did not test this, to confirm the error message.

📝 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)?