8e08d005989c6b5f7f05e0a1e0ba84f544a76d01 qt: Use parent-child relation to manage lifetime of OptionsModel object (Hennadii Stepanov)
Pull request description:
Both `BitcoinApplication` and `OptionsModel` classes are derived from the `QObject` class, therefore a parent-child relation could be established to manage the lifetime of an `OptionsModel` object:
5236b2e267/src/qt/optionsmodel.cpp (L29-L30)
This PR does not change behavior.
ACKs for top commit:
jonasschnelli:
utACK 8e08d005989c6b5f7f05e0a1e0ba84f544a76d01
promag:
ACK 8e08d005989c6b5f7f05e0a1e0ba84f544a76d01.
Tree-SHA512: 0223dddf5ba28b0bfaefeda1b03b4ff95bf7e7d0c1e7b32368171e561813e22129f2a664f09279fa3b4fa63259b7680d55aa3fe66db9c7ae0039b7f529777ec3
a9d28afe23a94efdccc53f9f10716f3a0c9337eb qt: Display warnings as rich text (Hennadii Stepanov)
Pull request description:
On master (6621be53517d69ab855cee4a5978a44d6a133ba3), warnings that contain `<hr />` HTML tag are not displayed correctly:
Fixed:
ACKs for top commit:
jonasschnelli:
utACK a9d28afe23a94efdccc53f9f10716f3a0c9337eb
promag:
Code review ACK a9d28afe23a94efdccc53f9f10716f3a0c9337eb.
Tree-SHA512: ba5b3837d5f6ea15c3255a3120c9753fc58ee67a370c388556214048ab993c45be720af7cb8d43bb0f12088956cb78abc77546ed1fc691082880438072fe774b
faca73000fa8975c28f6be8be01957c1ae94ea62 ci: Install fixed version of clang-format for linters (MarcoFalke)
fa4695da4c69646b58a8fa0b6b30146bb234deb8 build: Sort Makefile.am after renaming file (MarcoFalke)
cccc2784a3bb10fa8e43be7e68207cafb12bd915 scripted-diff: Move ui_interface to the node lib (MarcoFalke)
fa72ca6a9d90d66012765b0043fd819698b94ba8 qt: Remove unused includes (MarcoFalke)
fac96e6450d595fe67168cb7afa7692da6cc9973 wallet: Do not include server symbols (MarcoFalke)
fa0f6c58c1c6d10f04c4e65a424cc51ebca50a8c Revert "Fix link error with --enable-debug" (MarcoFalke)
Pull request description:
This reverts a hacky workaround from commit b83cc0f, which only happens to work due to compiler optimizations. Then, it actually fixes the linker error.
The underlying problem is that the wallet includes symbols from the server (ui_interface), which usually results in linker failures. Though, in this specific case the linker failures have not been observed (unless `-O0`) because our compilers were smart enough to strip unused symbols.
Fix the underlying problem by creating a new header-only with the needed symbol and move ui_interface to node to clarify that this is part of libbitcoin_server.
ACKs for top commit:
Sjors:
ACK faca730
laanwj:
ACK faca73000fa8975c28f6be8be01957c1ae94ea62
hebasto:
re-ACK faca73000fa8975c28f6be8be01957c1ae94ea62, since the [previous](https://github.com/bitcoin/bitcoin/pull/19331#pullrequestreview-434420539) review:
Tree-SHA512: e9731f249425aaea50b6db5fc7622e10078cf006721bb87989cac190a2ff224412f6f8a7dd83efd018835302337611f5839e29e15bef366047ed591cef58dfb4
f5a3a5b9ab362c58fa424261f313aa9cf46d2a98 gui: Add close window shortcut (Miguel Herranz)
Pull request description:
CMD+W is the standard shortcut in macOS to close a window without
exiting the program.
This adds support to use the shortcut in both main and debug windows.
ACKs for top commit:
jonasschnelli:
Tested ACK f5a3a5b9ab362c58fa424261f313aa9cf46d2a98
hebasto:
ACK f5a3a5b9ab362c58fa424261f313aa9cf46d2a98, tested on Linux Mint 19.3 by manually opening available dialogs and sub-windows, and applying the `Ctrl+W` shortcut. Also tested with "Minimize on close" option enabled / disabled.
Tree-SHA512: 39851f6680cf97c334d5759c6f8597cb45685359417493ff8b0566672edbd32303fa15ac4260ec8ab5ea1458a600a329153014f25609e1db9cf399aa851ae2f9
8d75115844baefe5cad4d82ec8dce001dd16eb9c qt: Add privacy feature to Overview page (Hennadii Stepanov)
73d8ef72742ab9193e9e95158b26176bfaab3f66 qt: Add BitcoinUnits::formatWithPrivacy() function (Hennadii Stepanov)
Pull request description:
This PR allows to hide/reveal values on the Overviewpage by checking/unchecking Menu->Settings-> Mask Values
Closes#16407
Privacy mode is OFF (the default behavior):
Privacy mode is ON:
ACKs for top commit:
jonatack:
Tested ACK 8d75115
jonasschnelli:
Tested ACK 8d75115844baefe5cad4d82ec8dce001dd16eb9c
Tree-SHA512: 42f396d5bf0d343b306fb7e925f86f66b3fc3a257af370a812f4be181b5269298f9b23bd8a3ce25ab61de92908c4018d8c2dc8591d11bc58d79c4eb7206fc6ec
c4b574899abfa27f83c6948593838ed418c78f9c gui: Add Close All Wallets action (João Barbosa)
f30960adc02877267cb6efeb378962ed96628741 gui: Add closeAllWallets to WalletController (João Barbosa)
Pull request description:
This PR adds the action to close all wallets.
<img width="405" alt="Screenshot 2020-06-01 at 01 06 12" src="https://user-images.githubusercontent.com/3534524/83365986-25a8b980-a3a4-11ea-9613-24dcd8eaa55c.png">
ACKs for top commit:
jonasschnelli:
Tested ACK c4b574899abfa27f83c6948593838ed418c78f9c
Tree-SHA512: 049ad77ac79949fb55f6bde47b583fbf946f4bfaf3d56d768e85f813d814cff0fe326b700f7b5e383cda4af7b5666e13043a6aaeee3798a69fc94385d88ce809
b72925e7cea11522aca65580c136dbacb2753e83 lint: remove qt SIGNAL/SLOT lint (fanquake)
Pull request description:
I think we are past the point where we need to lint for this, the CPU
can probably be better utilized.
ACKs for top commit:
laanwj:
ACK b72925e7cea11522aca65580c136dbacb2753e83
Tree-SHA512: 3da6e4811cdd16ff64c7e26f641f7b24f0405cc86cec36666de58691d447eca8662c924df31c6c60b3523c13590bdc62205a3237b1b1794dd8cdef35519309b3
## Issue being fixed or feature implemented
Dead-code, useless conditions can be potential source of bug.
## What was done?
See each particular commit.
This particular commit "fix: check ptr in assert before usage" fixes
potential UB - `assert` is better than UB.
All other commits are not fixing any real issue, just to tidy-up code a
bit or to shut a potential warning.
## How Has This Been Tested?
Run unit/functional 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
- [x] I have assigned this pull request to a milestone
e7fc50681e99e3c726db2bc4d3d425ed8a0fc6b3 qt: Override BitcoinApplication::event() to handle QEvent::Quit (Hennadii Stepanov)
Pull request description:
bitcoin-core/gui#336 introduced a regression when termination requests from a platform are not handled properly.
This PR fixes this regression. On macOS shutdown after clicking "Quit" in Dock icon menu, and during logout works again.
Fixes bitcoin-core/gui#545.
ACKs for top commit:
RandyMcMillan:
tACK e7fc50681e99e3c726db2bc4d3d425ed8a0fc6b3
Sjors:
tACK e7fc50681e99e3c726db2bc4d3d425ed8a0fc6b3 (rebased on master) indeed fixes the crash described in #545
promag:
Tested ACK e7fc50681e99e3c726db2bc4d3d425ed8a0fc6b3 on macOS 10.15 with Qt 5.15.2.
Tree-SHA512: 236a483dc0828f22999469e133b8ac9f0b6267ec2a27004c3ebaa967689ddb972ea1fa90c1dd41f3bff3d17bf571a707babcef53bd79fd711fda98cfbf120131
71d33380ed6858b4a65b396332bfb22d984642a6 qt: prevent negative values of progressPerHour (HiLivin)
Pull request description:
Added a similar guard to _progressPerHour_ as is placed at _remainingMSecs_.
It prevents the display of negative values like "-0.00%" in some cases.
ACKs for top commit:
hebasto:
ACK 71d33380ed6858b4a65b396332bfb22d984642a6
jarolrod:
ACK 71d3338
shaavan:
reACK 71d33380ed6858b4a65b396332bfb22d984642a6
Tree-SHA512: 5427cdf4441b542196008034355ea00a075adf8b9aeeb383bacdb4e5fbda23d665448a50035aac93cbf401d5d6211d39a2c7c294568d9f5548a5c7579e201c44
faa5e171e6bdb8f3b4027a3f06497f0de5abf766 RPCConsole: Throw when overflowing size_t type for array indices (MarcoFalke)
Pull request description:
To test:
-> `getblock(getbestblockhash(), 1)[tx][22222222222222222222222222222]`
Before:
<- `868693731dea69a197c13c2cfaa41c9f78fcdeb8ab8e9f8cdf2c9025147ee7d1` (hash of the coinbase tx)
After:
<- `Error: Invalid result query`
ACKs for top commit:
jarolrod:
ACK faa5e171e6bdb8f3b4027a3f06497f0de5abf766
shaavan:
ACK faa5e17
Tree-SHA512: ddff39aae1c15db45928b110a9f1c42eadc5404cdfa89d67ccffc4c6af24091967d43c068ce9e0c1b863cfc4eb5b4f12373a73756a9474f8294e8a44aabc28d8
9bd168bf5457c6fd9770769547d8757bf14813b0 qt: add missing tooltips to options menu settings (Jarol Rodriguez)
Pull request description:
This adds missing tooltips to the text of the `Size of database cache` and the `Number of script verification threads` settings.
All settings in the Options window will now have appropriate tooltip texts.
ACKs for top commit:
jonatack:
ACK 9bd168bf5457c6fd9770769547d8757bf14813b0 tested on Debian 5.10.46-4 (2021-08-03)
hebasto:
ACK 9bd168bf5457c6fd9770769547d8757bf14813b0, tested on Linux Mint 20.2 (Qt 5.12.8).
Tree-SHA512: d71946bfee33c624a8b79eafe514d2c902090a40bc25097be4c7da4a80270f53305002af1b27d5fd082a0f45f838e22036632f9445918c4b8898073b33c09c08
93cc53a2b27eeb05fe00b8bf17465037815473a1 gui: Unregister wallet notifications before unloading wallets (Russell Yanofsky)
Pull request description:
This change was originally part of both bitcoin/bitcoin#10102 and bitcoin/bitcoin#19101 and is required for both because it avoids the IPC wallet implementation in bitcoin/bitcoin#10102 and the WalletContext implementation in bitcoin/bitcoin#19101 needing to deal with notification objects that have stale pointers to deleted wallets.
ACKs for top commit:
promag:
Code review ACK 93cc53a2b27eeb05fe00b8bf17465037815473a1.
hebasto:
ACK 93cc53a2b27eeb05fe00b8bf17465037815473a1
Tree-SHA512: 805f50a493291ad0f7c48725fbc5058d58ebbdb0770befd51d8aa241209a13f8a46f5982481336ab8338cdc83e9017668089a71deccf1587308e841cf8697825
facef3d4131f9980a4516282f11731361559509c doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke)
fae2fb2a196ee864e9a13fffc24a0279cd5d17e6 doc: Expand section on Getting Started (MarcoFalke)
100000d1b2c2e38d7a14a31b0af79e0e4316b04c doc: Add headings to CONTRIBUTING.md (MarcoFalke)
fab893e0caf510d4836a20194892ef9c71426c51 doc: Fix unrelated typos reported by codespell (MarcoFalke)
Pull request description:
Some random doc changes:
* Add sections to docs, so that they can be linked to
* Explain that anyone (even maintainers) are allowed to work on good first issues
* Expand section on Getting Started slightly
ACKs for top commit:
hebasto:
ACK facef3d4131f9980a4516282f11731361559509c
fanquake:
ACK facef3d4131f9980a4516282f11731361559509c
Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
aaaaad6ac95b402fe18d019d67897ced6b316ee0 scripted-diff: Bump copyright of files changed in 2019 (MarcoFalke)
Pull request description:
ACKs for top commit:
practicalswift:
ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0
promag:
ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0 🎉
fanquake:
ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0 - going to merge this now because the year is over and conflicts are minimal.
Tree-SHA512: 58cb1f53bc4c1395b2766f36fabc7e2332e213780a802762fff0afd59468dad0c3265f553714d761c7a2c44ff90f7dc250f04458f4b2eb8eef8b94f8c9891321
4935ac583bbdc289dd31a1caae3d711edef742b6 qt: Improve GUI responsiveness (Hennadii Stepanov)
75850106aeecfed1d2dc16d8a67ec210c5826a47 qt, macos: Fix GUIUtil::PolishProgressDialog bug (Hennadii Stepanov)
Pull request description:
[`QProgressDialog`](https://doc.qt.io/qt-5/qprogressdialog.html) estimates the time the operation will take (based on time for steps), and only shows itself if that estimate is beyond [`minimumDuration`](https://doc.qt.io/qt-5/qprogressdialog.html#minimumDuration-prop).
The default `minimumDuration` value is [4 seconds](https://doc.qt.io/qt-5/qprogressdialog.html#details), and it could make users think that the GUI is frozen.
This PR sets `minimumDuration` to zero for all progress dialogs, that affects ones in the `WalletControllerActivity` class.
ACKs for top commit:
ryanofsky:
Code review ACK 4935ac583bbdc289dd31a1caae3d711edef742b6. I'm not very familiar with this API but all the changes and explanations make sense and are very clear, and this seems like it should be an improvement.
promag:
Code review ACK 4935ac583bbdc289dd31a1caae3d711edef742b6.
jarolrod:
ACK 4935ac583bbdc289dd31a1caae3d711edef742b6
Tree-SHA512: 2ddd74e7fd87894d341d2439dbaa544d031a350f7f57d4c7e9fbba977dc24080fe60fd7a80a542b1647f1de9091d7fd04a36eab695088d4d75fb836548e99b5f
7f3a5980c1d54988a707b961fd2ef647cebb4c5b qt: Do not use QClipboard::Selection on Windows and macOS. (Hennadii Stepanov)
Pull request description:
Windows and macOS do [not support](https://doc.qt.io/qt-5/qclipboard.html#notes-for-windows-and-macos-users) the global mouse selection.
Fixes#258.
ACKs for top commit:
promag:
Code review ACK 7f3a5980c1d54988a707b961fd2ef647cebb4c5b.
jarolrod:
ACK 7f3a5980c1d54988a707b961fd2ef647cebb4c5b
Tree-SHA512: be2beeef7d25af6f4d4a4548325d8d29f08e4342f499666bc4a670ed468a63195d514077c2cd0dba197e12bd43316fd3e2813cdc0954364b6aa4ae6b90c118bf
e21276a82a9996c73e43990ccf927397f71399ea qt test: Don't bind to regtest port (Andrew Chow)
Pull request description:
The qt tests don't need to bind to the regtest port. By not binding, it will no longer conflict with existing regtest instances and the tests will run as normal.
Fixes#10
ACKs for top commit:
MarcoFalke:
cr ACK e21276a82a9996c73e43990ccf927397f71399ea
jarolrod:
re-ACK e21276a82a9996c73e43990ccf927397f71399ea, tested on macOS 11.2
Tree-SHA512: 5a269ee043f9aff7900e092c166de71912a2bf86ebe2982b3fb0e26bdebfb91869ee5d0f62082fd608c1288bfb7981f6c8647e504b11176711d7fec993a09164
2a39ccf1334ef3c48c6f9969a0fc916b9e10aae1 Add include for std::bind. (sinetek)
Pull request description:
Hi, this patch adds in <functional> because the GUI code makes use of std::bind.
That's all.
ACKs for top commit:
jonasschnelli:
utACK 2a39ccf1334ef3c48c6f9969a0fc916b9e10aae1
Tree-SHA512: fb5ac07d9cd5d006182b52857b289a9926362a2f1bfa4f7f1c78a088670e2ccf39ca28214781df82e8de3909fa3e69685fe1124a7e3ead758575839f5f2277a9
90f9fc274bf69b33adea593146ff5d6793123781 qt: Save QSplitter state in QSettings (Hennadii Stepanov)
Pull request description:
This PR adds the ability to save the `QSplitter` widget state in `QSettings` during shutdown, and restore it on startup.
A user no longer needs to adjust the splitter every time :)
ACKs for top commit:
jonasschnelli:
utACK 90f9fc274bf69b33adea593146ff5d6793123781
jonatack:
ACK 90f9fc274bf69b33adea593146ff5d6793123781 this sets the "PeersTabSplitterSizes" value in the RPCConsole dtor and restores it in the RPCConsole ctor; tested in Debian with various split settings, tab open/close sequences, and shutdown methods, and the Peers window split state was faithfully maintained.
Tree-SHA512: efbd6a4cee512982944955d36775e75a8a217b1dc49e62d42c6e402d2710dd44324b2c3c1edeb5fe38d9229e0e4a39734d1f4e63405ade8694762e1bbf72020b
1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0 qt: Reset toolbar after all wallets are closed (Hennadii Stepanov)
Pull request description:
If the last open wallet is closed from the non-"Overview" tab, that tab remains active when a new wallet is opened:
This PR fixes this bug.
ACKs for top commit:
promag:
Code review ACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0.
luke-jr:
utACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0
jonasschnelli:
utACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0
Tree-SHA512: a8dfd7591267e9544ad40c87581d554e5cfaad4b2a5bbfdbaf2596dc6869d2ac6cf7877adfef3d528fc61b081d40c6e30d787bbd7280ef7946aa7f7d9bc8b18e
705c1f0648c72aa97e0ee699ff9a3da23fc9bd61 qt, refactor: Fix 'buttonClicked is deprecated' warnings (Hennadii Stepanov)
c2f4e5ea1d6f01713ac69aaf6018884028aa55bd qt, refactor: Fix 'split is deprecated' warnings (Hennadii Stepanov)
8e12d6996116e786e928077b22d9f47cee27319e qt, refactor: Fix 'QFlags is deprecated' warnings (Hennadii Stepanov)
fa5749c805878304c107bcae0ae5ffa401dc7c4d qt, refactor: Fix 'pixmap is deprecated' warnings (Hennadii Stepanov)
b02264cb5dfcef50eec8a6346471cbaa25370e00 qt, refactor: Fix 'QDateTime is deprecated' warnings (Hennadii Stepanov)
Pull request description:
[What's New in Qt 5.15](https://doc.qt.io/qt-5/whatsnew515.html#deprecated-modules):
> To help preparing for the transition to Qt 6, numerous classes and member functions that will be removed from Qt 6.0 have been marked as deprecated in the Qt 5.15 release.
Fixes#36
ACKs for top commit:
jonasschnelli:
utACK 705c1f0648c72aa97e0ee699ff9a3da23fc9bd61
promag:
Tested ACK 705c1f0648c72aa97e0ee699ff9a3da23fc9bd61 on macos with Apple clang version 11.0.3 (clang-1103.0.32.62) and brew qt 5.15.1.
Tree-SHA512: 29e00535b4583ceec0dfb29612e86ee29bdea13651b548c6d22167917a4a10464af49160a12b05151030699f690f437ebb9c4ae9f130f66a722415222165b44f
86b1ab64b1a5b56518787ef16ea54ddbbc97d83e refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date (Hennadii Stepanov)
Pull request description:
As all deprecated warning in Qt 5.15.0 were eliminated in #46, Qt 5.15.1 introduced another one that is fixed in this PR.
Required for https://github.com/bitcoin/bitcoin/pull/20182.
Details in Qt docs:
- https://doc.qt.io/qt-5/qdatetime.html#toString-1
- https://doc.qt.io/qt-5/qdate.html#toString-1
ACKs for top commit:
jarolrod:
Tested ACK 86b1ab6 on MacOS 10.15.7 and Arch Linux both with Qt 5.15.1
jonasschnelli:
Tested ACK 86b1ab64b1a5b56518787ef16ea54ddbbc97d83e
Tree-SHA512: 1dbba8ee70c895bf58317172a9901cdbe5503b1d6258f51caaae88d88d332d9fbd4697c995192d31e3618ddfd532c5f5881289b3af1184422e5a9263a1224115
It fixes this and similar warnings:
```
qt/test/trafficgraphdatatests.cpp:145:23: warning: ‘int qrand()’ is deprecated: use QRandomGenerator instead [-Wdeprecated-declarations]
145 | int in = qrand() % 1000;
| ~~~~~^~
```
Call of `qsrand` is not needed because QRandomGenerator is already randomly initialized
## Issue being fixed or feature implemented
fix: possible assert call if nHeight in CDeterministicMNListDiff is
higher than Tip
Example of new log:
```
2023-09-28T17:35:50Z GetProjectedMNPayeesAtChainTip WARNING pindex is nullptr due to height=914160 chain height=914159
```
instead assert call:
```
...
#6 0x00007ffff7a33b86 in __assert_fail (assertion=0x55555783afd2 "pindex", file=0x5555577f2ed8 "llmq/utils.cpp", line=730,
function=0x5555577f2448 "bool llmq::utils::IsMNRewardReallocationActive(const CBlockIndex*)") at ./assert/assert.c:101
#7 0x0000555555ab7daf in llmq::utils::IsMNRewardReallocationActive (pindex=<optimized out>) at llmq/utils.cpp:730
#8 0x00005555559458ad in CDeterministicMNList::GetProjectedMNPayees (this=this@entry=0x7fffffffc690, pindex=0x0, nCount=<optimized out>, nCount@entry=2147483647)
at evo/deterministicmns.cpp:231
#9 0x000055555594614f in CDeterministicMNList::GetProjectedMNPayeesAtChainTip (this=this@entry=0x7fffffffc690, nCount=nCount@entry=2147483647) at evo/deterministicmns.cpp:216
#10 0x00005555558c9f51 in MasternodeList::updateDIP3List (this=this@entry=0x55555908cfd0) at qt/masternodelist.cpp:194
#11 0x00005555558ca9a0 in MasternodeList::updateDIP3ListScheduled (this=0x55555908cfd0) at qt/masternodelist.cpp:157
#12 0x000055555684a60f in void doActivate<false>(QObject*, int, void**) ()
#13 0x00005555568525b1 in QTimer::timerEvent(QTimerEvent*) ()
#14 0x0000555556844ce5 in QObject::event(QEvent*) ()
#15 0x0000555556ac3252 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
#16 0x000055555681e6b8 in QCoreApplication::sendEvent(QObject*, QEvent*) ()
#17 0x000055555686de2a in QTimerInfoList::activateTimers() ()
#18 0x000055555686be84 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
#19 0x00005555569bf8a2 in QXcbUnixEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
#20 0x000055555681caf6 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
#21 0x0000555556825f8a in QCoreApplication::exec() ()
...
```
## What was done?
ClientModel returns now a pair: MNList and CBlockIndex; so, we always
know the which one has been used even if current chain is switched.
## How Has This Been Tested?
Run on my localhost from `c034ff0c2606142ba3e8894bc74f693b87374e5c` -
aborted with backtrace like above.
With both of commit - no assert more.
## 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
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
4444dbf4d5047dd1c92973f7167a74a0779e61a3 gui: Remove un-actionable TODO (MarcoFalke)
Pull request description:
With encryption turned on by default for all wallets in consideration (#18889), I believe that wallet decryption will not be implemented ever or at least any time soon. So remove that TODO comment for now. If deemed important, a brainstorming issue can be opened instead.
Also remove some TODOs in the RPC console, which I don't understand. Maybe the gui was meant to show the debug log interactively? In any case, if deemed important, this should be filed as a brainstorming feature request, so that trade-offs of different solutions can be discussed.
ACKs for top commit:
laanwj:
Thanks. ACK 4444dbf4d5047dd1c92973f7167a74a0779e61a3
achow101:
ACK 4444dbf4d5047dd1c92973f7167a74a0779e61a3
Tree-SHA512: f7ddb37a14178f575da5409ea1c34e34bde37d79b2b56eaaf606a069e2b91c9d7b734529f5c68664b2fa5aa831117c8d19cce823743671cd6c31b81d68b8c70c
## Issue being fixed or feature implemented
fix 3 gui issues reported about beta2
## What was done?
pls see individual commits
## How Has This Been Tested?
to test ad0c0eb591 (there were no css
issues on my machine with locally compiled binaries): compile, run
to test 09800cff8f: run beta2 guix
compiled binaries with css from this PR e.g. on macos:
`/path/to/Dash-Qt.app/Contents/MacOS/Dash-Qt --regtest --debug-ui
--custom-css-dir=/path/to/dash/src/qt/res/css`
before 139d70701c:
<img width="224" alt="Screenshot 2023-09-30 at 22 02 32"
src="https://github.com/dashpay/dash/assets/1935069/b7a26ae9-f87f-4dde-b1f6-9eb796c22495">
after 139d70701c:
<img width="229" alt="Screenshot 2023-09-30 at 22 02 59"
src="https://github.com/dashpay/dash/assets/1935069/57d92e97-b25d-4035-9d1b-da373e51a574">
## 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
## Issue being fixed or feature implemented
Return by reference is generally not ideal, and especially as there is
only one return path per function, all returns will be done via NRVO.
Additionally, call sites are simpler now.
## What was done?
Refactored to return by value
## How Has This Been Tested?
Building
## Breaking Changes
Should be none
## 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)_
## Motivation
CoinJoin's subsystems are initialized by variables and managers that
occupy the global context. The _extent_ to which these subsystems
entrench themselves into the codebase is difficult to assess and moving
them out of the global context forces us to enumerate the subsystems in
the codebase that rely on CoinJoin logic and enumerate the order in
which components are initialized and destroyed.
Keeping this in mind, the scope of this pull request aims to:
* Reduce the amount of CoinJoin-specific entities present in the global
scope
* Make the remaining usage of these entities in the global scope
explicit and easily searchable
## Additional Information
* The initialization of `CCoinJoinClientQueueManager` is dependent on
blocks-only mode being disabled (which can be alternatively interpreted
as enabling the relay of transactions). The same applies to
`CBlockPolicyEstimator`, which `CCoinJoinClientQueueManager` depends.
Therefore, `CCoinJoinClientQueueManager` is only initialized if
transaction relaying is enabled and so is its scheduled maintenance
task. This can be found by looking at `init.cpp`
[here](93f8df1c31/src/init.cpp (L1681-L1683)),
[here](93f8df1c31/src/init.cpp (L2253-L2255))
and
[here](93f8df1c31/src/init.cpp (L2326-L2327)).
For this reason, `CBlockPolicyEstimator` is not a member of `CJContext`
and its usage is fulfilled by passing it as a reference when
initializing the scheduling task.
* `CJClientManager` has not used `CConnman` or `CTxMemPool` as `const`
as existing code that is outside the scope of this PR would cast away
constness, which would be unacceptable. Furthermore, some logical paths
are taken that will grind to a halt if they are stored as `const`.
Examples of such a call chains would be:
* `CJClientManager::DoMaintenance >
CCoinJoinClientManager::DoMaintenance > DoAutomaticDenominating >
CCoinJoinClientSession::DoAutomaticDenominating >
CCoinJoinClientSession::StartNewQueue > CConnman::AddPendingMasternode`
which modifies `CConnman::vPendingMasternodes`, which is non-const
behaviour
* `CJClientManager::DoMaintenance >
CCoinJoinClientManager::DoMaintenance > DoAutomaticDenominating >
CCoinJoin::IsCollateralValid > AcceptToMemoryPool` which adds a
transaction to the memory pool, which is non-const behaviour
* There were cppcheck [linter
failures](https://github.com/dashpay/dash/pull/5337#issuecomment-1685084688)
that seemed to be caused by the usage of `Assert` in
`coinjoin/client.h`. This seems to be resolved by backporting
[bitcoin#24714](https://github.com/bitcoin/bitcoin/pull/24714). (Thanks
@knst!)
* Depends on #5546
---------
Co-authored-by: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
## Issue being fixed or feature implemented
Let GUI users control all CJ params (on the fly) without the need to
edit `dash.conf`.
<img width="643" alt="Screenshot 2023-08-27 at 12 29 07"
src="https://github.com/dashpay/dash/assets/1935069/2d90db0d-c7b2-43a9-9f7f-1c4ad9517408">
## What was done?
Add 3 corresponding spin boxes in Options (with a simple sanity check).
I tried my best to come up with the least confusing labels/tooltips for
these, not sure if I'm 100% happy with the result though.
## How Has This Been Tested?
Run qt wallet, play with values and make sure they are saved/loaded/used
in mixing correctly.
## 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: thephez <thephez@users.noreply.github.com>
