dwn-sdk-js
dwn-sdk-js copied to clipboard
refactor: replaced built-in crypto library with @web5/crypto
What type of PR is this? (check all applicable)
- [x] ♻️ Refactor
- [ ] ✨ New Feature
- [ ] 🐛 Bug Fix
- [ ] 📝 Documentation Update
- [ ] 👷 Example Application
- [ ] 🧑💻 Code Snippet
- [ ] 🎨 Design
- [ ] 📖 Content
- [ ] 🧪 Tests
- [ ] 🔖 Release
- [ ] 🚩 Other
Description
This PR replaces the built-in crypto library with @web5/crypto in the encryption.ts file
Related Tickets & Documents
Resolves https://github.com/TBD54566975/dwn-sdk-js/issues/672
Mobile & Desktop Screenshots/Recordings
Added code snippets?
- [ ] 👍 yes
- [x] 🙅 no, because they aren't needed
Added tests?
- [ ] 👍 yes
- [ ] 🙅 no, because they aren't needed
- [ ] 🙋 no, because I need help
- [x] 🥺 I ran unit tests for encryption.ts file and it passed
[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?

Hi @Toheeb-Ojuolape! Thanks for this 🙏.
It seems like the tests are failing around encryption, please see: https://github.com/TBD54566975/dwn-sdk-js/actions/runs/11186574275/job/31314960415?pr=816#step:9:1057
Hi @Toheeb-Ojuolape! Thanks for this 🙏.
It seems like the tests are failing around encryption, please see: https://github.com/TBD54566975/dwn-sdk-js/actions/runs/11186574275/job/31314960415?pr=816#step:9:1057
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 Report
Attention: Patch coverage is 94.33962% with 6 lines in your changes missing coverage. Please review.
Project coverage is 98.95%. Comparing base (
8b6eb13) to head (3d68bc2). Report is 13 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/utils/encryption.ts | 94.28% | 6 Missing :warning: |
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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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 npm run test:browser locally to reproduce the failure. It seems to be around base64url encoding, you can take a look at https://github.com/TBD54566975/dwn-sdk-js/blob/main/src/utils/encoder.ts for some common methods we use which could be helpful.
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
Hi @Toheeb-Ojuolape - can you resolve to the comments above so that @LiranCohen may review/merge? Thank you!
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!