search

ZenGo-X / multi-party-ecdsa

Rust implementation of {t,n}-threshold ECDSA (elliptic curve digital signature algorithm).
GNU General Public License v3.0
966 stars 309 forks source link

Problem with gg20 #161

Closed eatruffle closed 1 year ago

eatruffle commented 2 years ago

Hi, I have problem with signing a "hello" sentence in the GG20 example. I am trying commands from the example and the result is:

Error: online stage failed

Caused by: couldn't complete signing: round 7: InvalidSig

Please give me a small advice :) What I am doing wrong.

survived commented 2 years ago

Hi @eatruffle, what are exact commands you executed that produce the error?

eatruffle commented 2 years ago

Hi,

first of all thank you for this amazing project, and for interest of my question. All comments, I am taking from the GG20 demo,

In ./target/release/examples/ folder,

I am starting the manager ./gg20_sm_manager After this, in three different terminals I am running

./gg20_keygen -t 1 -n 3 -i 1 --output local-share1.json ./gg20_keygen -t 1 -n 3 -i 2 --output local-share2.json ./gg20_keygen -t 1 -n 3 -i 3 --output local-share3.json

And last: Also, in two different terminals ./gg20_signing -p 1,2 -d "hello" -l local-share1.json ./gg20_signing -p 1,2 -d "hello" -l local-share2.json

And the result is

Error: online stage failed

Caused by: couldn't complete signing: round 7: InvalidSig

survived commented 2 years ago

Confirm, it reproduces for me with some probability. Not sure what's exact problem. cc @elichai @MatanHamilis

eatruffle commented 2 years ago

Hi I have modified the if statement below line 900 https://github.com/ZenGo-X/multi-party-ecdsa/blob/c80c87d130f4b742c346cbd3095f24d602834b2c/src/protocols/multi_party_ecdsa/gg_2020/party_i.rs#L900 and is OK Now I am checking what exactly is wrong with input parameters for this function.

kanishkatn commented 2 years ago

I'm getting a similar error, but it fails in round6. Did y'all get a chance to look into it? @survived @elichai @MatanHamilis

drewstone commented 2 years ago

Can confirm this happens with some probability on our end too:

Incorrect Alice's range proof in MtA: InvalidKey
DmytroTym commented 2 years ago

Hello everyone! @drewstone I think your error might be a duplicate of https://github.com/ZenGo-X/multi-party-ecdsa/issues/165. At least I was only able to reproduce it when trying to sign with incorrect keys/keys in the incorrect order. @eatruffle I'm not sure what is the modification you're suggesting. Anyway, after playing around a little bit, I noticed that sometimes partial signatures that the parties are receiving (here they are: https://github.com/ZenGo-X/multi-party-ecdsa/blob/master/examples/gg20_signing.rs#L78) are their own. I.e. when party 1 and party 2 are signing a message, during the last round, party 1 should send their share to party 2 and vice versa. Instead, sometimes party 1 ends up with their own share and 2 with their own. Needless to say, in such cases, they are unable to sign. The problem is that indices assigned to parties during the offline stage of signing might differ from indices assigned to parties during the online stage. My solution is to simply remove _ from _i here: https://github.com/ZenGo-X/multi-party-ecdsa/blob/master/examples/gg20_signing.rs#L58, so that the new index shadows variable i and is used in the subsequent exchange of messages (as it probably should). @survived, do you agree? In any case, I think the error originates in the demo, and not the main library code, which is probably good news

DmytroTym commented 2 years ago

@MatanHamilis could we maybe implement this fix? It's a one-liner, I'll do the PR Also, I could write a better user guide for the demo as at least two people got confused by running parties in the incorrect order and getting errors. Finally the most important addition to README imo - stating explicitly that GG18 in its current form is insecure (there were also questions about that)

0xYao commented 1 year ago

@MatanHamilis could we maybe implement this fix? It's a one-liner, I'll do the PR Also, I could write a better user guide for the demo as at least two people got confused by running parties in the incorrect order and getting errors. Finally the most important addition to README imo - stating explicitly that GG18 in its current form is insecure (there were also questions about that)

Hey, I also have the problem of getting the InvalidSig error from time-to-time. Did we get a chance to confirm and push the fix for it?

0xYao commented 1 year ago

Finally the most important addition to README imo - stating explicitly that GG18 in its current form is insecure (there were also questions about that)

Could you expand on why gg18 is insecure?

DmytroTym commented 1 year ago

@0xYao I believe the fix hasn't been implemented yet. As to the (in)security of gg18: the current ZenGo implementation does not use range proofs for MtA subprotocol which was shown to be insecure. The problem is solved in the implementation of gg20.

KanievskyiDanylo commented 1 year ago

@MatanHamilis could we maybe implement this fix? It's a one-liner, I'll do the PR Also, I could write a better user guide for the demo as at least two people got confused by running parties in the incorrect order and getting errors. Finally the most important addition to README imo - stating explicitly that GG18 in its current form is insecure (there were also questions about that)

Thanks for your help. Despite your advice, I still face this issue. Even if run signing clients in the right order. 

Error: online stage failed

Caused by:
    couldn't complete signing: round 7: InvalidSig

I did the same steps from the tutorial. But unfortunately, sometimes got this issue. Do you know how to prevent this problem?

KanievskyiDanylo commented 1 year ago

@MatanHamilis could we maybe implement this fix? It's a one-liner, I'll do the PR Also, I could write a better user guide for the demo as at least two people got confused by running parties in the incorrect order and getting errors. Finally the most important addition to README imo - stating explicitly that GG18 in its current form is insecure (there were also questions about that)

Thanks for your help. Despite your advice, I still face this issue. Even if run signing clients in the right order.

Error: online stage failed

Caused by:
    couldn't complete signing: round 7: InvalidSig

I did the same steps from the tutorial. But unfortunately, sometimes got this issue. Do you know how to prevent this problem?

Problem is solved. Just remove _ from _i here, as @DmytroTym mentioned above. https://github.com/ZenGo-X/multi-party-ecdsa/blob/master/examples/gg20_signing.rs#L58