core/bitcoin: Show warning if nLockTime is set but ineffective due to all nSequence values being 0xffffffff.

core/CHANGELOG.md

@@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Security
 - Show non-empty passphrase on device when it was entered on host.
+- Show warning if nLockTime is set but ineffective due to all nSequence values being 0xffffffff.
 
 ## 2.3.2 [5th August 2020]
 

core/src/apps/bitcoin/sign_tx/__init__.py

@@ -72,7 +72,9 @@ async def sign_tx(
             )
             progress.report_init()
         elif isinstance(req, helpers.UiConfirmNonDefaultLocktime):
-            res = await layout.confirm_nondefault_locktime(ctx, req.lock_time)
+            res = await layout.confirm_nondefault_locktime(
+                ctx, req.lock_time, req.lock_time_disabled
+            )
             progress.report_init()
         elif isinstance(req, helpers.UiConfirmForeignAddress):
             res = await paths.show_path_warning(ctx, req.address_n)

core/src/apps/bitcoin/sign_tx/approvers.py

@@ -15,6 +15,9 @@ from . import helpers, tx_weight
 if False:
     from ..authorization import CoinJoinAuthorization
 
+# Setting nSequence to this value for every input in a transaction disables nLockTime.
+_SEQUENCE_FINAL = const(0xFFFFFFFF)
+
 
 # An Approver object computes the transaction totals and either prompts the user
 # to confirm transaction parameters (output addresses, amounts and fees) or uses
@@ -25,6 +28,7 @@ class Approver:
         self.tx = tx
         self.coin = coin
         self.weight = tx_weight.TxWeightCalculator(tx.inputs_count, tx.outputs_count)
+        self.min_sequence = _SEQUENCE_FINAL  # the minimum nSequence of all inputs
 
         # amounts
         self.total_in = 0  # sum of input amounts
@@ -35,11 +39,13 @@ class Approver:
     async def add_internal_input(self, txi: TxInputType, amount: int) -> None:
         self.weight.add_input(txi)
         self.total_in += amount
+        self.min_sequence = min(self.min_sequence, txi.sequence)
 
     def add_external_input(self, txi: TxInputType) -> None:
         self.weight.add_input(txi)
         self.total_in += txi.amount
         self.external_in += txi.amount
+        self.min_sequence = min(self.min_sequence, txi.sequence)
 
     def add_change_output(self, txo: TxOutputType, script_pubkey: bytes) -> None:
         self.weight.add_output(script_pubkey)
@@ -100,7 +106,10 @@ class BasicApprover(Approver):
         if self.change_count > self.MAX_SILENT_CHANGE_COUNT:
             await helpers.confirm_change_count_over_threshold(self.change_count)
         if self.tx.lock_time > 0:
-            await helpers.confirm_nondefault_locktime(self.tx.lock_time)
+            lock_time_disabled = self.min_sequence == _SEQUENCE_FINAL
+            await helpers.confirm_nondefault_locktime(
+                self.tx.lock_time, lock_time_disabled
+            )
         if not self.external_in:
             await helpers.confirm_total(total, fee, self.coin)
         else:

core/src/apps/bitcoin/sign_tx/helpers.py

@@ -76,8 +76,9 @@ class UiConfirmForeignAddress:
 
 
 class UiConfirmNonDefaultLocktime:
-    def __init__(self, lock_time: int):
+    def __init__(self, lock_time: int, lock_time_disabled):
         self.lock_time = lock_time
+        self.lock_time_disabled = lock_time_disabled
 
     __eq__ = utils.obj_eq
 
@@ -106,8 +107,8 @@ def confirm_foreign_address(address_n: list) -> Awaitable[Any]:  # type: ignore
     return (yield UiConfirmForeignAddress(address_n))
 
 
-def confirm_nondefault_locktime(lock_time: int) -> Awaitable[Any]:  # type: ignore
+def confirm_nondefault_locktime(lock_time: int, lock_time_disabled: bool) -> Awaitable[Any]:  # type: ignore
-    return (yield UiConfirmNonDefaultLocktime(lock_time))
+    return (yield UiConfirmNonDefaultLocktime(lock_time, lock_time_disabled))
 
 
 def request_tx_meta(tx_req: TxRequest, coin: CoinInfo, tx_hash: bytes = None) -> Awaitable[Any]:  # type: ignore

