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(core): Verify wallet id on reconstruction #443

Merged
merged 6 commits into from
Dec 15, 2023
Merged

feat(core): Verify wallet id on reconstruction #443

merged 6 commits into from
Dec 15, 2023

Conversation

amanCypherock
Copy link
Contributor

With this change, the wallet ID is generated and verified every time the seed/mnemonics is reconstructed.

  • This would prevent wallet collision when the two shares are generated from the same mnemonics and wallet name but in different sessions.

@amanCypherock amanCypherock added the enhancement Improvements to an existing feature label Nov 27, 2023
@amanCypherock amanCypherock self-assigned this Nov 27, 2023
@rohanagarwal94
Copy link

Task linked: PRF-6555 Verify wallet id on recreation

@amanCypherock amanCypherock changed the title feat(core): Save wallet nonce while creation feat(core): Verify wallet id on reconstruction added Nov 27, 2023
@ujjwal-cyph ujjwal-cyph changed the title feat(core): Verify wallet id on reconstruction added feat(core): Verify wallet id on reconstruction Dec 1, 2023
ujjwal-cyph
ujjwal-cyph requested changes Dec 4, 2023
View reviewed changes
Copy link
Collaborator

@ujjwal-cyph ujjwal-cyph left a comment •
edited
Loading

Choose a reason for hiding this comment

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

the functions reconstruct_seed & reconstruct_mnemonics have too much commonality. can we reduce the duplicity in logic? Since we are at this now.

@amanCypherock amanCypherock force-pushed the feat/verify-wallet-id/PRF-6555 branch from 56917fb to 371e5bd Compare December 8, 2023 13:28
ujjwal-cyph
ujjwal-cyph approved these changes Dec 13, 2023
View reviewed changes
Copy link
Collaborator

@ujjwal-cyph ujjwal-cyph left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. we can merge after closing the active comments.

@amanCypherock amanCypherock merged commit 22b2d7d into refactor/core-framework Dec 15, 2023
13 checks passed
@amanCypherock amanCypherock deleted the feat/verify-wallet-id/PRF-6555 branch December 15, 2023 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ujjwal-cyph ujjwal-cyph ujjwal-cyph approved these changes

Assignees

@amanCypherock amanCypherock

Labels
enhancement Improvements to an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amanCypherock @rohanagarwal94 @ujjwal-cyph