refactor: replaced built-in crypto library with @web5/crypto#816
refactor: replaced built-in crypto library with @web5/crypto#816Toheeb-Ojuolape wants to merge 5 commits intodecentralized-identity:mainfrom
Conversation
|
Hi @Toheeb-Ojuolape! Thanks for this 🙏. It seems like the tests are failing around encryption, please see: |
Hi @LiranCohen thanks for your message. I have refactored my code for encryption using web5/crypto and ran the failing npm run test:node command on my local and all tests passed. I think all should be well now. Looking forward to your feedback |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #816 +/- ##
==========================================
+ Coverage 98.71% 98.95% +0.23%
==========================================
Files 74 74
Lines 11469 11560 +91
Branches 1652 1680 +28
==========================================
+ Hits 11322 11439 +117
+ Misses 141 115 -26
Partials 6 6 ☔ View full report in Codecov by Sentry. |
|
Hey @Toheeb-Ojuolape! Thanks for the update! There seems to still be a failure in the browser tests: https://github.com/TBD54566975/dwn-sdk-js/actions/runs/11299952497/job/31617946660?pr=816 you can run Additionally if you could add test cases for the failures to get patch coverage to 100%, the Codecov link above will be helpful in ensuring that. |
|
Hi @LiranCohen I've refactored the code it seems all should be fine now. I ran the browser test locally before I pushed and it worked fine |
LiranCohen
left a comment
There was a problem hiding this comment.
Finally got around to doing a more thorough review now that tests + build passes.
@Toheeb-Ojuolape Just a few small things and a couple of questions and this should be good! Appreciate you taking this on! 🙇
|
Hi @Toheeb-Ojuolape - can you resolve to the comments above so that @LiranCohen may review/merge? Thank you! |
Hi @taniashiba I think I've resolved all the comments now and I think the PR is ready. Fingers crossed 🤞🏽 |
|
Hey @LiranCohen - could you please review this PR and if approved, add the hacktoberfest-accepted label to it as well as soon as you can? Thank you! |
What type of PR is this? (check all applicable)
Description
This PR replaces the built-in crypto library with @web5/crypto in the encryption.ts file
Related Tickets & Documents
Resolves #672
Mobile & Desktop Screenshots/Recordings
Added code snippets?
Added tests?
[optional] Are there any post-deployment tasks we need to perform?
Please run npm install or make sure @web5/crypto package is installed before testing
[optional] What gif best describes this PR or how it makes you feel?