b261100e71697dd4859cdeb5a69f5a1d6c557099 [qt] Set peer version and subversion to N/A when not available or detecting (WakeTrainDev)
Pull request description:
In the debug console peer detail window, display "N/A" for the User Agent and Version when the peer is still detecting or the information is unavailable, instead of retaining the previous values.
ACKs for top commit:
maflcko:
lgtm ACK b261100e71697dd4859cdeb5a69f5a1d6c557099
luke-jr:
utACK b261100e71697dd4859cdeb5a69f5a1d6c557099
Tree-SHA512: ffcba716fe6173062fe00e2d428d41bbdcaebfe8c76c804519e46a448ade2785ae32efb1a30322adc19cf29e07ea8ab4d7593ef5a17b6c418c8dd77b381e4f77
89372213048adf37a47427112a1ff836ee84c50e doc: add release notes for 29415 (Vasil Dimov)
582016fa5f013817db650bbba0a40d9195c18e2e test: add unit test for the private broadcast storage (Vasil Dimov)
e74d54e04896a86cad4e4b1bd9641afcc3a026c2 test: add functional test for private broadcast (Vasil Dimov)
818b780a05db126dcfe7efe12c46c84b5cfc3de6 rpc: use private broadcast from sendrawtransaction RPC if -privatebroadcast is ON (Vasil Dimov)
eab595f9cf13f7cb1d25a0db51409535cfe053b1 net_processing: retry private broadcast (Vasil Dimov)
37b79f9c39db5a4a61d360a6a29c8853bb5c7ac0 net_processing: stop private broadcast of a transaction after round-trip (Vasil Dimov)
2de53eee742da11b0e3f6fc44c39f2b5b5929da1 net_processing: handle ConnectionType::PRIVATE_BROADCAST connections (Vasil Dimov)
30a9853ad35365af8545e8e766d75cf398968480 net_processing: move a debug check in VERACK processing earlier (Vasil Dimov)
d1092e5d48ce67bd517068550c78bfcab062a554 net_processing: modernize PushNodeVersion() (Vasil Dimov)
9937a12a2fd5a0033f37f4dda5d75bfc5f15c3b6 net_processing: move the debug log about receiving VERSION earlier (Vasil Dimov)
a098f37b9e240291077a7f440e9f57e61f30e158 net_processing: reorder the code that handles the VERSION message (Vasil Dimov)
679ce3a0b8df6e8cab07965301382d2036ef2368 net_processing: store transactions for private broadcast in PeerManager (Vasil Dimov)
a3faa6f944a672faccac5dd201c8d33a638d9091 node: extend node::TxBroadcast with a 3rd option (Vasil Dimov)
95c051e21051bd469fda659fe7c495d5e264d221 net_processing: rename RelayTransaction() to better describe what it does (Vasil Dimov)
bb49d26032c57714c62a4b31ff1fdd969751683f net: implement opening PRIVATE_BROADCAST connections (Vasil Dimov)
01dad4efe2b38b7a71c96b6222147f395e0c11d9 net: introduce a new connection type for private broadcast (Vasil Dimov)
94aaa5d31b6ff1d0122319fc70e70a7e27e1a0ba init: introduce a new option to enable/disable private broadcast (Vasil Dimov)
d6ee490e0a9a81b69a4751087918303163ba8869 log: introduce a new category for private broadcast (Vasil Dimov)
Pull request description:
_Parts of this PR are isolated in independent smaller PRs to ease review:_
* [x] _https://github.com/bitcoin/bitcoin/pull/29420_
* [x] _https://github.com/bitcoin/bitcoin/pull/33454_
* [x] _https://github.com/bitcoin/bitcoin/pull/33567_
* [x] _https://github.com/bitcoin/bitcoin/pull/33793_
---
To improve privacy, broadcast locally submitted transactions (from the `sendrawtransaction` RPC) to the P2P network only via Tor or I2P short-lived connections, or to IPv4/IPv6 peers but through the Tor network.
* Introduce a new connection type for private broadcast of transactions with the following properties:
* started whenever there are local transactions to be sent
* opened to Tor or I2P peers or IPv4/IPv6 via the Tor proxy
* opened regardless of max connections limits
* after handshake is completed one local transaction is pushed to the peer, `PING` is sent and after receiving `PONG` the connection is closed
* ignore all incoming messages after handshake is completed (except `PONG`)
* Broadcast transactions submitted via `sendrawtransaction` using this new mechanism, to a few peers. Keep doing this until we receive back this transaction from one of our ordinary peers (this takes about 1 second on mainnet).
* The transaction is stored in peerman and does not enter the mempool.
* Once we get an `INV` from one of our ordinary peers, then the normal flow executes: we request the transaction with `GETDATA`, receive it with a `TX` message, put it in our mempool and broadcast it to all our existent connections (as if we see it for the first time).
* After we receive the full transaction as a `TX` message, in reply to our `GETDATA` request, only then consider the transaction has propagated through the network and remove it from the storage in peerman, ending the private broadcast attempts.
The messages exchange should look like this:
```
tx-sender >--- connect -------> tx-recipient
tx-sender >--- VERSION -------> tx-recipient (dummy VERSION with no revealing data)
tx-sender <--- VERSION -------< tx-recipient
tx-sender <--- WTXIDRELAY ----< tx-recipient (maybe)
tx-sender <--- SENDADDRV2 ----< tx-recipient (maybe)
tx-sender <--- SENDTXRCNCL ---< tx-recipient (maybe)
tx-sender <--- VERACK --------< tx-recipient
tx-sender >--- VERACK --------> tx-recipient
tx-sender >--- INV/TX --------> tx-recipient
tx-sender <--- GETDATA/TX ----< tx-recipient
tx-sender >--- TX ------------> tx-recipient
tx-sender >--- PING ----------> tx-recipient
tx-sender <--- PONG ----------< tx-recipient
tx-sender disconnects
```
Whenever a new transaction is received from `sendrawtransaction` RPC, the node will send it to a few (`NUM_PRIVATE_BROADCAST_PER_TX`) recipients right away. If after some time we still have not heard anything about the transaction from the network, then it will be sent to 1 more peer (see `PeerManagerImpl::ReattemptPrivateBroadcast()`).
A few considerations:
* The short-lived private broadcast connections are very cheap and fast wrt network traffic. It is expected that some of those peers could blackhole the transaction. Just one honest/proper peer is enough for successful propagation.
* The peers that receive the transaction could deduce that this is initial transaction broadcast from the transaction originator. This is ok, they can't identify the sender.
---
<details>
<summary>How to test this?</summary>
Thank you, @stratospher and @andrewtoth!
Start `bitcoind` with `-privatebroadcast=1 -debug=privatebroadcast`.
Create a wallet and get a new address, go to the Signet faucet and request some coins to that address:
```bash
build/bin/bitcoin-cli -chain="signet" createwallet test
build/bin/bitcoin-cli -chain="signet" getnewaddress
```
Get a new address for the test transaction recipient:
```bash
build/bin/bitcoin-cli -chain="signet" loadwallet test
new_address=$(build/bin/bitcoin-cli -chain="signet" getnewaddress)
```
Create the transaction:
```bash
# Option 1: `createrawtransaction` and `signrawtransactionwithwallet`:
txid=$(build/bin/bitcoin-cli -chain="signet" listunspent | jq -r '.[0] | .txid')
vout=$(build/bin/bitcoin-cli -chain="signet" listunspent | jq -r '.[0] | .vout')
echo "txid: $txid"
echo "vout: $vout"
tx=$(build/bin/bitcoin-cli -chain="signet" createrawtransaction "[{\"txid\": \"$txid\", \"vout\": $vout}]" "[{\"$new_address\": 0.00001000}]" 0 false)
echo "tx: $tx"
signed_tx=$(build/bin/bitcoin-cli -chain="signet" signrawtransactionwithwallet "$tx" | jq -r '.hex')
echo "signed_tx: $signed_tx"
# OR Option 2: `walletcreatefundedpsbt` and `walletprocesspsbt`:
# This makes it not have to worry about inputs and also automatically sends back change to the wallet.
# Start `bitcoind` with `-fallbackfee=0.00003000` for instance for 3 sat/vbyte fee.
psbt=$(build/bin/bitcoin-cli -chain="signet" walletcreatefundedpsbt "[]" "[{\"$new_address\": 0.00001000}]" | jq -r '.psbt')
echo "psbt: $psbt"
signed_tx=$(build/bin/bitcoin-cli -chain="signet" walletprocesspsbt "$psbt" | jq -r '.hex')
echo "signed_tx: $signed_tx"
```
Finally, send the transaction:
```bash
raw_tx=$(build/bin/bitcoin-cli -chain="signet" sendrawtransaction "$signed_tx")
echo "raw_tx: $raw_tx"
```
</details>
---
<details>
<summary>High-level explanation of the commits</summary>
* New logging category and config option to enable private broadcast
* `log: introduce a new category for private broadcast`
* `init: introduce a new option to enable/disable private broadcast`
* Implement the private broadcast connection handling on the `CConnman` side:
* `net: introduce a new connection type for private broadcast`
* `net: implement opening PRIVATE_BROADCAST connections`
* Prepare `BroadcastTransaction()` for private broadcast requests:
* `net_processing: rename RelayTransaction to better describe what it does`
* `node: extend node::TxBroadcast with a 3rd option`
* `net_processing: store transactions for private broadcast in PeerManager`
* Implement the private broadcast connection handling on the `PeerManager` side:
* `net_processing: reorder the code that handles the VERSION message`
* `net_processing: move the debug log about receiving VERSION earlier`
* `net_processing: modernize PushNodeVersion()`
* `net_processing: move a debug check in VERACK processing earlier`
* `net_processing: handle ConnectionType::PRIVATE_BROADCAST connections`
* `net_processing: stop private broadcast of a transaction after round-trip`
* `net_processing: retry private broadcast`
* Engage the new functionality from `sendrawtransaction`:
* `rpc: use private broadcast from sendrawtransaction RPC if -privatebroadcast is ON`
* New tests:
* `test: add functional test for private broadcast`
* `test: add unit test for the private broadcast storage`
</details>
---
**This PR would resolve the following issues:**
https://github.com/bitcoin/bitcoin/issues/3828 Clients leak IPs if they are recipients of a transaction
https://github.com/bitcoin/bitcoin/issues/14692 Can't configure bitocoind to only send tx via Tor but receive clearnet transactions
https://github.com/bitcoin/bitcoin/issues/19042 Tor-only transaction broadcast onlynet=onion alternative
https://github.com/bitcoin/bitcoin/issues/24557 Option for receive events with all networks, but send transactions and/or blocks only with anonymous network[s]?
https://github.com/bitcoin/bitcoin/issues/25450 Ability to broadcast wallet transactions only via dedicated oneshot Tor connections
https://github.com/bitcoin/bitcoin/issues/32235 Tor: TX circuit isolation
**Issues that are related, but (maybe?) not to be resolved by this PR:**
https://github.com/bitcoin/bitcoin/issues/21876 Broadcast a transaction to specific nodes
https://github.com/bitcoin/bitcoin/issues/28636 new RPC: sendrawtransactiontopeer
---
Further extensions:
* Have the wallet do the private broadcast as well, https://github.com/bitcoin/bitcoin/issues/11887 would have to be resolved.
* Have the `submitpackage` RPC do the private broadcast as well, [draft diff in the comment below](https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-2972293733), thanks ismaelsadeeq!
* Add some stats via RPC, so that the user can better monitor what is going on during and after the broadcast. Currently this can be done via the debug log, but that is not convenient.
* Make the private broadcast storage, currently in peerman, persistent over node restarts.
* Add (optional) random delay before starting to broadcast the transaction in order to avoid correlating unrelated transactions based on the time when they were broadcast. Suggested independently of this PR [here](https://github.com/bitcoin/bitcoin/issues/30471).
* Consider periodically sending transactions that did not originate from the node as decoy, discussed [here](https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2035414972).
* Consider waiting for peer's FEEFILTER message and if the transaction that was sent to the peer is below that threshold, then assume the peer is going to drop it. Then use this knowledge to retry more aggressively with another peer, instead of the current 10 min. See [comment below](https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3258611648).
* It may make sense to be able to override the default policy -- eg so submitrawtransaction can go straight to the mempool and relay, even if txs are normally privately relayed. See [comment below](https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3427086681).
* As a side effect we have a new metric available - the time it takes for a transaction to reach a random node in the network (from the point of view of the private broadcast recipient the tx originator is a random node somewhere in the network). This can be useful for monitoring, unrelated to privacy characteristics of this feature.
---
_A previous incarnation of this can be found at https://github.com/bitcoin/bitcoin/pull/27509. It puts the transaction in the mempool and (tries to) hide it from the outside observers. This turned out to be too error prone or maybe even impossible._
ACKs for top commit:
l0rinc:
code review diff ACK 89372213048adf37a47427112a1ff836ee84c50e
andrewtoth:
ACK 89372213048adf37a47427112a1ff836ee84c50e
pinheadmz:
ACK 89372213048adf37a47427112a1ff836ee84c50e
w0xlt:
ACK 8937221304 with nit https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2654849875
mzumsande:
re-ACK 89372213048adf37a47427112a1ff836ee84c50e
Tree-SHA512: d51dadc865c2eb080c903cbe2f669e69a967e5f9fc64e9a20a68f39a67bf0db6ac2ad682af7fa24ef9f0942a41c89959341a16ba7b616475e1c5ab8e563b9b96
002b792b9a85100d89e47706c29cf1fd355d9727 gui: decouple WalletModel from RPCExecutor (furszy)
Pull request description:
A more comprehensive fix for the issue described in #837.
Since the `WalletModel` class is unavailable when compiling without wallet support
`(-DENABLE_WALLET=0)`, the RPC executor class should not be coupled to it.
This decoupling ensures GUI compatibility with builds that omit wallet support.
This also drops an extra `#ifdef ENABLE_WALLET` block which is always good.
ACKs for top commit:
w0xlt:
Code Review ACK 002b792b9a
pablomartin4btc:
tACK 002b792b9a85100d89e47706c29cf1fd355d9727
BrandonOdiwuor:
tACK 002b792b9a85100d89e47706c29cf1fd355d9727
hebasto:
ACK 002b792b9a85100d89e47706c29cf1fd355d9727, I have reviewed the code and it looks OK.
Tree-SHA512: a8e6b7e9d88dd8e0ff5e2d0de91be2f85fd0559265267d3bf6cae5a37606cf1ab6bc7415d5817a11006008de362f2ca3557ba772b4e1bd9fbef5f564be3b53bb
Since the `WalletModel` class is unavailable when compiling
without wallet support `(-DENABLE_WALLET=0)`, the RPC executor
class should not be coupled to it. This decoupling ensures GUI
compatibility with builds that omit wallet support.
189c987386a0da132d7ef076cdf539f9eb75fc3c Showing local addresses on the Node Window (Jadi)
a5d7aff867a3df9ac77664deed03e930e2636db0 net: Providing an interface for mapLocalHost (Jadi)
Pull request description:
This change adds a new row to the Node Window (debugwindow.ui)
under the Network section which shows the LocalAddresses.
fixes#564
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the change.
* Bug fixes are most welcome when they come with steps to reproduce or an
explanation of the potential issue as well as reasoning for the way the bug
was fixed.
* Features are welcome, but might be rejected due to design or scope issues.
If a feature is based on a lot of dependencies, contributors should first
consider building the system outside of Bitcoin Core, if possible.
* Refactoring changes are only accepted if they are required for a feature or
bug fix or otherwise improve developer experience significantly. For example,
most "code style" refactoring changes require a thorough explanation why they
are useful, what downsides they have and why they *significantly* improve
developer experience or avoid serious programming bugs. Note that code style
is often a subjective matter. Unless they are explicitly mentioned to be
preferred in the [developer notes](/doc/developer-notes.md), stylistic code
changes are usually rejected.
-->
<!--
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
ACKs for top commit:
pablomartin4btc:
re-ACK 189c987386a0da132d7ef076cdf539f9eb75fc3c
furszy:
utACK 189c987
Tree-SHA512: 93f201bc6d21d81b27b87be050a447b841f01e3efb69b9eca2cc7af103023d7cd69eb5e16e2875855573ef51a5bf74a6ee6028636c1b6798cb4bb11567cb4996
41a1a8615dd48fdd9811b9824c49ceb934c6375e gui: Hide peers details (@RandyMcMillan)
Pull request description:
Add a close (X) button to the Peers Detail panel.
Reuse the same icon used in the Console Tab.
The close button deselects the peer highlighted
in the PeerTableView and hides the detail panel.
fixes#485
Co-authored-by: @w0xlt <w0xlt@users.noreply.github.com>
ACKs for top commit:
pablomartin4btc:
re ACK 41a1a8615dd48fdd9811b9824c49ceb934c6375e
hebasto:
ACK 41a1a8615dd48fdd9811b9824c49ceb934c6375e, tested on Ubuntu 23.10.
Tree-SHA512: fc692891eec61bd1e6878f2433b478de3c69bf0b3ce3471f2faafda6f63d371e2cc125ae8290fd2ac3e4d8659031b79d85665318cfc5a9481e967ef99d245f9c
There are no changes to behavior. Changes in this commit are all additions, and
are easiest to review using "git diff -U0 --word-diff-regex=." options.
Motivation for this change is to keep util functions with really generic names
like "Split" and "Join" out of the global namespace so it is easier to see
where these functions are defined, and so they don't interfere with function
overloading, especially since the util library is a dependency of the kernel
library and intended to be used with external code.
Add a close (X) button to the Peers Detail panel.
Reuse the same icon used in the Console Tab.
The close button deselects the peer highlighted
in the PeerTableView and hides the detail panel.
This PR addresses issue #485:
Co-authored-by: @w0xlt <w0xlt@users.noreply.github.com>
During shutdown, already queue events dispatched from the backend such
'numConnectionsChanged' and 'networkActiveChanged' could try to access
the clientModel object, which might not exist because we manually delete
it inside 'BitcoinApplication::requestShutdown()'.
9d37886a3b6ce24f4a4a05193eb0d071655a8457 gui: Update Node window title with chain type (pablomartin4btc)
Pull request description:
It fixes#544.
Enhance the Node window title by appending the chain type to it, except for the `mainnet`, mirroring the behavior in the main window.
There was also some [interest](https://github.com/bitcoin-core/gui/issues/78#issuecomment-695755972) on this while discussing network switching.
ACKs for top commit:
MarnixCroes:
tACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457
hernanmarino:
tACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457
BrandonOdiwuor:
tested ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457
alfonsoromanz:
Tested ACK 9d37886a3b
kristapsk:
ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457
hebasto:
ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457, tested on Ubuntu 23.10.
Tree-SHA512: 8c34c4586bd59b1c522662e8aa0726dccc8f12e020f7a6a1af5200a29e5817e1c51e0f467c7923041fc41535ea093c3e0dd787befbbcc84d6b9f7ff0d969db04
99c0eb9701e71f16aa360a420b7e4851d5b92510 Fix RPCConsole wallet selection (John Moffett)
Pull request description:
If a user opens multiple wallets in the GUI from the menu bar, the last one opened is the active one in the main window. However, For the RPC Console window, the _first_ one opened is active. This can be confusing, as wallet RPC commands may be sent to a wallet the user didn't intend.
This PR makes the RPC Console switch to the wallet just opened / restored / created from the menu bar, which is how the main GUI now works.
Similar to https://github.com/bitcoin-core/gui/pull/665 and specifically requested [in a comment](https://github.com/bitcoin-core/gui/pull/665#issuecomment-1270003660).
ACKs for top commit:
luke-jr:
utACK 99c0eb9701e71f16aa360a420b7e4851d5b92510
hebasto:
ACK 99c0eb9701e71f16aa360a420b7e4851d5b92510, tested on Ubuntu 23.04.
Tree-SHA512: d5e5acdaa114130ad4d27fd3f25393bc8d02d92b5001cd39352601d04283cdad3bd62c4da6d369c69764e3b188e9cd3e83152c00b09bd42966082ad09037c328
If a user opens multiple wallets in the GUI from the
menu bar, the last one opened is the active one in
the main window. However, For the RPC Console window,
the _first_ one opened is active. This can be
confusing, as wallet RPC commands may be sent to a
wallet the user didn't intend.
This commit makes the RPC Console switch to the wallet
opened from the menu bar.
Since the kernel library no longer depends on the system file, move it
to the common library instead in accordance to the diagram in
doc/design/libraries.md.
This commit effectively moves the definition of these constants
out of the chainparamsbase to their own file.
Using the ChainType enums provides better type safety compared to
passing around strings.
The commit is part of an ongoing effort to decouple the libbitcoinkernel
library from the ArgsManager and other functionality that should not be
part of the kernel library.
The previous code visualized the `CSubNet` object as string, then
parsed that string back to `CSubNet`. This is sub-optimal given that
the original `CSubNet` object can be used directly instead.
This avoids calling `LookupSubNet()` from the GUI.
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
In certain circumstances, the GUI console will display
the message 'Executing command without any wallet' when
it is, in fact, using the default wallet.
In RPC calls, if no wallet is explicitly selected and
there is exactly one wallet loaded, the default is to
act on that loaded wallet.
The GUI console acts that way in reality, but
erroneously reports that it's not acting on any
particular wallet.
During connection setup for a peer, getpeerinfo returns "version": 0, "subver": ""
and the GUI Peers window displays 0 and an empty field, respectively.
Give these fields the same behavior as the other fields in the GUI Peers window:
display the fallback value in src/qt/forms/debugwindow.ui (i.e. "N/A") until a
valid result is available after the peer connection completes.
e3daecae0333e5474b54f64619ae6f512b683787 scripted-diff: replace deprecated Q_OS_MAC with Q_OS_MACOS (João Barbosa)
Pull request description:
`Q_OS_MAC` is deprecated but it is also defined when Qt is configured with `-xplatform macx-ios-clang`, and currently it guards some features not available on iOS, like `QProcess`.
ACKs for top commit:
jarolrod:
tACK e3daecae0333e5474b54f64619ae6f512b683787
hebasto:
ACK e3daecae0333e5474b54f64619ae6f512b683787.
Tree-SHA512: 17b4b891c70f027f6a420be830e61bd87fde5297a4473a5b122e4e34bdf83141635bd5cf5143efe95a0dd6f8cf50bc67a2de6cbfed7956952369587c74ece225
fa9af218780b7960d756db80c57222e5bf2137b1 scripted-diff: Use getInt<T> over get_int/get_int64 (MacroFake)
Pull request description:
Seems better to see the return type directly and be able to modify it easier, as the return type is used for exceptions (in-range checking and parsing feedback).
ACKs for top commit:
fanquake:
ACK fa9af218780b7960d756db80c57222e5bf2137b1
Tree-SHA512: 284aa2527d0f663ca01550115025c9c64c787531d595f866c718f6ad09b9b0cac1e683a7d77f8009b75de990fd37166b44063ffa83fba8a04e9a31600b4c2725
