Bitcoin Core Github
42 subscribers
124K links
Download Telegram
📝 jonatack reopened a pull request: "test: add peer connection unit test coverage"
(https://github.com/bitcoin/bitcoin/pull/28749)
This PR is based on #28155 and adds unit test coverage for each change in it (i.e. tests that fail on master), as well as some missing unit test coverage in general for `CConnman::AddNode()`, `ConnectNode()`, `GetAddedNodeInfo()`, `ThreadOpenAddedConnections()`, and RPC `addnode` -- see coverage check at https://corecheck.dev/bitcoin/bitcoin/pulls/28749. I'm building on these initial unit tests for writing coverage for #28248.

A version of this branch based on current master instead of #2815
...
💬 vasild commented on pull request "Do the SOCKS5 handshake reliably":
(https://github.com/bitcoin/bitcoin/pull/28649#issuecomment-1787664526)
`608f8aa1c4...af0fca530e`: use a `Span` for `Sock::SendComplete()`, looks better, thanks for the [suggestion](https://github.com/bitcoin/bitcoin/pull/28649#issuecomment-1765229184).
💬 jonatack commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1377947935)
Thanks, I've repushed #28749 with the update.
💬 sr-gi commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1377955800)
Rebased
jonatack closed a pull request: "test: add peer connection unit test coverage"
(https://github.com/bitcoin/bitcoin/pull/28749)
💬 jonatack commented on pull request "test: add peer connection unit test coverage":
(https://github.com/bitcoin/bitcoin/pull/28749#issuecomment-1787695089)
Re-opened to push an update and re-closed after it was cherry-picked into #28155.
💬 fanquake commented on pull request "test: add peer connection unit test coverage":
(https://github.com/bitcoin/bitcoin/pull/28749#issuecomment-1787698569)
In future, there is no need to reopen and reclose pull requests, for someone to cherry pick a commit out of a branch. Someone can just fetch your remote after you've pushed changes, and cherry-pick what they like.
💬 achow101 commented on pull request "gui: fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1787704366)
Should this be backported?
💬 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