👍 vasild approved a pull request: "net: call `Select` with reachable networks in `ThreadOpenConnections`"
(https://github.com/bitcoin/bitcoin/pull/29436#pullrequestreview-1885261790)
ACK 7edb07ca800991f7c88d582b880a392efe17f31d

About the graph in the OP: "... until it finds an address from a network that compose 20% ...". The chance of not getting the result after `N` tries is `(8/10)^N`. For example for `N=25` that is less than `0.004`. What is the explanation of having so many dots so high? Uneven distribution within addrman?

📝 glozow opened a pull request: "[26.1] final changes for 26.1rc1"
(https://github.com/bitcoin/bitcoin/pull/29440)
Final changes to tag a 26.1rc1.
Bumps version numbers, man pages, adds release notes etc.

💬 vasild commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1492529744)
Does not matter now, but it should work with vector as well.

🤔 hebasto reviewed a pull request: "lint: Check for missing bitcoin-config.h includes"
(https://github.com/bitcoin/bitcoin/pull/29408#pullrequestreview-1885268833)
https://api.cirrus-ci.com/v1/task/5330278671974400/logs/ci.log:

```
crypto/sha256.cpp should remove these lines:
- #include <config/bitcoin-config.h> // lines 6-6
```

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-1948481906)
Windows test fails with:

```
test/sv2_noise_tests.cpp(62): fatal error: in "sv2_noise_tests/certificate_test": critical check !alice_certificate.Validate(XOnlyPubKey(alice_authority_key.GetPubKey())) has failed
```

Odd. Part of https://github.com/bitcoin/bitcoin/pull/29346 so can be debugged there.

💬 vasild commented on pull request "refactor: Compile unreachable walletdb code":
(https://github.com/bitcoin/bitcoin/pull/29315#issuecomment-1948486081)
> When unreachable code isn't compiled, compile failures are not detected.

Another approach to this is to run CI with both `USE_BDB` defined and `USE_BDB` undefined.

> Fix this by leaving it unreachable, but compiling it.

This produces [compiler warnings](https://github.com/bitcoin/bitcoin/issues/29334) about unreachable code. I guess that if we are to have deliberately unreachable code, then some pragma is needed to tell compilers not to warn about it in those particular places where i
...

💬 vasild commented on issue "Clang 14 emits `-Wunreachable-code` warnings":
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1948489550)
I see, commented in https://github.com/bitcoin/bitcoin/pull/29315#issuecomment-1948486081

💬 theStack commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#issuecomment-1948492950)
Concept ACK

💬 Kixunil commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1948497448)
> Removal happens at some unknown time after deprecation

That doesn't help people who really needed the library. Yeah, they will get screwed later but still they are getting screwed. I meant period to voice opposition to this change. Unless you're open to un-deprecating of course.

> What exactly do you mean by "verified"?

It must be possible to include it in a block at some later time when the parent is confirmed and time locks are satisfied. Basically the same requirement as LN force c
...

💬 ryanofsky commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#discussion_r1492564530)
In commit "wallet: Retrieve ID from loaded DescSPKM directly" (ac246f68299d0dc208ae513dfad1a8fc91b5e6d4)

Not important, but you could avoid the dynamic_cast and remove some boilerplate with:

```c++
DescriptorScriptPubKeyMan& CWallet::LoadDescriptorScriptPubKeyMan(uint256 id, WalletDescriptor& desc)
{
DescriptorScriptPubKeyMan* spkm;
if (IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) {
spkm = new ExternalSignerScriptPubKeyMan(*this, desc, m_keypool_size);
} else {

...

🤔 ryanofsky reviewed a pull request: "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/26008#pullrequestreview-1885278916)
Code ACK ac246f68299d0dc208ae513dfad1a8fc91b5e6d4

Looks good, but wondering about a possible bug in the GetScriptPubKeyMans legacy case (see below)

💬 ryanofsky commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#discussion_r1492548296)
In commit "wallet: Use scriptPubKey cache in GetScriptPubKeyMans" (b9dd0fecda98bd14d8d305a69e6ce65f1e0a73fa)

This also needs to check GetLegacyScriptPubKeyMan()->CanProvide() to match previous behavior, I think

💬 hebasto commented on pull request "[26.1] final changes for 26.1rc1":
(https://github.com/bitcoin/bitcoin/pull/29440#issuecomment-1948517549)
@glozow

Feel free to pick up the https://github.com/hebasto/bitcoin/commit/fb29011cbec8997c70f30797bbba3784702158a0 commit from the https://github.com/hebasto/bitcoin/commits/240216-26.1rc1-tr/ branch to pull the recent translations from the Transifex website.

(same as https://github.com/bitcoin/bitcoin/pull/28763 etc)

💬 maflcko commented on pull request "refactor: Compile unreachable walletdb code":
(https://github.com/bitcoin/bitcoin/pull/29315#issuecomment-1948526834)
> Another approach to this is to run CI with both `USE_BDB` defined and `USE_BDB` undefined.

This is already done, but generally it is better to notify devs of compile failures immediately, instead of going through the lengthy CI pipeline.



> This produces [compiler warnings](https://github.com/bitcoin/bitcoin/issues/29334) about unreachable code. I guess that if we are to have deliberately unreachable code, then some pragma is needed to tell compilers not to warn about it in those part
...

🤔 kevkevinpal reviewed a pull request: "test/BIP324: disconnection scenarios during v2 handshake"
(https://github.com/bitcoin/bitcoin/pull/29431#pullrequestreview-1884162177)
Concept ACK

added two comments I will continue to test these changes as I am still trying to understand fully what the tests do exactly

💬 kevkevinpal commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1491848110)
do you think it would make sense to use `test_earlykeyresponse` here and then make `test_v2disconnection` into `run_test`

💬 kevkevinpal commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1491897558)
Are the `test_type`'s in `test_v2disconnection` dependent on `test_earlykeyresponse` to run first?

I tried running the tests individually for each `test_type` but I ran into this error, using this diff a74f4ddcc50eeba04977d8eeb96ffee2a94dbbf9
```
2024-02-16T02:38:17.055000Z TestFramework (INFO): PRNG seed is: 8074818075718275852
2024-02-16T02:38:17.056000Z TestFramework (INFO): Initializing test directory /var/folders/9g/cvx014yx4dq5lwl_x5zvn_j80000gn/T/bitcoin_func_test_dmwxb4ar
2024-02
...

💬 vasild commented on pull request "rpc: Fixed signed integer overflow for large feerates":
(https://github.com/bitcoin/bitcoin/pull/29434#discussion_r1492622692)
I can reproduce now, my bad probably removing the newlines from the base64 input (even though my `base64` tool succeeds in decoding that).

📝 maflcko opened a pull request: "ci: Avoid CI failures from temp env file reuse"
(https://github.com/bitcoin/bitcoin/pull/29441)
* Currently, running separate CI tasks at the same time may intermittently fail, because they race to read/write `/tmp/env`. Fix this by adding `$CONTAINER_NAME` to the file name.

* Also, add `$USER`, while touching the line, to allow different users to run the same CI task at the same time.

* Also, skip the git install if there is no need.

Ref: https://github.com/bitcoin/bitcoin/pull/29274

💬 hebasto commented on pull request "[26.1] final changes for 26.1rc1":
(https://github.com/bitcoin/bitcoin/pull/29440#issuecomment-1948586673)
To fix the "macOS 13 native" job failure we should backport https://github.com/bitcoin/bitcoin/pull/29195.