Merge pull request #696 from LedgerHQ/feat/apa/blind_signing_non_zero…

…_amount

Show non-zero amounts in blind-signing flow

src/shared_context.h

@@ -131,6 +131,7 @@ typedef struct txStringProperties_s {
     char maxFee[50];
     char nonce[8];  // 10M tx per account ought to be enough for everybody
     char network_name[NETWORK_STRING_MAX_SIZE + 1];
+    char tx_hash[2 + (INT256_LENGTH * 2) + 1];
 } txStringProperties_t;
 
 #ifdef TARGET_NANOS

src_bagl/ui_flow_signTx.c

@@ -120,7 +120,7 @@ UX_STEP_NOCB(ux_approval_tx_hash_step,
 #else
       .title = "Transaction hash",
 #endif
-      .text = strings.common.fullAmount
+      .text = strings.common.tx_hash
     });
 UX_STEP_NOCB(
     ux_approval_amount_step,
@@ -249,8 +249,8 @@ void ux_approve_tx(bool fromPlugin) {
     } else {
         if (tmpContent.txContent.dataPresent) {
 #pragma GCC diagnostic ignored "-Wformat"
-            snprintf(strings.common.fullAmount,
-                     sizeof(strings.common.fullAmount),
+            snprintf(strings.common.tx_hash,
+                     sizeof(strings.common.tx_hash),
                      "0x%.*h",
                      sizeof(tmpCtx.transactionContext.hash),
                      tmpCtx.transactionContext.hash);
@@ -261,7 +261,8 @@ void ux_approve_tx(bool fromPlugin) {
         if (strings.common.fromAddress[0] != 0) {
             ux_approval_tx_flow[step++] = &ux_approval_from_step;
         }
-        if (!tmpContent.txContent.dataPresent) {
+        if (!tmpContent.txContent.dataPresent ||
+            !allzeroes(tmpContent.txContent.value.value, tmpContent.txContent.value.length)) {
             ux_approval_tx_flow[step++] = &ux_approval_amount_step;
         }
 #ifdef HAVE_TRUSTED_NAME

src_nbgl/ui_approve_tx.c

@@ -122,14 +122,14 @@ static uint8_t setTagValuePairs(void) {
     } else {
         if (tmpContent.txContent.dataPresent) {
 #pragma GCC diagnostic ignored "-Wformat"
-            snprintf(strings.common.fullAmount,
-                     sizeof(strings.common.fullAmount),
+            snprintf(strings.common.tx_hash,
+                     sizeof(strings.common.tx_hash),
                      "0x%.*h",
                      sizeof(tmpCtx.transactionContext.hash),
                      tmpCtx.transactionContext.hash);
 #pragma GCC diagnostic warning "-Wformat"
             pairs[nbPairs].item = "Transaction hash";
-            pairs[nbPairs].value = strings.common.fullAmount;
+            pairs[nbPairs].value = strings.common.tx_hash;
             nbPairs++;
         }
 
@@ -139,7 +139,8 @@ static uint8_t setTagValuePairs(void) {
             nbPairs++;
         }
 
-        if (!tmpContent.txContent.dataPresent) {
+        if (!tmpContent.txContent.dataPresent ||
+            !allzeroes(tmpContent.txContent.value.value, tmpContent.txContent.value.length)) {
             pairs[nbPairs].item = "Amount";
             pairs[nbPairs].value = strings.common.fullAmount;
             nbPairs++;

tests/ragger/test_blind_sign.py

@@ -1,6 +1,8 @@
 from pathlib import Path
 import json
 from typing import Optional
+import time
+
 import pytest
 from web3 import Web3
 
@@ -23,22 +25,27 @@
 # TODO: do one test with nonce display
 
 
-@pytest.fixture(name="sign", params=[True, False])
-def sign_fixture(request) -> bool:
+@pytest.fixture(name="reject", params=[False, True])
+def reject_fixture(request) -> bool:
     return request.param
 
 
-def common_tx_params() -> dict:
+@pytest.fixture(name="amount", params=[0.0, 1.2])
+def amount_fixture(request) -> float:
+    return request.param
+
+
+def common_tx_params(amount: float) -> dict:
     with open(f"{ABIS_FOLDER}/erc20.json", encoding="utf-8") as file:
         contract = Web3().eth.contract(
             abi=json.load(file),
             address=None
         )
-    data = contract.encode_abi("approve", [
-        # Uniswap Protocol: Permit2
-        bytes.fromhex("000000000022d473030f116ddee9f6b43ac78ba3"),
-        Web3.to_wei("2", "ether")
+    # this function is not handled by the internal plugin, so won't check if value == 0
+    data = contract.encode_abi("balanceOf", [
+        bytes.fromhex("d8dA6BF26964aF9D7eEd9e03E53415D37aA96045"),
     ])
+
     return {
         "nonce": 235,
         "maxFeePerGas": Web3.to_wei(100, "gwei"),
@@ -47,7 +54,8 @@ def common_tx_params() -> dict:
         # Maker: Dai Stablecoin
         "to": bytes.fromhex("6b175474e89094c44da98b954eedeac495271d0f"),
         "data": data,
-        "chainId": 1
+        "chainId": 1,
+        "value": Web3.to_wei(amount, "ether"),
     }
 
 
@@ -57,24 +65,28 @@ def test_blind_sign(firmware: Firmware,
                     navigator: Navigator,
                     default_screenshot_path: Path,
                     test_name: str,
-                    sign: bool):
+                    reject: bool,
+                    amount: float):
     global DEVICE_ADDR
     app_client = EthAppClient(backend)
 
+    if reject and amount > 0.0:
+        pytest.skip()
     settings_toggle(firmware, navigator, [SettingID.BLIND_SIGNING])
     if DEVICE_ADDR is None:
         with app_client.get_public_addr(bip32_path=BIP32_PATH, display=False):
             pass
         _, DEVICE_ADDR, _ = ResponseParser.pk_addr(app_client.response().data)
 
-    tx_params = common_tx_params()
+    tx_params = common_tx_params(amount)
     try:
         with app_client.sign(BIP32_PATH, tx_params):
-            if sign:
-                test_name += "_signed"
-            else:
+            if reject:
                 test_name += "_rejected"
 
+            if amount > 0.0:
+                test_name += "_nonzero"
+
             moves = []
             if firmware.is_nano:
                 # blind signing warning
@@ -95,6 +107,10 @@ def test_blind_sign(firmware: Firmware,
                 else:
                     moves += [NavInsID.RIGHT_CLICK]
 
+                # amount
+                if amount > 0.0:
+                    moves += [NavInsID.RIGHT_CLICK]
+
                 # to
                 if firmware == Firmware.NANOS:
                     moves += [NavInsID.RIGHT_CLICK] * 3
@@ -104,7 +120,7 @@ def test_blind_sign(firmware: Firmware,
                 # fees
                 moves += [NavInsID.RIGHT_CLICK]
 
-                if not sign:
+                if reject:
                     moves += [NavInsID.RIGHT_CLICK]
 
                 moves += [NavInsID.BOTH_CLICK]
@@ -113,7 +129,7 @@ def test_blind_sign(firmware: Firmware,
 
                 moves += [NavInsID.SWIPE_CENTER_TO_LEFT] * 3
 
-                if sign:
+                if not reject:
                     moves += [NavInsID.USE_CASE_REVIEW_CONFIRM]
                 else:
                     moves += [NavInsID.USE_CASE_REVIEW_REJECT]
@@ -123,10 +139,10 @@ def test_blind_sign(firmware: Firmware,
                                            test_name,
                                            moves)
     except ExceptionRAPDU as e:
-        assert not sign
+        assert reject
         assert e.status == StatusWord.CONDITION_NOT_SATISFIED
     else:
-        assert sign
+        assert not reject
         # verify signature
         vrs = ResponseParser.signature(app_client.response().data)
         addr = recover_transaction(tx_params, vrs)
@@ -144,7 +160,7 @@ def test_blind_sign_reject_in_risk_review(firmware: Firmware,
     settings_toggle(firmware, navigator, [SettingID.BLIND_SIGNING])
 
     try:
-        with app_client.sign(BIP32_PATH, common_tx_params()):
+        with app_client.sign(BIP32_PATH, common_tx_params(0.0)):
             moves = [NavInsID.USE_CASE_CHOICE_CONFIRM]
             navigator.navigate(moves)
     except ExceptionRAPDU as e:
@@ -170,7 +186,7 @@ def test_sign_parameter_selector(firmware: Firmware,
 
     settings_toggle(firmware, navigator, [SettingID.BLIND_SIGNING, SettingID.DEBUG_DATA])
 
-    tx_params = common_tx_params()
+    tx_params = common_tx_params(0.0)
     data_len = len(bytes.fromhex(tx_params["data"][2:]))
     params = (data_len - 4) // 32
     with app_client.sign(BIP32_PATH, tx_params):
@@ -179,11 +195,7 @@ def test_sign_parameter_selector(firmware: Firmware,
             # verify | selector
             moves += [NavInsID.RIGHT_CLICK] * 2 + [NavInsID.BOTH_CLICK]
             if firmware == Firmware.NANOS:
-                # hardcoded for each because for some params take more pages than others
-                # parameter 1
-                moves += [NavInsID.RIGHT_CLICK] * 4 + [NavInsID.BOTH_CLICK]
-                # parameter 2
-                moves += [NavInsID.RIGHT_CLICK] * 3 + [NavInsID.BOTH_CLICK]
+                moves += ([NavInsID.RIGHT_CLICK] * 4 + [NavInsID.BOTH_CLICK]) * params
                 # blind signing | review | tx hash | from | to | fees
                 moves += [NavInsID.RIGHT_CLICK] * 13
             else:
@@ -216,10 +228,11 @@ def test_blind_sign_not_enabled_error(firmware: Firmware,
     app_client = EthAppClient(backend)
 
     try:
-        with app_client.sign(BIP32_PATH, common_tx_params()):
+        with app_client.sign(BIP32_PATH, common_tx_params(0.0)):
             moves = []
             if firmware.is_nano:
                 if firmware == Firmware.NANOS:
+                    time.sleep(0.5)  # seems to take some time
                     moves += [NavInsID.RIGHT_CLICK]
                 moves += [NavInsID.BOTH_CLICK]
             else: