💬 tdb3 commented on issue "ci: macOS 14 CI failure `Invalid value detected for '-wallet' or '-nowallet'`":
(https://github.com/bitcoin/bitcoin/issues/31019#issuecomment-2389081000)
At the time of the failure, the PR commits were on top of commit a74bdeea1b8e27b2335f0f7da78006e87ecfb235 (Sept 2nd).
I'm probably missing something, but it almost seemed like the CI job was executing code that was introduced in commit ee47ca29d6ef55650a0af63bca817c5d494f31ef (dated Aug 22nd, from PR #30684, but merged with commit fb52023ee69c346dd101770716b6d8c0525c38aa on Sept 9th).

💬 Sjors commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2389082792)
I also get the segfault when rebuilding (depends and bitcoin-qt) with 354f5d79270c0784a6ba59d61191ee7eaaa2a53d.

```
% lldb build-depends/src/qt/bitcoin-qt
(lldb) target create "build-depends/src/qt/bitcoin-qt"
Current executable set to '/Users/sjors/dev/bitcoin/build-depends/src/qt/bitcoin-qt' (x86_64).
(lldb) run
Process 7195 launched: '/Users/sjors/dev/bitcoin/build-depends/src/qt/bitcoin-qt' (x86_64)
2024-10-02 18:18:32.237316+0200 bitcoin-qt[7195:247862] SecTaskLoadEntitlements fai
...

💬 jonatack commented on issue "rfc: DATUM mining interface requirements":
(https://github.com/bitcoin/bitcoin/issues/31002#issuecomment-2389106217)
> So far it's proprietary and undocumented, but hopefully that changes.

Agree. I believe it may be MIT-licensed and public soon.

🤔 glozow reviewed a pull request: "rpc: add getorphantxs"
(https://github.com/bitcoin/bitcoin/pull/30793#pullrequestreview-2343446923)
ACK b6368fc285b. I have some picky review comments if you're interested, but I don't feel that strongly on anything except the default argument one.

I tested with @0xB10C's visualizer as well, very cool!
<img width="833" alt="image" src="https://github.com/user-attachments/assets/c1e81c89-3d4d-4a5a-a092-51d633ea10db">

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1784873140)
It is a bit funky that a negative verbosity turns into 0! I was kind of expecting a `JSONRPCError`. I don't mind and it looks like it's the same for `getblock`, so not asking for a change, but could be worth a test case?

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1784854966)
nit: comment is unnecessary. Also my personal preference is to not put them on the same line

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1784861016)
an annotation can help here
```suggestion
int verbosity{ParseVerbosity(request.params[1], /*default_verbosity=*/1)};
```

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1784859575)
I find default arguments to be pretty footgunny and prefer to require explicit. Since you're introducing this function for the first time, I suggest no default arg.

```suggestion
int ParseVerbosity(const UniValue& arg, int default_verbosity);
```

🤔 instagibbs reviewed a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2343039535)
some last nits/suggestions

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1784580197)
not strictly a superset, right? Uses `NON_GOOD_RESPONSES` if not good which excludes `TX_REAL`

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1784885638)
I'm not sure what this comment means. I know the `honestid` peer needs to be wtxid relaying to ensure that it can properly serve `malleated_tx` when necessary, otherwise `GetCorrectTx` will fetch the wrong response by giving back `correct_tx`.

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1784688753)
This should catch issues marking things as received properly, at least for wtxid version since we're forcing wtxid relay for good peer.
```Suggestion
// Honest peer only tells us about INV_REAL in this scenario, should be empty since REAL_TX received
if (peer.m_nodeid == honestid) {
txdownloadman.CheckIsEmpty(honestid);
}
txdownloadman.DisconnectedPeer(peer.m_nodeid);
```

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1784582239)
Could use `PickCoins` here as well

⚠️ Khalilheyrani6367 opened an issue: "Bitcoin 💵"
(https://github.com/bitcoin/bitcoin/issues/31020)
Bitcoin (BTC) Address: bc1qumf5ap6g85qanrrjhutqrkrvl3phch4fn66frn

💬 Khalilheyrani6367 commented on issue "Bitcoin 💵":
(https://github.com/bitcoin/bitcoin/issues/31020#issuecomment-2389135162)
Deposit bitcoin wallet exodus

✅ pinheadmz closed an issue: "Bitcoin 💵"
(https://github.com/bitcoin/bitcoin/issues/31020)

🤔 vasild reviewed a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2337397321)
Approach ACK 5b59cfc944f9eac73e1e69bcc660280b6809724a

💬 vasild commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1781040991)
That `68` probably comes from here:

https://github.com/bitcoin/bitcoin/blob/a0473442d1c22043f5a288bd9255c006fd85d947/test/functional/interface_usdt_net.py#L22-L23

but I just did this:

Added to `/etc/hosts`:
```
24.116.163.227 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
```
(that is 70 `a`s)

Then `addnode aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:8333` and `CNode::m_addr_name` gets assigned that 70-chars long string.

Less
...

💬 vasild commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1784658950)
For outbound connections, the peer does not "connect from".

```suggestion
4. Network of the peer as `uint32` (1 = IPv4, 2 = IPv6, 3 = Onion, 4 = I2P, 5 = CJDNS). See `Network` enum in `netaddress.h`.
```

or "Network to which the peer's address belongs".

💬 vasild commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1784806169)
nit: currently we evict only inbounds, but this may change in the future. Maybe drop the "inbound" from here so that this does not get outdated:

```suggestion
Is called when a connection is evicted by us. Passes information about the evicted peer and the time at connection establishment.
```