chore: keep sync fp test (#58)

Add tests to created functions at #52

finality-provider/service/app.go

@@ -265,10 +265,6 @@ func (app *FinalityProviderApp) SyncFinalityProviderStatus() (fpInstanceRunning
 			}
 		}
 
-		if !fp.ShouldSyncStatusFromVotingPower(vp) {
-			continue
-		}
-
 		bip340PubKey := fp.GetBIP340BTCPK()
 		if app.fpManager.IsFinalityProviderRunning(bip340PubKey) {
 			// there is a instance running, no need to keep syncing
@@ -283,13 +279,15 @@ func (app *FinalityProviderApp) SyncFinalityProviderStatus() (fpInstanceRunning
 			return false, err
 		}
 
-		app.logger.Info(
-			"Update FP status",
-			zap.String("fp_addr", fp.FPAddr),
-			zap.String("old_status", oldStatus.String()),
-			zap.String("new_status", newStatus.String()),
-		)
-		fp.Status = newStatus
+		if oldStatus != newStatus {
+			app.logger.Info(
+				"Update FP status",
+				zap.String("fp_addr", fp.FPAddr),
+				zap.String("old_status", oldStatus.String()),
+				zap.String("new_status", newStatus.String()),
+			)
+			fp.Status = newStatus
+		}
 
 		if !fp.ShouldStart() {
 			continue
@@ -697,6 +695,7 @@ func (app *FinalityProviderApp) syncChainFpStatusLoop() {
 		zap.Float64("interval seconds", interval.Seconds()),
 	)
 	syncFpStatusTicker := time.NewTicker(interval)
+	defer syncFpStatusTicker.Stop()
 
 	for {
 		select {
@@ -710,7 +709,6 @@ func (app *FinalityProviderApp) syncChainFpStatusLoop() {
 			}
 
 		case <-app.quit:
-			syncFpStatusTicker.Stop()
 			app.logger.Info("exiting sync FP status loop")
 			return
 		}

finality-provider/service/app_test.go

@@ -1,11 +1,16 @@
 package service_test
 
 import (
+	"errors"
+	"fmt"
 	"math/rand"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
+	"time"
 
+	"github.com/babylonlabs-io/babylon/testutil/datagen"
 	bbntypes "github.com/babylonlabs-io/babylon/types"
 	bstypes "github.com/babylonlabs-io/babylon/x/btcstaking/types"
 	"github.com/golang/mock/gomock"
@@ -138,3 +143,81 @@ func FuzzRegisterFinalityProvider(f *testing.F) {
 		require.Equal(t, true, fpInfo.IsRunning)
 	})
 }
+
+func FuzzSyncFinalityProviderStatus(f *testing.F) {
+	testutil.AddRandomSeedsToFuzzer(f, 14)
+	f.Fuzz(func(t *testing.T, seed int64) {
+		r := rand.New(rand.NewSource(seed))
+
+		logger := zap.NewNop()
+
+		pathSuffix := datagen.GenRandomHexStr(r, 10)
+		// create an EOTS manager
+		eotsHomeDir := filepath.Join(t.TempDir(), "eots-home", pathSuffix)
+		eotsCfg := eotscfg.DefaultConfigWithHomePath(eotsHomeDir)
+		dbBackend, err := eotsCfg.DatabaseConfig.GetDbBackend()
+		require.NoError(t, err)
+		em, err := eotsmanager.NewLocalEOTSManager(eotsHomeDir, eotsCfg.KeyringBackend, dbBackend, logger)
+		require.NoError(t, err)
+
+		// Create randomized config
+		fpHomeDir := filepath.Join(t.TempDir(), "fp-home", pathSuffix)
+		fpCfg := config.DefaultConfigWithHome(fpHomeDir)
+		fpCfg.SyncFpStatusInterval = time.Millisecond * 100
+		// no need for other intervals to run
+		fpCfg.StatusUpdateInterval = time.Minute * 10
+		fpCfg.SubmissionRetryInterval = time.Minute * 10
+		fpdb, err := fpCfg.DatabaseConfig.GetDbBackend()
+		require.NoError(t, err)
+
+		randomStartingHeight := uint64(r.Int63n(100) + 1)
+		currentHeight := randomStartingHeight + uint64(r.Int63n(10)+2)
+		mockClientController := testutil.PrepareMockedClientController(t, r, randomStartingHeight, currentHeight)
+
+		blkInfo := &types.BlockInfo{Height: currentHeight}
+
+		mockClientController.EXPECT().QueryLatestFinalizedBlocks(gomock.Any()).Return(nil, nil).AnyTimes()
+		mockClientController.EXPECT().QueryBestBlock().Return(blkInfo, nil).Return(blkInfo, nil).AnyTimes()
+		mockClientController.EXPECT().QueryBlock(gomock.Any()).Return(nil, errors.New("chain not online")).AnyTimes()
+
+		noVotingPowerTable := r.Int31n(10) > 5
+		if noVotingPowerTable {
+			allowedErr := fmt.Sprintf("failed to query Finality Voting Power at Height %d: rpc error: code = Unknown desc = %s: unknown request", currentHeight, bstypes.ErrVotingPowerTableNotUpdated.Wrapf("height: %d", currentHeight).Error())
+			mockClientController.EXPECT().QueryFinalityProviderVotingPower(gomock.Any(), gomock.Any()).Return(uint64(0), errors.New(allowedErr)).AnyTimes()
+			mockClientController.EXPECT().QueryActivatedHeight().Return(uint64(0), errors.New(allowedErr)).AnyTimes()
+		} else {
+			mockClientController.EXPECT().QueryActivatedHeight().Return(currentHeight, nil).AnyTimes()
+			mockClientController.EXPECT().QueryFinalityProviderVotingPower(gomock.Any(), gomock.Any()).Return(uint64(2), nil).AnyTimes()
+		}
+
+		app, err := service.NewFinalityProviderApp(&fpCfg, mockClientController, em, fpdb, logger)
+		require.NoError(t, err)
+
+		err = app.Start()
+		require.NoError(t, err)
+
+		fp := testutil.GenStoredFinalityProvider(r, t, app, "", hdPath, nil)
+
+		require.Eventually(t, func() bool {
+			fpPk := fp.GetBIP340BTCPK()
+			fpInfo, err := app.GetFinalityProviderInfo(fpPk)
+			if err != nil {
+				return false
+			}
+
+			expectedStatus := proto.FinalityProviderStatus_ACTIVE
+			if noVotingPowerTable {
+				expectedStatus = proto.FinalityProviderStatus_REGISTERED
+			}
+			fpInstance, err := app.GetFinalityProviderInstance(fpPk)
+			if err != nil {
+				return false
+			}
+
+			// TODO: verify why mocks are failing
+			btcPkEqual := fpInstance.GetBtcPk().IsEqual(fp.BtcPk)
+			statusEqual := strings.EqualFold(fpInfo.Status, expectedStatus.String())
+			return statusEqual && btcPkEqual
+		}, time.Second*5, time.Millisecond*200, "should eventually be registered or active")
+	})
+}

finality-provider/store/fpstore.go

@@ -117,7 +117,12 @@ func (s *FinalityProviderStore) SetFpStatus(btcPk *btcec.PublicKey, status proto
 func (s *FinalityProviderStore) UpdateFpStatusFromVotingPower(
 	vp uint64,
 	fp *StoredFinalityProvider,
-) (proto.FinalityProviderStatus, error) {
+) (newStatus proto.FinalityProviderStatus, err error) {
+	if fp.Status == proto.FinalityProviderStatus_SLASHED {
+		// Slashed FP should not update status
+		return proto.FinalityProviderStatus_SLASHED, nil
+	}
+
 	if vp > 0 {
 		// voting power > 0 then set the status to ACTIVE
 		return proto.FinalityProviderStatus_ACTIVE, s.SetFpStatus(fp.BtcPk, proto.FinalityProviderStatus_ACTIVE)

finality-provider/store/fpstore_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/babylonlabs-io/finality-provider/finality-provider/config"
+	"github.com/babylonlabs-io/finality-provider/finality-provider/proto"
 	fpstore "github.com/babylonlabs-io/finality-provider/finality-provider/store"
 	"github.com/babylonlabs-io/finality-provider/testutil"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -78,3 +79,143 @@ func FuzzFinalityProvidersStore(f *testing.F) {
 		require.ErrorIs(t, err, fpstore.ErrFinalityProviderNotFound)
 	})
 }
+
+func TestUpdateFpStatusFromVotingPower(t *testing.T) {
+	r := rand.New(rand.NewSource(10))
+	anyFpStatus := proto.FinalityProviderStatus(100)
+
+	tcs := []struct {
+		name               string
+		fpStoredStatus     proto.FinalityProviderStatus
+		votingPowerOnChain uint64
+		expStatus          proto.FinalityProviderStatus
+		expErr             error
+	}{
+		{
+			"zero vp: Created to Registered",
+			proto.FinalityProviderStatus_CREATED,
+			0,
+			proto.FinalityProviderStatus_REGISTERED,
+			nil,
+		},
+		{
+			"zero vp: Active to Inactive",
+			proto.FinalityProviderStatus_ACTIVE,
+			0,
+			proto.FinalityProviderStatus_INACTIVE,
+			nil,
+		},
+		{
+			"zero vp: Registered should not update the status, but also not error out",
+			proto.FinalityProviderStatus_REGISTERED,
+			0,
+			proto.FinalityProviderStatus_REGISTERED,
+			nil,
+		},
+		{
+			"zero vp: Slashed to Slashed",
+			proto.FinalityProviderStatus_SLASHED,
+			0,
+			proto.FinalityProviderStatus_SLASHED,
+			nil,
+		},
+		{
+			"err: Slashed should not update status",
+			proto.FinalityProviderStatus_SLASHED,
+			15,
+			proto.FinalityProviderStatus_SLASHED,
+			nil,
+		},
+		{
+			"vp > 0: Created to Active",
+			proto.FinalityProviderStatus_CREATED,
+			1,
+			proto.FinalityProviderStatus_ACTIVE,
+			nil,
+		},
+		{
+			"vp > 0: Registered to Active",
+			proto.FinalityProviderStatus_REGISTERED,
+			1,
+			proto.FinalityProviderStatus_ACTIVE,
+			nil,
+		},
+		{
+			"vp > 0: Inactive to Active",
+			proto.FinalityProviderStatus_INACTIVE,
+			1,
+			proto.FinalityProviderStatus_ACTIVE,
+			nil,
+		},
+		{
+			"err: fp not found and vp > 0",
+			proto.FinalityProviderStatus_INACTIVE,
+			1,
+			anyFpStatus,
+			fpstore.ErrFinalityProviderNotFound,
+		},
+		{
+			"err: fp not found and vp == 0 && created",
+			proto.FinalityProviderStatus_CREATED,
+			0,
+			anyFpStatus,
+			fpstore.ErrFinalityProviderNotFound,
+		},
+		{
+			"err: fp not found and vp == 0 && active",
+			proto.FinalityProviderStatus_ACTIVE,
+			0,
+			anyFpStatus,
+			fpstore.ErrFinalityProviderNotFound,
+		},
+	}
+
+	homePath := t.TempDir()
+	cfg := config.DefaultDBConfigWithHomePath(homePath)
+
+	fpdb, err := cfg.GetDbBackend()
+	require.NoError(t, err)
+	fps, err := fpstore.NewFinalityProviderStore(fpdb)
+	require.NoError(t, err)
+
+	defer func() {
+		err := fpdb.Close()
+		require.NoError(t, err)
+		err = os.RemoveAll(homePath)
+		require.NoError(t, err)
+	}()
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			fp := testutil.GenRandomFinalityProvider(r, t)
+			fp.Status = tc.fpStoredStatus
+			if tc.expErr == nil {
+				err = fps.CreateFinalityProvider(
+					sdk.MustAccAddressFromBech32(fp.FPAddr),
+					fp.BtcPk,
+					fp.Description,
+					fp.Commission,
+					fp.KeyName,
+					fp.ChainID,
+					fp.Pop.BtcSig,
+				)
+				require.NoError(t, err)
+
+				err = fps.SetFpStatus(fp.BtcPk, fp.Status)
+				require.NoError(t, err)
+			}
+
+			actStatus, err := fps.UpdateFpStatusFromVotingPower(tc.votingPowerOnChain, fp)
+			if tc.expErr != nil {
+				require.EqualError(t, err, tc.expErr.Error())
+				return
+			}
+			require.NoError(t, err)
+			require.Equal(t, tc.expStatus, actStatus)
+
+			storedFp, err := fps.GetFinalityProvider(fp.BtcPk)
+			require.NoError(t, err)
+			require.Equal(t, tc.expStatus, storedFp.Status)
+		})
+	}
+}

