NEAR-Edu / near-certification-tools

2 stars 2 forks source link

Feature/image hash #67

Closed ryancwalsh closed 2 years ago

ryancwalsh commented 2 years ago

@encody Can you please pull this down and run it locally and play around and carefully review this PR, ideally today if you have time (or tomorrow morning)?

ryancwalsh commented 2 years ago

@encody One part I'm concerned about in particular is the crypto.createHash function at https://github.com/NEAR-Edu/near-certification-tools/pull/67/files#diff-834ea6e4b35123beeb46e2faddceeca3470af430719d7b30f4fee954f77f358cR25

I'm seeing it generate values like GTs0iNzqCTn9VDuOHuPIu3+nE78GZkf744DpeeNR6qY=, which then https://codebeautify.org/base64-decode decodes to gobbledygook, while my uneducated guess would have been that it would decode to a hex string (which is usually how hashes are represented).

Docs at https://nodejs.org/api/crypto.html#cryptocreatehashalgorithm-options weren't clear to me.

Thanks. cc @petarvujovic98

ryancwalsh commented 2 years ago

@petarvujovic98 Can you please look at this in your morning since Jacob didn't get to it?

petarvujovic98 commented 2 years ago

@ryancwalsh As far as I am aware, representing a hash in hex encoding is just a matter of making the hash eye friendly, meaning it is just a different view of the data. Base64 does a similar thing, it is presenting us with a view of the data in 64 common characters, making them more eye friendly, so I am pretty sure that the function is working fine.

To clarify further. You probably used to seeing hashes in hex encoding because often times the output form is selected to be hex, like we are choosing it to be base64 here (.digest('base64') vs .digest('hex')). If you were to omit the 'base64' parameter in the digest call you would get the same value that you get from base64 decoding the result of this function