💬 BrandonOdiwuor commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1958792354)
@maflcko When wallet is disabled these RPS are reported as uncovered;


💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1498754677)
Not really related. Change these constants in any way, and the `0.001` value won't be affected.

💬 maflcko commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1498772309)
This change is wrong, no?

In general, please pay attention to the details yourself, before and after pushing the changes. The goal of review is not to write the changeset for you line-by-line, but to double check a finished, complete changeset for any accidental oversights.

💬 naumenkogs commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-1958853401)
ACK 4a45b28a3f258bf156eca980aace69e0a90554cd

💬 naumenkogs commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1498775688)
I think we can go ahead with this PR without waiting for clarification.

💬 maflcko commented on pull request "[25.2] Final backports and changes for 25.2rc1":
(https://github.com/bitcoin/bitcoin/pull/29464#issuecomment-1958866836)
lgtm

👋 maflcko's pull request is ready for review: "test: Remove struct.pack from almost all places"
(https://github.com/bitcoin/bitcoin/pull/29401)

💬 naumenkogs commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1958901201)
Concept ACK. Looking forward to addressing already pending comments, then i review.

💬 naumenkogs commented on issue "Gathering Priorities of 28.0":
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1958905829)
Erlay #28646

💬 maflcko commented on pull request "[fuzz] Avoid partial negative result":
(https://github.com/bitcoin/bitcoin/pull/29462#issuecomment-1958940749)
Tested with:

```
./autogen.sh && CC=clang CXX=clang++ ./configure -q --enable-c++20 --enable-fuzz --with-sanitizers=fuzzer,undefined,integer,float-divide-by-zero && make clean && make
```

and https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1495735626 to confirm the fix

ACK 9dae3b970a7a82e8d9f3f755048d427da78c49da

💬 maflcko commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-1958944099)
ACK 4a45b28a3f258bf156eca980aace69e0a90554cd 🛌

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 4a45b28a3f258bf156eca980aa
...

💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1498892353)
fixed

💬 maflcko commented on pull request "test: Remove struct.pack from almost all places":
(https://github.com/bitcoin/bitcoin/pull/29401#issuecomment-1958997514)
rebased

💬 maflcko commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-1959006624)
review ACK 16baf8651d17ac1e0c7d7fd3526c19ad018ceb9c

💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1498907558)
@murchandamus Indeed, my suggestion was wrong. The overall result never underflows, just the temporary subexpression.

💬 TheCharlatan commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1498934743)
If I'm reading this right, not specifying a failure value will lead to it being default constructed, so this will have a value of `ChainstateLoadError::FAILURE`. Could omitting a failure value be a compile-time error if the failure type is non-void?

💬 sipa commented on pull request "[fuzz] Avoid partial negative result":
(https://github.com/bitcoin/bitcoin/pull/29462#issuecomment-1959050896)
utACK 9dae3b970a7a82e8d9f3f755048d427da78c49da

💬 maflcko commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1959076888)
Why would they? If the RPCs are not compiled into the binary, how would the test know they exist?

📝 fanquake locked a pull request: "Revert "multiprocess: Add basic type conversion hooks""
(https://github.com/bitcoin/bitcoin/pull/29463)
Reverts bitcoin/bitcoin#28921

💬 Sjors commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#issuecomment-1959130540)
re-utACK 36c82cd0e23b4149fb145d2dd271ab1de385c53a