💬 vasild commented on pull request "Improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1296888861)
```suggestion
std::vector<AddedNodeInfo> vInfo = GetAddedNodeInfo(/*include_connected=*/false);
```
and in other call sites too

💬 vasild commented on pull request "Improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1296885786)
style (same in the declaration):
```suggestion
std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo(bool include_connected) const
```

💬 vasild commented on pull request "Improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1296832854)
The old code would pick a random address from the list of resolved addresses. The new code would pick the last one. I think it is good to preserve the randomness. One way to achieve that would be to shuffle the result before iterating and picking the last one.

```cpp
if (!resolved.empty()) {
Shuffle(resolved.begin(), resolved.end(), FastRandomContext{});
for (const auto& r : resolved) {
```

or if the shuffle is perceived expensive for big lists, before the loop `auto to_pick =
...

💬 MarcoFalke commented on pull request "ci: Add test-each-commit task":
(https://github.com/bitcoin/bitcoin/pull/28279#issuecomment-1681905132)
> Probably but I am wondering a bit if these task could end up clogging our self-hosted worker queue and hinder other jobs from running in their regular time?

Right, that is another reason *for* self-hosted runners, because it is possible to assign a label to them, so that they are task-specific, and not interfere with other tasks. On GHA "free" runners will stop being spun up after ~n total running tasks.

💬 vasild commented on issue "p2p sends local-scope addresses when IPv4 is behind NAT":
(https://github.com/bitcoin/bitcoin/issues/8616#issuecomment-1681907760)
https://github.com/bitcoin/bitcoin/pull/27411 should have resolved 3.

💬 dergoegge commented on pull request "fuzz: call `LookupSubNet` before calling `Ban` with a subnet":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1681918508)
This still crashes for me on the following input `/r8BAAAC/9LcAF8gMiWADQUA//////////////8AN1wNXMnJyQ3///8Afv///wABAAAAgAAAAADPvg==`

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1681920298)
Rebased on master to fix the macOS CI.

💬 darosior commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-1681922742)
Rebased on master to fix the macOS CI.

💬 MarcoFalke commented on pull request "validation: don't clear cache on periodic flush":
(https://github.com/bitcoin/bitcoin/pull/28233#issuecomment-1681928913)
Not sure what's up with CI. Maybe rebase?

💬 MarcoFalke commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#issuecomment-1681931026)
Looks like the functional test times out in CI?

💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1681932118)
Could rebase for CI, if still relevant?

💬 MarcoFalke commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-1681933926)
Maybe mark as draft while CI is red and this is based on other pulls?

💬 MarcoFalke commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1296928288)
```suggestion
assert_raises_rpc_error,
```

style-nit: Add the missing comma here? Otherwise someone will have to touch this line again when appending something.

A (force) push should also make CI less red.

💬 MarcoFalke commented on pull request "ci: label docker images and prune dangling images selectively":
(https://github.com/bitcoin/bitcoin/pull/27793#issuecomment-1681938003)
Could rebase for green CI, if still relevant?

💬 MarcoFalke commented on pull request "indexes: Stop using node internal types and locking cs_main, improve sync logic":
(https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-1681939421)
Maybe a silent merge conflict in a functional test, see CI?

```
�[0m�[0;31mfeature_index_prune.py | ✖ Failed | 2401 s

💬 MarcoFalke commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1681940885)
Could rebase for green CI, if still relevant?

💬 MarcoFalke commented on pull request "test doc: tests `acceptstalefeeestimates` option is only supported on regtest chain":
(https://github.com/bitcoin/bitcoin/pull/28157#issuecomment-1681941214)
Could rebase for green CI, if still relevant?

💬 RicYashiroLee commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1681944080)
> This should summarize this whole debate which started from `laser-eyed maxis` Twitter accounts virtue signaling on Twitter and calling for miners to not mine ordinal transactions in Q1-Q2 this year.
>
> `Bitcoin for me, but not for thee`. Respectfully, anyone who wants to dictate the `right` use of bitcoin for what I call frankly, the Church of Satoshi... needs to go and _kindly_... **fuck off**.
>
> You are turning out to be no better than the central bankers you demonize in the process
...

💬 MarcoFalke commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#issuecomment-1681944567)
Needs rebase, if still relevant.

📝 fanquake locked a pull request: "set `DEFAULT_PERMIT_BAREMULTISIG` to false"
(https://github.com/bitcoin/bitcoin/pull/28217)
The default activation of the `permitbaremultisig=0` option proposes an enhancement for the Bitcoin network. By refusing non-P2SH multisignature transactions from the outset, this modification would contribute to reducing spam attempts and maintaining a healthy decentralization by discouraging undesirable activities.