💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1935886953)
Force-pushed addressing:
- https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1480525652
- https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1480624819
- https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1481727947
- https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1483435534

Thanks @sr-gi for reviewing!

💬 ryanofsky commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1484292817)
re: https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1484199895

> [3c31173](https://github.com/bitcoin/bitcoin/commit/3c311734d2fc6a4ca410f254ba3c8e923d58be70):
>
> I don't understand this. For example, if `SubStream` is a `DataStream` that holds data, this will now create a copy of this data, when previously it didn't?

I will add a comment here, but this does not happen due to the "Template deduction guide for a single params argument that's slightly different from the default
...

💬 jadijadi commented on issue "Weird focus rect displayed on inital sync":
(https://github.com/bitcoin-core/gui/issues/783#issuecomment-1935930424)
Tried to reproduce this on 26.9.9 with no successful.
May I ask how you produced the "expected result" image? what do you do to "fix" the issue? And is there a way to "show/hide" the strange rectangular? I can see that in the problematic photo you have the Hide button focused too. Does losing the focus hides the strange highlight square?

💬 maflcko commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1484322490)
In any case `m_substream{substream}` still creates a copy when it shouldn't, due to a missing `std::forward`, no?

💬 TheCharlatan commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1935947181)
Concept ACK

📝 vasild opened a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415)
To improve privacy, broadcast locally submitted transactions (from the `sendrawtransaction` RPC) to the P2P network only via Tor or I2P short-lived connections.

* Introduce a new connection type for private broadcast of transactions with the following properties:
* started whenever there are local transactions to be sent
* only opened to Tor or I2P peers
* opened regardless of max connections limits
* after handshake is completed one local transaction is pushed to the peer, `PING`
...

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1935973179)
I opened a new PR at https://github.com/bitcoin/bitcoin/pull/29415 to continue with this.

> in the current implementation, we schedule opening 5 connections per transaction, but then there's randomness around which one of the transactions each connection decides to broadcast. this could lead to a lot of variability if there are multiple transactions queued around the same time, which doesn't seem desirable. It makes more sense to me for there to be a direct mapping

@amitiuttarwar, I was th
...

✅ vasild closed a pull request: "Relay own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/27509)

💬 stratospher commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#discussion_r1484356402)
interesting script! sounds good to be on safer side.

nit: only if you have to retouch, you could update commit message in 100e8a75 to not mention `addmultisigaddress`

💬 stratospher commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#issuecomment-1935983955)
tested ACK 98570fe.

💬 1440000bytes commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-1936003083)
I am not sure about this approach to improve privacy. Is it necessary to open new short lived tor/i2p connections for broadcasting the transaction? What are the trade-offs in this implementation vs a simple implementation to relay tx to one or more peers that our node is already connected to?

Related issues:

https://github.com/bitcoin/bitcoin/issues/21876
https://github.com/bitcoin/bitcoin/issues/28636

💬 epiccurious commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#issuecomment-1936008685)
I'm seeing an issue during testing. The debug log incorrectly reports 0 MB. Are others able to reproduce my result?

```
user1@comp1:~/Documents/GitHub/btc29402$ src/bitcoin-cli --rpcwait getmempoolinfo | jq -c '{ transactions: (.size), memory_usage_in_mb: (.usage/1000000) }' | jq; src/bitcoin-cli --rpcwait stop; sleep 10; grep -a Writing ~/.bitcoin/debug.log | tail -2
{
"transactions": 2529,
"memory_usage_in_mb": 6.061648
}
Bitcoin Core stopping
2024-02-09T14:11:49Z Writing 2529 me
...

🤔 sdaftuar reviewed a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1871529777)
Code review ACK. Left some comments that are just nits, nothing that I think needs to be addressed unless you have to fix more things anyway.

Will do another round of testing shortly...

💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483769698)
nit: "replaceability"
(not worth fixing unless you need to touch this PR again)

💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484331218)
Is dropping the locktime argument here intentional? (I guess all the explicit arguments are caught in kwargs?)

💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483776227)
I can't figure out what this comment means? Maybe a holdover from when we were trimming 0-fee stuff?

💬 epiccurious commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1484386762)
very minor nit - To make the log more readable, can you please add a comma after the 'file'?

So it displays as:
```
Writing 2529 mempool transactions to file, xx MB...
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484387932)
I still would like a test for this, maybe as a follow-up

💬 epiccurious commented on pull request "fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse`":
(https://github.com/bitcoin/bitcoin/pull/29413#issuecomment-1936030123)
utACK 864e2e9097de8f1fda63137f803687dd5cc96c03.

💬 epiccurious commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1936033190)
Concept ACK.