From c1282c974d50c30fc7009d1a486bea46536c31b5 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 9 Jan 2023 13:29:08 +0100 Subject: [PATCH 1/8] Confirm DKG start before triggering off-chain protocol So far, the tECDSA DKG was triggered using a simple event listener acting on `DKGStarted` events. However, small chain reorgs may cause different `startBlock` to be received by DKG participants. That can lead to invalid signatures over the produced DKG result as the `startBlock` is one of the signature's components. Here we improve that by adding a confirmation mechanism that delays the start of the off-chain protocol until the event and DKG on-chain state is confirmed. --- pkg/chain/ethereum/tbtc.go | 37 +++++++++++++++++++++++++ pkg/tbtc/chain.go | 15 +++++++++++ pkg/tbtc/chain_test.go | 6 +++++ pkg/tbtc/dkg.go | 4 +++ pkg/tbtc/tbtc.go | 55 ++++++++++++++++++++++++++++++++++---- 5 files changed, 112 insertions(+), 5 deletions(-) diff --git a/pkg/chain/ethereum/tbtc.go b/pkg/chain/ethereum/tbtc.go index 9f99230656..2e8ed655da 100644 --- a/pkg/chain/ethereum/tbtc.go +++ b/pkg/chain/ethereum/tbtc.go @@ -373,6 +373,43 @@ func (tc *TbtcChain) OnDKGStarted( return tc.walletRegistry.DkgStartedEvent(nil, nil).OnEvent(onEvent) } +func (tc *TbtcChain) PastDKGStartedEvents( + filter *tbtc.DKGStartedEventFilter, +) ([]*tbtc.DKGStartedEvent, error) { + var startBlock uint64 + var endBlock *uint64 + var seed []*big.Int + + if filter != nil { + startBlock = filter.StartBlock + endBlock = filter.EndBlock + seed = filter.Seed + } + + events, err := tc.walletRegistry.PastDkgStartedEvents( + startBlock, + endBlock, + seed, + ) + if err != nil { + return nil, err + } + + dkgStartedEvents := make([]*tbtc.DKGStartedEvent, len(events)) + for i, event := range events { + dkgStartedEvents[i] = &tbtc.DKGStartedEvent{ + Seed: event.Seed, + BlockNumber: event.Raw.BlockNumber, + } + } + + sort.SliceStable(dkgStartedEvents, func(i, j int) bool { + return dkgStartedEvents[i].BlockNumber < dkgStartedEvents[j].BlockNumber + }) + + return dkgStartedEvents, err +} + func (tc *TbtcChain) OnDKGResultSubmitted( handler func(event *tbtc.DKGResultSubmittedEvent), ) subscription.EventSubscription { diff --git a/pkg/tbtc/chain.go b/pkg/tbtc/chain.go index 2cebe013c9..7a870bbbeb 100644 --- a/pkg/tbtc/chain.go +++ b/pkg/tbtc/chain.go @@ -49,6 +49,14 @@ type DistributedKeyGenerationChain interface { func(event *DKGStartedEvent), ) subscription.EventSubscription + // PastDKGStartedEvents fetches past DKG started events according to the + // provided filter or unfiltered if the filter is nil. Returned events + // are sorted by the block number in the ascending order, i.e. the latest + // event is at the end of the slice. + PastDKGStartedEvents( + filter *DKGStartedEventFilter, + ) ([]*DKGStartedEvent, error) + // OnDKGResultSubmitted registers a callback that is invoked when an on-chain // notification of the DKG result submission is seen. OnDKGResultSubmitted( @@ -131,6 +139,13 @@ type DKGStartedEvent struct { BlockNumber uint64 } +// DKGStartedEventFilter is a component allowing to filter DKGStartedEvent. +type DKGStartedEventFilter struct { + StartBlock uint64 + EndBlock *uint64 + Seed []*big.Int +} + // DKGResultSubmittedEvent represents a DKG result submission event. It is // emitted after a submitted DKG result lands on the chain. type DKGResultSubmittedEvent struct { diff --git a/pkg/tbtc/chain_test.go b/pkg/tbtc/chain_test.go index 60fbc3799d..891cf0368b 100644 --- a/pkg/tbtc/chain_test.go +++ b/pkg/tbtc/chain_test.go @@ -131,6 +131,12 @@ func (lc *localChain) OnDKGStarted( panic("unsupported") } +func (lc *localChain) PastDKGStartedEvents( + filter *DKGStartedEventFilter, +) ([]*DKGStartedEvent, error) { + panic("unsupported") +} + func (lc *localChain) OnDKGResultSubmitted( handler func(event *DKGResultSubmittedEvent), ) subscription.EventSubscription { diff --git a/pkg/tbtc/dkg.go b/pkg/tbtc/dkg.go index e1e066cd40..ffb3103ae2 100644 --- a/pkg/tbtc/dkg.go +++ b/pkg/tbtc/dkg.go @@ -20,6 +20,10 @@ import ( ) const ( + // dkgStartedConfirmationBlocks determines the block length of the + // confirmation period that is preserved after a DKG start. Once the period + // elapses, the DKG state is checked to confirm the protocol can be started. + dkgStartedConfirmationBlocks = 20 // dkgResultSubmissionDelayStep determines the delay step in blocks that // is used to calculate the submission delay period that should be respected // by the given member to avoid all members submitting the same DKG result diff --git a/pkg/tbtc/tbtc.go b/pkg/tbtc/tbtc.go index 324d94af54..2b36661e77 100644 --- a/pkg/tbtc/tbtc.go +++ b/pkg/tbtc/tbtc.go @@ -146,16 +146,61 @@ func Initialize( return } + confirmationBlock := event.BlockNumber + dkgStartedConfirmationBlocks + logger.Infof( - "DKG started with seed [0x%x] at block [%v]", + "observed DKG started event with seed [0x%x] and "+ + "starting block [%v]; waiting for block [%v] to confirm", event.Seed, event.BlockNumber, + confirmationBlock, ) - node.joinDKGIfEligible( - event.Seed, - event.BlockNumber, - ) + err := node.waitForBlockHeight(ctx, confirmationBlock) + if err != nil { + logger.Errorf("failed to confirm DKG started event: [%v]", err) + return + } + + dkgState, err := chain.GetDKGState() + if err != nil { + logger.Errorf("failed to check DKG state: [%v]", err) + return + } + + if dkgState == AwaitingResult { + // Fetch all past DKG started events starting from one + // confirmation period before the original event's block. + pastEvents, err := chain.PastDKGStartedEvents( + &DKGStartedEventFilter{ + StartBlock: event.BlockNumber - dkgStartedConfirmationBlocks, + }, + ) + if err != nil { + logger.Errorf("failed to get past DKG started events: [%v]", err) + return + } + + lastEvent := pastEvents[len(pastEvents)-1] + + logger.Infof( + "DKG started with seed [0x%x] at block [%v]", + lastEvent.Seed, + lastEvent.BlockNumber, + ) + + node.joinDKGIfEligible( + lastEvent.Seed, + lastEvent.BlockNumber, + ) + } else { + logger.Infof( + "DKG started event with seed [0x%x] and starting "+ + "block [%v] was not confirmed", + event.Seed, + event.BlockNumber, + ) + } }() }) From 1f6484614c1cafdd08392cbb81cc5f9c0ea03b73 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Tue, 10 Jan 2023 08:34:16 +0100 Subject: [PATCH 2/8] Add a delay to the DKG execution The off-chain protocol should be started as close as possible to the current block or even further. Starting the off-chain protocol with a past block will likely cause a failure of the first attempt as the start block is used to synchronize the announcements and the state machine. Here we ensure a proper start point by delaying the execution by the confirmation period length. --- pkg/tbtc/dkg.go | 11 ++++++++--- pkg/tbtc/node.go | 11 ++++++++--- pkg/tbtc/tbtc.go | 8 ++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/pkg/tbtc/dkg.go b/pkg/tbtc/dkg.go index ffb3103ae2..40c4241187 100644 --- a/pkg/tbtc/dkg.go +++ b/pkg/tbtc/dkg.go @@ -108,10 +108,12 @@ func (de *dkgExecutor) preParamsCount() int { // executeDkgIfEligible is the main function of dkgExecutor. It performs the // full execution of ECDSA Distributed Key Generation: determining members // selected to the signing group, executing off-chain protocol, and publishing -// the result to the chain. +// the result to the chain. The execution can be delayed by an arbitrary number +// of blocks using the delayBlocks argument. func (de *dkgExecutor) executeDkgIfEligible( seed *big.Int, startBlock uint64, + delayBlocks uint64, ) { dkgLogger := logger.With( zap.String("seed", fmt.Sprintf("0x%x", seed)), @@ -149,6 +151,7 @@ func (de *dkgExecutor) executeDkgIfEligible( memberIndexes, groupSelectionResult, startBlock, + delayBlocks, ) } else { dkgLogger.Infof("not eligible for DKG") @@ -228,13 +231,15 @@ func (de *dkgExecutor) setupBroadcastChannel( // generateSigningGroup executes off-chain protocol for each member controlled // by the current operator and upon successful execution of the protocol -// publishes the result to the chain. +// publishes the result to the chain. The execution can be delayed by an +// arbitrary number of blocks using the delayBlocks argument. func (de *dkgExecutor) generateSigningGroup( dkgLogger *zap.SugaredLogger, seed *big.Int, memberIndexes []uint8, groupSelectionResult *GroupSelectionResult, startBlock uint64, + delayBlocks uint64, ) { membershipValidator := group.NewMembershipValidator( dkgLogger, @@ -300,7 +305,7 @@ func (de *dkgExecutor) generateSigningGroup( retryLoop := newDkgRetryLoop( dkgLogger, seed, - startBlock, + startBlock+delayBlocks, memberIndex, groupSelectionResult.OperatorsAddresses, de.groupParameters, diff --git a/pkg/tbtc/node.go b/pkg/tbtc/node.go index 4338c5970f..5d0fc372b0 100644 --- a/pkg/tbtc/node.go +++ b/pkg/tbtc/node.go @@ -143,9 +143,14 @@ func (n *node) operatorID() (chain.OperatorID, error) { // distributed key generation if this node's operator proves to be eligible for // the group generated by that seed. This is an interactive on-chain process, // and joinDKGIfEligible can block for an extended period of time while it -// completes the on-chain operation. -func (n *node) joinDKGIfEligible(seed *big.Int, startBlock uint64) { - n.dkgExecutor.executeDkgIfEligible(seed, startBlock) +// completes the on-chain operation. The execution can be delayed by an +// arbitrary number of blocks using the delayBlocks argument. +func (n *node) joinDKGIfEligible( + seed *big.Int, + startBlock uint64, + delayBlocks uint64, +) { + n.dkgExecutor.executeDkgIfEligible(seed, startBlock, delayBlocks) } // validateDKG performs the submitted DKG result validation process. diff --git a/pkg/tbtc/tbtc.go b/pkg/tbtc/tbtc.go index 2b36661e77..9a3f73ccd6 100644 --- a/pkg/tbtc/tbtc.go +++ b/pkg/tbtc/tbtc.go @@ -189,9 +189,17 @@ func Initialize( lastEvent.BlockNumber, ) + // The off-chain protocol should be started as close as possible + // to the current block or even further. Starting the off-chain + // protocol with a past block will likely cause a failure of the + // first attempt as the start block is used to synchronize + // the announcements and the state machine. Here we ensure + // a proper start point by delaying the execution by the + // confirmation period length. node.joinDKGIfEligible( lastEvent.Seed, lastEvent.BlockNumber, + dkgStartedConfirmationBlocks, ) } else { logger.Infof( From f2c9ff81685d629a0357e462527111bddc644950 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 12 Jan 2023 12:23:40 +0100 Subject: [PATCH 3/8] Improve docs around the `delayBlocks` argument --- pkg/tbtc/dkg.go | 8 ++++++-- pkg/tbtc/node.go | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/tbtc/dkg.go b/pkg/tbtc/dkg.go index 40c4241187..0f080e5432 100644 --- a/pkg/tbtc/dkg.go +++ b/pkg/tbtc/dkg.go @@ -109,7 +109,9 @@ func (de *dkgExecutor) preParamsCount() int { // full execution of ECDSA Distributed Key Generation: determining members // selected to the signing group, executing off-chain protocol, and publishing // the result to the chain. The execution can be delayed by an arbitrary number -// of blocks using the delayBlocks argument. +// of blocks using the delayBlocks argument. This allows confirming the state +// on-chain - e.g. wait for the required number of confirming blocks - before +//executing the off-chain action. func (de *dkgExecutor) executeDkgIfEligible( seed *big.Int, startBlock uint64, @@ -232,7 +234,9 @@ func (de *dkgExecutor) setupBroadcastChannel( // generateSigningGroup executes off-chain protocol for each member controlled // by the current operator and upon successful execution of the protocol // publishes the result to the chain. The execution can be delayed by an -// arbitrary number of blocks using the delayBlocks argument. +// arbitrary number of blocks using the delayBlocks argument. This allows +// confirming the state on-chain - e.g. wait for the required number of +// confirming blocks - before executing the off-chain action. func (de *dkgExecutor) generateSigningGroup( dkgLogger *zap.SugaredLogger, seed *big.Int, diff --git a/pkg/tbtc/node.go b/pkg/tbtc/node.go index 5d0fc372b0..5a34abc182 100644 --- a/pkg/tbtc/node.go +++ b/pkg/tbtc/node.go @@ -144,7 +144,9 @@ func (n *node) operatorID() (chain.OperatorID, error) { // the group generated by that seed. This is an interactive on-chain process, // and joinDKGIfEligible can block for an extended period of time while it // completes the on-chain operation. The execution can be delayed by an -// arbitrary number of blocks using the delayBlocks argument. +// arbitrary number of blocks using the delayBlocks argument. This allows +// confirming the state on-chain - e.g. wait for the required number of +// confirming blocks - before executing the off-chain action. func (n *node) joinDKGIfEligible( seed *big.Int, startBlock uint64, From 02888efc39ce89053e6c57c9406c17dd2b2bcfc3 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 12 Jan 2023 12:28:00 +0100 Subject: [PATCH 4/8] Improve log about duplicated `DKGStarted` event --- pkg/tbtc/tbtc.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/tbtc/tbtc.go b/pkg/tbtc/tbtc.go index 9a3f73ccd6..1212f35eef 100644 --- a/pkg/tbtc/tbtc.go +++ b/pkg/tbtc/tbtc.go @@ -137,11 +137,10 @@ func Initialize( if ok := deduplicator.notifyDKGStarted( event.Seed, ); !ok { - logger.Warnf( - "DKG started event with seed [0x%x] and "+ - "starting block [%v] has been already processed", + logger.Infof( + "DKG started event with seed [0x%x] has been " + + "already processed", event.Seed, - event.BlockNumber, ) return } From 90955c8073971f73b484f483137be9056772ee36 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 12 Jan 2023 12:30:17 +0100 Subject: [PATCH 5/8] Add a check for empty past DKG started events --- pkg/tbtc/tbtc.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/tbtc/tbtc.go b/pkg/tbtc/tbtc.go index 1212f35eef..a849340a9f 100644 --- a/pkg/tbtc/tbtc.go +++ b/pkg/tbtc/tbtc.go @@ -180,6 +180,12 @@ func Initialize( return } + // Should not happen but just in case. + if len(pastEvents) == 0 { + logger.Errorf("no past DKG started events") + return + } + lastEvent := pastEvents[len(pastEvents)-1] logger.Infof( From e89b0059b917f9155d7fd55f9608034fda0ce1cd Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 12 Jan 2023 12:39:25 +0100 Subject: [PATCH 6/8] Clarify the block range of fetched past DKG started events --- pkg/tbtc/tbtc.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/tbtc/tbtc.go b/pkg/tbtc/tbtc.go index a849340a9f..2e293b8932 100644 --- a/pkg/tbtc/tbtc.go +++ b/pkg/tbtc/tbtc.go @@ -170,6 +170,9 @@ func Initialize( if dkgState == AwaitingResult { // Fetch all past DKG started events starting from one // confirmation period before the original event's block. + // If there was a chain reorg, the event we received could be + // moved to a block with a lower number than the one + // we received. pastEvents, err := chain.PastDKGStartedEvents( &DKGStartedEventFilter{ StartBlock: event.BlockNumber - dkgStartedConfirmationBlocks, From c8e4270b630288b28c33c8eb89a2611fb6bb2a75 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 12 Jan 2023 12:44:47 +0100 Subject: [PATCH 7/8] Make formatter happy --- pkg/tbtc/tbtc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tbtc/tbtc.go b/pkg/tbtc/tbtc.go index 2e293b8932..bb326feec3 100644 --- a/pkg/tbtc/tbtc.go +++ b/pkg/tbtc/tbtc.go @@ -138,7 +138,7 @@ func Initialize( event.Seed, ); !ok { logger.Infof( - "DKG started event with seed [0x%x] has been " + + "DKG started event with seed [0x%x] has been "+ "already processed", event.Seed, ) From 890f7e4c1871792d37ef3115bd6558cdcf15bbe7 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 12 Jan 2023 12:50:31 +0100 Subject: [PATCH 8/8] Improve docs for `generateSigningGroup` --- pkg/tbtc/dkg.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/tbtc/dkg.go b/pkg/tbtc/dkg.go index 0f080e5432..3c33876776 100644 --- a/pkg/tbtc/dkg.go +++ b/pkg/tbtc/dkg.go @@ -236,7 +236,9 @@ func (de *dkgExecutor) setupBroadcastChannel( // publishes the result to the chain. The execution can be delayed by an // arbitrary number of blocks using the delayBlocks argument. This allows // confirming the state on-chain - e.g. wait for the required number of -// confirming blocks - before executing the off-chain action. +// confirming blocks - before executing the off-chain action. Note that the +// startBlock represents the block at which DKG started on-chain. This is +// important for the result submission. func (de *dkgExecutor) generateSigningGroup( dkgLogger *zap.SugaredLogger, seed *big.Int,