Skip to content

Commit eef2092

Browse files
benmabenma
committed
Merge branch 'high-fee-warning'
2 parents 87b3d09 + ee6692a commit eef2092

File tree

11 files changed

+151
-30
lines changed

11 files changed

+151
-30
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
77
## Firmware
88

99
### [Unreleased]
10+
- Bitcoin: warn if the transaction fee is higher than 10% of the coins sent
1011

1112
### 9.13.0
1213
- Bitcoin: allow displaying BTC values in the 'sat' unit

‎py/send_message.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,33 @@ def _sign_btc_normal(
428428
for input_index, sig in sigs:
429429
print("Signature for input {}: {}".format(input_index, sig.hex()))
430430

431+
def _sign_btc_high_fee(self) -> None:
432+
# pylint: disable=no-member
433+
bip44_account: int = 0 + HARDENED
434+
inputs, outputs = _btc_demo_inputs_outputs(bip44_account)
435+
outputs[1].value = int(1e8 * 0.18)
436+
sigs = self._device.btc_sign(
437+
bitbox02.btc.BTC,
438+
[
439+
bitbox02.btc.BTCScriptConfigWithKeypath(
440+
script_config=bitbox02.btc.BTCScriptConfig(
441+
simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH
442+
),
443+
keypath=[84 + HARDENED, 0 + HARDENED, bip44_account],
444+
),
445+
bitbox02.btc.BTCScriptConfigWithKeypath(
446+
script_config=bitbox02.btc.BTCScriptConfig(
447+
simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH_P2SH
448+
),
449+
keypath=[49 + HARDENED, 0 + HARDENED, bip44_account],
450+
),
451+
],
452+
inputs=inputs,
453+
outputs=outputs,
454+
)
455+
for input_index, sig in sigs:
456+
print("Signature for input {}: {}".format(input_index, sig.hex()))
457+
431458
def _sign_btc_multiple_changes(self) -> None:
432459
# pylint: disable=no-member
433460
bip44_account: int = 0 + HARDENED
@@ -647,6 +674,7 @@ def _sign_btc_tx(self) -> None:
647674
format_unit=bitbox02.btc.BTCSignInitRequest.FormatUnit.SAT
648675
),
649676
),
677+
("High fee warning", self._sign_btc_high_fee),
650678
("Multiple change outputs", self._sign_btc_multiple_changes),
651679
("Locktime/RBF", self._sign_btc_locktime_rbf),
652680
("Taproot inputs", self._sign_btc_taproot_inputs),

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

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -741,9 +741,11 @@ async fn _process(request: &pb::BtcSignInitRequest) -> Result<Response, Error> {
741741
let fee: u64 = total_out
742742
.checked_sub(outputs_sum_out)
743743
.ok_or(Error::InvalidInput)?;
744+
let fee_percentage: f64 = 100. * (fee as f64) / (outputs_sum_out as f64);
744745
transaction::verify_total_fee(
745746
&format_amount(coin_params, format_unit, total_out)?,
746747
&format_amount(coin_params, format_unit, fee)?,
748+
Some(fee_percentage),
747749
)
748750
.await?;
749751
status::status("Transaction\nconfirmed", true).await;
@@ -1225,7 +1227,7 @@ mod tests {
12251227
mock(Data {
12261228
ui_confirm_create: Some(Box::new(move |_params| true)),
12271229
ui_transaction_address_create: Some(Box::new(|_amount, _address| true)),
1228-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
1230+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
12291231
..Default::default()
12301232
});
12311233
}
@@ -1546,7 +1548,7 @@ mod tests {
15461548
_ => panic!("unexpected UI dialog"),
15471549
}
15481550
})),
1549-
ui_transaction_fee_create: Some(Box::new(move |total, fee| {
1551+
ui_transaction_fee_create: Some(Box::new(move |total, fee, longtouch| {
15501552
match unsafe {
15511553
UI_COUNTER += 1;
15521554
UI_COUNTER
@@ -1569,6 +1571,7 @@ mod tests {
15691571
}
15701572
_ => panic!("unexpected coin"),
15711573
}
1574+
assert!(longtouch);
15721575
true
15731576
}
15741577
_ => panic!("unexpected UI dialog"),
@@ -1895,7 +1898,7 @@ mod tests {
18951898
UI_COUNTER += 1;
18961899
UI_COUNTER != CURRENT_COUNTER
18971900
})),
1898-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| unsafe {
1901+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| unsafe {
18991902
UI_COUNTER += 1;
19001903
UI_COUNTER != CURRENT_COUNTER
19011904
})),
@@ -1988,7 +1991,7 @@ mod tests {
19881991
unsafe { LOCKTIME_CONFIRMED = false }
19891992
mock_default_ui();
19901993
mock(Data {
1991-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
1994+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
19921995
ui_transaction_address_create: Some(Box::new(|_amount, _address| true)),
19931996
ui_confirm_create: Some(Box::new(move |params| {
19941997
if params.body.contains("Locktime") {
@@ -2018,6 +2021,53 @@ mod tests {
20182021
}
20192022
}
20202023

2024+
// Test a transaction with an unusually high fee.
2025+
#[test]
2026+
fn test_high_fee_warning() {
2027+
let transaction =
2028+
alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new(pb::BtcCoin::Btc)));
2029+
transaction.borrow_mut().outputs[1].value = 1034567890;
2030+
// One more confirmation for the high fee warning.
2031+
transaction.borrow_mut().total_confirmations += 1;
2032+
mock_host_responder(transaction.clone());
2033+
static mut UI_COUNTER: u32 = 0;
2034+
static mut FEE_CHECKED: bool = false;
2035+
mock(Data {
2036+
ui_transaction_address_create: Some(Box::new(|_amount, _address| unsafe {
2037+
UI_COUNTER += 1;
2038+
true
2039+
})),
2040+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| unsafe {
2041+
UI_COUNTER += 1;
2042+
assert_eq!(total, "13.399999 BTC");
2043+
assert_eq!(fee, "2.0541901 BTC");
2044+
assert!(!longtouch);
2045+
true
2046+
})),
2047+
ui_confirm_create: Some(Box::new(move |params| unsafe {
2048+
match {
2049+
UI_COUNTER += 1;
2050+
UI_COUNTER
2051+
} {
2052+
7 => {
2053+
assert_eq!(params.title, "High fee");
2054+
assert_eq!(params.body, "The fee rate\nis 18.1%.\nProceed?");
2055+
assert!(params.longtouch);
2056+
FEE_CHECKED = true;
2057+
true
2058+
}
2059+
_ => true,
2060+
}
2061+
})),
2062+
..Default::default()
2063+
});
2064+
mock_unlocked();
2065+
let tx = transaction.borrow();
2066+
assert!(block_on(process(&tx.init_request())).is_ok());
2067+
assert_eq!(unsafe { UI_COUNTER }, tx.total_confirmations);
2068+
assert!(unsafe { FEE_CHECKED });
2069+
}
2070+
20212071
// Test a P2TR output. It is not part of the default test transaction because Taproot is not
20222072
// active on Litecoin yet.
20232073
#[test]
@@ -2040,7 +2090,7 @@ mod tests {
20402090
}
20412091
true
20422092
})),
2043-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
2093+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
20442094
ui_confirm_create: Some(Box::new(move |_params| true)),
20452095
..Default::default()
20462096
});
@@ -2262,7 +2312,7 @@ mod tests {
22622312
}
22632313
})),
22642314
ui_transaction_address_create: Some(Box::new(|_amount, _address| true)),
2265-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
2315+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
22662316
..Default::default()
22672317
});
22682318
mock_unlocked();
@@ -2318,14 +2368,15 @@ mod tests {
23182368
}
23192369
true
23202370
})),
2321-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
2371+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
23222372
match unsafe {
23232373
UI_COUNTER += 1;
23242374
UI_COUNTER
23252375
} {
23262376
5 => {
23272377
assert_eq!(total, "0.00090175 TBTC");
23282378
assert_eq!(fee, "0.00000175 TBTC");
2379+
assert!(longtouch);
23292380
}
23302381
_ => panic!("unexpected UI dialog"),
23312382
}

