💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1428467257)
> and C should be just "our highest block is older than 48h" (regardless of whether mapBlocksInFlight is empty) => then don't connect to limited peers.

Hmm, this observation makes me think..
what if `m_is_close_to_tip=true` and then the node lags behind; there could be an edge case scenario where the node starts slowly syncing-up the historical blocks before `CheckForStaleTipAndEvictPeers()` is executed, so `mapBlocksInFlight` wouldn't be empty, so.. it would still connect to limited peers e
...

💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1428484311)
> Interesting. Try cleaning your working directory (e.g. `git clean -dfx`) and building again. To try on an older version of master, you can you check out any old commit and then do a `git cherry-pick ...`.

Git bisecting this I found that the tests start timing out with commit 4f473ea515bc77b9138323dab8a741c063d32e8f

Are you able to verify this on your setup?

@darosior would you be able to check as well? 🙏

This is how I did it:
```
git checkout 4f473ea515bc77b9138323dab8a741c063d
...

💬 achow101 commented on pull request "tests: Don't depend on value of DEFAULT_PERMIT_BAREMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/29088#issuecomment-1858499111)
ACK 7b45744df33c6a4759eae1a3984f389cbac837c2

🚀 achow101 merged a pull request: "tests: Don't depend on value of DEFAULT_PERMIT_BAREMULTISIG"
(https://github.com/bitcoin/bitcoin/pull/29088)

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1858509374)
Good news, found something after all the walls of texts.
Having the ´TipMayBeStale()´ call surrounded by `GetUseAddrmanOutgoing()` check makes the node accept limited peers incoming connections indiscriminately (which is what we are trying to avoid here) when it was initialized with outgoing connections disabled.

💬 fjahr commented on pull request "refactor: C++20: Use lambda implicit capture and std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1858564296)
> Also, C++20 is enabled for years, so I don't understand the other commit.

What's the 'other' commit?

> I think you either forgot to compile locally, or your compiler doesn't emit any warnings that the changes are wrong?

I compiled locally just fine and ran all test using `Apple clang version 15.0.0` as the compiler. I will look into the warnings.

🤔 mzumsande reviewed a pull request: "refactor: C++20: Use lambda implicit capture and std::rotl"
(https://github.com/bitcoin/bitcoin/pull/29085#pullrequestreview-1785029422)
I think the issue is that most of the spots that were changed don't actually make use of the implicit capture of `this` (e.g. by accessing a class or struct member in the body of the lambda) and therefore don't need to be changed.
Only the example in the scheduler does that (which is a doc). Not the use of `[=]` in general is deprecated, just using `[=]` in combination with accessing `this` via implicit capture.

👍 TheCharlatan approved a pull request: "contrib: drop GCC MAX_VERSION to 4.3.0 in symbol-check"
(https://github.com/bitcoin/bitcoin/pull/28844#pullrequestreview-1785045706)
ACK ff896d25819da1c1e80591595c922fb093942645

Ran a guix build and:

```
nm -D bitcoind | grep -i gcc
...
U _Unwind_GetDataRelBase@GCC_3.0
U _Unwind_GetIPInfo@GCC_4.2.0
U _Unwind_GetLanguageSpecificData@GCC_3.0
...
```

💬 fjahr commented on pull request "refactor: C++20: Use lambda implicit capture and std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1858599666)
Seems right that the capture doesn't always need to be explicit, I dropped the second commit.

I don't see any performance impact on chacha20 from the first commit:

<details>
<summary>MASTER</summary>

$ src/bench/bench_bitcoin -filter="CHACHA20_1MB|CHACHA20_256BYTES|CHACHA20_64BYTES"

| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1.92 | 522,177,61
...

💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1428581458)
That's an excellent point, but I doubt there is anything already present that will prevent this.

Here are two ideas, what do you think?

- Whatever argument is given to `-datadir=`, _always_ make (if necessary) and use a subdirectory (within the specified directory) called `test_bitcoin`. That way, even if the user specifies their real data directory, all the test stuff will be within this subdirectory. (It's similar to how there already are subdirectories for testnet, signet, and regtest.)
...

📝 1wiz1 opened a pull request: "Rename SECURITY.md to SECURITY.md."
(https://github.com/bitcoin/bitcoin/pull/29096)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ fanquake closed a pull request: "Rename SECURITY.md to SECURITY.md."
(https://github.com/bitcoin/bitcoin/pull/29096)

📝 fanquake locked a pull request: "Rename SECURITY.md to SECURITY.md."
(https://github.com/bitcoin/bitcoin/pull/29096)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1428713566)
> It would just be a one-line function to hold the code you already wrote in two places:

oh got it now.

> I wonder if this can be generalized to pass test-only options (along with a helper HasTestOption, similar to IsDeprecatedRPCEnabled?

good idea - liked how we can make sure users don't accidentally use these test-only options in their config file.
done.

💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1428713673)
> I think it would be ideal if we could avoid the addpeeraddress test leaking into getaddrmaninfo and getrawaddrman tests.

it would be cleaner to not need to know the existing context of addrman when dealing with getaddrmaninfo/getrawaddrman tests. both the options sound good to me!

> The easiest way of producing a collision is by adding more and more addresses to the tried table (as we do in the unit test). This would leave us with quite a few additional addresses already in the tried tab
...

💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1428713695)
done.

⚠️ helpau opened an issue: "Connection between nodes on the PC interfaces doesn't work"
(https://github.com/bitcoin/bitcoin/issues/29097)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

The nodes haven't started synchronising, User-Agent are doubled


### Expected behaviour

The nodes have started synchronising, User-Agents are different

### Steps to reproduce

Preconditions: Bitcoin Core 25.1(reference node, partially synchronized) and self-compiled Bitcoin Core(from mast
...

💬 helpau commented on issue "Connection between nodes on the PC interfaces doesn't work":
(https://github.com/bitcoin/bitcoin/issues/29097#issuecomment-1858753241)
Probably related #21747 #22559

⚠️ Sjors opened an issue: "Performance decrease after tapscript miniscript"
(https://github.com/bitcoin/bitcoin/issues/29098)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

@eriknylund noticed while testing large multisigs that after 4f473ea515bc77b9138323dab8a741c063d32e8f `sendtoaddress` causes a timeout for much smaller (though still rather large) multisigs than before.

https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1428484311

### Expected behaviour

Not sure, perhaps the performance decrease is justified. Ideally it would not slow down.


...

💬 Sjors commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1428740238)
Great find! Let's track this in a separate issue: #29098