Skip to content

Add assertions to mnemonic generation - #976

Open
insumisos wants to merge 1 commit into
SeedSigner:devfrom
insumisos:dev
Open

Add assertions to mnemonic generation#976
insumisos wants to merge 1 commit into
SeedSigner:devfrom
insumisos:dev

Conversation

@insumisos

Copy link
Copy Markdown

Description

Problem or Issue being addressed

The dice and coin seed generation functions are trusting that the UI will submit the appropriate number of dice rolls or coin flips. Should the UI not do this, for whatever reason, then insufficient entropy could be passed to the hash function.

Solution

Don't trust the UI (client side validation), do local validation and fail hard if there isn't sufficient entropy.

Additional Information

Since the asserts are not handled then will probably crash the UI but that's probably desirable.

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

Checklist

I ran pytest locally

  • All tests passed before submitting the PR
  • I couldn't run the tests
  • N/A

I added or updated tests

Any new or altered functionality should be covered in a unit test. Any new or updated sequences require FlowTests.

  • Yes
  • No, I’m a fool
  • N/A

Removed ColdCard test as their example has small number of rolls (since it was supposed to be mixed with a HRNG) which fails. Instead tests whether assertion is thrown for invalid rolls.


I tested this PR hands-on on the following platform(s):


I have reviewed these notes:

  • Keep your changes limited in scope.
  • If you uncover other issues or improvements along the way, ideally submit those as a separate PR.
  • The more complicated the PR, the harder it is to review, test, and merge.
  • We appreciate your efforts, but we're a small team of volunteers so PR review can be a very slow process.
  • Please only "@" mention a contributor if their input is truly needed to enable further progress.
  • I understand

Should the UI not pass the correct number of dice rolls or coin flips
then fail hard.
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 this pull request may close these issues.

1 participant