chore: Remove todos (#221)

Remaining todos are tracked by:
- #222
- #223
- #224
- #225
- #219
- #196

CHANGELOG.md

@@ -45,6 +45,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * [#214](https://github.com/babylonlabs-io/finality-provider/pull/214) Gradual benchmark
 * [#216](https://github.com/babylonlabs-io/finality-provider/pull/216) Add multiple fpd connecting to one eotsd in e2e tests
 * [#218](https://github.com/babylonlabs-io/finality-provider/pull/218) Prune used merkle proof
+* [#221](https://github.com/babylonlabs-io/finality-provider/pull/221) Cleanup TODOs
 
 ## v0.13.1
 

clientcontroller/babylon.go

@@ -183,7 +183,7 @@ func (bc *BabylonController) SubmitFinalitySig(
 	fpPk *btcec.PublicKey,
 	block *types.BlockInfo,
 	pubRand *btcec.FieldVal,
-	proof []byte, // TODO: have a type for proof
+	proof []byte,
 	sig *btcec.ModNScalar,
 ) (*types.TxResponse, error) {
 	return bc.SubmitBatchFinalitySigs(

docs/finality-provider-operation.md

@@ -577,6 +577,12 @@ For more information on statuses please refer to diagram in the core documentati
 
 <!-- vitsalis: TODO: native way of showing the finality provider has the statuses -->
 
+After successful registration, you may start the finality provider instance
+by running:
+```shell
+fpd start --eots-pk <hex-string-of-eots-public-key>
+```
+
 ### 5.2. Withdrawing Rewards
 
 As a participant in the Finality Provider Program, you will earn rewards that 
@@ -607,6 +613,9 @@ Parameters:
 
 > ⚠️ Before unjailing, ensure you've fixed the underlying issue that caused jailing
 
+If unjailing is successful, you may start running the finality provider by
+`fpd start --eots-pk <hex-string-of-eots-public-key>`.
+
 ### 5.4. Slashing
 
 **Slashing occurs** when a finality provider **double signs**, meaning that the

eotsmanager/localmanager.go

@@ -172,10 +172,6 @@ func (lm *LocalEOTSManager) LoadBIP340PubKeyFromKeyName(keyName string) (*bbntyp
 	}
 }
 
-// TODO the current implementation is a PoC, which does not contain any anti-slasher mechanism
-//
-//	a simple anti-slasher mechanism could be that the manager remembers the tuple (fpPk, chainID, height) or
-//	the hash of each generated randomness and return error if the same randomness is requested twice
 func (lm *LocalEOTSManager) CreateRandomnessPairList(fpPk []byte, chainID []byte, startHeight uint64, num uint32, passphrase string) ([]*btcec.FieldVal, error) {
 	prList := make([]*btcec.FieldVal, 0, num)
 
@@ -323,7 +319,6 @@ func (lm *LocalEOTSManager) getRandomnessPair(fpPk []byte, chainID []byte, heigh
 	return privRand, pubRand, nil
 }
 
-// TODO: we ignore passPhrase in local implementation for now
 func (lm *LocalEOTSManager) KeyRecord(fpPk []byte, passphrase string) (*eotstypes.KeyRecord, error) {
 	name, err := lm.es.GetEOTSKeyName(fpPk)
 	if err != nil {

finality-provider/proto/finality_providers.proto

@@ -113,12 +113,10 @@ message QueryFinalityProviderResponse {
 }
 
 message QueryFinalityProviderListRequest {
-    // TODO add pagination in case the list gets large
 }
 
 message QueryFinalityProviderListResponse {
     repeated FinalityProviderInfo finality_providers = 1;
-    // TODO add pagination in case the list gets large
 }
 
 // FinalityProvider defines current state of finality provider.

finality-provider/service/chain_poller.go

@@ -16,15 +16,13 @@ import (
 )
 
 var (
-	// TODO: Maybe configurable?
 	RtyAttNum = uint(5)
 	RtyAtt    = retry.Attempts(RtyAttNum)
 	RtyDel    = retry.Delay(time.Millisecond * 400)
 	RtyErr    = retry.LastErrorOnly(true)
 )
 
 const (
-	// TODO: Maybe configurable?
 	maxFailedCycles = 20
 )
 
@@ -110,9 +108,7 @@ func (cp *ChainPoller) IsRunning() bool {
 	return cp.isStarted.Load()
 }
 
-// Return read only channel for incoming blocks
-// TODO: Handle the case when there is more than one consumer. Currently with more than
-// one consumer blocks most probably will be received out of order to those consumers.
+// GetBlockInfoChan returns the read-only channel for incoming blocks
 func (cp *ChainPoller) GetBlockInfoChan() <-chan *types.BlockInfo {
 	return cp.blockInfoChan
 }
@@ -175,8 +171,6 @@ func (cp *ChainPoller) pollChain() {
 	for {
 		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 {

finality-provider/service/chain_poller_test.go

@@ -46,7 +46,6 @@ func FuzzChainPoller_Start(f *testing.F) {
 			mockClientController.EXPECT().QueryBlock(i).Return(resBlock, nil).AnyTimes()
 		}
 
-		// TODO: use mock metrics
 		m := metrics.NewFpMetrics()
 		pollerCfg := fpcfg.DefaultChainPollerConfig()
 		pollerCfg.PollInterval = 10 * time.Millisecond
@@ -98,7 +97,6 @@ func FuzzChainPoller_SkipHeight(f *testing.F) {
 			mockClientController.EXPECT().QueryBlock(i).Return(resBlock, nil).AnyTimes()
 		}
 
-		// TODO: use mock metrics
 		m := metrics.NewFpMetrics()
 		pollerCfg := fpcfg.DefaultChainPollerConfig()
 		pollerCfg.PollInterval = 1 * time.Second

finality-provider/service/eots_manager_adapter.go

@@ -4,11 +4,12 @@ import (
 	"fmt"
 
 	bbntypes "github.com/babylonlabs-io/babylon/types"
-	"github.com/babylonlabs-io/finality-provider/types"
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/btcec/v2/schnorr"
 	"github.com/cometbft/cometbft/crypto/tmhash"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/babylonlabs-io/finality-provider/types"
 )
 
 func (fp *FinalityProviderInstance) getPubRandList(startHeight uint64, numPubRand uint32) ([]*btcec.FieldVal, error) {
@@ -26,7 +27,6 @@ func (fp *FinalityProviderInstance) getPubRandList(startHeight uint64, numPubRan
 	return pubRandList, nil
 }
 
-// TODO: have this function in Babylon side
 func getHashToSignForCommitPubRand(startHeight uint64, numPubRand uint64, commitment []byte) ([]byte, error) {
 	hasher := tmhash.New()
 	if _, err := hasher.Write(sdk.Uint64ToBigEndian(startHeight)); err != nil {
@@ -51,7 +51,6 @@ func (fp *FinalityProviderInstance) signPubRandCommit(startHeight uint64, numPub
 	return fp.em.SignSchnorrSig(fp.btcPk.MustMarshal(), hash, fp.passphrase)
 }
 
-// TODO: have this function in Babylon side
 func getMsgToSignForVote(blockHeight uint64, blockHash []byte) []byte {
 	return append(sdk.Uint64ToBigEndian(blockHeight), blockHash...)
 }

finality-provider/service/fp_instance.go

@@ -220,7 +220,6 @@ func (fp *FinalityProviderInstance) getAllBlocksFromChan() []*types.BlockInfo {
 	for {
 		select {
 		case b := <-fp.poller.GetBlockInfoChan():
-			// TODO: in cases of catching up, this could issue frequent RPC calls
 			shouldProcess, err := fp.shouldProcessBlock(b)
 			if err != nil {
 				if !errors.Is(err, ErrFinalityProviderShutDown) {
@@ -387,8 +386,8 @@ func (fp *FinalityProviderInstance) retrySubmitSigsUntilFinalized(targetBlocks [
 					zap.String("pk", fp.GetBtcPkHex()),
 					zap.Uint64("target_height", targetHeight),
 				)
-				// TODO: returning nil here is to safely break the loop
-				//  the error still exists
+				// returning nil here is to safely break the loop
+				// the error still exists
 				return nil, nil
 			}
 
@@ -421,8 +420,6 @@ func (fp *FinalityProviderInstance) retryCommitPubRandUntilBlockFinalized(target
 		// this part should not be retried here. We need to separate the function into
 		// 1) determining the starting height to commit, 2) generating pub rand and inclusion
 		//  proofs, and 3) committing public randomness.
-		// TODO: make 3) a part of `select` statement. The function terminates upon either the block
-		// is finalised or the pub rand is committed successfully
 		res, err := fp.CommitPubRand(targetBlock.Height)
 		if err != nil {
 			if clientcontroller.IsUnrecoverable(err) {
@@ -457,8 +454,8 @@ func (fp *FinalityProviderInstance) retryCommitPubRandUntilBlockFinalized(target
 					zap.String("pk", fp.GetBtcPkHex()),
 					zap.Uint64("target_height", targetBlock.Height),
 				)
-				// TODO: returning nil here is to safely break the loop
-				//  the error still exists
+				// returning nil here is to safely break the loop
+				// the error still exists
 				return nil, nil
 			}
 
@@ -608,8 +605,6 @@ func (fp *FinalityProviderInstance) TestCommitPubRandWithStartHeight(startHeight
 
 	fp.logger.Info("Start committing pubrand from block height", zap.Uint64("start_height", startHeight))
 
-	// TODO: instead of sending multiple txs, a better way is to bundle all the commit messages into
-	// one like we do for batch finality signatures. see discussion https://bit.ly/3OmbjkN
 	for startHeight <= targetBlockHeight {
 		_, err = fp.commitPubRandPairs(startHeight)
 		if err != nil {

finality-provider/service/rpcserver.go

@@ -206,11 +206,6 @@ func (r *rpcServer) UnjailFinalityProvider(_ context.Context, req *proto.UnjailF
 		return nil, fmt.Errorf("failed to unjail the finality-provider: %w", err)
 	}
 
-	// todo: keep passphrase as empty for now
-	if err := r.app.StartFinalityProvider(fpPk, ""); err != nil {
-		return nil, fmt.Errorf("failed to start the finality provider instance after unjailing: %w", err)
-	}
-
 	return &proto.UnjailFinalityProviderResponse{TxHash: res.TxHash}, nil
 }
 

finality-provider/store/pub_rand.go

@@ -3,6 +3,7 @@ package store
 import (
 	"bytes"
 	"fmt"
+
 	"github.com/btcsuite/btcwallet/walletdb"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 

itest/test_manager.go

@@ -442,7 +442,6 @@ func (tm *TestManager) InsertCovenantSigForDelegation(t *testing.T, btcDel *bsty
 
 	stakingInfo, err := btcstaking.BuildStakingInfo(
 		btcDel.BtcPk.MustToBTCPK(),
-		// TODO: Handle multiple providers
 		[]*btcec.PublicKey{btcDel.FpBtcPkList[0].MustToBTCPK()},
 		params.CovenantPks,
 		params.CovenantQuorum,

testutil/port.go

@@ -2,7 +2,7 @@ package testutil
 
 import (
 	"fmt"
-	mrand "math/rand" // todo(lazar): upgrade to v2 once we move to go1.23
+	mrand "math/rand/v2"
 	"net"
 	"sync"
 	"testing"
@@ -18,7 +18,7 @@ var (
 // by testing multiple random ports within a specified range.
 func AllocateUniquePort(t *testing.T) int {
 	randPort := func(base, spread int) int {
-		return base + mrand.Intn(spread)
+		return base + mrand.IntN(spread)
 	}
 
 	// Base port and spread range for port selection