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

[Qt] Show more significant warning if we fall back to the default fee #9481

Merged
merged 3 commits into from Mar 14, 2017

Conversation

Projects
None yet
8 participants
@jonasschnelli
Member

jonasschnelli commented Jan 6, 2017

There are multiple reports (bitcoin.stackoverflow, etc.) from users sending (very-)low-fee-transactions with bitcoin-core.
The reason for this is probably that some users are not waiting long enough until smartfee estimation is possible.
The current fallback fee is 0.0002BTC/kb, one of my ~0.13.0 nodes is telling me 0.00069534 for a confirmation target of 25 blocks. Using the current fallback fee can be troublesome.

If we have to use the fallback-ferrate, we should probably warn the user more significant (there is no "warning" right now.).

Screenshots:
bildschirmfoto 2017-01-06 um 11 00 07
bildschirmfoto 2017-01-06 um 11 00 09
bildschirmfoto 2017-01-06 um 11 02 18

@jonasschnelli jonasschnelli added the GUI label Jan 6, 2017

@@ -760,10 +760,30 @@
</layout>
</item>
<item>
<widget class="QLabel" name="fallbackFeeWarningLabel">
<property name="toolTip">
<string>Using the fallbackfee can result in sending a transaction that will take serval hours or days (or never) to confirm. Consider choosing your fee manually or wait until your have validated the complete chain.</string>

This comment has been minimized.

@MarcoFalke

MarcoFalke Jan 6, 2017

Member

validating the whole chain is not enough. You also need the mempool at all times of validation.

@MarcoFalke

MarcoFalke Jan 6, 2017

Member

validating the whole chain is not enough. You also need the mempool at all times of validation.

This comment has been minimized.

@jonasschnelli

jonasschnelli Jan 6, 2017

Member

I have though about that, but to not over-explain this, I was picking out the most important part, which is probably a validated chain. And since we persist the mempool, this is less of a problem.

@jonasschnelli

jonasschnelli Jan 6, 2017

Member

I have though about that, but to not over-explain this, I was picking out the most important part, which is probably a validated chain. And since we persist the mempool, this is less of a problem.

@@ -760,10 +760,30 @@
</layout>
</item>
<item>
<widget class="QLabel" name="fallbackFeeWarningLabel">
<property name="toolTip">
<string>Using the fallbackfee can result in sending a transaction that will take serval hours or days (or never) to confirm. Consider choosing your fee manually or wait until your have validated the complete chain.</string>

This comment has been minimized.

@MarcoFalke

MarcoFalke Jan 6, 2017

Member

The warning is one sided. Using the fallbackfee might also overpay

@MarcoFalke

MarcoFalke Jan 6, 2017

Member

The warning is one sided. Using the fallbackfee might also overpay

This comment has been minimized.

@MarcoFalke

MarcoFalke Jan 6, 2017

Member

Maybe just mention that the fallbackfee is hardcoded and the fee should be selected manually. (or the node should be run for some hours to initialize smart fee)

@MarcoFalke

MarcoFalke Jan 6, 2017

Member

Maybe just mention that the fallbackfee is hardcoded and the fee should be selected manually. (or the node should be run for some hours to initialize smart fee)

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jan 6, 2017

Member

Concept ACK

Member

MarcoFalke commented Jan 6, 2017

Concept ACK

@@ -760,10 +760,30 @@
</layout>
</item>
<item>
<widget class="QLabel" name="fallbackFeeWarningLabel">
<property name="toolTip">
<string>Using the fallbackfee can result in sending a transaction that will take serval hours or days (or never) to confirm. Consider choosing your fee manually or wait until your have validated the complete chain.</string>

This comment has been minimized.

@paveljanik

paveljanik Jan 6, 2017

Contributor

serval -> several

@paveljanik

paveljanik Jan 6, 2017

Contributor

serval -> several

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jan 6, 2017

Member

Fixed the text after recommendation from @paveljanik and @MarcoFalke.
Happy to exchange the text(s) (tooltip / label) with better ones if someone has intentions to provide better ones.

Member

jonasschnelli commented Jan 6, 2017

Fixed the text after recommendation from @paveljanik and @MarcoFalke.
Happy to exchange the text(s) (tooltip / label) with better ones if someone has intentions to provide better ones.

@morcos

This comment has been minimized.

Show comment
Hide comment
@morcos

morcos Jan 6, 2017

Member

Concept ACK

I think there are a few other things we could potentially do:

  • Clarify via release notes that fallbackfee can be set on the command line and should be when starting a new node or catching up from a long time ago.
  • Change the default fallbackfee every release (yeah i know... no one likes this, including me)
  • Prevent transactions from going out using fee estimation if we would fall back to the fallbackfee (unless it was explicitly set by the user?)

It's always going to be problem to get transactions out the door before we've had a chance to bootstrap fee estimation but would be nice to make it harder for users to shoot themselves in the foot. At least now the fallbackfee is such that their transactions will probably be confirmed within a day or two, but that might not remain true...

In any case, warning with the current behavior is a good first step.

Member

