💬 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.
```

💬 vasild commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1784640440)
I couldn't find the doc of `bpf_usdt_readarg_p()`. What would it do if the argument is longer than the supplied buffer? I presume the last arg is the length of the buffer, so it is not going to overflow, I guess. But would it add a trailing `'\0'`? That is - could it copy `MAX_PEER_ADDR_LENGTH` from the input to the output and leave it like that? If yes, would that be a problem - a string without a terminating `'\0'`?