-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but somebody who is familiar with test.sh
probably need to have a look at this.
Also needs to be rebased
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -1099,7 +1099,7 @@ fn save_key(path: &Path, key: &Secret) { | |||
if let Err(e) = restrict_permissions_owner(path, true, false) { | |||
warn!(target: "network", "Failed to modify permissions of the file ({})", e); | |||
} | |||
if let Err(e) = file.write(&key.hex().into_bytes()) { | |||
if let Err(e) = file.write(&key.hex().into_bytes()[2..]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this needed or was changed just for "style"? I.e. will Secret::from_str
work with and without the 0x
prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was required, test caught it.
Related https://github.com/paritytech/ethabi/pull/85#issuecomment-377168526