chore: Refactor CommitPubRand

CHANGELOG.md

@@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * [#228](https://github.com/babylonlabs-io/finality-provider/pull/228) Save key name mapping in eotsd import commands
 * [#227](https://github.com/babylonlabs-io/finality-provider/pull/227) Fix FP submission loop
 * [#226](https://github.com/babylonlabs-io/finality-provider/pull/226) Update local fp before register
+* [#233](https://github.com/babylonlabs-io/finality-provider/pull/233) Refactor CommitPubRand
 
 ## v0.13.1
 

Makefile

@@ -58,7 +58,7 @@ build-docker:
 
 .PHONY: test
 test:
-	go test ./...
+	go test -v ./...
 
 test-e2e:
 	go test -mod=readonly -failfast -timeout=25m -v $(PACKAGES_E2E) -count=1 --tags=e2e
@@ -139,4 +139,4 @@ release:
 else
 release:
 	@echo "Error: GITHUB_TOKEN is not defined. Please define it before running 'make release'."
-endif
+endif

docs/commit-pub-rand.md

@@ -42,27 +42,35 @@ committed before finality votes can be sent. Otherwise, the finality provider
 looses voting power for this height.
 
 To this end, when a finality provider is started, it runs a loop to periodically
-check whether it needs to make a new commit. In particualar,
-the following statement is checked:
+check whether it needs to make a new commit and calculate the start height of
+the next commit. In particular:
 
 ```go
-if lastCommittedHeight < currentHeight + uint64(MinRandHeightGap)
+	tipHeightWithDelay := tipHeight + uint64(fp.cfg.TimestampingDelayBlocks)
+	var startHeight uint64
+	switch {
+	case lastCommittedHeight < tipHeightWithDelay:
+		// the start height should consider the timestamping delay
+		// as it is only available to use after tip height + estimated timestamping delay
+		startHeight = tipHeightWithDelay
+	case lastCommittedHeight < tipHeightWithDelay+uint64(fp.cfg.NumPubRand):
+		startHeight = lastCommittedHeight + 1
+	default:
+        // randomness is sufficient, do not need to make a commit
 ```
 
 where:
 
 - `lastCommittedHeight` is the end height (`startHeight + numRand - 1`)
 from the latest public randomness commit recorded on the consumer chain
-- `currentHeight` is the current height of the consumer chain
-- `MinRandHeightGap` is a configuration value, which measures when to make a
+- `tipHeight` is the current height of the consumer chain
+- `TimestampingDelayBlocks` is a configuration value, which measures when to make a
   new commit
+- `NumPubRand` is the number of randomness in a commit defined in the config.
 
-If the statement is true, a new commit should be made to ensure sufficient
-randomness is available for future blocks.
+### Determining TimestampingDelayBlocks
 
-### Determining MinRandHeightGap
-
-The value of `MinRandHeightGap` must account for BTC-timestamping
+The value of `TimestampingDelayBlocks` must account for BTC-timestamping
 delays, which is needed to activate the randomness for a specific height
 after the committed epoch is BTC-timestamped. Here's an example:
 
@@ -82,7 +90,7 @@ The BTC-timestamping protocol requires:
 
 Therefore,
 
-- `MinRandHeightGap` should be > 6,000 to ensure randomness is always available
+- `TimestampingDelayBlocks` should be around 6,000
 - Recommended production value: > 10,000 to provide additional safety margin
 
 ### Determining Start Height
@@ -125,7 +133,7 @@ reasons:
 
 Additionally, given that the end height of a commit equals to
 `startHeight + NumPubRand - 1`, we should ensure that the condition
-`lastCommittedHeight > currentHeight + uint64(MinRandHeightGap)` can hold for
+`lastCommittedHeight > tipHeight + uint64(TimestampingDelayBlocks)` can hold for
 a long period of time to avoid frequent commit of randomness.
 In real life, the value of `NumPubRand` should be much larger than
-`MinRandHeightGap`, e.g., `NumPubRand = 2 * MinRandHeightGap`.
+`TimestampingDelayBlocks`, e.g., `NumPubRand = 2 * TimestampingDelayBlocks`.

eotsmanager/service/server.go

@@ -83,9 +83,7 @@ func (s *Server) RunUntilShutdown() error {
 		return fmt.Errorf("failed to listen on %s: %w", listenAddr, err)
 	}
 	defer func() {
-		if err := lis.Close(); err != nil {
-			s.logger.Error(fmt.Sprintf("Failed to close network listener: %v", err))
-		}
+		_ = lis.Close()
 	}()
 
 	grpcServer := grpc.NewServer()

finality-provider/config/config.go

@@ -27,9 +27,8 @@ const (
 	defaultConfigFileName              = "fpd.conf"
 	defaultNumPubRand                  = 70000 // support running of 1 week with block production time as 10s
 	defaultNumPubRandMax               = 100000
-	defaultMinRandHeightGap            = 35000
+	defaultTimestampingDelayBlocks     = 6000 // 100 BTC blocks * 600s / 10s
 	defaultBatchSubmissionSize         = 1000
-	defaultStatusUpdateInterval        = 20 * time.Second
 	defaultRandomInterval              = 30 * time.Second
 	defaultSubmitRetryInterval         = 1 * time.Second
 	defaultSignatureSubmissionInterval = 1 * time.Second
@@ -57,11 +56,10 @@ type Config struct {
 	ChainType                   string        `long:"chaintype" description:"the type of the consumer chain" choice:"babylon"`
 	NumPubRand                  uint32        `long:"numPubRand" description:"The number of Schnorr public randomness for each commitment"`
 	NumPubRandMax               uint32        `long:"numpubrandmax" description:"The upper bound of the number of Schnorr public randomness for each commitment"`
-	MinRandHeightGap            uint32        `long:"minrandheightgap" description:"The minimum gap between the last committed rand height and the current Babylon block height"`
+	TimestampingDelayBlocks     uint32        `long:"timestampingdelayblocks" description:"The delay, measured in blocks, between a randomness commit submission and the randomness is BTC-timestamped"`
 	MaxSubmissionRetries        uint32        `long:"maxsubmissionretries" description:"The maximum number of retries to submit finality signature or public randomness"`
 	EOTSManagerAddress          string        `long:"eotsmanageraddress" description:"The address of the remote EOTS manager; Empty if the EOTS manager is running locally"`
 	BatchSubmissionSize         uint32        `long:"batchsubmissionsize" description:"The size of a batch in one submission"`
-	StatusUpdateInterval        time.Duration `long:"statusupdateinterval" description:"The interval between each update of finality-provider status"`
 	RandomnessCommitInterval    time.Duration `long:"randomnesscommitinterval" description:"The interval between each attempt to commit public randomness"`
 	SubmissionRetryInterval     time.Duration `long:"submissionretryinterval" description:"The interval between each attempt to submit finality signature or public randomness after a failure"`
 	SignatureSubmissionInterval time.Duration `long:"signaturesubmissioninterval" description:"The interval between each finality signature(s) submission"`
@@ -94,9 +92,8 @@ func DefaultConfigWithHome(homePath string) Config {
 		PollerConfig:                &pollerCfg,
 		NumPubRand:                  defaultNumPubRand,
 		NumPubRandMax:               defaultNumPubRandMax,
-		MinRandHeightGap:            defaultMinRandHeightGap,
+		TimestampingDelayBlocks:     defaultTimestampingDelayBlocks,
 		BatchSubmissionSize:         defaultBatchSubmissionSize,
-		StatusUpdateInterval:        defaultStatusUpdateInterval,
 		RandomnessCommitInterval:    defaultRandomInterval,
 		SubmissionRetryInterval:     defaultSubmitRetryInterval,
 		SignatureSubmissionInterval: defaultSignatureSubmissionInterval,

finality-provider/service/app.go

@@ -7,7 +7,6 @@ import (
 	"sync"
 
 	sdkmath "cosmossdk.io/math"
-	"github.com/avast/retry-go/v4"
 	bbntypes "github.com/babylonlabs-io/babylon/types"
 	bstypes "github.com/babylonlabs-io/babylon/x/btcstaking/types"
 	"github.com/btcsuite/btcd/btcec/v2"
@@ -26,7 +25,6 @@ import (
 	"github.com/babylonlabs-io/finality-provider/finality-provider/store"
 	fpkr "github.com/babylonlabs-io/finality-provider/keyring"
 	"github.com/babylonlabs-io/finality-provider/metrics"
-	"github.com/babylonlabs-io/finality-provider/types"
 )
 
 type FinalityProviderApp struct {
@@ -253,18 +251,14 @@ func (app *FinalityProviderApp) SyncAllFinalityProvidersStatus() error {
 
 			continue
 		}
-		// power == 0 and slashed_height == 0, change to INACTIVE if the current status is ACTIVE
-		if oldStatus == proto.FinalityProviderStatus_ACTIVE {
-			app.fps.MustSetFpStatus(fp.BtcPk, proto.FinalityProviderStatus_INACTIVE)
 
-			app.logger.Debug(
-				"the finality-provider status is changed to INACTIVE",
-				zap.String("fp_btc_pk", pkHex),
-				zap.String("old_status", oldStatus.String()),
-			)
+		app.fps.MustSetFpStatus(fp.BtcPk, proto.FinalityProviderStatus_INACTIVE)
 
-			continue
-		}
+		app.logger.Debug(
+			"the finality-provider status is changed to INACTIVE",
+			zap.String("fp_btc_pk", pkHex),
+			zap.String("old_status", oldStatus.String()),
+		)
 	}
 
 	return nil
@@ -281,10 +275,9 @@ func (app *FinalityProviderApp) Start() error {
 			return
 		}
 
-		app.wg.Add(5)
+		app.wg.Add(4)
 		go app.metricsUpdateLoop()
 		go app.monitorCriticalErr()
-		go app.monitorStatusUpdate()
 		go app.registrationLoop()
 		go app.unjailFpLoop()
 	})
@@ -550,32 +543,6 @@ func (app *FinalityProviderApp) setFinalityProviderJailed(fpi *FinalityProviderI
 	}
 }
 
-func (app *FinalityProviderApp) getLatestBlockWithRetry() (*types.BlockInfo, error) {
-	var (
-		latestBlock *types.BlockInfo
-		err         error
-	)
-
-	if err := retry.Do(func() error {
-		latestBlock, err = app.cc.QueryBestBlock()
-		if err != nil {
-			return err
-		}
-		return nil
-	}, RtyAtt, RtyDel, RtyErr, retry.OnRetry(func(n uint, err error) {
-		app.logger.Debug(
-			"failed to query the consumer chain for the latest block",
-			zap.Uint("attempt", n+1),
-			zap.Uint("max_attempts", RtyAttNum),
-			zap.Error(err),
-		)
-	})); err != nil {
-		return nil, err
-	}
-
-	return latestBlock, nil
-}
-
 // 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, "")

finality-provider/service/app_test.go

@@ -3,21 +3,21 @@ package service_test
 import (
 	"errors"
 	"fmt"
-	btcstakingtypes "github.com/babylonlabs-io/babylon/x/btcstaking/types"
 	"math/rand"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 	"time"
 
+	btcstakingtypes "github.com/babylonlabs-io/babylon/x/btcstaking/types"
+
 	"github.com/babylonlabs-io/babylon/testutil/datagen"
 	bbntypes "github.com/babylonlabs-io/babylon/types"
 	finalitytypes "github.com/babylonlabs-io/babylon/x/finality/types"
 	sdkkeyring "github.com/cosmos/cosmos-sdk/crypto/keyring"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/zap"
 
 	"github.com/babylonlabs-io/finality-provider/clientcontroller"
 	"github.com/babylonlabs-io/finality-provider/eotsmanager"
@@ -44,7 +44,7 @@ func FuzzCreateFinalityProvider(f *testing.F) {
 	f.Fuzz(func(t *testing.T, seed int64) {
 		r := rand.New(rand.NewSource(seed))
 
-		logger := zap.NewNop()
+		logger := testutil.GetTestLogger(t)
 		// create an EOTS manager
 		eotsHomeDir := filepath.Join(t.TempDir(), "eots-home")
 		eotsCfg := eotscfg.DefaultConfigWithHomePath(eotsHomeDir)
@@ -170,7 +170,6 @@ func FuzzSyncFinalityProviderStatus(f *testing.F) {
 		fpHomeDir := filepath.Join(t.TempDir(), "fp-home", pathSuffix)
 		fpCfg := config.DefaultConfigWithHome(fpHomeDir)
 		// no need for other intervals to run
-		fpCfg.StatusUpdateInterval = time.Minute * 10
 		fpCfg.SubmissionRetryInterval = time.Minute * 10
 
 		// Create fp app
@@ -188,7 +187,7 @@ func FuzzSyncFinalityProviderStatus(f *testing.F) {
 			case 1:
 				expectedStatus = proto.FinalityProviderStatus_JAILED
 			case 2:
-				expectedStatus = proto.FinalityProviderStatus_REGISTERED
+				expectedStatus = proto.FinalityProviderStatus_INACTIVE
 			}
 		}
 
@@ -210,7 +209,6 @@ func FuzzUnjailFinalityProvider(f *testing.F) {
 		fpHomeDir := filepath.Join(t.TempDir(), "fp-home", pathSuffix)
 		fpCfg := config.DefaultConfigWithHome(fpHomeDir)
 		// use shorter interval for the test to end faster
-		fpCfg.StatusUpdateInterval = time.Millisecond * 10
 		fpCfg.SubmissionRetryInterval = time.Millisecond * 10
 
 		blkInfo := &types.BlockInfo{Height: currentHeight}
@@ -241,56 +239,12 @@ func FuzzUnjailFinalityProvider(f *testing.F) {
 	})
 }
 
-func FuzzStatusUpdate(f *testing.F) {
-	testutil.AddRandomSeedsToFuzzer(f, 10)
-	f.Fuzz(func(t *testing.T, seed int64) {
-		r := rand.New(rand.NewSource(seed))
-
-		randomStartingHeight := uint64(r.Int63n(100) + 1)
-		currentHeight := randomStartingHeight + uint64(r.Int63n(10)+2)
-		mockClientController := testutil.PrepareMockedClientController(t, r, randomStartingHeight, currentHeight, 0)
-
-		// setup mocks
-		votingPower := uint64(r.Intn(2))
-		mockClientController.EXPECT().QueryFinalityProviderVotingPower(gomock.Any(), currentHeight).Return(votingPower, nil).AnyTimes()
-		mockClientController.EXPECT().Close().Return(nil).AnyTimes()
-		mockClientController.EXPECT().QueryLatestFinalizedBlocks(gomock.Any()).Return(nil, nil).AnyTimes()
-		mockClientController.EXPECT().QueryFinalityProviderHighestVotedHeight(gomock.Any()).Return(uint64(0), nil).AnyTimes()
-		mockClientController.EXPECT().QueryLastCommittedPublicRand(gomock.Any(), uint64(1)).Return(nil, nil).AnyTimes()
-		mockClientController.EXPECT().SubmitFinalitySig(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.TxResponse{TxHash: ""}, nil).AnyTimes()
-		mockClientController.EXPECT().QueryFinalityProviderSlashedOrJailed(gomock.Any()).Return(false, false, nil).AnyTimes()
-
-		// Create randomized config
-		pathSuffix := datagen.GenRandomHexStr(r, 10)
-		fpHomeDir := filepath.Join(t.TempDir(), "fp-home", pathSuffix)
-		fpCfg := config.DefaultConfigWithHome(fpHomeDir)
-		// use shorter interval for the test to end faster
-		fpCfg.StatusUpdateInterval = time.Millisecond * 10
-		fpCfg.SubmissionRetryInterval = time.Millisecond * 10
-
-		// Create fp app
-		app, fpPk, cleanup := startFPAppWithRegisteredFp(t, r, fpHomeDir, &fpCfg, mockClientController)
-		defer cleanup()
-
-		err := app.StartFinalityProvider(fpPk, passphrase)
-		require.NoError(t, err)
-		fpIns, err := app.GetFinalityProviderInstance()
-		require.NoError(t, err)
-
-		if votingPower > 0 {
-			waitForStatus(t, fpIns, proto.FinalityProviderStatus_ACTIVE)
-		} else if fpIns.GetStatus() == proto.FinalityProviderStatus_ACTIVE {
-			waitForStatus(t, fpIns, proto.FinalityProviderStatus_INACTIVE)
-		}
-	})
-}
-
 func FuzzSaveAlreadyRegisteredFinalityProvider(f *testing.F) {
 	testutil.AddRandomSeedsToFuzzer(f, 10)
 	f.Fuzz(func(t *testing.T, seed int64) {
 		r := rand.New(rand.NewSource(seed))
 
-		logger := zap.NewNop()
+		logger := testutil.GetTestLogger(t)
 		// create an EOTS manager
 		eotsHomeDir := filepath.Join(t.TempDir(), "eots-home")
 		eotsCfg := eotscfg.DefaultConfigWithHomePath(eotsHomeDir)
@@ -375,15 +329,8 @@ func FuzzSaveAlreadyRegisteredFinalityProvider(f *testing.F) {
 	})
 }
 
-func waitForStatus(t *testing.T, fpIns *service.FinalityProviderInstance, s proto.FinalityProviderStatus) {
-	require.Eventually(t,
-		func() bool {
-			return fpIns.GetStatus() == s
-		}, eventuallyWaitTimeOut, eventuallyPollTime)
-}
-
 func startFPAppWithRegisteredFp(t *testing.T, r *rand.Rand, homePath string, cfg *config.Config, cc clientcontroller.ClientController) (*service.FinalityProviderApp, *bbntypes.BIP340PubKey, func()) {
-	logger := zap.NewNop()
+	logger := testutil.GetTestLogger(t)
 	// create an EOTS manager
 	eotsHomeDir := filepath.Join(t.TempDir(), "eots-home")
 	eotsCfg := eotscfg.DefaultConfigWithHomePath(eotsHomeDir)

finality-provider/service/benchmark_helper.go

@@ -2,9 +2,11 @@ package service
 
 import (
 	"fmt"
-	"github.com/babylonlabs-io/finality-provider/types"
-	"go.uber.org/zap"
 	"time"
+
+	"go.uber.org/zap"
+
+	"github.com/babylonlabs-io/finality-provider/types"
 )
 
 // CommitPubRandTiming - helper struct used to capture times for benchmark
@@ -26,7 +28,7 @@ func (fp *FinalityProviderInstance) HelperCommitPubRand(tipHeight uint64) (*type
 	case lastCommittedHeight == uint64(0):
 		// the finality-provider has never submitted public rand before
 		startHeight = tipHeight + 1
-	case lastCommittedHeight < uint64(fp.cfg.MinRandHeightGap)+tipHeight:
+	case lastCommittedHeight < uint64(fp.cfg.TimestampingDelayBlocks)+tipHeight:
 		// (should not use subtraction because they are in the type of uint64)
 		// we are running out of the randomness
 		startHeight = lastCommittedHeight + 1