✅ maflcko closed an issue: "Operation Sweep"
(https://github.com/bitcoin/bitcoin/issues/29132)

💬 maflcko commented on issue "Operation Sweep":
(https://github.com/bitcoin/bitcoin/issues/29132#issuecomment-1867410103)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.

General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.

Network-wide consensus and/or P2P changes first need to be discussed with the greater community, for example the `bitcoin-dev` maili
...

✅ maflcko closed a pull request: "test: autogenerate bash completion"
(https://github.com/bitcoin/bitcoin/pull/25243)

💬 naumenkogs commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1434855153)
Okay, so for this PR i'd just suggest clarifying the *newly added* comments like
``` // Attempt v2 connection if we support v2 - if our peer doesn't support it, we'll reconnect with v1.```
Right now they are not accurate, as if the banning thing (or some other disconnection reason) don't exist.

If you feel like it, might as well touch the existing comments as well (say around `ShouldReconnectV1`) :)

In future, we might communicate the disconnection reason (Ban, v1, eviction,
...

👋 maflcko's pull request is ready for review: "util: Faster std::byte (pre)vector (un)serialize"
(https://github.com/bitcoin/bitcoin/pull/29114)

💬 maflcko commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1867422517)
Rebased and taken out of draft after the dependency https://github.com/bitcoin/bitcoin/pull/29056 was merged

💬 maflcko commented on issue "failure in wallet_basic.py --descriptors":
(https://github.com/bitcoin/bitcoin/issues/27249#issuecomment-1867447005)
Reproduced locally: https://drahtbot.space/temp_scratch/wallet_basic_coins_2.tar.xz

💬 maflcko commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1867457231)
It may be good to check the upstream source code inside macOS for recently introduced issues.

💬 naumenkogs commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1434889375)
I've spent a ridiculous amount of time understanding what's going on here, and I'm still far from being 100% sure. Perhaps if you touch this comment, you can expand it even further? E.g., connecting to each of the conditions in `else if`

💬 naumenkogs commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1434899330)
8d8e0b796dabba601742a9585237be5892c5f05a

maybe expand `for every reachable network`

💬 naumenkogs commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1434903032)
dfd635bcc9b3f3615cabe115af302df33efba6a1

nit: The first sentence talks about `might`, while the second makes a reader think this is unconditionally a fallback (which is not true). I'd suggest aligning the two

💬 Sjors commented on pull request "rpc: add path to gethdkey":
(https://github.com/bitcoin/bitcoin/pull/22341#issuecomment-1867496111)
I plan to re-work this on top of #29130 with a similar interface to `createwalletdescriptor`. That is, if you have a regular wallet with normal descriptors, it will Just Work(tm). Otherwise you need to specify which master key to use.

Initially I'll make a separate PR. If #26728 is closed in favor of the new approach in #29130 then this PR can be closed too.

💬 maflcko commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1867496666)
cc @martinus

💬 maflcko commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1434850353)
```suggestion
peers_dat = self.nodes[i].chain_path / "peers.dat"
```

nit: You can use `/` to join paths

💬 fanquake commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#discussion_r1434931358)
Maybe we can shuffle this all around again, the next time we bump Qt (new LTS has just been released), and have to inevitably rebase/update all these patches?

💬 fanquake commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1867514479)
> EDIT: I guess this is a cache issue, just weird that both my machines ran into this. Will rebuild.

Thankfully the likelyhood if this occuring will be largely reduced after #28880.

🚀 fanquake merged a pull request: "build: switch to using LLVM 17.x for macOS builds"
(https://github.com/bitcoin/bitcoin/pull/28880)

💬 Sjors commented on pull request "refactor: share and use `GenerateRandomKey` helper":
(https://github.com/bitcoin/bitcoin/pull/28455#discussion_r1434949134)
Indeed directly including `key.h` makes sense.

💬 Sjors commented on pull request "refactor: share and use `GenerateRandomKey` helper":
(https://github.com/bitcoin/bitcoin/pull/28455#issuecomment-1867538485)
ACK 81187d04c2925bbd1281fe3cbdc2a30c2a02cca5

The Stratum v2 Template Provider in #28983 also uses the `CKey static_key; static_key.MakeNewKey(true);` pattern, so would (slightly) benefit from this change.

> do we plan on making this modification to these files aswell?

I can re-review if more tests are modified to use this new helper, but I'm also fine with leaving as is. New code can use it.

💬 martinus commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#issuecomment-1867538883)
I actually found a way to get the compiler to produce a warning for these cases, but it is a bit tricky and has limitations: https://godbolt.org/z/GfzseM4q3

How it works:
* Add a static counter variable to `FuzzedDataProvider
* Add a default argument that uses `++` for the counter

That way a `-Wsequence-point` warning is triggered. This is also triggered for initializer lists, and gcc man page has some arguments why that is so.