💬 dergoegge commented on pull request "fuzz: avoid returning non-conforming results from FuzzedSock::GetSockName()":
(https://github.com/bitcoin/bitcoin/pull/32109#issuecomment-3498443216)
Also leaning towards concept NACK on this

1. It does not seem like fuzzers struggle to produce canonical `getsockname` results using our current approach
2. Even if super unlikely, this would catch incorrect handling of `getsockname` results (in case of OS bug) on our side, which seems like a nice thing if it's basically free (given 1.)

👍 stickies-v approved a pull request: "util: Allow `Assert` (et al.) in contexts without __func__"
(https://github.com/bitcoin/bitcoin/pull/33785#pullrequestreview-3429500574)
ACK fad6efd3bef1d123f806d492f019e29530b03a5e

💬 stickies-v commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2500004322)
There are 2 places in `validation.cpp` where `STR_INTERNAL_BUG` could be used too, but I suppose you didn't because it (afaik) requires us to abandon the compile time format string checking?

📝 waketraindev converted_to_draft a pull request: "Comment out sensitive console commands in history to prevent re-execution"
(https://github.com/bitcoin-core/gui/pull/909)
Prefix filtered commands with "#" before adding them to the RPC console history.

Console lines starting with "#" are ignored by the executor, preventing accidental re-execution of filtered sensitive commands when recalled from history.

Also adds a noop signal to handle ignored commands cleanly and documents comment behavior in the console help text (`help-console`).

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500031838)
This is true when all inputs have been fetched from the block.

💬 Crypt-iQ commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3498504204)
crACK fa6c0bedd33ac7ad27454adaf9522fd27bef6ea3

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500047237)
The work done on the other threads is much slower than what the main thread is doing. Fetching inputs possibly from disk vs inserting coins into the temporary cache. So, in cases where there are fewer worker threads the main thread will likely be waiting on the work to be done. In these cases we can just start fetching as well. I think this would have a large impact in cases where there are few worker threads (like rpis), or if using a low but > 1 -par value. For instance, using `-par=2` this wo
...

🤔 vasild reviewed a pull request: "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency"
(https://github.com/bitcoin/bitcoin/pull/26988#pullrequestreview-3429510882)
Reviewed as of 016ab85a13

💬 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_r2500012759)
nit:
```suggestion
UniValue ProcessReply(const UniValue& batch_in) override
```

💬 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)?