ethereum: warn if fee is higher than 10%

Only for standard transactions, where the fee and amount are of the
same unit. In ERC20-txs, the fees are in ETH while the token is of a
different unit.

Author: benma

CHANGELOG.md

@@ -7,7 +7,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
 ## Firmware
 
 ### [Unreleased]
-- Bitcoin: warn if the transaction fee is higher than 10% of the coins sent
+- Bitcoin, Ethereum: warn if the transaction fee is higher than 10% of the coins sent
 - ETH Testnets: add Goerli and remove deprecated Rinkeby and Ropsten
 
 ### 9.13.1

src/rust/Cargo.lock

@@ -35,6 +35,7 @@ sha2 = { version = "0.9.2", default-features = false }
 sha3 = { version = "0.9.1", default-features = false, optional = true }
 zeroize = "1.5.5"
 num-bigint = { version = "0.4.3", default-features = false, optional = true }
+num-traits = { version = "0.2", default-features = false, optional = true }
 bip32-ed25519 = { git = "https://github.com/digitalbitbox/rust-bip32-ed25519", tag = "v0.1.0", optional = true }
 bs58 = { version = "0.4.0", default-features = false, features = ["alloc", "check"], optional = true }
 bech32 = { version = "0.8.1", default-features = false, optional = true }
@@ -70,6 +71,7 @@ app-ethereum = [
   # enable these dependencies
   "sha3",
   "num-bigint",
+  "num-traits",
   # enable this feature in the deps
   "bitbox02/app-ethereum",
 ]

src/rust/bitbox02-rust/Cargo.toml

@@ -14,6 +14,7 @@
 
 use alloc::string::String;
 use num_bigint::BigUint;
+use num_traits::{ToPrimitive, Zero};
 
 pub struct Amount<'a> {
     pub unit: &'a str,
@@ -45,6 +46,15 @@ impl<'a> Amount<'a> {
     }
 }
 
+/// 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> {
+    if amount.is_zero() {
+        return None;
+    }
+    Some(100. * fee.to_f64()? / amount.to_f64()?)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -136,4 +146,26 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    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!(
+            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)
+        );
+    }
 }

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

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use super::amount::Amount;
+use super::amount::{calculate_percentage, Amount};
 use super::params::Params;
 use super::pb;
 use super::Error;
@@ -174,9 +174,10 @@ async fn verify_standard_transaction(
     let total = Amount {
         unit: params.unit,
         decimals: WEI_DECIMALS,
-        value: amount.value.add(&fee.value),
+        value: (&amount.value).add(&fee.value),
     };
-    transaction::verify_total_fee(&total.format(), &fee.format(), None).await?;
+    let percentage = calculate_percentage(&fee.value, &amount.value);
+    transaction::verify_total_fee(&total.format(), &fee.format(), percentage).await?;
     Ok(())
 }
 
@@ -384,7 +385,58 @@ mod tests {
         );
     }
 
-    /// Standard ETH transaction on an unusual keypath (Goerly on mainnet keypath)
+    /// Test a transaction with an unusually high fee.
+    #[test]
+    fn test_high_fee_warning() {
+        const KEYPATH: &[u32] = &[44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0];
+
+        static mut UI_COUNTER: u32 = 0;
+        mock(Data {
+            ui_transaction_address_create: Some(Box::new(|_amount, _address| true)),
+            ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
+                assert_eq!(total, "0.59427728 ETH");
+                assert_eq!(fee, "0.06371328 ETH");
+                assert!(!longtouch);
+                true
+            })),
+            ui_confirm_create: Some(Box::new(move |params| {
+                match unsafe {
+                    UI_COUNTER += 1;
+                    UI_COUNTER
+                } {
+                    1 => {
+                        assert_eq!(params.title, "High fee");
+                        assert_eq!(params.body, "The fee rate\nis 12.0%.\nProceed?");
+                        assert!(params.longtouch);
+                        true
+                    }
+                    _ => panic!("too many user confirmations"),
+                }
+            })),
+            ..Default::default()
+        });
+        mock_unlocked();
+        assert!(block_on(process(&pb::EthSignRequest {
+            coin: pb::EthCoin::Eth as _,
+            keypath: KEYPATH.to_vec(),
+            nonce: b"\x1f\xdc".to_vec(),
+            // fee=gas_price*gas_limit=63713280000000000
+            gas_price: b"\x01\x65\xa0\xbc\x00\x00".to_vec(),
+            gas_limit: b"\xa2\x08".to_vec(),
+            recipient:
+                b"\x04\xf2\x64\xcf\x34\x44\x03\x13\xb4\xa0\x19\x2a\x35\x28\x14\xfb\xe9\x27\xb8\x85"
+                    .to_vec(),
+            // 530564000000000000
+            value: b"\x07\x5c\xf1\x25\x9e\x9c\x40\x00".to_vec(),
+            data: b"".to_vec(),
+            host_nonce_commitment: None,
+            chain_id: 0,
+        }))
+        .is_ok());
+        assert_eq!(unsafe { UI_COUNTER }, 1);
+    }
+
+    /// Standard ETH transaction on an unusual keypath (Goerli on mainnet keypath)
     #[test]
     pub fn test_process_warn_unusual_keypath() {
         const KEYPATH: &[u32] = &[44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0];