feat(certificate): first draft implementation #166
Conversation
| expect(certificate.encode({ ...cert, signature: sig })).toEqual( | ||
| certificate.encode({ | ||
| ...cert, | ||
| signature: sig | ||
| }) |
There was a problem hiding this comment.
Why are we checking if encode of the same input returns the same output? @fabiorigam
Also please add tests on casing of the signature like in thor-devkit
There was a problem hiding this comment.
Silly mistake.
However for the casing of the signature, it appears the encode function is case sensitive for the signer field. Is that an error in the implementation? @pierobassa
This is the test:
expect(certificate.encode(cert)).toStrictEqual(
certificate.encode({ ...cert2, signer: cert2.signer.toUpperCase() })
);
I think we need to add again the "lowercase conversion" into the encode function. @fabiorigam would you agree?
There was a problem hiding this comment.
Agreed with @fabiorigam not to implement the casing tests to be aligned with ethers convention:
264c909
| export interface Certificate { | ||
| purpose: string; | ||
| payload: { | ||
| type: string; |
There was a problem hiding this comment.
Just curious what values this can take ... if its an enumerated type for example?
There was a problem hiding this comment.
you mean the payload? @claytonneal
| * @param cert - The certificate object with a digital signature. | ||
| * @throws {Error} If the signature is missing, invalid, or doesn't match the signer's public key. | ||
| */ | ||
| function verify(cert: Certificate): void { |
There was a problem hiding this comment.
should this also verify that fields are not null/empty, e.g. if domain is empty string for example?
There was a problem hiding this comment.
it verifies that signature is not empty. Do we know what other fields ware mandatory and why?
Old implementation checked only for signature. It doesn't mean it is correct but do we know what we need to ccheck there?
Test CoverageSummary
|
Implementation and tests done
Test is failing calling the blake function
The hash message is 66 bytes instead of 32
The test has been renamed as certificate.torename.ts
@fabiorigam to review