diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd23d116..2e0b9167 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,14 +7,19 @@ on: jobs: lint_test: - uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.1.0 + uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.6.0 with: run-unit-tests: true run-integration-tests: true run-lint: true + run-build: true + run-gosec: true + gosec-args: "-exclude-generated -exclude-dir=itest -exclude-dir=testutil ./..." docker_pipeline: - uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.1.0 + uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.6.0 secrets: inherit with: publish: false + dockerfile: ./Dockerfile + repoName: finality-provider diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d8e3d1bb..f5568d5a 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -9,15 +9,20 @@ on: jobs: lint_test: - uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.1.0 + uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.6.0 with: run-unit-tests: true run-integration-tests: true run-lint: true + run-build: true + run-gosec: true + gosec-args: "-exclude-generated -exclude-dir=itest -exclude-dir=testutil ./..." docker_pipeline: needs: ["lint_test"] - uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.1.0 + uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.6.0 secrets: inherit with: publish: true + dockerfile: ./Dockerfile + repoName: finality-provider diff --git a/clientcontroller/babylon.go b/clientcontroller/babylon.go index 44e48bbb..d1f545d7 100644 --- a/clientcontroller/babylon.go +++ b/clientcontroller/babylon.go @@ -6,7 +6,7 @@ import ( "time" sdkErr "cosmossdk.io/errors" - "cosmossdk.io/math" + sdkmath "cosmossdk.io/math" bbnclient "github.com/babylonlabs-io/babylon/client/client" bbntypes "github.com/babylonlabs-io/babylon/types" btcctypes "github.com/babylonlabs-io/babylon/x/btccheckpoint/types" @@ -55,7 +55,7 @@ func NewBabylonController( return nil, fmt.Errorf("failed to create Babylon client: %w", err) } - // makes sure that the key in config really exists and it is a valid bech 32 addr + // makes sure that the key in config really exists and is a valid bech32 addr // to allow using mustGetTxSigner if _, err := bc.GetAddr(); err != nil { return nil, err @@ -112,7 +112,7 @@ func (bc *BabylonController) reliablySendMsgs(msgs []sdk.Msg, expectedErrs []*sd func (bc *BabylonController) RegisterFinalityProvider( fpPk *btcec.PublicKey, pop []byte, - commission *math.LegacyDec, + commission *sdkmath.LegacyDec, description []byte, ) (*types.TxResponse, error) { var bbnPop btcstakingtypes.ProofOfPossessionBTC @@ -323,13 +323,13 @@ func (bc *BabylonController) QueryLastCommittedPublicRand(fpPk *btcec.PublicKey, return res.PubRandCommitMap, nil } -func (bc *BabylonController) QueryBlocks(startHeight, endHeight, limit uint64) ([]*types.BlockInfo, error) { +func (bc *BabylonController) QueryBlocks(startHeight, endHeight uint64, limit uint32) ([]*types.BlockInfo, error) { if endHeight < startHeight { return nil, fmt.Errorf("the startHeight %v should not be higher than the endHeight %v", startHeight, endHeight) } count := endHeight - startHeight + 1 - if count > limit { - count = limit + if count > uint64(limit) { + count = uint64(limit) } return bc.queryLatestBlocks(sdk.Uint64ToBigEndian(startHeight), count, finalitytypes.QueriedBlockStatus_ANY, false) } @@ -405,10 +405,14 @@ func (bc *BabylonController) queryCometBestBlock() (*types.BlockInfo, error) { return nil, err } + headerHeightInt64 := chainInfo.BlockMetas[0].Header.Height + if headerHeightInt64 < 0 { + return nil, fmt.Errorf("block height %v should be positive", headerHeightInt64) + } // Returning response directly, if header with specified number did not exist // at request will contain nil header return &types.BlockInfo{ - Height: uint64(chainInfo.BlockMetas[0].Header.Height), + Height: uint64(headerHeightInt64), Hash: chainInfo.BlockMetas[0].Header.AppHash, }, nil } diff --git a/clientcontroller/interface.go b/clientcontroller/interface.go index 3b84a2aa..366c860a 100644 --- a/clientcontroller/interface.go +++ b/clientcontroller/interface.go @@ -59,7 +59,7 @@ type ClientController interface { QueryBlock(height uint64) (*types.BlockInfo, error) // QueryBlocks returns a list of blocks from startHeight to endHeight - QueryBlocks(startHeight, endHeight, limit uint64) ([]*types.BlockInfo, error) + QueryBlocks(startHeight, endHeight uint64, limit uint32) ([]*types.BlockInfo, error) // QueryBestBlock queries the tip block of the consumer chain QueryBestBlock() (*types.BlockInfo, error) diff --git a/eotsmanager/cmd/eotsd/daemon/sign.go b/eotsmanager/cmd/eotsd/daemon/sign.go index 04aca0b5..c2404c75 100644 --- a/eotsmanager/cmd/eotsd/daemon/sign.go +++ b/eotsmanager/cmd/eotsd/daemon/sign.go @@ -186,6 +186,7 @@ func SignSchnorr(ctx *cli.Context) error { func hashFromFile(inputFilePath string) ([]byte, error) { h := sha256.New() + // #nosec G304 - The log file path is provided by the user and not externally f, err := os.Open(inputFilePath) if err != nil { return nil, fmt.Errorf("failed to open the file %s: %w", inputFilePath, err) diff --git a/eotsmanager/service/server.go b/eotsmanager/service/server.go index f566bf65..450b451f 100644 --- a/eotsmanager/service/server.go +++ b/eotsmanager/service/server.go @@ -66,8 +66,11 @@ func (s *Server) RunUntilShutdown() error { defer func() { s.logger.Info("Closing database...") - s.db.Close() - s.logger.Info("Database closed") + if err := s.db.Close(); err != nil { + s.logger.Error(fmt.Sprintf("Failed to close database: %v", err)) // Log the error + } else { + s.logger.Info("Database closed") + } metricsServer.Stop(context.Background()) s.logger.Info("Metrics server stopped") }() @@ -79,7 +82,11 @@ func (s *Server) RunUntilShutdown() error { if err != nil { return fmt.Errorf("failed to listen on %s: %w", listenAddr, err) } - defer lis.Close() + defer func() { + if err := lis.Close(); err != nil { + s.logger.Error(fmt.Sprintf("Failed to close network listener: %v", err)) + } + }() grpcServer := grpc.NewServer() defer grpcServer.Stop() diff --git a/finality-provider/cmd/fpd/daemon/daemon_commands.go b/finality-provider/cmd/fpd/daemon/daemon_commands.go index 30501050..1eaac417 100644 --- a/finality-provider/cmd/fpd/daemon/daemon_commands.go +++ b/finality-provider/cmd/fpd/daemon/daemon_commands.go @@ -50,7 +50,11 @@ func runCommandGetDaemonInfo(cmd *cobra.Command, args []string) error { if err != nil { return err } - defer cleanUp() + defer func() { + if err := cleanUp(); err != nil { + fmt.Printf("Failed to clean up grpc client: %v\n", err) + } + }() info, err := client.GetInfo(context.Background()) if err != nil { @@ -126,7 +130,11 @@ func runCommandCreateFP(ctx client.Context, cmd *cobra.Command, _ []string) erro if err != nil { return err } - defer cleanUp() + defer func() { + if err := cleanUp(); err != nil { + fmt.Printf("Failed to clean up grpc client: %v\n", err) + } + }() chainId, err := flags.GetString(chainIdFlag) if err != nil { @@ -202,7 +210,11 @@ func runCommandUnjailFP(_ client.Context, cmd *cobra.Command, args []string) err if err != nil { return err } - defer cleanUp() + defer func() { + if err := cleanUp(); err != nil { + fmt.Printf("Failed to clean up grpc client: %v\n", err) + } + }() _, err = client.UnjailFinalityProvider(context.Background(), args[0]) if err != nil { @@ -263,7 +275,11 @@ func runCommandLsFP(cmd *cobra.Command, args []string) error { if err != nil { return err } - defer cleanUp() + defer func() { + if err := cleanUp(); err != nil { + fmt.Printf("Failed to clean up grpc client: %v\n", err) + } + }() resp, err := client.QueryFinalityProviderList(context.Background()) if err != nil { @@ -303,7 +319,11 @@ func runCommandInfoFP(cmd *cobra.Command, args []string) error { if err != nil { return err } - defer cleanUp() + defer func() { + if err := cleanUp(); err != nil { + fmt.Printf("Failed to clean up grpc client: %v\n", err) + } + }() resp, err := client.QueryFinalityProviderInfo(context.Background(), fpPk) if err != nil { @@ -346,7 +366,11 @@ func runCommandRegisterFP(cmd *cobra.Command, args []string) error { if err != nil { return err } - defer cleanUp() + defer func() { + if err := cleanUp(); err != nil { + fmt.Printf("Failed to clean up grpc client: %v\n", err) + } + }() passphrase, err := flags.GetString(passphraseFlag) if err != nil { @@ -402,7 +426,11 @@ func runCommandAddFinalitySig(cmd *cobra.Command, args []string) error { if err != nil { return err } - defer cleanUp() + defer func() { + if err := cleanUp(); err != nil { + fmt.Printf("Failed to clean up grpc client: %v\n", err) + } + }() appHash, err := hex.DecodeString(appHashHex) if err != nil { diff --git a/finality-provider/cmd/fpd/daemon/export.go b/finality-provider/cmd/fpd/daemon/export.go index 8686628b..19890724 100644 --- a/finality-provider/cmd/fpd/daemon/export.go +++ b/finality-provider/cmd/fpd/daemon/export.go @@ -64,7 +64,11 @@ func runCommandExportFP(ctx client.Context, cmd *cobra.Command, args []string) e if err != nil { return fmt.Errorf("failled to connect to daemon addr %s: %w", daemonAddress, err) } - defer cleanUp() + defer func() { + if err := cleanUp(); err != nil { + fmt.Printf("Failed to clean up grpc client: %v\n", err) + } + }() fpBtcPkHex := args[0] fpPk, err := bbn.NewBIP340PubKeyFromHex(fpBtcPkHex) diff --git a/finality-provider/config/config.go b/finality-provider/config/config.go index d8a8b423..80dc9866 100644 --- a/finality-provider/config/config.go +++ b/finality-provider/config/config.go @@ -58,15 +58,15 @@ type Config struct { LogLevel string `long:"loglevel" description:"Logging level for all subsystems" choice:"trace" choice:"debug" choice:"info" choice:"warn" choice:"error" choice:"fatal"` // ChainName and ChainID (if any) of the chain config identify a consumer chain ChainName string `long:"chainname" description:"the name of the consumer chain" choice:"babylon"` - NumPubRand uint64 `long:"numPubRand" description:"The number of Schnorr public randomness for each commitment"` - NumPubRandMax uint64 `long:"numpubrandmax" description:"The upper bound of the number of Schnorr public randomness for each commitment"` - MinRandHeightGap uint64 `long:"minrandheightgap" description:"The minimum gap between the last committed rand height and the current Babylon block height"` + 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"` 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"` - MaxSubmissionRetries uint64 `long:"maxsubmissionretries" description:"The maximum number of retries to submit finality signature or public randomness"` + MaxSubmissionRetries uint32 `long:"maxsubmissionretries" description:"The maximum number of retries to submit finality signature or public randomness"` FastSyncInterval time.Duration `long:"fastsyncinterval" description:"The interval between each try of fast sync, which is disabled if the value is 0"` - FastSyncLimit uint64 `long:"fastsynclimit" description:"The maximum number of blocks to catch up for each fast sync"` + FastSyncLimit uint32 `long:"fastsynclimit" description:"The maximum number of blocks to catch up for each fast sync"` FastSyncGap uint64 `long:"fastsyncgap" description:"The block gap that will trigger the fast sync"` EOTSManagerAddress string `long:"eotsmanageraddress" description:"The address of the remote EOTS manager; Empty if the EOTS manager is running locally"` MaxNumFinalityProviders uint32 `long:"maxnumfinalityproviders" description:"The maximum number of finality-provider instances running concurrently within the daemon"` diff --git a/finality-provider/service/client/rpcclient.go b/finality-provider/service/client/rpcclient.go index 833ec5bb..eee41673 100644 --- a/finality-provider/service/client/rpcclient.go +++ b/finality-provider/service/client/rpcclient.go @@ -18,14 +18,14 @@ type FinalityProviderServiceGRpcClient struct { } // NewFinalityProviderServiceGRpcClient creates a new GRPC connection with finality provider daemon. -func NewFinalityProviderServiceGRpcClient(remoteAddr string) (client *FinalityProviderServiceGRpcClient, cleanUp func(), err error) { +func NewFinalityProviderServiceGRpcClient(remoteAddr string) (client *FinalityProviderServiceGRpcClient, cleanUp func() error, err error) { conn, err := grpc.Dial(remoteAddr, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { return nil, nil, fmt.Errorf("failed to build gRPC connection to %s: %w", remoteAddr, err) } - cleanUp = func() { - conn.Close() + cleanUp = func() error { + return conn.Close() } return &FinalityProviderServiceGRpcClient{ diff --git a/finality-provider/service/eots_manager_adapter.go b/finality-provider/service/eots_manager_adapter.go index 6b9ef569..ed97f554 100644 --- a/finality-provider/service/eots_manager_adapter.go +++ b/finality-provider/service/eots_manager_adapter.go @@ -11,12 +11,12 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -func (fp *FinalityProviderInstance) getPubRandList(startHeight uint64, numPubRand uint64) ([]*btcec.FieldVal, error) { +func (fp *FinalityProviderInstance) getPubRandList(startHeight uint64, numPubRand uint32) ([]*btcec.FieldVal, error) { pubRandList, err := fp.em.CreateRandomnessPairList( fp.btcPk.MustMarshal(), fp.GetChainID(), startHeight, - uint32(numPubRand), + numPubRand, fp.passphrase, ) if err != nil { diff --git a/finality-provider/service/fastsync_test.go b/finality-provider/service/fastsync_test.go index 240c1fac..af2207b4 100644 --- a/finality-provider/service/fastsync_test.go +++ b/finality-provider/service/fastsync_test.go @@ -52,7 +52,7 @@ func FuzzFastSync_SufficientRandomness(f *testing.F) { expectedTxHash := testutil.GenRandomHexStr(r, 32) finalizedBlock := &types.BlockInfo{Height: finalizedHeight, Hash: testutil.GenRandomByteArray(r, 32)} mockClientController.EXPECT().QueryLatestFinalizedBlocks(uint64(1)).Return([]*types.BlockInfo{finalizedBlock}, nil).AnyTimes() - mockClientController.EXPECT().QueryBlocks(finalizedHeight+1, currentHeight, uint64(10)). + mockClientController.EXPECT().QueryBlocks(finalizedHeight+1, currentHeight, uint32(10)). Return(catchUpBlocks, nil) mockClientController.EXPECT().SubmitBatchFinalitySigs(fpIns.GetBtcPk(), catchUpBlocks, gomock.Any(), gomock.Any(), gomock.Any()). Return(&types.TxResponse{TxHash: expectedTxHash}, nil).AnyTimes() @@ -110,7 +110,7 @@ func FuzzFastSync_NoRandomness(f *testing.F) { expectedTxHash := testutil.GenRandomHexStr(r, 32) finalizedBlock := &types.BlockInfo{Height: finalizedHeight, Hash: testutil.GenRandomByteArray(r, 32)} mockClientController.EXPECT().QueryLatestFinalizedBlocks(uint64(1)).Return([]*types.BlockInfo{finalizedBlock}, nil).AnyTimes() - mockClientController.EXPECT().QueryBlocks(finalizedHeight+1, currentHeight, uint64(10)). + mockClientController.EXPECT().QueryBlocks(finalizedHeight+1, currentHeight, uint32(10)). Return(catchUpBlocks, nil) mockClientController.EXPECT().SubmitBatchFinalitySigs(fpIns.GetBtcPk(), catchUpBlocks[:lastHeightWithPubRand-finalizedHeight], gomock.Any(), gomock.Any(), gomock.Any()). Return(&types.TxResponse{TxHash: expectedTxHash}, nil).AnyTimes() diff --git a/finality-provider/service/fp_instance.go b/finality-provider/service/fp_instance.go index d643ee8a..03676c50 100644 --- a/finality-provider/service/fp_instance.go +++ b/finality-provider/service/fp_instance.go @@ -3,6 +3,7 @@ package service import ( "errors" "fmt" + "math" "strings" "sync" "time" @@ -468,7 +469,7 @@ func (fp *FinalityProviderInstance) retrySubmitFinalitySignatureUntilBlockFinali } failedCycles += 1 - if failedCycles > uint32(fp.cfg.MaxSubmissionRetries) { + if failedCycles > fp.cfg.MaxSubmissionRetries { return nil, fmt.Errorf("reached max failed cycles with err: %w", err) } } else { @@ -538,7 +539,7 @@ func (fp *FinalityProviderInstance) retryCommitPubRandUntilBlockFinalized(target ) failedCycles += 1 - if failedCycles > uint32(fp.cfg.MaxSubmissionRetries) { + if failedCycles > fp.cfg.MaxSubmissionRetries { return nil, fmt.Errorf("reached max failed cycles with err: %w", err) } } else { @@ -583,7 +584,7 @@ func (fp *FinalityProviderInstance) CommitPubRand(tipHeight uint64) (*types.TxRe if lastCommittedHeight == uint64(0) { // the finality-provider has never submitted public rand before startHeight = tipHeight + 1 - } else if lastCommittedHeight < fp.cfg.MinRandHeightGap+tipHeight { + } else if lastCommittedHeight < uint64(fp.cfg.MinRandHeightGap)+tipHeight { // (should not use subtraction because they are in the type of uint64) // we are running out of the randomness startHeight = lastCommittedHeight + 1 @@ -683,8 +684,13 @@ func (fp *FinalityProviderInstance) SubmitBatchFinalitySignatures(blocks []*type return nil, fmt.Errorf("should not submit batch finality signature with zero block") } + if len(blocks) > math.MaxUint32 { + return nil, fmt.Errorf("should not submit batch finality signature with too many blocks") + } + // get public randomness list - prList, err := fp.getPubRandList(blocks[0].Height, uint64(len(blocks))) + // #nosec G115 -- performed the conversion check above + prList, err := fp.getPubRandList(blocks[0].Height, uint32(len(blocks))) if err != nil { return nil, fmt.Errorf("failed to get public randomness list: %v", err) } diff --git a/finality-provider/service/server.go b/finality-provider/service/server.go index c6eb2743..27c510e0 100644 --- a/finality-provider/service/server.go +++ b/finality-provider/service/server.go @@ -64,8 +64,11 @@ func (s *Server) RunUntilShutdown() error { defer func() { s.logger.Info("Closing database...") - s.db.Close() - s.logger.Info("Database closed") + if err := s.db.Close(); err != nil { + s.logger.Error(fmt.Sprintf("Failed to close database: %v", err)) // Log the error + } else { + s.logger.Info("Database closed") + } metricsServer.Stop(context.Background()) s.logger.Info("Metrics server stopped") }() @@ -77,7 +80,11 @@ func (s *Server) RunUntilShutdown() error { if err != nil { return fmt.Errorf("failed to listen on %s: %w", listenAddr, err) } - defer lis.Close() + defer func() { + if err := lis.Close(); err != nil { + s.logger.Error(fmt.Sprintf("Failed to close network listener: %v", err)) + } + }() grpcServer := grpc.NewServer() defer grpcServer.Stop() diff --git a/log/log.go b/log/log.go index f81f79e9..9bd5f31a 100644 --- a/log/log.go +++ b/log/log.go @@ -63,7 +63,8 @@ func NewRootLoggerWithFile(logFile string, level string) (*zap.Logger, error) { if err := util.MakeDirectory(filepath.Dir(logFile)); err != nil { return nil, err } - f, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0660) + // #nosec G304 - The log file path is provided by the user and not externally + f, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) if err != nil { return nil, err } diff --git a/metrics/server.go b/metrics/server.go index 15e850b4..af8acfe0 100644 --- a/metrics/server.go +++ b/metrics/server.go @@ -3,6 +3,7 @@ package metrics import ( "context" "net/http" + "time" "github.com/prometheus/client_golang/prometheus/promhttp" "go.uber.org/zap" @@ -20,8 +21,12 @@ func Start(addr string, logger *zap.Logger) *Server { // Create the HTTP server with the custom ServeMux as the handler server := &http.Server{ - Addr: addr, - Handler: mux, + Addr: addr, + Handler: mux, + ReadHeaderTimeout: 2 * time.Second, + ReadTimeout: 5 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 30 * time.Second, } // Store the logger in the server struct diff --git a/testutil/mocks/babylon.go b/testutil/mocks/babylon.go index 7ba26689..1f7dcb3b 100644 --- a/testutil/mocks/babylon.go +++ b/testutil/mocks/babylon.go @@ -113,7 +113,7 @@ func (mr *MockClientControllerMockRecorder) QueryBlock(height interface{}) *gomo } // QueryBlocks mocks base method. -func (m *MockClientController) QueryBlocks(startHeight, endHeight, limit uint64) ([]*types0.BlockInfo, error) { +func (m *MockClientController) QueryBlocks(startHeight, endHeight uint64, limit uint32) ([]*types0.BlockInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "QueryBlocks", startHeight, endHeight, limit) ret0, _ := ret[0].([]*types0.BlockInfo)