feat: consensus on successful DKG prior to rotate-keys submission #1285

cylewitruk · 2025-01-30T02:37:13Z

Description

During DKG it is possible in certain situations, particularly when Bitcoin flash blocks occur while DKG is running, that DKG may succeed as observed by a majority of the signers, but actually fail for a subset. This can result in a coordinator in a such situation proceeding to rotate keys, which will succeed as long as threshold signers sign with their Stacks private key.

Part of: #1300

⚠️ This PR targets #1287 and is targeted by #1301, which needs to merge into this branch before this branch merges into #1287's branch.

This builds on-top of @xoloki 's #1265 with feat/frost-coordinator merged in (though now I have downgraded to WSTS 10) and proposes running a FROST signing round prior to signing and submitting a rotate-keys contract call. In short, it's basically exploiting FROST's requirement of 100% participation to act as a consensus mechanism for a preceding DKG round and provides a solid guarantee that a rotate-keys transaction can never be signed/submitted even in the event of multiple byzantine or faulty coordinators and/or signers.

After a DKG round has completed successfully (at least in the eyes of the coordinator), the coordinator initiates a FROST signing round on some random data using the newly generated DKG keys.
Each signer instantiates their own FROST coordinator state machine in addition to the regular Signer state machine. This state machine id is keyed on the new aggregate key + the current chain tip.
All signing-related messages are passed to both state machines, however only the Signer state machine publishes any messages to the other signers. In effect, each signer is acting as a "passive" coordinator. As FROST requires 100% participation, this gives us the side-effect that all signers must observe that all other signers have successfully participated in the signing round using the new key shares, and that we have generated a valid signature.
Upon completion of the signing round, each signer caches the signing result from its FROST coordinator.
Given success, the sBTC coordinator for the tenure will proceed to attempt to create, sign and submit the corresponding rotate-keys transaction.
When the Stacks signing request is received by the other signers, the signers will check the cached result of the preceding FROST signing round, and refuse to sign if it was not successful, didn't complete or was never run.

P.S. I'm not in love with a lot of the naming/messages, but in favor of getting this done I opted to not spend much time thinking on it -- it's easy to change later. But if you've got some awesome names, plz do suggest :)

Changes

Will update this later, have some cleaning up to do and will probably split some stuff out into separate PRs.

Testing Information

All existing tests pass and devenv works like a dream.

Checklist:

I have performed a self-review of my code
My changes generate no new warnings
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

…ds where all signers must participate

…s-for-rotate-keys

signer/src/transaction_coordinator.rs

signer/src/transaction_signer.rs

xoloki

LGTM, module a couple of quibbles

signer/src/transaction_coordinator.rs

signer/src/transaction_signer.rs

…-for-rotate-keys

matteojug

LGTM, new comments can be addressed directly in #1301 (if still relevant)

signer/src/transaction_signer.rs

commit 2168f58 Author: Cyle Witruk <[email protected]> Date: Thu Feb 6 16:08:22 2025 +0100 feat: consensus on successful DKG prior to rotate-keys submission (#1285) * update for wsts 11 * use wsts 11.0.0 from crates * fix rebase * use wsts-12.0.0 from crates * add FrostCoordinator to wsts_state_machine so we can run signing rounds where all signers must participate * wip * run a dkg signing before deploying contracts * new wsts commit * wip * seems to work * working version * remove dead code * minor wsts logging tweaks * rename wstsmessageid variant * it works, now need to clean up * downgrade wsts * proto backwards compatability fixes * some wsts cleanup/refactor * think that's it * add message support for a specific dkg wstsmessageid * add a dkg wstsmessageid variant * use block hash instead of random data * fmt * remove 100% requirement for stacks signing of rotate keys * bump p256k1 to 7.2.2 * fix merge artifacts * tracing fields * storage mut * lovely cascading changes * remove wstsmessage.txid infavor of id * remove save and prefer trait default impls * remove unused trait methods * logging stuff * rename message ids * use tracing constants * remove stale comment * remove box from some types * missed dkg_begin_pause * leftover dbg!() * refactor some validation * various pr comments * mut thing * remove unneeded allow(deprecated) * confused merge tool --------- Co-authored-by: Joey Yandle <[email protected]>

* update for wsts 11 * use wsts 11.0.0 from crates * fix rebase * use wsts-12.0.0 from crates * add FrostCoordinator to wsts_state_machine so we can run signing rounds where all signers must participate * wip * run a dkg signing before deploying contracts * new wsts commit * wip * seems to work * working version * remove dead code * minor wsts logging tweaks * rename wstsmessageid variant * it works, now need to clean up * downgrade wsts * proto backwards compatability fixes * some wsts cleanup/refactor * think that's it * add message support for a specific dkg wstsmessageid * add a dkg wstsmessageid variant * use block hash instead of random data * fmt * remove 100% requirement for stacks signing of rotate keys * bump p256k1 to 7.2.2 * pop stash * wip * working verifications * saving * fixing tests * seems to work * remove const generic, use 0 amount and add test for random keypair * vet bitcoinconsensus * working on cleaning up * fix merge artifacts * tracing fields * migration needs to update block hash/height * storage mut * lovely cascading changes * remove wstsmessage.txid infavor of id * remove save and prefer trait default impls * remove unused trait methods * logging stuff * rename message ids * use tracing constants * remove stale comment * remove box from some types * missed dkg_begin_pause * leftover dbg!() * refactor some validation * various pr comments * mut thing * remove unneeded allow(deprecated) * import some changes manually from parent branch to help a confused merge tool * more diff-reducing imports * confused merge tool * reduce diff * add validation for nonceresponse + signatureshareresponse * newline * pr comments * pr comments utxo * utxo comments * remove error conversion method * Squashed commit of the following: commit 2168f58 Author: Cyle Witruk <[email protected]> Date: Thu Feb 6 16:08:22 2025 +0100 feat: consensus on successful DKG prior to rotate-keys submission (#1285) * update for wsts 11 * use wsts 11.0.0 from crates * fix rebase * use wsts-12.0.0 from crates * add FrostCoordinator to wsts_state_machine so we can run signing rounds where all signers must participate * wip * run a dkg signing before deploying contracts * new wsts commit * wip * seems to work * working version * remove dead code * minor wsts logging tweaks * rename wstsmessageid variant * it works, now need to clean up * downgrade wsts * proto backwards compatability fixes * some wsts cleanup/refactor * think that's it * add message support for a specific dkg wstsmessageid * add a dkg wstsmessageid variant * use block hash instead of random data * fmt * remove 100% requirement for stacks signing of rotate keys * bump p256k1 to 7.2.2 * fix merge artifacts * tracing fields * storage mut * lovely cascading changes * remove wstsmessage.txid infavor of id * remove save and prefer trait default impls * remove unused trait methods * logging stuff * rename message ids * use tracing constants * remove stale comment * remove box from some types * missed dkg_begin_pause * leftover dbg!() * refactor some validation * various pr comments * mut thing * remove unneeded allow(deprecated) * confused merge tool --------- Co-authored-by: Joey Yandle <[email protected]> * Change the DKG shares status type. Lots of small follow-up modifications * Update the migration query * Fix up remaining queries * Oops forgot this one * Forgot this rename * Rename the new status field to match the new column * Fix up the tests and add a new one * Change the return value of some of the queries and simplify the validation check. Also add a test. * Change it back * Update the tests * Clean up the comments in the shares enum * rename the rotate keys error variant * Use a better error variant when extracting the started_at from the state machine Id * Remove some of our unused error variants * Match the behavior in the in memory store with the postgres implementation * We do not need these errors anymore either * address nits --------- Co-authored-by: Joey Yandle <[email protected]> Co-authored-by: djordon <[email protected]> Co-authored-by: Francesco Leacche <[email protected]>

…fication (#1324) * update for wsts 11 * use wsts 11.0.0 from crates * fix rebase * use wsts-12.0.0 from crates * add FrostCoordinator to wsts_state_machine so we can run signing rounds where all signers must participate * wip * run a dkg signing before deploying contracts * new wsts commit * wip * seems to work * working version * remove dead code * minor wsts logging tweaks * rename wstsmessageid variant * it works, now need to clean up * downgrade wsts * proto backwards compatability fixes * some wsts cleanup/refactor * think that's it * add message support for a specific dkg wstsmessageid * add a dkg wstsmessageid variant * use block hash instead of random data * fmt * remove 100% requirement for stacks signing of rotate keys * bump p256k1 to 7.2.2 * pop stash * wip * working verifications * saving * fixing tests * seems to work * remove const generic, use 0 amount and add test for random keypair * vet bitcoinconsensus * working on cleaning up * fix merge artifacts * tracing fields * migration needs to update block hash/height * storage mut * lovely cascading changes * remove wstsmessage.txid infavor of id * remove save and prefer trait default impls * remove unused trait methods * logging stuff * rename message ids * use tracing constants * remove stale comment * remove box from some types * missed dkg_begin_pause * leftover dbg!() * refactor some validation * various pr comments * mut thing * remove unneeded allow(deprecated) * import some changes manually from parent branch to help a confused merge tool * more diff-reducing imports * confused merge tool * reduce diff * add validation for nonceresponse + signatureshareresponse * newline * pr comments * pr comments utxo * utxo comments * remove error conversion method * Squashed commit of the following: commit 2168f58 Author: Cyle Witruk <[email protected]> Date: Thu Feb 6 16:08:22 2025 +0100 feat: consensus on successful DKG prior to rotate-keys submission (#1285) * update for wsts 11 * use wsts 11.0.0 from crates * fix rebase * use wsts-12.0.0 from crates * add FrostCoordinator to wsts_state_machine so we can run signing rounds where all signers must participate * wip * run a dkg signing before deploying contracts * new wsts commit * wip * seems to work * working version * remove dead code * minor wsts logging tweaks * rename wstsmessageid variant * it works, now need to clean up * downgrade wsts * proto backwards compatability fixes * some wsts cleanup/refactor * think that's it * add message support for a specific dkg wstsmessageid * add a dkg wstsmessageid variant * use block hash instead of random data * fmt * remove 100% requirement for stacks signing of rotate keys * bump p256k1 to 7.2.2 * fix merge artifacts * tracing fields * storage mut * lovely cascading changes * remove wstsmessage.txid infavor of id * remove save and prefer trait default impls * remove unused trait methods * logging stuff * rename message ids * use tracing constants * remove stale comment * remove box from some types * missed dkg_begin_pause * leftover dbg!() * refactor some validation * various pr comments * mut thing * remove unneeded allow(deprecated) * confused merge tool --------- Co-authored-by: Joey Yandle <[email protected]> * Change the DKG shares status type. Lots of small follow-up modifications * Update the migration query * Fix up remaining queries * Oops forgot this one * Forgot this rename * Rename the new status field to match the new column * Fix up the tests and add a new one * Change the return value of some of the queries and simplify the validation check. Also add a test. * Change it back * Update the tests * Clean up the comments in the shares enum * rename the rotate keys error variant * Use a better error variant when extracting the started_at from the state machine Id * Remove some of our unused error variants * Match the behavior in the in memory store with the postgres implementation * We do not need these errors anymore either * wip * seems to be a working version * integrated into signer * fmt * remove unused errors * another unused error * cleanup * don't cache the mock transaction, for now * change cache structure * address nits * comments * comments and dead code * comment * missed some stuff * comment * comments * comment * pr comments * test stuff * attribute and move mark_processed * rename unwrap_one to single * remove error generic * remove unused error variant * rename display variant * fix mod comment * comments * use bool to simplify processing loop * some refactoring and fix issue with multiple messages * more validation, test improvements and a little cleanup * move validate_sender in relay_message * import mock tx verify/revoke match statement from main * re-add block hash (well, block ref) * add break-early-on-coordinator-state-change * allow unlimited timeout (a'la None) * some final cleanup * pr comments * remove testing.rs comment leftovers * remove unused error variant * remove unneeded sleep * log state on error/expired * statemachineid display impl --------- Co-authored-by: Joey Yandle <[email protected]> Co-authored-by: djordon <[email protected]> Co-authored-by: Francesco Leacche <[email protected]>

xoloki and others added 21 commits January 23, 2025 21:10

update for wsts 11

9306dfa

use wsts 11.0.0 from crates

c986d2a

fix rebase

25ac3be

use wsts-12.0.0 from crates

cf13953

add FrostCoordinator to wsts_state_machine so we can run signing roun…

6cb670a

…ds where all signers must participate

wip

31726ee

Merge branch 'feat/frost-coordinator' into feat/require-all-signature…

63731bb

…s-for-rotate-keys

run a dkg signing before deploying contracts

105d9a7

new wsts commit

4d406ee

wip

1218eb6

Merge branch 'main' into feat/require-all-signatures-for-rotate-keys

4179cd6

seems to work

be05f5e

working version

43501cb

Merge branch 'main' into feat/require-all-signatures-for-rotate-keys

a0ce153

remove dead code

a75f5a5

minor wsts logging tweaks

2214427

Merge branch 'main' into feat/require-all-signatures-for-rotate-keys

1511969

rename wstsmessageid variant

25d214a

it works, now need to clean up

18b69d2

downgrade wsts

7412f87

proto backwards compatability fixes

5a16608

cylewitruk added the key rotation The functionality to rotate a private key for a signer in sBTC-v1. label Jan 30, 2025

cylewitruk added this to the sBTC: Deposits milestone Jan 30, 2025

cylewitruk requested review from xoloki, matteojug and djordon January 30, 2025 02:37

cylewitruk self-assigned this Jan 30, 2025

cylewitruk requested a review from aldur January 30, 2025 02:37

cylewitruk added 2 commits January 30, 2025 03:42

Merge branch 'main' into feat/require-all-signatures-for-rotate-keys

0e624e6

some wsts cleanup/refactor

e867b72

matteojug reviewed Feb 4, 2025

View reviewed changes

xoloki approved these changes Feb 4, 2025

View reviewed changes

signer/src/transaction_coordinator.rs Show resolved Hide resolved

signer/src/transaction_signer.rs Outdated Show resolved Hide resolved

signer/src/transaction_signer.rs Outdated Show resolved Hide resolved

signer/src/transaction_signer.rs Show resolved Hide resolved

cylewitruk added 17 commits February 5, 2025 11:50

Merge branch 'main' into refactor/wsts-cleanup

f3587e0

remove wstsmessage.txid infavor of id

d22ed61

remove save and prefer trait default impls

76b9b49

remove unused trait methods

cb95417

logging stuff

a38d1c9

rename message ids

5f99a29

Merge branch 'refactor/wsts-cleanup' into feat/require-all-signatures…

b31f6f7

…-for-rotate-keys

use tracing constants

92dbf17

remove stale comment

51a39d3

remove box from some types

2fe82e6

missed dkg_begin_pause

6598b11

leftover dbg!()

576a004

refactor some validation

5c0304b

various pr comments

5d2f807

mut thing

1e74dbf

remove unneeded allow(deprecated)

68bf8f5

Merge branch 'refactor/wsts-cleanup' into feat/require-all-signatures…

d30a0c7

…-for-rotate-keys

cylewitruk changed the base branch from refactor/wsts-cleanup to main February 5, 2025 21:36

Merge branch 'main' into feat/require-all-signatures-for-rotate-keys

f350a7f

matteojug approved these changes Feb 5, 2025

View reviewed changes

signer/src/transaction_signer.rs Show resolved Hide resolved

signer/src/transaction_signer.rs Show resolved Hide resolved

signer/src/transaction_signer.rs Show resolved Hide resolved

confused merge tool

dea3625

djordon mentioned this pull request Feb 6, 2025

feat: add function for using smart contract for deposit status #1306

Open

1 task

djordon approved these changes Feb 6, 2025

View reviewed changes

cylewitruk merged commit 2168f58 into main Feb 6, 2025
8 checks passed

cylewitruk mentioned this pull request Feb 7, 2025

feat: state machine for out-of-order message handling during dkg verification #1324

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: consensus on successful DKG prior to rotate-keys submission #1285

feat: consensus on successful DKG prior to rotate-keys submission #1285

cylewitruk commented Jan 30, 2025 •

edited

Loading

xoloki left a comment

matteojug left a comment •

edited

Loading

feat: consensus on successful DKG prior to rotate-keys submission #1285

feat: consensus on successful DKG prior to rotate-keys submission #1285

Conversation

cylewitruk commented Jan 30, 2025 • edited Loading

Description

Changes

Testing Information

Checklist:

xoloki left a comment

Choose a reason for hiding this comment

matteojug left a comment • edited Loading

Choose a reason for hiding this comment

cylewitruk commented Jan 30, 2025 •

edited

Loading

matteojug left a comment •

edited

Loading