Added verifySignature function in PublicAccount class

[aizaiz]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 28

• 2 of 13 (15.38%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-1.2%) to 67.358%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/model/account/PublicAccount.ts	2	13	15.38%

Totals
Change from base Build 25:	-1.2%
Covered Lines:	722
Relevant Lines:	1004

---

💛 - Coveralls

[Vektrat]

hi thanks for the submission, three details I'd like to mention for now:

1. I'd the "!" from the errors, we are informing users, not punishing them :)
2. There are a few magic numbers related to sizes, it'd be great to use their declarations if they exist declared in some file (and otherwise declare them here for clarity), @aleixmorgadas do you know where can they be found?
3. Avoid start naming variables with "_", it is usually a convention for implying no visibility on them, name it something more readable and informative if it collides with the parameter, such as "hexData" or "convertedData"
4. Would it be possible to add some tests?

@aleixmorgadas should this function safe-convert from utf8? looks like a precondition thing to me

[ghost]

changing publicKey: string by publicAccount: PublicAccount will remove the next code,

if (publicKey.length !== 64 && publicKey.length !== 66) {
    throw new Error('Not a valid public key');
}

making the design safer.

[ghost]

signature has some constrains too, I'm thinking in create a class to represent the signature and add there the invariants. It should remove the:

if (signature.length !== 128) {
    throw new Error('Signature length is incorrect !');
}
// ...
const _signature = convert.hexToUint8(signature);

Also, it hasn't been checked that the signature is an hexadecimal string.

[aizaiz]

Also, it hasn't been checked that the signature is an hexadecimal string

well, this "if condition" below made sure that the signature must be hexadecimal

if (convert.isHexString(signature)) {
        throw new Error('Signature must be hexadecimal only');
}

[aizaiz]

halo @Vektrat, thank you for the input. regarding the test, I will consider it later after I finish this first, because I have another important stuff to do.

@aleixmorgadas so what is the next step? should I add some change base on @Vektrat and your input and create another pull request?

[ghost]

@aleixmorgadas so what is the next step? should I add some change base on @Vektrat and your input and create another pull request?

Take the input from @Vektrat, and commit to your branch, this PR will be updated automatically, so we will be able to see the latest changes.

[aizaiz]

@aleixmorgadas I have pushed a new commit, please review it. I changed publicKey to publicAccount but it's a little bit weird because the function it self is inside PublicAccount class. I think we should move the function to an utility class.

[ghost]

The next step should be adding the tests. They should guide the usage and then we find out if it has to belong to another class or not.

[aizaiz]

@aleixmorgadas I added some test
but I got errors from nem2-library code

4 failing

  1) Signature verification
       Can verify a signature:
     TypeError: array.every is not a function
      at Object.isZero (node_modules/nem2-library/dist/coders/array.js:61:16)
      at Object.verify (node_modules/nem2-library/dist/crypto/keyPair.js:162:24)
      at Object.verify (node_modules/nem2-library/dist/crypto/keyPair.js:256:25)
      at Function.verifySignature (dist/src/model/account/PublicAccount.js:82:39)
      at Context.it (dist/test/model/account/PublicAccount.spec.js:36:53)

  2) Signature verification
       Return false if wrong public key provided:
     TypeError: array.every is not a function
      at Object.isZero (node_modules/nem2-library/dist/coders/array.js:61:16)
      at Object.verify (node_modules/nem2-library/dist/crypto/keyPair.js:162:24)
      at Object.verify (node_modules/nem2-library/dist/crypto/keyPair.js:256:25)
      at Function.verifySignature (dist/src/model/account/PublicAccount.js:82:39)
      at Context.it (dist/test/model/account/PublicAccount.spec.js:61:53)

  3) Signature verification
       Return false if data is not corresponding to signature provided:
     TypeError: array.every is not a function
      at Object.isZero (node_modules/nem2-library/dist/coders/array.js:61:16)
      at Object.verify (node_modules/nem2-library/dist/crypto/keyPair.js:162:24)
      at Object.verify (node_modules/nem2-library/dist/crypto/keyPair.js:256:25)
      at Function.verifySignature (dist/src/model/account/PublicAccount.js:82:39)
      at Context.it (dist/test/model/account/PublicAccount.spec.js:69:53)

  4) Signature verification
       Return false if signature is not corresponding to data provided:
     TypeError: array.every is not a function
      at Object.isZero (node_modules/nem2-library/dist/coders/array.js:61:16)
      at Object.verify (node_modules/nem2-library/dist/crypto/keyPair.js:162:24)
      at Object.verify (node_modules/nem2-library/dist/crypto/keyPair.js:256:25)
      at Function.verifySignature (dist/src/model/account/PublicAccount.js:82:39)
      at Context.it (dist/test/model/account/PublicAccount.spec.js:77:53)