Skip to content

Commit

Permalink
ethereum: warn if fee is higher than 10%
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
benma committed Nov 9, 2022
1 parent eea8ff0 commit e9e8950
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 5 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions src/rust/bitbox02-rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -70,6 +71,7 @@ app-ethereum = [
# enable these dependencies
"sha3",
"num-bigint",
"num-traits",
# enable this feature in the deps
"bitbox02/app-ethereum",
]
Expand Down
32 changes: 32 additions & 0 deletions src/rust/bitbox02-rust/src/hww/api/ethereum/amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -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)
);
}
}
60 changes: 56 additions & 4 deletions src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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];
Expand Down

0 comments on commit e9e8950

Please sign in to comment.