Use integer progress fractions

.ci/check-unwanted-symbols

@@ -1,23 +1,59 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
-set -x
-set -e
+set -euo pipefail
 
-# Disallow some symbols in the final binary that we don't want.
-if arm-none-eabi-nm build/bin/firmware.elf | grep -q "float_to_decimal_common_shortest"; then
-    echo "Rust fmt float formatting like {.1} adds significant binary bloat."
-    echo "Use something simpler like (float*10).round() as u64, then format with util::decimal::format"
-    exit 1
-fi
-if arm-none-eabi-nm build/bin/firmware.elf | grep -q "strftime"; then
-    echo "strftime adds significant binary bloat. Use custom formatting like in `format_dateimte()`."
-    exit 1
-fi
-if arm-none-eabi-nm build/bin/firmware.elf | grep -q "sha26sha512"; then
-    # sha26sha512 is a mangled Rust symbol standing for `sha2::sha512`.
-    # One can use rustfilt to see the demangled symbols:
-    # cargo install rustfilt; arm-none-eabi-nm build/bin/firmware.elf | rustfilt
-    echo "sha2::Sha512 adds significant binary bloat."
-    echo "Only use it if there is no other sha512 impl available that is smaller."
-    exit 1
+elf=build/bin/firmware.elf
+if [[ ! -f "$elf" ]]; then
+    echo "ELF file not found: $elf" >&2
+    exit 2
 fi
+
+# Disallow symbols in the final linked ELF that indicate expensive code paths.
+symbols=$(arm-none-eabi-nm -C "$elf")
+failed=0
+
+check_symbols() {
+    local name=$1
+    local pattern=$2
+    shift 2
+
+    local matches
+    matches=$(grep -E "$pattern" <<<"$symbols" || true)
+    if [[ -z "$matches" ]]; then
+        return
+    fi
+
+    echo "Found unwanted symbols for: $name" >&2
+    echo "$matches" | sed -n '1,20p' >&2
+    for line in "$@"; do
+        echo "$line" >&2
+    done
+    echo >&2
+    failed=1
+}
+
+check_symbols \
+    "Rust float formatting" \
+    "float_to_decimal_common_shortest" \
+    "Rust fmt float formatting like {:.1} adds significant binary bloat." \
+    "Use integer arithmetic and format the resulting integer parts explicitly."
+
+check_symbols \
+    "strftime" \
+    "(^|[[:space:]])strftime($|[[:space:]])" \
+    "strftime adds significant binary bloat." \
+    "Use custom formatting like in format_datetime()."
+
+check_symbols \
+    "sha2::Sha512" \
+    "sha26sha512|sha2::sha512" \
+    "sha2::Sha512 adds significant binary bloat." \
+    "Only use it if there is no other sha512 implementation available that is smaller."
+
+check_symbols \
+    "software f32 arithmetic helpers" \
+    "(__aeabi_f(add|sub|mul|div)|__(add|sub|mul|div|neg)sf3|compiler_builtins::float::(add|sub|mul|div)::.*f32)" \
+    "Software f32 arithmetic helpers add significant binary bloat." \
+    "Use integer arithmetic in firmware code instead."
+
+exit "$failed"

src/rust/bitbox-hal/src/ui.rs

@@ -62,8 +62,9 @@ pub enum CanCancel {
 }
 
 pub trait Progress {
-    /// Set progress. `progress` should be in the range `[0.0, 1.0]`.
-    fn set(&mut self, progress: f32);
+    /// Set progress as a fraction. `denominator` must be non-zero and
+    /// `numerator <= denominator`.
+    fn set_fraction(&mut self, numerator: u32, denominator: u32);
 }
 
 pub trait Empty {}

src/rust/bitbox02-rust/src/hal/testing/ui.rs

@@ -73,7 +73,7 @@ pub struct TestingUi<'a> {
 pub struct NoopProgress;
 
 impl Progress for NoopProgress {
-    fn set(&mut self, _progress: f32) {}
+    fn set_fraction(&mut self, _numerator: u32, _denominator: u32) {}
 }
 
 pub struct NoopEmpty;

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

