chainflip-io / chainflip-backend

The Chainflip backend repo, including the Chainflip Node and CFE.
50 stars 14 forks source link

[SC-2391] Keygen verification #271

Closed dandanlen closed 2 years ago

dandanlen commented 3 years ago

SC-2391

It seems like it ought to be possible to verify a newly generated AggKey.

In particular, it would be good if some validator_id can be proven to have participated in the signing ceremony.

Maybe a signature with threshold 1/150? Or is there a simpler method?

Without this we have to simply trust anyone who submits the new key and says "I participated".

morelazers commented 3 years ago

I think we answered this question in the Discord right? I seem to recall we decided that there would be a 150/150 signing ceremony immediately after keygen.

Is this reflected in the currently-merged version of rotations, or is something extra required to support this?

dandanlen commented 3 years ago

This isn't reflected anywhere currently.

How would we want to structure this?

Do we need to verify the signature at all, or can we trust the witnesses to do so?

kylezs commented 3 years ago

Can we wrap it up and consider it part of the keygen ceremony (ie. a final verification step),

If we do it at all, I think this makes the most sense - it can post the new agg pub key + a signature over that key, with it's key. Keyception.

dandanlen commented 3 years ago

Do you think we need to post the signature back on-chain? I think this makes sense if we want to verify it on-chain, but we could also require that the sig is verified by the CFE - ie. the CFE will only post the new key to the SC if it has verified it.

morelazers commented 3 years ago

I think there's an extra layer of safety if the State Chain verifies the signature.

Effectively we're doubling the signature check (CFE + State Chain), but the State Chain is our source of truth for successful migrations, and we should ensure via the rules of the protocol (ie, not just trust) that the signature provided by the 150/150 signing ceremony as a part of the keygen ceremony is valid.

It's possible (to make no mention of likelihood) that some CFE implementation (or bug) could attest that some valid signature has been created with the new key, and that it is happy to proceed even if this key or signature is not actually valid. Then we might be in an unrecoverable state where the State Chain thinks we're good to go but the CFE is incapable of producing a valid signature.

We should (at this stage, until we know better) always be verifying signatures on the State Chain where possible before allowing the protocol to continue. Keygen happens roughly once per month, so efficiency is not particularly important, safety is.

dandanlen commented 3 years ago

I created a story for this https://app.clubhouse.io/chainflip/story/1863/on-chain-signature-verification

dandanlen commented 3 years ago

So shall we require the keygen ceremony to produce two artefacts: the public component of the threshold signature, and a signature over some hash? Hash of what? The public key itself?

kylezs commented 3 years ago

signature over some hash? Hash of what? The public key itself?

Public key itself makes sense to me, we already have the data to verify against that way, saves introducing another variable.

msgmaxim commented 3 years ago

I created a story for this https://app.clubhouse.io/chainflip/story/1863/on-chain-signature-verification

I'm assuming this story is only about verification on SC after keygen? If so, should I create another story for actually initiating the signing ceremony with the new key as part of the keygen ceremony? (I looks like we decided that SC shouldn't make an explicit request to CFE for that)

dandanlen commented 3 years ago

I'm assuming this story is only about verification on SC after keygen? If so, should I create another story for actually initiating the signing ceremony with the new key as part of the keygen ceremony? (I looks like we decided that SC shouldn't make an explicit request to CFE for that)

You're right, sorry.

Yes, I think signing should be an automatic final stage of the keygen ceremony. The without verification isn't much use, and by generating both in one step we avoid having to wait an extra block before starting the signing phase.

msgmaxim commented 3 years ago

OK, I created a CH story for that: https://app.clubhouse.io/chainflip/story/1892/check-threshold-key-with-a-dummy-signing-ceremony

morelazers commented 2 years ago

Related to #737

morelazers commented 2 years ago

On second thoughts, is this now a duplicate of #737 @dandanlen, or does this require API changes to enforce?

dandanlen commented 2 years ago

This is separate (but related) - still merits its own issue IMO. It's about the extra 'verification' signing ceremony to double-check a newly generated aggkey as was discussed in #698 .

kylezs commented 2 years ago

The linked SC issue is closed, seems this can be closed?

morelazers commented 2 years ago

The linked SC issue was not the correct one IMO. This one should stick around and I'll edit the link in the OP.

msgmaxim commented 2 years ago

Just to clarify one more time, here we say:

Yes, I think signing should be an automatic final stage of the keygen ceremony.

But it seems like we later decided (in #698) that SC will handle this instead:

I think it would be more consistent to let the SC do this (no extra special case, easier to track the ceremony id)

Did we simply forget to update the current issue, or is it the case that we still want verification to be automatic in CFE? @dandanlen

dandanlen commented 2 years ago

@msgmaxim, agree it's not very clear but if you scroll down a bit further in #698 I think after further discussion we went back to the idea of handling this on the CFE. Meaning:

Fyi. this will require some changes on the SC:

@morelazers let us know if this sounds right.

msgmaxim commented 2 years ago

if you scroll down a bit further in #698 I think after further discussion we went back to the idea of handling this on the CFE

I can only see the discussion around ceremony ids, but I can't find where we said it CFE will perform the signing automatically. Can you point me to the exact location where we say this or quote it here? @dandanlen

dandanlen commented 2 years ago

There is no exact location - it flip-flops a bit but we end up deciding that we will re-use the keygen ceremony id for the signing, which in my mind implies automatic verification. The only reason for another SC round-trip would be to generate a new ceremony id.

Happy to re-discuss if you think there's a good reason to do so.

@msgmaxim

morelazers commented 2 years ago

Re-defining keygen to include a signature process sounds good to me.

msgmaxim commented 2 years ago

I had assumed that we simply talked about what ceremony ids to expect from SC (needed for the purposes of dissallowing past ceremony ids), but that the signing request would still come from SC because we wanted to keep the CFE simple by removing this special case. I'm happy to go with the automatic approach though.

Unification of ceremony ids for keygen and signing.

Not sure what this means exactly. My current understanding is that keygen and signing share the same id space (can't overlap), except for the dummy signing ceremonies which have the same id as the corresponding keygen. Something like:

Keygen: 1 4
Signing: 1 2 3 4 5 6 7

Does this require some changes on SC?

Keygen completion extrinsic to take verification signature as an extra argument + verify the new aggkey before proceeding.

I'm assuming the work on CFE could start without these changes (performing a signing ceremony but without submitting the signature). Then when the SC is ready, we could simply provide the signature as an additional argument.

Edit: saw you created an issue when I posted this.

dandanlen commented 2 years ago

Not sure what this means exactly. My current understanding is that keygen and signing share the same id space (can't overlap), except for the dummy signing ceremonies which have the same id as the corresponding keygen. Something like:

Keygen: 1 4 Signing: 1 2 3 4 5 6 7

Does this require some changes on SC?

Yes, I opened an issue #1242

Currently the ids are independent, so they do overlap.

I'm assuming the work on CFE could start without these changes (performing a signing ceremony but without submitting the signature). Then when the SC is ready, we could simply provide the signature as an additional argument.

Yes, pretty sure we can all work in parallel and then activate the feature when everything is ready.

morelazers commented 2 years ago

Currently the ids are independent, so they do overlap.

Could this be the cause of #1081, if we're making separate assumptions on CFE and State Chain?

msgmaxim commented 2 years ago

Could this be the cause of #1081, if we're making separate assumptions on CFE and State Chain?

I don't think so. In CFE we do allow overlapping ids between signing and keygen (at least so automatic signing is possible under the same id).

morelazers commented 2 years ago

I am going to close this one in favour of #1227, since it has a lot more detail.