💬 w0xlt commented on pull request "wallettool, test: Remove BDB/ legacy wallets dump feature":
(https://github.com/bitcoin/bitcoin/pull/33161#discussion_r2447059077)
If I am understanding correctly, wouldn't be better to skip only `test_legacy_dump_is_no_longer_allowed` if there are no previous releases ?

🚀 fanquake merged a pull request: "test: [move-only] binary utils to utils.py"
(https://github.com/bitcoin/bitcoin/pull/33633)

🤔 rkrux reviewed a pull request: "rpc: add optional peer_ids param to filter getpeerinfo"
(https://github.com/bitcoin/bitcoin/pull/32741#pullrequestreview-3359254765)
crACK 599628ab9c23cd53a30fcbcb5d4ac370bb536547

💬 rkrux commented on pull request "rpc: add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2447121145)
Nit if retouched:

```diff
diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py
index 209e804021..47834c8375 100755
--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -200,6 +200,7 @@ class NetTest(BitcoinTestFramework):
nonexistent_id = max(p["id"] for p in all_peers) + 1000
assert_equal(node.getpeerinfo(nonexistent_id), [])
assert_raises_rpc_error(-8, "must be a number or an array of numbers", node.getpeerinfo, {})
+ assert_eq
...

🤔 maflcko reviewed a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3359282710)
re-ACK c864a4c1940d682f7eb6fdb3b91b18d638b59330 🌥

<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: re-ACK c864a4c1940d682f7eb6
...

💬 maflcko commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#discussion_r2447142735)
nit in c864a4c1940d682f7eb6fdb3b91b18d638b59330: I understand that you probably used the std namespace here and then used `auto` to silence the linter? However, this seems to go in the wrong direction. If we think our `fs::` namespace should be preferred over the `std::` fs one, we should try to use it as much as possible and only call `std_path` when needed. I guess restoring `fs::path` here would require some overloads in the wrapper class, or restoring some `fs::path` constructors below. But
...

💬 alexanderwiederin commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3425341211)
re-ACK https://github.com/bitcoin/bitcoin/pull/30595/commits/edf99b88e644c7d2a2db434c8db298c9c5303cf9

💬 fanquake commented on pull request "wallet: refactor to remove redundant sighash calculation":
(https://github.com/bitcoin/bitcoin/pull/33665#issuecomment-3425407567)
https://github.com/bitcoin/bitcoin/actions/runs/18676072881/job/53246386885?pr=33665#step:9:3427:
```bash
Run script_sign with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/script_sign')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2902865681
INFO: Loaded 1 modules (615865 inline 8-bit counters): 615865 [0x56e10e6f9bb8, 0x56e10e790171),
INFO: Loaded 1 PC
...

💬 fanquake commented on pull request "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script":
(https://github.com/bitcoin/bitcoin/pull/33470#issuecomment-3425433100)
Guix Build
```bash
0dd1c0d4567f1eabfa23b88b1d26ca584ac97381f9572abaff3e0ad37f3b7ecf guix-build-4b41f99d57d8/output/aarch64-linux-gnu/SHA256SUMS.part
75bf3226050a5aad14bcf59b55dbf8b59ad7c287389c8ef42161bbff6eea070e guix-build-4b41f99d57d8/output/aarch64-linux-gnu/bitcoin-4b41f99d57d8-aarch64-linux-gnu-debug.tar.gz
d45f01491dd6254066689e9516a3c96aee30ea6f60648f06dfdb8c364194908e guix-build-4b41f99d57d8/output/aarch64-linux-gnu/bitcoin-4b41f99d57d8-aarch64-linux-gnu.tar.gz
14338708b2ba862f2
...

💬 maflcko commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3425448023)
lgtm ACK 08cf213ec7e671dd4162d4ae4d0d20810547a444

Seems harmless to add and could be useful information when it is present.

💬 instagibbs commented on pull request "p2p: implement sender-initiated package relay":
(https://github.com/bitcoin/bitcoin/pull/33500#discussion_r2406742176)
wer're sending the parent here, I don't think we're marking the parent as in the known inventory, which means we will keep sending it even if it's just new children, if f.e. the parent is 0-fee.

🤔 instagibbs reviewed a pull request: "p2p: implement sender-initiated package relay"
(https://github.com/bitcoin/bitcoin/pull/33500#pullrequestreview-3305146357)
just dropping this before I forget

🚀 fanquake merged a pull request: "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script"
(https://github.com/bitcoin/bitcoin/pull/33470)

💬 fanquake commented on pull request "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3425470273)
Please rebase. Please also remove all the (what looks like AI generated) text from the commit messages & PR description. You should also post your Guix build result.

💬 fanquake commented on pull request "fix: invalid plist format and update values to meet macOS 1.0 standard":
(https://github.com/bitcoin/bitcoin/pull/33654#discussion_r2447310112)
> the correct minimum version is 10.14.0

Our minimum supported version is 14.0.

✅ fanquake closed a pull request: "fix: invalid plist format and update values to meet macOS 1.0 standard"
(https://github.com/bitcoin/bitcoin/pull/33654)

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3425525600)
> For reference https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/chain.cpp.gcov.html is pretty well covered otherwise, but these tests can help with the specifics.

Thanks for the link! Indirectly it is tested extensively, as the skip mechanism is used while searching in the chain. However, there are no tests focusing on this specifically, and the why and how on the exact skip generation algorithm is not documented.

💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3425621674)
> I can't reproduce locally.

Are you sure? Above I tried on Fedora podman-docker, and today it also failed on Ubuntu podman-docker:

```
(root)# docker --version
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
podman version 4.9.3

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2447496922)
> I also considered having all cluster size limits being treated as RECONSIDERABLE, but decided that is also not great, partly because we certainly shouldn't let a transaction whose cluster size just with its own unconfirmed ancestors be treated as RECONSIDERABLE

👍 definitely think we should not allow reconsideration in that case

Sounds good to me to make it non-reconsiderable for now, and maybe change it in the future in a consistent way. I agree it's possible for a child to make a tx no
...

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2447498384)
Great, marking resolved and will refer to this in a followup!