💬 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

👍 TheCharlatan approved a pull request: "lint: Check for missing bitcoin-config.h includes"
(https://github.com/bitcoin/bitcoin/pull/29408#pullrequestreview-1895491742)
ACK fa58ae74ea67485495dbc2d003adfbca68d6869b

💬 alfonsoromanz commented on issue "RFC: Migration from NSUserNotificationCenter to UNUserNotificationCenter on macOS":
(https://github.com/bitcoin-core/gui/issues/112#issuecomment-1959153389)
I was not familiar with the process of bundling and signing the application, however I ran the commands mentioned (`make deploy`, `codesign -s - ./Bitcoin-Qt.app`, and `open ./Bitcoin-Qt.app`) and the app ran with no issues. Having said that, I don't see an issue in having to run those commands in order to make notifications work when compiling the code

💬 Sjors commented on pull request "Double -dbache maximum to 32GB":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-1959155804)
Fixed broken `feature_config_args.py` test, taking into account the lower limit on 32 bit systems.