Skip to content

Commit

Permalink
PR review.
Browse files Browse the repository at this point in the history
  • Loading branch information
agrojean-ledger committed May 28, 2024
1 parent b09dcc1 commit 02b1c32
Show file tree
Hide file tree
Showing 17 changed files with 58 additions and 126 deletions.
12 changes: 8 additions & 4 deletions src/app_ui/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ use crate::AppSW;
use core::str::from_utf8_mut;

#[cfg(not(any(target_os = "stax", target_os = "flex")))]
use ledger_device_sdk::ui::bitmaps::{CROSSMARK, EYE, VALIDATE_14};
#[cfg(not(any(target_os = "stax", target_os = "flex")))]
use ledger_device_sdk::ui::gadgets::{Field, MultiFieldReview};
use ledger_device_sdk::ui::{
bitmaps::{CROSSMARK, EYE, VALIDATE_14},
gadgets::{Field, MultiFieldReview},
};

#[cfg(any(target_os = "stax", target_os = "flex"))]
use ledger_device_sdk::nbgl::{NbglAddressReview, NbglGlyph};
Expand Down Expand Up @@ -68,6 +69,9 @@ pub fn ui_display_pk(addr: &[u8]) -> Result<bool, AppSW> {
// Load glyph from 64x64 4bpp gif file with include_gif macro. Creates an NBGL compatible glyph.
const FERRIS: NbglGlyph = NbglGlyph::from_include(include_gif!("crab_64x64.gif", NBGL));
// Display the address confirmation screen.
Ok(NbglAddressReview::new().glyph(&FERRIS).show(addr_hex))
Ok(NbglAddressReview::new()
.glyph(&FERRIS)
.verify_str("Verify CRAB address")
.show(addr_hex))
}
}
7 changes: 4 additions & 3 deletions src/app_ui/menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ use include_gif::include_gif;
use ledger_device_sdk::io::{Comm, Event};

#[cfg(not(any(target_os = "stax", target_os = "flex")))]
use ledger_device_sdk::ui::bitmaps::{Glyph, BACK, CERTIFICATE, DASHBOARD_X};
#[cfg(not(any(target_os = "stax", target_os = "flex")))]
use ledger_device_sdk::ui::gadgets::{EventOrPageIndex, MultiPageMenu, Page};
use ledger_device_sdk::ui::{
bitmaps::{Glyph, BACK, CERTIFICATE, DASHBOARD_X},
gadgets::{EventOrPageIndex, MultiPageMenu, Page},
};

#[cfg(any(target_os = "stax", target_os = "flex"))]
use crate::settings::Settings;
Expand Down
11 changes: 6 additions & 5 deletions src/app_ui/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ use crate::utils::concatenate;
use crate::AppSW;

#[cfg(not(any(target_os = "stax", target_os = "flex")))]
use ledger_device_sdk::ui::bitmaps::{CROSSMARK, EYE, VALIDATE_14};
#[cfg(not(any(target_os = "stax", target_os = "flex")))]
use ledger_device_sdk::ui::gadgets::{Field, MultiFieldReview};
use ledger_device_sdk::ui::{
bitmaps::{CROSSMARK, EYE, VALIDATE_14},
gadgets::{Field, MultiFieldReview},
};

#[cfg(any(target_os = "stax", target_os = "flex"))]
use crate::settings::Settings;
Expand Down Expand Up @@ -98,8 +99,8 @@ pub fn ui_display_tx(tx: &Tx) -> Result<bool, AppSW> {
// with constant generic parameters of NbglReview. Default values are 32 and 1024 respectively.
let mut review: NbglReview = NbglReview::new()
.titles(
"Please review transaction",
"To send CRAB",
"Review transaction\nto send CRAB",
"",
"Sign transaction\nto send CRAB",
)
.glyph(&FERRIS);
Expand Down
Binary file modified tests/snapshots/flex/test_get_public_key_confirm_accepted/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/flex/test_get_public_key_confirm_refused/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/flex/test_sign_tx_long_tx/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/flex/test_sign_tx_refused/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/flex/test_sign_tx_short_tx/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/flex/test_sign_tx_short_tx_no_memo/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/stax/test_get_public_key_confirm_accepted/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/stax/test_get_public_key_confirm_refused/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/stax/test_sign_tx_long_tx/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/stax/test_sign_tx_refused/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/stax/test_sign_tx_short_tx/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/stax/test_sign_tx_short_tx_no_memo/00000.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
54 changes: 12 additions & 42 deletions tests/test_pubkey_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,13 @@ def test_get_public_key_no_confirm(backend):


# In this test we check that the GET_PUBLIC_KEY works in confirmation mode
def test_get_public_key_confirm_accepted(firmware, backend, navigator, test_name):
def test_get_public_key_confirm_accepted(backend, scenario_navigator):
client = BoilerplateCommandSender(backend)
path = "m/44'/1'/0'/0/0"

with client.get_public_key_with_confirmation(path=path):
if firmware.device.startswith("nano"):
navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
[NavInsID.BOTH_CLICK],
"Approve",
ROOT_SCREENSHOT_PATH,
test_name)
else:
instructions = [
NavInsID.USE_CASE_REVIEW_TAP,
NavInsID.USE_CASE_ADDRESS_CONFIRMATION_CONFIRM,
NavInsID.WAIT_FOR_HOME_SCREEN
]
navigator.navigate_and_compare(ROOT_SCREENSHOT_PATH,
test_name,
instructions)
scenario_navigator.address_review_approve()

