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 1 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
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
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
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
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
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
idaapayo added 1 commit 2022-01-24 18:13:01 +01:00
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
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
bug
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: chaintool/funga-eth#2
No description provided.
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?