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.

Sign up for GitHub

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

Jump to bottom

entropy-mnemonic conversions #19

Closed
wants to merge 13 commits into from
Closed

entropy-mnemonic conversions #19

wants to merge 13 commits into from

Conversation

sobuch
Copy link
Collaborator

@sobuch sobuch commented Oct 13, 2019

No description provided.

@sobuch sobuch self-assigned this Oct 13, 2019
@sobuch
Copy link
Collaborator Author

sobuch commented Oct 13, 2019

Should we include test cases for private functions? (its kind of design antipatern) If so, how to do it with current includes and so on?

mvondracek
mvondracek requested changes Oct 13, 2019
View reviewed changes
PA193_mnemonic_Slytherin/mnemonic.py Show resolved Hide resolved
PA193_mnemonic_Slytherin/mnemonic.py Outdated Show resolved Hide resolved
PA193_mnemonic_Slytherin/mnemonic.py Outdated Show resolved Hide resolved
PA193_mnemonic_Slytherin/mnemonic.py Show resolved Hide resolved
PA193_mnemonic_Slytherin/mnemonic.py Outdated Show resolved Hide resolved
@mvondracek
Copy link
Owner

Should we include test cases for private functions? (its kind of design antipatern) If so, how to do it with current includes and so on?

If there are tests for private functions, they depend on current implementation. If you change the private implementation, these tests will break. For example tests for _secure_seed_compare are more like doctests and can definitely be transformed into proper doctests if we do not want to test private functions. There is also question, whether everything except generate, recover, and verify should be private...

If you want to test private function, single underscore allows simple imports.

@mvondracek mvondracek mentioned this pull request Oct 14, 2019
Closed
@mvondracek
Copy link
Owner

Please see #22, thanks.

@sobuch sobuch changed the title WIP: entropy-mnemonic conversions entropy-mnemonic conversions Oct 14, 2019
@sobuch sobuch force-pushed the task-11-entropy-mnemonic-transformations branch from 24b6d07 to 013716c Compare October 15, 2019 06:30
mvondracek
mvondracek requested changes Oct 15, 2019
View reviewed changes
Copy link
Owner

@mvondracek mvondracek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related #26.

PA193_mnemonic_Slytherin/mnemonic.py Outdated Show resolved Hide resolved
PA193_mnemonic_Slytherin/mnemonic.py Outdated Show resolved Hide resolved
PA193_mnemonic_Slytherin/mnemonic.py Outdated Show resolved Hide resolved
PA193_mnemonic_Slytherin/mnemonic.py Show resolved Hide resolved
This was referenced Oct 17, 2019
Closed
Closed
@mvondracek
Copy link
Owner

Waits until #21 is merged. #19 (comment)

@mvondracek
Merge branch 'dev' into task-11-entropy-mnemonic-transformations
dc70166
@mvondracek
remove is_valid_mnemonic from mnemoniccli
f3f45f6 
Function `__is_valid_mnemonic`/`_is_valid_mnemonic`/`is_valid_mnemonic`
was removed in 545d161, then
0d2b555 removed calls to this function
inside `mnemonic` module.
This commit removes calls from mnemoniccli and reacts to raised
exceptions.
@mvondracek
remove is_valid_seed from mnemoniccli, EAFP
b644243
@mvondracek
Copy link
Owner

Updated from dev. Updated the program (mnemoniccli) and package init, because is_valid_mnemonic was removed. The program now uses EAFP instead of LBYL. Tests are OK now.

@mvondracek
add test_verify_invalid_mnemonic_too_long, failing until bug in `ve…
f0b267c 
…rify` is fixed

Too long mnemonic which was not validated causes `OverflowError:
password is too long.` in `pbkdf2_hmac` during `_generate_seed`.
@mvondracek mvondracek mentioned this pull request Oct 19, 2019
Closed
@mvondracek
Copy link
Owner

mvondracek commented Oct 19, 2019 •
edited
Loading

Detected bug #28. Added test in f0b267c, which will be failing until the bug is fixed.

ERROR: test_verify_invalid_mnemonic_too_long (PA193_mnemonic_Slytherin.test_mnemonic.TestMnemonicPublic)

Too long mnemonic phrase which should not be propagated to BPKDF2.

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/travis/build/mvondracek/PA193_mnemonic_Slytherin/PA193_mnemonic_Slytherin/test_mnemonic.py", line 217, in test_verify_invalid_mnemonic_too_long

    verify(mnemonic, seed)

  File "/home/travis/build/mvondracek/PA193_mnemonic_Slytherin/PA193_mnemonic_Slytherin/mnemonic.py", line 217, in verify

    generated_seed = _generate_seed(mnemonic, seed_password)

  File "/home/travis/build/mvondracek/PA193_mnemonic_Slytherin/PA193_mnemonic_Slytherin/mnemonic.py", line 39, in _generate_seed

    return pbkdf2_hmac('sha512', mnemonic, passphrase, PBKDF2_ROUNDS, SEED_LEN)

OverflowError: password is too long.

@sobuch
Copy link
Collaborator Author

sobuch commented Oct 20, 2019

superceded by #36

@sobuch sobuch closed this Oct 20, 2019
@sobuch sobuch deleted the task-11-entropy-mnemonic-transformations branch October 21, 2019 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsolodkova lsolodkova lsolodkova left review comments

@mvondracek mvondracek mvondracek requested changes

Assignees

@sobuch sobuch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sobuch @mvondracek @lsolodkova