Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Create an account for chain=dev #5612

Merged
merged 4 commits into from May 19, 2017
Merged

Create an account for chain=dev #5612

merged 4 commits into from May 19, 2017

Conversation

tomusdrw
Copy link
Collaborator

Closes #5335
Also contains #4414 - I've noticed that Secret semantics was changed anyway (it doesn't necessarily store a valid secret any more), so I've implemented convenient conversions and replaced occurrences in code.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels May 12, 2017
@tomusdrw tomusdrw requested review from debris and keorn May 12, 2017 14:00
@rphmeier
Copy link
Contributor

rphmeier commented May 12, 2017

it doesn't necessarily store a valid secret any more

I think this is really bad. There isn't a point in having a Secret type which can store invalid secrets.

@tomusdrw
Copy link
Collaborator Author

Yeah, I agree. Maybe I'm mistaken, but afaiu the code, secret-arithmetic may result in some invalid secrets, we can log it as a separate issue then.

@debris
Copy link
Collaborator

debris commented May 12, 2017

afaik, it can only result in invalid secrets if secp context is not initialized properly

@tomusdrw
Copy link
Collaborator Author

0 is not a valid secret and you can get 0 by subtracting a secret from itself.

@gavofyork
Copy link
Contributor

would be nice to have an option in the chain spec file that allows you to provide a secret rather than an account as a key to the genesis accounts section; in this case it would auto-create the account for you.

but this is a good first effort in this direction.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels May 19, 2017
@gavofyork gavofyork merged commit 3ff7279 into master May 19, 2017
@gavofyork gavofyork deleted the chain-dev branch May 19, 2017 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants