Skip to content

Commit

Permalink
Add godox linter and resolve trivial TODO comments (#1329)
Browse files Browse the repository at this point in the history
* Added: godox linter,tasks for unresolved TODOs

* cr fix
  • Loading branch information
stana-miric authored Mar 27, 2023
1 parent 83ffef6 commit 3b0e13b
Show file tree
Hide file tree
Showing 38 changed files with 243 additions and 250 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ linters:
- errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13
- gocritic
- errcheck # Errcheck is a go lint rule for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases
- godox # Godox is a linter for TODOs and FIXMEs left in the code

linters-settings:
gofmt:
Expand Down
2 changes: 0 additions & 2 deletions blockchain/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ func NewTestBlockchain(t *testing.T, headers []*types.Header) *Blockchain {
}
}

// TODO, find a way to add the snapshot, this will fail until that is fixed.
// snap, _ := state.NewSnapshot(types.Hash{})
return b
}

Expand Down
2 changes: 0 additions & 2 deletions command/genesis/polybft_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ func (p *genesisParams) generatePolyBftChainConfig(o command.OutputFormatter) er
// use 1st account as governance address
Governance: manifest.GenesisValidators[0].Address,
Bridge: bridge,
ValidatorSetAddr: contracts.ValidatorSetContract,
StateReceiverAddr: contracts.StateReceiverContract,
InitialTrieRoot: types.StringToHash(p.initialStateRoot),
MintableERC20Token: p.mintableNativeToken,
}
Expand Down
3 changes: 2 additions & 1 deletion consensus/polybft/block_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import (
hcf "github.com/hashicorp/go-hclog"
)

// TODO: Add opentracing
//nolint:godox
// TODO: Add opentracing (to be fixed in EVM-540)

