fb1b1e0f3eae32b087347889ed7502b7f2c48549 qt: Save/restore column sizes of the tables in the Peers tab (Hennadii Stepanov)
Pull request description:
ACKs for top commit:
jonatack:
ACK fb1b1e0f3eae32b087347889ed7502b7f2c48549 code review, debug-built and tested
jarolrod:
ACK fb1b1e0f3eae32b087347889ed7502b7f2c48549
Tree-SHA512: f93495ecd13e4202aba61b407fffbeec855f5b0c1cc027197c78edddd7d11c87ebdb0fcb1daac242f0407323b31f4e7e0313bd76113a5241e4c868a8829af20a
8b419b5163d41c0caef7b9fa6d3008b73917e8ee qt: make console buttons look clickable (Jarol Rodriguez)
Pull request description:
On master, for macOS, the console buttons' hitboxes are quite small. This makes clicking on the button with your mouse a little more tedious than it should be. The Issue is related to recent versions of Qt (>5.9.8) not playing so nice on macOS when there are "incorrect" `width` and `height` values set for a `QPushButton` (here is another example: https://github.com/bitcoin-core/gui/pull/319#pullrequestreview-652907740).
This fixes this small hitbox issue by converting the buttons from `QPushButton` to `QToolButton`, which in turn makes the buttons look explicitly clickable. This approach was chosen as it helps us avoid having to play around with `width` and `height` values until we find values that play nice with macOS and look good on Linux & Windows. Also, `QToolButton` is an appropriate class for these buttons.
Per [Qt Docs](https://doc.qt.io/qt-5/qtoolbutton.html#details):
> A tool button is a special button that provides quick-access to specific commands or options. As opposed to a normal command button, a tool button usually doesn't show a text label, but shows an icon instead.
Since we are changing the type of the buttons, we need to change the respective actions connection logic in `rpcconsole`. Instead of plugging in `QToolButton`, we abstract it to the base class: `QAbstractButton`.
per [Qt Dev Notes](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Developer-Notes-for-Qt-Code#inherited-signals-and-slot)
> Use base class functions as this makes the code more general, e.g., use QAbstractButton::clicked instead of QPushButton::clicked.
While here, we also update the size of the icons to `22x22` to be consistent with other tool buttons.
**macOS: Master vs PR:**
| Master | PR |
| ----------- | ----------- |
|  |  |
**Linux: Master vs PR:**
| Master | PR |
| ----------- | ----------- |
|  |  |
ACKs for top commit:
hebasto:
ACK 8b419b5163d41c0caef7b9fa6d3008b73917e8ee, tested on Linux Mint 20.1 (Qt 5.12.8).
promag:
Tested ACK 8b419b5163d41c0caef7b9fa6d3008b73917e8ee on macOS Big Sur M1, this drops only relevant usages to `flat` buttons.
Tree-SHA512: 3f3cdcbe83398136a1d1ee8fc2835be8681f2ed39e79db1e939cab6a00a779f528343d54992807a845cc84d9ef13591affb7a6dbca9e5753a2b8665b0af4d611
38eb37c0bd29b4cb825de905e8eec87636a5221b qt, rpc: Do not accept command while executing another one (Hennadii Stepanov)
0c32b9c5273a4933bda90aa9eb9b7eace6dcaa14 qt, rpc: Accept stop RPC even another command is executing (Hennadii Stepanov)
ccf790287c53edbc7b18983e07f520823436c003 qt, rpc, refactor: Return early in RPCConsole::on_lineEdit_returnPressed (Hennadii Stepanov)
5b9c8c9cdd8e12d2e477840df9d6ab809a613c12 qt, rpc: Add "Executing…" message (Hennadii Stepanov)
Pull request description:
On master (3f512f3d563954547061ee743648b57a900cbe04) it is possible to enter another command while the current command is still being executed. That makes a mess in the output.
With this PR:
Some previous context: https://github.com/bitcoin-core/gui/pull/59#issuecomment-715275185
---
It is still possible to enter and execute the `stop` command any time.
ACKs for top commit:
jarolrod:
ACK 38eb37c
promag:
Tested ACK 38eb37c0bd29b4cb825de905e8eec87636a5221b.
Tree-SHA512: 2b37a4b6838bf586b1b5c878192106721f713caeb6252514a6540356aab898986396e0777e73891d331b1be797a4926c20d3f9f38ba2c984ea90d55b0c34f664
0f3d955a38fe59a4e8cc4bf9f4442e6e1fd8bcbb qt: Make RPC console welcome message translation-friendly (Hennadii Stepanov)
Pull request description:
The best practice is do not split a translatable multi-line message into single lines. This helps translators to follow the context.
ACKs for top commit:
jarolrod:
re-ACK 0f3d955a38fe59a4e8cc4bf9f4442e6e1fd8bcbb
Tree-SHA512: 30911ff3a972a7787804bb8b27d0b77bfff15939bb478c199261866bfb55d9acd12ab4d44b8b9fc1d4898222cabc4007cc897f9b65728924d121f31e914c44ac
d29ea72393ac1d9b32a6976062e9c9fb75876295 gui: Add access to the Peers tab from the network icon (Hennadii Stepanov)
Pull request description:
This PR add a small context menu to the network activity icon that provides an access to the Peers tab:
Closes#93.
ACKs for top commit:
Sjors:
re-ACK d29ea72393ac1d9b32a6976062e9c9fb75876295
kristapsk:
re-ACK d29ea72393ac1d9b32a6976062e9c9fb75876295
promag:
Code review ACK d29ea72393ac1d9b32a6976062e9c9fb75876295.
Tree-SHA512: dd871415fe514a19c6a22100d58f31954d9e55b80585d5a3f26e17a8d51dadf912441786fc0d23beabd812f1b501658fec1dbe345cd41beae5832a8eda890f77
df4c81fda490e556775f6006f3d4f7d54f61073b English translations update (Hennadii Stepanov)
bfb53ddda9a31bbeb8697e65ff9987e61dac136f scripted-diff: Fix ellipsis after pr20773 (Hennadii Stepanov)
Pull request description:
Update for Transifex.
After changing translator comments in #332 this update will show if Transifex triggers strings to be re-translated.
ACKs for top commit:
laanwj:
ACK df4c81fda490e556775f6006f3d4f7d54f61073b
jarolrod:
ACK df4c81fda490e556775f6006f3d4f7d54f61073b
Tree-SHA512: 1e54812bc04db6ae39e0b4d735b220ed8730a9941b17a0a2d09e21bcdd08e829adba86c35cf43c9be5e492ccb13e53a90149fcd7d6c0f5fdd022b978a1ff785c
a02c970eb001b456d74ddc30750fe8b55348ddac qt, refactor: Revert explicit including QStringBuilder (Hennadii Stepanov)
3fd3a0fc87a81d42755246830124833e9ca3f0a9 qt, build: Optimize string concatenation (Hennadii Stepanov)
Pull request description:
From [Qt docs](https://doc.qt.io/qt-5/qstring.html#more-efficient-string-construction):
> ... multiple uses of the \[`QString`\] '+' operator usually means multiple memory allocations. When concatenating n substrings, where n > 2, there can be as many as n - 1 calls to the memory allocator.
With this PR
> ... the '+' will automatically be performed as the `QStringBuilder` '%' everywhere.
The change in the `src/Makefile.qt.include` file does not justify submitting this PR into the main repo, IMHO.
ACKs for top commit:
laanwj:
Code review ACK a02c970eb001b456d74ddc30750fe8b55348ddac
Talkless:
utACK a02c970eb001b456d74ddc30750fe8b55348ddac, built successfully on Debian Sid with Qt 5.15.2, but did not check if any displayed strings are "wrong" after refactoring.
jarolrod:
ACK a02c970eb001b456d74ddc30750fe8b55348ddac
Tree-SHA512: cbb476ee96f27c3bd6e125efab74d8bf24bbdb4c30576b3feea45e203405f3bf5b497dd7d3e11361fc825fcbf4b893b152921a9efdeaf73b42d1865d85f0ae84
056d869571 refactor: use testdummy in feature_mnehf functional test, removed useless checks (Konstantin Akimov)
0351469bb5 refactor: removed duplicated meaningless condition from Check mnhftx (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Using mn_rr in feature_mnehf.py is a blocker for burying mn_rr fork.
## What was done?
Removed useless conditions, uses testdummy fork instead mn_rr in ehf functional test.
## How Has This Been Tested?
Run it `test/functional/feature_mnehf.py`
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK 056d869571
PastaPastaPasta:
utACK 056d869571
Tree-SHA512: aa016dc21fee3afdb5a4172e757cdd0b92867c23eb5241287f641d3c97d363ab1c46eba06423481ecc7ba05f6c0010b65eadfe77d4a1fd6074706cc6f3a71309
8320e0ca8e merge bitcoin#27411: Restrict self-advertisements with privacy networks to avoid fingerprinting (Kittywhiskers Van Gogh)
1376289b11 merge bitcoin#27467: skip netgroup diversity follow-up (Kittywhiskers Van Gogh)
a52b3a3bf0 merge bitcoin#27374: skip netgroup diversity of new connections for tor/i2p/cjdns (Kittywhiskers Van Gogh)
ab11e0f998 merge bitcoin#27324: bitcoin#27257 follow-ups (Kittywhiskers Van Gogh)
9023dd25af merge bitcoin#27257: End friendship of CNode, CConnman and ConnmanTestMsg (Kittywhiskers Van Gogh)
3465df2689 merge bitcoin#27264: Improve diversification of new connections (Kittywhiskers Van Gogh)
d3f5b3881b merge bitcoin#26888: simplify the call to vProcessMsg.splice() (Kittywhiskers Van Gogh)
d9e56f3e78 merge bitcoin#25962: Add CNodeOptions and increase constness (Kittywhiskers Van Gogh)
79e67fd96a merge bitcoin#25814: simplify GetLocalAddress() (Kittywhiskers Van Gogh)
6d4945418a partial bitcoin#25472: Increase MS Visual Studio minimum version (Kittywhiskers Van Gogh)
54bb3a438f merge bitcoin#25500: Move inbound eviction logic to its own translation unit (Kittywhiskers Van Gogh)
b50febc0f0 merge bitcoin#24531: Use designated initializers (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependent on https://github.com/dashpay/dash/pull/6254
* When backporting [bitcoin#27411](https://github.com/bitcoin/bitcoin/pull/27411), the `CNetAddr*` variant of `GetLocal()` was not removed (upstream it was replaced by the `CNode&` variant with additional checks that rely on fields in `CNode`) as `CActiveMasternodeManager` relies on `GetLocal()` to detect a valid external address.
* While it can also rely on other nodes to determine that, removing code that tests against a well-known public address would increase the number of reported failures esp. if the checks are run _before_ the node has a chance to connect to any peers.
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 8320e0ca8e
PastaPastaPasta:
utACK 8320e0ca8e
Tree-SHA512: 1d02bc33c8d62c392960d4dd044edf3de08515a5e8c8794d95cd95e9654da91b20e7290436cf9c79b0ea8dbd42b27dcc61c8eb17e573902574d7b281b8874584
The old `GetLocal()` has been moved to `masternode/node.cpp` due to its
use in determining a node's external address. We don't want the old
variant to be used otherwise so we'll move it out of `net.{cpp,h}` for
good measure.
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
cc998abec1 fmt: apply `clang-format-diff.py` suggestions (Kittywhiskers Van Gogh)
0401c581eb stats: `const`-ify variables and arguments (Kittywhiskers Van Gogh)
9f96723774 stats: stop using error codes, switch over to `bool` (Kittywhiskers Van Gogh)
1a81979c1e stats: initialize socket after we have a valid socket address (Kittywhiskers Van Gogh)
dbbfc8d766 stats: use `Socks` wrapper, use `CService` to generate our `sockaddr` (Kittywhiskers Van Gogh)
2def905044 stats: move init logic into constructor (Kittywhiskers Van Gogh)
4bc727cd6c stats: clean up randomization code, move `FastRandomContext` inward (Kittywhiskers Van Gogh)
840241eefd stats: cleanup error logging, improve code sanity (Kittywhiskers Van Gogh)
85890ddb13 docs: add copyright notice to source file, update notice in header (Kittywhiskers Van Gogh)
a9d1b1494d stats: move `_StatsdClientData` variables into `StatsdClient` (Kittywhiskers Van Gogh)
30c30c1397 stats: fetch all arguments needed when constructing `g_stats_client` (Kittywhiskers Van Gogh)
5133d88415 stats: s/statsClient/g_stats_client/g (Kittywhiskers Van Gogh)
f81951dd00 stats: make `statsClient` a `std::unique_ptr`, denote as global variable (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
Support for transmitting stats to a Statsd server has been courtesy of Statoshi ([repo](https://github.com/jlopp/statoshi)), implemented Dec, 2020 by [dash#2515](https://github.com/dashpay/dash/pull/2515) but since then, it hasn't gotten much attention aside from benefiting from codebase-wide changes and the occasional compiler appeasement. This pull request aims to give our statistics code some TLC.
Changes include:
* Limiting initialization to solely during construction and moving the responsibility of fetching arguments outside of `statsd::StatsdClient`.
* Using the RAII `Socks` wrapper as early as possible (we still need to construct a raw socket ourselves but this is done in the initializer and control is moved to the wrapper and everywhere else, the wrapper is used)
* Utilizing existing networking code to generate the socket address
* This lets us trivially allow IPv6 connections as the responsibility to construct it safely is moved to `CService`.
* Using `std::string` and our string manipulation capabilities (replacing `snprintf` with `strprintf`), replacing platform-specific types (replacing `short` with `uint16_t`).
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK [cc998ab](cc998abec1)
UdjinM6:
utACK cc998abec1
Tree-SHA512: 433c92160d6ac7ebb8582ada3cbb65ead7913618266b773619a528c90dfe0e286aafa46dc3b0bca62f246938e5948a732080e2cddba942d3627f007ca6efcc1f
6c7335e002 merge bitcoin#24331: Revert back `MoveFileExW` call for MinGW-w64 (Kittywhiskers Van Gogh)
15e794bdd8 merge bitcoin#24238: use arc4random on OpenBSD (Kittywhiskers Van Gogh)
e039aecbdc merge bitcoin#23936: Add and use EnsureArgsman helper (Kittywhiskers Van Gogh)
b4bfacfd24 merge bitcoin#23769: Disallow copies of CChain (Kittywhiskers Van Gogh)
5b66688160 merge bitcoin#22362: Drop only invalid entries when reading banlist.json (Kittywhiskers Van Gogh)
109c963f6a merge bitcoin#23175: Add CJDNS network to -addrinfo and -netinfo (Kittywhiskers Van Gogh)
d57c96ea37 merge bitcoin#23398: add return message to savemempool RPC (Kittywhiskers Van Gogh)
22e59fb464 merge bitcoin#23054: Use C++11 member initializer in CTxMemPoolEntry (Kittywhiskers Van Gogh)
d158063b6d merge bitcoin#22653: Rename JoinErrors and re-use it (Kittywhiskers Van Gogh)
e24324d266 merge bitcoin#22221: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested (Kittywhiskers Van Gogh)
68657efc03 merge bitcoin#22141: net processing: Remove hash and fValidatedHeaders from QueuedBlock (Kittywhiskers Van Gogh)
c0e6792e27 merge bitcoin#20018: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* When backporting [bitcoin#23054](https://github.com/bitcoin/bitcoin/pull/23054), `sigOpCount` and `nSigOpCountWithAncestors` were switched from `unsigned int` to `int64_t`. This change was done to prevent integer narrowing when accepting the `int64_t` taken from the constructor.
This isn't a problem upstream as the same changes were as part of [bitcoin#8149](https://github.com/bitcoin/bitcoin/pull/8149/files#diff-8a2230436880b65a205db9299ab2e4e4adb1d4069146791b5db566f3fb752adaL90-L107), which was omitted but the type changes remain valid as sigop count cannot be a negative number.
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK 6c7335e002
UdjinM6:
utACK 6c7335e002
Tree-SHA512: 29cae42dd82235305d3558562bae346170e742ce0b65897e396b331294b39cad0dd831fa9a09b34780a67844e55292e5b4e784cf544a894cc3f8897afe617ca1
As creating the socket is now the last step, we don't need
`m_init` anymore. We can just see if a socket is successfully
constructed and take that as our validity indicator.
We'll also move it out of the inner `send` function so we can bail out
before we bother with all the string processing and manipulation.
We can skip the computationally expensive dice-roll if our sample rate
is zero as that means we should never send it. Also get rid of the
`FastRandomContext::operator()` that isn't used anywhere else in the
codebase.
- Use `uint16_t` instead of `short`, `int64_t` instead of `size_t`
- Get rid of the `errmsg` buffer and use `LogPrintf` to report errors
- Use `strprintf` instead of `snprintf`
- Rephrase networking error logs to allow inclusion of error strings
0c5179462e test: rework feature_governance_cl.py (UdjinM6)
11ac0819da feat: bump_mocktime also bumps schedulers now (UdjinM6)
1937f503fe feat: regtest-only: do not auto-reset mnsync after 1h (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
1. The original idea behind forced reset was to help desktop clients which go into sleep/hibernation mode from time to time to sync with no issues once they are online again. For regtest however it doesn't do anything good and only causes issues.
2. We rely on schedulers a lot, bumping them should let nodes behave more like on a real network.
3. Forcing mnsync to skip governance votes doesn't always work as we'd expect cause we don't control connection creation. To make it more deterministic create a proposal that should get into one superblock only. This way both the proposal and the trigger will expire/be deleted after another superblock meaning we can be 100% sure the isolated node never gets any of them and can only sync thanks to chainlock.
## What was done?
## How Has This Been Tested?
run tests
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 0c5179462e
Tree-SHA512: 9a40eeaba880f3f27f86736e92afa19a4ecb2a2d157bc42b65dd9da0d4109c9cd1d83a5abdf1ec16be2f8a8b31821fb700a7f0d2265c094fd4fdff7f18bc6ec7
57fb0874ef feat: broadcast dsq messages using the inventory system (pasta)
Pull request description:
## Issue being fixed or feature implemented
DSQ messages are 142 bytes.
Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.
## What was done?
Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a `getdata` message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%
## How Has This Been Tested?
Has not been; @UdjinM6 especially please review well :)
## Breaking Changes
Does introduce a new protocol version, but in a backwards compatible way. I don't think this would need to be delayed to v22 for any reason.
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
Top commit has no ACKs.
Tree-SHA512: 3dc39a339cba29d8cf207cec76ecace5ad0e11d1892ca0f65f9253a2b1d90313da21c6c178c2476756c5566ece0fab777006cd609b7984df906a9206c25d921d
DSQ messages are 142 bytes.
Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.
Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a `getdata` message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%
23812555b1 fix: possible deadlock during calculation of signals for historical blocks during re-index (Konstantin Akimov)
1087489fd4 feat: bury v20 deployment (Konstantin Akimov)
64cedb30bd feat: actually test something EHF unit tests (Konstantin Akimov)
762a808b8c chore: drop irrelevant bip9 code from feature_llmq_rotation.py (Konstantin Akimov)
7735631aad fix: remove v20 from test feature_llmq_evo as far as mn_rr used (Konstantin Akimov)
ca83b26815 fix: crash in CreditPool: it meant to check that DIP0003 is activated (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
V20 is activated on mainnet: time to bury it!
https://github.com/dashpay/dash/issues/6186
## What was done?
Hard-fork v20 is buried and it requires to implement multiple fixes, simplifications, refactoring:
- some tests for EHF moved from functional tests to unit tests
- fixed crash in Credit Pool if DIP3 is not activated yet
- added a requirement for v20 activation for `CMNHFManager::GetSignalsStage`
- removed useless code from functional test feature_llmq_rotation
- renamed variables "v20" to "mn_rr" in feature_llmq_evo.py so far as actually used fork is mn_rr
## How Has This Been Tested?
Some unit and functional tests to succeed.
Done reindex (just in case):
src/qt/dash-qt -reindex -assumevalid=0
src/qt/dash-qt -reindex -assumevalid=0 -testnet
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
ACK 23812555b1
PastaPastaPasta:
utACK 23812555b1
Tree-SHA512: eec35745baa695f3f286d39b6a61fa0a9f34820b13d1dd4cfbd1efe707850283892c39bf7fe49c49c812e0c02465d64df11480b3f12aa7f21b59a71eeae7260e
