💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281768350)
_moving discussion from https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1659008138_

> 2. Rapidly sending multiple transactions "...incremented the number of connections to open from 20 to 25..." this number grows rapidly and decreases slowly (especially on signet) I wonder if it should max out at some point or reduce the tx:relay-peer ratio from 5:1 to just 1:1

`MAX_SENSITIVE_RELAY_CONNECTIONS` (64) is supposed to cap it. Did it exceed 64?

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281830292)
_moving discussion from https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1659008138_

> 3. Restarting the node and reloading the wallet logged messages about pushing the unbroadcast TXs to the mempool, but I do not think any sensitive-relay connections were triggered to deliver those. Eventually I tried sending another new TX and then, even though only 5 sensitive connections were being opened for that one new TX, over time those connections were used for some of the backlog ("preten
...

💬 fanquake commented on pull request "guix: use GCC 12.3.0 to build releases":
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1662118575)
Rebased on master.
Updated the time-machine to current Guix master.
Fixed macOS builds by retaining GCC 10 in it's manifest. This seems like the most straight-forward way to proceed here, given this code is going to be removed soon in any case (and GCC isn't used for the bitcoind builds).

There is currently a non-determinism issue (`x86_64` vs `aarch64`) with the `powerpc64le-linux-gnu` build.

💬 furszy commented on pull request "rpc: getblockfrompeer, introduce fetch from "any" functionality":
(https://github.com/bitcoin/bitcoin/pull/27836#issuecomment-1662121821)
Work moved to #27837. Closing.

✅ furszy closed a pull request: "rpc: getblockfrompeer, introduce fetch from "any" functionality"
(https://github.com/bitcoin/bitcoin/pull/27836)

💬 jamesob commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1662122993)
Concept ACK, looks like some great additional coverage.

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1662136231)
`2541f09439...6e6f83b0f7`: rebase due to conflicts and address some suggestions and issues. Also ditch `UseSensitiveRelay()` and use `m_opts.sensitiverelayowntx` directly.

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281846678)
Done

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281847092)
Done

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281847815)
Moved, thanks!

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281851228)
Good! This should be moved a bit below, after the transaction has been added to the mempool: `const MempoolAcceptResult result = m_chainman.ProcessTransaction(ptx);` and conditional on `result`.

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281854368)
Yes. This is the case for all connections - if the peer goes silent, but does not close the connection, then we will close it after 20 minutes. Do you think that is not suitable for sensitive relay connections? If yes, what to do better?

💬 brunoerg commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1662147529)
Concept ACK

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281863833)
For sensitive relay connections it is not possible to fingerprint based on the version handshake or any other messages exchanged (see the OP under "The messages exchange should look like this").

The only non-constant in the `VERSION` message we send is the peer nonce, then `VERACK` has no payload and the only payload in the `PING` is the ping nonce.

💬 dergoegge commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281864257)
Oh for some reason I thought our ping timeout was smaller than `TIMEOUT_INTERVAL` (turns out it's the same). Feel free to consider this a nit.

But I think it would be reasonable for these connections to timeout much quicker anyway, since we don't care about keeping them around.

💬 dergoegge commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281871273)
> The only non-constant in the VERSION message we send is the peer nonce

Yes and this can be used for fingerprinting, because we disconnect incoming connections that match an already existing nonce.

https://github.com/bitcoin/bitcoin/blob/2fa60f0b683cefd7956273986dafe3bde00c98fd/src/net_processing.cpp#L3361-L3366

And I wouldn't be surprised if there are more ways to fingerprint in the version handshake. This will also end up being a maintenance concern going forward, mostly as a burden
...

💬 jonatack commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1662205614)
Concept ACK

💬 brunoerg commented on pull request "rpc, test: `addnode` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#discussion_r1281911771)
Done!

💬 brunoerg commented on pull request "rpc, test: `addnode` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#discussion_r1281911898)
Done!

💬 brunoerg commented on pull request "rpc, test: `addnode` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#discussion_r1281912594)
Done

💬 brunoerg commented on pull request "rpc, test: `addnode` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#issuecomment-1662221927)
Force-pushed addressing @jonatack's review. Addressed:

- https://github.com/bitcoin/bitcoin/pull/26366#discussion_r1281091853
- https://github.com/bitcoin/bitcoin/pull/26366#discussion_r1281087542
- https://github.com/bitcoin/bitcoin/pull/26366#discussion_r1281083516