response = client.get_async_response().data
_, public_key, _, _ = unpack_get_public_key_response(response)

Expand All @@ -47,32 +35,14 @@ def test_get_public_key_confirm_accepted(firmware, backend, navigator, test_name


# In this test we check that the GET_PUBLIC_KEY in confirmation mode replies an error if the user refuses
def test_get_public_key_confirm_refused(firmware, backend, navigator, test_name):
def test_get_public_key_confirm_refused(backend, scenario_navigator):
client = BoilerplateCommandSender(backend)
path = "m/44'/1'/0'/0/0"

if firmware.device.startswith("nano"):
with pytest.raises(ExceptionRAPDU) as e:
with client.get_public_key_with_confirmation(path=path):
navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
[NavInsID.BOTH_CLICK],
"Reject",
ROOT_SCREENSHOT_PATH,
test_name)
# Assert that we have received a refusal
assert e.value.status == Errors.SW_DENY
assert len(e.value.data) == 0
else:
instructions = [
NavInsID.USE_CASE_REVIEW_TAP,
NavInsID.USE_CASE_ADDRESS_CONFIRMATION_CANCEL,
NavInsID.WAIT_FOR_HOME_SCREEN
]
with pytest.raises(ExceptionRAPDU) as e:
with client.get_public_key_with_confirmation(path=path):
navigator.navigate_and_compare(ROOT_SCREENSHOT_PATH,
test_name,
instructions)
# Assert that we have received a refusal
assert e.value.status == Errors.SW_DENY
assert len(e.value.data) == 0
with pytest.raises(ExceptionRAPDU) as e:
with client.get_public_key_with_confirmation(path=path):
scenario_navigator.address_review_reject()

# Assert that we have received a refusal
assert e.value.status == Errors.SW_DENY
assert len(e.value.data) == 0
100 changes: 28 additions & 72 deletions tests/test_sign_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@
from ragger.navigator import NavIns, NavInsID
from utils import ROOT_SCREENSHOT_PATH, check_signature_validity

# In this tests we check the behavior of the device when asked to sign a transaction
# In these tests we check the behavior of the device when asked to sign a transaction


# In this test se send to the device a transaction to sign and validate it on screen
# The transaction is short and will be sent in one chunk
# We will ensure that the displayed information is correct by using screenshots comparison
def test_sign_tx_short_tx(firmware, backend, navigator, test_name):
# In this test a transaction is sent to the device to be signed and validated on screen.
# The transaction is short and will be sent in one chunk.
# We will ensure that the displayed information is correct by using screenshots comparison.
def test_sign_tx_short_tx(backend, scenario_navigator, firmware, navigator):
# Use the app interface instead of raw interface
client = BoilerplateCommandSender(backend)
# The path used for this entire test
Expand All @@ -39,37 +38,24 @@ def test_sign_tx_short_tx(firmware, backend, navigator, test_name):
NavInsID.USE_CASE_SUB_SETTINGS_EXIT],
screen_change_before_first_instruction=False,
screen_change_after_last_instruction=False)

# Send the sign device instruction.
# As it requires on-screen validation, the function is asynchronous.
# It will yield the result when the navigation is done
with client.sign_tx(path=path, transaction=transaction):
# Validate the on-screen request by performing the navigation appropriate for this device
if firmware.device.startswith("nano"):
navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
[NavInsID.BOTH_CLICK],
"Approve",
ROOT_SCREENSHOT_PATH,
test_name)
else:
navigator.navigate_until_text_and_compare(NavInsID.USE_CASE_REVIEW_TAP,
[NavInsID.USE_CASE_REVIEW_CONFIRM,
NavInsID.WAIT_FOR_HOME_SCREEN],
"Hold to sign",
ROOT_SCREENSHOT_PATH,
test_name)
scenario_navigator.review_approve()

# The device as yielded the result, parse it and ensure that the signature is correct
response = client.get_async_response().data
_, der_sig, _ = unpack_sign_tx_response(response)

assert check_signature_validity(public_key, der_sig, transaction)

# In this test se send to the device a transaction to sign and validate it on screen
# In this test a transaction is sent to the device to be signed and validated on screen.
# The transaction is short and will be sent in one chunk
# We will ensure that the displayed information is correct by using screenshots comparison
# The transaction memo should not be displayed as we have not enabled it in the app settings.
def test_sign_tx_short_tx_no_memo(firmware, backend, navigator, test_name):
def test_sign_tx_short_tx_no_memo(backend, scenario_navigator, firmware):
if firmware.device.startswith("nano"):
pytest.skip("Skipping this test for Nano devices")

