💬 brunoerg commented on pull request "test: add support for all networks in `deserialize_v2`":
(https://github.com/bitcoin/bitcoin/pull/27295#discussion_r1165900736)
You're correct, seems better your approach.

💬 brunoerg commented on pull request "test: add support for all networks in `deserialize_v2`":
(https://github.com/bitcoin/bitcoin/pull/27295#issuecomment-1507449831)
Force-pushed changing the approach for torv3 addresses, added @pinheadmz as co-author.

💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#discussion_r1165903453)
good catch, removed NET_UNKNOWN.

💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#discussion_r1165903653)
fixed

💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#discussion_r1165903972)
done

💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#discussion_r1165904167)
fixed

💬 jonatack commented on pull request "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns":
(https://github.com/bitcoin/bitcoin/pull/27374#issuecomment-1507486375)
Concept ACK, will review.

💬 kevkevinpal commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#issuecomment-1507496302)
Concept ACK 846ff37fe7ac2a5b4f855faaadb3434158ed6364 (need to check latest commit)

### tested by doing the following in regtest

`bitcoin-cli createwallet "testwallet"`
`bitcoin-cli getnewaddress`
`bitcoin-cli generateto address 6 <previous commands output>`
`bitcoin-cli generate 20 (did this a couple times)`
`bitcoin-cli getbalance` (validate you have a balance)
`bitcoin-cli getnewaddress`
`bitcoin-cli send '{"<above output>": 0.1}' null "unset" null '{"fee_rate": 1}'` (did this a fe
...

💬 furszy commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1165959935)
In f998cdbc:

Would be good to move this `NotifyTransactionChanged` to `RecursiveUpdateTxState` too. So `MarkConflicted` and the `BlockDisconnected` txes state changes are notified to the upper layers.

💬 furszy commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1165954948)
In f998cdbc:
This lock isn't needed. All callers have `cs_wallet` acquired. Could add the lock required annotation instead.

🤔 furszy reviewed a pull request: "refactor: Remove CAddressBookData::destdata"
(https://github.com/bitcoin/bitcoin/pull/27224#pullrequestreview-1384148699)
Code ACK a5986e82

💬 furszy commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1165987641)
Would be good to check that the map contains the element prior accessing it. Otherwise the GUI could be adding a new address book entry by mistake.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1507577994)
Thanks so much for the great review @vasild I think I addressed all your feedback up to the adding UNIX to CNetAddr. I understand your concerns there and we can chat about it, I'll look at alternatives as well. What I did do in this last push was mostly separate unix sockets from regular sockets, and adding `Proxy.GetSocket()` to decide which socket factory to call.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1166002075)
I'm a bit intimidated by the syntax `prevector<ADDR_IPV6_SIZE, uint8_t> m_addr{ADDR_IPV6_SIZE, 0x0};` -- is that only 16 bytes?

📝 theuni opened a pull request: "verify-commits: error and exit cleanly when git is too old."
(https://github.com/bitcoin/bitcoin/pull/27461)
Requested by fanquake. Rather than failing with a cryptic error with older git, fail gracefully and mention why.

The new option semantics [are explained here](https://github.com/git/git/commit/1f0c3a29da3515d88537902cd267cc726020eea5).

Note: my local git versions are currently too old to test the new functionality, so I've only verified the failure case.

💬 achow101 commented on pull request "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns":
(https://github.com/bitcoin/bitcoin/pull/27374#issuecomment-1507674903)
ACK b5585ba5f97a19d1b435d9ab69b5a55cfd45dd70

🚀 achow101 merged a pull request: "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns"
(https://github.com/bitcoin/bitcoin/pull/27374)

🤔 jonatack reviewed a pull request: "Add wallet method to detect if a key is "active""
(https://github.com/bitcoin/bitcoin/pull/27216#pullrequestreview-1384012329)
ACK with some suggestions below.

The release note and added test coverage could optionally all be in separate commits, provided each commit is hygienic, i.e. builds and all tests pass. It's a bit easier for reviewers if updates needed for tests to pass with a change are in the same commit, while additional coverage is in separate commits.

💬 jonatack commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1165903530)
```suggestion
address is derived from an active descriptor or HD seed. (#27216)
```

💬 jonatack commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1165902595)
Per `doc/release-notes-empty-template.md` would suggest

```
Updated RPCs
------------
```