search

trezor / trezor-firmware

:lock: Trezor Firmware Monorepo
https://trezor.io
Other
1.35k stars 654 forks source link

Fee rate not displayed when replacing transaction #2442

Closed matejcik closed 2 years ago

matejcik commented 2 years ago

Describe the bug

1294 implements fee rate for "sign transaction" flow.

Fee rate should probably be displayed on "modify fee" screen as well.

screenshots: https://satoshilabs.gitlab.io/-/trezor/trezor-firmware/-/jobs/2826526641/artifacts/test_ui_report/passed/TT_bitcoin-test_signtx_replacement.py::test_p2tr_fee_bump.html

sime commented 2 years ago

We have the space to include the fee rate?

Concept: https://www.figma.com/file/l0gG9XeRJ8FTDQ3cfb1wv9/Trezor-T%2C-Trezor-One?node-id=3539%3A10964

matejcik commented 2 years ago

this design is for the "total for transaction" screen, not for "modify fee" when replacing. screenshots: https://satoshilabs.gitlab.io/-/trezor/trezor-firmware/-/jobs/2930791419/artifacts/test_ui_report/passed/TT_bitcoin-test_signtx_replacement.py::test_p2pkh_fee_bump.html relevant screen copied so it doesn't expire again: image

with that said, the same concept should work here -- the screen fits exactly one additional line, so if the fee rate does not have a heading of its own, it will work fine

grdddj commented 2 years ago

QA testing instructions:

image

grdddj commented 2 years ago

We might consider doing it also for legacy, the fee-rate is not shown there when replacing.

There is also one free row left, where we could fit it. Question is whether it is worth the effort...

image

matejcik commented 2 years ago

this issue is also tagged legacy so I believe that applies, yes

grdddj commented 2 years ago

this issue is also tagged legacy so I believe that applies, yes

Done in https://github.com/trezor/trezor-firmware/pull/2481

STew790 commented 2 years ago

QA OK

Trezor T: image Trezor One: image

Info: