RareData
commented
2 years ago Displaying 160-bit HEX strings is a suboptimal user-experience
Hex strings that can be decoded to all ASCII are supposed to be decoded to ASCII. I see a bug on line https://github.com/LedgerHQ/app-xrp/blob/master/src/xrp/amount.c#L176, that's supposed to be negated. If "XRP" is in the hex string, we should proceed to the else-case and display as hex.
Earlier versions of this firmware properly decoded all ASCII hex strings, e.g. 5553445400000000000000000000000000000000 to USDT. The decoder for all ASCII can be seen here: https://github.com/LedgerHQ/app-xrp/blame/master/src/xrp/strings.c#L21 and it behaves as expected for other decoding of e.g. MemoData.
In the XRPL, there are two different ways to assemble a currency-code. The first way (ISO standard codes) support only three-character ASCII strings (e.g.,
USD); the other way (non-standard, but still legitimate) is a 160-bit HEX-string value (for codes longer than 3 ASCII characters).I propose that when using the Nano to sign a payment transaction containing a non-standard currency code, the device will display both the HEX and the ASCII equivalent of the currency code.
Displaying the HEX string is correct IMO (i.e., not every HEX string maps to valid ASCII) and likely just good security practice (i.e., we want users to see the real bytes for what they're about to sign). However, displaying 160-bit HEX strings is a sub-optimal user-experience (if that's all that is displayed), especially given that many issued currency codes are actually just ASCII strings that have more than 3 characters (and are thus non-standard).
For example, I envision something like this scrolling across the device's display, as part of payment transaction signature approval:
Curious to hear thoughts around this proposal.