ui: use a trait for workflows

This allows us to plug in a local trait impl for unit tests, allowin
us to remove the ugly unsafe static muts, RefCells and UI_COUNTERS in
the testing code and makes UI unit tests cleaner.

Another benefit of this is that the API code would not hardcode the UI
workflows, so future BitBox products with different UI can use the
same API code but plug its own UI workflows.

We do this by using a `Workflows` trait param in all functions that
need workflows, and use an implementation suitable for unit tests.

One instance of `RealWorkflows` is created that can be added as an arg
in any API function.

This commit is not a complete migration, we can migrate file by file
or function by function. The end goal is to remove all direct calls to
`workflow::*`.

Author: benma

src/rust/bitbox02-rust/src/hww/api.rs

@@ -46,6 +46,8 @@ use pb::request::Request;
 use pb::response::Response;
 use prost::Message;
 
+use crate::workflow::{RealWorkflows, Workflows};
+
 /// Encodes a protobuf Response message.
 pub fn encode(response: Response) -> Vec<u8> {
     let response = pb::Response {
@@ -65,21 +67,27 @@ pub fn decode(input: &[u8]) -> Result<Request, Error> {
 }
 
 #[cfg(any(feature = "app-bitcoin", feature = "app-litecoin"))]
-async fn process_api_btc(request: &Request) -> Result<Response, Error> {
+async fn process_api_btc<W: Workflows>(
+    workflows: &mut W,
+    request: &Request,
+) -> Result<Response, Error> {
     match request {
-        Request::BtcPub(ref request) => bitcoin::process_pub(request).await,
-        Request::BtcSignInit(ref request) => bitcoin::signtx::process(request).await,
+        Request::BtcPub(ref request) => bitcoin::process_pub(workflows, request).await,
+        Request::BtcSignInit(ref request) => bitcoin::signtx::process(workflows, request).await,
         Request::Btc(pb::BtcRequest {
             request: Some(request),
-        }) => bitcoin::process_api(request)
+        }) => bitcoin::process_api(workflows, request)
             .await
             .map(|r| Response::Btc(pb::BtcResponse { response: Some(r) })),
         _ => Err(Error::Generic),
     }
 }
 
 #[cfg(not(any(feature = "app-bitcoin", feature = "app-litecoin")))]
-async fn process_api_btc(_request: &Request) -> Result<Response, Error> {
+async fn process_api_btc<W: Workflows>(
+    _workflows: &mut W,
+    _request: &Request,
+) -> Result<Response, Error> {
     Err(Error::Disabled)
 }
 
@@ -152,6 +160,7 @@ fn can_call(request: &Request) -> bool {
 
 /// Handle a protobuf api call.
 async fn process_api(request: &Request) -> Result<Response, Error> {
+    let workflows = &mut RealWorkflows;
     match request {
         Request::Reboot(ref request) => system::reboot(request).await,
         Request::DeviceInfo(_) => device_info::process(),
@@ -176,7 +185,7 @@ async fn process_api(request: &Request) -> Result<Response, Error> {
         #[cfg(feature = "app-ethereum")]
         Request::Eth(pb::EthRequest {
             request: Some(ref request),
-        }) => ethereum::process_api(request)
+        }) => ethereum::process_api(workflows, request)
             .await
             .map(|r| Response::Eth(pb::EthResponse { response: Some(r) })),
         #[cfg(not(feature = "app-ethereum"))]
@@ -185,12 +194,12 @@ async fn process_api(request: &Request) -> Result<Response, Error> {
         Request::Fingerprint(pb::RootFingerprintRequest {}) => rootfingerprint::process(),
         request @ Request::BtcPub(_)
         | request @ Request::Btc(_)
-        | request @ Request::BtcSignInit(_) => process_api_btc(request).await,
+        | request @ Request::BtcSignInit(_) => process_api_btc(workflows, request).await,
 
         #[cfg(feature = "app-cardano")]
         Request::Cardano(pb::CardanoRequest {
             request: Some(ref request),
-        }) => cardano::process_api(request)
+        }) => cardano::process_api(workflows, request)
             .await
             .map(|r| Response::Cardano(pb::CardanoResponse { response: Some(r) })),
         #[cfg(not(feature = "app-cardano"))]

src/rust/bitbox02-rust/src/hww/api/bitcoin.rs

@@ -32,7 +32,7 @@ pub mod signtx;
 use super::pb;
 use super::Error;
 
-use crate::workflow::confirm;
+use crate::workflow::{confirm, Workflows};
 
 use util::bip32::HARDENED;
 
@@ -189,7 +189,8 @@ async fn address_simple(
 }
 
 /// Processes a multisig address api call.
-pub async fn address_multisig(
+pub async fn address_multisig<W: Workflows>(
+    workflows: &mut W,
     coin: BtcCoin,
     multisig: &Multisig,
     keypath: &[u32],
@@ -206,7 +207,7 @@ pub async fn address_multisig(
     };
     let title = "Receive to";
     if display {
-        multisig::confirm(title, coin_params, &name, multisig).await?;
+        multisig::confirm(workflows, title, coin_params, &name, multisig).await?;
     }
     let address = common::Payload::from_multisig(
         coin_params,
@@ -216,19 +217,21 @@ pub async fn address_multisig(
     )?
     .address(coin_params)?;
     if display {
-        confirm::confirm(&confirm::Params {
-            title,
-            body: &address,
-            scrollable: true,
-            ..Default::default()
-        })
-        .await?;
+        workflows
+            .confirm(&confirm::Params {
+                title,
+                body: &address,
+                scrollable: true,
+                ..Default::default()
+            })
+            .await?;
     }
     Ok(Response::Pub(pb::PubResponse { r#pub: address }))
 }
 
 /// Processes a policy address api call.
-async fn address_policy(
+async fn address_policy<W: Workflows>(
+    workflows: &mut W,
     coin: BtcCoin,
     policy: &Policy,
     keypath: &[u32],
@@ -247,26 +250,30 @@ async fn address_policy(
 
     if display {
         parsed
-            .confirm(title, coin_params, &name, policies::Mode::Basic)
+            .confirm(workflows, title, coin_params, &name, policies::Mode::Basic)
             .await?;
     }
 
     let address =
         common::Payload::from_policy(coin_params, &parsed, keypath)?.address(coin_params)?;
     if display {
-        confirm::confirm(&confirm::Params {
-            title,
-            body: &address,
-            scrollable: true,
-            ..Default::default()
-        })
-        .await?;
+        workflows
+            .confirm(&confirm::Params {
+                title,
+                body: &address,
+                scrollable: true,
+                ..Default::default()
+            })
+            .await?;
     }
     Ok(Response::Pub(pb::PubResponse { r#pub: address }))
 }
 
 /// Handle a Bitcoin xpub/address protobuf api call.
-pub async fn process_pub(request: &pb::BtcPubRequest) -> Result<Response, Error> {
+pub async fn process_pub<W: Workflows>(
+    workflows: &mut W,
+    request: &pb::BtcPubRequest,
+) -> Result<Response, Error> {
     let coin = BtcCoin::try_from(request.coin)?;
     coin_enabled(coin)?;
     match request.output {
@@ -283,24 +290,27 @@ pub async fn process_pub(request: &pb::BtcPubRequest) -> Result<Response, Error>
         }
         Some(Output::ScriptConfig(BtcScriptConfig {
             config: Some(Config::Multisig(ref multisig)),
-        })) => address_multisig(coin, multisig, &request.keypath, request.display).await,
+        })) => address_multisig(workflows, coin, multisig, &request.keypath, request.display).await,
         Some(Output::ScriptConfig(BtcScriptConfig {
             config: Some(Config::Policy(ref policy)),
-        })) => address_policy(coin, policy, &request.keypath, request.display).await,
+        })) => address_policy(workflows, coin, policy, &request.keypath, request.display).await,
         _ => Err(Error::InvalidInput),
     }
 }
 
 /// Handle a nexted Bitcoin protobuf api call.
-pub async fn process_api(request: &Request) -> Result<pb::btc_response::Response, Error> {
+pub async fn process_api<W: Workflows>(
+    workflows: &mut W,
+    request: &Request,
+) -> Result<pb::btc_response::Response, Error> {
     match request {
         Request::IsScriptConfigRegistered(ref request) => {
             registration::process_is_script_config_registered(request)
         }
         Request::RegisterScriptConfig(ref request) => {
-            registration::process_register_script_config(request).await
+            registration::process_register_script_config(workflows, request).await
         }
-        Request::SignMessage(ref request) => signmsg::process(request).await,
+        Request::SignMessage(ref request) => signmsg::process(workflows, request).await,
         // These are streamed asynchronously using the `next_request()` primitive in
         // bitcoin/signtx.rs and are not handled directly.
         Request::PrevtxInit(_)
@@ -317,6 +327,7 @@ mod tests {
 
     use crate::bb02_async::block_on;
     use crate::bip32::parse_xpub;
+    use crate::workflow::RealWorkflows; // instead of TestingWorkflows until the tests are migrated
     use alloc::boxed::Box;
     use alloc::vec::Vec;
     use bitbox02::testing::{
@@ -471,7 +482,7 @@ mod tests {
             };
 
             assert_eq!(
-                block_on(process_pub(&req)),
+                block_on(process_pub(&mut RealWorkflows, &req)),
                 Ok(Response::Pub(pb::PubResponse {
                     r#pub: test.expected_xpub.into(),
                 })),
@@ -492,7 +503,7 @@ mod tests {
             });
             mock_unlocked_using_mnemonic(test.mnemonic, "");
             assert_eq!(
-                block_on(process_pub(&req)),
+                block_on(process_pub(&mut RealWorkflows,&req)),
                 Ok(Response::Pub(pb::PubResponse {
                     r#pub: test.expected_xpub.into(),
                 })),
@@ -513,7 +524,7 @@ mod tests {
             });
             mock_unlocked();
             assert_eq!(
-                block_on(process_pub(&pb::BtcPubRequest {
+                block_on(process_pub(&mut RealWorkflows, &pb::BtcPubRequest {
                     coin: BtcCoin::Btc as _,
                     keypath: [1 + HARDENED, 2 + HARDENED, 3 + HARDENED, 4].to_vec(),
                     display: false,
@@ -553,7 +564,7 @@ mod tests {
             });
             mock_unlocked();
             assert_eq!(
-                block_on(process_pub(&pb::BtcPubRequest {
+                block_on(process_pub(&mut RealWorkflows, &pb::BtcPubRequest {
                     coin: BtcCoin::Btc as _,
                     keypath: [1 + HARDENED, 2 + HARDENED, 3 + HARDENED, 4].to_vec(),
                     display: true,
@@ -575,11 +586,11 @@ mod tests {
         // -- Wrong coin: MIN-1
         let mut req_invalid = req.clone();
         req_invalid.coin = BtcCoin::Btc as i32 - 1;
-        assert!(block_on(process_pub(&req_invalid)).is_err());
+        assert!(block_on(process_pub(&mut RealWorkflows, &req_invalid)).is_err());
         // -- Wrong coin: MAX + 1
         let mut req_invalid = req.clone();
         req_invalid.coin = BtcCoin::Rbtc as i32 + 1;
-        assert!(block_on(process_pub(&req_invalid)).is_err());
+        assert!(block_on(process_pub(&mut RealWorkflows, &req_invalid)).is_err());
     }
 
     #[test]
@@ -778,7 +789,7 @@ mod tests {
             // Without display.
             mock_unlocked_using_mnemonic(test.mnemonic, "");
             assert_eq!(
-                block_on(process_pub(&req)),
+                block_on(process_pub(&mut RealWorkflows, &req)),
                 Ok(Response::Pub(pb::PubResponse {
                     r#pub: test.expected_address.into(),
                 })),
@@ -799,7 +810,7 @@ mod tests {
             });
             mock_unlocked_using_mnemonic(test.mnemonic, "");
             assert_eq!(
-                block_on(process_pub(&req)),
+                block_on(process_pub(&mut RealWorkflows, &req)),
                 Ok(Response::Pub(pb::PubResponse {
                     r#pub: test.expected_address.into()
                 })),
@@ -817,28 +828,31 @@ mod tests {
                 config: Some(Config::SimpleType(SimpleType::P2wpkhP2sh as _)),
             })),
         };
-        assert!(block_on(process_pub(&req)).is_ok());
+        assert!(block_on(process_pub(&mut RealWorkflows, &req)).is_ok());
         // -- Wrong coin: MIN-1
         let mut req_invalid = req.clone();
         req_invalid.coin = BtcCoin::Btc as i32 - 1;
-        assert!(block_on(process_pub(&req_invalid)).is_err());
+        assert!(block_on(process_pub(&mut RealWorkflows, &req_invalid)).is_err());
         // -- Wrong coin: MAX + 1
         let mut req_invalid = req.clone();
         req_invalid.coin = BtcCoin::Tltc as i32 + 1;
-        assert!(block_on(process_pub(&req_invalid)).is_err());
+        assert!(block_on(process_pub(&mut RealWorkflows, &req_invalid)).is_err());
         // -- Wrong keypath
         let mut req_invalid = req.clone();
         req_invalid.keypath = [49 + HARDENED, 0 + HARDENED, 1 + HARDENED, 1, 10000].to_vec();
-        assert!(block_on(process_pub(&req_invalid)).is_err());
+        assert!(block_on(process_pub(&mut RealWorkflows, &req_invalid)).is_err());
         // -- No taproot in Litecoin
-        assert!(block_on(process_pub(&pb::BtcPubRequest {
-            coin: BtcCoin::Ltc as _,
-            keypath: [86 + HARDENED, 0 + HARDENED, 0 + HARDENED, 0, 0].to_vec(),
-            display: false,
-            output: Some(Output::ScriptConfig(BtcScriptConfig {
-                config: Some(Config::SimpleType(SimpleType::P2tr as _)),
-            })),
-        }))
+        assert!(block_on(process_pub(
+            &mut RealWorkflows,
+            &pb::BtcPubRequest {
+                coin: BtcCoin::Ltc as _,
+                keypath: [86 + HARDENED, 0 + HARDENED, 0 + HARDENED, 0, 0].to_vec(),
+                display: false,
+                output: Some(Output::ScriptConfig(BtcScriptConfig {
+                    config: Some(Config::SimpleType(SimpleType::P2tr as _)),
+                })),
+            }
+        ))
         .is_err());
     }
 
@@ -1053,7 +1067,7 @@ mod tests {
                 })),
             };
             assert_eq!(
-                block_on(process_pub(&req)),
+                block_on(process_pub(&mut RealWorkflows, &req)),
                 Ok(Response::Pub(pb::PubResponse {
                     r#pub: test.expected_address.into(),
                 })),
@@ -1202,7 +1216,7 @@ mod tests {
                 })),
             };
             assert_eq!(
-                block_on(process_pub(&req)),
+                block_on(process_pub(&mut RealWorkflows, &req)),
                 Ok(Response::Pub(pb::PubResponse {
                     r#pub: test.expected_address.into(),
                 })),