💬 theStack commented on issue "RFC: support for writing UTXO set dump (`dumptxoutset` RPC) to a named pipe":
(https://github.com/bitcoin/bitcoin/issues/31373#issuecomment-2558601261)
> i'll review/test your PR when ready.
>
> i think 1 ,3 and 4 could/should be optional as it also can be handled by the process reading the pipe

Yes, I agree that named pipe creation (1) / deletion (4) and the call of the tool (3) shouldn't be done by the RPC itself. My plan was to base the change on #27432 and then provide an additional mode where the user has to provide the path to the bitcoin-cli binary rather than an input file, which would roughly do the following:
```
fifoname=/tmp
...

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2558616951)
@alfonsoromanz While https://github.com/bitcoin/bitcoin/pull/30909 is stalling a bit it seems like it is the way we'll move forward when there is some more review since there was no push back on concept/approach. So I would suggest you proceed here assuming that #30909 is acceptable. Ideally you would change this PR to have the second test standalone, if that's not possible you can base it on #30909.

💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2558635023)
Thank you for the suggestions @hodlinator, I appreciate the time you took for this. I was originally intent on tackling the 32bit case in a follow-up, mainly because I was thinking whether we should trap the max amount of cache at `numeric_limits<size_t>::max()`. That might be a bit friendlier than just running into an assert, some kind of allocation failure, or much less cache being allocated than requested. With your suggestions, I agree that this should be handled in this PR now.

🤔 PlatinumWrist5055 reviewed a pull request: "[28.x] 28.1rc2 backports"
(https://github.com/bitcoin/bitcoin/pull/31469#pullrequestreview-2519727585)
https://github.com/bitcoin/bitcoin/issues/31418

💬 PlatinumWrist5055 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/0725a374941355349bb4bc8a79dad1affb27d3b9#commitcomment-150656825)
0725a374941355349bb4bc8a79dad1affb27d3b9

💬 PlatinumWrist5055 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/5be34bacf6d07fb73a0cedfb63a384968d538f3e#commitcomment-150657416)
src/qt/rpcconsole.cpp

💬 pythcoiner commented on issue "RFC: support for writing UTXO set dump (`dumptxoutset` RPC) to a named pipe":
(https://github.com/bitcoin/bitcoin/issues/31373#issuecomment-2558813248)
i'll take a look at 27432 then

💬 Msirma4life commented on something "":
(https://github.com/bitcoin/bitcoin/commit/ce6df7df9bab2405cfe7d6e382f5682cf30de476#r150659924)
[$motivation4eva](url)

👍 zaidmstrr approved a pull request: "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo"
(https://github.com/bitcoin/bitcoin/pull/31531#pullrequestreview-2520063454)
Tested ACK [ecaa786](https://github.com/bitcoin/bitcoin/pull/31531/commits/ecaa786cc103cf7cc63ae899ec13d81a54e2fd1e)
I tested the changes on the default signet and the result provided by both `getblockchaininfo` and `getmininginfo` RPC includes `signet_challenge` field. I also manually reviewed the new code for any errors and missing statements.
The updated test `test/functional/feature_signet.py ` also works fine.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1895605317)
Yeah, I was thinking the same... Should I change it to the following?

```diff
- if [ ! -e "$f" ] && [ "$FILE_ENV" != "./ci/test/00_setup_env_native_asan.sh" ] ; then
+ if [ ! -e "$f" ] && [ "$CONTAINER_NAME" != "ci_native_asan" ] ; then
```

👍 vasild approved a pull request: "doc: Install `net/py-pyzmq` port on FreeBSD for `interface_zmq.py`"
(https://github.com/bitcoin/bitcoin/pull/31526#pullrequestreview-2520424440)
ACK 0a76c292ac8fa29166a7ec6efda1fafce86af0d3

Without `net/py-pyzmq`:

```
$ .../interface_zmq.py
...
2024-12-23T11:21:49.815000Z TestFramework (WARNING): Test Skipped: python3-zmq module not available.
```

After `pkg install net/py-pyzmq` the test runs and passes, thanks!

📝 brunoerg opened a pull request: "descriptor: remove unreachable verification for `pkh`"
(https://github.com/bitcoin/bitcoin/pull/31555)
This PR removes an unreachable verification in the `ParseScript` function. It returns an error if `pkh` is not being used at top level, sh, wsh or tr. However, any usage of `pkh` without these contexts will not reach this verification but other ones like "invalid keys" (e.g. `wpkh(pkh(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd))`).

💬 rfarzad601 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/ecaa786cc103cf7cc63ae899ec13d81a54e2fd1e#r150671389)
rfarhad601@gmail.com

💬 marcofleon commented on pull request "fuzz: Limit wallet_notifications iterations (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31467#discussion_r1895747732)
Keeping the two `FundTx` calls doesn't make the target significantly slower for me (`-runs=1` takes 8 sec vs 6 sec without them). But I agree that if they are removed then the function should be removed from `FuzzedWallet`.

💬 0xB10C commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1895767330)
Yet another alternative would be to set a flag like `CI_FAIL_IF_NO_TCPDUMP_FILE` (or similar) in 00_setup_env_native_asan.sh and add a comment mentioning the dependency:
- if the script name or the container name are renamed, this will still work
- if script is removed, someone might see the comment about the dependency during review
- it's easy to add more tasks to the list of required tasks by just adding `CI_FAIL_IF_NO_TCPDUMP="true"`

💬 sipa commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2559801865)
Concept ACK

💬 sipa commented on pull request "fuzz: Limit wallet_notifications iterations (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31467#discussion_r1895857303)
I don't see too much time difference in a corpus made using both with/without this `FundTx` call, so it seems better to keep the coverage it could provide.

If knapsack is causing slowness, that seems mostly an argument to improve it, reduce its iteration counts, or get rid of it. Paging @murchandamus .

💬 sipa commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#issuecomment-2559886700)
utACK ecaa786cc103cf7cc63ae899ec13d81a54e2fd1e

💬 sipa commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#issuecomment-2559898587)
ACK e36640859089baabc46f68217843f96a3ebdc20c

Took me a while to understand why, but it's just because `ParseScript` itself cannot be reached with `ctx == ParseScriptContext::WPKH`. Perhaps an assert or Assume could be added for that general property?

🤔 scgbckbone reviewed a pull request: "wallet: allow lable for external descriptor & disallow label for ranged descriptors"
(https://github.com/bitcoin/bitcoin/pull/31514#pullrequestreview-2520783072)
@achow101 - looks like logic bugs to me