morcos commented Jan 6, 2017

Concept ACK

I think there are a few other things we could potentially do:

  • Clarify via release notes that fallbackfee can be set on the command line and should be when starting a new node or catching up from a long time ago.
  • Change the default fallbackfee every release (yeah i know... no one likes this, including me)
  • Prevent transactions from going out using fee estimation if we would fall back to the fallbackfee (unless it was explicitly set by the user?)

It's always going to be problem to get transactions out the door before we've had a chance to bootstrap fee estimation but would be nice to make it harder for users to shoot themselves in the foot. At least now the fallbackfee is such that their transactions will probably be confirmed within a day or two, but that might not remain true...

In any case, warning with the current behavior is a good first step.

@gmaxwell

This comment has been minimized.

Show comment
Hide comment
@gmaxwell

gmaxwell Jan 7, 2017

Member

@morcos to your list: get bumpfee working really well and worry less about this. :)

Member

gmaxwell commented Jan 7, 2017

@morcos to your list: get bumpfee working really well and worry less about this. :)

Show outdated Hide outdated src/qt/forms/sendcoinsdialog.ui
<string>Using the static fallback-fee can result in sending a transaction that will take several hours or days (or never) to confirm or that you overpay the fees. Consider choosing your fee manually or wait until your have validated the complete chain.</string>
</property>
<property name="styleSheet">
<string notr="true">color: rgb(255, 150, 0);

This comment has been minimized.

@luke-jr

luke-jr Feb 9, 2017

Member

This affects the tooltip style as well. Additionally, it is hard to read on some styles.

@luke-jr

luke-jr Feb 9, 2017

Member

This affects the tooltip style as well. Additionally, it is hard to read on some styles.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 9, 2017

Member

This is an excellent idea, thanks for implementing it. Concept ACK!

Member

laanwj commented Feb 9, 2017

This is an excellent idea, thanks for implementing it. Concept ACK!

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Feb 17, 2017

Member

Actually, this seems redundant with labelSmartFee2...? Maybe that should just be made clearer (disable the slider??)

Edit: (Oh, nevermind. That's not visible by default.)

Member

luke-jr commented Feb 17, 2017

Actually, this seems redundant with labelSmartFee2...? Maybe that should just be made clearer (disable the slider??)

Edit: (Oh, nevermind. That's not visible by default.)

luke-jr added some commits Feb 17, 2017

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Feb 17, 2017

Member

Added @luke-jr commits (@thanks!).
Looks good on Windows.
Maybe we can get a windows and linux dark scheme screenshot?

bildschirmfoto 2017-02-17 um 11 43 00

Member

jonasschnelli commented Feb 17, 2017

Added @luke-jr commits (@thanks!).
Looks good on Windows.
Maybe we can get a windows and linux dark scheme screenshot?

bildschirmfoto 2017-02-17 um 11 43 00

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 14, 2017

Member

Worksforme. ACK 7abe7bb:
untitled

Member

laanwj commented Mar 14, 2017

Worksforme. ACK 7abe7bb:
untitled

@laanwj laanwj merged commit 7abe7bb into bitcoin:master Mar 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Mar 14, 2017

Merge #9481: [Qt] Show more significant warning if we fall back to th…
…e default fee

7abe7bb Qt/Send: Give fallback fee a reasonable indent (Luke Dashjr)
3e4d7bf Qt/Send: Figure a decent warning colour from theme (Luke Dashjr)
c5adf8f [Qt] Show more significant warning if we fall back to the default fee (Jonas Schnelli)

Tree-SHA512: 9e85b5b398d7a49aaf6c42578d63750b1b7aa9cc9e84d008fe21d6c53f1ffe2fb69286a1a764e634ebca3286564615578eea0a1bc883e4b332be8306d9883d14
@practicalswift

This comment has been minimized.

Show comment
Hide comment
@practicalswift

practicalswift Mar 16, 2017

Member

I found a typo in this PR - see #10008 for details.

Sorry for not catching it pre-merge :-)

Member

practicalswift commented Mar 16, 2017

I found a typo in this PR - see #10008 for details.

Sorry for not catching it pre-merge :-)

luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Jun 5, 2017

luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Jun 5, 2017

luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Jun 5, 2017

@MarcoFalke MarcoFalke referenced this pull request Dec 20, 2017

Closed

cost too many fees? #10247

codablock added a commit to codablock/dash that referenced this pull request Jan 26, 2018

Merge #9481: [Qt] Show more significant warning if we fall back to th…
…e default fee

7abe7bb Qt/Send: Give fallback fee a reasonable indent (Luke Dashjr)
3e4d7bf Qt/Send: Figure a decent warning colour from theme (Luke Dashjr)
c5adf8f [Qt] Show more significant warning if we fall back to the default fee (Jonas Schnelli)

Tree-SHA512: 9e85b5b398d7a49aaf6c42578d63750b1b7aa9cc9e84d008fe21d6c53f1ffe2fb69286a1a764e634ebca3286564615578eea0a1bc883e4b332be8306d9883d14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment