💬 lcharles123 commented on issue ""netinfo" doesn't show IPv6 "Local addresses"":
(https://github.com/bitcoin/bitcoin/issues/30165#issuecomment-2130694823)
> Edit: probably not the issue, but did you try using the -discover config option?

Yes, but the addresses are unreachable anyway in local network, because the node are IPv4 only (and behind NAT).

I guess I found the cause, because function `AddLocal` checks for connectivity on each -externalip entry using local interfaces, for IPv4 and others, its easy because it takes the normal way to the address. But there is no solution to reach v6 addresses on a machine v4 only without some config and
...

💬 maflcko commented on pull request "fuzz: Fix wallet_bdb_parser stdlib error matching":
(https://github.com/bitcoin/bitcoin/pull/30169#issuecomment-2131027363)
> Was this caught by review or did you run into it? Just curious if there are others.

It can be tested by reverting this commit and running the OSS-Fuzz config (uses libc++), or the `ci_native_fuzz_msan` CI config (also uses libc++), or any other way that uses this fuzz target with libc++.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2131092539)
> The first two commits of https://github.com/bitcoin/bitcoin/pull/26812 would make it possible to test and fuzz how this code interacts with a router.

So ive been thinking about this, do we have a mockable replacement for `Sock socket(int domain, int type, int protocol)`? i was thinking of passing in a `SocketFactory` (please don't kill me, this could be just a functor 😅 ). It's slightly nicer than passing in a pre-made `Sock` because it allows testing the "create a socket of the right fami
...

📝 maflcko opened a pull request: "test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure"
(https://github.com/bitcoin/bitcoin/pull/30174)
Otherwise, the test may fail on slow hardware when running in valgrind.

Also, use named args for the absolute timepoint, while touching this file.

💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2131136559)
Addressed @vasild comments (thanks!) and rebased.

💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1614481665)
I decided against this. You don't strictly need to be root (https://hackmd.io/@willcl-ark/r19LVO2_6 or https://github.com/bitcoin/bitcoin/pull/24358#issuecomment-1083149220). Checking for getuid == 0 would require you to run as root.

💬 0xB10C commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2131226810)
> Concept NACK, consumers should just check if features are available

The [current recommendation](https://github.com/bitcoin/bitcoin/blob/42d5a1ff25a8045b6f4c09fa1fb71736dbccc034/doc/JSON-RPC-interface.md?plain=1#L69-L70) is already to use the `version` field from the `getnetworkinfo` RPC call. This PR doesn't really add anything new that wasn't already exposed and recommend for RPC clients to use.

---

Throwing this in for even more confusion about this new RPC and the software vs RPC
...

💬 king-11 commented on issue ""Rolling forward" at startup can take a long time, and is not interruptible":
(https://github.com/bitcoin/bitcoin/issues/11600#issuecomment-2131241170)
I have been encountering this myself the `bitcoind` runs inside of a docker container and when its asked to stop and doesn't respond to `SIGTERM` the docker after a timeout uses `SIGKILL` resulting in exit code of `137` being unclear stop. Can we attach listener for all the signals somewhere?

💬 epiccurious commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1614633882)
nit: this sentence might be better structured starting with a verb:

```
Implement associated `-deprecatedrpc=` option to retain previous RPC behavior when modifying RPC interface JSON structure, including (but not limited to) the following:
```

💬 epiccurious commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1614634416)
nit: this sentence might be better structured starting with a verb:

```
Prefer to choose RPC JSON data types that are flexible to expansion without change of data type.
```

💬 epiccurious commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#issuecomment-2131241674)
ACK 61853a25bbbdb6d5dfb2f1570e41f541a4b7de78.

⚠️ sipa opened an issue: "Enable `importprivkey`, `addmultisigaddress` in descriptor wallets"
(https://github.com/bitcoin/bitcoin/issues/30175)
### Please describe the feature you'd like to see added.

Descriptor wallets currently do not support the "legacy" import commands `importprivkey`, `importpubkey`, `importaddress`, `addmultisigaddress`, `importmulti`, and `importwallet`. This was done because the semantics of these RPC cannot be replicated in descriptor wallets, as they just fundamentally work differently (being explicit about what to watch rather than "whatever matches the bag of keys and scripts we have").

I want to suggest
...

✅ kevkevinpal closed a pull request: "test: Added test to ensure log and failure happen when work is less than active chainstate"
(https://github.com/bitcoin/bitcoin/pull/30105)

💬 kevkevinpal commented on pull request "test: Added test to ensure log and failure happen when work is less than active chainstate":
(https://github.com/bitcoin/bitcoin/pull/30105#issuecomment-2131273947)
> Just realised this is similar to this PR: #29428

Oh nice I didnt see this, closing this now

💬 kevkevinpal commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2131274346)
utACK [df6dc2a](https://github.com/bitcoin/bitcoin/pull/29428/commits/df6dc2aaaeffc664006b86ee8c8797dc484ec40e)

💬 kevkevinpal commented on pull request "bench: enable wallet creation benchmarks on all platforms":
(https://github.com/bitcoin/bitcoin/pull/30122#issuecomment-2131286969)
utACK [7c8abf3](https://github.com/bitcoin/bitcoin/pull/30122/commits/7c8abf3c2001152423da06d25f9f4906611685ea)

🤔 tdb3 reviewed a pull request: "doc: update mention of generating bitcoin.conf"
(https://github.com/bitcoin/bitcoin/pull/30154#pullrequestreview-2079033825)
ACK for 9013e2b97e8f50d2be63ce740c42d0b0e0b9b7f2
Good find. Makes sense to point the user to conf file generation (helping to ensure the conf reflects the implementation). Followed the link (seemed to work fine).

💬 hebasto commented on issue "make cov fails with lcov-2":
(https://github.com/bitcoin/bitcoin/issues/28468#issuecomment-2131301980)
FWIW, the https://github.com/hebasto/bitcoin/pull/191 supports both LCOV versions for CMake.

💬 furszy commented on issue "Enable `importprivkey`, `addmultisigaddress` in descriptor wallets":
(https://github.com/bitcoin/bitcoin/issues/30175#issuecomment-2131302391)
I think `importaddress` and `addmultisigaddress` intents are clear enough and they map well to descriptors. Both specify the address type. Forcing users to learn how to craft a descriptor when they merely want to watch an address seems overwhelming to me. -> #27034 maps `importaddress` to `addr()` and `raw()` descriptors. And #28307 fixes and extends `createmultisig`/`addmultisigaddress`.

Not sure about `importprivkey` as it would clash with the new `void(KEY)` descriptor (#29136). The intent
...

👍 tdb3 approved a pull request: "cli: restrict multiple exclusive argument usage in bitcoin-cli"
(https://github.com/bitcoin/bitcoin/pull/30148#pullrequestreview-2079066296)
ACK for 628d2d4173660c1ad35b2d84524f71b92593d7cd

Thanks for picking this up. Built and ran all functional tests (all passed). Manually executed `bitcoin-cli` with one and multiple instances of the argument group covered by this PR. The error was seen (as expected) if more than one argument of the group was specified.

Left a few notes.