chaintool/funga-eth
4
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

Ida/pbkdf2 #2

Closed
idaapayo wants to merge 8 commits from idaapayo/funga-eth:Ida/pbkdf2 into master
pull from: idaapayo/funga-eth:Ida/pbkdf2
merge into: chaintool:master
Conversation 11 Commits 8 Files Changed 5 +151 -50
idaapayo commented 2022-01-19 15:12:37 +01:00
First-time contributor
Copy Link

implementing pbkdf2

implementing pbkdf2
idaapayo added 2 commits 2022-01-19 15:12:38 +01:00
pkdf2 implementation 01598a8c59
second pbkdf2 implementation 1f5d057a9a
kamikazechaser reviewed 2022-01-19 15:21:54 +01:00
funga/eth/keystore/keyfile.py Outdated
@@ -29,2 +28,4 @@
'pbkdf2'
]
default_kdfparams = {
kamikazechaser commented 2022-01-19 15:21:54 +01:00
Owner
Copy Link

This should be renamed to default_scrypt_params

Thoughts? @lash Not sure which naming convention you prefer

This should be renamed to `default_scrypt_params` Thoughts? @lash Not sure which naming convention you prefer
idaapayo marked this conversation as resolved
kamikazechaser reviewed 2022-01-19 15:29:18 +01:00
funga/eth/keystore/keyfile.py
@@ -79,3 +103,2 @@
def to_dict(private_key_bytes, passphrase=''):
def to_dict(private_key_bytes, kdf :str, passphrase=''):
kamikazechaser commented 2022-01-19 15:29:18 +01:00
Owner
Copy Link

This is a potentially breaking change if this method is being used elsewhere. I suggest we default to scrypt for now.

This is a potentially breaking change if this method is being used elsewhere. I suggest we default to `scrypt` for now.
kamikazechaser reviewed 2022-01-19 15:48:41 +01:00
funga/eth/keystore/keyfile.py Outdated
@@ -57,0 +71,4 @@
itr = int(kdfparams['c'])
dklen = int(kdfparams['dklen'])
derived_key = hashlib.pbkdf2_hmac(
kamikazechaser commented 2022-01-19 15:48:41 +01:00
Owner
Copy Link

We can pass the params directly and return on on it to reduce verbosity.

We can pass the params directly and return on on it to reduce verbosity.
kamikazechaser reviewed 2022-01-19 15:51:42 +01:00
funga/eth/keystore/keyfile.py Outdated
@@ -57,0 +65,4 @@
@staticmethod
def from_pbkdf2(kdfparams=pbkdf2_kdfparams, passphrase=''):
hashname = kdfparams['prf']
kamikazechaser commented 2022-01-19 15:51:42 +01:00
Owner
Copy Link

prf key-value from keystore files could also be hmac-sha256, which isn't a valid param for hashlib.pbkdf2_hmac default to sha256 instead.

`prf` key-value from keystore files could also be `hmac-sha256`, which isn't a valid param for `hashlib.pbkdf2_hmac` default to `sha256` instead.
kamikazechaser marked this conversation as resolved
idaapayo added 1 commit 2022-01-21 09:14:45 +01:00
making review changes b9c6db414b
idaapayo closed this pull request 2022-01-21 09:21:13 +01:00
idaapayo reopened this pull request 2022-01-24 11:20:59 +01:00
idaapayo added 1 commit 2022-01-24 11:23:55 +01:00
review changes with tests b20853312d
kamikazechaser reviewed 2022-01-24 12:08:21 +01:00
funga/eth/keystore/keyfile.py Outdated
@@ -60,0 +69,4 @@
kdfparams['prf'].replace('sha256')
derived_key = hashlib.pbkdf2_hmac(
hash_name=kdfparams['prf'],
kamikazechaser commented 2022-01-24 12:08:21 +01:00
Owner
Copy Link

Change this to hash_name="sha256"

Change this to `hash_name="sha256"`
kamikazechaser marked this conversation as resolved
kamikazechaser added 1 commit 2022-01-24 12:10:38 +01:00
Merge branch 'master' into Ida/pbkdf2 0a4f3eaa98
kamikazechaser reviewed 2022-01-24 12:11:40 +01:00
kamikazechaser reviewed 2022-01-24 12:12:09 +01:00
funga/eth/runnable/keyfile.py Outdated
@@ -16,6 +16,15 @@ from funga.eth.keystore.keyfile import (
from_file,
to_dict,
)
# from pathlib import Path
kamikazechaser commented 2022-01-24 12:12:09 +01:00
Owner
Copy Link

Remove these comments

Remove these comments
kamikazechaser marked this conversation as resolved
kamikazechaser reviewed 2022-01-24 12:13:40 +01:00
funga/eth/keystore/keyfile.py Outdated
@@ -36,2 +37,3 @@
}
}
pbkdf2_kdfparams = {
kamikazechaser commented 2022-01-24 12:13:40 +01:00
Owner
Copy Link

Chnage this to default_pbkdf2_kdfparams

Chnage this to `default_pbkdf2_kdfparams`
idaapayo added 1 commit 2022-01-24 13:29:27 +01:00
making final review changes 1c0047398a
kamikazechaser reviewed 2022-01-24 17:48:55 +01:00
tests/test_cli.py Outdated
@@ -5,3 +5,3 @@
# external imports
from hexathon import strip_0x
from hexathon import strip_0x, add_0x
kamikazechaser commented 2022-01-24 17:48:55 +01:00
Owner
Copy Link

Revert these changes

Revert these changes
lash requested changes 2022-01-24 18:00:54 +01:00
funga/eth/keystore/keyfile.py Outdated
@@ -79,3 +99,2 @@
def to_dict(private_key_bytes, passphrase=''):
def to_dict(private_key_bytes, kdf :str, passphrase=''):
lash commented 2022-01-24 17:59:43 +01:00
Owner
Copy Link

can we have scrypt as default please? that makes it backward compatible

can we have scrypt as default please? that makes it backward compatible
funga/eth/runnable/keyfile.py Outdated
@@ -78,3 +80,3 @@
pk_bytes = os.urandom(32)
pk = coincurve.PrivateKey(secret=pk_bytes)
o = to_dict(pk_bytes, passphrase)
o = to_dict(pk_bytes, 'scrypt', passphrase)
lash commented 2022-01-24 18:00:17 +01:00
Owner
Copy Link

... and then you don't have to change this.

... and then you don't have to change this.
idaapayo added 1 commit 2022-01-24 18:08:16 +01:00
defaulting to scrypt in to_dict fun 23a94c3ba1
idaapayo added 1 commit 2022-01-24 18:13:01 +01:00
remove unused import and renaming pbkdf2 default params 4a4f76b19c
lash commented 2022-01-24 18:57:38 +01:00
Owner
Copy Link

merged e5cd1cad58

merged https://git.grassecon.net/chaintool/funga-eth/commit/e5cd1cad58fb2d9e39f37817843ad7f2fc5004d3
lash closed this pull request 2022-01-24 18:57:38 +01:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
kamikazechaser
lash
Labels
Clear labels
bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 good first issue
help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
kamikazechaser lash williamluke willruddick
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: chaintool/funga-eth#2
Delete Branch "idaapayo/funga-eth:Ida/pbkdf2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?