From 4f702d69ceb8f68f7f1b0a561b447f83a75b1698 Mon Sep 17 00:00:00 2001 From: Cirrus Gai Date: Mon, 16 Dec 2024 22:00:57 +0800 Subject: [PATCH] chore: Remove todos (#221) Remaining todos are tracked by: - https://github.com/babylonlabs-io/finality-provider/issues/222 - https://github.com/babylonlabs-io/finality-provider/issues/223 - https://github.com/babylonlabs-io/finality-provider/issues/224 - https://github.com/babylonlabs-io/finality-provider/issues/225 - https://github.com/babylonlabs-io/finality-provider/issues/219 - https://github.com/babylonlabs-io/finality-provider/issues/196 --- CHANGELOG.md | 1 + clientcontroller/babylon.go | 2 +- docs/finality-provider-operation.md | 9 +++++++++ eotsmanager/localmanager.go | 5 ----- finality-provider/proto/finality_providers.pb.go | 2 +- finality-provider/proto/finality_providers.proto | 2 -- finality-provider/service/chain_poller.go | 8 +------- finality-provider/service/chain_poller_test.go | 2 -- finality-provider/service/eots_manager_adapter.go | 5 ++--- finality-provider/service/fp_instance.go | 13 ++++--------- finality-provider/service/rpcserver.go | 5 ----- finality-provider/store/pub_rand.go | 1 + itest/test_manager.go | 1 - testutil/port.go | 4 ++-- 14 files changed, 22 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 735cdaea..1080566d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clientcontroller/babylon.go b/clientcontroller/babylon.go index 191e1fcf..0b209ba1 100644 --- a/clientcontroller/babylon.go +++ b/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( diff --git a/docs/finality-provider-operation.md b/docs/finality-provider-operation.md index 0eced53b..ef4cae8e 100644 --- a/docs/finality-provider-operation.md +++ b/docs/finality-provider-operation.md @@ -577,6 +577,12 @@ For more information on statuses please refer to diagram in the core documentati +After successful registration, you may start the finality provider instance +by running: +```shell +fpd start --eots-pk +``` + ### 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 `. + ### 5.4. Slashing **Slashing occurs** when a finality provider **double signs**, meaning that the diff --git a/eotsmanager/localmanager.go b/eotsmanager/localmanager.go index 7cd66c70..3c5a0feb 100644 --- a/eotsmanager/localmanager.go +++ b/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 { diff --git a/finality-provider/proto/finality_providers.pb.go b/finality-provider/proto/finality_providers.pb.go index 58b3e662..685da2c7 100644 --- a/finality-provider/proto/finality_providers.pb.go +++ b/finality-provider/proto/finality_providers.pb.go @@ -713,7 +713,7 @@ type QueryFinalityProviderListResponse struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - FinalityProviders []*FinalityProviderInfo `protobuf:"bytes,1,rep,name=finality_providers,json=finalityProviders,proto3" json:"finality_providers,omitempty"` // TODO add pagination in case the list gets large + FinalityProviders []*FinalityProviderInfo `protobuf:"bytes,1,rep,name=finality_providers,json=finalityProviders,proto3" json:"finality_providers,omitempty"` } func (x *QueryFinalityProviderListResponse) Reset() { diff --git a/finality-provider/proto/finality_providers.proto b/finality-provider/proto/finality_providers.proto index 87d1561c..87e0704b 100644 --- a/finality-provider/proto/finality_providers.proto +++ b/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. diff --git a/finality-provider/service/chain_poller.go b/finality-provider/service/chain_poller.go index c3bc874e..e59abfad 100644 --- a/finality-provider/service/chain_poller.go +++ b/finality-provider/service/chain_poller.go @@ -16,7 +16,6 @@ import ( ) var ( - // TODO: Maybe configurable? RtyAttNum = uint(5) RtyAtt = retry.Attempts(RtyAttNum) RtyDel = retry.Delay(time.Millisecond * 400) @@ -24,7 +23,6 @@ var ( ) 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 { diff --git a/finality-provider/service/chain_poller_test.go b/finality-provider/service/chain_poller_test.go index f3ddcdbf..b07a5e82 100644 --- a/finality-provider/service/chain_poller_test.go +++ b/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 diff --git a/finality-provider/service/eots_manager_adapter.go b/finality-provider/service/eots_manager_adapter.go index ed97f554..29be16bf 100644 --- a/finality-provider/service/eots_manager_adapter.go +++ b/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...) } diff --git a/finality-provider/service/fp_instance.go b/finality-provider/service/fp_instance.go index 211060d4..89e3801c 100644 --- a/finality-provider/service/fp_instance.go +++ b/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 { diff --git a/finality-provider/service/rpcserver.go b/finality-provider/service/rpcserver.go index 55c9fdd1..502780f3 100644 --- a/finality-provider/service/rpcserver.go +++ b/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 } diff --git a/finality-provider/store/pub_rand.go b/finality-provider/store/pub_rand.go index b9303171..51fa45b6 100644 --- a/finality-provider/store/pub_rand.go +++ b/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" diff --git a/itest/test_manager.go b/itest/test_manager.go index a2740a00..20319795 100644 --- a/itest/test_manager.go +++ b/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, diff --git a/testutil/port.go b/testutil/port.go index ccaf03b2..19ad7f76 100644 --- a/testutil/port.go +++ b/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