💬 brunoerg commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1555757479)
Also, maybe we do not need this verification anymore?
```py
if res["success"]:
self.log.info(f"Added {addr} to tx_originator's addrman")
else:
self.log.info(f"Could not add {addr} to tx_originator's addrman (collision?)")
```

💬 cbergqvist commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2042641779)
Happy for my first merged PR! :)

## Post-merge post-mortem

### Working stupidly, not smart or hard

I have spent too many evenings focusing on this:
> Agree that it might be fruitful to investigate the history of wallet_importdescriptors.py to pinpoint regressions, didn't have time today.

In over >110 runs, I have observed how the chance of `wallet_importdescriptors.py --descriptors` failure has increased with time. As patterns emerged around certain commits, I've tested further only
...

💬 0xB10C commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2042651718)
I just learned that nanobench is able to fill in an [output format template](https://nanobench.ankerl.com/tutorial.html#rendering-mustache-like-templates). It might make sense to try that route first.

👍 theuni approved a pull request: "depends: add new LLVM debug macro"
(https://github.com/bitcoin/bitcoin/pull/29781#pullrequestreview-1986389340)
ACK 5efebc0edbb479d2041b3fb2d43be3a77e817b3e

💬 laanwj commented on pull request "doc: 25.2 historical release notes":
(https://github.com/bitcoin/bitcoin/pull/29830#issuecomment-2042709408)
ACK 93bd2e2f6c9672fbf1120d1e25349f6edd29cfef
Matches the file on tag `v25.2`.

💬 cbergqvist commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-2042723373)
ACK 2842e51a246b162a586941184b7694f187d7aee7

Diffed top 2 commits in that and 78482a09e06beb841e70781eb509c2b2fdea8bd9 which I previously tested & acked. Only scope of `start`-variable was narrowed as suggested by @davidgumberg in https://github.com/bitcoin/bitcoin/pull/28016#discussion_r1526788396.

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555830543)
Made it shorter :)

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555831622)
bit of an annoying circular reference issue(blocktools requiring utils) so for now at least make it grep-able what the magic value should be(and more greppable)

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832375)
done

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832564)
done, I think

💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832735)
done

💬 laanwj commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2042741859)
For some reason, after building with this, I dont get any output from:
```
$ gdb src/bitcoind
...
(gdb) info tracepoints
No tracepoints.
```
Tested with gdb `15.0.50.20240219-git` and `13.1`.

I checked that the tracepoints are included:
```
$ readelf -n ./src/bitcoind | grep NT_STAPSDT -A 4 -B 2
stapsdt 0x00000062 NT_STAPSDT (SystemTap probe descriptors)
Provider: net
Name: outbound_message
Location: 0x00000000001809bc, Base: 0x0000000000a2cb60, S
...

💬 stickies-v commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2042758379)
ACK 78407b99ed6dd17f687fcbfb0486ecc433302287

👍 ismaelsadeeq approved a pull request: "doc: 25.2 historical release notes"
(https://github.com/bitcoin/bitcoin/pull/29830#pullrequestreview-1986526306)
ACK 93bd2e2f6c9672fbf1120d1e25349f6edd29cfef

💬 laanwj commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2042827943)
Oh, it's `info probes` not `info tracepoints`. Whoops. It checks out now.

Code review and lightly tested ACK 3933bdd6cfc9accab9e0ed47e83a5cc27ada6b68

💬 laanwj commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1555936795)
It's slightly inconsistent to pass this parameter as int, while the other enum (Connection Type) gets passed as string. Might similarly pass `GetNetworkName(...).c_str()`. Especially after #26593 which would remove the call overhead in the happy path.
No strong opinion though, I think the integer value is easier to use if you're going to make a histogram or such.

💬 sr-gi commented on pull request "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type":
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2042913328)
> Concept ACK, nit: Could you also take a look at `announce_tx_and_wait_for_getdata` in `p2p_segwit.py` for a similar cleanup?

Covered that in [067b009](https://github.com/bitcoin/bitcoin/pull/29748/commits/067b009bbf47f7bc5adeb6b500042f7c44bfb03f) by deleting the data on checks (pop instead of get) plus allowing to check the inv type.

For the latter, I've done it in a way that all items need to be of the same type. We could address this so every single inv has its own type, but for that w
...

💬 laanwj commented on pull request "Drop Windows Socket dependency for `randomenv.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29786#issuecomment-2042945377)
Looks correct to me "If the function succeeds, the return value is a nonzero value.".

I'm not sure the netbios name is always the name as the hostname, but it is a good enough equivalent for randomness purposes, and I agree it is preferable not to depend on winsock for the kernel. The API, introduced in Windows 2000 is also old enough.

Code review ACK 03b87a3e64305ba651e22a730e35271dea8fea64.

💬 epompeii commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2042950381)
> For Bitcoin Core, it would be useful to have an adapter for the nanobench JSON output. To track this, I've opened https://github.com/bencherdev/bencher/issues/361.

@0xB10C I would be more than happy to implement a nanobench JSON output adapter. It is going to take me a couple of weeks or so to get to it though. So you could either:

1. Use the nanobench output format template to [Bencher Metric Format](https://bencher.dev/docs/explanation/adapters/#-json)
2. Implement the adapter in Benc
...

🚀 fanquake merged a pull request: "doc: 25.2 historical release notes"
(https://github.com/bitcoin/bitcoin/pull/29830)

💬 laanwj commented on pull request "net: update comment for service bit support info for seed.bitcoin.sipa.be":
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2042981215)
> Perhaps this tool can be modified to scan which filters are in use: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py

This would be nice, and fairly easy if we can assume disallowed filters return nothing/an error, but seems like there's a exponential number of potential combinations to test? Or is something smarter than brute force possible?