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.

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

Message methods should accept more argument types #5

Open
trevinhofmann opened this issue Feb 8, 2015 · 4 comments
Open

Message methods should accept more argument types #5

trevinhofmann opened this issue Feb 8, 2015 · 4 comments

Comments

@trevinhofmann
Copy link
Contributor

For example, sign could check if privateKey is a String or Buffer and convert accordingly. I can add these, but I'll wait for #2 to be merged to avoid any conflicts.

@maraoz
Copy link
Contributor

maraoz commented Feb 9, 2015

+1!

@yemel
Copy link

yemel commented Feb 13, 2015

+1

@trevinhofmann
Copy link
Contributor Author

I'm working on this now. Would it be best to call a new constructor and let that handle the conversion? The constructor would also throw a TypeError for bad arguments.

Message.prototype.sign = function sign(privateKey) {
  privateKey = new PrivateKey(privateKey);
  ...
}

@braydonf
Copy link
Contributor

I'd use PrivateKey.fromString() and PrivateKey.fromBuffer() and throw an error early. The code path that is taken within type recognition in PrivateKey (and others) can be unpredictable, and lead to some very strange bugs that are not very clear to what went wrong.

Likewise, it may be better to only accept one type of private key, and have less automatic type handling, for similar reasons. Type handling can be done prior to that and will be more visible and flexible to implementation.

There are multiple was that a private key can be represented as a string, and thus could be better handled prior via using:

var privateKey = bitcore.PrivateKey.fromWIF(someWIF); //wif
// or
var privateKey = bitcore.PrivateKey.fromString(someString); //hexa

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants