💬 jonatack commented on pull request "test: add peer connection unit test coverage":
(https://github.com/bitcoin/bitcoin/pull/28749#issuecomment-1787712031)
Yes. It was to allow verifying the CI was green, if desired, prior to cherry-picking.

💬 jonatack commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#issuecomment-1787716146)
re-ACK 0420f99f429ce2382057e101859067f40de47be0

💬 theStack commented on pull request "gui: fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1787748102)
> Should this be backported?

Yes, I think so. The bug first occured with release 25.0 (according to `git tag --contains 4492de1be11` in the main repo), so backporting to 25.x and 26.x would make sense.

💬 achow101 commented on pull request "gui: fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1787761090)
ACK e26e665f9f64a962dd56053be817cc953e714847

📝 instagibbs opened a pull request: "Fuzz: Check individual and package transaction invariants"
(https://github.com/bitcoin/bitcoin/pull/28764)
Poached from https://github.com/bitcoin/bitcoin/pull/26711 since that PR is being split apart, and modified to match current behavior.

💬 toolsopen commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1787882572)
> If you import the descriptor as above and use `"timestamp": "now"` then it won't rescan for historical transactions. Although if any time in the future you happened to rescan your chain, these would be picked back up by the wallet. But AFAIK you can't "clear" them from the wallet.

1.is to turn off some mining settings
2.The following versions and commands were used for testing
> https://bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-x86_64-linux-gnu.tar.gz
> curl -X POST 'http://te
...

💬 toolsopen commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1787885034)
> > Will wallet.dat 9.3G affect performance?
>
> Yes, significantly.
>
> > Is there any way to clear the used transactions in it?
>
> Not automatically. However you can use `removeprunedfunds` and delete old transactions manually.

This helps, let me write a script and try it out

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1378086895)
yeah sure, good idea.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1378088543)
Sure. This is because we execute the stale check every 10 min, so needed to push the last stale check time forward.
Will re-order the test checks so this becomes clearer.

💬 mzumsande commented on issue "Evicting and filling attack for linking multiple network addresses":
(https://github.com/bitcoin/bitcoin/issues/28760#issuecomment-1787932314)
I may be too pessimistic but think that preventing fingerprinting as a whole is close to a lost cause currently - not in theory, but in the sense that solving all possible fingerprinting vectors would be **a lot** of work.

I think there are fingerprinting vectors in all three relay types (addrs, transactions, blocks) - some are public, e.g. #24571, some maybe aren't documented but are pretty obvious (probably not helpful to document specifics in one public place, so I won't do that here) - an
...

🤔 furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1707131031)
Updated per feedback, thanks @andrewtoth.

* Re-ordered test checks for improved clarity.
* Included coverage for when the node is stale but still recoverable (below the limited peers threshold).

🤔 jonatack reviewed a pull request: "Do the SOCKS5 handshake reliably"
(https://github.com/bitcoin/bitcoin/pull/28649#pullrequestreview-1707129039)
ACK af0fca530e4d8311bcb24a14c416e5ad7c30ff78

💬 jonatack commented on pull request "Do the SOCKS5 handshake reliably":
(https://github.com/bitcoin/bitcoin/pull/28649#discussion_r1378097032)
<details><summary>The first commit could be smaller and simpler/clearer (feel free to ignore).</summary><p>

```diff
@@ -283,13 +283,6 @@ void Sock::SendComplete(Span<const unsigned char> data,
}
}

-void Sock::SendComplete(Span<const char> data,
- std::chrono::milliseconds timeout,
- CThreadInterrupt& interrupt) const
-{
- SendComplete(MakeUCharSpan(data), timeout, interrupt);
-}
-
std::string Sock::RecvUntilTerminator(uint8
...

👍 hebasto approved a pull request: "[26.x] Backports for rc2"
(https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707143194)
ACK 0989d00b7b298bef9e1faf04c1753a024b7dfd9e, I've applied backport locally and got a zero diff. Also I've reviewed the last two commits.

👍 TheCharlatan approved a pull request: "[26.x] Backports for rc2"
(https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707161659)
lgtm ACK 9c29048dac5dfa5a06323b350156b1212b87d56d

💬 maflcko commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1787970115)
No idea what is wrong with Microsoft/Azure/GHA again. Locally it works fine, but not on their infra:

https://github.com/bitcoin/bitcoin/actions/runs/6711963492/job/18240509895?pr=28349#step:6:210

```
checking whether clang is Clang... yes
checking whether pthreads work with "-pthread" and "-lpthread"... yes
checking whether Clang needs flag to prevent "argument unused" warning when linking with -pthread... no
checking for joinable pthread attribute... PTHREAD_CREATE_JOINABLE
checking
...

💬 theuni commented on pull request "build: Windows SSP roundup":
(https://github.com/bitcoin/bitcoin/pull/28461#issuecomment-1787983662)
Concept ACK
[This comment](https://github.com/mesonbuild/meson/issues/10673#issuecomment-1243061073 ) (and the rest of that thread) helped me to understand what was going on here. As I understand it, "-lssp" should've never been needed. It was used as a hack for years, but is no longer required after mingw-w64 v11.

I'm confused like @fanquake though. I would have expected us to need to carry the hack for a while, considering that the fix is from this year.

💬 mzumsande commented on pull request "addrman: log AS only when using asmap":
(https://github.com/bitcoin/bitcoin/pull/28729#issuecomment-1788025388)
Code Review ACK 02a4f1a3859ed7e865641b35ca1bc9ce711e696f

💬 jonatack commented on issue "Evicting and filling attack for linking multiple network addresses":
(https://github.com/bitcoin/bitcoin/issues/28760#issuecomment-1788045181)
Introducing some randomness into the eviction algorithm to make it less deterministic -- instead of, or in addition to, random disconnection delay -- might be a potentially low-hanging countermeasure.

*"Bitcoin allows multiple connections from one single address (Saad et al. [2021](https://cybersecurity.springeropen.com/articles/10.1186/s42400-023-00182-9#ref-CR45)). This property significantly reduces the attack cost."*

#28248 proposes to improve or address this in several ways.

*"We d
...

💬 pablomartin4btc commented on pull request "gui: fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1788048860)
I do agree with @theStack backporting (checked it with the `git tag` command) since the issue technically is the "conflict" between these 2 lines:
https://github.com/bitcoin-core/gui/blob/3c0b66c2ece7695ab59f466d7b4d81a4c18ffd76/src/qt/transactionview.cpp#L532-L533

The last one (#L533) was added by the #708 (merged in v25 - I wanted to say I'm innocent but no haha), so another way to solve this issue is just by removing #L532, which I've tested and works fine.