qml: allow manual editing of fee/feerate#10371
Conversation
|
marked as draft, missing filtering invalid alphanumeric input |
accumulator
force-pushed
the
qml_manual_fee_feerate_edit
branch
3 times, most recently
from
0c5ba67 to
17ae77b
Compare
also improve warning box styling consistency across finalizers, add CPFP new feerate > old feerate check, add relayfee checks for CPFP, DSCancel, proper warning string for no dynamic fee estimates
accumulator
force-pushed
the
qml_manual_fee_feerate_edit
branch
from
17ae77b to
fffcf4a
Compare
f321x
left a comment
f321x
left a comment
There was a problem hiding this comment.
Tested this on android, lgtm.
Related to this, we seem to allow broadcasting a tx on QML when showing a warning (e.g. too low relayfee), which then fails with a server error, while disabling the Broadcast/Pay button in Qt.
Should this be unified?
This sounds like a bug |
instead use font colors to hint which textedit is being used for target
SomberNight
left a comment
SomberNight
left a comment
There was a problem hiding this comment.
Mostly looks good, but have some comments.
| Label { | ||
| Layout.preferredWidth: 1 | ||
| visible: showPicker && manualFeeEntry | ||
| color: Material.accentColor | ||
| text: qsTr('Total') | ||
| } |
There was a problem hiding this comment.
This layout seems very brittle and magic o.O
So there is a grid with 2 columns: we put the "Rate" label into it. Then we put a sub-grid into it which spans two rows. Then we put the "Total" label into it, expecting it to be just under "Rate" and to the left of the sub-grid. o.O
It works atm but if I try to do the subtlest change, it breaks :/
There was a problem hiding this comment.
I changed the layout in 8a3d9fd
it looks a bit worse as it is not column-aligned :( but at least it behaves predictably >.<
| TextField { | ||
| id: rate | ||
| Layout.fillWidth: true | ||
| text: finalizer.userFeerate | ||
| inputMethodHints: Qt.ImhDigitsOnly | ||
| validator: RegularExpressionValidator { | ||
| regularExpression: /^[0-9]*\.[0-9]?$/ | ||
| } | ||
| onTextEdited: { | ||
| finalizer.userFeerate = text | ||
| } | ||
| } |
There was a problem hiding this comment.
Note that the desktop Qt GUI does not always expose editing both the feerate and abs fee.
- for simple-sends, it exposes both
- for RBF bump-fee and dscancel, it only exposes feerate
- for CPFP, it only exposes abs fee
This is because the underlying wallet.py methods are limited accordingly. E.g. wallet.bump_fee wants a feerate.
The problem with exposing both as done by this PR currently is that it is lying to the user in a kind of unexpected way. Try the RbfBumpFeeDialog: in manual fee edit mode, start increasing the absolute fee by 1 satoshi a time: +1 sat, +1 sat, +1 sat. Observe that the actual abs fee does not increase, only occasionally. The calculation relies on the feerate with a 0.1 sat/vbyte precision to be an intermediate value: so the actual abs fee can only be changed with a granularity of ~15 sats (depending on tx size -- it is only the feerate that the user can truly change)
somewhat related: ce8f564
There was a problem hiding this comment.
I have a patch that tries to tackle this, but it breaks the layout (hence my other comment that the layout is brittle :D)
See 154b704
SomberNight
left a comment
SomberNight
left a comment
There was a problem hiding this comment.
Thanks a lot for the work here.
It only took us 10 years to add precise fee selection to the Android app! :D
I think a lot of users will be happy :)
3 participants
also improve warning box styling consistency across finalizers,
add CPFP new feerate > old feerate check,
add relayfee checks for CPFP, DSCancel,
proper warning string for no dynamic fee estimates.
fixes #6710 #10273