backport: fix updating pub rand commit start height (#108)

Co-authored-by: Rafael Tenfen <[email protected]>

CHANGELOG.md

@@ -37,6 +37,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## Unreleased
 
+### Improvements
+
+* [#104](https://github.com/babylonlabs-io/finality-provider/pull/104) Print fpd binary version
+
+### Bug Fixes
+
+* [#107](https://github.com/babylonlabs-io/finality-provider/pull/107) Fix commit
+start height when the finality activation height is higher than the current
+block tip
+
 ## v0.9.0
 
 ### Improvements

finality-provider/cmd/fpd/daemon/daemon_commands.go

@@ -5,20 +5,21 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"github.com/babylonlabs-io/finality-provider/finality-provider/proto"
 	"strconv"
 
 	"cosmossdk.io/math"
 	"github.com/babylonlabs-io/babylon/types"
 	bbntypes "github.com/babylonlabs-io/babylon/types"
-	fpcmd "github.com/babylonlabs-io/finality-provider/finality-provider/cmd"
-	fpcfg "github.com/babylonlabs-io/finality-provider/finality-provider/config"
-	dc "github.com/babylonlabs-io/finality-provider/finality-provider/service/client"
 	"github.com/cosmos/cosmos-sdk/client"
 	sdkflags "github.com/cosmos/cosmos-sdk/client/flags"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+
+	fpcmd "github.com/babylonlabs-io/finality-provider/finality-provider/cmd"
+	fpcfg "github.com/babylonlabs-io/finality-provider/finality-provider/config"
+	"github.com/babylonlabs-io/finality-provider/finality-provider/proto"
+	dc "github.com/babylonlabs-io/finality-provider/finality-provider/service/client"
 )
 
 var (

finality-provider/service/chain_poller.go

@@ -205,18 +205,17 @@ func (cp *ChainPoller) validateStartHeight(startHeight uint64) error {
 func (cp *ChainPoller) waitForActivation() {
 	// ensure that the startHeight is no lower than the activated height
 	for {
-		activatedHeight, err := cp.cc.QueryActivatedHeight()
-		if err != nil {
-			cp.logger.Debug("failed to query the consumer chain for the activated height", zap.Error(err))
-		} else {
-			if cp.nextHeight < activatedHeight {
-				cp.nextHeight = activatedHeight
-			}
-			return
-		}
-
 		select {
 		case <-time.After(cp.cfg.PollInterval):
+			activatedHeight, err := cp.cc.QueryActivatedHeight()
+			if err != nil {
+				cp.logger.Debug("failed to query the consumer chain for the activated height", zap.Error(err))
+			} else {
+				if cp.nextHeight < activatedHeight {
+					cp.nextHeight = activatedHeight
+				}
+				return
+			}
 
 		case <-cp.quit:
 			return
@@ -233,41 +232,39 @@ func (cp *ChainPoller) pollChain() {
 	var failedCycles uint32
 
 	for {
-		// TODO: Handlig of request cancellation, as otherwise shutdown will be blocked
-		// until request is finished
-		blockToRetrieve := cp.nextHeight
-		block, err := cp.blockWithRetry(blockToRetrieve)
-		if err != nil {
-			failedCycles++
-			cp.logger.Debug(
-				"failed to query the consumer chain for the block",
-				zap.Uint32("current_failures", failedCycles),
-				zap.Uint64("block_to_retrieve", blockToRetrieve),
-				zap.Error(err),
-			)
-		} else {
-			// no error and we got the header we wanted to get, bump the state and push
-			// notification about data
-			cp.nextHeight = blockToRetrieve + 1
-			failedCycles = 0
-			cp.metrics.RecordLastPolledHeight(block.Height)
-
-			cp.logger.Info("the poller retrieved the block from the consumer chain",
-				zap.Uint64("height", block.Height))
-
-			// push the data to the channel
-			// Note: if the consumer is too slow -- the buffer is full
-			// the channel will block, and we will stop retrieving data from the node
-			cp.blockInfoChan <- block
-		}
-
-		if failedCycles > maxFailedCycles {
-			cp.logger.Fatal("the poller has reached the max failed cycles, exiting")
-		}
-
 		select {
 		case <-time.After(cp.cfg.PollInterval):
+			// TODO: Handlig of request cancellation, as otherwise shutdown will be blocked
+			// until request is finished
+			blockToRetrieve := cp.nextHeight
+			block, err := cp.blockWithRetry(blockToRetrieve)
+			if err != nil {
+				failedCycles++
+				cp.logger.Debug(
+					"failed to query the consumer chain for the block",
+					zap.Uint32("current_failures", failedCycles),
+					zap.Uint64("block_to_retrieve", blockToRetrieve),
+					zap.Error(err),
+				)
+			} else {
+				// no error and we got the header we wanted to get, bump the state and push
+				// notification about data
+				cp.nextHeight = blockToRetrieve + 1
+				failedCycles = 0
+				cp.metrics.RecordLastPolledHeight(block.Height)
+
+				cp.logger.Info("the poller retrieved the block from the consumer chain",
+					zap.Uint64("height", block.Height))
+
+				// push the data to the channel
+				// Note: if the consumer is too slow -- the buffer is full
+				// the channel will block, and we will stop retrieving data from the node
+				cp.blockInfoChan <- block
+			}
 
+			if failedCycles > maxFailedCycles {
+				cp.logger.Fatal("the poller has reached the max failed cycles, exiting")
+			}
 		case req := <-cp.skipHeightChan:
 			// no need to skip heights if the target height is not higher
 			// than the next height to retrieve

finality-provider/service/client/rpcclient.go

@@ -25,6 +25,9 @@ func NewFinalityProviderServiceGRpcClient(remoteAddr string) (client *FinalityPr
 	}
 
 	cleanUp = func() error {
+		if conn == nil {
+			return nil
+		}
 		return conn.Close()
 	}
 

finality-provider/service/fp_instance.go

@@ -465,36 +465,37 @@ func (fp *FinalityProviderInstance) retrySubmitFinalitySignatureUntilBlockFinali
 	// we break the for loop if the block is finalized or the signature is successfully submitted
 	// error will be returned if maximum retries have been reached or the query to the consumer chain fails
 	for {
-		// error will be returned if max retries have been reached
-		res, err := fp.SubmitFinalitySignature(targetBlock)
-		if err != nil {
+		select {
+		case <-time.After(fp.cfg.SubmissionRetryInterval):
+			// error will be returned if max retries have been reached
+			res, err := fp.SubmitFinalitySignature(targetBlock)
+			if err != nil {
 
-			fp.logger.Debug(
-				"failed to submit finality signature to the consumer chain",
-				zap.String("pk", fp.GetBtcPkHex()),
-				zap.Uint32("current_failures", failedCycles),
-				zap.Uint64("target_block_height", targetBlock.Height),
-				zap.Error(err),
-			)
+				fp.logger.Debug(
+					"failed to submit finality signature to the consumer chain",
+					zap.String("pk", fp.GetBtcPkHex()),
+					zap.Uint32("current_failures", failedCycles),
+					zap.Uint64("target_block_height", targetBlock.Height),
+					zap.Error(err),
+				)
 
-			if clientcontroller.IsUnrecoverable(err) {
-				return nil, err
-			}
+				if clientcontroller.IsUnrecoverable(err) {
+					return nil, err
+				}
 
-			if clientcontroller.IsExpected(err) {
-				return nil, nil
-			}
+				if clientcontroller.IsExpected(err) {
+					return nil, nil
+				}
 
-			failedCycles += 1
-			if failedCycles > fp.cfg.MaxSubmissionRetries {
-				return nil, fmt.Errorf("reached max failed cycles with err: %w", err)
+				failedCycles += 1
+				if failedCycles > fp.cfg.MaxSubmissionRetries {
+					return nil, fmt.Errorf("reached max failed cycles with err: %w", err)
+				}
+			} else {
+				// the signature has been successfully submitted
+				return res, nil
 			}
-		} else {
-			// the signature has been successfully submitted
-			return res, nil
-		}
-		select {
-		case <-time.After(fp.cfg.SubmissionRetryInterval):
+
 			// periodically query the index block to be later checked whether it is Finalized
 			finalized, err := fp.checkBlockFinalization(targetBlock.Height)
 			if err != nil {
@@ -620,11 +621,14 @@ func (fp *FinalityProviderInstance) CommitPubRand(tipHeight uint64) (*types.TxRe
 		return nil, err
 	}
 
+	// make sure that the start height is at least the finality activation height
+	// and updated to generate the list with the same as the commited height.
+	startHeight = fppath.MaxUint64(startHeight, activationBlkHeight)
 	// generate a list of Schnorr randomness pairs
 	// NOTE: currently, calling this will create and save a list of randomness
 	// in case of failure, randomness that has been created will be overwritten
 	// for safety reason as the same randomness must not be used twice
-	pubRandList, err := fp.getPubRandList(fppath.MaxUint64(startHeight, activationBlkHeight), fp.cfg.NumPubRand)
+	pubRandList, err := fp.getPubRandList(startHeight, fp.cfg.NumPubRand)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate randomness: %w", err)
 	}

finality-provider/service/rpcserver.go

@@ -2,15 +2,17 @@ package service
 
 import (
 	"context"
+	"errors"
+	"fmt"
+	"sync"
+	"sync/atomic"
+
 	"cosmossdk.io/math"
 	sdkmath "cosmossdk.io/math"
-	"fmt"
 	bbntypes "github.com/babylonlabs-io/babylon/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	"google.golang.org/grpc"
 	protobuf "google.golang.org/protobuf/proto"
-	"sync"
-	"sync/atomic"
 
 	"github.com/babylonlabs-io/finality-provider/finality-provider/proto"
 	"github.com/babylonlabs-io/finality-provider/types"
@@ -140,64 +142,77 @@ func (r *rpcServer) RegisterFinalityProvider(ctx context.Context, req *proto.Reg
 // AddFinalitySignature adds a manually constructed finality signature to Babylon
 // NOTE: this is only used for presentation/testing purposes
 func (r *rpcServer) AddFinalitySignature(ctx context.Context, req *proto.AddFinalitySignatureRequest) (
-	*proto.AddFinalitySignatureResponse, error) {
-
-	fpPk, err := bbntypes.NewBIP340PubKeyFromHex(req.BtcPk)
-	if err != nil {
-		return nil, err
-	}
-
-	fpi, err := r.app.GetFinalityProviderInstance()
-	if err != nil {
-		return nil, err
-	}
-
-	if fpi.GetBtcPkHex() != req.BtcPk {
-		return nil, fmt.Errorf(
-			"the finality provider running does not match the request, got: %s, expected: %s",
-			req.BtcPk, fpi.GetBtcPkHex())
-	}
+	res *proto.AddFinalitySignatureResponse,
+	err error,
+) {
+	r.app.wg.Add(1)
+	defer r.app.wg.Done()
+
+	select {
+	case <-r.app.quit:
+		r.app.logger.Info("exiting metrics update loop")
+		return res, nil
+	default:
+		fpPk, err := bbntypes.NewBIP340PubKeyFromHex(req.BtcPk)
+		if err != nil {
+			return nil, err
+		}
 
-	b := &types.BlockInfo{
-		Height: req.Height,
-		Hash:   req.AppHash,
-	}
+		fpi, err := r.app.GetFinalityProviderInstance()
+		if err != nil {
+			return nil, err
+		}
 
-	txRes, privKey, err := fpi.TestSubmitFinalitySignatureAndExtractPrivKey(b)
-	if err != nil {
-		return nil, err
-	}
+		if fpi.GetBtcPkHex() != req.BtcPk {
+			errMsg := fmt.Sprintf("the finality provider running does not match the request, got: %s, expected: %s",
+				req.BtcPk, fpi.GetBtcPkHex())
+			return nil, errors.New(errMsg)
+		}
 
-	res := &proto.AddFinalitySignatureResponse{TxHash: txRes.TxHash}
+		b := &types.BlockInfo{
+			Height: req.Height,
+			Hash:   req.AppHash,
+		}
 
-	// if privKey is not empty, then this BTC finality-provider
-	// has voted for a fork and will be slashed
-	if privKey != nil {
-		localPrivKey, err := r.app.getFpPrivKey(fpPk.MustMarshal())
-		res.ExtractedSkHex = privKey.Key.String()
+		txRes, privKey, err := fpi.TestSubmitFinalitySignatureAndExtractPrivKey(b)
 		if err != nil {
 			return nil, err
 		}
-		localSkHex := localPrivKey.Key.String()
-		localSkNegateHex := localPrivKey.Key.Negate().String()
-		if res.ExtractedSkHex == localSkHex {
-			res.LocalSkHex = localSkHex
-		} else if res.ExtractedSkHex == localSkNegateHex {
-			res.LocalSkHex = localSkNegateHex
-		} else {
-			return nil, fmt.Errorf("the finality-provider's BTC private key is extracted but does not match the local key,"+
-				"extrated: %s, local: %s, local-negated: %s",
-				res.ExtractedSkHex, localSkHex, localSkNegateHex)
+
+		res = &proto.AddFinalitySignatureResponse{TxHash: txRes.TxHash}
+
+		// if privKey is not empty, then this BTC finality-provider
+		// has voted for a fork and will be slashed
+		if privKey != nil {
+			localPrivKey, err := r.app.getFpPrivKey(fpPk.MustMarshal())
+			if err != nil {
+				r.app.logger.Error(fmt.Sprintf("err get priv key %s", err.Error()))
+				return nil, err
+			}
+
+			res.ExtractedSkHex = privKey.Key.String()
+			localSkHex := localPrivKey.Key.String()
+			localSkNegateHex := localPrivKey.Key.Negate().String()
+			if res.ExtractedSkHex == localSkHex {
+				res.LocalSkHex = localSkHex
+			} else if res.ExtractedSkHex == localSkNegateHex {
+				res.LocalSkHex = localSkNegateHex
+			} else {
+				msg := fmt.Sprintf(
+					"the finality-provider's BTC private key is extracted but does not match the local key,"+
+						"extrated: %s, local: %s, local-negated: %s",
+					res.ExtractedSkHex, localSkHex, localSkNegateHex,
+				)
+				return nil, errors.New(msg)
+			}
 		}
+		return res, nil
 	}
-
-	return res, nil
 }
 
 // UnjailFinalityProvider unjails a finality-provider
 func (r *rpcServer) UnjailFinalityProvider(ctx context.Context, req *proto.UnjailFinalityProviderRequest) (
 	*proto.UnjailFinalityProviderResponse, error) {
-
 	fpPk, err := bbntypes.NewBIP340PubKeyFromHex(req.BtcPk)
 	if err != nil {
 		return nil, err

log/log.go

@@ -52,11 +52,13 @@ func NewRootLogger(format string, level string, w io.Writer) (*zap.Logger, error
 		return nil, fmt.Errorf("unsupported log level: %s", level)
 	}
 
-	return zap.New(zapcore.NewCore(
-		enc,
-		zapcore.AddSync(w),
-		lvl,
-	)), nil
+	return zap.New(
+		zapcore.NewCore(
+			enc,
+			zapcore.AddSync(w),
+			lvl,
+		),
+	), nil
 }
 
 func NewRootLoggerWithFile(logFile string, level string) (*zap.Logger, error) {