550ed1bed2 build: update qt qpaint non determinism patch for 5.12.10 (fanquake)
6093ae4d30 build: update qt no-xlib patch for 5.12.10 (fanquake)
84928c4e73 build: update qt android jni static patch for 5.12.10 (fanquake)
cc6f47d51a build: update qt lrelease patch for 5.12.10 (fanquake)
286d07ff17 build, qt: Fix lib paths in *.pc files (Hennadii Stepanov)
fa5e97e8c2 build: disable qt SDK version checking (fanquake)
1be8e0f238 build: Add QMacStyle support (Hennadii Stepanov)
e674e94302 build: revert to using Qts internal zlib (fanquake)
06cd0da21f build: qt 5.12.10 (fanquake)
3272e34f9c build: Add xkbcommon 0.8.4 (Hennadii Stepanov)
d769b3372d build: only pass -optimized-tools to qt in debug mode (fanquake)
Pull request description:
Switch to Qt 5.12.10 in depends. Based on #21363. This is a much smaller changeset, and should be easier to review than #19716. Also postpones needing to bring a bunch of new libs into depends.
Big thanks to Hebasto that has been helping with this.
ACKs for top commit:
laanwj:
Code review ACK 550ed1bed2
hebasto:
ACK 550ed1bed2
jarolrod:
ACK 550ed1bed2 , tested on macOS 11.2 built from depends
Tree-SHA512: cb6b70f5a5372ba0b64f7ddfa696eda0411922cd261c67bfa2d9332c685a7b358ab18e5cfaa677b414ae8ad78296bba6ed0eecd071fdacdf736a0d030f679fe5
c524dc54bb qt: Fix regression with initial sorting after pr205 (Hennadii Stepanov)
Pull request description:
Unfortunately, #205 introduced a regression. After opening the "Receive" or "Transaction" tab at first time despite of the "Date" header is marked as sorted, table rows are not sorted actually:
It appears that sorting the table must be triggered _after_ the `QTableView::setModel` call.
With this PR (and pre-#205):
ACKs for top commit:
Talkless:
tACK c524dc54bb, tested on Debian Sid with Qt 5.15.2. I can confirm @leonardojobim observations.
leonardojobim:
Tested ACK c524dc54bb on Ubuntu 20.04.2 Qt 5.12.8
jonatack:
ACK c524dc54bb
jarolrod:
ACK c524dc54bb, tested on macOS 11.1 Qt 5.15.2
Tree-SHA512: e370229979a70d63a0b64dbc11c4eca338695a070881d4d8f015644617f180e6accc24d6bdf98a75e7c9ba9be2a0ace9a2b7eb9c783ebb2992c3b2c3b3deb408
e21276a82a 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 e21276a82a
jarolrod:
re-ACK e21276a82a, tested on macOS 11.2
Tree-SHA512: 5a269ee043f9aff7900e092c166de71912a2bf86ebe2982b3fb0e26bdebfb91869ee5d0f62082fd608c1288bfb7981f6c8647e504b11176711d7fec993a09164
6242beeb06 Hoist repeated translated strings to RPCConsole struct members (Jon Atack)
0f035c12fb RPCConsole::updateDetailWidget: convert strings to translated strings (Jon Atack)
Pull request description:
- fixups from #206 review feedback (thanks!), see commit message for details
- hoists repeatedly used translatable strings to the `RPCConsole` class for reuse
ACKs for top commit:
hebasto:
re-ACK 6242beeb06
Talkless:
tACK 6242beeb06, tested on Debian Sid with Qt 5.15.2. I see "Ban for.." translated to my native language as before, "To/From/Yes/No" are not but that's expected, as `.ts` files are not updated.
jarolrod:
ACK 6242beeb06
Tree-SHA512: 20a296511c5ac03a816766237fa2731b0360dedebf1bea02711eb21d7e4eae2a63a051fe48f4726052edc3e6318952f01fef920cd4b22a8196c39c23d8e5cc3a
1d5d832d5c qt, refactor: Use enum type as switch argument in TransactionTableModel (Hennadii Stepanov)
52f122c11f qt, refactor: Use enum type as switch argument in PeerTableModel (Hennadii Stepanov)
a35223f1cd qt, refactor: Use enum type as switch argument in BanTableModel (Hennadii Stepanov)
ab8a747d1c qt, refactor: Use enum type as switch argument in AddressTableModel (Hennadii Stepanov)
Pull request description:
This PR makes code more maintainable by leveraging `-Wswitch` compiler warnings.
Only the `RecentRequestsTableModel` is not refactored, because its `enum ColumnIndex` contains additional `NUMBER_OF_COLUMNS` value.
No behavior change.
ACKs for top commit:
hebasto:
Do you mind mentioning the _top_ pr commit with your ACK, i.e., 1d5d832d5c, not ab8a747d1ced9f20ca32f9898418be70670da71a?
jarolrod:
ACK 1d5d832d5c, tested on macOS 11.1 Qt 5.15.2
leonardojobim:
ACK 1d5d832d5c, tested on Ubuntu 20.04 Qt 5.12.8
promag:
Code review ACK 1d5d832d5c.
Tree-SHA512: 0d474d226a2fa0069495d1aa5ec13b2470708ec7b8a6ab35402236c7bf57cb9939577677a30dfa54f5e3bc0477c6cfffd20ed6f19e4eb394a938569cc9347851
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.
67c59ae479 qt: Make warning label look clickable (Jarol Rodriguez)
Pull request description:
The warning icon on the overview page indicates that there is something important the user should know about, but a user may not be aware that they can click it because, on `master`, the warning label does not look clickable. As detailed in issue #23, the reason to make it look clickable is that it if they "had a more clickable-appearance (borders or beveled button edges) it could help users more quickly understand what they are being alerted to."
This PR removes the `flat` property from both `QPushButton`'s to make them look like a button, and therefore clickable. Furthermore, it updates the `Maximum Width` to `45` to fix the small hit-box issue outlined in issue #215.
Below are screenshots showing how the warning icon looks under `master` and this `PR`:
**macOS 11.1: Qt 5.15**
| Master | PR |
| ----------- | ----------- |
| <img width="754" alt="Screen Shot 2021-02-22 at 5 00 40 PM" src="https://user-images.githubusercontent.com/23396902/108776135-f6d50380-752f-11eb-9f96-25163c6a2a02.png"> | <img width="754" alt="Screen Shot 2021-02-22 at 3 08 40 PM" src="https://user-images.githubusercontent.com/23396902/108776068-e0c74300-752f-11eb-9545-3580e2b8f187.png"> |
**Ubuntu 20.04: Qt 5.12**
| Master | PR |
| ----------- | ----------- |
| <img width="783" alt="Screen Shot 2021-02-22 at 4 57 32 PM" src="https://user-images.githubusercontent.com/23396902/108776249-284dcf00-7530-11eb-8325-7fe13a9243a7.png"> |  |
Closes #23
Closes #215
ACKs for top commit:
Talkless:
tACK 67c59ae479, tested on Debian Sid. Does look as expected.
Tree-SHA512: 2b7302fb990ea49e2f01df6f4a23e2bc3de0797da89deaeb299742e6b285a0c21ea80d8259dc0222640cccc2bccc4ea09df443b9a11bf8b88a828e5fb2aec12c
faa06ecc9c build: Bump minimum Qt version to 5.9.5 (Hennadii Stepanov)
Pull request description:
Close #20104.
ACKs for top commit:
laanwj:
Code review ACK faa06ecc9c
jarolrod:
ACK faa06ecc9c
fanquake:
ACK faa06ecc9c - this should be ok to do now.
Tree-SHA512: 7295472b5fd37ffb30f044e88c39d375a5a5187d3f2d44d4e73d0eb0c7fd923cf9949c2ddab6cddd8c5da7e375fff38112b6ea9779da4fecce6f024d05ba9c08
70d3c5d0b9 gui: add "Last Block" (CNodeStats::nLastBlockTime) to peer details (Jon Atack)
a21be7c401 gui: add "Last Tx" (CNodeStats::nLastTXTime) to peer details (Jon Atack)
4dc2fd6c37 qt: add RPCConsole::TimeDurationField helper, call systime only once (Jon Atack)
Pull request description:
- add `RPCConsole::TimeDurationField` helper to replace repeated code and call system time only once in `RPCConsole::updateDetailWidget`
- add "Last Tx" (`CNodeStats::nLastTXTime`) field to peer details
- add "Last Block" (`CNodeStats::nLastBlockTime`) field to peer details
ACKs for top commit:
hebasto:
re-ACK 70d3c5d0b9
Tree-SHA512: 2611b71fd358ba9ffb6a6206275c08ecb5e683b6f87d022faaaba9802a15030430113afdb434814a9ae2681d04429aa733164bc110b64337ceaae12a0420f4f1
5440c07457 qt: Rename "Edit label" to "Edit address label" (Wladimir J. van der Laan)
22664d6287 Revert "qt: Remove Transactionview Edit Label Action" (Wladimir J. van der Laan)
Pull request description:
This reverts PR #211.
I disagree with this change, I use the functionality a lot, it was the primary way I used to organize and edit transactions labels and am sad to see this go.
> you can edit a sending address in the send tab Address Book
Using the address book should not be encouraged at all! A while ago it was even proposed to remove it. There's rarely need to scroll through all historical addresses used and unused. The transaction list does just fine for this.
> While all other actions apply directly to the selected transaction, the Edit Label action applies to the selected transaction's address.
**In practice** when bitcoin is used in the commonly advised way, generate a new address for each transaction, those are equivalent though.
I doubt I (and **luke-jr**) will be the only users that will stumblle on this. Further discussion here: https://github.com/bitcoin-core/gui/pull/211#issuecomment-784755998
ACKs for top commit:
hebasto:
ACK 5440c07457, verified that 22664d6287 is a clean revert of 8f9644890a.
Tree-SHA512: 3a86a730279bc454d0bd25d874dbfb6b1c0492480e66c3164e7c60d8658d622d4522de11bf8564876dc3ee056b53db71ecbe8a37281bf25d41a27e6e0d72ad8f
bb3da8fe41 qt: Disable requests context menu actions when appropriate (Jarol Rodriguez)
Pull request description:
The recent requests table will allow you to copy data points even if they do not exist. This PR implements checks to disable the `copy label`, `copy message`, and `copy amount` context menu actions if the respective fields are empty. This brings the recent requests table context menu behavior closer to the behavior seen in the transaction view.
On a payment request entry which does not have a value for label, message, or amount:
| Master | PR |
| ----------- | ----------- |
|<img width="169" alt="Screen Shot 2021-02-19 at 1 22 28 AM" src="https://user-images.githubusercontent.com/23396902/108466086-167adc00-7251-11eb-8bd6-13984042bdb3.png">| <img width="169" alt="Screen Shot 2021-02-19 at 1 21 49 AM" src="https://user-images.githubusercontent.com/23396902/108466185-3e6a3f80-7251-11eb-9dd8-492ed07fd638.png">|
`copy URI` never needs to be disabled as an entry in the recent requests table must have a URI even if it doesn't have a label, message, or amount. #213 will add a `copy address` context menu action. This also does not need a check as an entry must be associated with an address.
Below are some more examples of how this PR will behave:
**Has Label, Message, and Amount**
<img width="780" alt="Screen Shot 2021-02-19 at 12 05 38 AM" src="https://user-images.githubusercontent.com/23396902/108466507-c18b9580-7251-11eb-8875-f3aeb9c4c8e9.png">
**Has Label and Amount, but no Message**
<img width="780" alt="Screen Shot 2021-02-19 at 12 05 58 AM" src="https://user-images.githubusercontent.com/23396902/108466421-9b65f580-7251-11eb-97eb-a3bfaa21fa7d.png">
ACKs for top commit:
hebasto:
re-ACK bb3da8fe41
Tree-SHA512: d98f1a6e05ebf6d9d056bc5754aca78ca7ecda93f497dba88187b947ca4a261eb7dc5e8c872956315acaa0008cc39320fb5806e17117e0c873090ad917ca4a3d
ca5bd1c8e5 qt: Prevent the main window popup menu (Hennadii Stepanov)
Pull request description:
https://github.com/bitcoin/bitcoin/issues/11168 is not fixed by https://github.com/bitcoin/bitcoin/pull/11169 completely, as users are allowed to right click on the menu bar:
This PR moves the context menu prohibition from `QToolBar` instance to its parent `BitcoinGUI` instance, which is derived from `QMainWindow`.
ACKs for top commit:
jarolrod:
ACK ca5bd1c8e5, tested on Ubuntu 20.04 Qt 5.12. Confirming that I can replicate the behavior described on `master` and this `PR` fixes it.
leonardojobim:
tACK ca5bd1c8e5
Tree-SHA512: a654ecf7ee35bb271df039be77077c1e1f9514e332587ba8622cea18da6a5b3ae8a7eb421e404ec5993c31a2f4d028e0e456fcc01facdbf61a2bc3b1e8423982
The recent requests table will allow you to copy data points even if they do not exist.
This PR implements checks to disable the 'copy label', 'copy message', and 'copy amount' context menu action if the respective fields are empty.
By default, a popup menu contains checkable entries for the toolbars
and dock widgets present in the main window. This allows users to
accidentally hide the toolbar.
Currently, the only way to copy the address of a payment request is to double-click on the payment request and then click on the copy address button. This commit adds a convenient context menu action to copy the address of a payment request.
The warning label shown on the overview page does not look clickable.
This PR makes the warning label look clickable by removing the 'flat' property.
Additionally, the Maximum Width is updated to fix the small hit-box issue.
8f9644890a qt: Remove Transactionview Edit Label Action (Jarol Rodriguez)
Pull request description:
This PR removes the `Edit Label` action from the `transactionview` context menu. Since the `Edit Label` action will no longer be utilized in the `transactionview`, the `Edit Label` function logic is also removed.
| Master | PR |
| ----------- | ----------- |
|<img width="248" alt="Screen Shot 2021-02-17 at 8 34 34 PM" src="https://user-images.githubusercontent.com/23396902/108292189-9b86c800-7161-11eb-9e80-6238523bc27e.png">|<img width="248" alt="Screen Shot 2021-02-17 at 8 35 10 PM" src="https://user-images.githubusercontent.com/23396902/108292204-a17ca900-7161-11eb-8582-7f33d3e2ba8f.png">|
Among the context menu actions for each transaction in the `transactionview` is the `Edit Label` action.
While all other actions apply directly to the selected transaction, the `Edit Label` action applies to the selected transaction's address. As documented in issue #209 and [#1168](https://github.com/bitcoin/bitcoin/issues/1168) , this is an "unfortunate" placement for such an action. The current placement creates a confusing UX scenario where the outcome of the action is ambiguous.
**Example of Ambiguous Behavior:**
The context menu gives the wrong impression that the `Edit Label` action will edit a `Label` for the specific transaction that has been right-clicked on. This impression can be because all other actions in this menu will relate to the specific transaction and the misconception between `Comment` and `Label`.
<img width="1062" alt="editlabel-start" src="https://user-images.githubusercontent.com/23396902/108296385-6da48200-7167-11eb-89f0-b21ccc58f6f4.png">
Let's say I wanted to give the transaction selected in the screenshot above a comment of "2-17[17:43]". Given all the context clues, it will be reasonable to assume that the `Edit Label` function will give a label to this transaction. Instead, it edits the `Label` for the address behind this transaction. Thus, changing the `Label` for all transactions associated with this address.
<img width="971" alt="editlabel-end" src="https://user-images.githubusercontent.com/23396902/108297179-e35d1d80-7168-11eb-86a9-0d2796c51829.png">
**Maintaining `Edit Label` Functionality:**
The action of Editing a Label should instead be reserved for the respective address tables of the `Send` and `Receive` tabs. As documented in this [comment](https://github.com/bitcoin-core/gui/issues/209#issuecomment-780922101), `Edit Label` is currently implemented in the `Send` tab and is missing in the `Receive` tab. A follow-up PR can add the `Edit Label` functionality to the `Receive` tab.
ACKs for top commit:
MarcoFalke:
review ACK 8f9644890a
Talkless:
tACK 8f9644890a, tested on Debian Sid.
Tree-SHA512: 70bbcc8be3364b0d4f476a9760aa14ad1ad1f53b0b130ce0ffe75190d76c386e6e26c530c0a55d1742402fe2b45c68a2af6dbfaf58ee9909ad93b06f0b6559d4
142807af8b gui: display BIP152 high bandwidth relay in peer details (Jon Atack)
9476886353 gui: display fRelayTxes in peer details (Jon Atack)
Pull request description:
This pull adds two fields to the peer details, "Wants Tx Relay" (fRelayTxes) and "High Bandwidth" (bip152_highbandwidth to/from). See the added tooltips for more info.
ACKs for top commit:
MarcoFalke:
review ACK 142807af8b
jarolrod:
ACK 142807af8b
Tree-SHA512: 956c7fa54c9c2ea76ee879d370711be0bed4af05484a17d35a1dd77713ed34ff441ed3957d0ef3a7ca7cf59a2f5d898be49b12af609a16b3e3cbfc4a1ba8f54e
8353e8cecc peers-tab: bug fix right panel toggle (randymcmillan)
Pull request description:
Initial Presentation:
When node row selected - panel is presented:
When network disabled - right panel is hidden:
ACKs for top commit:
jarolrod:
ACK 8353e8cecc
jonatack:
ACK 8353e8cecc tested rebased on current master. Behavior is initially a bit surprising but this would allow more columns to be added to the peers tab window. Verified that selecting more than one peer, clicking on a column header, or running `disconnectnode "" <currently-selected-peer-id>` in the console (or on the CLI with the `-server` startup option) returns the window to its full size. If this is merged, it might be nice to have an obvious way to close the details area like a clickable "close this" icon in the upper left corner of the area.
Talkless:
tACK 8353e8cecc, tested on Debian Sid. Made `bitcoind` connect to `bitcoin-qt` with the PR changes, and after I quit the `bitcoind` instance, right panel do disappear, compared to the previous commit where it didn't.
Tree-SHA512: 8fc156f40bdd61e3ba8db333c729a2a07fd5f0fd1eed56f2fd2aa5ae5864756f8ab6fad74ae2fb0552ee7518b6d489f5800709e6c80c6f31f61fd8ce21cece5f
be4cf4832f gui: update to "Direction/Type" peer details name/tooltip (Jon Atack)
151888383a gui: add "Type" column to Peers main window (Jon Atack)
6fc72bd6f0 gui: allow ConnectionTypeToQString to prepend direction optionally (Jon Atack)
Pull request description:
This pull:
- adds a sortable `Type` column to the GUI Peers tab window
- updates the peer details row to `Direction/Type`, so the `Type` column without a direction makes sense (the tooltip is also updated)
ACKs for top commit:
jarolrod:
ACK be4cf4832f
leonardojobim:
Tested ACK be4cf4832f on Ubuntu 20.04 on VMWare.
Tree-SHA512: 6c6d1dbe7d6bdb616acff0aaf8f4223546f1d2524566e9cd6e5b1b3bed2be1e9b20b1bc52ed3b627df53ba1f2fe0bc76f036cf16ad934d8a446b515d9bece3b1
Setting the "Monospace" font family in a `*.ui` file does not work on
macOS, at least on Big Sur with Qt 5.15 (neither via the "font" property
nor via the "styleSheet" property). Qt chooses the ".AppleSystemUIFont"
instead of ".AppleSystemUIFontMonospaced".
This change makes macOS choose the correct monospaced font.
e829c9afbf refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17) (Sebastian Falbesoner)
365539c846 refactor: init vectors via std::{begin,end} to avoid pointer arithmetic (Sebastian Falbesoner)
63d4ee1968 refactor: iterate arrays via C++11 range-based for loops if idx is not needed (Sebastian Falbesoner)
Pull request description:
This refactoring PR picks up the idea of #19626 and replaces all occurences of `sizeof(x)/sizeof(x[0])` (or `sizeof(x)/sizeof(*x)`, respectively) with the now-available C++17 [`std::size`](https://en.cppreference.com/w/cpp/iterator/size) (as [suggested by sipa](https://github.com/bitcoin/bitcoin/pull/19626#issuecomment-666487228)), making the macro `ARRAYLEN` obsolete.
As preparation for this, two other changes are done to eliminate `sizeof(x)/sizeof(x[0])` usage:
* all places where arrays are iterated via an index are changed to use C++11 range-based for loops If the index' only purpose is to access the array element (as [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/19626#discussion_r463404541)).
* `std::vector` initializations are done via `std::begin` and `std::end` rather than using pointer arithmetic to calculate the end (also [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/20429#discussion_r567418821)).
ACKs for top commit:
practicalswift:
cr ACK e829c9afbf: patch looks correct
fanquake:
ACK e829c9afbf
MarcoFalke:
review ACK e829c9afbf 🌩
Tree-SHA512: b01d32c04b9e04d562b7717cae00a651ec9a718645047a90761be6959e0cc2adbd67494e058fe894641076711bb09c3b47a047d0275c736f0b2218e1ce0d193d
Among the context menu actions for each transaction in the transactionview is the 'Edit Label' action.
While all other actions apply directly to the selected transaction, the 'Edit Label' action applies to the address of the selected transaction. This creates a confusing UX scenario where the outcome of the action is ambiguous. The action of Editing a Label should instead be reserved for the Send and Receive tabs.
This PR removes the 'Edit Label' action from the transactionview context menu. Since the 'Edit Label' action will no longer be utilized in the transactionview, the 'Edit Label' function logic is also removed.
