💬 vasild commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2500071189)
This requests a return type of `int` (getInt<_int_>) but it assigns it to `uint64_t` variable.

💬 vasild commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2500078521)
I agree that these long keys look weird.

💬 vasild commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2500065220)
This assumes that the `all_networks` key is the last key in the JSON returned by `getaddrmaninfo` but JSON objects don't have order (unlike arrays). So it is possible that the order changes any time (in theory) and then this code will break. Better write it in such a way that it works regardless of the order in which keys are returned.

Maybe something like:
```
iterate all from i = 0 to i < network_types.size()
if key is all_networks
this is the total
else
this is one of
...

💬 dergoegge commented on issue "Fuzz: compare our AES implementation to AES-NI ":
(https://github.com/bitcoin/bitcoin/issues/27548#issuecomment-3498557523)
Seems fine but also super niche and I'm not sure if maintaining a separate AES-NI impl or linking in an existing one is worth it (maintenance wise) for us (also considering the lack of interest here for the past 2+ years).

@brunoerg perhaps something to add to `bitcoinfuzz`? Otherwise if `cryptofuzz` gets revived, that might also be something (if it wasn't already in there).

📝 waketraindev opened a pull request: "Added test coverage for qt gui#901 console history filter"
(https://github.com/bitcoin-core/gui/pull/910)
Add test coverage for the QT rpc console newly added filtered commands in #901

💬 A-Manning commented on pull request "rest: Query predecessor headers using negative count param":
(https://github.com/bitcoin/bitcoin/pull/33752#issuecomment-3498636613)
@stickies-v Updated to handle the case where the requested block header is no longer in the active chain.

💬 kevkevinpal commented on pull request "test: skip interface_ipc if python version is freethreaded and PYTHON_GIL=0 is not set":
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3498642865)
I went with the suggestion to suppress the warning here [4ab4fd7](https://github.com/bitcoin/bitcoin/pull/33795/commits/4ab4fd76dd861d84715079aa9118519d14362894)

Not sure how verbose you want me to be with the `message` I can make it more exact to the message in the error if wanted

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3498693076)
ACK fa6c0bedd33ac7ad27454adaf9522fd27bef6ea3

I have recreated the change locally (see https://github.com/bitcoin/bitcoin/pull/33724#pullrequestreview-3393067389), my only objection was that I would like the methods to return the values in the same type as how we're storing them, otherwise it's confusing to know when it's okay to store them in 32 bits and when in 64 - but with the operations (e.g. multiplication) we could more easily tell if they're close to the threshold or not since they're
...

👋 waketraindev's pull request is ready for review: "Added test coverage for qt gui#901 console history filter"
(https://github.com/bitcoin-core/gui/pull/910)

👋 waketraindev's pull request is ready for review: "Comment out sensitive console commands in history to prevent re-execution"
(https://github.com/bitcoin-core/gui/pull/909)

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500222372)
I'm not sure I understand why, what's the difference between *two threads working while main is waiting* vs *one thread working and main also working*?
Where does the difference come from (especially given that the work is not fully cpu-bound)?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500300779)
> two threads working while main is waiting vs one thread working and main also working?

It would be two threads working while main is waiting vs two threads working and main also working? So the latter has 3 threads working vs 2 threads working. If using `-par=3` this is the case. 50% more work is done in parallel.

💬 w0xlt commented on pull request "[kernel] Expose `CheckTransaction` consensus validation function":
(https://github.com/bitcoin/bitcoin/pull/33796#issuecomment-3498877580)
> I am not sure how useful this check by itself is … what is the purpose served from surfacing the context‑free checks, but not the others?

The new API intentionally exposes only the context‑free consensus predicate (`consensus/tx_check::CheckTransaction`) so callers can **fail fast** on malformed transactions without needing a kernel context, UTXO set, or policy knobs.

This gives library users (indexers, gateways, alternative mempool layers, etc.) a **cheap pre‑filter** to catch structura
...

💬 maflcko commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2500379088)
(resolved this conversation, and most of the others, but I am happy to review follow-ups, if there are any)

🤔 maflcko requested changes to a pull request: "test: skip interface_ipc if python version is freethreaded and PYTHON_GIL=0 is not set"
(https://github.com/bitcoin/bitcoin/pull/33795#pullrequestreview-3429988977)
not sure about this

💬 maflcko commented on pull request "test: skip interface_ipc if python version is freethreaded and PYTHON_GIL=0 is not set":
(https://github.com/bitcoin/bitcoin/pull/33795#discussion_r2500400726)
not sure. This seems to go in the wrong direction. It disables a legit? warning without any reason why disabling it would be safe or is even desirable. See https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3386064377 and https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3495381887

The correct fix would be to just require the GIL

🤔 mzumsande reviewed a pull request: "net: option to disallow v1 connection on ipv4 and ipv6 peers"
(https://github.com/bitcoin/bitcoin/pull/30951#pullrequestreview-3429992454)
> deny v1 incoming (only allow v2 incoming) which is a problem if this is widely adopted because then old nodes that do not support v2 will have a hard time finding peers to connect to and will be subject to eclipse from an attacker who runs many v1-accepting nodes.

I still don't find the reason for tying it to `-listen=0` instead of also disallowing v1 inbounds compelling. Yes, if everyone disabled v1 incoming peers that could lead to v1 nodes being eclipsed. But with the suggested approach,
...

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500406322)
So why not just do the work that `par` defined and leave main asleep, wouldn't that be simpler while basically achieving exactly the same?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500437404)
No because we would have to insert all entries into temp cache at the end, instead of parallelizing that work as well. That was the previous implementation where we waited for every thread to be done and then inserted everything in series before exiting.
Now, the main thread does both. It inserts while others are fetching, but if it inserts fast enough where it is waiting for new entries it will also fetch entries.

💬 ryanofsky commented on pull request "test: skip interface_ipc if python version is freethreaded and PYTHON_GIL=0 is not set":
(https://github.com/bitcoin/bitcoin/pull/33795#discussion_r2500447299)
> The correct fix would be to just require the GIL

That's right, the text of the warning (from #33582) states "The global interpreter lock (GIL) has been enabled to load module 'capnp.lib.capnp'", so my understanding is when this test is run, the GIL is enabled and the test succeeds, except there is a spurious warning at the end which causes the test runner to fail.

So to me it seems like a good fix is disable the warning, since we know about it and are expecting it, and there not a good r
...