From 5f8d43a0b6b676491138eb870e2703f83b74c6db Mon Sep 17 00:00:00 2001 From: Cirrus Gai Date: Mon, 6 Jan 2025 21:54:15 +0800 Subject: [PATCH] chore: Allow running of jailed fp (#260) Closes #259. This allows jailed fps to still be able to send randomness and it could start voting immdiately after unjailing. --- CHANGELOG.md | 5 ++- finality-provider/service/app.go | 7 --- finality-provider/service/app_test.go | 12 ++++-- finality-provider/service/event_loops.go | 12 +++--- finality-provider/service/fp_instance.go | 35 +++++++++++++-- finality-provider/store/storedfp.go | 14 ------ finality-provider/store/storedfp_test.go | 54 ------------------------ 7 files changed, 49 insertions(+), 90 deletions(-) delete mode 100644 finality-provider/store/storedfp_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index fb593898..70677edf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,9 +39,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Improvements -* [#251](https://github.com/babylonlabs-io/finality-provider/pull/251) chore: nlreturn lint -* [#252](https://github.com/babylonlabs-io/finality-provider/pull/252) feat: rm interceptors and use context +* [#251](https://github.com/babylonlabs-io/finality-provider/pull/251) Add nlreturn lint +* [#252](https://github.com/babylonlabs-io/finality-provider/pull/252) Remove interceptors and use context * [#253](https://github.com/babylonlabs-io/finality-provider/issues/253) Refactor to start from the last finalized height +* [#260](https://github.com/babylonlabs-io/finality-provider/pull/260) Allow running of jailed fp ## v0.14.2 diff --git a/finality-provider/service/app.go b/finality-provider/service/app.go index 0c92f026..46dd46a7 100644 --- a/finality-provider/service/app.go +++ b/finality-provider/service/app.go @@ -549,13 +549,6 @@ func (app *FinalityProviderApp) setFinalityProviderSlashed(fpi *FinalityProvider } } -func (app *FinalityProviderApp) setFinalityProviderJailed(fpi *FinalityProviderInstance) { - fpi.MustSetStatus(proto.FinalityProviderStatus_JAILED) - if err := app.removeFinalityProviderInstance(); err != nil { - panic(fmt.Errorf("failed to terminate a jailed finality-provider %s: %w", fpi.GetBtcPkHex(), err)) - } -} - // NOTE: this is not safe in production, so only used for testing purpose func (app *FinalityProviderApp) getFpPrivKey(fpPk []byte) (*btcec.PrivateKey, error) { record, err := app.eotsManager.KeyRecord(fpPk, "") diff --git a/finality-provider/service/app_test.go b/finality-provider/service/app_test.go index 82de8b3d..85b6ca96 100644 --- a/finality-provider/service/app_test.go +++ b/finality-provider/service/app_test.go @@ -210,6 +210,7 @@ func FuzzUnjailFinalityProvider(f *testing.F) { fpCfg := config.DefaultConfigWithHome(fpHomeDir) // use shorter interval for the test to end faster fpCfg.SubmissionRetryInterval = time.Millisecond * 10 + fpCfg.SignatureSubmissionInterval = time.Millisecond * 10 blkInfo := &types.BlockInfo{Height: currentHeight} @@ -230,12 +231,17 @@ func FuzzUnjailFinalityProvider(f *testing.F) { expectedTxHash := datagen.GenRandomHexStr(r, 32) mockClientController.EXPECT().UnjailFinalityProvider(fpPk.MustToBTCPK()).Return(&types.TxResponse{TxHash: expectedTxHash}, nil) + err := app.StartFinalityProvider(fpPk, "") + require.NoError(t, err) + fpIns, err := app.GetFinalityProviderInstance() + require.NoError(t, err) + require.True(t, fpIns.IsJailed()) res, err := app.UnjailFinalityProvider(fpPk) require.NoError(t, err) require.Equal(t, expectedTxHash, res.TxHash) - fpInfo, err := app.GetFinalityProviderInfo(fpPk) - require.NoError(t, err) - require.Equal(t, proto.FinalityProviderStatus_INACTIVE.String(), fpInfo.GetStatus()) + require.Eventually(t, func() bool { + return !fpIns.IsJailed() + }, eventuallyWaitTimeOut, eventuallyPollTime) }) } diff --git a/finality-provider/service/event_loops.go b/finality-provider/service/event_loops.go index 47021b11..ab6c877a 100644 --- a/finality-provider/service/event_loops.go +++ b/finality-provider/service/event_loops.go @@ -67,13 +67,6 @@ func (app *FinalityProviderApp) monitorCriticalErr() { continue } - if errors.Is(criticalErr.err, ErrFinalityProviderJailed) { - app.setFinalityProviderJailed(fpi) - app.logger.Debug("the finality-provider has been jailed", - zap.String("pk", criticalErr.fpBtcPk.MarshalHex())) - - continue - } app.logger.Fatal(instanceTerminatingMsg, zap.String("pk", criticalErr.fpBtcPk.MarshalHex()), zap.Error(criticalErr.err)) case <-app.quit: @@ -189,6 +182,11 @@ func (app *FinalityProviderApp) unjailFpLoop() { zap.String("txHash", res.TxHash), ) + // set the status to INACTIVE by default + // the status will be changed to ACTIVE + // if it has voting power for the next height + app.fps.MustSetFpStatus(req.btcPubKey.MustToBTCPK(), proto.FinalityProviderStatus_INACTIVE) + req.successResponse <- &UnjailFinalityProviderResponse{ TxHash: res.TxHash, } diff --git a/finality-provider/service/fp_instance.go b/finality-provider/service/fp_instance.go index 78050d0d..725d3e72 100644 --- a/finality-provider/service/fp_instance.go +++ b/finality-provider/service/fp_instance.go @@ -68,8 +68,8 @@ func NewFinalityProviderInstance( return nil, fmt.Errorf("failed to retrieve the finality provider %s from DB: %w", fpPk.MarshalHex(), err) } - if !sfp.ShouldStart() { - return nil, fmt.Errorf("the finality provider instance cannot be initiated with status %s", sfp.Status.String()) + if sfp.Status == proto.FinalityProviderStatus_SLASHED { + return nil, fmt.Errorf("the finality provider instance is already slashed") } return newFinalityProviderInstanceFromStore(sfp, cfg, s, prStore, cc, em, metrics, passphrase, errChan, logger) @@ -109,7 +109,8 @@ func (fp *FinalityProviderInstance) Start() error { } if fp.IsJailed() { - return fmt.Errorf("%w: %s", ErrFinalityProviderJailed, fp.GetBtcPkHex()) + fp.logger.Warn("the finality provider is jailed", + zap.String("pk", fp.GetBtcPkHex())) } startHeight, err := fp.DetermineStartHeight() @@ -163,7 +164,19 @@ func (fp *FinalityProviderInstance) IsRunning() bool { return fp.isStarted.Load() } +// IsJailed returns true if fp is JAILED +// NOTE: it retrieves the the status from the db to +// ensure status is up-to-date func (fp *FinalityProviderInstance) IsJailed() bool { + storedFp, err := fp.fpState.s.GetFinalityProvider(fp.GetBtcPk()) + if err != nil { + panic(fmt.Errorf("failed to retrieve the finality provider %s from db: %w", fp.GetBtcPkHex(), err)) + } + + if storedFp.Status != fp.GetStatus() { + fp.MustSetStatus(storedFp.Status) + } + return fp.GetStatus() == proto.FinalityProviderStatus_JAILED } @@ -178,6 +191,15 @@ func (fp *FinalityProviderInstance) finalitySigSubmissionLoop() { if len(pollerBlocks) == 0 { continue } + + if fp.IsJailed() { + fp.logger.Warn("the finality-provider is jailed", + zap.String("pk", fp.GetBtcPkHex()), + ) + + continue + } + targetHeight := pollerBlocks[len(pollerBlocks)-1].Height fp.logger.Debug("the finality-provider received new block(s), start processing", zap.String("pk", fp.GetBtcPkHex()), @@ -199,6 +221,13 @@ func (fp *FinalityProviderInstance) finalitySigSubmissionLoop() { res, err := fp.retrySubmitSigsUntilFinalized(processedBlocks) if err != nil { fp.metrics.IncrementFpTotalFailedVotes(fp.GetBtcPkHex()) + if errors.Is(err, ErrFinalityProviderJailed) { + fp.MustSetStatus(proto.FinalityProviderStatus_JAILED) + fp.logger.Debug("the finality-provider has been jailed", + zap.String("pk", fp.GetBtcPkHex())) + + continue + } if !errors.Is(err, ErrFinalityProviderShutDown) { fp.reportCriticalErr(err) } diff --git a/finality-provider/store/storedfp.go b/finality-provider/store/storedfp.go index 5da283ed..acf9df3c 100644 --- a/finality-provider/store/storedfp.go +++ b/finality-provider/store/storedfp.go @@ -69,17 +69,3 @@ func (sfp *StoredFinalityProvider) ToFinalityProviderInfo() *proto.FinalityProvi Status: sfp.Status.String(), } } - -// ShouldStart returns true if the finality provider should start his instance -// based on the current status of the finality provider. -// -// It returns false if the status is either 'REGISTERED', 'JAILED' or 'SLASHED'. -// It returns true for all the other status. -func (sfp *StoredFinalityProvider) ShouldStart() bool { - if sfp.Status == proto.FinalityProviderStatus_SLASHED || - sfp.Status == proto.FinalityProviderStatus_JAILED { - return false - } - - return true -} diff --git a/finality-provider/store/storedfp_test.go b/finality-provider/store/storedfp_test.go deleted file mode 100644 index d99d7f2b..00000000 --- a/finality-provider/store/storedfp_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package store_test - -import ( - "math/rand" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/babylonlabs-io/finality-provider/finality-provider/proto" - "github.com/babylonlabs-io/finality-provider/testutil" -) - -func TestShouldStart(t *testing.T) { - t.Parallel() - tcs := []struct { - name string - currFpStatus proto.FinalityProviderStatus - expShouldStart bool - }{ - { - "Slashed: Should NOT start", - proto.FinalityProviderStatus_SLASHED, - false, - }, - { - "Inactive: Should start", - proto.FinalityProviderStatus_INACTIVE, - true, - }, - { - "Registered: Should start", - proto.FinalityProviderStatus_REGISTERED, - true, - }, - { - "Active: Should start", - proto.FinalityProviderStatus_ACTIVE, - true, - }, - } - - r := rand.New(rand.NewSource(10)) - for _, tc := range tcs { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - fp := testutil.GenRandomFinalityProvider(r, t) - fp.Status = tc.currFpStatus - - shouldStart := fp.ShouldStart() - require.Equal(t, tc.expShouldStart, shouldStart) - }) - } -}