💬 laanwj commented on pull request "node: change a tx-relay on/off flag to enum":
(https://github.com/bitcoin/bitcoin/pull/33567#discussion_r2446862162)
i also prefer mapping the enum value to a string, instead of repeating the log call.

💬 laanwj commented on pull request "node: change a tx-relay on/off flag to enum":
(https://github.com/bitcoin/bitcoin/pull/33567#issuecomment-3424937092)
Concept and code review ACK 07a926474b5a6fa1d3d4656362a0117611f6da2f. Agree with the general reasoning and the change in #29415 is a valid motivation to change this interface.

💬 ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440265397)
In "Add test case for cluster size limits to TRUC logic" cf2f5211a8fab1438feb1cf1ccb86dccedd4b6c8

also add a test for cluster count

```suggestion
self.log.info("Test that a decreased limitclustercount also applies to TRUC cluster")
self.restart_node(0, extra_args=["-limitclustercount=1", "-acceptnonstdtxn=1"])
assert_raises_rpc_error(-26, "too-large-cluster", node.sendrawtransaction, tx_v3_child_large2["hex"])
self.check_mempool([tx_v3_parent_large2["tx
...

💬 ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440116718)
In "Use cluster linearization for transaction relay sort order" 936f04e770eeb4ef8477722cb1f23f29746883a1

nit: This comment is stale?
```suggestion
* higher mining score to sort later. */

```

💬 ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440174148)
In "Remove CTxMemPool::GetSortedDepthAndScore" 9b31836abfc086e4693ecdb1ceaec0a8e21dbb8f

nit
```suggestion
// We are iterating through the mempool entries sorted topologically and by mining score.

```

💬 ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440284422)
In "Use mempool/txgraph to determine if a tx has descendants" b23b156c7bdf112a4fe64acd71e398cc7c0bd231

nice I like this pattern, not leaking internals, I think it will be better to adopt this pattern also in https://github.com/bitcoin/bitcoin/pull/33629/commits/d2f75c555caf1f6a62b8ab1ef0f2543f0c84cc39#r2432661442

💬 ismaelsadeeq commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2440224630)
I think it will even be better to have a struct for this to avoid the repetition of the declaration of the variables and passing the as out params, according to guidelines we prefer returning values https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c-functions-and-methods

💬 maflcko commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3425093000)
> 'm not sure I understand - why would we do anything like this? The LLM is just a tool, it would be weird to say: "suggestions by: clang-tidy"

I think the goal is just to tag obvious obvious hallucinated "one-shot" vibe commits. Anyone using an llm as a tool, just as a way to implement what they understand and could implement fully themselves, can trivially remove the tag from the commit message. Though, it should be harmless to leave it in. Generally, we do mention when a change is motivate
...

📝 rkrux opened a pull request: "wallet: refactor to remove redundant sighash calculation"
(https://github.com/bitcoin/bitcoin/pull/33665)
In the MuSig2 signing flow, the same sighash ends up being recalculated for all the participant public keys involved in the MuSig aggregate keys. This happens for all the three parts of the signing flow: nonce, partial sig, and aggregate sig.

The original intent of this refactor to reduce seeing duplicate code, but it should also help in improving performance of the wallet signing operations.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a
...

💬 Sjors commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#discussion_r2447042601)
@m3dwards wrote:

> nit: The instruction could tell the LLM to add itself rather than the current literal instruction of adding the two hardcoded values. Perhaps they are clever enough to figure out what's being asked.

I tried with Claude Sonnet 4.5 and it just dropped the second line.

🤔 rkrux reviewed a pull request: "refactor: Avoid copies by using const references or by move-construction"
(https://github.com/bitcoin/bitcoin/pull/31650#pullrequestreview-3359165224)
ACK fa9b7f0630691cf6b0add88f646bbcae6466eeaa

Specially for the first two reasons mentioned in the PR description.

💬 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
...