diff --git a/core/.changelog.d/2442.added b/core/.changelog.d/2442.added new file mode 100644 index 0000000000..c9b38f7128 --- /dev/null +++ b/core/.changelog.d/2442.added @@ -0,0 +1 @@ +Show fee rate when replacing transaction diff --git a/core/src/apps/bitcoin/sign_tx/approvers.py b/core/src/apps/bitcoin/sign_tx/approvers.py index 0d0286539b..f3b0e33606 100644 --- a/core/src/apps/bitcoin/sign_tx/approvers.py +++ b/core/src/apps/bitcoin/sign_tx/approvers.py @@ -249,6 +249,7 @@ class BasicApprover(Approver): total = self.total_in - self.change_out spending = total - self.external_in tx_size_vB = self.weight.get_virtual_size() + fee_rate = fee / tx_size_vB # fee_threshold = (coin.maxfee per byte * tx size) fee_threshold = (self.coin.maxfee_kb / 1000) * tx_size_vB @@ -295,14 +296,14 @@ class BasicApprover(Approver): # coming entirely from the user's own funds and from decreases of external outputs. # We consider the decreases as belonging to the user. await helpers.confirm_modify_fee( - fee - orig_fee, fee, self.coin, self.amount_unit + fee - orig_fee, fee, fee_rate, self.coin, self.amount_unit ) elif spending > orig_spending: # PayJoin and user is spending more: Show the increase in the user's contribution # to the fee, ignoring any contribution from external inputs. Decreasing of # external outputs is not allowed in PayJoin, so there is no need to handle those. await helpers.confirm_modify_fee( - spending - orig_spending, fee, self.coin, self.amount_unit + spending - orig_spending, fee, fee_rate, self.coin, self.amount_unit ) else: # PayJoin and user is not spending more: When new external inputs are involved and @@ -317,7 +318,6 @@ class BasicApprover(Approver): ) if not self.external_in: - fee_rate = fee / tx_size_vB await helpers.confirm_total( total, fee, fee_rate, self.coin, self.amount_unit ) diff --git a/core/src/apps/bitcoin/sign_tx/helpers.py b/core/src/apps/bitcoin/sign_tx/helpers.py index 482b4113aa..e1277d5111 100644 --- a/core/src/apps/bitcoin/sign_tx/helpers.py +++ b/core/src/apps/bitcoin/sign_tx/helpers.py @@ -116,17 +116,24 @@ class UiConfirmModifyFee(UiConfirm): self, user_fee_change: int, total_fee_new: int, + fee_rate: float, coin: CoinInfo, amount_unit: AmountUnit, ): self.user_fee_change = user_fee_change self.total_fee_new = total_fee_new + self.fee_rate = fee_rate self.coin = coin self.amount_unit = amount_unit def confirm_dialog(self, ctx: wire.Context) -> Awaitable[Any]: return layout.confirm_modify_fee( - ctx, self.user_fee_change, self.total_fee_new, self.coin, self.amount_unit + ctx, + self.user_fee_change, + self.total_fee_new, + self.fee_rate, + self.coin, + self.amount_unit, ) @@ -230,8 +237,12 @@ def confirm_modify_output(txo: TxOutput, orig_txo: TxOutput, coin: CoinInfo, amo return (yield UiConfirmModifyOutput(txo, orig_txo, coin, amount_unit)) -def confirm_modify_fee(user_fee_change: int, total_fee_new: int, coin: CoinInfo, amount_unit: AmountUnit) -> Awaitable[Any]: # type: ignore [awaitable-is-generator] - return (yield UiConfirmModifyFee(user_fee_change, total_fee_new, coin, amount_unit)) +def confirm_modify_fee(user_fee_change: int, total_fee_new: int, fee_rate: float, coin: CoinInfo, amount_unit: AmountUnit) -> Awaitable[Any]: # type: ignore [awaitable-is-generator] + return ( + yield UiConfirmModifyFee( + user_fee_change, total_fee_new, fee_rate, coin, amount_unit + ) + ) def confirm_total(spending: int, fee: int, fee_rate: float, coin: CoinInfo, amount_unit: AmountUnit) -> Awaitable[None]: # type: ignore [awaitable-is-generator] diff --git a/core/src/apps/bitcoin/sign_tx/layout.py b/core/src/apps/bitcoin/sign_tx/layout.py index 904413a331..9ec585232d 100644 --- a/core/src/apps/bitcoin/sign_tx/layout.py +++ b/core/src/apps/bitcoin/sign_tx/layout.py @@ -154,6 +154,7 @@ async def confirm_modify_fee( ctx: wire.Context, user_fee_change: int, total_fee_new: int, + fee_rate: float, coin: CoinInfo, amount_unit: AmountUnit, ) -> None: @@ -162,6 +163,7 @@ async def confirm_modify_fee( user_fee_change, format_coin_amount(abs(user_fee_change), coin, amount_unit), format_coin_amount(total_fee_new, coin, amount_unit), + fee_rate_amount=_get_fee_rate_str(fee_rate, coin), ) @@ -179,6 +181,18 @@ async def confirm_joint_total( ) +def _get_fee_rate_str(fee_rate: float, coin: CoinInfo) -> str | None: + if fee_rate >= 0: + # Use format_amount to get correct thousands separator -- micropython's built-in + # formatting doesn't add thousands sep to floating point numbers. + # We multiply by 10 to get a fixed-point integer with one decimal place, + # and add 0.5 to round to the nearest integer. + fee_rate_formatted = format_amount(int(fee_rate * 10 + 0.5), 1) + return f"({fee_rate_formatted} sat/{'v' if coin.segwit else ''}B)" + else: + return None + + async def confirm_total( ctx: wire.Context, spending: int, @@ -187,21 +201,11 @@ async def confirm_total( coin: CoinInfo, amount_unit: AmountUnit, ) -> None: - fee_rate_str: str | None = None - - if fee_rate >= 0: - # Use format_amount to get correct thousands separator -- micropython's built-in - # formatting doesn't add thousands sep to floating point numbers. - # We multiply by 10 to get a fixed-point integer with one decimal place, - # and add 0.5 to round to the nearest integer. - fee_rate_formatted = format_amount(int(fee_rate * 10 + 0.5), 1) - fee_rate_str = f"({fee_rate_formatted} sat/{'v' if coin.segwit else ''}B)" - await layouts.confirm_total( ctx, total_amount=format_coin_amount(spending, coin, amount_unit), fee_amount=format_coin_amount(fee, coin, amount_unit), - fee_rate_amount=fee_rate_str, + fee_rate_amount=_get_fee_rate_str(fee_rate, coin), ) diff --git a/core/src/trezor/ui/layouts/tt/__init__.py b/core/src/trezor/ui/layouts/tt/__init__.py index 516b209ad7..9a87cd6b9b 100644 --- a/core/src/trezor/ui/layouts/tt/__init__.py +++ b/core/src/trezor/ui/layouts/tt/__init__.py @@ -980,6 +980,7 @@ async def confirm_modify_fee( sign: int, user_fee_change: str, total_fee_new: str, + fee_rate_amount: str | None = None, ) -> None: text = Text("Modify fee", ui.ICON_SEND, ui.GREEN, new_lines=False) if sign == 0: @@ -991,9 +992,10 @@ async def confirm_modify_fee( text.normal("Increase your fee by:\n") text.bold(user_fee_change) text.br() - text.br_half() text.normal("Transaction fee:\n") text.bold(total_fee_new) + if fee_rate_amount is not None: + text.normal("\n" + fee_rate_amount) await raise_if_cancelled( interact(ctx, HoldToConfirm(text), "modify_fee", ButtonRequestType.SignTx) ) diff --git a/core/src/trezor/ui/layouts/tt_v2/__init__.py b/core/src/trezor/ui/layouts/tt_v2/__init__.py index fc198eead3..f32d7e2635 100644 --- a/core/src/trezor/ui/layouts/tt_v2/__init__.py +++ b/core/src/trezor/ui/layouts/tt_v2/__init__.py @@ -743,6 +743,7 @@ async def confirm_modify_fee( sign: int, user_fee_change: str, total_fee_new: str, + fee_rate_amount: str | None = None, ) -> None: result = await interact( ctx, diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index 177bef3354..9f653dbefb 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -947,20 +947,20 @@ "TT_bitcoin-test_signtx_replacement.py::test_attack_fake_int_input_amount": "1c100ce4b7c1e47e72428f390de0846c1ff933e9f07894872644a369a9422738", "TT_bitcoin-test_signtx_replacement.py::test_attack_false_internal": "1c100ce4b7c1e47e72428f390de0846c1ff933e9f07894872644a369a9422738", "TT_bitcoin-test_signtx_replacement.py::test_attack_steal_change": "04ba3e05862eef616957f9f0c67f7a1d841a3bc7278c5fd999dfba4ebeee5314", -"TT_bitcoin-test_signtx_replacement.py::test_p2pkh_fee_bump": "9521ab8dcf72d514103ec16410e3ad7d8e384084c3a645178ab191315911cd13", -"TT_bitcoin-test_signtx_replacement.py::test_p2tr_fee_bump": "d2adcdaf62181c703628e355ba84ff979a81426429ee487c0059a9f161fbeb25", -"TT_bitcoin-test_signtx_replacement.py::test_p2tr_invalid_signature": "d2adcdaf62181c703628e355ba84ff979a81426429ee487c0059a9f161fbeb25", -"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_finalize": "66d0c98fe05fab4e77a516434883f81fa9516464373374097e91a8e3c278a6b3", -"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_in_p2sh_fee_bump_from_external": "d3d97e310fb2d1d29b351bdc35b359aab75b4bba7afd257eb85100031edaacbf", -"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_in_p2sh_remove_change": "ae9e7bc83227cf6187fb5c689515b66da7bf8fcc58a43128389f6eb7ab35240d", -"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_invalid_signature": "66d0c98fe05fab4e77a516434883f81fa9516464373374097e91a8e3c278a6b3", -"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_op_return_fee_bump": "f0d4e746eb927051ae7144fd9cbc63dc26aa61fe79e97eb31e74677ecf65eda9", +"TT_bitcoin-test_signtx_replacement.py::test_p2pkh_fee_bump": "46f53aa43a1ab3b040716ad7090f602695b77e8250e65b7024d2ace11ffdb37b", +"TT_bitcoin-test_signtx_replacement.py::test_p2tr_fee_bump": "cd02aade75d2b644867fa98d07fdc6f1b22372286718980c5aac8970c4594168", +"TT_bitcoin-test_signtx_replacement.py::test_p2tr_invalid_signature": "cd02aade75d2b644867fa98d07fdc6f1b22372286718980c5aac8970c4594168", +"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_finalize": "90948701923e3f288fdf04a27198644f042fed8993d683dbdd577e676785e5d4", +"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_in_p2sh_fee_bump_from_external": "9c625a9e2d1895d410e4c1c6a968b8b53deee3700e5b1902f491d1c7401de122", +"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_in_p2sh_remove_change": "6c4f7638aa2ebf7105b3bb263e2c68c9771581738d6e2aa137df89dfb6d383ee", +"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_invalid_signature": "90948701923e3f288fdf04a27198644f042fed8993d683dbdd577e676785e5d4", +"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_op_return_fee_bump": "a65bb3df6ba4f68c6060efe5eef5fd1e66d6e9457becf6c4e8112fec028d153b", "TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_payjoin[19909659-90000-02483045022100aa1b91-c9b963ae": "da3ec44de0435cca3828752a0ba44483b2f087d1ae02c99530be4bcd01b80e57", "TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_payjoin[19909718-90000-024730440220753f5304-ecb983d1": "da3ec44de0435cca3828752a0ba44483b2f087d1ae02c99530be4bcd01b80e57", "TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_payjoin[19909800-89859-0248304502210097a42b-7a89e474": "da3ec44de0435cca3828752a0ba44483b2f087d1ae02c99530be4bcd01b80e57", -"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_payjoin[19909859-89800-02483045022100af3a87-80428fad": "f5be02a50a1876ac0478e37e41bca38c5feb569613dc5b105b95c6bb4763514a", +"TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_payjoin[19909859-89800-02483045022100af3a87-80428fad": "546caace6b10e9c9993b12033a8df2627a85d183c1eff2e090c5ff280dd25665", "TT_bitcoin-test_signtx_replacement.py::test_p2wpkh_payjoin[19909859-89859-02483045022100eb74ab-881c7bef": "da3ec44de0435cca3828752a0ba44483b2f087d1ae02c99530be4bcd01b80e57", -"TT_bitcoin-test_signtx_replacement.py::test_tx_meld": "1a5221c169069689cd038a1b493932e42f0c740efad52e3c77bb636786af594e", +"TT_bitcoin-test_signtx_replacement.py::test_tx_meld": "a08f8f4b137150d03031a16a615f62ef8134f5599f43e5cd2057aa1a1d144b79", "TT_bitcoin-test_signtx_segwit.py::test_attack_change_input_address": "f3398d5814422383b9a6fa8cce53948af05e985616c20e7f3aed649a9d3fe1a6", "TT_bitcoin-test_signtx_segwit.py::test_attack_mixed_inputs": "a06e89d99c1e218fba7ceff794cb9dab98bbae666b397996e067fe9f0bd8a009", "TT_bitcoin-test_signtx_segwit.py::test_send_multisig_1": "cfd3b3e882067b1e42a36623da3523829bf579399b14f8711e09f7e89aa82d65",