Skip to content

Navigation Menu

Sign in
Sign up

Factorkey validation and rehydration error - #132

Open
lwin-kyaw wants to merge 4 commits into
master from
fix/rehydrate-manualSync
Open

Factorkey validation and rehydration error #132
lwin-kyaw wants to merge 4 commits into
master from
fix/rehydrate-manualSync

Conversation

@lwin-kyaw

@lwin-kyaw lwin-kyaw commented May 3, 2024
edited
Loading

Copy link
Copy Markdown
Contributor

This PR includes -

  • check 'SHARE_DELETED' on getFactorKeyMetadata and metadata validation
  • throw error instead of logging, for rehydration fail.
  • validate current factor key before copy or create new shares

lwin-kyaw added 2 commits May 3, 2024 11:01
'SHARE_DELETED' on getMetadata when using manualSync mode

ieow commented May 7, 2024

Copy link
Copy Markdown
Collaborator

Can you elaborate the issue ?
why the need to throw on rehydrate failed?

Comment thread src/mpcCoreKit.ts Outdated
await this.tKey.initialize({ neverInitializeNewKey: true });
const hashedFactorKey = getHashedPrivateKey(this.state.oAuthKey, this.options.hashedFactorNonce);
if ((await this.checkIfFactorKeyValid(hashedFactorKey)) && !this.options.disableHashedFactorKey) {
const isHashedFactorKeyValid = this.checkIfFactorKeyValid(hashedFactorKey);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

checkIfFactorKeyValid is async function

lwin-kyaw reacted with thumbs up emoji

lwin-kyaw commented May 7, 2024
edited
Loading

Copy link
Copy Markdown
Contributor Author

Can you elaborate the issue ? why the need to throw on rehydrate failed?

Hi, we don't necessarily need to throw rehydration error as when rehydration failed, all the later steps will also fail (factor login, create factor, etc...).
However throwing error on rehydration failed, can catch some errors in early stage and prevent users proceeding to other steps.
Anyway, we have logged out the error message when there's an error during rehydration, so, I think it would be better if we could throw the error also. It will also help us to write better test cases.

Sample case on rehydration failure

  • create first core kit with manualSync:true
  • create factor (factorKey1) and don't commit the changes
  • create second core kit and init (with default params, Mpc-Corekit will rehydrate with existing factorKey1 from sessionManager)
  • during rehydrate, we try to get the metadata for factorKey1, but since we don't commit changes in first corekit, factor metadata is not available, resulting in rehydration failure

@lwin-kyaw lwin-kyaw changed the title (削除) Throw error during rehydration (削除ここまで) (追記) Factorkey validation and rehydration error (追記ここまで) May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ieow ieow ieow left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /