💬 fjahr commented on pull request "ci: Add mising -Wno-error=maybe-uninitialized to armhf task":
(https://github.com/bitcoin/bitcoin/pull/30144#issuecomment-2122157154)
utACK fa73431dd4709754c34a4d5ad1c940ff9e628cf3

💬 fjahr commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2122161034)
re-ACK ef692e52b4121ba3d8de91e45b2d33aff946e8d2

Also reviewed https://github.com/bitcoin/bitcoin/pull/30144 to confirm the failure is ok.

💬 fanquake commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1607953823)
I'm going to do this in #30137.

🚀 fanquake merged a pull request: "util: avoid using thread_local variable that has a destructor"
(https://github.com/bitcoin/bitcoin/pull/30095)

🚀 fanquake merged a pull request: "doc: Update NetBSD Build Guide"
(https://github.com/bitcoin/bitcoin/pull/30143)

💬 maflcko commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2122188355)
I wonder if it makes sense to collect the fuzz inputs to qa-assets, from testers that still have them. However, the fuzz CI does not compile with BDB, so that'll probably need to be adjusted first.

🤔 virtu reviewed a pull request: "net: add ASMap info in `getrawaddrman` RPC"
(https://github.com/bitcoin/bitcoin/pull/30062#pullrequestreview-2068021191)
Concept ACK

Also did some light testing:
- Some sanity checks on `getrawaddrman` output (everything mapped to zero when asmap is disabled; almost all addresses mapped to non-zero when enabled)
- Functional tests

💬 virtu commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1607941908)
nit: Looking at my node's `getrawaddrman` output, it felt odd to have `mapped_as` appear after `source`. IMO, the fields become more intuitively understandable when put right after or at least close to the address they refer to.

💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2122215763)
note that https://github.com/bitcoin/bitcoin/pull/29575 removes the misbehavior score (which is exposed via the `net:misbehaving_connection` tracepoint). If/when #29575 is merged, the `misbehaving_connection` tracepoint will change.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1608069282)
`proxy_opt` exists only inside the `if`. What matters after this `if` are the values of `use_proxy` and `proxy`. I think your suggestion is equivalent to the current code. I.e.

```
set use_proxy and proxy (normal)
if (special case)
replace use_proxy and proxy with special values
```
vs
```
if (special case)
set use_proxy and proxy to special values
else
set use_proxy and proxy (normal)
```

Right?

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1608108288)
The transient sessions are not reused. `m_unused_i2p_sessions` description reads:

> If connecting to a host fails, then the created session is put to this pool for reuse.

The code is this:

https://github.com/bitcoin/bitcoin/blob/a786fd2041913d82ca90b561de309421bd24e41b/src/net.cpp#L472-L480

I.e. pop/extract a session from `m_unused_i2p_sessions` and if we cannot connect, then we did not use the session and thus put it back to `m_unused_i2p_sessions`.

Do you think that the descript
...

💬 hebasto commented on pull request "build: Enable `thread_local` for MinGW-w64 builds":
(https://github.com/bitcoin/bitcoin/pull/30099#issuecomment-2122373765)
Rebased on top of the merged #30095.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1608129277)
A few considerations:

* Private broadcast connections are not blocked on the `semOutbound` grant. They come to this code right away.

* Even if there is a high demand for private broadcast connections, we would try to open such ones only half of the time in `CConnman::ThreadOpenConnections()` to avoid such starvation of only opening private broadcast and not others for a long time (see how `MaybePickPrivateBroadcastNetwork()` yields to other connection types if we previously opened a privat
...

💬 laanwj commented on pull request "build: Remove `--enable-threadlocal`":
(https://github.com/bitcoin/bitcoin/pull/30137#issuecomment-2122380476)
Code review ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af
This removes, to my knowledge, all mentions of thread-local in the build system or comments.
Will test the guix binary on windows.

💬 hebasto commented on pull request "build: Enable `thread_local` for MinGW-w64 builds":
(https://github.com/bitcoin/bitcoin/pull/30099#issuecomment-2122388233)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/30137.

✅ hebasto closed a pull request: "build: Enable `thread_local` for MinGW-w64 builds"
(https://github.com/bitcoin/bitcoin/pull/30099)

👋 fanquake's pull request is ready for review: "build: Remove `--enable-threadlocal`"
(https://github.com/bitcoin/bitcoin/pull/30137)

💬 maflcko commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1608138344)
So when the minimum freeBSD is 15.0, this workaround can be reverted? It would be good to clarify this in the source code.

⚠️ BenWestgate opened an issue: "contrib/verify-binaries/verify.py incorrectly parses version strings with more than 2 dashes"
(https://github.com/bitcoin/bitcoin/issues/30145)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

When I run:
`./verify.py pub 27.0-x86_64-linux-gnu` instead of downloading the version I told it, it downloads every version in the SHA256SUMS file starting with the first.

The `version_os` and (also `version_rc`) are completely ignored.

When I attempt to work around and give it a 1 dash `version_os` string such as
`./verify.py pub 27.0-x86_64` it will start with bitcoin-27.0-x86_6
...

💬 BenWestgate commented on issue "contrib/verify-binaries/verify.py incorrectly parses version strings with more than 2 dashes":
(https://github.com/bitcoin/bitcoin/issues/30145#issuecomment-2122420660)
I would like to be assigned to this issue and will send a PR in an hour.