feat: update to askar 0.5 #81
Conversation
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
…rt for providing a custom Argon2 config. In addition it adds methods to list the open sessions and scans on a store. Signed-off-by: Timo Glastra <[email protected]>
🦋 Changeset detectedLatest commit: 1fc1b2c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
hacdias
left a comment
hacdias
left a comment
There was a problem hiding this comment.
Looks good to me, but it'd be great if someone with more context about Askar looks at it too.
Signed-off-by: Timo Glastra <[email protected]>
|
Yes that'd be good. Let's merge it for now so i can test with the alpha release. And then @berendsliedrecht or @genaris can review and I will open a follow up pr if needed. (especially the C++ stuff would be good to get your input on @berendsliedrecht , i'm not 100% if the config for argon2 in React Native is the best way to do it) |
|
|
||
| auto ret = createReturnValue(rt, code, &out); | ||
| askar_buffer_free(out); | ||
| if (code == ErrorCode::Success) { |
There was a problem hiding this comment.
why is this added?
There was a problem hiding this comment.
I noticed that when an error was thrown, e.g. the config to the argon 2 method was null while custom mode was used. This resulted in an error, and the return value not being written. Then it would crash trying to free the buffer.
Maybe the better approach would be to check if the return value is written or not?
There was a problem hiding this comment.
Hmm I think works good enough. It makes sense :).
| algorithm: number | ||
| version: number | ||
| parallelism: number | ||
| mem_cost: number |
There was a problem hiding this comment.
I think the pattern we use here is to convert the casing so the user provides memoryCost and we translate it to mem_cost. To have a JS-like interface.
There was a problem hiding this comment.
Yes that makes sense, that is the case in the top level api, but we can also do it like that in the lower level API
This updates Askar to 0.5, and integrates the new
Argon2custom config, allowing to replace react-native-argon2 in React Native.The only breaking change is in Argon2, which we don't use in Credo, so we can allow both 0.4.x and 0.5.x of Askar to be used with Credo 0.6. This also allows us to more easily test the new integration with Koffi
We need this to upgrade the Paradym Wallet to React Native 0.81 and use the new arch (replace react-native-argon2, while keeping the same derived password) animo/paradym-wallet#438