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.
fa83fb31619c19a1a30b4181486601a944941b16 wallet: Use steady clock to calculate number of derive iterations (MarcoFalke)
fa2c099cec1780a498e198750be0cf5bf0ca315a wallet: Use steady clock to measure scanning duration (MarcoFalke)
fa976218044f3ff244abbd797b183a1408375c74 qt: Use steady clock to throttle GUI notifications (MarcoFalke)
fa1d8044abc2cd0f149a2d526b3b03441443cdb0 test: Use steady clock in index tests (MarcoFalke)
fa454dcb20b9e7943cc25e6eeea72912b5f1c7b5 net: Use steady clock in InterruptibleRecv (MarcoFalke)
Pull request description:
`GetTimeMillis` has multiple issues:
* It doesn't denote the underlying clock type
* It isn't type-safe
* It is used incorrectly in places that should use a steady clock
Fix all issues here.
ACKs for top commit:
willcl-ark:
ACK fa83fb3161
martinus:
Code review ACK fa83fb3161, also ran all tests. All usages of the steady_clock are just for duration measurements, so the change to a different epoch is ok.
Tree-SHA512: 5ec4fede8c7f97e2e08863c011856e8304f16ba30a68fdeb42f96a50a04961092cbe46ccf9ea6ac99ff5203c09f9e0924eb483eb38d7df0759addc85116c8a9f
This is an extraction of ArgsManager related functions from util/system
into their own common file.
Config file related functions are moved to common/config.cpp.
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager. The ArgsManager belongs
into the common library, since the kernel library should not depend on
it. See doc/design/libraries.md for more information on this rationale.
c9d548c91fb12fba516dee896f1f97692cfa2104 net: remove CService::ToStringPort() (Vasil Dimov)
fd4f0f41e915d99c9b0eac1afd21c5628222e368 gui: simplify OptionsDialog::updateDefaultProxyNets() (Vasil Dimov)
96c791dd20fea54c17d224000dee677bc158f66a net: remove CService::ToString() use ToStringAddrPort() instead (Vasil Dimov)
944a9de08a00f8273e73cd28b40e46cc0eb0bad1 net: remove CNetAddr::ToString() and use ToStringAddr() instead (Vasil Dimov)
043b9de59aec88ae5e29daac7dc2a8b51a9414ce scripted-diff: rename ToStringIP[Port]() to ToStringAddr[Port]() (Vasil Dimov)
Pull request description:
Before this PR we had the somewhat confusing combination of methods:
`CNetAddr::ToStringIP()`
`CNetAddr::ToString()` (duplicate of the above)
`CService::ToStringIPPort()`
`CService::ToString()` (duplicate of the above, overrides a non-virtual method from `CNetAddr`)
`CService::ToStringPort()`
Avoid [overriding non-virtual methods](https://github.com/bitcoin/bitcoin/pull/25349/#issuecomment-1185226396).
"IP" stands for "Internet Protocol" and while sometimes "IP addresses" are called just "IPs", it is incorrect to call Tor or I2P addresses "IPs". Thus use "Addr" instead of "IP".
Change the above to:
`CNetAddr::ToStringAddr()`
`CService::ToStringAddrPort()`
The changes touch a lot of files, but are mostly mechanical.
ACKs for top commit:
sipa:
utACK c9d548c91fb12fba516dee896f1f97692cfa2104
achow101:
ACK c9d548c91fb12fba516dee896f1f97692cfa2104
jonatack:
re-ACK c9d548c91fb12fba516dee896f1f97692cfa2104 only change since my previous reviews is rebase, but as a sanity check rebased to current master and at each commit quickly re-reviewed and re-verified clean build and green unit tests
LarryRuane:
ACK c9d548c91fb12fba516dee896f1f97692cfa2104
Tree-SHA512: 633fb044bdecf9f551b5e3314c385bf10e2b78e8027dc51ec324b66b018da35e5b01f3fbe6295bbc455ea1bcd1a3629de1918d28de510693afaf6a52693f2157
7b7cd112444b996a8ae6a6edfed00bcee67546c8 clang-tidy, qt: Force checks for headers in `src/qt` (Hennadii Stepanov)
69eacf2c5ee1c84e92153b525fd4302aec0f5f2a clang-tidy, qt: Fix `modernize-use-default-member-init` in headers (Hennadii Stepanov)
Pull request description:
This PR split from bitcoin/bitcoin#26705 and contains only changes in `src/qt`.
Effectively, it fixes the clang-tidy's `modernize-use-default-member-init` errors, and forces clang-tidy checks for all headers in the `src/qt` directory.
ACKs for top commit:
jarolrod:
ACK 7b7cd112444b996a8ae6a6edfed00bcee67546c8
Tree-SHA512: 79525bb0f31ae7cad88c781e55091a21467c0485ddc1ed03ad62e051480fda3b3710619ea11af480437edba3c6e038f7c40edc6b373e3a37408c006d11b34686
"IP" stands for "Internet Protocol".
"IP address" is sometimes shortened to just "IP" or "address".
However, Tor or I2P addresses are not "IP addresses", nor "IPs".
Thus, use "Addr" instead of "IP" for addresses that could be IP, Tor or
I2P addresses:
`CService::ToStringIPPort()` -> `CService::ToStringAddrPort()`
`CNetAddr::ToStringIP()` -> `CNetAddr::ToStringAddr()`
-BEGIN VERIFY SCRIPT-
sed -i 's/ToStringIPPort/ToStringAddrPort/g' -- $(git grep -l ToStringIPPort src)
sed -i 's/ToStringIP/ToStringAddr/g' -- $(git grep -l ToStringIP src)
-END VERIFY SCRIPT-
This makes a number of changes:
- Get rid of the verification_progress argument in the node interface
NotifyHeaderTip (it was always 0.0).
- Instead of passing a CBlockIndex* in the UI interface's NotifyHeaderTip,
send separate height, timestamp fields. This is becuase in headers presync,
no actual CBlockIndex object is available.
- Add a bool presync argument to both of the above, to identify signals
pertaining to the first headers sync phase.
0eea83a85ec6b215d44facc2b16ee1b035275a6b scripted-diff: rename `proxyType` to `Proxy` (Vasil Dimov)
e53a8505dbb6f9deaae8ac82793a4fb760a1e0a6 net: respect -onlynet= when making outbound connections (Vasil Dimov)
Pull request description:
Do not make outbound connections to hosts which belong to a network
which is restricted by `-onlynet`.
This applies to hosts that are automatically chosen to connect to and to
anchors.
This does not apply to hosts given to `-connect`, `-addnode`,
`addnode` RPC, dns seeds, `-seednode`.
Fixes https://github.com/bitcoin/bitcoin/issues/13378
Fixes https://github.com/bitcoin/bitcoin/issues/22647
Supersedes https://github.com/bitcoin/bitcoin/pull/22651
ACKs for top commit:
naumenkogs:
utACK 0eea83a85ec6b215d44facc2b16ee1b035275a6b
prayank23:
reACK 0eea83a85e
jonatack:
ACK 0eea83a85ec6b215d44facc2b16ee1b035275a6b code review, rebased to master, debug built, and did some manual testing with various config options on signet
Tree-SHA512: 37d68b449dd6d2715843fc84d85f48fa2508be40ea105a7f4a28443b318d0b6bd39e3b2ca2a6186f2913836adf08d91038a8b142928e1282130f39ac81aa741b
5a4a15d2b4456272fd8aa080195f40a09576ae01 qt, refactor: Drop no longer used PeerTableModel::getRowByNodeId func (Hennadii Stepanov)
9a9f180df0d51396fee2468681df6dd935b0248e qt, refactor: Drop no longer used PeerTableModel::sort function (Hennadii Stepanov)
778a64af209e4fa692a3aca8376ba1bd5e1af881 qt: Use PeerTableSortProxy for sorting peer table (Hennadii Stepanov)
df2d165ba9e0acc53f36a326f68f57ad9c297872 qt: Add peertablesortproxy module (Hennadii Stepanov)
Pull request description:
The "Peers" table in the "Node" window does not hold multiple selection after sorting.
This PR introduces a `QSortFilterProxyModel` subclass, that is a standard Qt [practice](https://doc.qt.io/qt-5/model-view-programming.html#custom-sorting-models) for such cases.
Now the sorting code is encapsulated into the dedicated Qt class, and we do not need to maintain it.
Fixes#283 (additionally).
---
On **master** (7ae86b3c6845873ca96650fc69beb4ae5285c801):
- rows are sorted by "Ping", and a selection is made
- rows are sorted by "NodeId", and the previous selection is _lost_
With **this PR**:
- rows are sorted by "Ping", and a selection is made
- rows are sorted by "NodeId", and the row are still selected
ACKs for top commit:
jarolrod:
re-ACK 5a4a15d2b4456272fd8aa080195f40a09576ae01, tested on macOS 11.2 Qt 5.15.2 after rebase
promag:
Tested ACK 5a4a15d2b4456272fd8aa080195f40a09576ae01.
Tree-SHA512: f81c1385892fbf1a46ffb98b42094ca1cc97da52114bbbc94fedb553899b1f18c26a349e186bba6e27922a89426bd61e8bc88b1f7832512dbe211b5f834e076e
Always lock the mutexes `cs_main` and `m_cached_tip_mutex` in
the same order: `cs_main`, `m_cached_tip_mutex`. Otherwise we may end up
in a deadlock.
`ClientModel::m_cached_tip_blocks` is protected by
`ClientModel::m_cached_tip_mutex`. There are two access paths that
lock the two mutexes in opposite order:
```
validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main
validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip()
ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost
...
qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex
```
and
```
qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex
qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash()
interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main
```
From `debug.log`:
```
POTENTIAL DEADLOCK DETECTED
Previous lock order was:
m_cs_chainstate validation.cpp:2851
(1) cs_main validation.cpp:2868
::mempool.cs validation.cpp:2868
(2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255
Current lock order is:
(2) m_cached_tip_mutex qt/clientmodel.cpp:119
(1) ::cs_main interfaces/node.cpp:200
```
The possible deadlock was introduced in
https://github.com/bitcoin/bitcoin/pull/17993
96cb597325f64cadb3cf43e2cdb3d7c1e2e49891 gui: Avoid redundant tx status updates (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
In `TransactionTablePriv::index`, avoid calling `interfaces::Wallet::tryGetTxStatus` if the status is up to date as of the most recent `NotifyBlockTip` notification. Store height from the most recent notification in a new `ClientModel::cachedNumBlocks` variable in order to check this.
This avoids floods of IPC traffic from `tryGetTxStatus` with #10102 when there are a lot of transactions. It might also make the GUI a little more efficient even when there is no IPC.
ACKs for top commit:
promag:
Code review ACK 96cb597325f64cadb3cf43e2cdb3d7c1e2e49891.
hebasto:
ACK 96cb597325f64cadb3cf43e2cdb3d7c1e2e49891
Tree-SHA512: fce597bf52a813ad4923110d0a39229ea09e1631e0d580ea18cffb09e58cdbb4b111a40a9a9270ff16d8163cd47b0bd9f1fe7e3a6c7ebb19198f049f8dd1aa46
In TransactionTablePriv::index, avoid calling
interfaces::Wallet::tryGetTxStatus if the status is up to date as of the most
recent NotifyBlockTip notification. Store height from the most recent
notification in a new ClientModel::cachedNumBlocks variable in order to check
this.
This avoids floods of IPC traffic from tryGetTxStatus with #10102 when there
are a lot of transactions. It might also make the GUI a little more efficient
even when there is no IPC.
This commit resolves the checkpoints -> validation -> checkpoints
cirular dependency by moving
`CheckPoints::GetLastCheckpoint(const CCheckpointData& data)` from
`checkpoints.cpp` to `validation.cpp`.