‎src/rust/bitbox02-rust/src/hww/api/cardano/sign_transaction.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ async fn _process(request: &pb::CardanoSignTransactionRequest) -> Result<Respons
260260
transaction::verify_total_fee(
261261
&format_value(params, total + request.fee),
262262
&format_value(params, request.fee),
263+
None,
263264
)
264265
.await?;
265266
}
@@ -451,9 +452,10 @@ mod tests {
451452
_ => panic!("unexpected user confirmations"),
452453
}
453454
})),
454-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
455+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
455456
assert_eq!(total, "6.170499 ADA");
456457
assert_eq!(fee, "0.170499 ADA");
458+
assert!(longtouch);
457459
unsafe {
458460
TOTAL_CONFIRMED = true;
459461
}
@@ -826,7 +828,7 @@ mod tests {
826828
mock(Data {
827829
ui_confirm_create: Some(Box::new(|_params| true)),
828830
ui_transaction_address_create: Some(Box::new(|_amount, _address| true)),
829-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
831+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
830832
..Default::default()
831833
});
832834
mock_unlocked();
@@ -901,7 +903,7 @@ mod tests {
901903
})),
902904

903905
ui_transaction_address_create: Some(Box::new(|_amount, _address| true)),
904-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
906+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
905907
..Default::default()
906908
});
907909
mock_unlocked();
@@ -971,7 +973,7 @@ mod tests {
971973
}
972974
})),
973975
ui_transaction_address_create: Some(Box::new(|_amount, _address| true)),
974-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
976+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
975977
..Default::default()
976978
});
977979
mock_unlocked();
@@ -1032,7 +1034,7 @@ mod tests {
10321034
}
10331035
})),
10341036
ui_transaction_address_create: Some(Box::new(|_amount, _address| true)),
1035-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
1037+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
10361038
..Default::default()
10371039
});
10381040
mock_unlocked();
@@ -1158,7 +1160,7 @@ mod tests {
11581160
_ => panic!("unexpected user confirmation"),
11591161
}
11601162
})),
1161-
ui_transaction_fee_create: Some(Box::new(|_total, _fee| true)),
1163+
ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)),
11621164
..Default::default()
11631165
});
11641166
mock_unlocked();

