-
Notifications
You must be signed in to change notification settings - Fork 356
Adds non-mempool wallet balance to overview #911
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -130,14 +130,14 @@ | |||||
| </property> | ||||||
| </widget> | ||||||
| </item> | ||||||
| <item row="3" column="0" colspan="2"> | ||||||
| <item row="4" column="0" colspan="2"> | ||||||
| <widget class="Line" name="line"> | ||||||
| <property name="orientation"> | ||||||
| <enum>Qt::Horizontal</enum> | ||||||
| </property> | ||||||
| </widget> | ||||||
| </item> | ||||||
| <item row="4" column="0"> | ||||||
| <item row="5" column="0"> | ||||||
| <widget class="QLabel" name="labelTotalText"> | ||||||
| <property name="text"> | ||||||
| <string>Total:</string> | ||||||
|
|
@@ -183,7 +183,33 @@ | |||||
| </property> | ||||||
| </widget> | ||||||
| </item> | ||||||
| <item row="4" column="1"> | ||||||
| <item row="3" column="1"> | ||||||
| <widget class="QLabel" name="labelNonMempool"> | ||||||
| <property name="cursor"> | ||||||
| <cursorShape>IBeamCursor</cursorShape> | ||||||
| </property> | ||||||
| <property name="toolTip"> | ||||||
| <string>Balance for wallet transactions not in the mempool</string> | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I think the tooltip text doesn't tell the user what that means in practice, or why it's negative...
Suggested change
|
||||||
| </property> | ||||||
| <property name="text"> | ||||||
| <string notr="true">0.00000000 BTC</string> | ||||||
| </property> | ||||||
| <property name="alignment"> | ||||||
| <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> | ||||||
| </property> | ||||||
| <property name="textInteractionFlags"> | ||||||
| <set>Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set> | ||||||
| </property> | ||||||
| </widget> | ||||||
| </item> | ||||||
| <item row="3" column="0"> | ||||||
| <widget class="QLabel" name="labelNonMempoolText"> | ||||||
| <property name="text"> | ||||||
| <string>Non-mempool:</string> | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just a silly feel-free-to-ignore nit: in the rpc we use Also maybe mempool can be tooo techincal for some users? What about
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking the same and have expressed it in my previous comment but I've checked in the code and it seems that unbroadcast has a different meaning I think (transactions that ARE in the mempool but haven't been relayed to peers yet -
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The label has a tooltip already, I left a suggestion there. |
||||||
| </property> | ||||||
| </widget> | ||||||
| </item> | ||||||
| <item row="5" column="1"> | ||||||
| <widget class="QLabel" name="labelTotal"> | ||||||
| <property name="cursor"> | ||||||
| <cursorShape>IBeamCursor</cursorShape> | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -190,13 +190,20 @@ void OverviewPage::setBalance(const interfaces::WalletBalances& balances) | |||||||
| ui->labelBalance->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); | ||||||||
| ui->labelUnconfirmed->setText(BitcoinUnits::formatWithPrivacy(unit, balances.unconfirmed_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); | ||||||||
| ui->labelImmature->setText(BitcoinUnits::formatWithPrivacy(unit, balances.immature_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); | ||||||||
| ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); | ||||||||
| ui->labelNonMempool->setText(BitcoinUnits::formatWithPrivacy(unit, balances.nonmempool_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); | ||||||||
| ui->labelTotal->setText(BitcoinUnits::formatWithPrivacy(unit, balances.balance + balances.unconfirmed_balance + balances.immature_balance + balances.nonmempool_balance, BitcoinUnits::SeparatorStyle::ALWAYS, m_privacy)); | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: the non-mempool balance is currently a plain text with no visual emphasis, perhaps since it's always negative, it should be red (like negative amounts in the transaction list)... (haven't tested it)
Suggested change
|
||||||||
| // only show immature (newly mined) balance if it's non-zero, so as not to complicate things | ||||||||
| // for the non-mining users | ||||||||
| bool showImmature = balances.immature_balance != 0; | ||||||||
|
|
||||||||
| ui->labelImmature->setVisible(showImmature); | ||||||||
| ui->labelImmatureText->setVisible(showImmature); | ||||||||
|
|
||||||||
| // likewise for non-mempool balances | ||||||||
| bool showNonMempool = balances.nonmempool_balance != 0; | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe |
||||||||
|
|
||||||||
| ui->labelNonMempool->setVisible(showNonMempool); | ||||||||
| ui->labelNonMempoolText->setVisible(showNonMempool); | ||||||||
| } | ||||||||
|
|
||||||||
| void OverviewPage::setClientModel(ClientModel *model) | ||||||||
|
|
@@ -296,5 +303,6 @@ void OverviewPage::setMonospacedFont(const QFont& f) | |||||||
| ui->labelBalance->setFont(f); | ||||||||
| ui->labelUnconfirmed->setFont(f); | ||||||||
| ui->labelImmature->setFont(f); | ||||||||
| ui->labelNonMempool->setFont(f); | ||||||||
| ui->labelTotal->setFont(f); | ||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Maybe this change motivation should be documented in the commit message. It is not clear why is needed.
Just a simple line saying that
m_mine_nonmempoolis always negative so this assert is no longer true should be enough.