finality-provider/store/storedfp.go

@@ -78,20 +78,6 @@ func (sfp *StoredFinalityProvider) ToFinalityProviderInfo() *proto.FinalityProvi
 	}
 }
 
-// ShouldSyncStatusFromVotingPower returns true if the status should be updated
-// based on the provided voting power or the current status of the finality provider.
-//
-// It returns true if the voting power is greater than zero, or if the status
-// is either 'CREATED' or 'ACTIVE'.
-func (sfp *StoredFinalityProvider) ShouldSyncStatusFromVotingPower(vp uint64) bool {
-	if vp > 0 {
-		return true
-	}
-
-	return sfp.Status == proto.FinalityProviderStatus_CREATED ||
-		sfp.Status == proto.FinalityProviderStatus_ACTIVE
-}
-
 // ShouldStart returns true if the finality provider should start his instance
 // based on the current status of the finality provider.
 //

finality-provider/store/storedfp_test.go

@@ -0,0 +1,55 @@
+package store_test
+
+import (
+	"math/rand"
+	"testing"
+
+	"github.com/babylonlabs-io/finality-provider/finality-provider/proto"
+	"github.com/babylonlabs-io/finality-provider/testutil"
+	"github.com/stretchr/testify/require"
+)
+
+func TestShouldStart(t *testing.T) {
+	tcs := []struct {
+		name           string
+		currFpStatus   proto.FinalityProviderStatus
+		expShouldStart bool
+	}{
+		{
+			"Created: Should NOT start",
+			proto.FinalityProviderStatus_CREATED,
+			false,
+		},
+		{
+			"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 {
+		t.Run(tc.name, func(t *testing.T) {
+			fp := testutil.GenRandomFinalityProvider(r, t)
+			fp.Status = tc.currFpStatus
+
+			shouldStart := fp.ShouldStart()
+			require.Equal(t, tc.expShouldStart, shouldStart)
+		})
+	}
+}