Expand All @@ -95,12 +81,8 @@ def test_sign_tx_short_tx_no_memo(firmware, backend, navigator, test_name):
# As it requires on-screen validation, the function is asynchronous.
# It will yield the result when the navigation is done
with client.sign_tx(path=path, transaction=transaction):
navigator.navigate_until_text_and_compare(NavInsID.USE_CASE_REVIEW_TAP,
[NavInsID.USE_CASE_REVIEW_CONFIRM,
NavInsID.WAIT_FOR_HOME_SCREEN],
"Hold to sign",
ROOT_SCREENSHOT_PATH,
test_name)
# Validate the on-screen request by performing the navigation appropriate for this device
scenario_navigator.review_approve()

# The device as yielded the result, parse it and ensure that the signature is correct
response = client.get_async_response().data
Expand All @@ -109,10 +91,11 @@ def test_sign_tx_short_tx_no_memo(firmware, backend, navigator, test_name):
assert check_signature_validity(public_key, der_sig, transaction)


# In this test se send to the device a transaction to sign and validate it on screen
# In this test a transaction is sent to the device to be signed and validated on screen.
# This test is mostly the same as the previous one but with different values.
# In particular the long memo will force the transaction to be sent in multiple chunks
def test_sign_tx_long_tx(firmware, backend, navigator, test_name):
# def test_sign_tx_long_tx(firmware, backend, navigator, test_name):
def test_sign_tx_long_tx(backend, scenario_navigator, firmware, navigator):
# Use the app interface instead of raw interface
client = BoilerplateCommandSender(backend)
path: str = "m/44'/1'/0'/0/0"
Expand All @@ -139,28 +122,21 @@ def test_sign_tx_long_tx(firmware, backend, navigator, test_name):
screen_change_before_first_instruction=False,
screen_change_after_last_instruction=False)

# Send the sign device instruction.
# As it requires on-screen validation, the function is asynchronous.
# It will yield the result when the navigation is done
with client.sign_tx(path=path, transaction=transaction):
if firmware.device.startswith("nano"):
navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
[NavInsID.BOTH_CLICK],
"Approve",
ROOT_SCREENSHOT_PATH,
test_name)
else:
navigator.navigate_until_text_and_compare(NavInsID.USE_CASE_REVIEW_TAP,
[NavInsID.USE_CASE_REVIEW_CONFIRM,
NavInsID.WAIT_FOR_HOME_SCREEN],
"Hold to sign",
ROOT_SCREENSHOT_PATH,
test_name)
# Validate the on-screen request by performing the navigation appropriate for this device
scenario_navigator.review_approve()

response = client.get_async_response().data
_, der_sig, _ = unpack_sign_tx_response(response)
assert check_signature_validity(public_key, der_sig, transaction)


# Transaction signature refused test
# The test will ask for a transaction signature that will be refused on screen
def test_sign_tx_refused(firmware, backend, navigator, test_name):
def test_sign_tx_refused(backend, scenario_navigator):
# Use the app interface instead of raw interface
client = BoilerplateCommandSender(backend)
path: str = "m/44'/1'/0'/0/0"
Expand All @@ -176,30 +152,10 @@ def test_sign_tx_refused(firmware, backend, navigator, test_name):
memo="This transaction will be refused by the user"
).serialize()

if firmware.device.startswith("nano"):
with pytest.raises(ExceptionRAPDU) as e:
with client.sign_tx(path=path, transaction=transaction):
navigator.navigate_until_text_and_compare(NavInsID.RIGHT_CLICK,
[NavInsID.BOTH_CLICK],
"Reject",
ROOT_SCREENSHOT_PATH,
test_name)

# Assert that we have received a refusal
assert e.value.status == Errors.SW_DENY
assert len(e.value.data) == 0
else:
instructions = [NavInsID.USE_CASE_REVIEW_TAP,
NavInsID.USE_CASE_REVIEW_TAP,
NavInsID.USE_CASE_REVIEW_REJECT,
NavInsID.USE_CASE_CHOICE_CONFIRM,
NavInsID.WAIT_FOR_HOME_SCREEN
]
with pytest.raises(ExceptionRAPDU) as e:
with client.sign_tx(path=path, transaction=transaction):
navigator.navigate_and_compare(ROOT_SCREENSHOT_PATH,
test_name,
instructions)
# Assert that we have received a refusal
assert e.value.status == Errors.SW_DENY
assert len(e.value.data) == 0
with pytest.raises(ExceptionRAPDU) as e:
with client.sign_tx(path=path, transaction=transaction):
scenario_navigator.review_reject()

# Assert that we have received a refusal
assert e.value.status == Errors.SW_DENY
assert len(e.value.data) == 0

0 comments on commit 02b1c32

Please sign in to comment.