core/src/apps/bitcoin/sign_tx/layout.py

@@ -101,13 +101,21 @@ async def confirm_change_count_over_threshold(
     await require_confirm(ctx, text, ButtonRequestType.SignTx)
 
 
-async def confirm_nondefault_locktime(ctx: wire.Context, lock_time: int) -> None:
+async def confirm_nondefault_locktime(
+    ctx: wire.Context, lock_time: int, lock_time_disabled: bool
+) -> None:
+    if lock_time_disabled:
+        text = Text("Warning", ui.ICON_SEND, ui.GREEN)
+        text.normal("Locktime is set but will", "have no effect.")
+        text.br_half()
+    else:
         text = Text("Confirm locktime", ui.ICON_SEND, ui.GREEN)
-    text.normal("Locktime for this transaction is set to")
+        text.normal("Locktime for this", "transaction is set to")
         if lock_time < _LOCKTIME_TIMESTAMP_MIN_VALUE:
             text.normal("blockheight:")
         else:
             text.normal("timestamp:")
         text.bold(str(lock_time))
+
     text.normal("Continue?")
     await require_confirm(ctx, text, ButtonRequestType.SignTx)

core/tests/test_apps.bitcoin.approver.py

@@ -35,6 +35,7 @@ class TestApprover(unittest.TestCase):
             TxInputType(
                 amount=denomination + 1000000 * (i + 1),
                 script_type=InputScriptType.EXTERNAL,
+                sequence=0xffffffff,
             ) for i in range(99)
         ]
 
@@ -45,6 +46,7 @@ class TestApprover(unittest.TestCase):
                 address_n=[H_(84), H_(0), H_(0), 0, 1],
                 amount=denomination + 1000000,
                 script_type=InputScriptType.SPENDWITNESS,
+                sequence=0xffffffff,
             )
         )
 

core/tests/test_apps.bitcoin.segwit.signtx.native_p2wpkh_grs.py

@@ -103,7 +103,7 @@ class TestSignSegwitTxNativeP2WPKH_GRS(unittest.TestCase):
             helpers.UiConfirmOutput(out2, coin),
             True,
 
-            helpers.UiConfirmNonDefaultLocktime(tx.lock_time),
+            helpers.UiConfirmNonDefaultLocktime(tx.lock_time, lock_time_disabled=False),
             True,
 
             helpers.UiConfirmTotal(12300000, 11000, coin),
@@ -225,7 +225,7 @@ class TestSignSegwitTxNativeP2WPKH_GRS(unittest.TestCase):
             TxRequest(request_type=TXOUTPUT, details=TxRequestDetailsType(request_index=1, tx_hash=None), serialized=EMPTY_SERIALIZED),
             TxAck(tx=TransactionType(outputs=[out2])),
 
-            helpers.UiConfirmNonDefaultLocktime(tx.lock_time),
+            helpers.UiConfirmNonDefaultLocktime(tx.lock_time, lock_time_disabled=False),
             True,
 
             helpers.UiConfirmTotal(5000000 + 11000, 11000, coin),

core/tests/test_apps.bitcoin.segwit.signtx.p2wpkh_in_p2sh_grs.py

@@ -103,7 +103,7 @@ class TestSignSegwitTxP2WPKHInP2SH_GRS(unittest.TestCase):
             helpers.UiConfirmOutput(out2, coin),
             True,
 
-            helpers.UiConfirmNonDefaultLocktime(tx.lock_time),
+            helpers.UiConfirmNonDefaultLocktime(tx.lock_time, lock_time_disabled=False),
             True,
 
             helpers.UiConfirmTotal(123445789 + 11000, 11000, coin),
@@ -225,7 +225,7 @@ class TestSignSegwitTxP2WPKHInP2SH_GRS(unittest.TestCase):
             TxRequest(request_type=TXOUTPUT, details=TxRequestDetailsType(request_index=1, tx_hash=None), serialized=EMPTY_SERIALIZED),
             TxAck(tx=TransactionType(outputs=[out2])),
 
-            helpers.UiConfirmNonDefaultLocktime(tx.lock_time),
+            helpers.UiConfirmNonDefaultLocktime(tx.lock_time, lock_time_disabled=False),
             True,
 
             helpers.UiConfirmTotal(12300000 + 11000, 11000, coin),

tests/ui_tests/fixtures.json

@@ -332,14 +332,14 @@
 "test_msg_signtx_external.py::test_p2wsh_external_presigned": "8374d50b803db0160de39ce7e5a4170112f4c99d703490920a2de735bd261bda",
 "test_msg_signtx_grs.py-test_legacy": "3a80ea724a93ed225d64f8def739d63b11f8c096455f971feabec8be6f7597fb",
 "test_msg_signtx_grs.py-test_legacy_change": "8dfc140534bdaa08f6916831dc0d510f57b07617f30df748e4e0456d4dd93ece",
-"test_msg_signtx_grs.py-test_send_segwit_native": "a1098f71dd6c197cb8a4a45806808fbacc130044585cb003e61110d8bda80114",
-"test_msg_signtx_grs.py-test_send_segwit_native_change": "2b9119aea79b50a26bd86cc49fd5421aedee42245b83919bf142d2ca2e50dd16",
-"test_msg_signtx_grs.py-test_send_segwit_p2sh": "e596ff7f2e10633d8de6bd5b98928020dfacf578ce71d6d6e0ca27011ed8bd53",
-"test_msg_signtx_grs.py-test_send_segwit_p2sh_change": "65a33cc9ecf52bdab93d1066bfaff7f30c9908fc5ada8a672c39df57eb6129c2",
 "test_msg_signtx_invalid_path.py-test_invalid_path_fail": "b0f22cba2dbab2cd21c15c002b66ed89b6c728b10daa8d0c0e78abd4164a3912",
 "test_msg_signtx_invalid_path.py-test_invalid_path_pass_forkid": "667dcb09b569e5b4e091e6b1ac7e8e057c0c730c931b22f8c0ee64050f3f467b",
-"test_msg_signtx_komodo.py-test_one_one_fee_sapling": "aa32424c04f65c0e2de57f6eb26830eb037c7b4daaf300ae61d30968affd9193",
+"test_msg_signtx_grs.py-test_send_segwit_native": "82dfa15178d33e757da58943aff36dcc0eebb984e34832b71f6ca09b2a525cbc",
-"test_msg_signtx_komodo.py-test_one_one_rewards_claim": "e53f221fda81469027e39e21877a81a8fafbffbece0a45aeda12aae8873b0464",
+"test_msg_signtx_grs.py-test_send_segwit_native_change": "d8ae74de3aada1d136c4119f2306a63bd109901ce15d00ae916ba5b4457e798e",
+"test_msg_signtx_grs.py-test_send_segwit_p2sh": "9ab885dd3b390813f8a47e1d1587abe07ab713e9f8696dc667b3a2925f23c103",
+"test_msg_signtx_grs.py-test_send_segwit_p2sh_change": "6c352ab975a75a150f7c3415a967fb8635395ff8db0de89ecb9c2011cb519509",
+"test_msg_signtx_komodo.py-test_one_one_fee_sapling": "14bad8852ee51f6fec12677cced9ffafa0cbae91b4ba94e988a800544072ed21",
+"test_msg_signtx_komodo.py-test_one_one_rewards_claim": "751e83d63bf01c6c57047b5e004629d613df75342371cd43a7b4b80a07f4b88d",
 "test_msg_signtx_peercoin.py::test_timestamp_included": "825b9bdf5238c5c6415a254a6bae4b2bd9df8fc5cb31f66f0c20145cb4e60bbb",
 "test_msg_signtx_segwit.py-test_attack_change_input_address": "5ae71202c062ef7942626a80a4ceeb8d8c4ea5065a97f0de6a97505e9cb82c2c",
 "test_msg_signtx_segwit.py-test_attack_mixed_inputs": "ed4cf8ff26ca1abb0adf20e1020dad7861b5e63ead2a1a9c0c5e9080d37f6f1c",