From 5abaa9218f3fd9f33c973d4413217b4dbe5b8abe Mon Sep 17 00:00:00 2001 From: maclane Date: Sat, 11 Jul 2026 08:17:38 -0400 Subject: [PATCH] Fix FROST DKG parameters and coordinator retries --- .../ethereum/ecdsa_dkg_validator_chain.go | 22 +- .../ethereum/frost_group_parameters_test.go | 29 + pkg/chain/ethereum/tbtc.go | 40 +- pkg/tbtc/deduplicator.go | 122 ++++- pkg/tbtc/deduplicator_test.go | 25 + pkg/tbtc/frost_dkg_coordinator.go | 96 +++- pkg/tbtc/frost_dkg_coordinator_test.go | 497 ++++++++++++++++++ pkg/tbtc/frost_dkg_execution_default.go | 8 +- pkg/tbtc/frost_dkg_execution_default_test.go | 26 + pkg/tbtc/frost_dkg_execution_frost_native.go | 34 +- pkg/tbtc/frost_dkg_result_signing.go | 4 +- pkg/tbtc/node.go | 51 +- pkg/tbtc/node_test.go | 58 ++ pkg/tbtc/tbtc.go | 40 ++ 14 files changed, 990 insertions(+), 62 deletions(-) create mode 100644 pkg/chain/ethereum/frost_group_parameters_test.go create mode 100644 pkg/tbtc/frost_dkg_execution_default_test.go diff --git a/pkg/chain/ethereum/ecdsa_dkg_validator_chain.go b/pkg/chain/ethereum/ecdsa_dkg_validator_chain.go index 5835ba9df7..9f184cd639 100644 --- a/pkg/chain/ethereum/ecdsa_dkg_validator_chain.go +++ b/pkg/chain/ethereum/ecdsa_dkg_validator_chain.go @@ -102,17 +102,33 @@ func ecdsaWalletGroupParametersFromValidator( return nil, err } - groupSize, err := bigIntPositiveIntLimited("groupSize", gsBig, maxReasonableTBTCGroupSize) + return walletGroupParametersFromValidatorValues(gsBig, atBig, gtBig) +} + +func walletGroupParametersFromValidatorValues( + groupSizeValue *big.Int, + activeThresholdValue *big.Int, + groupThresholdValue *big.Int, +) (*tbtc.GroupParameters, error) { + groupSize, err := bigIntPositiveIntLimited( + "groupSize", + groupSizeValue, + maxReasonableTBTCGroupSize, + ) if err != nil { return nil, err } - groupQuorum, err := bigIntPositiveIntLimited("activeThreshold", atBig, maxReasonableTBTCGroupSize) + groupQuorum, err := bigIntPositiveIntLimited( + "activeThreshold", + activeThresholdValue, + maxReasonableTBTCGroupSize, + ) if err != nil { return nil, err } honestThreshold, err := bigIntPositiveIntLimited( "groupThreshold", - gtBig, + groupThresholdValue, maxReasonableTBTCGroupSize, ) if err != nil { diff --git a/pkg/chain/ethereum/frost_group_parameters_test.go b/pkg/chain/ethereum/frost_group_parameters_test.go new file mode 100644 index 0000000000..bbb561094a --- /dev/null +++ b/pkg/chain/ethereum/frost_group_parameters_test.go @@ -0,0 +1,29 @@ +package ethereum + +import ( + "math/big" + "testing" +) + +func TestWalletGroupParametersFromValidatorValuesMapsFrostConstants( + t *testing.T, +) { + parameters, err := walletGroupParametersFromValidatorValues( + big.NewInt(5), // FrostDkgValidator.groupSize() + big.NewInt(4), // FrostDkgValidator.activeThreshold() + big.NewInt(3), // FrostDkgValidator.groupThreshold() + ) + if err != nil { + t.Fatalf("unexpected error: [%v]", err) + } + + if parameters.GroupSize != 5 { + t.Fatalf("unexpected group size: [%d]", parameters.GroupSize) + } + if parameters.GroupQuorum != 4 { + t.Fatalf("unexpected group quorum: [%d]", parameters.GroupQuorum) + } + if parameters.HonestThreshold != 3 { + t.Fatalf("unexpected honest threshold: [%d]", parameters.HonestThreshold) + } +} diff --git a/pkg/chain/ethereum/tbtc.go b/pkg/chain/ethereum/tbtc.go index 5d1a42ce5f..7edb98871a 100644 --- a/pkg/chain/ethereum/tbtc.go +++ b/pkg/chain/ethereum/tbtc.go @@ -423,7 +423,8 @@ func connectFrostDkgValidator( if frostDkgValidatorAddress == (common.Address{}) { logger.Infof( "%s contract address not configured; pre-submit FROST digest "+ - "view checks disabled", + "view checks disabled (the address is required when the FROST "+ + "wallet registry is enabled)", FrostDkgValidatorContractName, ) return nil, nil @@ -460,6 +461,43 @@ func (tc *TbtcChain) EcdsaWalletGroupParametersFromChain( ) } +// FrostWalletGroupParametersFromChain mirrors FrostDkgValidator sizing +// constants. FROST and ECDSA validators are independent and may deliberately +// use different group sizes and thresholds, so callers must not reuse the +// legacy ECDSA parameters for FROST protocols. +func (tc *TbtcChain) FrostWalletGroupParametersFromChain( + ctx context.Context, +) (*tbtc.GroupParameters, error) { + if tc.frostWalletRegistry == nil { + return nil, nil + } + if tc.frostDkgValidator == nil { + return nil, fmt.Errorf( + "FrostDkgValidator is required when FrostWalletRegistry is configured", + ) + } + + callOpts := &bind.CallOpts{Context: ctx, From: tc.key.Address} + groupSize, err := tc.frostDkgValidator.GroupSize(callOpts) + if err != nil { + return nil, fmt.Errorf("read FrostDkgValidator groupSize: %w", err) + } + activeThreshold, err := tc.frostDkgValidator.ActiveThreshold(callOpts) + if err != nil { + return nil, fmt.Errorf("read FrostDkgValidator activeThreshold: %w", err) + } + groupThreshold, err := tc.frostDkgValidator.GroupThreshold(callOpts) + if err != nil { + return nil, fmt.Errorf("read FrostDkgValidator groupThreshold: %w", err) + } + + return walletGroupParametersFromValidatorValues( + groupSize, + activeThreshold, + groupThreshold, + ) +} + // Staking returns address of the TokenStaking contract the WalletRegistry is // connected to. func (tc *TbtcChain) Staking() (chain.Address, error) { diff --git a/pkg/tbtc/deduplicator.go b/pkg/tbtc/deduplicator.go index 37d0b1704f..14c7f45d0d 100644 --- a/pkg/tbtc/deduplicator.go +++ b/pkg/tbtc/deduplicator.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "math/big" "strconv" + "sync" "time" "github.com/keep-network/keep-common/pkg/cache" @@ -39,6 +40,9 @@ type deduplicator struct { dkgSeedCache *cache.TimeCache dkgResultHashCache *cache.TimeCache walletClosedCache *cache.TimeCache + + mutex sync.Mutex + inProgress map[string]struct{} } func newDeduplicator() *deduplicator { @@ -46,6 +50,85 @@ func newDeduplicator() *deduplicator { dkgSeedCache: cache.NewTimeCache(DKGSeedCachePeriod), dkgResultHashCache: cache.NewTimeCache(DKGResultHashCachePeriod), walletClosedCache: cache.NewTimeCache(WalletClosedCachePeriod), + inProgress: make(map[string]struct{}), + } +} + +// deduplicationLease suppresses concurrent handling of an event without +// marking it permanently handled up front. Completing the lease moves the key +// to the long-lived cache; releasing it after an error lets a subscription or +// recovery replay retry the work. +type deduplicationLease struct { + owner *deduplicator + cache *cache.TimeCache + cacheKey string + inProgressKey string +} + +func (d *deduplicator) beginDKGStarted( + seed *big.Int, +) (*deduplicationLease, bool) { + cacheKey := seed.Text(16) + return d.begin( + d.dkgSeedCache, + cacheKey, + "dkg-started:"+cacheKey, + ) +} + +func (d *deduplicator) beginDKGResultSubmitted( + seed *big.Int, + resultHash DKGChainResultHash, + block uint64, +) (*deduplicationLease, bool) { + cacheKey := dkgResultSubmittedCacheKey(seed, resultHash, block) + return d.begin( + d.dkgResultHashCache, + cacheKey, + "dkg-result-submitted:"+cacheKey, + ) +} + +func (d *deduplicator) begin( + completedCache *cache.TimeCache, + cacheKey string, + inProgressKey string, +) (*deduplicationLease, bool) { + completedCache.Sweep() + + d.mutex.Lock() + defer d.mutex.Unlock() + + if completedCache.Has(cacheKey) { + return nil, false + } + if _, ok := d.inProgress[inProgressKey]; ok { + return nil, false + } + if d.inProgress == nil { + d.inProgress = make(map[string]struct{}) + } + + d.inProgress[inProgressKey] = struct{}{} + return &deduplicationLease{ + owner: d, + cache: completedCache, + cacheKey: cacheKey, + inProgressKey: inProgressKey, + }, true +} + +func (dl *deduplicationLease) finish(completed bool) { + dl.owner.mutex.Lock() + defer dl.owner.mutex.Unlock() + + if _, ok := dl.owner.inProgress[dl.inProgressKey]; !ok { + return + } + + delete(dl.owner.inProgress, dl.inProgressKey) + if completed { + dl.cache.Add(dl.cacheKey) } } @@ -59,16 +142,9 @@ func (d *deduplicator) notifyDKGStarted( // The cache key is the hexadecimal representation of the seed. cacheKey := newDKGSeed.Text(16) - // If the key is not in the cache, that means the seed was not handled - // yet and the client should proceed with the execution. - if !d.dkgSeedCache.Has(cacheKey) { - d.dkgSeedCache.Add(cacheKey) - return true - } - // Otherwise, the DKG seed is a duplicate and the client should not proceed - // with the execution. - return false + // Add performs the presence check and insertion atomically. + return d.dkgSeedCache.Add(cacheKey) } // notifyDKGResultSubmitted notifies the client wants to start some actions @@ -81,20 +157,24 @@ func (d *deduplicator) notifyDKGResultSubmitted( ) bool { d.dkgResultHashCache.Sweep() - cacheKey := newDKGResultSeed.Text(16) + - hex.EncodeToString(newDKGResultHash[:]) + - strconv.Itoa(int(newDKGResultBlock)) + cacheKey := dkgResultSubmittedCacheKey( + newDKGResultSeed, + newDKGResultHash, + newDKGResultBlock, + ) - // If the key is not in the cache, that means the result was not handled - // yet and the client should proceed with the execution. - if !d.dkgResultHashCache.Has(cacheKey) { - d.dkgResultHashCache.Add(cacheKey) - return true - } + // Add performs the presence check and insertion atomically. + return d.dkgResultHashCache.Add(cacheKey) +} - // Otherwise, the DKG result is a duplicate and the client should not - // proceed with the execution. - return false +func dkgResultSubmittedCacheKey( + seed *big.Int, + resultHash DKGChainResultHash, + block uint64, +) string { + return seed.Text(16) + + hex.EncodeToString(resultHash[:]) + + strconv.FormatUint(block, 10) } func (d *deduplicator) notifyWalletClosed( diff --git a/pkg/tbtc/deduplicator_test.go b/pkg/tbtc/deduplicator_test.go index b75432a8c0..365acf6f2c 100644 --- a/pkg/tbtc/deduplicator_test.go +++ b/pkg/tbtc/deduplicator_test.go @@ -51,6 +51,31 @@ func TestNotifyDKGStarted(t *testing.T) { } } +func TestDKGStartedDeduplicationLease(t *testing.T) { + deduplicator := newDeduplicator() + seed := big.NewInt(100) + + lease, ok := deduplicator.beginDKGStarted(seed) + if !ok { + t.Fatal("expected first handler to acquire the event") + } + + if _, ok := deduplicator.beginDKGStarted(seed); ok { + t.Fatal("expected in-progress duplicate to be suppressed") + } + + lease.finish(false) + lease, ok = deduplicator.beginDKGStarted(seed) + if !ok { + t.Fatal("expected released event to be retryable") + } + + lease.finish(true) + if _, ok := deduplicator.beginDKGStarted(seed); ok { + t.Fatal("expected completed event to be suppressed") + } +} + func TestNotifyDKGResultSubmitted(t *testing.T) { deduplicator := deduplicator{ dkgResultHashCache: cache.NewTimeCache(testDKGResultHashCachePeriod), diff --git a/pkg/tbtc/frost_dkg_coordinator.go b/pkg/tbtc/frost_dkg_coordinator.go index dfea4188b6..4e754d8724 100644 --- a/pkg/tbtc/frost_dkg_coordinator.go +++ b/pkg/tbtc/frost_dkg_coordinator.go @@ -62,13 +62,17 @@ func handleFrostDKGStarted( event *FrostDKGStartedEvent, waitForConfirmation bool, ) { - if ok := deduplicator.notifyDKGStarted(event.Seed); !ok { + lease, ok := deduplicator.beginDKGStarted(event.Seed) + if !ok { logger.Infof( - "FROST DKG started event with seed [0x%x] has already been processed", + "FROST DKG started event with seed [0x%x] is already completed or "+ + "being processed", event.Seed, ) return } + completed := false + defer func() { lease.finish(completed) }() if waitForConfirmation { confirmationBlock := event.BlockNumber + dkgStartedConfirmationBlocks @@ -84,6 +88,10 @@ func handleFrostDKGStarted( logger.Errorf("failed to confirm FROST DKG started event: [%v]", err) return } + if ctx.Err() != nil { + logger.Errorf("stopping FROST DKG started event handling: [%v]", ctx.Err()) + return + } } dkgState, err := frostChain.GetFrostDKGState() @@ -98,6 +106,7 @@ func handleFrostDKGStarted( event.Seed, event.BlockNumber, ) + completed = true return } @@ -121,6 +130,33 @@ func handleFrostDKGStarted( } lastEvent := pastEvents[len(pastEvents)-1] + if lastEvent.Seed.Cmp(event.Seed) != 0 { + logger.Infof( + "FROST DKG started event with seed [0x%x] was superseded by seed "+ + "[0x%x]; transferring handling to the latest event", + event.Seed, + lastEvent.Seed, + ) + + // Mark the triggering event as terminal before re-entering so stale + // subscription replays remain suppressed. The recursive handler must + // acquire a fresh lease for the resolved seed before it performs any + // membership or DKG work. If another handler already owns or completed + // that seed, beginDKGStarted suppresses this path; if it previously + // failed and released its lease, this path safely becomes the retry. + completed = true + lease.finish(completed) + handleFrostDKGStarted( + ctx, + node, + frostChain, + deduplicator, + lastEvent, + waitForConfirmation, + ) + return + } + memberIndexes, groupSelectionResult, err := localFrostMembership( node, frostChain, @@ -137,10 +173,11 @@ func handleFrostDKGStarted( lastEvent.Seed, lastEvent.BlockNumber, ) + completed = true return } - executeFrostDKGIfPossible( + completed = executeFrostDKGIfPossible( ctx, node, frostChain, @@ -227,20 +264,23 @@ func handleFrostDKGResultSubmitted( deduplicator *deduplicator, event *FrostDKGResultSubmittedEvent, ) { - if ok := deduplicator.notifyDKGResultSubmitted( + lease, ok := deduplicator.beginDKGResultSubmitted( event.Seed, event.ResultHash, event.BlockNumber, - ); !ok { + ) + if !ok { logger.Infof( "FROST DKG result with hash [0x%x] for seed [0x%x] at block [%v] "+ - "has already been processed", + "is already completed or being processed", event.ResultHash, event.Seed, event.BlockNumber, ) return } + completed := false + defer func() { lease.finish(completed) }() valid, reason, err := frostChain.IsFrostDKGResultValid(event.Result) if err != nil { @@ -258,7 +298,7 @@ func handleFrostDKGResultSubmitted( event.ResultHash, reason, ) - challengeInvalidFrostDKGResult(ctx, node, frostChain, event) + completed = challengeInvalidFrostDKGResult(ctx, node, frostChain, event) return } @@ -273,6 +313,7 @@ func handleFrostDKGResultSubmitted( "selected group and will not approve", event.ResultHash, ) + completed = true return } @@ -281,6 +322,14 @@ func handleFrostDKGResultSubmitted( logger.Errorf("failed to get FROST DKG parameters: [%v]", err) return } + if params == nil { + logger.Errorf("FROST DKG parameters are nil") + return + } + if ctx.Err() != nil { + logger.Errorf("stopping FROST DKG result handling: [%v]", ctx.Err()) + return + } challengePeriodEndBlock := event.BlockNumber + params.ChallengePeriodBlocks approvePrecedencePeriodStartBlock := challengePeriodEndBlock + 1 @@ -306,6 +355,7 @@ func handleFrostDKGResultSubmitted( approvalBlock, ) } + completed = true } func challengeInvalidFrostDKGResult( @@ -313,7 +363,7 @@ func challengeInvalidFrostDKGResult( node *node, frostChain FrostDKGChain, event *FrostDKGResultSubmittedEvent, -) { +) bool { for attempt := uint64(1); ; attempt++ { select { case <-ctx.Done(): @@ -321,21 +371,21 @@ func challengeInvalidFrostDKGResult( "stopping FROST DKG challenge confirmation: [%v]", ctx.Err(), ) - return + return false default: } state, err := frostChain.GetFrostDKGState() if err != nil { logger.Errorf("failed to check FROST DKG state before challenge: [%v]", err) - return + return false } if state != Challenge { logger.Infof( "invalid FROST DKG result [0x%x] challenged successfully", event.ResultHash, ) - return + return true } if err := frostChain.ChallengeFrostDKGResult(event.Result); err != nil { @@ -346,7 +396,7 @@ func challengeInvalidFrostDKGResult( "operator", event.ResultHash, ) - return + return true } logger.Errorf( @@ -360,7 +410,7 @@ func challengeInvalidFrostDKGResult( stateErr, ) } - return + return false } currentBlock, err := currentFrostDKGBlock(node) @@ -369,7 +419,7 @@ func challengeInvalidFrostDKGResult( "failed to get current block after FROST DKG challenge: [%v]", err, ) - return + return false } confirmationBlock := currentBlock + dkgResultChallengeConfirmationBlocks @@ -386,27 +436,27 @@ func challengeInvalidFrostDKGResult( "failed to wait for FROST DKG challenge confirmation: [%v]", err, ) - return + return false } if ctx.Err() != nil { logger.Errorf( "stopping FROST DKG challenge confirmation: [%v]", ctx.Err(), ) - return + return false } state, err = frostChain.GetFrostDKGState() if err != nil { logger.Errorf("failed to check FROST DKG state after challenge: [%v]", err) - return + return false } if state != Challenge { logger.Infof( "invalid FROST DKG result [0x%x] challenged successfully", event.ResultHash, ) - return + return true } logger.Infof( @@ -441,6 +491,14 @@ func scheduleFrostDKGResultApproval( ) return } + if ctx.Err() != nil { + logger.Errorf( + "stopping FROST DKG result approval for member [%d]: [%v]", + memberIndex, + ctx.Err(), + ) + return + } state, err := frostChain.GetFrostDKGState() if err != nil { @@ -533,7 +591,7 @@ func frostDKGRecoveryStartBlock( lookBackBlocks, err := frostDKGRecoveryLookBackBlocks( params, - node.groupParameters, + node.frostGroupParameters, ) if err != nil { return 0, err diff --git a/pkg/tbtc/frost_dkg_coordinator_test.go b/pkg/tbtc/frost_dkg_coordinator_test.go index 9f38c16f8d..bc45727afc 100644 --- a/pkg/tbtc/frost_dkg_coordinator_test.go +++ b/pkg/tbtc/frost_dkg_coordinator_test.go @@ -2,13 +2,316 @@ package tbtc import ( "context" + "fmt" + "math/big" "sync" "testing" "time" + "github.com/keep-network/keep-core/pkg/chain" "github.com/keep-network/keep-core/pkg/frost/registry" ) +func TestHandleFrostDKGStartedReleasesDeduplicationKeyAfterFailure(t *testing.T) { + localChain := Connect() + node := &node{chain: localChain} + event := &FrostDKGStartedEvent{Seed: big.NewInt(100), BlockNumber: 500} + frostChain := &transientFrostDKGStartedChain{event: event} + deduplicator := newDeduplicator() + + // Transient state, past-log, and membership failures must each release the + // key so the same event can advance on the next replay. + for i := 0; i < 4; i++ { + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + event, + false, + ) + } + + if frostChain.stateCalls != 4 || + frostChain.pastEventsCalls != 3 || + frostChain.selectionCalls != 2 { + t.Fatalf( + "unexpected retry calls: state [%d], past events [%d], selection [%d]", + frostChain.stateCalls, + frostChain.pastEventsCalls, + frostChain.selectionCalls, + ) + } + + // The successful replay determined this operator is not a member, which is + // terminal local handling; further duplicates must stay suppressed. + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + event, + false, + ) + if frostChain.stateCalls != 4 { + t.Fatalf("completed event was processed again") + } +} + +func TestHandleFrostDKGStartedRekeysToLatestSeedAlreadyInProgress( + t *testing.T, +) { + localChain := Connect() + node := &node{chain: localChain} + staleEvent := &FrostDKGStartedEvent{Seed: big.NewInt(100), BlockNumber: 500} + latestEvent := &FrostDKGStartedEvent{Seed: big.NewInt(200), BlockNumber: 501} + frostChain := newRekeyingFrostDKGStartedChain( + staleEvent, + latestEvent, + ) + frostChain.blockFirstState = true + deduplicator := newDeduplicator() + + latestDone := make(chan struct{}) + go func() { + defer close(latestDone) + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + latestEvent, + false, + ) + }() + + select { + case <-frostChain.firstStateStarted: + case <-time.After(time.Second): + t.Fatal("latest-seed handler did not acquire its lease") + } + + // The stale handler resolves the latest event while that event's handler + // owns its lease. It must complete the stale lease and must not start a + // second execution for the latest seed. + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + staleEvent, + false, + ) + + stateCalls, pastEventsCalls, selectionCalls := frostChain.calls() + if stateCalls != 2 || pastEventsCalls != 1 || selectionCalls != 0 { + t.Fatalf( + "stale handler duplicated latest-seed work: state [%d], past [%d], selection [%d]", + stateCalls, + pastEventsCalls, + selectionCalls, + ) + } + + close(frostChain.releaseFirstState) + select { + case <-latestDone: + case <-time.After(time.Second): + t.Fatal("latest-seed handler did not finish") + } + + stateCalls, pastEventsCalls, selectionCalls = frostChain.calls() + if stateCalls != 2 || pastEventsCalls != 2 || selectionCalls != 1 { + t.Fatalf( + "unexpected final calls: state [%d], past [%d], selection [%d]", + stateCalls, + pastEventsCalls, + selectionCalls, + ) + } + assertFrostDKGStartedSeedCompleted(t, deduplicator, staleEvent.Seed) + assertFrostDKGStartedSeedCompleted(t, deduplicator, latestEvent.Seed) +} + +func TestHandleFrostDKGStartedRekeysToLatestSeedAlreadyCompleted( + t *testing.T, +) { + localChain := Connect() + node := &node{chain: localChain} + staleEvent := &FrostDKGStartedEvent{Seed: big.NewInt(100), BlockNumber: 500} + latestEvent := &FrostDKGStartedEvent{Seed: big.NewInt(200), BlockNumber: 501} + frostChain := newRekeyingFrostDKGStartedChain( + staleEvent, + latestEvent, + ) + deduplicator := newDeduplicator() + + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + latestEvent, + false, + ) + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + staleEvent, + false, + ) + + stateCalls, pastEventsCalls, selectionCalls := frostChain.calls() + if stateCalls != 2 || pastEventsCalls != 2 || selectionCalls != 1 { + t.Fatalf( + "completed latest seed was processed again: state [%d], past [%d], selection [%d]", + stateCalls, + pastEventsCalls, + selectionCalls, + ) + } + assertFrostDKGStartedSeedCompleted(t, deduplicator, staleEvent.Seed) + assertFrostDKGStartedSeedCompleted(t, deduplicator, latestEvent.Seed) +} + +func TestHandleFrostDKGStartedRekeyedSeedRetriesAfterTransientFailure( + t *testing.T, +) { + localChain := Connect() + node := &node{chain: localChain} + staleEvent := &FrostDKGStartedEvent{Seed: big.NewInt(100), BlockNumber: 500} + latestEvent := &FrostDKGStartedEvent{Seed: big.NewInt(200), BlockNumber: 501} + frostChain := newRekeyingFrostDKGStartedChain( + staleEvent, + latestEvent, + ) + frostChain.selectionFailures = 1 + deduplicator := newDeduplicator() + + // Handling the stale event transfers to the latest seed, whose first + // membership lookup fails. The stale seed is terminal, but the latest + // seed's lease must be released so its own replay can retry. + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + staleEvent, + false, + ) + assertFrostDKGStartedSeedCompleted(t, deduplicator, staleEvent.Seed) + if deduplicator.dkgSeedCache.Has(latestEvent.Seed.Text(16)) { + t.Fatal("latest seed was completed after transient failure") + } + + // A stale-event replay remains suppressed and therefore cannot race the + // latest event. Replaying the latest event retries and completes normally. + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + staleEvent, + false, + ) + handleFrostDKGStarted( + context.Background(), + node, + frostChain, + deduplicator, + latestEvent, + false, + ) + + stateCalls, pastEventsCalls, selectionCalls := frostChain.calls() + if stateCalls != 3 || pastEventsCalls != 3 || selectionCalls != 2 { + t.Fatalf( + "unexpected retry calls: state [%d], past [%d], selection [%d]", + stateCalls, + pastEventsCalls, + selectionCalls, + ) + } + assertFrostDKGStartedSeedCompleted(t, deduplicator, latestEvent.Seed) +} + +func TestHandleFrostDKGResultSubmittedReleasesDeduplicationKeyAfterFailure( + t *testing.T, +) { + localChain := Connect() + node := &node{chain: localChain} + event := &FrostDKGResultSubmittedEvent{ + Seed: big.NewInt(100), + ResultHash: [32]byte{0x01}, + Result: ®istry.Result{}, + BlockNumber: 500, + } + frostChain := &transientFrostDKGResultValidationChain{} + deduplicator := newDeduplicator() + + // A transient validation failure and a later membership failure must both + // release the key so the result can be replayed. + for i := 0; i < 3; i++ { + handleFrostDKGResultSubmitted( + context.Background(), + node, + frostChain, + deduplicator, + event, + ) + } + + if frostChain.validationCalls != 3 || frostChain.selectionCalls != 2 { + t.Fatalf( + "unexpected retry calls: validation [%d], selection [%d]", + frostChain.validationCalls, + frostChain.selectionCalls, + ) + } + + // The successful replay determined this operator is not a member, which is + // terminal local handling; further duplicates must stay suppressed. + handleFrostDKGResultSubmitted( + context.Background(), + node, + frostChain, + deduplicator, + event, + ) + if frostChain.validationCalls != 3 { + t.Fatalf("completed result was processed again") + } +} + +func TestScheduleFrostDKGResultApprovalStopsAfterContextCancellation( + t *testing.T, +) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + frostChain := &approvalRecordingFrostDKGChain{} + scheduleFrostDKGResultApproval( + ctx, + &node{chain: Connect()}, + frostChain, + &FrostDKGResultSubmittedEvent{ + ResultHash: [32]byte{0x01}, + Result: ®istry.Result{}, + }, + 1, + 1, + ) + + if frostChain.stateCalls != 0 { + t.Fatalf("queried state after cancellation") + } + if frostChain.approvalCalls != 0 { + t.Fatalf("approved result after cancellation") + } +} + func TestChallengeInvalidFrostDKGResultRetriesUntilStateLeavesChallenge(t *testing.T) { localChain := Connect(time.Millisecond) node := &node{chain: localChain} @@ -56,6 +359,200 @@ type retryingFrostDKGChallengeChain struct { successOnAttempt int } +type transientFrostDKGStartedChain struct { + FrostDKGChain + + event *FrostDKGStartedEvent + stateCalls int + pastEventsCalls int + selectionCalls int +} + +type rekeyingFrostDKGStartedChain struct { + FrostDKGChain + + mutex sync.Mutex + + events []*FrostDKGStartedEvent + stateCalls int + pastEventsCalls int + selectionCalls int + selectionFailures int + blockFirstState bool + firstStateStarted chan struct{} + releaseFirstState chan struct{} +} + +func newRekeyingFrostDKGStartedChain( + events ...*FrostDKGStartedEvent, +) *rekeyingFrostDKGStartedChain { + return &rekeyingFrostDKGStartedChain{ + events: events, + firstStateStarted: make(chan struct{}), + releaseFirstState: make(chan struct{}), + } +} + +func (rfdkgsc *rekeyingFrostDKGStartedChain) GetFrostDKGState() ( + DKGState, + error, +) { + rfdkgsc.mutex.Lock() + rfdkgsc.stateCalls++ + call := rfdkgsc.stateCalls + block := rfdkgsc.blockFirstState && call == 1 + rfdkgsc.mutex.Unlock() + + if block { + close(rfdkgsc.firstStateStarted) + <-rfdkgsc.releaseFirstState + } + + return AwaitingResult, nil +} + +func (rfdkgsc *rekeyingFrostDKGStartedChain) PastFrostDKGStartedEvents( + *FrostDKGStartedEventFilter, +) ([]*FrostDKGStartedEvent, error) { + rfdkgsc.mutex.Lock() + defer rfdkgsc.mutex.Unlock() + + rfdkgsc.pastEventsCalls++ + return append([]*FrostDKGStartedEvent{}, rfdkgsc.events...), nil +} + +func (rfdkgsc *rekeyingFrostDKGStartedChain) SelectFrostGroup() ( + *GroupSelectionResult, + error, +) { + rfdkgsc.mutex.Lock() + defer rfdkgsc.mutex.Unlock() + + rfdkgsc.selectionCalls++ + if rfdkgsc.selectionFailures > 0 { + rfdkgsc.selectionFailures-- + return nil, fmt.Errorf("transient selection error") + } + + return &GroupSelectionResult{ + OperatorsAddresses: chain.Addresses{ + "0x0000000000000000000000000000000000000001", + }, + }, nil +} + +func (rfdkgsc *rekeyingFrostDKGStartedChain) calls() (int, int, int) { + rfdkgsc.mutex.Lock() + defer rfdkgsc.mutex.Unlock() + + return rfdkgsc.stateCalls, + rfdkgsc.pastEventsCalls, + rfdkgsc.selectionCalls +} + +func assertFrostDKGStartedSeedCompleted( + t *testing.T, + deduplicator *deduplicator, + seed *big.Int, +) { + t.Helper() + + if !deduplicator.dkgSeedCache.Has(seed.Text(16)) { + t.Fatalf("seed [0x%x] was not completed", seed) + } +} + +func (tfdkgsc *transientFrostDKGStartedChain) GetFrostDKGState() (DKGState, error) { + tfdkgsc.stateCalls++ + if tfdkgsc.stateCalls == 1 { + return Idle, fmt.Errorf("transient state error") + } + + return AwaitingResult, nil +} + +func (tfdkgsc *transientFrostDKGStartedChain) PastFrostDKGStartedEvents( + *FrostDKGStartedEventFilter, +) ([]*FrostDKGStartedEvent, error) { + tfdkgsc.pastEventsCalls++ + if tfdkgsc.pastEventsCalls == 1 { + return nil, fmt.Errorf("transient past events error") + } + + return []*FrostDKGStartedEvent{tfdkgsc.event}, nil +} + +func (tfdkgsc *transientFrostDKGStartedChain) SelectFrostGroup() ( + *GroupSelectionResult, + error, +) { + tfdkgsc.selectionCalls++ + if tfdkgsc.selectionCalls == 1 { + return nil, fmt.Errorf("transient selection error") + } + + return &GroupSelectionResult{ + OperatorsAddresses: chain.Addresses{"0x0000000000000000000000000000000000000001"}, + }, nil +} + +type transientFrostDKGResultValidationChain struct { + FrostDKGChain + + validationCalls int + selectionCalls int +} + +func (tfdkgvc *transientFrostDKGResultValidationChain) IsFrostDKGResultValid( + *registry.Result, +) (bool, string, error) { + tfdkgvc.validationCalls++ + if tfdkgvc.validationCalls == 1 { + return false, "", fmt.Errorf("transient validation error") + } + + return true, "", nil +} + +func (tfdkgvc *transientFrostDKGResultValidationChain) SelectFrostGroup() ( + *GroupSelectionResult, + error, +) { + tfdkgvc.selectionCalls++ + if tfdkgvc.selectionCalls == 1 { + return nil, fmt.Errorf("transient selection error") + } + + return &GroupSelectionResult{ + OperatorsAddresses: chain.Addresses{"0x0000000000000000000000000000000000000001"}, + }, nil +} + +type approvalRecordingFrostDKGChain struct { + FrostDKGChain + + stateCalls int + approvalCalls int +} + +func (arfdkgc *approvalRecordingFrostDKGChain) GetFrostDKGState() (DKGState, error) { + arfdkgc.stateCalls++ + return Challenge, nil +} + +func (arfdkgc *approvalRecordingFrostDKGChain) IsFrostDKGResultValid( + *registry.Result, +) (bool, string, error) { + return true, "", nil +} + +func (arfdkgc *approvalRecordingFrostDKGChain) ApproveFrostDKGResult( + *registry.Result, +) error { + arfdkgc.approvalCalls++ + return nil +} + func (rfdgcc *retryingFrostDKGChallengeChain) GetFrostDKGState() (DKGState, error) { rfdgcc.mutex.Lock() defer rfdgcc.mutex.Unlock() diff --git a/pkg/tbtc/frost_dkg_execution_default.go b/pkg/tbtc/frost_dkg_execution_default.go index 4cbd02f4aa..8941d83f63 100644 --- a/pkg/tbtc/frost_dkg_execution_default.go +++ b/pkg/tbtc/frost_dkg_execution_default.go @@ -15,7 +15,7 @@ func executeFrostDKGIfPossible( event *FrostDKGStartedEvent, memberIndexes []group.MemberIndex, _ *GroupSelectionResult, -) { +) bool { logger.Infof( "FROST DKG with seed [0x%x] selected this operator as member "+ "indexes [%v], but native FROST DKG execution is unavailable "+ @@ -23,4 +23,10 @@ func executeFrostDKGIfPossible( event.Seed, memberIndexes, ) + + // This binary cannot gain native DKG support during its lifetime. Mark this + // event complete locally so periodic past-event polling does not repeat the + // full coordinator RPC sequence forever. A restart with a frost_native build + // gets a fresh in-memory deduplication cache and can handle a still-live DKG. + return true } diff --git a/pkg/tbtc/frost_dkg_execution_default_test.go b/pkg/tbtc/frost_dkg_execution_default_test.go new file mode 100644 index 0000000000..a6fe7b4dd8 --- /dev/null +++ b/pkg/tbtc/frost_dkg_execution_default_test.go @@ -0,0 +1,26 @@ +//go:build !frost_native + +package tbtc + +import ( + "context" + "math/big" + "testing" + + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +func TestExecuteFrostDKGUnavailableBuildIsTerminal(t *testing.T) { + completed := executeFrostDKGIfPossible( + context.Background(), + nil, + nil, + &FrostDKGStartedEvent{Seed: big.NewInt(100)}, + []group.MemberIndex{1}, + nil, + ) + + if !completed { + t.Fatal("unavailable build should complete local event handling") + } +} diff --git a/pkg/tbtc/frost_dkg_execution_frost_native.go b/pkg/tbtc/frost_dkg_execution_frost_native.go index 97ff028760..d78d4b2ef0 100644 --- a/pkg/tbtc/frost_dkg_execution_frost_native.go +++ b/pkg/tbtc/frost_dkg_execution_frost_native.go @@ -30,7 +30,7 @@ func executeFrostDKGIfPossible( event *FrostDKGStartedEvent, memberIndexes []group.MemberIndex, groupSelectionResult *GroupSelectionResult, -) { +) bool { nativeTBTCSignerEngine := frostsigning.CurrentNativeTBTCSignerEngine() if nativeTBTCSignerEngine == nil { logger.Infof( @@ -39,7 +39,7 @@ func executeFrostDKGIfPossible( event.Seed, memberIndexes, ) - return + return false } // Distributed DKG produces signing material usable ONLY via the interactive @@ -55,7 +55,7 @@ func executeFrostDKGIfPossible( event.Seed, frostsigning.InteractiveSigningOptInEnvVar, ) - return + return false } membershipValidator := group.NewMembershipValidator( @@ -68,7 +68,7 @@ func executeFrostDKGIfPossible( channel, err := node.netProvider.BroadcastChannelFor(channelName) if err != nil { logger.Errorf("failed to get FROST DKG broadcast channel: [%v]", err) - return + return false } registerFrostDKGResultSigningUnmarshaller(channel) @@ -76,19 +76,25 @@ func executeFrostDKGIfPossible( if err := channel.SetFilter(membershipValidator.IsInGroup); err != nil { logger.Errorf("failed to set FROST DKG broadcast channel filter: [%v]", err) - return + return false } params, err := frostChain.FrostDKGParameters() if err != nil { logger.Errorf("failed to get FROST DKG parameters: [%v]", err) - return + return false + } + if params == nil { + logger.Errorf("FROST DKG parameters are nil") + return false } - signatureThreshold, err := frostDKGSignatureThreshold(node.groupParameters) + signatureThreshold, err := frostDKGSignatureThreshold( + node.frostGroupParameters, + ) if err != nil { logger.Errorf("invalid FROST DKG group parameters: [%v]", err) - return + return false } fullMembers := frostFullMembers(groupSelectionResult) @@ -157,7 +163,7 @@ func executeFrostDKGIfPossible( tbtcSignerMemberIndexes, err := finalFrostDKGMemberIndexes( activeMemberIndexes, groupSelectionResult, - node.groupParameters, + node.frostGroupParameters, ) if err != nil { dkgLogger.Errorf("failed to resolve final FROST DKG member indexes: [%v]", err) @@ -249,6 +255,8 @@ func executeFrostDKGIfPossible( return } }() + + return true } type frostDKGExecutionResult struct { @@ -300,7 +308,7 @@ func executeDistributedFrostDKG( finalSigningGroupOperators, finalSigningGroupMembersIndexes, err := finalSigningGroup( groupSelectionResult.OperatorsAddresses, append([]group.MemberIndex{}, activeMemberIndexes...), - node.groupParameters, + node.frostGroupParameters, ) if err != nil { return nil, fmt.Errorf("cannot resolve the final signing group: [%v]", err) @@ -541,11 +549,11 @@ func announceFrostDKGReadiness( return nil, nil, ctx.Err() } - if len(activeMemberIndexes) < node.groupParameters.GroupQuorum { + if len(activeMemberIndexes) < node.frostGroupParameters.GroupQuorum { return nil, nil, fmt.Errorf( "FROST DKG readiness quorum not reached: [%d] active members, quorum [%d]", len(activeMemberIndexes), - node.groupParameters.GroupQuorum, + node.frostGroupParameters.GroupQuorum, ) } @@ -604,7 +612,7 @@ func registerFrostSignerWithMaterial( finalSigningGroup( groupSelectionResult.OperatorsAddresses, append([]group.MemberIndex{}, activeMemberIndexes...), - node.groupParameters, + node.frostGroupParameters, ) if err != nil { return fmt.Errorf("failed to resolve final FROST signing group members: [%w]", err) diff --git a/pkg/tbtc/frost_dkg_result_signing.go b/pkg/tbtc/frost_dkg_result_signing.go index cfb76d0e0b..428ee2220b 100644 --- a/pkg/tbtc/frost_dkg_result_signing.go +++ b/pkg/tbtc/frost_dkg_result_signing.go @@ -127,7 +127,9 @@ func signAndCollectFrostDKGResultSignatures( } } - expectedSignaturesCount, err := frostDKGSignatureThreshold(node.groupParameters) + expectedSignaturesCount, err := frostDKGSignatureThreshold( + node.frostGroupParameters, + ) if err != nil { return nil, err } diff --git a/pkg/tbtc/node.go b/pkg/tbtc/node.go index 76ac21db70..22d3e3df6b 100644 --- a/pkg/tbtc/node.go +++ b/pkg/tbtc/node.go @@ -53,7 +53,12 @@ const ( // node represents the current state of an ECDSA node. type node struct { + // groupParameters contains the legacy ECDSA wallet parameters. groupParameters *GroupParameters + // frostGroupParameters contains the independently configured FROST wallet + // parameters. FROST DKG must not use the legacy validator's constants: the + // two registries can deliberately use different group sizes and thresholds. + frostGroupParameters *GroupParameters chain Chain btcChain bitcoin.Chain @@ -175,7 +180,11 @@ func newNode( scheduler.RegisterProtocol(latch) node := &node{ - groupParameters: groupParameters, + groupParameters: groupParameters, + // Initialize this field for chains without FROST support and for tests that + // construct a node directly. Initialize replaces it with the parameters + // loaded from FrostDkgValidator whenever FROST is enabled. + frostGroupParameters: groupParameters, chain: chain, btcChain: btcChain, netProvider: netProvider, @@ -509,11 +518,16 @@ func (n *node) getSigningExecutor( ) } + walletGroupParameters, err := n.groupParametersForSigners(signers) + if err != nil { + return nil, false, err + } + executor := newSigningExecutor( signers, broadcastChannel, membershipValidator, - n.groupParameters, + walletGroupParameters, n.protocolLatch, blockCounter.CurrentBlock, n.waitForBlockHeight, @@ -708,12 +722,17 @@ func (n *node) getInactivityClaimExecutor( len(signers), ) + walletGroupParameters, err := n.groupParametersForSigners(signers) + if err != nil { + return nil, false, err + } + executor := newInactivityClaimExecutor( n.chain, signers, broadcastChannel, membershipValidator, - n.groupParameters, + walletGroupParameters, n.protocolLatch, n.waitForBlockHeight, ) @@ -723,6 +742,32 @@ func (n *node) getInactivityClaimExecutor( return executor, true, nil } +// groupParametersForSigners selects parameters from the registry that created +// the wallet. Native Schnorr signer material denotes a FROST wallet; legacy +// material denotes an ECDSA wallet. All signers in this slice belong to the +// same wallet, so the first signer is sufficient to identify the scheme. +func (n *node) groupParametersForSigners( + signers []*signer, +) (*GroupParameters, error) { + if len(signers) == 0 { + return nil, fmt.Errorf("cannot select group parameters without signers") + } + + if signingMaterialUsesSchnorrSignatures(signers[0].signingMaterial()) { + if n.frostGroupParameters == nil { + return nil, fmt.Errorf("FROST group parameters are not configured") + } + + return n.frostGroupParameters, nil + } + + if n.groupParameters == nil { + return nil, fmt.Errorf("ECDSA group parameters are not configured") + } + + return n.groupParameters, nil +} + // handleHeartbeatProposal handles an incoming heartbeat proposal by // orchestrating and dispatching an appropriate wallet action. func (n *node) handleHeartbeatProposal( diff --git a/pkg/tbtc/node_test.go b/pkg/tbtc/node_test.go index 69232fe9be..fe35a28890 100644 --- a/pkg/tbtc/node_test.go +++ b/pkg/tbtc/node_test.go @@ -14,6 +14,7 @@ import ( "github.com/keep-network/keep-core/internal/testutils" "github.com/keep-network/keep-core/pkg/bitcoin" "github.com/keep-network/keep-core/pkg/chain" + frostsigning "github.com/keep-network/keep-core/pkg/frost/signing" "github.com/keep-network/keep-core/pkg/generator" "github.com/keep-network/keep-core/pkg/internal/tecdsatest" "github.com/keep-network/keep-core/pkg/net/local" @@ -21,6 +22,63 @@ import ( "github.com/keep-network/keep-core/pkg/tecdsa" ) +func TestNode_GroupParametersForSignersUsesWalletScheme(t *testing.T) { + ecdsaGroupParameters := &GroupParameters{ + GroupSize: 100, + GroupQuorum: 90, + HonestThreshold: 51, + } + frostGroupParameters := &GroupParameters{ + GroupSize: 5, + GroupQuorum: 4, + HonestThreshold: 3, + } + node := &node{ + groupParameters: ecdsaGroupParameters, + frostGroupParameters: frostGroupParameters, + } + + tests := map[string]struct { + signer *signer + expected *GroupParameters + }{ + "legacy ECDSA wallet": { + signer: &signer{ + privateKeyShare: &tecdsa.PrivateKeyShare{}, + }, + expected: ecdsaGroupParameters, + }, + "native FROST wallet": { + signer: &signer{ + signerMaterial: &frostsigning.NativeSignerMaterial{ + Format: frostsigning.NativeSignerMaterialFormatFrostTBTCSignerV1, + Payload: []byte(`{ + "keyGroup":"key-group", + "keyGroupSource":"dkg-persisted" + }`), + }, + }, + expected: frostGroupParameters, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + actual, err := node.groupParametersForSigners([]*signer{test.signer}) + if err != nil { + t.Fatalf("unexpected error: [%v]", err) + } + if actual != test.expected { + t.Fatalf( + "unexpected group parameters\nexpected: [%+v]\nactual: [%+v]", + test.expected, + actual, + ) + } + }) + } +} + func TestNode_GetSigningExecutor(t *testing.T) { groupParameters := &GroupParameters{ GroupSize: 5, diff --git a/pkg/tbtc/tbtc.go b/pkg/tbtc/tbtc.go index 4b94b97259..fdf3253a5f 100644 --- a/pkg/tbtc/tbtc.go +++ b/pkg/tbtc/tbtc.go @@ -141,6 +141,7 @@ func Initialize( ethereumNetwork ethereum.Network, ) error { groupParameters := defaultGroupParameters(ethereumNetwork) + var frostGroupParameters *GroupParameters if ethChain, ok := chain.(interface { EcdsaWalletGroupParametersFromChain(context.Context) (*GroupParameters, error) @@ -164,6 +165,44 @@ func Initialize( ) } } + frostGroupParameters = groupParameters + + if frostChain, ok := chain.(FrostDKGChain); ok && + frostChain.FrostWalletRegistryAvailable() { + parameterSource, ok := chain.(interface { + FrostWalletGroupParametersFromChain(context.Context) (*GroupParameters, error) + }) + if !ok { + return fmt.Errorf( + "cannot read TBTC FROST group sizing: chain does not expose " + + "FrostDkgValidator parameters", + ) + } + + gp, err := parameterSource.FrostWalletGroupParametersFromChain(ctx) + if err != nil { + return fmt.Errorf( + "cannot read TBTC FROST group sizing from FROST validator: [%w]", + err, + ) + } + if gp == nil { + return fmt.Errorf( + "cannot read TBTC FROST group sizing from FROST validator: " + + "parameters are nil", + ) + } + + frostGroupParameters = gp + logger.Infof( + "TBTC FROST group parameters from validator contract (size=[%v] "+ + "groupQuorum/activeThreshold=[%v] "+ + "honestThreshold/groupThreshold=[%v])", + gp.GroupSize, + gp.GroupQuorum, + gp.HonestThreshold, + ) + } node, err := newNode( groupParameters, @@ -179,6 +218,7 @@ func Initialize( if err != nil { return fmt.Errorf("cannot set up TBTC node: [%v]", err) } + node.frostGroupParameters = frostGroupParameters // Note: the FROST signing-backend guard runs inside newNode above (right // after the backend is configured and before the legacy pre-params pool is