Skip to content

Commit 459a523

Browse files
committed
btc/paymentrequest: split memo into multiple screens
One screen to introduce the memo, and one screen per line in the memo. This will allow a nice UX for Pocket to display: "Memo from POCKET" "IBAN correct?" "CH 93 ...." title_short vs title_long to not change the UX of signmsg while being able to re-use the same workflow. Having Pocket add the wording about "IBAN correct?" (or similar) is a hack/workaround. It would be a better fit to modify SLIP-24 to add a IBAN memo type, so the BitBox can add specific wording about this. We don't do this for now to not deviate from the standard, but we might in the future.
1 parent 2549592 commit 459a523

File tree

5 files changed

+53
-24
lines changed

5 files changed

+53
-24
lines changed

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use super::script::serialize_varint;
2424
use pb::btc_payment_request_request::{memo, Memo};
2525
use pb::btc_sign_init_request::FormatUnit;
2626

27-
use crate::workflow::{confirm, transaction};
27+
use crate::workflow::{confirm, transaction, verify_message};
2828

2929
use sha2::{Digest, Sha256};
3030

@@ -74,20 +74,20 @@ pub async fn user_verify(
7474
Memo {
7575
memo: Some(memo::Memo::TextMemo(text_memo)),
7676
} => {
77-
if !util::ascii::is_printable_ascii(&text_memo.note, util::ascii::Charset::All) {
77+
if !util::ascii::is_printable_ascii(
78+
&text_memo.note,
79+
util::ascii::Charset::AllNewline,
80+
) {
7881
return Err(Error::InvalidInput);
7982
}
8083
confirm::confirm(&confirm::Params {
8184
title: "",
82-
body: &format!(
83-
"Memo from {}: {}",
84-
payment_request.recipient_name, text_memo.note,
85-
),
86-
scrollable: true,
85+
body: &format!("Memo from\n\n{}", payment_request.recipient_name),
8786
accept_is_nextarrow: true,
8887
..Default::default()
8988
})
9089
.await?;
90+
verify_message::verify("Memo", "Memo", text_memo.note.as_bytes(), false).await?;
9191
}
9292
_ => return Err(Error::InvalidInput),
9393
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ pub async fn process(request: &pb::BtcSignMessageRequest) -> Result<Response, Er
7777
};
7878
confirm::confirm(&confirm_params).await?;
7979

80-
verify_message::verify(&request.msg).await?;
80+
verify_message::verify("Sign message", "Sign", &request.msg, true).await?;
8181

8282
// See
8383
// https://github.com/spesmilo/electrum/blob/84dc181b6e7bb20e88ef6b98fb8925c5f645a765/electrum/ecc.py#L355-L358.
@@ -303,6 +303,7 @@ mod tests {
303303
3 => {
304304
assert_eq!(params.title, "Sign message");
305305
assert_eq!(params.body.as_bytes(), MESSAGE);
306+
assert!(params.longtouch);
306307
false
307308
}
308309
_ => panic!("too many user confirmations"),

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3178,14 +3178,14 @@ mod tests {
31783178
{
31793179
let mut tx = transaction.borrow_mut();
31803180
// An additional confirmation for the text memo.
3181-
tx.total_confirmations += 1;
3181+
tx.total_confirmations += 3;
31823182
let payment_request_output_index = 1;
31833183
let output_value = tx.outputs[payment_request_output_index].value;
31843184
let mut payment_request = pb::BtcPaymentRequestRequest {
31853185
recipient_name: "Test Merchant".into(),
31863186
memos: vec![Memo {
31873187
memo: Some(memo::Memo::TextMemo(memo::TextMemo {
3188-
note: "Test memo".into(),
3188+
note: "Test memo line1\nTest memo line2".into(),
31893189
})),
31903190
}],
31913191
nonce: vec![],
@@ -3222,12 +3222,12 @@ mod tests {
32223222
assert_eq!(amount, "12.34567890 BTC");
32233223
true
32243224
}
3225-
4 => {
3225+
6 => {
32263226
assert_eq!(address, "bc1qxvenxvenxvenxvenxvenxvenxvenxven2ymjt8");
32273227
assert_eq!(amount, "0.00006000 BTC");
32283228
true
32293229
}
3230-
5 => {
3230+
7 => {
32313231
assert_eq!(
32323232
address,
32333233
"bc1qg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zqd8sxw4"
@@ -3243,7 +3243,7 @@ mod tests {
32433243
UI_COUNTER += 1;
32443244
UI_COUNTER
32453245
} {
3246-
7 => {
3246+
9 => {
32473247
assert_eq!(total, "13.39999900 BTC");
32483248
assert_eq!(fee, "0.05419010 BTC");
32493249
true
@@ -3257,10 +3257,26 @@ mod tests {
32573257
UI_COUNTER
32583258
} {
32593259
3 => {
3260-
assert_eq!(params.body, "Memo from Test Merchant: Test memo");
3260+
assert_eq!(params.body, "Memo from\n\nTest Merchant");
3261+
assert!(params.accept_is_nextarrow);
3262+
assert!(!params.longtouch);
32613263
true
32623264
}
3263-
6 => {
3265+
4 => {
3266+
assert_eq!(params.title, "Memo 1/2");
3267+
assert_eq!(params.body, "Test memo line1");
3268+
assert!(params.accept_is_nextarrow);
3269+
assert!(!params.longtouch);
3270+
true
3271+
}
3272+
5 => {
3273+
assert_eq!(params.title, "Memo 2/2");
3274+
assert_eq!(params.body, "Test memo line2");
3275+
assert!(params.accept_is_nextarrow);
3276+
assert!(!params.longtouch);
3277+
true
3278+
}
3279+
8 => {
32643280
assert_eq!(params.title, "Warning");
32653281
assert_eq!(params.body, "There are 2\nchange outputs.\nProceed?");
32663282
true

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ pub async fn process(request: &pb::EthSignMessageRequest) -> Result<Response, Er
4848
// abort errors.
4949
super::pubrequest::process(&pub_request).await?;
5050

51-
verify_message::verify(&request.msg).await?;
51+
verify_message::verify("Sign message", "Sign", &request.msg, true).await?;
5252

5353
// Construct message to be signed. There is no standard for this. We match what MyEtherWallet,
5454
// Trezor, etc. do, e.g.:
@@ -121,6 +121,7 @@ mod tests {
121121
2 => {
122122
assert_eq!(params.title, "Sign message");
123123
assert_eq!(params.body.as_bytes(), MESSAGE);
124+
assert!(params.longtouch);
124125
true
125126
}
126127
_ => panic!("too many user confirmations"),

src/rust/bitbox02-rust/src/workflow/verify_message.rs

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,23 @@ impl core::convert::From<confirm::UserAbort> for Error {
2929
}
3030
}
3131

32-
/// Verify a message to be signed.
32+
/// Verify a message.
3333
///
3434
/// If the bytes are all printable ascii chars, the message is
3535
/// confirmed one line at a time (the str is split into lines).
3636
///
3737
/// Otherwise, it is displayed as hex.
38-
pub async fn verify(msg: &[u8]) -> Result<(), Error> {
38+
///
39+
/// title_long is shown if it is only one line/screen. title_short is shown if there are multiple
40+
/// line screens, suffixed with the progress label (e.g. 1/3).
41+
///
42+
/// is_final if this is the final step in a workflow. In this case,
43+
pub async fn verify(
44+
title_long: &str,
45+
title_short: &str,
46+
msg: &[u8],
47+
is_final: bool,
48+
) -> Result<(), Error> {
3949
if ascii::is_printable_ascii(msg, ascii::Charset::AllNewline) {
4050
// The message is all ascii and printable.
4151
let msg = core::str::from_utf8(msg).unwrap();
@@ -47,28 +57,29 @@ pub async fn verify(msg: &[u8]) -> Result<(), Error> {
4757
for (i, &page) in pages.iter().enumerate() {
4858
let is_last = i == pages.len() - 1;
4959
let title = if pages.len() == 1 {
50-
"Sign message".into()
60+
title_long.into()
5161
} else {
52-
format!("Sign {}/{}", i + 1, pages.len())
62+
format!("{} {}/{}", title_short, i + 1, pages.len())
5363
};
5464
let params = confirm::Params {
5565
title: &title,
5666
body: page,
5767
scrollable: true,
58-
accept_is_nextarrow: !is_last,
59-
longtouch: is_last,
68+
accept_is_nextarrow: true, // longtouch takes priority over this if enabled
69+
longtouch: is_last && is_final,
6070
..Default::default()
6171
};
6272
confirm::confirm(&params).await?;
6373
}
6474
Ok(())
6575
} else {
6676
let params = confirm::Params {
67-
title: "Sign message\ndata (hex)",
77+
title: &format!("{}\ndata (hex)", title_long),
6878
body: &hex::encode(msg),
6979
scrollable: true,
7080
display_size: msg.len(),
71-
longtouch: true,
81+
accept_is_nextarrow: true, // longtouch takes priority over this if enabled
82+
longtouch: is_final,
7283
..Default::default()
7384
};
7485
confirm::confirm(&params).await?;

0 commit comments

Comments
 (0)