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

Add experimental features flag #1243

Closed
tsusanka opened this issue Sep 3, 2020 · 4 comments · Fixed by #1297
Closed

Add experimental features flag #1243

tsusanka opened this issue Sep 3, 2020 · 4 comments · Fixed by #1297
Assignees
Milestone

Comments

@tsusanka
Copy link
Contributor

tsusanka commented Sep 3, 2020

Upon discussion about feature flags we come up with several conclusions.

  1. In an ideal world we would like to remove the feature completely from the build while maintaining it in the master branch. We do not want to use any if and ifdefs though. We will check if there is some nice tooling for this but we are somewhat skeptical.
  2. We might introduce one new setting called "experimental features" which would be stored in Trezor. By allowing it you could use such features. This has number of benefits: 1) developers can turn it on easily 2) the code stays in the firmware 3) low maintenance. We could link this to the unstable protobuf option we have introduced in Add "unstable" tag to protobuf definitions #1220.
  3. We might do (2) but merge the setting into the Safety checks (wording for "Safety checks" feature #1133) by using a bitfield. We have to specify what the bitfield should contain exactly. So far we have (paths_temp is not stored, right?):
  • paths_strict - Strict paths checks. On by default. If turned off warnings are showed.
  • experimental - Allows experimental features. Off by default.
@tsusanka tsusanka added this to the backlog milestone Sep 3, 2020
@mmilata
Copy link
Member

mmilata commented Sep 3, 2020

Side note regarding turning Safety checks into bitfield: currently setting the field to Prompt influences not only path checks but also transaction fee hard limit (#1087). If we decide to turn the setting into a bitfield the fee limit probably deserves its own bit and we need to either:

  • introduce storage upgrade which turns Prompt (0b00000001) to fee_checks | path_checks (0b00000011), or
  • live with this incompatibility and communicate it to users (not sure if it's an option).

Otherwise +1 for bitfield as I'm having trouble imagining adding meaningful levels that would form total order with the existing levels.

@matejcik
Copy link
Contributor

matejcik commented Sep 3, 2020

I don't think we should mix the "bitfield for experimental features" thing with the safety checks thing.

In my view, Safety checks are very much user-facing, and should be as easy to understand as possible:

  • if you're on Strict, you have the highest security standard. We want you to be here.
  • if you're on Prompt, we let you confirm some things that we otherwise wouldn't even ask about. Be very careful, but you can still do things if you know what you're doing.
  • if you're on a hypothetical lower level, you're probably trading security for convenience. Don't blame us if something goes wrong.

The bitfield is for mix-and-matching. You should be a power user (ideally a developer) to even touch this feature, and you eat what you cook.
Then the bitfield might allow you to do powerful things, like, e.g., we could use it to choose which USB interfaces to run on the debug build.

@tsusanka
Copy link
Contributor Author

I agree and I think we should go with (2) that is simply add a new unstable/experimental/beta flag which allow usage of some experimental API.

@tsusanka tsusanka modified the milestones: backlog, 2020-11 Sep 30, 2020
@tsusanka
Copy link
Contributor Author

Agreed on a standup we will have a new flag for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants