💬 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

💬 sipa commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#issuecomment-2559907064)
utACK fa494a1d53f3f030fafe7b533d72b2200428a0fd

💬 sipa commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2559915449)
Concept & approach ACK.

💬 sipa commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#issuecomment-2559989402)
utACK 69e95c2b4f99eb4c2af6b2b6cc6a66abfea753df.

I can't imagine any way that this could affect operations later. The key isn't used, so deleting it can't hurt backups, and with or without encryption key no ckey records can be added later. Is that correct?

💬 sipa commented on pull request "coins: warn on shutdown for big UTXO set flushes":
(https://github.com/bitcoin/bitcoin/pull/31534#issuecomment-2559992151)
utACK 5709718b830161b7c2ba0db545ef0cfa98423597

🤔 sipa reviewed a pull request: "test: Add mockable steady clock, tests for PCP and NATPMP implementations"
(https://github.com/bitcoin/bitcoin/pull/31022#pullrequestreview-2520898727)
Neat, utACK 258b2856cdc6e2c054ea573cf57de7021aebc3c5. I did not verify the test scenario byte sequences against the spec.

💬 sipa commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1895944792)
Perhaps document that this represents the expected error.

💬 sipa commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1895945169)
Could be marked `final`.