Web3Auth / mpc-core-kit

16 stars 5 forks source link

[v3] _UNSAFE_exportTssKey doesn't work for ed25519 #157

Closed AyushBherwani1998 closed 4 months ago

AyushBherwani1998 commented 5 months ago

Description

When the keyType is ed25519, the _UNSAFE_exportTssKey returns the SEC1 format key instead of ed25519. Ideally if exporting ed25519 key is not supported it should throw an error.

Screenshot_2024-06-18_at_10 05 44_AM
metalurgical commented 5 months ago

From what I can tell from the codebase.

This function should be being called for tkey here instead (this outputs the seed).

_UNSAFE_exportTssEd25519Seed

Please double check.

metalurgical commented 5 months ago

_UNSAFE_exportTssKey, by itself, will not return the correct value for ed25519.

It is actually called as part of the above function.

metalurgical commented 5 months ago

https://github.com/Web3Auth/mpc-core-kit/pull/160

matthiasgeihs commented 5 months ago

There is a few caveats here: Export seed is only available if the seed has been imported. (I.e., if the account was setup with the ed25519 import key flow.) Otherwise, we don't have the ed25519 seed, because the DKG doesn't produce a seed, just the scalar.

himanshuchawla009 commented 5 months ago

There is a few caveats here: Export seed is only available if the seed has been imported. (I.e., if the account was setup with the ed25519 import key flow.) Otherwise, we don't have the ed25519 seed, because the DKG doesn't produce a seed, just the scalar.

i think we still create a seed on frontend and save it encrypted with the scalar. Will revisit this.

matthiasgeihs commented 4 months ago

@himanshuchawla009 we don't, i tested the export seed flow for a fresh account (using demo) and then encountered the issue. afterwards went through the code to confirm. but feel free to double check. the reasoning why this is actually a valid approach: if we would generate the seed in the frontend and then store it and share it (like import flow just generate seed at random), we would create a single point of failure, which we typically don't want with MPC.

AyushBherwani1998 commented 4 months ago

@matthiasgeihs fix works fine, tested flow with importing seed, and without importing seed.