‎src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ async fn verify_erc20_transaction(
113113
None => ("Unknown token".into(), "Unknown amount".into()),
114114
};
115115
transaction::verify_recipient(&recipient_address, &formatted_value).await?;
116-
transaction::verify_total_fee(&formatted_total, &formatted_fee).await?;
116+
transaction::verify_total_fee(&formatted_total, &formatted_fee, None).await?;
117117
Ok(())
118118
}
119119

@@ -176,7 +176,7 @@ async fn verify_standard_transaction(
176176
decimals: WEI_DECIMALS,
177177
value: amount.value.add(&fee.value),
178178
};
179-
transaction::verify_total_fee(&total.format(), &fee.format()).await?;
179+
transaction::verify_total_fee(&total.format(), &fee.format(), None).await?;
180180
Ok(())
181181
}
182182

@@ -355,9 +355,10 @@ mod tests {
355355
assert_eq!(address, "0x04F264Cf34440313B4A0192A352814FBe927b885");
356356
true
357357
})),
358-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
358+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
359359
assert_eq!(total, "0.53069 ETH");
360360
assert_eq!(fee, "0.000126 ETH");
361+
assert!(longtouch);
361362
true
362363
})),
363364
..Default::default()
@@ -408,9 +409,10 @@ mod tests {
408409
assert_eq!(address, "0x04F264Cf34440313B4A0192A352814FBe927b885");
409410
true
410411
})),
411-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
412+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
412413
assert_eq!(total, "0.53069 TETH");
413414
assert_eq!(fee, "0.000126 TETH");
415+
assert!(longtouch);
414416
true
415417
})),
416418
..Default::default()
@@ -461,9 +463,10 @@ mod tests {
461463
assert_eq!(address, "0x04F264Cf34440313B4A0192A352814FBe927b885");
462464
true
463465
})),
464-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
466+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
465467
assert_eq!(total, "0.53069 ETH");
466468
assert_eq!(fee, "0.000126 ETH");
469+
assert!(longtouch);
467470
true
468471
})),
469472
..Default::default()
@@ -501,9 +504,10 @@ mod tests {
501504
assert_eq!(address, "0xE6CE0a092A99700CD4ccCcBb1fEDc39Cf53E6330");
502505
true
503506
})),
504-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
507+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
505508
assert_eq!(total, "57 USDT");
506509
assert_eq!(fee, "0.0012658164 ETH");
510+
assert!(longtouch);
507511
true
508512
})),
509513
..Default::default()
@@ -540,9 +544,10 @@ mod tests {
540544
assert_eq!(address, "0x857B3D969eAcB775a9f79cabc62Ec4bB1D1cd60e");
541545
true
542546
})),
543-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
547+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
544548
assert_eq!(total, "Unknown amount");
545549
assert_eq!(fee, "0.000067973 ETH");
550+
assert!(longtouch);
546551
true
547552
})),
548553
..Default::default()
@@ -665,9 +670,10 @@ mod tests {
665670
assert_eq!(address, "0x04F264Cf34440313B4A0192A352814FBe927b885");
666671
true
667672
})),
668-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
673+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
669674
assert_eq!(total, "0.53069 ETH");
670675
assert_eq!(fee, "0.000126 ETH");
676+
assert!(longtouch);
671677
false
672678
})),
673679
..Default::default()
@@ -678,7 +684,7 @@ mod tests {
678684
// Keystore locked.
679685
mock(Data {
680686
ui_transaction_address_create: Some(Box::new(|_, _| true)),
681-
ui_transaction_fee_create: Some(Box::new(|_, _| true)),
687+
ui_transaction_fee_create: Some(Box::new(|_, _, _| true)),
682688
..Default::default()
683689
});
684690
assert_eq!(block_on(process(&valid_request)), Err(Error::Generic));
@@ -727,14 +733,15 @@ mod tests {
727733
_ => panic!("unexpected user confirmation"),
728734
}
729735
})),
730-
ui_transaction_fee_create: Some(Box::new(|total, fee| {
736+
ui_transaction_fee_create: Some(Box::new(|total, fee, longtouch| {
731737
match unsafe {
732738
CONFIRM_COUNTER += 1;
733739
CONFIRM_COUNTER
734740
} {
735741
4 => {
736742
assert_eq!(total, "0.53069 ");
737743
assert_eq!(fee, "0.000126 ");
744+
assert!(longtouch);
738745
true
739746
}
740747
_ => panic!("unexpected user confirmation"),

0 commit comments

Comments
 (0)