// BlockBuilderParams are fields for the block that cannot be changed
type BlockBuilderParams struct {
Expand Down
6 changes: 3 additions & 3 deletions consensus/polybft/blockchain_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type blockchainBackend interface {
GetHeaderByHash(hash types.Hash) (*types.Header, bool)

// GetSystemState creates a new instance of SystemState interface
GetSystemState(config *PolyBFTConfig, provider contract.Provider) SystemState
GetSystemState(provider contract.Provider) SystemState

SubscribeEvents() blockchain.Subscription

Expand Down Expand Up @@ -171,8 +171,8 @@ func (p *blockchainWrapper) NewBlockBuilder(
}

// GetSystemState is an implementation of blockchainBackend interface
func (p *blockchainWrapper) GetSystemState(config *PolyBFTConfig, provider contract.Provider) SystemState {
return NewSystemState(config, provider)
func (p *blockchainWrapper) GetSystemState(provider contract.Provider) SystemState {
return NewSystemState(contracts.ValidatorSetContract, contracts.StateReceiverContract, provider)
}

func (p *blockchainWrapper) SubscribeEvents() blockchain.Subscription {
Expand Down
8 changes: 5 additions & 3 deletions consensus/polybft/consensus_runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ func (c *consensusRuntime) OnBlockInserted(fullBlock *types.FullBlock) {
var (
epoch = c.epoch
err error
// TODO - this will need to take inconsideration if slashing occurred
//nolint:godox
// TODO - this will need to take inconsideration if slashing occurred (to be fixed in EVM-519)
isEndOfEpoch = c.isFixedSizeOfEpochMet(fullBlock.Block.Header.Number, epoch)
)

Expand Down Expand Up @@ -316,7 +317,8 @@ func (c *consensusRuntime) FSM() error {
return fmt.Errorf("cannot create block builder for fsm: %w", err)
}

// TODO - recognize slashing occurred
//nolint:godox
// TODO - recognize slashing occurred (to be fixed in EVM-519)
slash := false

pendingBlockNumber := parent.Number + 1
Expand Down Expand Up @@ -584,7 +586,7 @@ func (c *consensusRuntime) getSystemState(header *types.Header) (SystemState, er
return nil, err
}

return c.config.blockchain.GetSystemState(c.config.PolyBFTConfig, provider), nil
return c.config.blockchain.GetSystemState(provider), nil
}

func (c *consensusRuntime) IsValidProposal(rawProposal []byte) bool {
Expand Down
14 changes: 6 additions & 8 deletions consensus/polybft/consensus_runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,9 @@ func Test_NewConsensusRuntime(t *testing.T) {
CheckpointAddr: types.Address{0x10},
JSONRPCEndpoint: "testEndpoint",
},
ValidatorSetAddr: types.Address{0x11},
EpochSize: 10,
SprintSize: 10,
BlockTime: 2 * time.Second,
EpochSize: 10,
SprintSize: 10,
BlockTime: 2 * time.Second,
}

validators := newTestValidators(t, 3).getPublicIdentities()
Expand Down Expand Up @@ -483,7 +482,7 @@ func Test_NewConsensusRuntime(t *testing.T) {
assert.Equal(t, runtime.config.DataDir, tmpDir)
assert.Equal(t, uint64(10), runtime.config.PolyBFTConfig.SprintSize)
assert.Equal(t, uint64(10), runtime.config.PolyBFTConfig.EpochSize)
assert.Equal(t, "0x1100000000000000000000000000000000000000", runtime.config.PolyBFTConfig.ValidatorSetAddr.String())
assert.Equal(t, "0x0000000000000000000000000000000000000101", contracts.ValidatorSetContract.String())
assert.Equal(t, "0x1300000000000000000000000000000000000000", runtime.config.PolyBFTConfig.Bridge.BridgeAddr.String())
assert.Equal(t, "0x1000000000000000000000000000000000000000", runtime.config.PolyBFTConfig.Bridge.CheckpointAddr.String())
assert.True(t, runtime.IsBridgeEnabled())
Expand Down Expand Up @@ -550,9 +549,8 @@ func TestConsensusRuntime_calculateCommitEpochInput_SecondEpoch(t *testing.T) {

validators := newTestValidatorsWithAliases(t, []string{"A", "B", "C", "D", "E"})
polybftConfig := &PolyBFTConfig{
ValidatorSetAddr: contracts.ValidatorSetContract,
EpochSize: epochSize,
SprintSize: sprintSize,
EpochSize: epochSize,
SprintSize: sprintSize,
}

lastBuiltBlock, headerMap := createTestBlocks(t, 19, epochSize, validators.getPublicIdentities())
Expand Down
2 changes: 2 additions & 0 deletions consensus/polybft/contracts_initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ func getInitChildValidatorSetInput(polyBFTConfig PolyBFTConfig) ([]byte, error)

// getInitChildERC20PredicateInput builds input parameters for ERC20Predicate SC initialization
func getInitChildERC20PredicateInput(config *BridgeConfig) ([]byte, error) {
//nolint:godox
// to be fixed with EVM-541
// TODO: @Stefan-Ethernal Temporary workaround just to be able to run cluster in non-bridge mode, until SC is fixed
rootERC20PredicateAddr := types.StringToAddress("0xDEAD")
rootERC20Addr := types.ZeroAddress
Expand Down
1 change: 0 additions & 1 deletion consensus/polybft/extra.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,6 @@ func (s *Signature) Verify(validators AccountSet, hash types.Hash, domain []byte
blsPublicKeys[i] = validator.BlsKey
}

// TODO: refactor AggregatedSignature
aggs, err := bls.UnmarshalSignature(s.AggregatedSignature)
if err != nil {
return err
Expand Down
9 changes: 5 additions & 4 deletions consensus/polybft/fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ func (f *fsm) BuildProposal(currentRound uint64) ([]byte, error) {
return nil, err
}

// TODO: we will need to revisit once slashing is implemented
//nolint:godox
// TODO: we will need to revisit once slashing is implemented (to be fixed in EVM-519)
extra := &Extra{Parent: extraParent.Committed}
// for non-epoch ending blocks, currentValidatorsHash is the same as the nextValidatorsHash
nextValidators := f.validators.Accounts()
Expand Down Expand Up @@ -213,7 +214,7 @@ func (f *fsm) createBridgeCommitmentTx() (*types.Transaction, error) {
return nil, fmt.Errorf("failed to encode input data for bridge commitment registration: %w", err)
}

return createStateTransactionWithData(f.config.StateReceiverAddr, inputData), nil
return createStateTransactionWithData(contracts.StateReceiverContract, inputData), nil
}

// getValidatorsTransition applies delta to the current validators,
Expand Down Expand Up @@ -246,7 +247,7 @@ func (f *fsm) createCommitEpochTx() (*types.Transaction, error) {
return nil, err
}

return createStateTransactionWithData(f.config.ValidatorSetAddr, input), nil
return createStateTransactionWithData(contracts.ValidatorSetContract, input), nil
}

// ValidateCommit is used to validate that a given commit is valid
Expand Down Expand Up @@ -553,7 +554,7 @@ func (f *fsm) ValidatorSet() ValidatorSet {
// getCurrentValidators queries smart contract on the given block height and returns currently active validator set
func (f *fsm) getCurrentValidators(pendingBlockState *state.Transition) (AccountSet, error) {
provider := f.backend.GetStateProvider(pendingBlockState)
systemState := f.backend.GetSystemState(f.config, provider)
systemState := f.backend.GetSystemState(provider)
newValidators, err := systemState.GetValidatorSet()

if err != nil {
Expand Down
Loading

0 comments on commit 3b0e13b

Please sign in to comment.