workin' on UI improvements

fw/src/main.rs

@@ -83,6 +83,8 @@ extern "C" fn sample_main() {
     // must be cleared with user interaction
     #[cfg(feature = "pre-release")]
     {
+        use ButtonEvent::*;
+
         clear_screen();
         "Pending Review".place(Location::Middle, Layout::Centered, false);
         screen_update();
@@ -91,7 +93,9 @@ extern "C" fn sample_main() {
             let evt = comm.next_event::<u8>();
 
             match evt {
-                io::Event::Button(_btn) => break,
+                io::Event::Button(LeftButtonRelease | RightButtonRelease | BothButtonsRelease) => {
+                    break
+                }
                 io::Event::Command(_cmd) => {
                     comm.reply(SyscallError::Security);
                 }
@@ -198,9 +202,9 @@ fn handle_btn<RNG: RngCore + CryptoRng>(
                 }
             })
         }
-        UiState::Complete(ref mut a) => {
+        UiState::Message(ref mut a) => {
             a.update(btn).map_exit(|_| {
-                // Reset engine on exit
+                // Reset engine on message clear
                 engine.reset()
             })
         }
@@ -211,7 +215,7 @@ fn handle_btn<RNG: RngCore + CryptoRng>(
         UiState::KeyRequest(..)
         | UiState::TxRequest(..)
         | UiState::Progress(..)
-        | UiState::Complete(..)
+        | UiState::Message(..)
             if r.is_exit() =>
         {
             ui.state = UiState::Menu;
@@ -348,9 +352,15 @@ fn handle_apdu<RNG: RngCore + CryptoRng>(
             render = true;
         }
 
-        // Update to complete when transaction is complete
-        State::Complete if !ui.state.is_complete() => {
-            ui.state = UiState::Complete(Complete::new());
+        // Set complete message when transaction is complete
+        State::Complete if !ui.state.is_message() => {
+            ui.state = UiState::Message(Message::new("Transaction Complete"));
+            render = true;
+        }
+
+        // Set cancelled message when transaction is aborted
+        State::Deny if !ui.state.is_message() => {
+            ui.state = UiState::Message(Message::new("Transaction Cancelled"));
             render = true;
         }
 

fw/src/ui/approver.rs

@@ -72,14 +72,14 @@ impl Approver {
                 RIGHT_ARROW.shift_v(12).display();
                 CROSS_ICON.shift_v(4).shift_h((128 - 16) / 2).display();
 
-                "DENY".place(Location::Custom(48), Layout::Centered, false);
+                "REJECT".place(Location::Custom(48), Layout::Centered, false);
             }
             Allow => {
                 LEFT_ARROW.shift_v(12).display();
                 CHECKMARK_ICON.shift_v(4).shift_h((128 - 16) / 2).display();
 
                 //bitmaps::CHECKMARK.draw((128 - 32) / 2, 24);
-                "ALLOW".place(Location::Custom(48), Layout::Centered, false);
+                "APPROVE".place(Location::Custom(48), Layout::Centered, false);
             }
         }
 

fw/src/ui/helpers.rs

@@ -5,11 +5,18 @@ use nanos_ui::{
     bagls::*,
     bitmaps,
     layout::{Draw, Layout, Location, StringPlace},
-    screen_util,
+    screen_util, ui,
 };
 
 const TX_REQ_APPROVE: &str = "Approve Transaction?";
-const TX_REQ_DENY: &str = "Deny Transaction?";
+const TX_REQ_DENY: &str = "Reject Transaction?";
+
+/// Clear screen wrapper that works both on hardware and speculos
+/// (required as speculos doesn't support the full screen clear syscall,
+/// and we want to run _exactly_ the same code on both)
+pub fn clear_screen() {
+    ui::clear_screen();
+}
 
 /// Convert to hex. Returns a static buffer of N bytes
 #[inline]

fw/src/ui/menu.rs

@@ -92,7 +92,7 @@ impl UiMenu {
         clear_screen();
 
         // Show arrows on menu pages
-        if state != MenuState::Hello && !self.selected {
+        if !self.selected {
             LEFT_ARROW.display();
             RIGHT_ARROW.display();
         }

fw/src/ui/complete.rs → fw/src/ui/message.rs

@@ -14,11 +14,13 @@ use ledger_mob_core::engine::{Driver, Engine};
 use super::{clear_screen, UiResult};
 
 #[derive(Copy, Clone, Debug, PartialEq)]
-pub struct Complete;
+pub struct Message {
+    value: &'static str,
+}
 
-impl Complete {
-    pub fn new() -> Self {
-        Self
+impl Message {
+    pub fn new(value: &'static str) -> Self {
+        Self { value }
     }
 
     pub fn update(&mut self, btn: &ButtonEvent) -> UiResult<bool> {
@@ -35,7 +37,7 @@ impl Complete {
         clear_screen();
 
         // Render transaction information
-        "Transaction Complete".place(Location::Middle, Layout::Centered, false);
+        self.value.place(Location::Middle, Layout::Centered, false);
 
         // Update screen
         screen_util::screen_update();

fw/src/ui/mod.rs

@@ -7,8 +7,6 @@ use rand_core::{CryptoRng, RngCore};
 
 use ledger_mob_core::engine::{Driver, Engine};
 
-use nanos_ui::{bagls::RectFull, layout::Draw, SCREEN_HEIGHT, SCREEN_WIDTH};
-
 mod helpers;
 pub use helpers::*;
 
@@ -21,8 +19,8 @@ pub use approver::*;
 mod progress;
 pub use progress::*;
 
-mod complete;
-pub use complete::*;
+mod message;
+pub use message::*;
 
 mod tx_blind_approver;
 pub use tx_blind_approver::*;
@@ -67,8 +65,8 @@ pub enum UiState {
     /// Progress indicator
     Progress(Progress),
 
-    /// Transaction complete
-    Complete(Complete),
+    /// Messages (transaction complete, rejected, etc.)
+    Message(Message),
 }
 
 impl UiState {
@@ -89,8 +87,8 @@ impl UiState {
         matches!(self, UiState::Progress(..))
     }
 
-    pub fn is_complete(&self) -> bool {
-        matches!(self, UiState::Complete(..))
+    pub fn is_message(&self) -> bool {
+        matches!(self, UiState::Message(..))
     }
 
     #[cfg(feature = "ident")]
@@ -120,7 +118,7 @@ impl Ui {
             #[cfg(feature = "ident")]
             UiState::IdentRequest(a) => a.render(engine),
             UiState::Progress(a) => a.render(engine),
-            UiState::Complete(a) => a.render(engine),
+            UiState::Message(a) => a.render(engine),
         }
     }
 }
@@ -171,13 +169,3 @@ impl<R> UiResult<R> {
         matches!(self, UiResult::Exit(..))
     }
 }
-
-/// Clear screen wrapper that works both on hardware and speculos
-/// (required as speculos doesn't support the full screen clear syscall,
-/// and we want to run _exactly_ the same code on both)
-pub fn clear_screen() {
-    RectFull::new()
-        .width(SCREEN_WIDTH as u32)
-        .height(SCREEN_HEIGHT as u32)
-        .erase();
-}

fw/src/ui/tx_blind_approver.rs

@@ -33,8 +33,8 @@ enum ApproverState {
     Init,
     Warn,
     Hash,
-    Deny,
     Allow,
+    Deny,
 }
 
 impl TxBlindApprover {
@@ -62,28 +62,28 @@ impl TxBlindApprover {
                 self.state = ApproverState::Hash
             }
 
-            // Hash display, left back to warning, right to deny
+            // Hash display, left back to warning, right to allow
             (ApproverState::Hash, ButtonEvent::LeftButtonRelease) => {
                 self.state = ApproverState::Warn
             }
             (ApproverState::Hash, ButtonEvent::RightButtonRelease) => {
-                self.state = ApproverState::Deny
+                self.state = ApproverState::Allow
             }
 
-            // Deny state, left back to hash, right to allow state, both to cancel
-            (ApproverState::Deny, ButtonEvent::LeftButtonRelease) => {
+            // Allow state, left back to hash, both to approve, right to deny
+            (ApproverState::Allow, ButtonEvent::LeftButtonRelease) => {
                 self.state = ApproverState::Hash
             }
-            (ApproverState::Deny, ButtonEvent::RightButtonRelease) => {
-                self.state = ApproverState::Allow
+            (ApproverState::Allow, ButtonEvent::BothButtonsRelease) => return UiResult::Exit(true),
+            (ApproverState::Allow, ButtonEvent::RightButtonRelease) => {
+                self.state = ApproverState::Deny
             }
 
-            // Allow state, left back to deny, both to approve
-            (ApproverState::Allow, ButtonEvent::LeftButtonRelease) => {
-                self.state = ApproverState::Deny
+            // Deny state, left back to allow, both to cancel
+            (ApproverState::Deny, ButtonEvent::LeftButtonRelease) => {
+                self.state = ApproverState::Allow
             }
-            // Allow state, both buttons exit and approve transaction
-            (ApproverState::Allow, ButtonEvent::BothButtonsRelease) => return UiResult::Exit(true),
+            (ApproverState::Deny, ButtonEvent::BothButtonsRelease) => return UiResult::Exit(false),
 
             // All other states, both buttons exit and cancel transaction
             (_, ButtonEvent::BothButtonsRelease) => return UiResult::Exit(false),
@@ -106,7 +106,7 @@ impl TxBlindApprover {
         if self.state != Init {
             LEFT_ARROW.shift_v(0).display();
         }
-        if self.state != Allow {
+        if self.state != Deny {
             RIGHT_ARROW.shift_v(0).display();
         }
 

fw/src/ui/tx_summary_approver.rs

@@ -40,8 +40,8 @@ pub enum TxSummaryApproverState {
     Init,
     Op(usize),
     Fee,
-    Deny,
     Allow,
+    Deny,
 }
 
 impl TxSummaryApprover {
@@ -71,17 +71,16 @@ impl TxSummaryApprover {
 
             // Fee information
             (Fee, LeftButtonRelease) => self.state = Op(self.op_count - 1),
-            (Fee, RightButtonRelease) => self.state = Deny,
-
-            // Deny page
-            (Deny, LeftButtonRelease) => self.state = Fee,
-            (Deny, BothButtonsRelease) => return UiResult::Exit(false),
-            (Deny, RightButtonRelease) => self.state = Allow,
+            (Fee, RightButtonRelease) => self.state = Allow,
 
             // Approve page
-            (Allow, LeftButtonRelease) => self.state = Deny,
+            (Allow, LeftButtonRelease) => self.state = Fee,
             (Allow, BothButtonsRelease) => return UiResult::Exit(true),
-            (Allow, RightButtonRelease) => (),
+            (Allow, RightButtonRelease) => self.state = Deny,
+
+            // Deny page
+            (Deny, LeftButtonRelease) => self.state = Allow,
+            (Deny, BothButtonsRelease) => return UiResult::Exit(false),
 
             // Both buttons pressed in other states cancels the request
             (_, BothButtonsRelease) => return UiResult::Exit(false),
@@ -117,7 +116,7 @@ impl TxSummaryApprover {
         if self.state != Init {
             LEFT_ARROW.shift_v(0).display();
         }
-        if self.state != Allow {
+        if self.state != Deny {
             RIGHT_ARROW.shift_v(0).display();
         }
 

lib/src/error.rs

@@ -19,7 +19,7 @@ pub enum Error<E: Display + Debug> {
     Transport(E),
 
     /// Invalid transaction state
-    #[error("Invalid transaction state (actual: {0}, expected: {1}")]
+    #[error("Invalid transaction state (actual: {0}, expected: {1})")]
     InvalidState(TxState, TxState),
 
     /// Unexpected APDU response

lib/src/tx/mod.rs

@@ -131,11 +131,14 @@ impl<T: Exchange + Send + Sync> TransactionHandle<T> {
 
             debug!("awaiting tx approval (state: {:?})", r);
 
-            // Handle responses, waiting for `Ready` or `Error` states
+            // Handle responses, waiting for `Ready`, `Denied` or `Error` states
             match r {
+                Ok(v) if v.state == TxState::Pending => (),
                 Ok(v) if v.state == TxState::Ready => return Ok(()),
+                Ok(v) if v.state == TxState::TxDenied => return Err(Error::UserDenied),
                 Ok(v) if v.state == TxState::Error => return Err(Error::Engine(0)),
-                _ => (),
+                Ok(v) => return Err(Error::InvalidState(v.state, TxState::Pending)),
+                Err(_) => (),
             }
 
             // Sleep while we wait

lib/tests/helpers.rs

@@ -158,8 +158,6 @@ pub async fn approve(h: &GenericHandle) {
         Button::Right,
         // Right button to move to hash screen
         Button::Right,
-        // Right button to move to deny screen
-        Button::Right,
         // Right button to move to allow screen
         Button::Right,
         // Both buttons to select allow

lib/tests/tx.rs

@@ -67,8 +67,6 @@ const BUTTONS_BLIND: &[Button] = &[
     Button::Right,
     // Right button to move to hash screen
     Button::Right,
-    // Right button to move to deny screen
-    Button::Right,
     // Right button to move to allow screen
     Button::Right,
     // Both buttons to select allow
@@ -82,8 +80,6 @@ const BUTTONS_SUMMARY: &[Button] = &[
     Button::Right,
     // Right button to show fee
     Button::Right,
-    // Right button to show deny
-    Button::Right,
     // Right button to show allow
     Button::Right,
     // Both buttons to select allow