Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wording for "Safety checks" feature #1133

Closed
matejcik opened this issue Jul 24, 2020 · 10 comments
Closed

wording for "Safety checks" feature #1133

matejcik opened this issue Jul 24, 2020 · 10 comments
Assignees
Labels
bug Something isn't working as expected core Trezor Core firmware. Runs on Trezor Model T and T2B1. protobuf Structure of messages exchanged between Trezor and the host trezorlib Python library and the command line trezorctl tool.
Milestone

Comments

@matejcik
Copy link
Contributor

in #1126 we introduced "Unsafe prompts" setting. This is confusing because of double-negatives: "enabling unsafe prompts" is the weaker setting and "disabling" is stronger.

We came up with an idea to rename this to "safety checks", with configurable levels, the default being "strict".

For trezorctl, we should change:

  • trezorctl set unsafe-prompts off to trezorctl set safety-checks strict
  • trezorctl set unsafe-prompts on to trezorctl set safety-checks prompt

For protobuf the following field should be changed:

optional bool unsafe_prompts = 9;  // allow or disallow unsafe prompts

to:

enum SafetyCheckLevel {
    STRICT = 0;
    PROMPT = 1;
}

optional SafetyCheckLevel safety_checks = 9;

This way binary compatibility holds, because bool is basically an enum with FALSE = 0, TRUE = 1

Open question: how to reword the on-device screens?

current, for lowering strictness:

Unsafe prompts
Trezor will allow you to confirm actions which might be dangerous.

Allow unsafe prompts?

for raising:

Unsafe prompts
Do you really want to disable unsafe prompts?

@matejcik matejcik added core Trezor Core firmware. Runs on Trezor Model T and T2B1. trezorlib Python library and the command line trezorctl tool. bug Something isn't working as expected protobuf Structure of messages exchanged between Trezor and the host enhancement labels Jul 24, 2020
@matejcik matejcik added this to the 2020-09 milestone Jul 24, 2020
@andrewkozlik
Copy link
Contributor

Another idea for the possible settings. It would be nice to have an "enable temporarily" option. This would enable unsafe prompts until the next reboot, i.e. it wouldn't be written to storage. The typical use-case for this would be when a user has BTC sent to a LTC address and just needs to do a one-off fix. So it decreases the number of steps that the support team needs to instruct the user to do.

@tsusanka
Copy link
Contributor

tsusanka commented Sep 2, 2020

Let's agree on this. I like both comments how about we have three stages then: strict, prompt and temporary.

  • strict is the default setting and does not allow anything
  • prompt prompts on every such action
  • temp prompts on every such action but changes to strict on reboot

Wording suggestions:

strict:

Safety checks
Trezor will forbid all uncommon actions which might be potentially dangerous.

Are you sure?

prompt:

Safety checks
Trezor will allow some uncommon actions which might be potentially dangerous.

Are you sure?

temp:

Safety checks
Trezor will allow some uncommon actions which might be potentially dangerous.
This setting will be revoked after reboot.

Are you sure?

The third might be too long and I don't like "uncommon" very much. Ideas?

@mmilata
Copy link
Member

mmilata commented Sep 2, 2020

Unusual?

Also not sure about "reboot", might sound like you have to reboot your computer:) Not sure if "unplugging your Trezor" fits on the screen.

@andrewkozlik
Copy link
Contributor

I would try to be more succinct:
Trezor will let you approve some actions which might be unsafe.
or
Trezor will allow you to approve some actions which might be dangerous.

@andrewkozlik
Copy link
Contributor

This fits nicely: Trezor will allow you to approve some actions which might be unsafe.
But I am still not satisfied. I would like it to say something like "Do you wish to set safety checks to 'Prompt'?" and then the explanation.
image

@andrewkozlik
Copy link
Contributor

andrewkozlik commented Sep 2, 2020

I don't think we need to go into details about unplugging/rebooting, because it makes the explanation too lengthy. Using the word "temporarily" should do:
image

Another possibility is having an "Info" button like we have in Shamir backup, I think.

@tsusanka tsusanka removed their assignment Sep 2, 2020
@tsusanka
Copy link
Contributor

tsusanka commented Sep 2, 2020

I like @andrewkozlik's suggestions, let's go that way! @mmilata feel free to do this one as well, I think @matejcik does not have any work done here yet.

@mmilata
Copy link
Member

mmilata commented Sep 2, 2020

👍

For future reference there's some discusion regarding the temporary-prompt option in #1240 (comment).

@tsusanka
Copy link
Contributor

I think we can close this one?

@matejcik
Copy link
Contributor Author

the wording was changed in #1240 it seems

@tsusanka tsusanka modified the milestones: 2020-10, 2020-11 Oct 1, 2020
@tsusanka tsusanka modified the milestones: 2020-11, 2020-12 Nov 6, 2020
@tsusanka tsusanka modified the milestones: 2020-12, 2021-01 Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected core Trezor Core firmware. Runs on Trezor Model T and T2B1. protobuf Structure of messages exchanged between Trezor and the host trezorlib Python library and the command line trezorctl tool.
Projects
None yet
Development

No branches or pull requests

4 participants