leather-io / extension

Leather browser extension
https://leather.io
MIT License
304 stars 142 forks source link

Parse function arguments for human readability during transaction signing #2273

Open markmhendrickson opened 2 years ago

markmhendrickson commented 2 years ago

Currently raw function arguments can be hard to read, especially for non-technical users.

We could parse their values for improved human readability while allowing the user to reveal the raw value as needed.

For example:

kyranjamie commented 2 years ago

I'm sure there's lots we can do to improve readability of function arguments in our UI.

But:

Unsigned integers: Remove the u prefix (e.g. u32 -> 32)

In Clarity an unsigned integer = u32, signed integer 32. So this would be confusing, and in some respects, wrong. Could we instead add some additional help text that explains what u32 means? Or, show 32 but still make it clear elsewhere its an unsigned integer?

markmhendrickson commented 2 years ago

I'm not sure I understand why communicating "Unsigned integer 32" is better for an end-user (and not just more confusing) than communicating "32"?

"32" is the value itself whereas "unsigned integer" speaks to the possible range of values, of which negative ones can't be manifested to the user here anyway.

We could and should certainly add a tooltip or something that lets the user understand better what format is used ("unsigned integer" here), especially for the case of a developer who's looking to debug. But that seems like a secondary concern.

kyranjamie commented 2 years ago

No disagreement that the number itself is the most important info, and should be communicated clearly for non-technical users. It should be the element with the greatest visual weight.

I'm just pushing the agenda that this shouldn't come at the cost of obfuscating/omitting information that is useful to a technical user, especially when it comes to info such as a function call's input.


32

Unsigned integer

larrysalibra commented 2 years ago

Thanks for opening this issue @markmhx.

Really like @kyranjamie 's suggestion for how to display u32.


The underlying problem is that many commonly used clarity smart contacts (pox, bns) have functions that should be taking strings but are instead asking for buffers. While I am sure there are very good technical reasons for this, it isn't in the spirit of Clarity - being human readable in order to make it easier to spot bugs and errors.

Instead of bringing Clarity, we are bringing confusion by asking users questions like:

We also ask developers to use bufferCVFromString boilerplate code to convert all of their human readable strings into unreadable buffers.

Instead of addressing this at the wallet level, perhaps this experience could be improved for both wallet builders, blockchain explorer makers, users and developers using clarity smart contracts somewhere else in the stack?

If that's not possible, perhaps wallet makers (or wallet library makers) could come up with a consistent way (a SIP? a library?) to display this data so that users can make informed decisions about confirming their transactions?

Examples

In this case, I am using the BNS contract and trying to make sure that my irreversible name transfer transaction is correct - that I'm operating on the correct name, that I'm not sending it to the wrong new owner, etc. The wallet asks me to confirm this:

Screen Shot 2022-02-26 at 19 52 37

Another example trying to renew a name:

Screen Shot 2022-02-26 at 20 53 33

How users are shown their pox stacking rewards bitcoin address:

Screen Shot 2022-02-28 at 12 26 42