From 3b0e13b1b338ea54033cd4136c0dc34fefdf340f Mon Sep 17 00:00:00 2001 From: Stana Miric <94966829+stana-miric@users.noreply.github.com> Date: Mon, 27 Mar 2023 14:06:40 +0200 Subject: [PATCH] Add godox linter and resolve trivial TODO comments (#1329) * Added: godox linter,tasks for unresolved TODOs * cr fix --- .golangci.yml | 1 + blockchain/testing.go | 2 - command/genesis/polybft_params.go | 2 - consensus/polybft/block_builder.go | 3 +- consensus/polybft/blockchain_wrapper.go | 6 +- consensus/polybft/consensus_runtime.go | 8 +- consensus/polybft/consensus_runtime_test.go | 14 +- consensus/polybft/contracts_initializer.go | 2 + consensus/polybft/extra.go | 1 - consensus/polybft/fsm.go | 9 +- consensus/polybft/fsm_test.go | 51 +++---- consensus/polybft/hash.go | 1 - consensus/polybft/mocks_test.go | 4 +- consensus/polybft/polybft_config.go | 6 +- consensus/polybft/sc_integration_test.go | 3 +- consensus/polybft/state_sync_manager_test.go | 3 +- consensus/polybft/system_state.go | 6 +- consensus/polybft/system_state_test.go | 7 +- consensus/polybft/validators_snapshot.go | 1 - consensus/utils.go | 1 - e2e-polybft/e2e/bridge_test.go | 136 ------------------ e2e-polybft/e2e/consensus_test.go | 10 +- e2e-polybft/e2e/helpers_test.go | 138 +++++++++++++++++++ e2e/jsonrpc_test.go | 13 +- jsonrpc/eth_endpoint.go | 3 +- jsonrpc/eth_state_test.go | 4 +- jsonrpc/filter_manager.go | 3 +- network/grpc/grpc.go | 12 +- network/server.go | 7 +- network/server_test.go | 3 +- state/immutable-trie/trie.go | 5 +- state/runtime/evm/instructions.go | 2 +- state/runtime/precompiled/blake2f.go | 3 +- state/runtime/precompiled/precompiled.go | 3 +- state/state.go | 3 +- state/testing.go | 5 +- state/txn.go | 6 +- syncer/syncer_test.go | 6 +- 38 files changed, 243 insertions(+), 250 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5be02fd380..fe83dec3f4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -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: diff --git a/blockchain/testing.go b/blockchain/testing.go index 0c389f518c..2f3b510cd0 100644 --- a/blockchain/testing.go +++ b/blockchain/testing.go @@ -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 } diff --git a/command/genesis/polybft_params.go b/command/genesis/polybft_params.go index 4e0ab5e924..561e68734f 100644 --- a/command/genesis/polybft_params.go +++ b/command/genesis/polybft_params.go @@ -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, } diff --git a/consensus/polybft/block_builder.go b/consensus/polybft/block_builder.go index 05733308db..c141c8c5e1 100644 --- a/consensus/polybft/block_builder.go +++ b/consensus/polybft/block_builder.go @@ -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 { diff --git a/consensus/polybft/blockchain_wrapper.go b/consensus/polybft/blockchain_wrapper.go index 33f11280c1..3a8b642108 100644 --- a/consensus/polybft/blockchain_wrapper.go +++ b/consensus/polybft/blockchain_wrapper.go @@ -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 @@ -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 { diff --git a/consensus/polybft/consensus_runtime.go b/consensus/polybft/consensus_runtime.go index ec2a5a9de0..9fba0ce854 100644 --- a/consensus/polybft/consensus_runtime.go +++ b/consensus/polybft/consensus_runtime.go @@ -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) ) @@ -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 @@ -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 { diff --git a/consensus/polybft/consensus_runtime_test.go b/consensus/polybft/consensus_runtime_test.go index 37c73fef35..aa2502a2fa 100644 --- a/consensus/polybft/consensus_runtime_test.go +++ b/consensus/polybft/consensus_runtime_test.go @@ -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() @@ -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()) @@ -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()) diff --git a/consensus/polybft/contracts_initializer.go b/consensus/polybft/contracts_initializer.go index 33fdb2366f..52633865fc 100644 --- a/consensus/polybft/contracts_initializer.go +++ b/consensus/polybft/contracts_initializer.go @@ -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 diff --git a/consensus/polybft/extra.go b/consensus/polybft/extra.go index 658cd0ddad..8b7776964a 100644 --- a/consensus/polybft/extra.go +++ b/consensus/polybft/extra.go @@ -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 diff --git a/consensus/polybft/fsm.go b/consensus/polybft/fsm.go index c2ab732319..239bb26010 100644 --- a/consensus/polybft/fsm.go +++ b/consensus/polybft/fsm.go @@ -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() @@ -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, @@ -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 @@ -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 { diff --git a/consensus/polybft/fsm_test.go b/consensus/polybft/fsm_test.go index 846f36d742..1a6808e8b4 100644 --- a/consensus/polybft/fsm_test.go +++ b/consensus/polybft/fsm_test.go @@ -62,7 +62,6 @@ func TestFSM_verifyCommitEpochTx(t *testing.T) { t.Parallel() fsm := &fsm{ - config: &PolyBFTConfig{ValidatorSetAddr: contracts.ValidatorSetContract}, isEndOfEpoch: true, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10), } @@ -76,7 +75,7 @@ func TestFSM_verifyCommitEpochTx(t *testing.T) { // submit tampered commit epoch transaction to the epoch ending block alteredCommitEpochTx := &types.Transaction{ - To: &fsm.config.ValidatorSetAddr, + To: &contracts.ValidatorSetContract, Input: []byte{}, Gas: 0, Type: types.StateTx, @@ -188,7 +187,7 @@ func TestFSM_BuildProposal_WithCommitEpochTxGood(t *testing.T) { blockChainMock := new(blockchainMock) blockChainMock.On("GetStateProvider", mock.Anything). Return(NewStateProvider(transition)).Once() - blockChainMock.On("GetSystemState", mock.Anything, mock.Anything).Return(systemStateMock).Once() + blockChainMock.On("GetSystemState", mock.Anything).Return(systemStateMock).Once() runtime := &consensusRuntime{ logger: hclog.NewNullLogger(), @@ -261,7 +260,7 @@ func TestFSM_BuildProposal_EpochEndingBlock_FailedToApplyStateTx(t *testing.T) { validatorSet := NewValidatorSet(validators.getPublicIdentities(), hclog.NewNullLogger()) - fsm := &fsm{parent: parent, blockBuilder: mBlockBuilder, config: &PolyBFTConfig{}, backend: &blockchainMock{}, + fsm := &fsm{parent: parent, blockBuilder: mBlockBuilder, backend: &blockchainMock{}, isEndOfEpoch: true, validators: validatorSet, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10), @@ -439,7 +438,7 @@ func TestFSM_BuildProposal_EpochEndingBlock_FailToCreateValidatorsDelta(t *testi func TestFSM_VerifyStateTransactions_MiddleOfEpochWithTransaction(t *testing.T) { t.Parallel() - fsm := &fsm{config: &PolyBFTConfig{}, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} + fsm := &fsm{commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} tx, err := fsm.createCommitEpochTx() assert.NoError(t, err) err = fsm.VerifyStateTransactions([]*types.Transaction{tx}) @@ -449,7 +448,7 @@ func TestFSM_VerifyStateTransactions_MiddleOfEpochWithTransaction(t *testing.T) func TestFSM_VerifyStateTransactions_MiddleOfEpochWithoutTransaction(t *testing.T) { t.Parallel() - fsm := &fsm{config: &PolyBFTConfig{}, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} + fsm := &fsm{commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} err := fsm.VerifyStateTransactions([]*types.Transaction{}) assert.NoError(t, err) } @@ -457,7 +456,7 @@ func TestFSM_VerifyStateTransactions_MiddleOfEpochWithoutTransaction(t *testing. func TestFSM_VerifyStateTransactions_EndOfEpochWithoutTransaction(t *testing.T) { t.Parallel() - fsm := &fsm{config: &PolyBFTConfig{}, isEndOfEpoch: true, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} + fsm := &fsm{isEndOfEpoch: true, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} assert.EqualError(t, fsm.VerifyStateTransactions([]*types.Transaction{}), "commit epoch transaction is not found in the epoch ending block") } @@ -465,7 +464,7 @@ func TestFSM_VerifyStateTransactions_EndOfEpochWithoutTransaction(t *testing.T) func TestFSM_VerifyStateTransactions_EndOfEpochWrongCommitEpochTx(t *testing.T) { t.Parallel() - fsm := &fsm{config: &PolyBFTConfig{}, isEndOfEpoch: true, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} + fsm := &fsm{isEndOfEpoch: true, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} commitEpochInput, err := createTestCommitEpochInput(t, 0, nil, 5).EncodeAbi() require.NoError(t, err) @@ -476,7 +475,7 @@ func TestFSM_VerifyStateTransactions_EndOfEpochWrongCommitEpochTx(t *testing.T) func TestFSM_VerifyStateTransactions_CommitmentTransactionAndSprintIsFalse(t *testing.T) { t.Parallel() - fsm := &fsm{config: &PolyBFTConfig{}} + fsm := &fsm{} encodedCommitment, err := createTestCommitmentMessage(t, 1).EncodeAbi() require.NoError(t, err) @@ -490,7 +489,7 @@ func TestFSM_VerifyStateTransactions_EndOfEpochMoreThanOneCommitEpochTx(t *testi t.Parallel() txs := make([]*types.Transaction, 2) - fsm := &fsm{config: &PolyBFTConfig{}, isEndOfEpoch: true, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} + fsm := &fsm{isEndOfEpoch: true, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10)} commitEpochTxOne, err := fsm.createCommitEpochTx() require.NoError(t, err) @@ -514,7 +513,6 @@ func TestFSM_VerifyStateTransactions_StateTransactionPass(t *testing.T) { validatorSet := NewValidatorSet(validators.getPublicIdentities(), hclog.NewNullLogger()) fsm := &fsm{ - config: &PolyBFTConfig{}, isEndOfEpoch: true, isEndOfSprint: true, validators: validatorSet, @@ -543,7 +541,6 @@ func TestFSM_VerifyStateTransactions_StateTransactionQuorumNotReached(t *testing validatorSet := NewValidatorSet(validators.getPublicIdentities(), hclog.NewNullLogger()) fsm := &fsm{ - config: &PolyBFTConfig{}, isEndOfEpoch: true, isEndOfSprint: true, validators: validatorSet, @@ -585,7 +582,6 @@ func TestFSM_VerifyStateTransactions_StateTransactionInvalidSignature(t *testing validatorSet := NewValidatorSet(validators.getPublicIdentities(), hclog.NewNullLogger()) fsm := &fsm{ - config: &PolyBFTConfig{}, isEndOfEpoch: true, isEndOfSprint: true, validators: validatorSet, @@ -713,7 +709,7 @@ func TestFSM_Validate_IncorrectHeaderParentHash(t *testing.T) { } parent.ComputeHash() - fsm := &fsm{parent: parent, config: &PolyBFTConfig{}, backend: &blockchainMock{}, + fsm := &fsm{parent: parent, backend: &blockchainMock{}, validators: validators.toValidatorSet(), logger: hclog.NewNullLogger()} stateBlock := createDummyStateBlock(parent.Number+1, types.Hash{100, 15}, parent.ExtraData) @@ -748,7 +744,7 @@ func TestFSM_Validate_InvalidNumber(t *testing.T) { for _, blockNum := range []uint64{parentBlockNumber - 1, parentBlockNumber, parentBlockNumber + 2} { stateBlock := createDummyStateBlock(blockNum, parent.Hash, parent.ExtraData) mBlockBuilder := newBlockBuilderMock(stateBlock) - fsm := &fsm{parent: parent, blockBuilder: mBlockBuilder, config: &PolyBFTConfig{}, backend: &blockchainMock{}, + fsm := &fsm{parent: parent, blockBuilder: mBlockBuilder, backend: &blockchainMock{}, validators: validators.toValidatorSet(), logger: hclog.NewNullLogger()} proposalHash, err := new(CheckpointData).Hash(fsm.backend.GetChainID(), stateBlock.Block.Number(), stateBlock.Block.Hash()) @@ -784,7 +780,7 @@ func TestFSM_Validate_TimestampOlder(t *testing.T) { ExtraData: parent.ExtraData, } stateBlock := &types.FullBlock{Block: consensus.BuildBlock(consensus.BuildBlockParams{Header: header})} - fsm := &fsm{parent: parent, config: &PolyBFTConfig{}, backend: &blockchainMock{}, + fsm := &fsm{parent: parent, backend: &blockchainMock{}, validators: validators.toValidatorSet(), logger: hclog.NewNullLogger()} checkpointHash, err := new(CheckpointData).Hash(fsm.backend.GetChainID(), header.Number, header.Hash) @@ -823,7 +819,6 @@ func TestFSM_Validate_IncorrectMixHash(t *testing.T) { fsm := &fsm{ parent: parent, - config: &PolyBFTConfig{}, backend: &blockchainMock{}, validators: validators.toValidatorSet(), logger: hclog.NewNullLogger(), @@ -871,7 +866,6 @@ func TestFSM_Insert_Good(t *testing.T) { f := &fsm{ parent: parent, blockBuilder: builderMock, - config: &PolyBFTConfig{}, target: builtBlock, backend: chainMock, validators: NewValidatorSet(validatorsMetadata[0:len(validatorsMetadata)-1], hclog.NewNullLogger()), @@ -957,7 +951,7 @@ func TestFSM_Insert_InvalidNode(t *testing.T) { validatorSet := NewValidatorSet(validatorsMetadata[0:len(validatorsMetadata)-1], hclog.NewNullLogger()) - fsm := &fsm{parent: parent, blockBuilder: mBlockBuilder, config: &PolyBFTConfig{}, backend: &blockchainMock{}, + fsm := &fsm{parent: parent, blockBuilder: mBlockBuilder, backend: &blockchainMock{}, validators: validatorSet, } @@ -1009,7 +1003,6 @@ func TestFSM_DecodeCommitmentStateTxs(t *testing.T) { _, signedCommitment, _ := buildCommitmentAndStateSyncs(t, eventsSize, uint64(3), from) f := &fsm{ - config: &PolyBFTConfig{}, proposerCommitmentToRegister: signedCommitment, commitEpochInput: createTestCommitEpochInput(t, 0, nil, 10), logger: hclog.NewNullLogger(), @@ -1072,7 +1065,6 @@ func TestFSM_VerifyStateTransaction_ValidBothTypesOfStateTransactions(t *testing f := &fsm{ isEndOfSprint: true, - config: &PolyBFTConfig{}, validators: validators.toValidatorSet(), } @@ -1083,7 +1075,7 @@ func TestFSM_VerifyStateTransaction_ValidBothTypesOfStateTransactions(t *testing require.NoError(t, err) if i == 0 { - tx := createStateTransactionWithData(f.config.StateReceiverAddr, inputData) + tx := createStateTransactionWithData(contracts.StateReceiverContract, inputData) txns = append(txns, tx) } } @@ -1100,12 +1092,11 @@ func TestFSM_VerifyStateTransaction_InvalidTypeOfStateTransactions(t *testing.T) f := &fsm{ isEndOfSprint: true, - config: &PolyBFTConfig{}, } var txns []*types.Transaction txns = append(txns, - createStateTransactionWithData(f.config.StateReceiverAddr, []byte{9, 3, 1, 1})) + createStateTransactionWithData(contracts.StateReceiverContract, []byte{9, 3, 1, 1})) require.ErrorContains(t, f.VerifyStateTransactions(txns), "unknown state transaction") } @@ -1117,7 +1108,6 @@ func TestFSM_VerifyStateTransaction_QuorumNotReached(t *testing.T) { _, commitmentMessageSigned, _ := buildCommitmentAndStateSyncs(t, 10, uint64(3), 2) f := &fsm{ isEndOfSprint: true, - config: &PolyBFTConfig{}, validators: validators.toValidatorSet(), } @@ -1133,7 +1123,7 @@ func TestFSM_VerifyStateTransaction_QuorumNotReached(t *testing.T) { require.NoError(t, err) txns = append(txns, - createStateTransactionWithData(f.config.StateReceiverAddr, inputData)) + createStateTransactionWithData(contracts.StateReceiverContract, inputData)) err = f.VerifyStateTransactions(txns) require.ErrorContains(t, err, "quorum size not reached for state tx") @@ -1146,7 +1136,6 @@ func TestFSM_VerifyStateTransaction_InvalidSignature(t *testing.T) { _, commitmentMessageSigned, _ := buildCommitmentAndStateSyncs(t, 10, uint64(3), 2) f := &fsm{ isEndOfSprint: true, - config: &PolyBFTConfig{}, validators: validators.toValidatorSet(), } @@ -1169,7 +1158,7 @@ func TestFSM_VerifyStateTransaction_InvalidSignature(t *testing.T) { require.NoError(t, err) txns = append(txns, - createStateTransactionWithData(f.config.StateReceiverAddr, inputData)) + createStateTransactionWithData(contracts.StateReceiverContract, inputData)) err = f.VerifyStateTransactions(txns) require.ErrorContains(t, err, "invalid signature for tx") @@ -1185,7 +1174,6 @@ func TestFSM_VerifyStateTransaction_TwoCommitmentMessages(t *testing.T) { f := &fsm{ isEndOfSprint: true, - config: &PolyBFTConfig{}, validators: validatorSet, } @@ -1201,12 +1189,12 @@ func TestFSM_VerifyStateTransaction_TwoCommitmentMessages(t *testing.T) { require.NoError(t, err) txns = append(txns, - createStateTransactionWithData(f.config.StateReceiverAddr, inputData)) + createStateTransactionWithData(contracts.StateReceiverContract, inputData)) inputData, err = commitmentMessageSigned.EncodeAbi() require.NoError(t, err) txns = append(txns, - createStateTransactionWithData(f.config.StateReceiverAddr, inputData)) + createStateTransactionWithData(contracts.StateReceiverContract, inputData)) err = f.VerifyStateTransactions(txns) require.ErrorContains(t, err, "only one commitment tx is allowed per block") } @@ -1241,7 +1229,6 @@ func TestFSM_Validate_FailToVerifySignatures(t *testing.T) { fsm := &fsm{ parent: parent, - config: &PolyBFTConfig{Bridge: &BridgeConfig{}}, backend: &blockchainMock{}, polybftBackend: polybftBackendMock, validators: validatorSet, diff --git a/consensus/polybft/hash.go b/consensus/polybft/hash.go index 810b80cbdd..7f769bc07c 100644 --- a/consensus/polybft/hash.go +++ b/consensus/polybft/hash.go @@ -19,7 +19,6 @@ func setupHeaderHashFunc() { // the extra field the seal and committed seal items extra, err := GetIbftExtraClean(h.ExtraData) if err != nil { - // TODO: log error? return types.ZeroHash } diff --git a/consensus/polybft/mocks_test.go b/consensus/polybft/mocks_test.go index 30603a9fe3..c723570dce 100644 --- a/consensus/polybft/mocks_test.go +++ b/consensus/polybft/mocks_test.go @@ -108,8 +108,8 @@ func (m *blockchainMock) GetHeaderByHash(hash types.Hash) (*types.Header, bool) panic("Unsupported mock for GetHeaderByHash") //nolint:gocritic } -func (m *blockchainMock) GetSystemState(config *PolyBFTConfig, provider contract.Provider) SystemState { - args := m.Called(config, provider) +func (m *blockchainMock) GetSystemState(provider contract.Provider) SystemState { + args := m.Called(provider) return args.Get(0).(SystemState) //nolint:forcetypeassert } diff --git a/consensus/polybft/polybft_config.go b/consensus/polybft/polybft_config.go index 048cfc0bce..e7fc3108c2 100644 --- a/consensus/polybft/polybft_config.go +++ b/consensus/polybft/polybft_config.go @@ -42,11 +42,7 @@ type PolyBFTConfig struct { // MintableERC20Token denotes whether mintable ERC20 token is used MintableERC20Token bool `json:"mintableERC20"` - // TODO: Remove these two addresses as they are hardcoded and known in advance - // Address of the system contracts, as of now (testing) this is populated automatically during genesis - ValidatorSetAddr types.Address `json:"validatorSetAddr"` - StateReceiverAddr types.Address `json:"stateReceiverAddr"` - InitialTrieRoot types.Hash `json:"initialTrieRoot"` + InitialTrieRoot types.Hash `json:"initialTrieRoot"` } // GetPolyBFTConfig deserializes provided chain config and returns PolyBFTConfig diff --git a/consensus/polybft/sc_integration_test.go b/consensus/polybft/sc_integration_test.go index ee8b383841..bdb8042c19 100644 --- a/consensus/polybft/sc_integration_test.go +++ b/consensus/polybft/sc_integration_test.go @@ -283,8 +283,7 @@ func TestIntegration_CommitEpoch(t *testing.T) { SprintSize: 5, EpochReward: reward, // use 1st account as governance address - Governance: currentValidators.toValidatorSet().validators.GetAddresses()[0], - ValidatorSetAddr: contracts.ValidatorSetContract, + Governance: currentValidators.toValidatorSet().validators.GetAddresses()[0], } // get data for ChildValidatorSet initialization diff --git a/consensus/polybft/state_sync_manager_test.go b/consensus/polybft/state_sync_manager_test.go index 6bc23700b5..f022d227e8 100644 --- a/consensus/polybft/state_sync_manager_test.go +++ b/consensus/polybft/state_sync_manager_test.go @@ -403,7 +403,8 @@ func TestStateSyncerManager_EventTracker_Sync(t *testing.T) { server := testutil.DeployTestServer(t, nil) - // TODO: Deploy local artifacts + //nolint:godox + // TODO: Deploy local artifacts (to be fixed in EVM-542) cc := &testutil.Contract{} cc.AddCallback(func() string { return ` diff --git a/consensus/polybft/system_state.go b/consensus/polybft/system_state.go index 81b9243995..2e2c7b8dfc 100644 --- a/consensus/polybft/system_state.go +++ b/consensus/polybft/system_state.go @@ -42,14 +42,14 @@ type SystemStateImpl struct { } // NewSystemState initializes new instance of systemState which abstracts smart contracts functions -func NewSystemState(config *PolyBFTConfig, provider contract.Provider) *SystemStateImpl { +func NewSystemState(valSetAddr types.Address, stateRcvAddr types.Address, provider contract.Provider) *SystemStateImpl { s := &SystemStateImpl{} s.validatorContract = contract.NewContract( - ethgo.Address(config.ValidatorSetAddr), + ethgo.Address(valSetAddr), contractsapi.ChildValidatorSet.Abi, contract.WithProvider(provider), ) s.sidechainBridgeContract = contract.NewContract( - ethgo.Address(config.StateReceiverAddr), + ethgo.Address(stateRcvAddr), contractsapi.StateReceiver.Abi, contract.WithProvider(provider), ) diff --git a/consensus/polybft/system_state_test.go b/consensus/polybft/system_state_test.go index 5c4ba919f0..46001d1c48 100644 --- a/consensus/polybft/system_state_test.go +++ b/consensus/polybft/system_state_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/0xPolygon/polygon-edge/chain" + "github.com/0xPolygon/polygon-edge/contracts" "github.com/0xPolygon/polygon-edge/state" itrie "github.com/0xPolygon/polygon-edge/state/immutable-trie" "github.com/0xPolygon/polygon-edge/types" @@ -77,7 +78,7 @@ func TestSystemState_GetValidatorSet(t *testing.T) { transition: transition, } - st := NewSystemState(&PolyBFTConfig{ValidatorSetAddr: result.Address}, provider) + st := NewSystemState(result.Address, contracts.StateReceiverContract, provider) validators, err := st.GetValidatorSet() assert.NoError(t, err) assert.Equal(t, types.Address(ethgo.HexToAddress("1")), validators[0].Address) @@ -117,7 +118,7 @@ func TestSystemState_GetNextCommittedIndex(t *testing.T) { transition: transition, } - systemState := NewSystemState(&PolyBFTConfig{StateReceiverAddr: result.Address}, provider) + systemState := NewSystemState(contracts.ValidatorSetContract, result.Address, provider) expectedNextCommittedIndex := uint64(45) input, err := sideChainBridgeABI.Encode([1]interface{}{expectedNextCommittedIndex}) @@ -164,7 +165,7 @@ func TestSystemState_GetEpoch(t *testing.T) { transition: transition, } - systemState := NewSystemState(&PolyBFTConfig{ValidatorSetAddr: result.Address}, provider) + systemState := NewSystemState(result.Address, contracts.StateReceiverContract, provider) expectedEpoch := uint64(50) input, err := setEpochMethod.Encode([1]interface{}{expectedEpoch}) diff --git a/consensus/polybft/validators_snapshot.go b/consensus/polybft/validators_snapshot.go index 8189004abb..84295e4ddf 100644 --- a/consensus/polybft/validators_snapshot.go +++ b/consensus/polybft/validators_snapshot.go @@ -26,7 +26,6 @@ func (vs *validatorSnapshot) copy() *validatorSnapshot { } } -// TODO: Should we switch validators snapshot to Edge implementation? (validators/store/snapshot/snapshot.go) type validatorsSnapshotCache struct { snapshots map[uint64]*validatorSnapshot state *State diff --git a/consensus/utils.go b/consensus/utils.go index 77d267c55f..093c3d9202 100644 --- a/consensus/utils.go +++ b/consensus/utils.go @@ -29,7 +29,6 @@ func BuildBlock(params BuildBlockParams) *types.Block { header.ReceiptsRoot = buildroot.CalculateReceiptsRoot(params.Receipts) } - // TODO: Compute uncles header.Sha3Uncles = types.EmptyUncleHash header.ComputeHash() diff --git a/e2e-polybft/e2e/bridge_test.go b/e2e-polybft/e2e/bridge_test.go index 4bdd65c359..d0fb109836 100644 --- a/e2e-polybft/e2e/bridge_test.go +++ b/e2e-polybft/e2e/bridge_test.go @@ -1,13 +1,8 @@ package e2e import ( - "bytes" - "encoding/json" - "errors" "fmt" - "io" "math/big" - "net/http" "path" "strconv" "strings" @@ -23,7 +18,6 @@ import ( "github.com/0xPolygon/polygon-edge/command/sidechain" "github.com/0xPolygon/polygon-edge/consensus/polybft" "github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi" - "github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi/artifact" "github.com/0xPolygon/polygon-edge/contracts" "github.com/0xPolygon/polygon-edge/e2e-polybft/framework" @@ -35,28 +29,6 @@ const ( manifestFileName = "manifest.json" ) -// checkStateSyncResultLogs is helper function which parses given StateSyncResultEvent event's logs, -// extracts status topic value and makes assertions against it. -func checkStateSyncResultLogs( - t *testing.T, - logs []*ethgo.Log, - expectedCount int, -) { - t.Helper() - require.Len(t, logs, expectedCount) - - var stateSyncResultEvent contractsapi.StateSyncResultEvent - for _, log := range logs { - doesMatch, err := stateSyncResultEvent.ParseLog(log) - require.True(t, doesMatch) - require.NoError(t, err) - - t.Logf("Block Number=%d, Decoded Log=%+v\n", log.BlockNumber, stateSyncResultEvent) - - require.True(t, stateSyncResultEvent.Status) - } -} - func TestE2E_Bridge_DepositAndWithdrawERC20(t *testing.T) { const ( num = 10 @@ -585,114 +557,6 @@ func TestE2E_Bridge_L2toL1ExitMultiple(t *testing.T) { } } -func sendExitTransaction( - sidechainKey *ethgow.Key, - rootchainKey ethgo.Key, - proof types.Proof, - checkpointBlock uint64, - stateSenderData []byte, - l1ExitTestAddr, - exitHelperAddr ethgo.Address, - l1TxRelayer txrelayer.TxRelayer, - exitEventID uint64) (bool, error) { - var exitEventAPI contractsapi.L2StateSyncedEvent - - proofExitEventEncoded, err := exitEventAPI.Encode(&polybft.ExitEvent{ - ID: exitEventID, - Sender: sidechainKey.Address(), - Receiver: l1ExitTestAddr, - Data: stateSenderData, - }) - if err != nil { - return false, err - } - - leafIndex, ok := proof.Metadata["LeafIndex"].(float64) - if !ok { - return false, fmt.Errorf("could not get leaf index from exit event proof. Leaf from proof: %v", proof.Metadata["LeafIndex"]) - } - - receipt, err := ABITransaction(l1TxRelayer, rootchainKey, contractsapi.ExitHelper, exitHelperAddr, - "exit", - big.NewInt(int64(checkpointBlock)), - uint64(leafIndex), - proofExitEventEncoded, - proof.Data, - ) - - if err != nil { - return false, err - } - - if receipt.Status != uint64(types.ReceiptSuccess) { - return false, errors.New("transaction execution failed") - } - - return isExitEventProcessed(exitEventID, exitHelperAddr, l1TxRelayer) -} - -func getExitProof(rpcAddress string, exitID uint64) (types.Proof, error) { - query := struct { - Jsonrpc string `json:"jsonrpc"` - Method string `json:"method"` - Params []string `json:"params"` - ID int `json:"id"` - }{ - "2.0", - "bridge_generateExitProof", - []string{fmt.Sprintf("0x%x", exitID)}, - 1, - } - - d, err := json.Marshal(query) - if err != nil { - return types.Proof{}, err - } - - resp, err := http.Post(rpcAddress, "application/json", bytes.NewReader(d)) - if err != nil { - return types.Proof{}, err - } - - s, err := io.ReadAll(resp.Body) - if err != nil { - return types.Proof{}, err - } - - rspProof := struct { - Result types.Proof `json:"result"` - }{} - - err = json.Unmarshal(s, &rspProof) - if err != nil { - return types.Proof{}, err - } - - return rspProof.Result, nil -} - -// TODO: Move this to some separate file, containing helper functions? -func ABICall(relayer txrelayer.TxRelayer, artifact *artifact.Artifact, contractAddress ethgo.Address, senderAddr ethgo.Address, method string, params ...interface{}) (string, error) { - input, err := artifact.Abi.GetMethod(method).Encode(params) - if err != nil { - return "", err - } - - return relayer.Call(senderAddr, contractAddress, input) -} - -func ABITransaction(relayer txrelayer.TxRelayer, key ethgo.Key, artifact *artifact.Artifact, contractAddress ethgo.Address, method string, params ...interface{}) (*ethgo.Receipt, error) { - input, err := artifact.Abi.GetMethod(method).Encode(params) - if err != nil { - return nil, err - } - - return relayer.SendTransaction(ðgo.Transaction{ - To: &contractAddress, - Input: input, - }, key) -} - func TestE2E_Bridge_ChangeVotingPower(t *testing.T) { const ( finalBlockNumber = 20 diff --git a/e2e-polybft/e2e/consensus_test.go b/e2e-polybft/e2e/consensus_test.go index ea5b927005..79f5912399 100644 --- a/e2e-polybft/e2e/consensus_test.go +++ b/e2e-polybft/e2e/consensus_test.go @@ -132,9 +132,8 @@ func TestE2E_Consensus_RegisterValidator(t *testing.T) { require.NoError(t, err) systemState := polybft.NewSystemState( - &polybft.PolyBFTConfig{ - StateReceiverAddr: contracts.StateReceiverContract, - ValidatorSetAddr: contracts.ValidatorSetContract}, + contracts.ValidatorSetContract, + contracts.StateReceiverContract, &e2eStateProvider{txRelayer: txRelayer}) // create the first account and extract the address @@ -454,9 +453,8 @@ func TestE2E_Consensus_Validator_Unstake(t *testing.T) { require.NoError(t, err) systemState := polybft.NewSystemState( - &polybft.PolyBFTConfig{ - StateReceiverAddr: contracts.StateReceiverContract, - ValidatorSetAddr: contracts.ValidatorSetContract}, + contracts.ValidatorSetContract, + contracts.StateReceiverContract, &e2eStateProvider{txRelayer: txRelayer}) validatorAcc, err := sidechain.GetAccountFromDir(validatorSecrets) diff --git a/e2e-polybft/e2e/helpers_test.go b/e2e-polybft/e2e/helpers_test.go index a74fdaa894..5065eec593 100644 --- a/e2e-polybft/e2e/helpers_test.go +++ b/e2e-polybft/e2e/helpers_test.go @@ -1,16 +1,25 @@ package e2e import ( + "bytes" + "encoding/json" "errors" + "fmt" + "io" "math/big" + "net/http" + "testing" "github.com/0xPolygon/polygon-edge/consensus/polybft" "github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi" + "github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi/artifact" "github.com/0xPolygon/polygon-edge/helper/hex" "github.com/0xPolygon/polygon-edge/txrelayer" "github.com/0xPolygon/polygon-edge/types" + "github.com/stretchr/testify/require" "github.com/umbracle/ethgo" "github.com/umbracle/ethgo/contract" + ethgow "github.com/umbracle/ethgo/wallet" ) type e2eStateProvider struct { @@ -98,3 +107,132 @@ func getRootchainValidators(relayer txrelayer.TxRelayer, checkpointManagerAddr e return validators, nil } + +func ABICall(relayer txrelayer.TxRelayer, artifact *artifact.Artifact, contractAddress ethgo.Address, senderAddr ethgo.Address, method string, params ...interface{}) (string, error) { + input, err := artifact.Abi.GetMethod(method).Encode(params) + if err != nil { + return "", err + } + + return relayer.Call(senderAddr, contractAddress, input) +} + +func ABITransaction(relayer txrelayer.TxRelayer, key ethgo.Key, artifact *artifact.Artifact, contractAddress ethgo.Address, method string, params ...interface{}) (*ethgo.Receipt, error) { + input, err := artifact.Abi.GetMethod(method).Encode(params) + if err != nil { + return nil, err + } + + return relayer.SendTransaction(ðgo.Transaction{ + To: &contractAddress, + Input: input, + }, key) +} + +func sendExitTransaction( + sidechainKey *ethgow.Key, + rootchainKey ethgo.Key, + proof types.Proof, + checkpointBlock uint64, + stateSenderData []byte, + l1ExitTestAddr, + exitHelperAddr ethgo.Address, + l1TxRelayer txrelayer.TxRelayer, + exitEventID uint64) (bool, error) { + var exitEventAPI contractsapi.L2StateSyncedEvent + + proofExitEventEncoded, err := exitEventAPI.Encode(&polybft.ExitEvent{ + ID: exitEventID, + Sender: sidechainKey.Address(), + Receiver: l1ExitTestAddr, + Data: stateSenderData, + }) + if err != nil { + return false, err + } + + leafIndex, ok := proof.Metadata["LeafIndex"].(float64) + if !ok { + return false, fmt.Errorf("could not get leaf index from exit event proof. Leaf from proof: %v", proof.Metadata["LeafIndex"]) + } + + receipt, err := ABITransaction(l1TxRelayer, rootchainKey, contractsapi.ExitHelper, exitHelperAddr, + "exit", + big.NewInt(int64(checkpointBlock)), + uint64(leafIndex), + proofExitEventEncoded, + proof.Data, + ) + + if err != nil { + return false, err + } + + if receipt.Status != uint64(types.ReceiptSuccess) { + return false, errors.New("transaction execution failed") + } + + return isExitEventProcessed(exitEventID, exitHelperAddr, l1TxRelayer) +} + +func getExitProof(rpcAddress string, exitID uint64) (types.Proof, error) { + query := struct { + Jsonrpc string `json:"jsonrpc"` + Method string `json:"method"` + Params []string `json:"params"` + ID int `json:"id"` + }{ + "2.0", + "bridge_generateExitProof", + []string{fmt.Sprintf("0x%x", exitID)}, + 1, + } + + d, err := json.Marshal(query) + if err != nil { + return types.Proof{}, err + } + + resp, err := http.Post(rpcAddress, "application/json", bytes.NewReader(d)) + if err != nil { + return types.Proof{}, err + } + + s, err := io.ReadAll(resp.Body) + if err != nil { + return types.Proof{}, err + } + + rspProof := struct { + Result types.Proof `json:"result"` + }{} + + err = json.Unmarshal(s, &rspProof) + if err != nil { + return types.Proof{}, err + } + + return rspProof.Result, nil +} + +// checkStateSyncResultLogs is helper function which parses given StateSyncResultEvent event's logs, +// extracts status topic value and makes assertions against it. +func checkStateSyncResultLogs( + t *testing.T, + logs []*ethgo.Log, + expectedCount int, +) { + t.Helper() + require.Len(t, logs, expectedCount) + + var stateSyncResultEvent contractsapi.StateSyncResultEvent + for _, log := range logs { + doesMatch, err := stateSyncResultEvent.ParseLog(log) + require.True(t, doesMatch) + require.NoError(t, err) + + t.Logf("Block Number=%d, Decoded Log=%+v\n", log.BlockNumber, stateSyncResultEvent) + + require.True(t, stateSyncResultEvent.Status) + } +} diff --git a/e2e/jsonrpc_test.go b/e2e/jsonrpc_test.go index 2326e61c29..64150ade25 100644 --- a/e2e/jsonrpc_test.go +++ b/e2e/jsonrpc_test.go @@ -18,7 +18,8 @@ var ( ) func TestJsonRPC(t *testing.T) { - // TODO: Reuse the same tests for websockets and IPC. + //nolint:godox + // TODO: Reuse the same tests for websockets and IPC. (to be fixed in EVM-521) fund, err := wallet.GenerateKey() require.NoError(t, err) @@ -58,14 +59,16 @@ func TestJsonRPC(t *testing.T) { require.NoError(t, err) require.Equal(t, balance1, newBalance) + //nolint:godox // Test. return 0 if the balance of an existing account is empty - // TODO + // TODO (to be fixed in EVM-521) }) t.Run("eth_getTransactionCount", func(t *testing.T) { key1, _ := wallet.GenerateKey() - // TODO. return zero if the account does not exists in the state (Does not work) + //nolint:godox + // TODO. return zero if the account does not exists in the state (Does not work) (to be fixed in EVM-521) _, err := client.GetNonce(key1.Address(), ethgo.Latest) require.NoError(t, err) @@ -88,7 +91,7 @@ func TestJsonRPC(t *testing.T) { require.NoError(t, err) require.Equal(t, nonce1, uint64(0)) - // Test. TODO. you can query the nonce at any block hash in time + // Test. TODO. you can query the nonce at any block hash in time (to be fixed in EVM-521) //nolint:godox block, err := client.GetBlockByNumber(ethgo.BlockNumber(txn.Receipt().BlockNumber)-1, false) require.NoError(t, err) @@ -132,7 +135,7 @@ func TestJsonRPC(t *testing.T) { cases := []ethgo.BlockNumberOrHash{ ethgo.Latest, ethgo.BlockNumber(receipt.BlockNumber), - // receipt.BlockHash, TODO: It does not work + // receipt.BlockHash, TODO: It does not work (to be fixed in EVM-521) //nolint:godox } for _, c := range cases { code, err = client.GetCode(codeAddr, c) diff --git a/jsonrpc/eth_endpoint.go b/jsonrpc/eth_endpoint.go index 83d8b876f9..ba06b25a65 100644 --- a/jsonrpc/eth_endpoint.go +++ b/jsonrpc/eth_endpoint.go @@ -358,7 +358,8 @@ func (e *Eth) GetStorageAt( return nil, err } - // TODO: GetStorage should return the values already parsed + //nolint:godox + // TODO: GetStorage should return the values already parsed (to be fixed in EVM-522) // Parse the RLP value p := &fastrlp.Parser{} diff --git a/jsonrpc/eth_state_test.go b/jsonrpc/eth_state_test.go index 8aec664ae8..af3ff5cbd5 100644 --- a/jsonrpc/eth_state_test.go +++ b/jsonrpc/eth_state_test.go @@ -616,8 +616,8 @@ func getExampleStore() *mockSpecialStore { // the latest block gas limit for the upper bound, or the specified // gas limit in the transaction func TestEth_EstimateGas_GasLimit(t *testing.T) { - // TODO Make this test run in parallel when the race - // condition is fixed in gas estimation + //nolint:godox + // TODO Make this test run in parallel when the race condition is fixed in gas estimation (to be fixed in EVM-523) store := getExampleStore() ethEndpoint := newTestEthEndpoint(store) diff --git a/jsonrpc/filter_manager.go b/jsonrpc/filter_manager.go index 8f463d68ba..2e20dd0ad0 100644 --- a/jsonrpc/filter_manager.go +++ b/jsonrpc/filter_manager.go @@ -265,7 +265,8 @@ func NewFilterManager(logger hclog.Logger, store filterManagerStore, blockRangeL // start blockstream with the current header header := store.Header() - // TODO: Make Header return jsonrpc.block object directly + //nolint:godox + // TODO: Make Header return jsonrpc.block object directly (to be fixed in EVM-524) block := toBlock(&types.Block{Header: header}, false) m.blockStream = newBlockStream(block) diff --git a/network/grpc/grpc.go b/network/grpc/grpc.go index 9de1fbaa1b..10e1eaa29a 100644 --- a/network/grpc/grpc.go +++ b/network/grpc/grpc.go @@ -74,7 +74,7 @@ func interceptor( ) } -func (g *GrpcStream) Client(stream network.Stream) *grpc.ClientConn { +func (g *GrpcStream) Client(stream network.Stream) (*grpc.ClientConn, error) { return WrapClient(stream) } @@ -124,18 +124,12 @@ func (g *GrpcStream) Close() error { // --- conn --- -func WrapClient(s network.Stream) *grpc.ClientConn { +func WrapClient(s network.Stream) (*grpc.ClientConn, error) { opts := grpc.WithContextDialer(func(ctx context.Context, peerIdStr string) (net.Conn, error) { return &streamConn{s}, nil }) - conn, err := grpc.Dial("", grpc.WithTransportCredentials(insecure.NewCredentials()), opts) - if err != nil { - // TODO: this should not fail at all - panic(err) //nolint:gocritic - } - - return conn + return grpc.Dial("", grpc.WithTransportCredentials(insecure.NewCredentials()), opts) } // streamConn represents a net.Conn wrapped to be compatible with net.conn diff --git a/network/server.go b/network/server.go index 51c097d96f..081bfcc061 100644 --- a/network/server.go +++ b/network/server.go @@ -397,9 +397,10 @@ func (s *Server) runDial() { } for { + //nolint:godox // TODO: Right now the dial task are done sequentially because Connect // is a blocking request. In the future we should try to make up to - // maxDials requests concurrently + // maxDials requests concurrently (to be fixed in EVM-541) for s.connectionCounts.HasFreeOutboundConn() { tt := s.dialQueue.PopTask() if tt == nil { @@ -617,7 +618,7 @@ func (s *Server) NewProtoConnection(protocol string, peerID peer.ID) (*rawGrpc.C return nil, err } - return p.Client(stream), nil + return p.Client(stream) } func (s *Server) NewStream(proto string, id peer.ID) (network.Stream, error) { @@ -625,7 +626,7 @@ func (s *Server) NewStream(proto string, id peer.ID) (network.Stream, error) { } type Protocol interface { - Client(network.Stream) *rawGrpc.ClientConn + Client(network.Stream) (*rawGrpc.ClientConn, error) Handler() func(network.Stream) } diff --git a/network/server_test.go b/network/server_test.go index 2b82dc7c97..62badbd85f 100644 --- a/network/server_test.go +++ b/network/server_test.go @@ -1027,9 +1027,10 @@ func TestPeerAdditionDeletion(t *testing.T) { t.Run("peers are added correctly", func(t *testing.T) { server := createServer() + //nolint:godox // TODO increase this number to something astronomical // when the networking package has an event system that actually works, - // as emitEvent can completely bug out when under load inside Server.AddPeer + // as emitEvent can completely bug out when under load inside Server.AddPeer (to be fixed in EVM-525) generateAndAddPeers(server, 10) }) diff --git a/state/immutable-trie/trie.go b/state/immutable-trie/trie.go index c34b17cb8a..8143893306 100644 --- a/state/immutable-trie/trie.go +++ b/state/immutable-trie/trie.go @@ -115,7 +115,10 @@ func hashit(k []byte) []byte { var accountArenaPool fastrlp.ArenaPool -var stateArenaPool fastrlp.ArenaPool // TODO, Remove once we do update in fastrlp +// TODO, Remove once we do update in fastrlp (to be fixed in EVM-528) +// +//nolint:godox +var stateArenaPool fastrlp.ArenaPool // Hash returns the root hash of the trie. It does not write to the // database and can be used even if the trie doesn't have one. diff --git a/state/runtime/evm/instructions.go b/state/runtime/evm/instructions.go index 773d168914..9ef721115e 100644 --- a/state/runtime/evm/instructions.go +++ b/state/runtime/evm/instructions.go @@ -1333,7 +1333,7 @@ func (c *state) buildCreateContract(op OpCode) (*runtime.Contract, error) { return nil, nil } - // Consume memory resize gas (TODO, change with get2) + // Consume memory resize gas (TODO, change with get2) (to be fixed in EVM-528) //nolint:godox if !c.consumeGas(gasCost) { return nil, nil } diff --git a/state/runtime/precompiled/blake2f.go b/state/runtime/precompiled/blake2f.go index ab7726c302..62ad2872b1 100644 --- a/state/runtime/precompiled/blake2f.go +++ b/state/runtime/precompiled/blake2f.go @@ -70,7 +70,8 @@ func (e *blake2f) run(input []byte, _ types.Address, _ runtime.Host) ([]byte, er return res, nil } -// TODO: Move this to own repo and include the assembly code from the Go repo +//nolint:godox +// TODO: Move this to own repo and include the assembly code from the Go repo (to be fixed in EVM-527) // Copied from keep-network/blake2f // IV is an initialization vector for BLAKE2b diff --git a/state/runtime/precompiled/precompiled.go b/state/runtime/precompiled/precompiled.go index 37f150d419..2a661bba13 100644 --- a/state/runtime/precompiled/precompiled.go +++ b/state/runtime/precompiled/precompiled.go @@ -161,7 +161,8 @@ func (p *Precompiled) Run(c *runtime.Contract, host runtime.Host, config *chain. var zeroPadding = make([]byte, 64) func (p *Precompiled) leftPad(buf []byte, n int) []byte { - // TODO, avoid buffer allocation + //nolint:godox + // TODO, avoid buffer allocation (to be fixed in EVM-528) l := len(buf) if l > n { return buf diff --git a/state/state.go b/state/state.go index 36c0ac01b0..0a2089b298 100644 --- a/state/state.go +++ b/state/state.go @@ -146,7 +146,8 @@ type Object struct { Nonce uint64 Deleted bool - // TODO: Move this to executor + //nolint:godox + // TODO: Move this to executor (to be fixed in EVM-527) DirtyCode bool Code []byte diff --git a/state/testing.go b/state/testing.go index 6880725b12..46322485d5 100644 --- a/state/testing.go +++ b/state/testing.go @@ -186,8 +186,9 @@ func testUpdateStateWithEmpty(t *testing.T, buildPreState buildPreState) { txn := newTxn(snap) txn.SetState(addr1, hash1, hash0) - // TODO, test with false (should not be deleted) - // TODO, test with balance on the account and nonce + //nolint:godox + // TODO, test with false (should not be deleted) (to be fixed in EVM-528) + // TODO, test with balance on the account and nonce (to be fixed in EVM-528) snap, _ = snap.Commit(txn.Commit(true)) txn = newTxn(snap) diff --git a/state/txn.go b/state/txn.go index bc5e2b6019..00e11364b3 100644 --- a/state/txn.go +++ b/state/txn.go @@ -388,7 +388,8 @@ func (txn *Txn) GetCode(addr types.Address) []byte { if object.DirtyCode { return object.Code } - // TODO; Should we move this to state? + //nolint:godox + // TODO; Should we move this to state? (to be fixed in EVM-527) v, ok := txn.codeCache.Get(addr) if ok { @@ -489,7 +490,8 @@ func (txn *Txn) TouchAccount(addr types.Address) { }) } -// TODO, check panics with this ones +//nolint:godox +// TODO, check panics with this ones (to be fixed in EVM-528) func (txn *Txn) Exist(addr types.Address) bool { _, exists := txn.getStateObject(addr) diff --git a/syncer/syncer_test.go b/syncer/syncer_test.go index c97f615479..eeb1d54391 100644 --- a/syncer/syncer_test.go +++ b/syncer/syncer_test.go @@ -544,7 +544,8 @@ func TestSync(t *testing.T) { } }, blocks: blocks[:10], - // TODO: need to fix implementation? + //nolint:godox + // TODO: need to fix implementation? (to be fixed in EVM-529) progressionStart: 0, progressionHighest: 0, err: nil, @@ -590,7 +591,8 @@ func TestSync(t *testing.T) { } }, blocks: blocks[:10], - // TODO: need to fix implementation? + //nolint:godox + // TODO: need to fix implementation? (to be fixed in EVM-529) progressionStart: 0, progressionHighest: 0, err: nil,