Skip to content
Draft

Rm f64 #1957

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 64 additions & 20 deletions .ci/check-unwanted-symbols
Original file line number Diff line number Diff line change
@@ -1,23 +1,67 @@
#!/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."

# Some f64 helpers are already pulled in by the existing C snprintf/newlib path.
# Keep this check to helpers that are absent after the integer fee/progress changes.
check_symbols \
"software f64 comparison/conversion helpers" \
"(__aeabi_dcmp[[:alnum:]_]*|__aeabi_d2(i|l|ul)z|compiler_builtins::float::cmp)" \
"Software f64 comparison/conversion helpers add significant binary bloat." \
"Use integer arithmetic in firmware code instead."

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"
5 changes: 3 additions & 2 deletions src/rust/bitbox-hal/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
2 changes: 1 addition & 1 deletion src/rust/bitbox02-rust/src/hal/testing/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 15 additions & 21 deletions src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,15 +359,15 @@ 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 + prevtx_init.num_outputs;

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(
input_index * prevtx_total_ios + prevtx_input_index,
num_inputs * prevtx_total_ios,
);

let prevtx_input = get_prevtx_input(input_index, prevtx_input_index, next_response).await?;
hasher.update(prevtx_input.prev_out_hash.as_slice());
Expand All @@ -382,12 +382,10 @@ 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(
input_index * prevtx_total_ios + prevtx_init.num_inputs + prevtx_output_index,
num_inputs * prevtx_total_ios,
);

let prevtx_output =
get_prevtx_output(input_index, prevtx_output_index, next_response).await?;
Expand Down Expand Up @@ -759,7 +757,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
Expand Down Expand Up @@ -846,7 +844,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();
Expand Down Expand Up @@ -1146,16 +1144,12 @@ async fn _process(
let fee: u64 = total_out
.checked_sub(outputs_sum_out)
.ok_or(Error::InvalidInput)?;
let fee_percentage: Option<f64> = if outputs_sum_out == 0 {
None
} else {
Some(100. * (fee as f64) / (outputs_sum_out as f64))
};
let fee_percentage = transaction::warning_fee_percentage(fee, outputs_sum_out);
transaction::verify_total_fee_maybe_warn(
hal,
&format_amount(coin_params, format_unit, total_out)?,
&format_amount(coin_params, format_unit, fee)?,
fee_percentage,
fee_percentage.as_deref(),
)
.await?;
hal.ui().status("Transaction\nconfirmed", true).await;
Expand Down Expand Up @@ -1316,7 +1310,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);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/rust/bitbox02-rust/src/hww/api/bluetooth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,12 @@ async fn _process(
})
.await?;
} else {
let fee_percentage: f64 = 100. * (request.fee as f64) / (total as f64);
let fee_percentage = transaction::warning_fee_percentage(request.fee, total);
transaction::verify_total_fee_maybe_warn(
hal,
&format_value(params, total + request.fee),
&format_value(params, request.fee),
Some(fee_percentage),
fee_percentage.as_deref(),
)
.await?;
}
Expand Down
31 changes: 18 additions & 13 deletions src/rust/bitbox02-rust/src/hww/api/ethereum/amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use alloc::string::String;
use num_bigint::BigUint;
use num_traits::{ToPrimitive, Zero};
use num_traits::Zero;

pub struct Amount<'a> {
pub unit: &'a str,
Expand Down Expand Up @@ -34,13 +34,17 @@ impl Amount<'_> {
}
}

/// Computes the percentage of the fee of the amount, up to one decimal point.
/// Returns None if the amount is 0 or either fee or amount cannot be represented by `f64`.
pub fn calculate_percentage(fee: &BigUint, amount: &BigUint) -> Option<f64> {
/// Computes the warning percentage string shown to the user, up to one decimal point.
/// Returns None if the amount is 0 or the raw fee percentage is below 10%.
pub fn calculate_percentage(fee: &BigUint, amount: &BigUint) -> Option<String> {
if amount.is_zero() {
return None;
}
Some(100. * fee.to_f64()? / amount.to_f64()?)
if fee * 10u8 < *amount {
return None;
}
let tenths = (fee * 1000u16 + amount / 2u8) / amount;
Some(util::decimal::format_no_trim(&tenths, 1))
}

#[cfg(test)]
Expand Down Expand Up @@ -139,21 +143,22 @@ mod tests {
pub fn test_calculate_percentage() {
let p = |f: u64, a: u64| calculate_percentage(&f.into(), &a.into());
assert_eq!(p(1, 0), None);
assert_eq!(p(3, 4), Some(75.));
assert_eq!(p(0, 100), Some(0.));
assert_eq!(p(1, 100), Some(1.));
assert_eq!(p(9, 100), Some(9.));
assert_eq!(p(10, 100), Some(10.));
assert_eq!(p(99, 100), Some(99.));
assert_eq!(p(909, 1000), Some(90.9));
assert_eq!(p(3, 4), Some("75.0".into()));
assert_eq!(p(0, 100), None);
assert_eq!(p(1, 100), None);
assert_eq!(p(9, 100), None);
assert_eq!(p(10, 100), Some("10.0".into()));
assert_eq!(p(99, 100), Some("99.0".into()));
assert_eq!(p(909, 1000), Some("90.9".into()));
assert_eq!(p(995, 10000), None);
assert_eq!(
calculate_percentage(
// 63713280000000000
&BigUint::from_bytes_be(b"\xe2\x5a\xe3\xfd\xe0\x00\x00"),
// 530564000000000000
&BigUint::from_bytes_be(b"\x07\x5c\xf1\x25\x9e\x9c\x40\x00"),
),
Some(12.008594627603833)
Some("12.0".into())
);
}
}
9 changes: 7 additions & 2 deletions src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,13 @@ async fn verify_standard_total_fee(
value: amount_value.add(&fee.value),
};
let percentage = calculate_percentage(&fee.value, amount_value);
transaction::verify_total_fee_maybe_warn(hal, &total.format(), &fee.format(), percentage)
.await?;
transaction::verify_total_fee_maybe_warn(
hal,
&total.format(),
&fee.format(),
percentage.as_deref(),
)
.await?;
Ok(())
}

Expand Down
Loading
Loading