Skip to content

Commit b56111a

Browse files
LbqdsLbqds
committed
Address comments
1 parent 1823236 commit b56111a

File tree

9 files changed

+109
-85
lines changed

9 files changed

+109
-85
lines changed

‎app/src/handler.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ use crate::{
66
error_code::ErrorCode,
77
public_key::{derive_pub_key, Address},
88
sign_tx_context::SignTxContext,
9-
ui::{
10-
bytes_to_string, check_blind_signing, review_address, sign_hash_ui, tx_reviewer::TxReviewer,
11-
},
9+
ui::{bytes_to_string, review_address, sign_hash_ui, tx_reviewer::TxReviewer},
1210
};
1311

1412
const MAX_TOKEN_SIZE: u8 = 5;
@@ -185,7 +183,7 @@ fn handle_sign_tx(
185183
let tx_data = &data[PATH_LENGTH..];
186184
let is_tx_execute_script = tx_data[SCRIPT_OFFSET - 1] == CALL_CONTRACT_FLAG;
187185
if is_tx_execute_script {
188-
check_blind_signing(&mut tx_reviewer.inner)?;
186+
tx_reviewer.check_blind_signing()?;
189187
}
190188
tx_reviewer.set_tx_execute_script(is_tx_execute_script);
191189

‎app/src/ledger_sdk_stub/nbgl_display.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
// The purpose of this file is to address the issue of the ledger rust sdk not supporting
2-
// navigation to the settings page.
3-
// To maintain consistency with the ledger rust sdk code, we have ignored clipply warnings here.
1+
// This file is a modified version of the Ledger Rust SDK code.
2+
// The modifications were made to address the issue of the Ledger Rust SDK
3+
// not supporting navigation to the settings page.
4+
// To maintain consistency with the original Ledger Rust SDK code,
5+
// we have disabled Clippy warnings for this file.
46
#![allow(clippy::all)]
57

68
use crate::settings::{SETTINGS_DATA, SETTINGS_SIZE};
@@ -108,6 +110,10 @@ where
108110
match comm.next_event() {
109111
Event::Command(t) => return Event::Command(t),
110112
_ => {
113+
// The Ledger Rust SDK does not refresh the UI after updating settings.
114+
// Here we refresh the UI manually upon detecting settings updates.
115+
// We have reported this issue to Ledger dev, and once the Rust SDK is fixed,
116+
// we can remove this workaround.
111117
if SETTINGS_UPDATED {
112118
SETTINGS_UPDATED = false;
113119
page_index = 0; // display the settings page
@@ -138,5 +144,5 @@ unsafe extern "C" fn settings_callback(token: c_int, _index: u8, _page: c_int) {
138144
}
139145

140146
unsafe extern "C" fn app_exit() {
141-
os_sched_exit(0);
147+
os_sched_exit(u8::MAX);
142148
}

‎app/src/main.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ extern "C" fn sample_main() {
5656
let settings_strings = &[["Blind signing", "Enable blind signing"]];
5757

5858
loop {
59-
let event = if tx_reviewer.inner.display_settings {
59+
let event = if tx_reviewer.display_settings() {
6060
nbgl_display::<Ins>(&mut comm, settings_strings, 0)
61-
} else if !tx_reviewer.inner.review_started {
61+
} else if !tx_reviewer.review_started() {
6262
nbgl_display::<Ins>(&mut comm, settings_strings, INIT_HOME_PAGE as u8)
6363
} else {
6464
comm.next_event()

‎app/src/ui/bagl/mod.rs

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,9 @@ use crate::{
55
error_code::ErrorCode,
66
ledger_sdk_stub::multi_field_review::{Field, MultiFieldReview},
77
public_key::sign_hash,
8-
settings::is_blind_signing_enabled,
9-
ui::TxReviewerInner,
108
};
119
use core::str::from_utf8;
12-
use ledger_device_sdk::{
13-
buttons::{ButtonEvent, ButtonsState},
14-
ui::{
15-
bitmaps::{CHECKMARK, CROSS, CROSSMARK, EYE},
16-
gadgets::{clear_screen, get_event, Page, PageStyle},
17-
screen_util::screen_update,
18-
},
19-
};
10+
use ledger_device_sdk::ui::bitmaps::{CHECKMARK, CROSS, EYE};
2011

2112
pub fn sign_hash_ui(path: &[u32], message: &[u8]) -> Result<([u8; 72], u32, u32), ErrorCode> {
2213
let hex: [u8; 64] = utils::to_hex(message).ok_or(ErrorCode::BadLen)?;
@@ -64,24 +55,3 @@ pub fn review_address(address: &str) -> Result<(), ErrorCode> {
6455
Err(ErrorCode::UserCancelled)
6556
}
6657
}
67-
68-
pub fn check_blind_signing(_tx_reviewer_inner: &mut TxReviewerInner) -> Result<(), ErrorCode> {
69-
if is_blind_signing_enabled() {
70-
return Ok(());
71-
}
72-
let page = Page::new(
73-
PageStyle::PictureNormal,
74-
["Blind signing", "must be enabled"],
75-
Some(&CROSSMARK),
76-
);
77-
clear_screen();
78-
page.place();
79-
screen_update();
80-
let mut buttons = ButtonsState::new();
81-
82-
loop {
83-
if let Some(ButtonEvent::BothButtonsRelease) = get_event(&mut buttons) {
84-
return Err(ErrorCode::BlindSigningDisabled);
85-
}
86-
}
87-
}

‎app/src/ui/bagl/tx_reviewer_inner.rs

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
11
use crate::error_code::ErrorCode;
22
use crate::ledger_sdk_stub::multi_field_review::{Field, MultiFieldReview};
3-
use ledger_device_sdk::ui::bitmaps::{CHECKMARK, CROSS, EYE, WARNING};
3+
use crate::settings::is_blind_signing_enabled;
4+
use ledger_device_sdk::{
5+
buttons::{ButtonEvent, ButtonsState},
6+
ui::bitmaps::{CHECKMARK, CROSS, CROSSMARK, EYE, WARNING},
7+
ui::gadgets::{clear_screen, get_event, Page, PageStyle},
8+
ui::screen_util::screen_update,
9+
};
410

11+
// Different Ledger devices use different UI libraries, so we've introduced the
12+
// `TxReviewInner` to facilitate the display of tx details across different devices.
13+
// The `TxReviewInner` here is for Ledger Nanos/Nanosp/Nanox.
514
pub struct TxReviewerInner {
615
is_tx_execute_script: bool,
716
}
@@ -42,11 +51,8 @@ impl TxReviewerInner {
4251
}
4352

4453
// Review transfer that sends to self
45-
pub fn review_self_transfer(&self, fee_field: &Field) -> Result<(), ErrorCode> {
46-
let fields = &[Field {
47-
name: fee_field.name,
48-
value: fee_field.value,
49-
}];
54+
pub fn review_self_transfer(&self, fee_field: Field) -> Result<(), ErrorCode> {
55+
let fields = &[fee_field];
5056
let review = if self.is_tx_execute_script {
5157
MultiFieldReview::new(
5258
fields,
@@ -125,4 +131,25 @@ impl TxReviewerInner {
125131
pub fn output_index_as_field(&self) -> bool {
126132
false
127133
}
134+
135+
pub fn check_blind_signing(&self) -> Result<(), ErrorCode> {
136+
if is_blind_signing_enabled() {
137+
return Ok(());
138+
}
139+
let page = Page::new(
140+
PageStyle::PictureNormal,
141+
["Blind signing", "must be enabled"],
142+
Some(&CROSSMARK),
143+
);
144+
clear_screen();
145+
page.place();
146+
screen_update();
147+
let mut buttons = ButtonsState::new();
148+
149+
loop {
150+
if let Some(ButtonEvent::BothButtonsRelease) = get_event(&mut buttons) {
151+
return Err(ErrorCode::BlindSigningDisabled);
152+
}
153+
}
154+
}
128155
}

‎app/src/ui/mod.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,34 @@ pub mod bagl;
44
pub mod nbgl;
55

66
#[cfg(not(any(target_os = "stax", target_os = "flex")))]
7-
pub use bagl::{
8-
check_blind_signing, review_address, sign_hash_ui, tx_reviewer_inner::TxReviewerInner,
9-
};
7+
pub use bagl::{review_address, sign_hash_ui, tx_reviewer_inner::TxReviewerInner};
108
#[cfg(any(target_os = "stax", target_os = "flex"))]
11-
pub use nbgl::{
12-
check_blind_signing, review_address, sign_hash_ui, tx_reviewer_inner::TxReviewerInner,
13-
};
9+
pub use nbgl::{review_address, sign_hash_ui, tx_reviewer_inner::TxReviewerInner};
1410

1511
use crate::error_code::ErrorCode;
1612
use core::str::from_utf8;
1713
pub mod tx_reviewer;
1814

1915
#[inline]
2016
pub fn bytes_to_string(bytes: &[u8]) -> Result<&str, ErrorCode> {
21-
match from_utf8(bytes) {
22-
Ok(str) => Ok(str),
23-
Err(_) => Err(ErrorCode::InternalError),
17+
#[cfg(not(target_os = "stax"))]
18+
{
19+
match from_utf8(bytes) {
20+
Ok(str) => Ok(str),
21+
Err(_) => Err(ErrorCode::InternalError),
22+
}
23+
}
24+
25+
// We encountered a strange bug on Ledger Stax where Speculos exits immediately after
26+
// loading the app (`syscall: os_sched_exit(0)[*] exit called (0)`), even though we haven't
27+
// run any tests yet. This may be a bug in Speculos, although this bug might not be related
28+
// to this function, the issue was resolved after changing to the following code,
29+
// so we implemented this workaround to address the issue.
30+
#[cfg(target_os = "stax")]
31+
{
32+
match from_utf8(bytes) {
33+
Ok(_) => Ok(from_utf8(bytes).unwrap()),
34+
Err(_) => Err(ErrorCode::InternalError),
35+
}
2436
}
2537
}

‎app/src/ui/nbgl/mod.rs

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
pub mod tx_reviewer_inner;
22

33
use crate::{
4-
error_code::ErrorCode, ledger_sdk_stub::nbgl_review::NbglStreamingReview,
5-
public_key::sign_hash, settings::is_blind_signing_enabled, ui::TxReviewerInner,
4+
error_code::ErrorCode, ledger_sdk_stub::nbgl_review::NbglStreamingReview, public_key::sign_hash,
65
};
76
use core::str::from_utf8;
87
use include_gif::include_gif;
@@ -75,19 +74,3 @@ pub fn review_address(address: &str) -> Result<(), ErrorCode> {
7574
Err(ErrorCode::UserCancelled)
7675
}
7776
}
78-
79-
pub fn check_blind_signing(tx_reviewer_inner: &mut TxReviewerInner) -> Result<(), ErrorCode> {
80-
if is_blind_signing_enabled() {
81-
return Ok(());
82-
}
83-
let go_to_settings = nbgl_review_warning(
84-
"This transaction cannot be clear-signed",
85-
"Enable blind signing in the settings to sign this transaction.",
86-
"Go to settings",
87-
"Reject transaction",
88-
);
89-
if go_to_settings {
90-
tx_reviewer_inner.set_display_settings(true);
91-
}
92-
Err(ErrorCode::BlindSigningDisabled)
93-
}

‎app/src/ui/nbgl/tx_reviewer_inner.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
use crate::{
22
error_code::ErrorCode,
33
ledger_sdk_stub::nbgl_review::NbglStreamingReview,
4+
settings::is_blind_signing_enabled,
45
ui::nbgl::{nbgl_review_warning, new_nbgl_review},
56
};
67
use ledger_device_sdk::nbgl::{Field, NbglReviewStatus, TransactionType};
78

9+
// Different Ledger devices use different UI libraries, so we've introduced the
10+
// `TxReviewInner` to facilitate the display of tx details across different devices.
11+
// The `TxReviewInner` here is for Ledger Stax/Flex.
812
pub struct TxReviewerInner {
913
pub review_started: bool,
1014
pub display_settings: bool,
@@ -28,10 +32,6 @@ impl TxReviewerInner {
2832
self.reviewer.as_ref().unwrap()
2933
}
3034

31-
pub fn set_display_settings(&mut self, display_settings: bool) {
32-
self.display_settings = display_settings;
33-
}
34-
3535
pub fn set_tx_execute_script(&mut self, is_tx_execute_script: bool) {
3636
assert!(self.reviewer.is_none());
3737
self.is_tx_execute_script = is_tx_execute_script;
@@ -71,11 +71,7 @@ impl TxReviewerInner {
7171
}
7272

7373
// Review transfer that sends to self
74-
pub fn review_self_transfer(&mut self, fee_field: &Field) -> Result<(), ErrorCode> {
75-
let fee_field = Field {
76-
name: fee_field.name,
77-
value: fee_field.value,
78-
};
74+
pub fn review_self_transfer(&mut self, fee_field: Field) -> Result<(), ErrorCode> {
7975
if self.is_tx_execute_script {
8076
self.finish_review(&[fee_field])
8177
} else {
@@ -137,4 +133,20 @@ impl TxReviewerInner {
137133
pub fn output_index_as_field(&self) -> bool {
138134
true
139135
}
136+
137+
pub fn check_blind_signing(&mut self) -> Result<(), ErrorCode> {
138+
if is_blind_signing_enabled() {
139+
return Ok(());
140+
}
141+
let go_to_settings = nbgl_review_warning(
142+
"This transaction cannot be clear-signed",
143+
"Enable blind signing in the settings to sign this transaction.",
144+
"Go to settings",
145+
"Reject transaction",
146+
);
147+
if go_to_settings {
148+
self.display_settings = true;
149+
}
150+
Err(ErrorCode::BlindSigningDisabled)
151+
}
140152
}

‎app/src/ui/tx_reviewer.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub struct TxReviewer {
4141
tx_fee: Option<U256>,
4242
token_metadata_length: usize,
4343
token_verifier: Option<TokenVerifier>,
44-
pub inner: TxReviewerInner,
44+
inner: TxReviewerInner,
4545
}
4646

4747
impl TxReviewer {
@@ -557,12 +557,28 @@ impl TxReviewer {
557557
value,
558558
};
559559
if self.next_output_index == FIRST_OUTPUT_INDEX {
560-
return self.inner.review_self_transfer(&fee_field);
560+
return self.inner.review_self_transfer(fee_field);
561561
}
562562

563563
let fields = &[fee_field];
564564
self.inner.finish_review(fields)
565565
}
566+
567+
pub fn check_blind_signing(&mut self) -> Result<(), ErrorCode> {
568+
self.inner.check_blind_signing()
569+
}
570+
571+
#[cfg(any(target_os = "stax", target_os = "flex"))]
572+
#[inline]
573+
pub fn display_settings(&self) -> bool {
574+
self.inner.display_settings
575+
}
576+
577+
#[cfg(any(target_os = "stax", target_os = "flex"))]
578+
#[inline]
579+
pub fn review_started(&self) -> bool {
580+
self.inner.review_started
581+
}
566582
}
567583

568584
// Output indexes for review

0 commit comments

Comments
 (0)