Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: support encrypt and decrypt json wallets #1041

Merged
merged 51 commits into from
Aug 11, 2023
Merged

feat: support encrypt and decrypt json wallets #1041

merged 51 commits into from
Aug 11, 2023

Conversation

Torres-ssf
Copy link
Contributor

@Torres-ssf Torres-ssf commented Jun 2, 2023 •
edited
Loading

This PR introduces new functionality to encrypt and decrypt JSON wallets, closely following the implementation found in the Rust SDK.

@Torres-ssf Torres-ssf self-assigned this Jun 2, 2023
@Torres-ssf Torres-ssf added the feat Issue is a feature label Jun 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023 •
edited
Loading

Coverage report

St.❔
 Category Percentage Covered / Total
🟢 Statements
84.98% (+0.19% 🔼)
 4663/5487
🟡 Branches
65.52% (+0.06% 🔼)
 705/1076
🟡 Functions
75.16% (+0.21% 🔼)
 802/1067
🟢 Lines
85.11% (+0.19% 🔼)
 4457/5237
Show new covered files 🐣
St.❔
 File Statements Branches Functions Lines
🟢
... / keystore-wallet.ts
 100% 100% 100% 100%
🟢
... / index.ts
 100% 100% 100% 100%
🟢
... / scrypt.ts
 100% 100% 100% 100%
🟢
... / keccak256.ts
 100% 100% 100% 100%
🟢
... / encryptJsonWalletData.ts
 100% 100% 100% 100%

Test suite run success

1139 tests passing in 197 suites.

Report generated by 🧪jest coverage report action from c4699d5

@Torres-ssf Torres-ssf marked this pull request as ready for review June 2, 2023 11:35
danielbate
danielbate previously requested changes Jun 2, 2023
View reviewed changes
Copy link
Contributor

@danielbate danielbate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking really good, love all the mocking in the unit test, I'm sure it was a bit painful. Haven't completely finished reviewing the encrypt and decrypt logic but will come back to it.

apps/docs/src/guide/wallets/encrypting-and-decrypting-json-wallets.md Outdated Show resolved Hide resolved
apps/docs/src/guide/wallets/encrypting-and-decrypting-json-wallets.md Outdated Show resolved Hide resolved
apps/docs/src/guide/wallets/encrypting-and-decrypting-json-wallets.md Outdated Show resolved Hide resolved
apps/docs/src/guide/wallets/encrypting-and-decrypting-json-wallets.md Outdated Show resolved Hide resolved
arboleya
arboleya requested changes Jun 5, 2023
View reviewed changes
packages/wallet/src/keystore-wallet.ts Outdated Show resolved Hide resolved
@Torres-ssf
Copy link
Contributor Author

This is blocked by #1049

@arboleya arboleya added blocked and removed blocked labels Jul 23, 2023
Dhaiwat10
Dhaiwat10 previously approved these changes Aug 7, 2023
View reviewed changes
arboleya
arboleya previously approved these changes Aug 10, 2023
View reviewed changes
@danielbate danielbate dismissed stale reviews from Dhaiwat10 and arboleya via bcf4a29 August 11, 2023 10:26
@danielbate
Copy link
Contributor

@Torres-ssf I have pushed this commit to resolve the failed CI. This is a new CI check brought in to support the docs hub via #1177. Unfortunately it has also dismissed the reviews @arboleya @Dhaiwat10 🙃

arboleya
arboleya previously approved these changes Aug 11, 2023
View reviewed changes
@Torres-ssf
Copy link
Contributor Author

@Torres-ssf I have pushed this commit to resolve the failed CI. This is a new CI check brought in to support the docs hub via #1177. Unfortunately it has also dismissed the reviews @arboleya @Dhaiwat10 upside_down_face

@danielbate thanks 🙏
Can you take a look at this one?

@Torres-ssf Torres-ssf enabled auto-merge (squash) August 11, 2023 11:14
danielbate
danielbate requested changes Aug 11, 2023
View reviewed changes
Copy link
Contributor

@danielbate danielbate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really clean implementation and great test examples. Couple small changes, thoughts and also nits (sorry).

apps/docs/src/guide/wallets/encrypting-and-decrypting-json-wallets.md Outdated Show resolved Hide resolved
packages/crypto/src/node/encryptJsonWalletData.ts Outdated Show resolved Hide resolved
packages/crypto/src/shared/keccak256.test.ts Outdated Show resolved Hide resolved
packages/crypto/src/shared/scrypt.test.ts Outdated Show resolved Hide resolved
packages/wallet/src/wallet-unlocked.test.ts Outdated Show resolved Hide resolved
packages/wallet/src/wallet-unlocked.test.ts Outdated Show resolved Hide resolved
packages/wallet/src/wallet.test.ts Outdated Show resolved Hide resolved
packages/crypto/src/shared/index.ts Outdated Show resolved Hide resolved
@danielbate danielbate self-requested a review August 11, 2023 12:06
danielbate
danielbate approved these changes Aug 11, 2023
View reviewed changes
Copy link
Contributor

@danielbate danielbate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@Torres-ssf Torres-ssf merged commit 9282391 into master Aug 11, 2023
7 checks passed
@Torres-ssf Torres-ssf deleted the st/feat/support-encrypt-json-wallet branch August 11, 2023 12:31
@github-actions github-actions bot mentioned this pull request Aug 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielbate danielbate danielbate approved these changes

@arboleya arboleya arboleya approved these changes

@digorithm digorithm Awaiting requested review from digorithm digorithm is a code owner

@camsjams camsjams Awaiting requested review from camsjams

@nedsalk nedsalk Awaiting requested review from nedsalk nedsalk is a code owner

@Dhaiwat10 Dhaiwat10 Awaiting requested review from Dhaiwat10 Dhaiwat10 is a code owner

Assignees

@Torres-ssf Torres-ssf

Labels
feat Issue is a feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add examples to the docs for storing and retrieve JSON-based wallets
4 participants
@Torres-ssf @danielbate @arboleya @Dhaiwat10