@@ -360,15 +360,26 @@ async fn handle_prevtx(
     let mut hasher = Sha256::new();
     hasher.update(prevtx_init.version.to_le_bytes());
 
+    let prevtx_total_ios = prevtx_init
+        .num_inputs
+        .checked_add(prevtx_init.num_outputs)
+        .ok_or(Error::InvalidInput)?;
+    let prevtx_progress_denominator = num_inputs
+        .checked_mul(prevtx_total_ios)
+        .ok_or(Error::InvalidInput)?;
+    let prevtx_progress_input_start = input_index
+        .checked_mul(prevtx_total_ios)
+        .ok_or(Error::InvalidInput)?;
+
     hasher.update(serialize(&VarInt(prevtx_init.num_inputs as u64)));
     for prevtx_input_index in 0..prevtx_init.num_inputs {
         // Update progress.
-        progress_component.set({
-            let step = 1f32 / (num_inputs as f32);
-            let subprogress: f32 = (prevtx_input_index as f32)
-                / (prevtx_init.num_inputs + prevtx_init.num_outputs) as f32;
-            (input_index as f32 + subprogress) * step
-        });
+        progress_component.set_fraction(
+            prevtx_progress_input_start
+                .checked_add(prevtx_input_index)
+                .ok_or(Error::InvalidInput)?,
+            prevtx_progress_denominator,
+        );
 
         let prevtx_input = get_prevtx_input(input_index, prevtx_input_index, next_response).await?;
         hasher.update(prevtx_input.prev_out_hash.as_slice());
@@ -383,12 +394,13 @@ async fn handle_prevtx(
     hasher.update(serialize(&VarInt(prevtx_init.num_outputs as u64)));
     for prevtx_output_index in 0..prevtx_init.num_outputs {
         // Update progress.
-        progress_component.set({
-            let step = 1f32 / (num_inputs as f32);
-            let subprogress: f32 = (prevtx_init.num_inputs + prevtx_output_index) as f32
-                / (prevtx_init.num_inputs + prevtx_init.num_outputs) as f32;
-            (input_index as f32 + subprogress) * step
-        });
+        progress_component.set_fraction(
+            prevtx_progress_input_start
+                .checked_add(prevtx_init.num_inputs)
+                .and_then(|progress| progress.checked_add(prevtx_output_index))
+                .ok_or(Error::InvalidInput)?,
+            prevtx_progress_denominator,
+        );
 
         let prevtx_output =
             get_prevtx_output(input_index, prevtx_output_index, next_response).await?;
@@ -760,7 +772,7 @@ async fn _process(
         progress_component
             .as_mut()
             .unwrap()
-            .set((input_index as f32) / (request.num_inputs as f32));
+            .set_fraction(input_index, request.num_inputs);
 
         let tx_input = get_tx_input(input_index, &mut next_response).await?;
         let script_config_account = validated_script_configs
@@ -847,7 +859,7 @@ async fn _process(
     }
 
     // The progress for loading the inputs is 100%.
-    progress_component.as_mut().unwrap().set(1.);
+    progress_component.as_mut().unwrap().set_fraction(1, 1);
 
     let hash_prevouts = hasher_prevouts.finalize();
     let hash_sequence = hasher_sequence.finalize();
@@ -1317,7 +1329,7 @@ async fn _process(
 
         // Update progress.
         if let Some(ref mut c) = progress_component {
-            c.set((input_index + 1) as f32 / (request.num_inputs as f32));
+            c.set_fraction(input_index + 1, request.num_inputs);
         }
     }
 

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

@@ -97,7 +97,7 @@ async fn process_upgrade_helper<M: Memory>(
         memory.ble_firmware_flash_chunk(inactive_slot, chunk_index, &chunk)?;
 
         // Update progress.
-        progress.set((chunk_index + 1) as f32 / (num_chunks as f32));
+        progress.set_fraction(chunk_index + 1, num_chunks);
     }
 
     let firmware_hash: [u8; 32] = firmware_hasher.finalize().into();
@@ -242,8 +242,8 @@ mod tests {
     }
 
     impl Progress for TestProgress {
-        fn set(&mut self, progress: f32) {
-            self.values.push(progress);
+        fn set_fraction(&mut self, numerator: u32, denominator: u32) {
+            self.values.push(numerator as f32 / denominator as f32);
         }
     }
 

src/rust/bitbox02-sys/build.rs

@@ -131,7 +131,7 @@ const ALLOWLIST_FNS: &[&str] = &[
     "platform_product",
     "printf",
     "progress_create",
-    "progress_set",
+    "progress_set_fraction",
     "random_32_bytes_mcu",
     "random_32_bytes",
     "random_fake_reset",

src/rust/bitbox02/src/hal/ui.rs

@@ -19,8 +19,8 @@ pub struct BitBox02Progress {
 }
 
 impl HalProgress for BitBox02Progress {
-    fn set(&mut self, progress: f32) {
-        crate::ui::progress_set(&mut self.component, progress);
+    fn set_fraction(&mut self, numerator: u32, denominator: u32) {
+        crate::ui::progress_set_fraction(&mut self.component, numerator, denominator);
     }
 }
 

src/rust/bitbox02/src/ui/ui.rs

@@ -781,8 +781,8 @@ pub fn progress_create(title: &str) -> Component {
     }
 }
 
-pub fn progress_set(component: &mut Component, progress: f32) {
-    unsafe { bitbox02_sys::progress_set(component.component, progress) }
+pub fn progress_set_fraction(component: &mut Component, numerator: u32, denominator: u32) {
+    unsafe { bitbox02_sys::progress_set_fraction(component.component, numerator, denominator) }
 }
 
 pub fn empty_create() -> Component {

src/rust/bitbox02/src/ui/ui_stub.rs

@@ -93,7 +93,7 @@ pub fn progress_create(_title: &str) -> Component {
     Component { is_pushed: false }
 }
 
-pub fn progress_set(_component: &mut Component, _progress: f32) {}
+pub fn progress_set_fraction(_component: &mut Component, _numerator: u32, _denominator: u32) {}
 
 pub fn empty_create() -> Component {
     Component { is_pushed: false }

src/rust/bitbox02/src/ui/ui_stub_c_unit_tests.rs

@@ -116,7 +116,7 @@ pub fn progress_create(_title: &str) -> Component {
     Component { is_pushed: false }
 }
 
-pub fn progress_set(_component: &mut Component, _progress: f32) {}
+pub fn progress_set_fraction(_component: &mut Component, _numerator: u32, _denominator: u32) {}
 
 pub fn empty_create() -> Component {
     Component { is_pushed: false }

src/rust/bitbox03/src/ui.rs

@@ -38,7 +38,7 @@ impl<Timer> Drop for ScreenGuard<'_, Timer> {
 }
 
 impl hal::ui::Progress for BitBox03UiProgress {
-    fn set(&mut self, _progress: f32) {
+    fn set_fraction(&mut self, _numerator: u32, _denominator: u32) {
         todo!()
     }
 }

src/ui/components/progress.c

@@ -10,15 +10,17 @@
 #include <string.h>
 
 typedef struct {
-    float progress;
+    uint16_t filled_width;
 } data_t;
 
 static void _render(component_t* component)
 {
     const data_t* data = (const data_t*)component->data;
     const uint16_t bar_height = 5;
-    UG_FillFrame(
-        0, SCREEN_HEIGHT - bar_height, SCREEN_WIDTH * data->progress, SCREEN_HEIGHT, C_WHITE);
+    if (data->filled_width > 0) {
+        UG_FillFrame(
+            0, SCREEN_HEIGHT - bar_height, data->filled_width - 1, SCREEN_HEIGHT - 1, C_WHITE);
+    }
 
     ui_util_component_render_subcomponents(component);
 }
@@ -49,8 +51,8 @@ component_t* progress_create(const char* title)
     return component;
 }
 
-void progress_set(component_t* component, float progress)
+void progress_set_fraction(component_t* component, uint32_t numerator, uint32_t denominator)
 {
     data_t* data = (data_t*)component->data;
-    data->progress = progress;
+    data->filled_width = (uint16_t)((uint64_t)SCREEN_WIDTH * numerator / denominator);
 }

src/ui/components/progress.h

@@ -11,9 +11,9 @@
 component_t* progress_create(const char* title);
 
 /**
- * Set the progress.
- * @param[in] progress value must be in [0, 1].
+ * Set the progress as an exact fraction.
+ * @param[in] denominator must be non-zero.
  */
-void progress_set(component_t* component, float progress);
+void progress_set_fraction(component_t* component, uint32_t numerator, uint32_t denominator);
 
 #endif