-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
fix: use all refresh msgs for combining new keyshares #30
Conversation
Webassembly check failing, unsure why since it's deep in the crates. |
@ivokub @drewstone , so this webassembly error I resolved twice, once when using the nightly compiler by removing the reference (ZenGo-X/curv#183) but after I migrated to the The github runner appears to be installing the stable toolchain so I am not sure what's happening here. Will compare to the successful runs on my earlier PR to see if anything changed. Edit: the earlier build that passed on CI was on nightly, looks like I got it wrong, need to look into it more. |
Ok ,this may be only on Linux both of these work for me locally on Apple silicon:
|
Yep, it's Linux specific I just managed to reproduce on a Linux box using the |
Ok, so this is a real mess, the fix on Linux is to remove the reference as it uses We can fix this with conditional compilation (but I suspect there is a much better fix - like not using the From what I can tell it does not seem to be maintained and there is an important issue which @ivokub and I discussed (ZenGo-X/curv#177) that we should implement to make the WASM version more robust (see the warning here) and ideally pave the way to removing the native GMP bindings. So the question then becomes do you want me to fix this and we use What do you think? |
@drewstone, can you please review and merge this if you are happy and I will fix the webassembly checks in a subsequent PR please? |
Summary of changes
In FS-DKR subprotocol for key refresh, the parties secret share their current private share and send the corresponding shares to other parties who then recombine their shares to get their new private share.
However, in the current implementation, all parties only use first
current_t+1
received shares to combine their new secret share. The new private shares are valid (they are secret shared parts of the global private key) due to SS, but they do not include the contribution of all parties taking part in the key refresh round. This degrades the entropy of the private shares.This PR changes the behaviour of the parties to take into account of all received messages. Additionally I removed the threshold argument in methods which do not need it anymore.
This PR corresponds to tangle-network/fs-dkr#16 with also using all refresh messages in join messages.
The tests mostly succeed. May it be that #28 affects also refresh tests?