👍 Sjors approved a pull request: "refactor: remove unused param from legacy pubkey interface"
(https://github.com/bitcoin/bitcoin/pull/27274)
ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e

⚠️ osas90 opened an issue: "Import in Bitcoin Core or a different software? What are the steps to reproduce?"
(https://github.com/bitcoin/bitcoin/issues/27370)
I got a private key in bitcoin on my blockchain.com for one of my receiving addresses. When I went to import it, it imported another address but i already used it as receiving addresses now funds now available...

_Originally posted by @MarcoFalke in https://github.com/bitcoin/bitcoin/issues/17846#issuecomment-570241898_

✅ fanquake closed an issue: "Import in Bitcoin Core or a different software? What are the steps to reproduce?"
(https://github.com/bitcoin/bitcoin/issues/27370)

:lock: fanquake locked an issue: "Import in Bitcoin Core or a different software? What are the steps to reproduce?"
(https://github.com/bitcoin/bitcoin/issues/27370)

💬 ismaelsadeeq commented on pull request "test: refactor: dedup mempool_package_limits.py subtests via decorator":
(https://github.com/bitcoin/bitcoin/pull/27350#issuecomment-1490199581)
ACK e669833943bda13b2840a174dc8e45194187fc8e

💬 Sjors commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1490210669)
This is definitely nicer. When you doubleclick on the zip file it magically reveals the orange icon:

<img width="612" alt="Scherm­afbeelding 2023-03-30 om 14 15 54" src="https://user-images.githubusercontent.com/10217/228833150-647e3460-c174-43d0-b026-c9dbffb10b64.png">

<img width="845" alt="Scherm­afbeelding 2023-03-30 om 14 16 10" src="https://user-images.githubusercontent.com/10217/228833181-c82d0639-91a3-476f-a309-e9732577a657.png">

The only thing we need to remind the user of, is t
...

💬 Sjors commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1490218629)
I'd like to test the Guix build too. @achow101 can you sign it? Or is there a way to self-sign it?

💬 Sjors commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#discussion_r1153198925)
💿

💬 dergoegge commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153208873)
No reason, it should be in `TxRelay`, which also results in a small resource optimization because that filter now only exists for tx relay peers. Thanks!

🚀 fanquake merged a pull request: "refactor: Drop no longer used `CNetMsgMaker` instances"
(https://github.com/bitcoin/bitcoin/pull/27368)

👍 stickies-v approved a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261)
re-ACK 30efd6d36

I think my comments aren't too controversial to address but they only affect documentation and style/readability so I'm happy with the status quo and a follow-up, too. We've had quite a bit of back and forth already on this PR.

💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1140315012)
There's another such update needed for the `LookupNumeric` docstring which currently reads:

```
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
* for additional parameter descriptions.
```

💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153145784)
```suggestion
const CService ui_proxy{ui_proxy_netaddr.value_or(CNetAddr{}), ui->proxyPort->text().toUShort()};
```

💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153136412)
Largely unrelated, so just leaving this here for follow-up. I think this can be cleaned up quite a bit:

<details>
<summary>git diff</summary>

```diff
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 49c2a85e1..46d095633 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -311,17 +311,17 @@ static bool HTTPBindAddresses(struct evhttp* http)
}

// Bind addresses
- for (std::vector<std::pair<std::string, uint16_t> >::iterator i = endpoints.begin(); i
...

💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153170805)
This docstring is updated in ab4257be34209fd61efb409593dc7aa21dba0fb7, but there it still returns a `bool`. The signature is only updated to return `std::optional<CNetAddr>` in 30efd6d36e24c83f5c094e30f8cb25e66e913d73

Moreover, the function never returns `addresses`, it's only ever singular. I also don't like the phrasing of 'empty', so would prefer updating this to:
```suggestion
* @returns The resulting network address to which the specified host
* string resolved or std::null
...

💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153157545)
A bit more readable and avoids default initializing CServices:

```suggestion
services.reserve(addresses.size());
for (const auto& addr : addresses)
services.emplace_back(addr, port);
```

💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153142035)
The comment was on an older commit, so apologies if I'm missing something, but I'm not sure it's fully addressed? In `if (i->first.empty() || (addr.has_value() && addr->IsBindAny()))`, I think the first `i->first.empty()` is indeed still superfluous as theStack pointed out.

🚀 fanquake merged a pull request: "ci: use LLVM/clang-16 in native_fuzz (ASAN) job"
(https://github.com/bitcoin/bitcoin/pull/27363)

💬 hebasto commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#issuecomment-1490246908)
> The fact that the version of libevent was being pinned to the earlier version explains why testing didn't catch this.

To be precise, the vcpkg version is pinned now. Maybe actual pinning `libevent` version, similar to https://github.com/bitcoin/bitcoin/commit/20b6c871178f20661b849ad5677bd8ecae55cf19, could provide a better user experience of native building on Windows. Otherwise, with the current suggested approach, Windows users will be forced to update their vcpkg installations.

cc @si
...

💬 RandyMcMillan commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1490249383)
note:
a bundled README should be in README.txt format - we can't assume the user can open .md files by default.