From 12ab0f0240625f890355d4c29d523bbfeafc7478 Mon Sep 17 00:00:00 2001 From: Jerry Date: Thu, 19 May 2022 17:55:23 -0700 Subject: [PATCH 1/3] Lint consensus/bor module Cleanup, lint, and enable linters for consenssus/bor module. Disabled linter "gomnd" and "tagliatelle" because they are not easy to fix and the return on the time investment is very low. Disabled linter "prealloc" because it is not easy to guess and pre-allocate the slice accurately in many cases. --- .github/workflows/ci.yml | 2 +- .golangci.yml | 10 +- Makefile | 4 +- consensus/bor/api.go | 42 +++++++- consensus/bor/bor.go | 123 +++++++++++++++++----- consensus/bor/bor_test.go | 7 +- consensus/bor/clerk.go | 2 +- consensus/bor/genesis_contracts_client.go | 10 ++ consensus/bor/merkle.go | 16 +-- consensus/bor/rest.go | 14 ++- consensus/bor/snapshot.go | 24 +++-- consensus/bor/snapshot_test.go | 22 ++++ consensus/bor/validator.go | 9 ++ consensus/bor/validator_set.go | 76 ++++++++++--- 14 files changed, 291 insertions(+), 70 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7db530c346..72647b56eb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,7 +6,7 @@ on: pull_request: branches: - "**" - types: [opened, synchronize] + types: [opened, synchronize, edited] concurrency: group: build-${{ github.event.pull_request.number || github.ref }} diff --git a/.golangci.yml b/.golangci.yml index 7dc4fc37e3..daea4e1e0b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -28,7 +28,7 @@ linters: - exportloopref - gocognit - gofmt - - gomnd + # - gomnd - gomoddirectives - gosec - makezero @@ -38,11 +38,11 @@ linters: - noctx #- nosprintfhostport # TODO: do we use IPv6? - paralleltest - - prealloc + # - prealloc - predeclared #- promlinter #- revive - - tagliatelle + # - tagliatelle - tenv - thelper - tparallel @@ -65,7 +65,7 @@ linters-settings: local-prefixes: github.com/ethereum/go-ethereum nestif: - min-complexity: 3 + min-complexity: 5 prealloc: for-loops: true @@ -183,4 +183,4 @@ issues: max-issues-per-linter: 0 max-same-issues: 0 #new: true - new-from-rev: origin/master \ No newline at end of file + # new-from-rev: origin/master \ No newline at end of file diff --git a/Makefile b/Makefile index b82757e7fb..53b3cd9fdb 100644 --- a/Makefile +++ b/Makefile @@ -65,7 +65,9 @@ escape: cd $(path) && go test -gcflags "-m -m" -run none -bench=BenchmarkJumpdest* -benchmem -memprofile mem.out lint: - @./build/bin/golangci-lint run --config ./.golangci.yml + @./build/bin/golangci-lint run --config ./.golangci.yml \ + internal/cli \ + consensus/bor lintci-deps: rm -f ./build/bin/golangci-lint diff --git a/consensus/bor/api.go b/consensus/bor/api.go index 8f172e47a6..361e439bf7 100644 --- a/consensus/bor/api.go +++ b/consensus/bor/api.go @@ -13,6 +13,7 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/rpc" + lru "github.com/hashicorp/golang-lru" "github.com/xsleonard/go-merkle" "golang.org/x/crypto/sha3" @@ -44,6 +45,7 @@ func (api *API) GetSnapshot(number *rpc.BlockNumber) (*Snapshot, error) { if header == nil { return nil, errUnknownBlock } + return api.bor.snapshot(api.chain, header.Number.Uint64(), header.Hash(), nil) } @@ -59,11 +61,11 @@ type difficultiesKV struct { } func rankMapDifficulties(values map[common.Address]uint64) []difficultiesKV { - var ss []difficultiesKV for k, v := range values { ss = append(ss, difficultiesKV{k, v}) } + sort.Slice(ss, func(i, j int) bool { return ss[i].Difficulty > ss[j].Difficulty }) @@ -74,11 +76,15 @@ func rankMapDifficulties(values map[common.Address]uint64) []difficultiesKV { // GetSnapshotProposerSequence retrieves the in-turn signers of all sprints in a span func (api *API) GetSnapshotProposerSequence(number *rpc.BlockNumber) (BlockSigners, error) { snapNumber := *number - 1 + var difficulties = make(map[common.Address]uint64) + snap, err := api.GetSnapshot(&snapNumber) + if err != nil { return BlockSigners{}, err } + proposer := snap.ValidatorSet.GetProposer().Address proposerIndex, _ := snap.ValidatorSet.GetByAddress(proposer) @@ -88,6 +94,7 @@ func (api *API) GetSnapshotProposerSequence(number *rpc.BlockNumber) (BlockSigne if tempIndex < proposerIndex { tempIndex = tempIndex + len(signers) } + difficulties[signers[i]] = uint64(len(signers) - (tempIndex - proposerIndex)) } @@ -97,6 +104,7 @@ func (api *API) GetSnapshotProposerSequence(number *rpc.BlockNumber) (BlockSigne if err != nil { return BlockSigners{}, err } + diff := int(difficulties[*author]) blockSigners := &BlockSigners{ Signers: rankedDifficulties, @@ -111,9 +119,11 @@ func (api *API) GetSnapshotProposerSequence(number *rpc.BlockNumber) (BlockSigne func (api *API) GetSnapshotProposer(number *rpc.BlockNumber) (common.Address, error) { *number -= 1 snap, err := api.GetSnapshot(number) + if err != nil { return common.Address{}, err } + return snap.ValidatorSet.GetProposer().Address, nil } @@ -130,7 +140,9 @@ func (api *API) GetAuthor(number *rpc.BlockNumber) (*common.Address, error) { if header == nil { return nil, errUnknownBlock } + author, err := api.bor.Author(header) + return &author, err } @@ -140,6 +152,7 @@ func (api *API) GetSnapshotAtHash(hash common.Hash) (*Snapshot, error) { if header == nil { return nil, errUnknownBlock } + return api.bor.snapshot(api.chain, header.Number.Uint64(), header.Hash(), nil) } @@ -156,10 +169,13 @@ func (api *API) GetSigners(number *rpc.BlockNumber) ([]common.Address, error) { if header == nil { return nil, errUnknownBlock } + snap, err := api.bor.snapshot(api.chain, header.Number.Uint64(), header.Hash(), nil) + if err != nil { return nil, err } + return snap.signers(), nil } @@ -169,10 +185,13 @@ func (api *API) GetSignersAtHash(hash common.Hash) ([]common.Address, error) { if header == nil { return nil, errUnknownBlock } + snap, err := api.bor.snapshot(api.chain, header.Number.Uint64(), header.Hash(), nil) + if err != nil { return nil, err } + return snap.signers(), nil } @@ -182,6 +201,7 @@ func (api *API) GetCurrentProposer() (common.Address, error) { if err != nil { return common.Address{}, err } + return snap.ValidatorSet.GetProposer().Address, nil } @@ -191,6 +211,7 @@ func (api *API) GetCurrentValidators() ([]*Validator, error) { if err != nil { return make([]*Validator, 0), err } + return snap.ValidatorSet.Validators, nil } @@ -199,26 +220,36 @@ func (api *API) GetRootHash(start uint64, end uint64) (string, error) { if err := api.initializeRootHashCache(); err != nil { return "", err } + key := getRootHashKey(start, end) + if root, known := api.rootHashCache.Get(key); known { return root.(string), nil } - length := uint64(end - start + 1) + + length := end - start + 1 + if length > MaxCheckpointLength { return "", &MaxCheckpointLengthExceededError{start, end} } + currentHeaderNumber := api.chain.CurrentHeader().Number.Uint64() + if start > end || end > currentHeaderNumber { return "", &InvalidStartEndBlockError{start, end, currentHeaderNumber} } + blockHeaders := make([]*types.Header, end-start+1) wg := new(sync.WaitGroup) concurrent := make(chan bool, 20) + for i := start; i <= end; i++ { wg.Add(1) concurrent <- true + go func(number uint64) { - blockHeaders[number-start] = api.chain.GetHeaderByNumber(uint64(number)) + blockHeaders[number-start] = api.chain.GetHeaderByNumber(number) + <-concurrent wg.Done() }(i) @@ -227,6 +258,7 @@ func (api *API) GetRootHash(start uint64, end uint64) (string, error) { close(concurrent) headers := make([][32]byte, nextPowerOfTwo(length)) + for i := 0; i < len(blockHeaders); i++ { blockHeader := blockHeaders[i] header := crypto.Keccak256(appendBytes32( @@ -237,6 +269,7 @@ func (api *API) GetRootHash(start uint64, end uint64) (string, error) { )) var arr [32]byte + copy(arr[:], header) headers[i] = arr } @@ -245,8 +278,10 @@ func (api *API) GetRootHash(start uint64, end uint64) (string, error) { if err := tree.Generate(convert(headers), sha3.NewLegacyKeccak256()); err != nil { return "", err } + root := hex.EncodeToString(tree.Root().Hash) api.rootHashCache.Add(key, root) + return root, nil } @@ -255,6 +290,7 @@ func (api *API) initializeRootHashCache() error { if api.rootHashCache == nil { api.rootHashCache, err = lru.NewARC(10) } + return err } diff --git a/consensus/bor/bor.go b/consensus/bor/bor.go index 2623011d79..ada601f838 100644 --- a/consensus/bor/bor.go +++ b/consensus/bor/bor.go @@ -32,7 +32,6 @@ import ( "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/ethdb" - "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/params" @@ -56,9 +55,6 @@ var ( uncleHash = types.CalcUncleHash(nil) // Always Keccak256(RLP([])) as uncles are meaningless outside of PoW. - diffInTurn = big.NewInt(2) // Block difficulty for in-turn signatures - diffNoTurn = big.NewInt(1) // Block difficulty for out-of-turn signatures - validatorHeaderBytesLength = common.AddressLength + 20 // address + power systemAddress = common.HexToAddress("0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE") ) @@ -72,18 +68,6 @@ var ( // that is not part of the local blockchain. errUnknownBlock = errors.New("unknown block") - // errInvalidCheckpointBeneficiary is returned if a checkpoint/epoch transition - // block has a beneficiary set to non-zeroes. - errInvalidCheckpointBeneficiary = errors.New("beneficiary in checkpoint block non-zero") - - // errInvalidVote is returned if a nonce value is something else that the two - // allowed constants of 0x00..0 or 0xff..f. - errInvalidVote = errors.New("vote nonce not 0x00..0 or 0xff..f") - - // errInvalidCheckpointVote is returned if a checkpoint/epoch transition block - // has a vote nonce set to non-zeroes. - errInvalidCheckpointVote = errors.New("vote nonce in checkpoint block non-zero") - // errMissingVanity is returned if a block's extra-data section is shorter than // 32 bytes, which is required to store the signer vanity. errMissingVanity = errors.New("extra-data 32 byte vanity prefix missing") @@ -136,6 +120,7 @@ func ecrecover(header *types.Header, sigcache *lru.ARCCache, c *params.BorConfig if len(header.Extra) < extraSeal { return common.Address{}, errMissingSignature } + signature := header.Extra[len(header.Extra)-extraSeal:] // Recover the public key and the Ethereum address @@ -143,10 +128,13 @@ func ecrecover(header *types.Header, sigcache *lru.ARCCache, c *params.BorConfig if err != nil { return common.Address{}, err } + var signer common.Address + copy(signer[:], crypto.Keccak256(pubkey[1:])[12:]) sigcache.Add(hash, signer) + return signer, nil } @@ -155,6 +143,7 @@ func SealHash(header *types.Header, c *params.BorConfig) (hash common.Hash) { hasher := sha3.NewLegacyKeccak256() encodeSigHeader(hasher, header, c) hasher.Sum(hash[:0]) + return hash } @@ -176,11 +165,13 @@ func encodeSigHeader(w io.Writer, header *types.Header, c *params.BorConfig) { header.MixDigest, header.Nonce, } + if c.IsJaipur(header.Number.Uint64()) { if header.BaseFee != nil { enc = append(enc, header.BaseFee) } } + if err := rlp.Encode(w, enc); err != nil { panic("can't encode: " + err.Error()) } @@ -194,9 +185,11 @@ func CalcProducerDelay(number uint64, succession int, c *params.BorConfig) uint6 if number%c.Sprint == 0 { delay = c.ProducerDelay } + if succession > 0 { delay += uint64(succession) * c.CalculateBackupMultiplier(number) } + return delay } @@ -210,6 +203,7 @@ func CalcProducerDelay(number uint64, succession int, c *params.BorConfig) uint6 func BorRLP(header *types.Header, c *params.BorConfig) []byte { b := new(bytes.Buffer) encodeSigHeader(b, header, c) + return b.Bytes() } @@ -233,7 +227,6 @@ type Bor struct { HeimdallClient IHeimdallClient WithoutHeimdall bool - scope event.SubscriptionScope // The fields below are for testing only fakeDiff bool // Skip difficulty verifications } @@ -314,6 +307,7 @@ func (c *Bor) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types. } } }() + return abort, results } @@ -325,6 +319,7 @@ func (c *Bor) verifyHeader(chain consensus.ChainHeaderReader, header *types.Head if header.Number == nil { return errUnknownBlock } + number := header.Number.Uint64() // Don't waste time checking blocks from the future @@ -344,32 +339,40 @@ func (c *Bor) verifyHeader(chain consensus.ChainHeaderReader, header *types.Head if !isSprintEnd && signersBytes != 0 { return errExtraValidators } + if isSprintEnd && signersBytes%validatorHeaderBytesLength != 0 { return errInvalidSpanValidators } + // Ensure that the mix digest is zero as we don't have fork protection currently if header.MixDigest != (common.Hash{}) { return errInvalidMixDigest } + // Ensure that the block doesn't contain any uncles which are meaningless in PoA if header.UncleHash != uncleHash { return errInvalidUncleHash } + // Ensure that the block's difficulty is meaningful (may not be correct at this point) if number > 0 { if header.Difficulty == nil { return errInvalidDifficulty } } + // Verify that the gas limit is <= 2^63-1 - cap := uint64(0x7fffffffffffffff) - if header.GasLimit > cap { - return fmt.Errorf("invalid gasLimit: have %v, max %v", header.GasLimit, cap) + gasCap := uint64(0x7fffffffffffffff) + + if header.GasLimit > gasCap { + return fmt.Errorf("invalid gasLimit: have %v, max %v", header.GasLimit, gasCap) } + // If all checks passed, validate any special fields for hard forks if err := misc.VerifyForkHashes(chain.Config(), header, false); err != nil { return err } + // All basic checks passed, verify cascading fields return c.verifyCascadingFields(chain, header, parents) } @@ -380,9 +383,11 @@ func validateHeaderExtraField(extraBytes []byte) error { if len(extraBytes) < extraVanity { return errMissingVanity } + if len(extraBytes) < extraVanity+extraSeal { return errMissingSignature } + return nil } @@ -393,12 +398,14 @@ func validateHeaderExtraField(extraBytes []byte) error { func (c *Bor) verifyCascadingFields(chain consensus.ChainHeaderReader, header *types.Header, parents []*types.Header) error { // The genesis block is the always valid dead-end number := header.Number.Uint64() + if number == 0 { return nil } // Ensure that the block's timestamp isn't too close to it's parent var parent *types.Header + if len(parents) > 0 { parent = parents[len(parents)-1] } else { @@ -413,11 +420,13 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainHeaderReader, header *t if header.GasUsed > header.GasLimit { return fmt.Errorf("invalid gasUsed: have %d, gasLimit %d", header.GasUsed, header.GasLimit) } + if !chain.Config().IsLondon(header.Number) { // Verify BaseFee not present before EIP-1559 fork. if header.BaseFee != nil { return fmt.Errorf("invalid baseFee before fork: have %d, want ", header.BaseFee) } + if err := misc.VerifyGaslimit(parent.GasLimit, header.GasLimit); err != nil { return err } @@ -432,6 +441,7 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainHeaderReader, header *t // Retrieve the snapshot needed to verify this header and cache it snap, err := c.snapshot(chain, number-1, header.ParentHash, parents) + if err != nil { return err } @@ -444,6 +454,7 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainHeaderReader, header *t currentValidators := snap.ValidatorSet.Copy().Validators // sort validator by address sort.Sort(ValidatorsByAddress(currentValidators)) + for i, validator := range currentValidators { copy(validatorsBytes[i*validatorHeaderBytesLength:], validator.HeaderBytes()) } @@ -458,6 +469,7 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainHeaderReader, header *t } // snapshot retrieves the authorization snapshot at a given point in time. +// nolint: gocognit func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash common.Hash, parents []*types.Header) (*Snapshot, error) { // Search for a snapshot in memory or on disk for checkpoints var ( @@ -465,10 +477,12 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co snap *Snapshot ) + //nolint:govet for snap == nil { // If an in-memory snapshot was found, use that if s, ok := c.recents.Get(hash); ok { snap = s.(*Snapshot) + break } @@ -476,7 +490,9 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co if number%checkpointInterval == 0 { if s, err := loadSnapshot(c.config, c.signatures, c.db, hash, c.ethAPI); err == nil { log.Trace("Loaded snapshot from disk", "number", number, "hash", hash) + snap = s + break } } @@ -486,6 +502,7 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co // up more headers than allowed to be reorged (chain reinit from a freezer), // consider the checkpoint trusted and snapshot it. // TODO fix this + // nolint:nestif if number == 0 { checkpoint := chain.GetHeaderByNumber(number) if checkpoint != nil { @@ -503,7 +520,9 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co if err := snap.store(c.db); err != nil { return nil, err } + log.Info("Stored checkpoint snapshot to disk", "number", number, "hash", hash) + break } } @@ -516,6 +535,7 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co if header.Hash() != hash || header.Number.Uint64() != number { return nil, consensus.ErrUnknownAncestor } + parents = parents[:len(parents)-1] } else { // No explicit parents (or no more left), reach out to the database @@ -524,6 +544,7 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co return nil, consensus.ErrUnknownAncestor } } + headers = append(headers, header) number, hash = number-1, header.ParentHash } @@ -542,6 +563,7 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co if err != nil { return nil, err } + c.recents.Add(snap.Hash, snap) // If we've generated a new checkpoint snapshot, save to disk @@ -549,8 +571,10 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co if err = snap.store(c.db); err != nil { return nil, err } + log.Trace("Stored snapshot to disk", "number", snap.Number, "hash", snap.Hash) } + return snap, err } @@ -560,6 +584,7 @@ func (c *Bor) VerifyUncles(chain consensus.ChainReader, block *types.Block) erro if len(block.Uncles()) > 0 { return errors.New("uncles not allowed") } + return nil } @@ -590,6 +615,7 @@ func (c *Bor) verifySeal(chain consensus.ChainHeaderReader, header *types.Header if err != nil { return err } + if !snap.ValidatorSet.HasAddress(signer.Bytes()) { // Check the UnauthorizedSignerError.Error() msg to see why we pass number-1 return &UnauthorizedSignerError{number - 1, signer.Bytes()} @@ -643,6 +669,7 @@ func (c *Bor) Prepare(chain consensus.ChainHeaderReader, header *types.Header) e if len(header.Extra) < extraVanity { header.Extra = append(header.Extra, bytes.Repeat([]byte{0x00}, extraVanity-len(header.Extra))...) } + header.Extra = header.Extra[:extraVanity] // get validator set if number @@ -654,6 +681,7 @@ func (c *Bor) Prepare(chain consensus.ChainHeaderReader, header *types.Header) e // sort validator by address sort.Sort(ValidatorsByAddress(newValidators)) + for _, validator := range newValidators { header.Extra = append(header.Extra, validator.HeaderBytes()...) } @@ -673,7 +701,7 @@ func (c *Bor) Prepare(chain consensus.ChainHeaderReader, header *types.Header) e var succession int // if signer is not empty - if bytes.Compare(c.signer.Bytes(), common.Address{}.Bytes()) != 0 { + if !bytes.Equal(c.signer.Bytes(), common.Address{}.Bytes()) { succession, err = snap.GetSignerSuccessionNumber(c.signer) if err != nil { return err @@ -684,6 +712,7 @@ func (c *Bor) Prepare(chain consensus.ChainHeaderReader, header *types.Header) e if header.Time < uint64(time.Now().Unix()) { header.Time = uint64(time.Now().Unix()) } + return nil } @@ -693,7 +722,9 @@ func (c *Bor) Finalize(chain consensus.ChainHeaderReader, header *types.Header, stateSyncData := []*types.StateSyncData{} var err error + headerNumber := header.Number.Uint64() + if headerNumber%c.config.Sprint == 0 { cx := chainContext{Chain: chain, Bor: c} // check and commit span @@ -728,13 +759,17 @@ func (c *Bor) Finalize(chain consensus.ChainHeaderReader, header *types.Header, func decodeGenesisAlloc(i interface{}) (core.GenesisAlloc, error) { var alloc core.GenesisAlloc + b, err := json.Marshal(i) + if err != nil { return nil, err } + if err := json.Unmarshal(b, &alloc); err != nil { return nil, err } + return alloc, nil } @@ -745,12 +780,14 @@ func (c *Bor) changeContractCodeIfNeeded(headerNumber uint64, state *state.State if err != nil { return fmt.Errorf("failed to decode genesis alloc: %v", err) } + for addr, account := range allocs { log.Info("change contract code", "address", addr) state.SetCode(addr, account.Code) } } } + return nil } @@ -856,10 +893,12 @@ func (c *Bor) Seal(chain consensus.ChainHeaderReader, block *types.Block, result if err != nil { return err } + copy(header.Extra[len(header.Extra)-extraSeal:], sighash) // Wait until sealing is terminated or delay timeout. log.Trace("Waiting for slot to sign and propagate", "delay", common.PrettyDuration(delay)) + go func() { select { case <-stop: @@ -874,6 +913,7 @@ func (c *Bor) Seal(chain consensus.ChainHeaderReader, block *types.Block, result "in-turn-signer", snap.ValidatorSet.GetProposer().Address.Hex(), ) } + log.Info( "Sealing successful", "number", number, @@ -887,6 +927,7 @@ func (c *Bor) Seal(chain consensus.ChainHeaderReader, block *types.Block, result log.Warn("Sealing result was not read by miner", "number", number, "sealhash", SealHash(header, c.config)) } }() + return nil } @@ -898,6 +939,7 @@ func (c *Bor) CalcDifficulty(chain consensus.ChainHeaderReader, time uint64, par if err != nil { return nil } + return new(big.Int).SetUint64(snap.Difficulty(c.signer)) } @@ -948,6 +990,7 @@ func (c *Bor) GetCurrentSpan(headerHash common.Hash) (*Span, error) { To: &toAddress, Data: &msgData, }, blockNr, nil) + if err != nil { return nil, err } @@ -998,15 +1041,16 @@ func (c *Bor) GetCurrentValidators(headerHash common.Hash, blockNumber uint64) ( To: &toAddress, Data: &msgData, }, blockNr, nil) + if err != nil { panic(err) - // return nil, err } var ( ret0 = new([]common.Address) ret1 = new([]*big.Int) ) + out := &[]interface{}{ ret0, ret1, @@ -1034,13 +1078,16 @@ func (c *Bor) checkAndCommitSpan( ) error { headerNumber := header.Number.Uint64() span, err := c.GetCurrentSpan(header.ParentHash) + if err != nil { return err } + if c.needToCommitSpan(span, headerNumber) { err := c.fetchAndCommitSpan(span.ID+1, state, header, chain) return err } + return nil } @@ -1072,10 +1119,11 @@ func (c *Bor) fetchAndCommitSpan( var heimdallSpan HeimdallSpan if c.WithoutHeimdall { - s, err := c.getNextHeimdallSpanForTest(newSpanID, state, header, chain) + s, err := c.getNextHeimdallSpanForTest(newSpanID, header, chain) if err != nil { return err } + heimdallSpan = *s } else { response, err := c.HeimdallClient.FetchWithRetry(fmt.Sprintf("bor/span/%d", newSpanID), "") @@ -1102,7 +1150,9 @@ func (c *Bor) fetchAndCommitSpan( for _, val := range heimdallSpan.ValidatorSet.Validators { validators = append(validators, val.MinimalVal()) } + validatorBytes, err := rlp.EncodeToBytes(validators) + if err != nil { return err } @@ -1112,13 +1162,16 @@ func (c *Bor) fetchAndCommitSpan( for _, val := range heimdallSpan.SelectedProducers { producers = append(producers, val.MinimalVal()) } + producerBytes, err := rlp.EncodeToBytes(producers) + if err != nil { return err } // method method := "commitSpan" + log.Info("✅ Committing new span", "id", heimdallSpan.ID, "startBlock", heimdallSpan.StartBlock, @@ -1156,6 +1209,7 @@ func (c *Bor) CommitStates( stateSyncs := make([]*types.StateSyncData, 0) number := header.Number.Uint64() _lastStateID, err := c.GenesisContractsClient.LastStateId(number - 1) + if err != nil { return nil, err } @@ -1166,7 +1220,13 @@ func (c *Bor) CommitStates( "Fetching state updates from Heimdall", "fromID", lastStateID+1, "to", to.Format(time.RFC3339)) + eventRecords, err := c.HeimdallClient.FetchStateSyncEvents(lastStateID+1, to.Unix()) + + if err != nil { + log.Error("Error occurred when fetching state sync events", err) + } + if c.config.OverrideStateSyncRecords != nil { if val, ok := c.config.OverrideStateSyncRecords[strconv.FormatUint(number, 10)]; ok { eventRecords = eventRecords[0:val] @@ -1174,10 +1234,12 @@ func (c *Bor) CommitStates( } chainID := c.chainConfig.ChainID.String() + for _, eventRecord := range eventRecords { if eventRecord.ID <= lastStateID { continue } + if err := validateEventRecord(eventRecord, number, to, lastStateID, chainID); err != nil { log.Error(err.Error()) break @@ -1196,6 +1258,7 @@ func (c *Bor) CommitStates( } lastStateID++ } + return stateSyncs, nil } @@ -1204,6 +1267,7 @@ func validateEventRecord(eventRecord *EventRecordWithTime, number uint64, to tim if lastStateID+1 != eventRecord.ID || eventRecord.ChainID != chainID || !eventRecord.Time.Before(to) { return &InvalidStateReceivedError{number, lastStateID, &to, eventRecord} } + return nil } @@ -1217,12 +1281,12 @@ func (c *Bor) SetHeimdallClient(h IHeimdallClient) { func (c *Bor) getNextHeimdallSpanForTest( newSpanID uint64, - state *state.StateDB, header *types.Header, chain core.ChainContext, ) (*HeimdallSpan, error) { headerNumber := header.Number.Uint64() span, err := c.GetCurrentSpan(header.ParentHash) + if err != nil { return nil, err } @@ -1242,12 +1306,14 @@ func (c *Bor) getNextHeimdallSpanForTest( } else { span.StartBlock = span.EndBlock + 1 } + span.EndBlock = span.StartBlock + (100 * c.config.Sprint) - 1 selectedProducers := make([]Validator, len(snap.ValidatorSet.Validators)) for i, v := range snap.ValidatorSet.Validators { selectedProducers[i] = *v } + heimdallSpan := &HeimdallSpan{ Span: *span, ValidatorSet: *snap.ValidatorSet, @@ -1335,10 +1401,11 @@ func applyMessage( func validatorContains(a []*Validator, x *Validator) (*Validator, bool) { for _, n := range a { - if bytes.Compare(n.Address.Bytes(), x.Address.Bytes()) == 0 { + if bytes.Equal(n.Address.Bytes(), x.Address.Bytes()) { return n, true } } + return nil, false } @@ -1347,6 +1414,7 @@ func getUpdatedValidatorSet(oldValidatorSet *ValidatorSet, newVals []*Validator) oldVals := v.Validators var changes []*Validator + for _, ov := range oldVals { if f, ok := validatorContains(newVals, ov); ok { ov.VotingPower = f.VotingPower @@ -1363,7 +1431,10 @@ func getUpdatedValidatorSet(oldValidatorSet *ValidatorSet, newVals []*Validator) } } - v.UpdateWithChangeSet(changes) + if err := v.UpdateWithChangeSet(changes); err != nil { + log.Error("Error while updating change set", err) + } + return v } diff --git a/consensus/bor/bor_test.go b/consensus/bor/bor_test.go index b0ebc96861..6225532ca9 100644 --- a/consensus/bor/bor_test.go +++ b/consensus/bor/bor_test.go @@ -4,6 +4,8 @@ import ( "math/big" "testing" + "github.com/stretchr/testify/assert" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core" @@ -12,10 +14,11 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/params" - "github.com/stretchr/testify/assert" ) func TestGenesisContractChange(t *testing.T) { + t.Parallel() + addr0 := common.Address{0x1} b := &Bor{ @@ -101,6 +104,8 @@ func TestGenesisContractChange(t *testing.T) { } func TestEncodeSigHeaderJaipur(t *testing.T) { + t.Parallel() + // As part of the EIP-1559 fork in mumbai, an incorrect seal hash // was used for Bor that did not included the BaseFee. The Jaipur // block is a hard fork to fix that. diff --git a/consensus/bor/clerk.go b/consensus/bor/clerk.go index d7e6982873..ca610282a5 100644 --- a/consensus/bor/clerk.go +++ b/consensus/bor/clerk.go @@ -23,7 +23,7 @@ type EventRecordWithTime struct { Time time.Time `json:"record_time" yaml:"record_time"` } -// String returns the string representatin of span +// String returns the string representations of span func (e *EventRecordWithTime) String() string { return fmt.Sprintf( "id %v, contract %v, data: %v, txHash: %v, logIndex: %v, chainId: %v, time %s", diff --git a/consensus/bor/genesis_contracts_client.go b/consensus/bor/genesis_contracts_client.go index 582358e0cb..2b36b6e5e0 100644 --- a/consensus/bor/genesis_contracts_client.go +++ b/consensus/bor/genesis_contracts_client.go @@ -38,6 +38,7 @@ func NewGenesisContractsClient( ) *GenesisContractsClient { vABI, _ := abi.JSON(strings.NewReader(validatorsetABI)) sABI, _ := abi.JSON(strings.NewReader(stateReceiverABI)) + return &GenesisContractsClient{ validatorSetABI: vABI, stateReceiverABI: sABI, @@ -56,21 +57,27 @@ func (gc *GenesisContractsClient) CommitState( ) error { eventRecord := event.BuildEventRecord() recordBytes, err := rlp.EncodeToBytes(eventRecord) + if err != nil { return err } + method := "commitState" t := event.Time.Unix() data, err := gc.stateReceiverABI.Pack(method, big.NewInt(0).SetInt64(t), recordBytes) + if err != nil { log.Error("Unable to pack tx for commitState", "error", err) return err } + log.Info("→ committing new state", "eventRecord", event.String()) + msg := getSystemMessage(common.HexToAddress(gc.StateReceiverContract), data) if err := applyMessage(msg, state, header, gc.chainConfig, chCtx); err != nil { return err } + return nil } @@ -78,6 +85,7 @@ func (gc *GenesisContractsClient) LastStateId(snapshotNumber uint64) (*big.Int, blockNr := rpc.BlockNumber(snapshotNumber) method := "lastStateId" data, err := gc.stateReceiverABI.Pack(method) + if err != nil { log.Error("Unable to pack tx for LastStateId", "error", err) return nil, err @@ -91,6 +99,7 @@ func (gc *GenesisContractsClient) LastStateId(snapshotNumber uint64) (*big.Int, To: &toAddress, Data: &msgData, }, rpc.BlockNumberOrHash{BlockNumber: &blockNr}, nil) + if err != nil { return nil, err } @@ -99,5 +108,6 @@ func (gc *GenesisContractsClient) LastStateId(snapshotNumber uint64) (*big.Int, if err := gc.stateReceiverABI.UnpackIntoInterface(ret, method, result); err != nil { return nil, err } + return *ret, nil } diff --git a/consensus/bor/merkle.go b/consensus/bor/merkle.go index bdfbaba983..5c61eb4a12 100644 --- a/consensus/bor/merkle.go +++ b/consensus/bor/merkle.go @@ -2,12 +2,12 @@ package bor func appendBytes32(data ...[]byte) []byte { var result []byte + for _, v := range data { - paddedV, err := convertTo32(v) - if err == nil { - result = append(result, paddedV[:]...) - } + paddedV := convertTo32(v) + result = append(result, paddedV[:]...) } + return result } @@ -24,25 +24,29 @@ func nextPowerOfTwo(n uint64) uint64 { n |= n >> 16 n |= n >> 32 n++ + return n } -func convertTo32(input []byte) (output [32]byte, err error) { +func convertTo32(input []byte) (output [32]byte) { l := len(input) if l > 32 || l == 0 { return } + copy(output[32-l:], input[:]) + return } func convert(input []([32]byte)) [][]byte { var output [][]byte + for _, in := range input { newInput := make([]byte, len(in[:])) copy(newInput, in[:]) output = append(output, newInput) - } + return output } diff --git a/consensus/bor/rest.go b/consensus/bor/rest.go index 3ef531de18..49ecb38af5 100644 --- a/consensus/bor/rest.go +++ b/consensus/bor/rest.go @@ -40,39 +40,49 @@ func NewHeimdallClient(urlString string) (*HeimdallClient, error) { h := &HeimdallClient{ urlString: urlString, client: http.Client{ - Timeout: time.Duration(5 * time.Second), + Timeout: 5 * time.Second, }, closeCh: make(chan struct{}), } + return h, nil } func (h *HeimdallClient) FetchStateSyncEvents(fromID uint64, to int64) ([]*EventRecordWithTime, error) { eventRecords := make([]*EventRecordWithTime, 0) + for { queryParams := fmt.Sprintf("from-id=%d&to-time=%d&limit=%d", fromID, to, stateFetchLimit) log.Info("Fetching state sync events", "queryParams", queryParams) response, err := h.FetchWithRetry("clerk/event-record/list", queryParams) + if err != nil { return nil, err } + var _eventRecords []*EventRecordWithTime + if response.Result == nil { // status 204 break } + if err := json.Unmarshal(response.Result, &_eventRecords); err != nil { return nil, err } + eventRecords = append(eventRecords, _eventRecords...) + if len(_eventRecords) < stateFetchLimit { break } + fromID += uint64(stateFetchLimit) } sort.SliceStable(eventRecords, func(i, j int) bool { return eventRecords[i].ID < eventRecords[j].ID }) + return eventRecords, nil } @@ -130,7 +140,7 @@ func (h *HeimdallClient) FetchWithRetry(rawPath string, rawQuery string) (*Respo // internal fetch method func (h *HeimdallClient) internalFetch(u *url.URL) (*ResponseWithHeight, error) { - res, err := h.client.Get(u.String()) + res, err := h.client.Get(u.String()) // nolint: noctx if err != nil { return nil, err } diff --git a/consensus/bor/snapshot.go b/consensus/bor/snapshot.go index 8d212f33ef..ef92cc99b4 100644 --- a/consensus/bor/snapshot.go +++ b/consensus/bor/snapshot.go @@ -25,13 +25,6 @@ type Snapshot struct { Recents map[uint64]common.Address `json:"recents"` // Set of recent signers for spam protections } -// signersAscending implements the sort interface to allow sorting a list of addresses -type signersAscending []common.Address - -func (s signersAscending) Len() int { return len(s) } -func (s signersAscending) Less(i, j int) bool { return bytes.Compare(s[i][:], s[j][:]) < 0 } -func (s signersAscending) Swap(i, j int) { s[i], s[j] = s[j], s[i] } - // newSnapshot creates a new snapshot with the specified startup parameters. This // method does not initialize the set of recent signers, so only ever use if for // the genesis block. @@ -52,6 +45,7 @@ func newSnapshot( ValidatorSet: NewValidatorSet(validators), Recents: make(map[uint64]common.Address), } + return snap } @@ -61,10 +55,13 @@ func loadSnapshot(config *params.BorConfig, sigcache *lru.ARCCache, db ethdb.Dat if err != nil { return nil, err } + snap := new(Snapshot) + if err := json.Unmarshal(blob, snap); err != nil { return nil, err } + snap.config = config snap.sigcache = sigcache snap.ethAPI = ethAPI @@ -83,6 +80,7 @@ func (s *Snapshot) store(db ethdb.Database) error { if err != nil { return err } + return db.Put(append([]byte("bor-"), s.Hash[:]...), blob) } @@ -115,6 +113,7 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) { return nil, errOutOfRangeChain } } + if headers[0].Number.Uint64() != s.Number+1 { return nil, errOutOfRangeChain } @@ -126,7 +125,7 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) { number := header.Number.Uint64() // Delete the oldest signer from the recent list to allow it signing again - if number >= s.config.Sprint && number-s.config.Sprint >= 0 { + if number >= s.config.Sprint { delete(snap.Recents, number-s.config.Sprint) } @@ -153,6 +152,7 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) { if err := validateHeaderExtraField(header.Extra); err != nil { return nil, err } + validatorBytes := header.Extra[extraVanity : len(header.Extra)-extraSeal] // get validators from headers and use that for new validator set @@ -162,6 +162,7 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) { snap.ValidatorSet = v } } + snap.Number += uint64(len(headers)) snap.Hash = headers[len(headers)-1].Hash() @@ -173,10 +174,13 @@ func (s *Snapshot) GetSignerSuccessionNumber(signer common.Address) (int, error) validators := s.ValidatorSet.Validators proposer := s.ValidatorSet.GetProposer().Address proposerIndex, _ := s.ValidatorSet.GetByAddress(proposer) + if proposerIndex == -1 { return -1, &UnauthorizedProposerError{s.Number, proposer.Bytes()} } + signerIndex, _ := s.ValidatorSet.GetByAddress(signer) + if signerIndex == -1 { return -1, &UnauthorizedSignerError{s.Number, signer.Bytes()} } @@ -187,6 +191,7 @@ func (s *Snapshot) GetSignerSuccessionNumber(signer common.Address) (int, error) tempIndex = tempIndex + len(validators) } } + return tempIndex - proposerIndex, nil } @@ -196,13 +201,14 @@ func (s *Snapshot) signers() []common.Address { for _, sig := range s.ValidatorSet.Validators { sigs = append(sigs, sig.Address) } + return sigs } // Difficulty returns the difficulty for a particular signer at the current snapshot number func (s *Snapshot) Difficulty(signer common.Address) uint64 { // if signer is empty - if bytes.Compare(signer.Bytes(), common.Address{}.Bytes()) == 0 { + if bytes.Equal(signer.Bytes(), common.Address{}.Bytes()) { return 1 } diff --git a/consensus/bor/snapshot_test.go b/consensus/bor/snapshot_test.go index 6bb8547843..2bbbc32123 100644 --- a/consensus/bor/snapshot_test.go +++ b/consensus/bor/snapshot_test.go @@ -16,6 +16,8 @@ const ( ) func TestGetSignerSuccessionNumber_ProposerIsSigner(t *testing.T) { + t.Parallel() + validators := buildRandomValidatorSet(numVals) validatorSet := NewValidatorSet(validators) snap := Snapshot{ @@ -25,17 +27,22 @@ func TestGetSignerSuccessionNumber_ProposerIsSigner(t *testing.T) { // proposer is signer signer := validatorSet.Proposer.Address successionNumber, err := snap.GetSignerSuccessionNumber(signer) + if err != nil { t.Fatalf("%s", err) } + assert.Equal(t, 0, successionNumber) } func TestGetSignerSuccessionNumber_SignerIndexIsLarger(t *testing.T) { + t.Parallel() + validators := buildRandomValidatorSet(numVals) // sort validators by address, which is what NewValidatorSet also does sort.Sort(ValidatorsByAddress(validators)) + proposerIndex := 32 signerIndex := 56 // give highest ProposerPriority to a particular val, so that they become the proposer @@ -47,13 +54,17 @@ func TestGetSignerSuccessionNumber_SignerIndexIsLarger(t *testing.T) { // choose a signer at an index greater than proposer index signer := snap.ValidatorSet.Validators[signerIndex].Address successionNumber, err := snap.GetSignerSuccessionNumber(signer) + if err != nil { t.Fatalf("%s", err) } + assert.Equal(t, signerIndex-proposerIndex, successionNumber) } func TestGetSignerSuccessionNumber_SignerIndexIsSmaller(t *testing.T) { + t.Parallel() + validators := buildRandomValidatorSet(numVals) proposerIndex := 98 signerIndex := 11 @@ -66,13 +77,17 @@ func TestGetSignerSuccessionNumber_SignerIndexIsSmaller(t *testing.T) { // choose a signer at an index greater than proposer index signer := snap.ValidatorSet.Validators[signerIndex].Address successionNumber, err := snap.GetSignerSuccessionNumber(signer) + if err != nil { t.Fatalf("%s", err) } + assert.Equal(t, signerIndex+numVals-proposerIndex, successionNumber) } func TestGetSignerSuccessionNumber_ProposerNotFound(t *testing.T) { + t.Parallel() + validators := buildRandomValidatorSet(numVals) snap := Snapshot{ ValidatorSet: NewValidatorSet(validators), @@ -89,6 +104,8 @@ func TestGetSignerSuccessionNumber_ProposerNotFound(t *testing.T) { } func TestGetSignerSuccessionNumber_SignerNotFound(t *testing.T) { + t.Parallel() + validators := buildRandomValidatorSet(numVals) snap := Snapshot{ ValidatorSet: NewValidatorSet(validators), @@ -101,9 +118,12 @@ func TestGetSignerSuccessionNumber_SignerNotFound(t *testing.T) { assert.Equal(t, dummySignerAddress.Bytes(), e.Signer) } +// nolint: unparam func buildRandomValidatorSet(numVals int) []*Validator { rand.Seed(time.Now().Unix()) + validators := make([]*Validator, numVals) + for i := 0; i < numVals; i++ { validators[i] = &Validator{ Address: randomAddress(), @@ -114,11 +134,13 @@ func buildRandomValidatorSet(numVals int) []*Validator { // sort validators by address, which is what NewValidatorSet also does sort.Sort(ValidatorsByAddress(validators)) + return validators } func randomAddress() common.Address { bytes := make([]byte, 32) rand.Read(bytes) + return common.BytesToAddress(bytes) } diff --git a/consensus/bor/validator.go b/consensus/bor/validator.go index 00e9fdc645..57905fdf89 100644 --- a/consensus/bor/validator.go +++ b/consensus/bor/validator.go @@ -43,9 +43,12 @@ func (v *Validator) Cmp(other *Validator) *Validator { if v == nil { return other } + if other == nil { return v } + + // nolint:nestif if v.ProposerPriority > other.ProposerPriority { return v } else if v.ProposerPriority < other.ProposerPriority { @@ -66,6 +69,7 @@ func (v *Validator) String() string { if v == nil { return "nil-Validator" } + return fmt.Sprintf("Validator{%v Power:%v Priority:%v}", v.Address.Hex(), v.VotingPower, @@ -87,6 +91,7 @@ func (v *Validator) HeaderBytes() []byte { result := make([]byte, 40) copy(result[:20], v.Address.Bytes()) copy(result[20:], v.PowerBytes()) + return result } @@ -95,6 +100,7 @@ func (v *Validator) PowerBytes() []byte { powerBytes := big.NewInt(0).SetInt64(v.VotingPower).Bytes() result := make([]byte, 20) copy(result[20-len(powerBytes):], powerBytes) + return result } @@ -114,6 +120,7 @@ func ParseValidators(validatorsBytes []byte) ([]*Validator, error) { } result := make([]*Validator, len(validatorsBytes)/40) + for i := 0; i < len(validatorsBytes); i += 40 { address := make([]byte, 20) power := make([]byte, 20) @@ -142,6 +149,7 @@ func SortMinimalValByAddress(a []MinimalVal) []MinimalVal { sort.Slice(a, func(i, j int) bool { return bytes.Compare(a[i].Signer.Bytes(), a[j].Signer.Bytes()) < 0 }) + return a } @@ -150,5 +158,6 @@ func ValidatorsToMinimalValidators(vals []Validator) (minVals []MinimalVal) { for _, val := range vals { minVals = append(minVals, val.MinimalVal()) } + return } diff --git a/consensus/bor/validator_set.go b/consensus/bor/validator_set.go index 0b5c10ebd0..892891bbda 100644 --- a/consensus/bor/validator_set.go +++ b/consensus/bor/validator_set.go @@ -56,12 +56,15 @@ type ValidatorSet struct { func NewValidatorSet(valz []*Validator) *ValidatorSet { vals := &ValidatorSet{} err := vals.updateWithChangeSet(valz, false) + if err != nil { panic(fmt.Sprintf("cannot create validator set: %s", err)) } + if len(valz) > 0 { vals.IncrementProposerPriority(1) } + return vals } @@ -72,9 +75,10 @@ func (vals *ValidatorSet) IsNilOrEmpty() bool { // Increment ProposerPriority and update the proposer on a copy, and return it. func (vals *ValidatorSet) CopyIncrementProposerPriority(times int) *ValidatorSet { - copy := vals.Copy() - copy.IncrementProposerPriority(times) - return copy + validatorCopy := vals.Copy() + validatorCopy.IncrementProposerPriority(times) + + return validatorCopy } // IncrementProposerPriority increments ProposerPriority of each validator and updates the @@ -84,6 +88,7 @@ func (vals *ValidatorSet) IncrementProposerPriority(times int) { if vals.IsNilOrEmpty() { panic("empty validator set") } + if times <= 0 { panic("Cannot call IncrementProposerPriority with non-positive times") } @@ -120,6 +125,7 @@ func (vals *ValidatorSet) RescalePriorities(diffMax int64) { // NOTE: This may make debugging priority issues easier as well. diff := computeMaxMinPriorityDiff(vals) ratio := (diff + diffMax - 1) / diffMax + if diff > diffMax { for _, val := range vals.Validators { val.ProposerPriority = val.ProposerPriority / ratio @@ -145,10 +151,13 @@ func (vals *ValidatorSet) incrementProposerPriority() *Validator { func (vals *ValidatorSet) computeAvgProposerPriority() int64 { n := int64(len(vals.Validators)) sum := big.NewInt(0) + for _, val := range vals.Validators { sum.Add(sum, big.NewInt(val.ProposerPriority)) } + avg := sum.Div(sum, big.NewInt(n)) + if avg.IsInt64() { return avg.Int64() } @@ -162,17 +171,22 @@ func computeMaxMinPriorityDiff(vals *ValidatorSet) int64 { if vals.IsNilOrEmpty() { panic("empty validator set") } + max := int64(math.MinInt64) min := int64(math.MaxInt64) + for _, v := range vals.Validators { if v.ProposerPriority < min { min = v.ProposerPriority } + if v.ProposerPriority > max { max = v.ProposerPriority } } + diff := max - min + if diff < 0 { return -1 * diff } else { @@ -185,6 +199,7 @@ func (vals *ValidatorSet) getValWithMostPriority() *Validator { for _, val := range vals.Validators { res = res.Cmp(val) } + return res } @@ -192,7 +207,9 @@ func (vals *ValidatorSet) shiftByAvgProposerPriority() { if vals.IsNilOrEmpty() { panic("empty validator set") } + avgProposerPriority := vals.computeAvgProposerPriority() + for _, val := range vals.Validators { val.ProposerPriority = safeSubClip(val.ProposerPriority, avgProposerPriority) } @@ -203,10 +220,13 @@ func validatorListCopy(valsList []*Validator) []*Validator { if valsList == nil { return nil } + valsCopy := make([]*Validator, len(valsList)) + for i, val := range valsList { valsCopy[i] = val.Copy() } + return valsCopy } @@ -225,6 +245,7 @@ func (vals *ValidatorSet) HasAddress(address []byte) bool { idx := sort.Search(len(vals.Validators), func(i int) bool { return bytes.Compare(address, vals.Validators[i].Address.Bytes()) <= 0 }) + return idx < len(vals.Validators) && bytes.Equal(vals.Validators[idx].Address.Bytes(), address) } @@ -237,6 +258,7 @@ func (vals *ValidatorSet) GetByAddress(address common.Address) (index int, val * if idx < len(vals.Validators) && bytes.Equal(vals.Validators[idx].Address.Bytes(), address.Bytes()) { return idx, vals.Validators[idx].Copy() } + return -1, nil } @@ -247,7 +269,9 @@ func (vals *ValidatorSet) GetByIndex(index int) (address []byte, val *Validator) if index < 0 || index >= len(vals.Validators) { return nil, nil } + val = vals.Validators[index] + return val.Address.Bytes(), val.Copy() } @@ -258,7 +282,6 @@ func (vals *ValidatorSet) Size() int { // Force recalculation of the set's total voting power. func (vals *ValidatorSet) updateTotalVotingPower() error { - sum := int64(0) for _, val := range vals.Validators { // mind overflow @@ -267,7 +290,9 @@ func (vals *ValidatorSet) updateTotalVotingPower() error { return &TotalVotingPowerExceededError{sum, vals.Validators} } } + vals.totalVotingPower = sum + return nil } @@ -276,11 +301,13 @@ func (vals *ValidatorSet) updateTotalVotingPower() error { func (vals *ValidatorSet) TotalVotingPower() int64 { if vals.totalVotingPower == 0 { log.Info("invoking updateTotalVotingPower before returning it") + if err := vals.updateTotalVotingPower(); err != nil { // Can/should we do better? panic(err) } } + return vals.totalVotingPower } @@ -290,9 +317,11 @@ func (vals *ValidatorSet) GetProposer() (proposer *Validator) { if len(vals.Validators) == 0 { return nil } + if vals.Proposer == nil { vals.Proposer = vals.findProposer() } + return vals.Proposer.Copy() } @@ -303,6 +332,7 @@ func (vals *ValidatorSet) findProposer() *Validator { proposer = proposer.Cmp(val) } } + return proposer } @@ -343,6 +373,7 @@ func processChanges(origChanges []*Validator) (updates, removals []*Validator, e removals = make([]*Validator, 0, len(changes)) updates = make([]*Validator, 0, len(changes)) + var prevAddr common.Address // Scan changes by address and append valid validators to updates or removals lists. @@ -351,22 +382,27 @@ func processChanges(origChanges []*Validator) (updates, removals []*Validator, e err = fmt.Errorf("duplicate entry %v in %v", valUpdate, changes) return nil, nil, err } + if valUpdate.VotingPower < 0 { err = fmt.Errorf("voting power can't be negative: %v", valUpdate) return nil, nil, err } + if valUpdate.VotingPower > MaxTotalVotingPower { err = fmt.Errorf("to prevent clipping/ overflow, voting power can't be higher than %v: %v ", MaxTotalVotingPower, valUpdate) return nil, nil, err } + if valUpdate.VotingPower == 0 { removals = append(removals, valUpdate) } else { updates = append(updates, valUpdate) } + prevAddr = valUpdate.Address } + return updates, removals, err } @@ -382,12 +418,12 @@ func processChanges(origChanges []*Validator) (updates, removals []*Validator, e // by processChanges for duplicates and invalid values. // No changes are made to the validator set 'vals'. func verifyUpdates(updates []*Validator, vals *ValidatorSet) (updatedTotalVotingPower int64, numNewValidators int, err error) { - updatedTotalVotingPower = vals.TotalVotingPower() for _, valUpdate := range updates { address := valUpdate.Address _, val := vals.GetByAddress(address) + if val == nil { // New validator, add its voting power the the total. updatedTotalVotingPower += valUpdate.VotingPower @@ -396,11 +432,14 @@ func verifyUpdates(updates []*Validator, vals *ValidatorSet) (updatedTotalVoting // Updated validator, add the difference in power to the total. updatedTotalVotingPower += valUpdate.VotingPower - val.VotingPower } + overflow := updatedTotalVotingPower > MaxTotalVotingPower + if overflow { err = fmt.Errorf( "failed to add/update validator %v, total voting power would exceed the max allowed %v", valUpdate, MaxTotalVotingPower) + return 0, 0, err } } @@ -414,10 +453,10 @@ func verifyUpdates(updates []*Validator, vals *ValidatorSet) (updatedTotalVoting // 'updates' parameter must be a list of unique validators to be added or updated. // No changes are made to the validator set 'vals'. func computeNewPriorities(updates []*Validator, vals *ValidatorSet, updatedTotalVotingPower int64) { - for _, valUpdate := range updates { address := valUpdate.Address _, val := vals.GetByAddress(address) + if val == nil { // add val // Set ProposerPriority to -C*totalVotingPower (with C ~= 1.125) to make sure validators can't @@ -432,7 +471,6 @@ func computeNewPriorities(updates []*Validator, vals *ValidatorSet, updatedTotal valUpdate.ProposerPriority = val.ProposerPriority } } - } // Merges the vals' validator list with the updates list. @@ -440,7 +478,6 @@ func computeNewPriorities(updates []*Validator, vals *ValidatorSet, updatedTotal // Expects updates to be a list of updates sorted by address with no duplicates or errors, // must have been validated with verifyUpdates() and priorities computed with computeNewPriorities(). func (vals *ValidatorSet) applyUpdates(updates []*Validator) { - existing := vals.Validators merged := make([]*Validator, len(existing)+len(updates)) i := 0 @@ -478,24 +515,25 @@ func (vals *ValidatorSet) applyUpdates(updates []*Validator) { // Checks that the validators to be removed are part of the validator set. // No changes are made to the validator set 'vals'. func verifyRemovals(deletes []*Validator, vals *ValidatorSet) error { - for _, valUpdate := range deletes { address := valUpdate.Address _, val := vals.GetByAddress(address) + if val == nil { return fmt.Errorf("failed to find validator %X to remove", address) } } + if len(deletes) > len(vals.Validators) { panic("more deletes than validators") } + return nil } // Removes the validators specified in 'deletes' from validator set 'vals'. // Should not fail as verification has been done before. func (vals *ValidatorSet) applyRemovals(deletes []*Validator) { - existing := vals.Validators merged := make([]*Validator, len(existing)-len(deletes)) @@ -509,6 +547,7 @@ func (vals *ValidatorSet) applyRemovals(deletes []*Validator) { merged[i] = existing[0] i++ } + existing = existing[1:] } @@ -526,7 +565,6 @@ func (vals *ValidatorSet) applyRemovals(deletes []*Validator) { // are not allowed and will trigger an error if present in 'changes'. // The 'allowDeletes' flag is set to false by NewValidatorSet() and to true by UpdateWithChangeSet(). func (vals *ValidatorSet) updateWithChangeSet(changes []*Validator, allowDeletes bool) error { - if len(changes) <= 0 { return nil } @@ -596,19 +634,19 @@ func (vals *ValidatorSet) UpdateWithChangeSet(changes []*Validator) error { func IsErrTooMuchChange(err error) bool { switch err.(type) { - case errTooMuchChange: + case tooMuchChangeError: return true default: return false } } -type errTooMuchChange struct { +type tooMuchChangeError struct { got int64 needed int64 } -func (e errTooMuchChange) Error() string { +func (e tooMuchChangeError) Error() string { return fmt.Sprintf("Invalid commit -- insufficient old voting power: got %v, needed %v", e.got, e.needed) } @@ -622,11 +660,14 @@ func (vals *ValidatorSet) StringIndented(indent string) string { if vals == nil { return "nil-ValidatorSet" } + var valStrings []string + vals.Iterate(func(index int, val *Validator) bool { valStrings = append(valStrings, val.String()) return false }) + return fmt.Sprintf(`ValidatorSet{ %s Proposer: %v %s Validators: @@ -636,7 +677,6 @@ func (vals *ValidatorSet) StringIndented(indent string) string { indent, indent, strings.Join(valStrings, "\n"+indent+" "), indent) - } //------------------------------------- @@ -668,6 +708,7 @@ func safeAdd(a, b int64) (int64, bool) { } else if b < 0 && a < math.MinInt64-b { return -1, true } + return a + b, false } @@ -677,6 +718,7 @@ func safeSub(a, b int64) (int64, bool) { } else if b < 0 && a > math.MaxInt64+b { return -1, true } + return a - b, false } @@ -686,8 +728,10 @@ func safeAddClip(a, b int64) int64 { if b < 0 { return math.MinInt64 } + return math.MaxInt64 } + return c } @@ -697,7 +741,9 @@ func safeSubClip(a, b int64) int64 { if b > 0 { return math.MinInt64 } + return math.MaxInt64 } + return c } From 45a72bc49e40516b2d8969a3849d82655f0927f2 Mon Sep 17 00:00:00 2001 From: Jerry Date: Fri, 20 May 2022 13:49:37 -0700 Subject: [PATCH 2/3] Lint internal/cli --- internal/cli/account.go | 1 + internal/cli/account_import.go | 5 +++++ internal/cli/account_list.go | 5 +++++ internal/cli/account_new.go | 1 + internal/cli/attach.go | 27 ++++++++++++++++++++------- internal/cli/bootnode.go | 12 +++++++++++- internal/cli/bor_fingerprint.go | 12 +++++++++--- internal/cli/chain.go | 1 + internal/cli/chain_sethead.go | 4 ++++ internal/cli/chain_watch.go | 8 +++++++- internal/cli/command.go | 17 +++++++++++++++-- internal/cli/debug.go | 15 ++++++++++++--- internal/cli/markdown_test.go | 1 + internal/cli/peers.go | 1 + internal/cli/peers_add.go | 2 ++ internal/cli/peers_list.go | 5 +++++ internal/cli/peers_remove.go | 2 ++ internal/cli/peers_status.go | 4 ++++ internal/cli/status.go | 5 +++++ internal/cli/version.go | 2 ++ 20 files changed, 113 insertions(+), 17 deletions(-) diff --git a/internal/cli/account.go b/internal/cli/account.go index 7ce6c09b63..bb8b30b892 100644 --- a/internal/cli/account.go +++ b/internal/cli/account.go @@ -19,6 +19,7 @@ func (a *Account) MarkDown() string { "- [```account list```](./account_list.md): List the wallets in the Bor client.", "- [```account import```](./account_import.md): Import an account to the Bor client.", } + return strings.Join(items, "\n\n") } diff --git a/internal/cli/account_import.go b/internal/cli/account_import.go index d7ab14601a..a3f65ab512 100644 --- a/internal/cli/account_import.go +++ b/internal/cli/account_import.go @@ -20,6 +20,7 @@ func (a *AccountImportCommand) MarkDown() string { "The ```account import``` command imports an account in Json format to the Bor data directory.", a.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -58,7 +59,9 @@ func (a *AccountImportCommand) Run(args []string) int { a.UI.Error("Expected one argument") return 1 } + key, err := crypto.LoadECDSA(args[0]) + if err != nil { a.UI.Error(fmt.Sprintf("Failed to load the private key '%s': %v", args[0], err)) return 1 @@ -80,6 +83,8 @@ func (a *AccountImportCommand) Run(args []string) int { if err != nil { utils.Fatalf("Could not create the account: %v", err) } + a.UI.Output(fmt.Sprintf("Account created: %s", acct.Address.String())) + return 0 } diff --git a/internal/cli/account_list.go b/internal/cli/account_list.go index db77158b8e..854934c447 100644 --- a/internal/cli/account_list.go +++ b/internal/cli/account_list.go @@ -19,6 +19,7 @@ func (a *AccountListCommand) MarkDown() string { "The `account list` command lists all the accounts in the Bor data directory.", a.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -53,7 +54,9 @@ func (a *AccountListCommand) Run(args []string) int { a.UI.Error(fmt.Sprintf("Failed to get keystore: %v", err)) return 1 } + a.UI.Output(formatAccounts(keystore.Accounts())) + return 0 } @@ -64,10 +67,12 @@ func formatAccounts(accts []accounts.Account) string { rows := make([]string, len(accts)+1) rows[0] = "Index|Address" + for i, d := range accts { rows[i+1] = fmt.Sprintf("%d|%s", i, d.Address.String()) } + return formatList(rows) } diff --git a/internal/cli/account_new.go b/internal/cli/account_new.go index f6591fc53b..aef272a389 100644 --- a/internal/cli/account_new.go +++ b/internal/cli/account_new.go @@ -18,6 +18,7 @@ func (a *AccountNewCommand) MarkDown() string { "The `account new` command creates a new local account file on the Bor data directory. Bor should not be running to execute this command.", a.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } diff --git a/internal/cli/attach.go b/internal/cli/attach.go index df1c76ff3d..1aa888c12b 100644 --- a/internal/cli/attach.go +++ b/internal/cli/attach.go @@ -6,12 +6,12 @@ import ( "path/filepath" "strings" - "github.com/ethereum/go-ethereum/internal/cli/flagset" - "github.com/ethereum/go-ethereum/rpc" - "github.com/ethereum/go-ethereum/cmd/utils" "github.com/ethereum/go-ethereum/console" + "github.com/ethereum/go-ethereum/internal/cli/flagset" "github.com/ethereum/go-ethereum/node" + "github.com/ethereum/go-ethereum/rpc" + "github.com/mitchellh/cli" ) @@ -33,6 +33,7 @@ func (c *AttachCommand) MarkDown() string { "Connect to remote Bor IPC console.", c.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -49,7 +50,6 @@ func (c *AttachCommand) Synopsis() string { } func (c *AttachCommand) Flags() *flagset.Flagset { - f := flagset.NewFlagSet("attach") f.StringFlag(&flagset.StringFlag{ @@ -75,13 +75,13 @@ func (c *AttachCommand) Flags() *flagset.Flagset { // Run implements the cli.Command interface func (c *AttachCommand) Run(args []string) int { - flags := c.Flags() //check if first arg is flag or IPC location if len(args) == 0 { args = append(args, "") } + if args[0] != "" && strings.HasPrefix(args[0], "--") { if err := flags.Parse(args); err != nil { c.UI.Error(err.Error()) @@ -94,6 +94,7 @@ func (c *AttachCommand) Run(args []string) int { return 1 } } + if err := c.remoteConsole(); err != nil { c.UI.Error(err.Error()) return 1 @@ -104,25 +105,30 @@ func (c *AttachCommand) Run(args []string) int { // remoteConsole will connect to a remote bor instance, attaching a JavaScript // console to it. +// nolint: unparam func (c *AttachCommand) remoteConsole() error { // Attach to a remotely running geth instance and start the JavaScript console - path := node.DefaultDataDir() if c.Endpoint == "" { if c.Meta.dataDir != "" { path = c.Meta.dataDir } + if path != "" { homeDir, _ := os.UserHomeDir() path = filepath.Join(homeDir, "/.bor/data") } + c.Endpoint = fmt.Sprintf("%s/bor.ipc", path) } + client, err := dialRPC(c.Endpoint) + if err != nil { utils.Fatalf("Unable to attach to remote bor: %v", err) } + config := console.Config{ DataDir: path, DocRoot: c.JSpathFlag, @@ -134,7 +140,12 @@ func (c *AttachCommand) remoteConsole() error { if err != nil { utils.Fatalf("Failed to start the JavaScript console: %v", err) } - defer console.Stop(false) + + defer func() { + if err := console.Stop(false); err != nil { + c.UI.Error(err.Error()) + } + }() if c.ExecCMD != "" { console.Evaluate(c.ExecCMD) @@ -159,6 +170,7 @@ func dialRPC(endpoint string) (*rpc.Client, error) { // these prefixes. endpoint = endpoint[4:] } + return rpc.Dial(endpoint) } @@ -175,5 +187,6 @@ func (c *AttachCommand) makeConsolePreloads() []string { for _, file := range strings.Split(c.PreloadJSFlag, ",") { preloads = append(preloads, strings.TrimSpace(file)) } + return preloads } diff --git a/internal/cli/bootnode.go b/internal/cli/bootnode.go index f9127494a6..9e1a0fcde9 100644 --- a/internal/cli/bootnode.go +++ b/internal/cli/bootnode.go @@ -19,6 +19,7 @@ import ( "github.com/ethereum/go-ethereum/p2p/discover" "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/p2p/nat" + "github.com/mitchellh/cli" ) @@ -45,6 +46,7 @@ func (c *BootnodeCommand) MarkDown() string { "# Bootnode", c.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -103,6 +105,7 @@ func (b *BootnodeCommand) Synopsis() string { } // Run implements the cli.Command interface +// nolint: gocognit func (b *BootnodeCommand) Run(args []string) int { flags := b.Flags() if err := flags.Parse(args); err != nil { @@ -118,6 +121,7 @@ func (b *BootnodeCommand) Run(args []string) int { } else { glogger.Verbosity(log.LvlInfo) } + log.Root().SetHandler(glogger) natm, err := nat.Parse(b.nat) @@ -128,6 +132,7 @@ func (b *BootnodeCommand) Run(args []string) int { // create a one time key var nodeKey *ecdsa.PrivateKey + // nolint: nestif if b.nodeKey != "" { // try to read the key either from file or command line if _, err := os.Stat(b.nodeKey); errors.Is(err, os.ErrNotExist) { @@ -157,7 +162,7 @@ func (b *BootnodeCommand) Run(args []string) int { } // save the public key pubRaw := fmt.Sprintf("%x", crypto.FromECDSAPub(&nodeKey.PublicKey)[1:]) - if err := ioutil.WriteFile(filepath.Join(path, "pub.key"), []byte(pubRaw), 0755); err != nil { + if err := ioutil.WriteFile(filepath.Join(path, "pub.key"), []byte(pubRaw), 0600); err != nil { b.UI.Error(fmt.Sprintf("failed to write node pub key: %v", err)) return 1 } @@ -169,7 +174,9 @@ func (b *BootnodeCommand) Run(args []string) int { b.UI.Error(fmt.Sprintf("could not resolve udp addr '%s': %v", b.listenAddr, err)) return 1 } + conn, err := net.ListenUDP("udp", addr) + if err != nil { b.UI.Error(fmt.Sprintf("failed to listen udp addr '%s': %v", b.listenAddr, err)) return 1 @@ -180,7 +187,9 @@ func (b *BootnodeCommand) Run(args []string) int { if !realaddr.IP.IsLoopback() { go nat.Map(natm, nil, "udp", realaddr.Port, realaddr.Port, "ethereum discovery") } + if ext, err := natm.ExternalIP(); err == nil { + // nolint: govet realaddr = &net.UDPAddr{IP: ext, Port: realaddr.Port} } } @@ -198,6 +207,7 @@ func (b *BootnodeCommand) Run(args []string) int { PrivateKey: nodeKey, Log: log.Root(), } + if b.v5 { if _, err := discover.ListenV5(conn, ln, cfg); err != nil { utils.Fatalf("%v", err) diff --git a/internal/cli/bor_fingerprint.go b/internal/cli/bor_fingerprint.go index e9f4e70398..4e21f02956 100644 --- a/internal/cli/bor_fingerprint.go +++ b/internal/cli/bor_fingerprint.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/ethereum/go-ethereum/params" + "github.com/mitchellh/cli" "github.com/shirou/gopsutil/cpu" "github.com/shirou/gopsutil/disk" @@ -25,6 +26,7 @@ func (c *FingerprintCommand) MarkDown() string { "# Fingerprint", "Display the system fingerprint", } + return strings.Join(items, "\n\n") } @@ -45,6 +47,7 @@ func getCoresCount(cp []cpu.InfoStat) int { for i := 0; i < len(cp); i++ { cores += int(cp[i].Cores) } + return cores } @@ -76,6 +79,7 @@ func formatFingerprint(borFingerprint *BorFingerprint) string { fmt.Sprintf("RAM :: total : %v GB, free : %v GB, used : %v GB", borFingerprint.MemoryDetails.TotalMem, borFingerprint.MemoryDetails.FreeMem, borFingerprint.MemoryDetails.UsedMem), fmt.Sprintf("STORAGE :: total : %v GB, free : %v GB, used : %v GB", borFingerprint.DiskDetails.TotalDisk, borFingerprint.DiskDetails.FreeDisk, borFingerprint.DiskDetails.UsedDisk), }) + return base } @@ -85,13 +89,13 @@ func convertBytesToGB(bytesValue uint64) float64 { // Checks if fio exists on the node func (c *FingerprintCommand) checkFio() error { - cmd := exec.Command("/bin/sh", "-c", "fio -v") _, err := cmd.CombinedOutput() if err != nil { message := "\nFio package not installed. Install Fio for IOPS Benchmarking :\n\nDebianOS : 'sudo apt-get update && sudo apt-get install fio -y'\nAWS AMI/CentOS : 'sudo yum install fio -y'\nOracle LinuxOS : 'sudo dnf install fio -y'\n" c.UI.Output(message) + return err } @@ -101,9 +105,12 @@ func (c *FingerprintCommand) checkFio() error { // Run the IOPS benchmark for the node func (c *FingerprintCommand) benchmark() error { var b []byte + err := c.checkFio() + if err != nil { - return nil + // Missing Fio is not a fatal error. A message will be logged in console when it is missing in "checkFio()". + return nil //nolint:nilerr } c.UI.Output("\nRunning a 10 second test...\n") @@ -123,7 +130,6 @@ func (c *FingerprintCommand) benchmark() error { // Run implements the cli.Command interface func (c *FingerprintCommand) Run(args []string) int { - v, err := mem.VirtualMemory() if err != nil { c.UI.Error(err.Error()) diff --git a/internal/cli/chain.go b/internal/cli/chain.go index 896ce42cc4..9a7e9e8537 100644 --- a/internal/cli/chain.go +++ b/internal/cli/chain.go @@ -19,6 +19,7 @@ func (c *ChainCommand) MarkDown() string { "- [```chain sethead```](./chain_sethead.md): Set the current chain to a certain block.", "- [```chain watch```](./chain_watch.md): Watch the chainHead, reorg and fork events in real-time.", } + return strings.Join(items, "\n\n") } diff --git a/internal/cli/chain_sethead.go b/internal/cli/chain_sethead.go index 4d34479e0b..718ada4648 100644 --- a/internal/cli/chain_sethead.go +++ b/internal/cli/chain_sethead.go @@ -26,6 +26,7 @@ func (a *ChainSetHeadCommand) MarkDown() string { "- ```number```: The block number to roll back.", a.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -45,6 +46,7 @@ func (c *ChainSetHeadCommand) Flags() *flagset.Flagset { Default: false, Value: &c.yes, }) + return flags } @@ -88,6 +90,7 @@ func (c *ChainSetHeadCommand) Run(args []string) int { c.UI.Error(err.Error()) return 1 } + if response != "y" { c.UI.Output("set head aborted") return 0 @@ -100,5 +103,6 @@ func (c *ChainSetHeadCommand) Run(args []string) int { } c.UI.Output("Done!") + return 0 } diff --git a/internal/cli/chain_watch.go b/internal/cli/chain_watch.go index 72bd21b85d..17a65a8d99 100644 --- a/internal/cli/chain_watch.go +++ b/internal/cli/chain_watch.go @@ -24,6 +24,7 @@ func (c *ChainWatchCommand) MarkDown() string { "# Chain watch", "The ```chain watch``` command is used to view the chainHead, reorg and fork events in real-time.", } + return strings.Join(items, "\n\n") } @@ -70,7 +71,10 @@ func (c *ChainWatchCommand) Run(args []string) int { go func() { <-signalCh - sub.CloseSend() + + if err := sub.CloseSend(); err != nil { + c.UI.Error(err.Error()) + } }() for { @@ -80,6 +84,7 @@ func (c *ChainWatchCommand) Run(args []string) int { c.UI.Output(err.Error()) break } + c.UI.Output(formatHeadEvent(msg)) } @@ -95,5 +100,6 @@ func formatHeadEvent(msg *proto.ChainWatchResponse) string { } else if msg.Type == core.Chain2HeadReorgEvent { out = fmt.Sprintf("Reorg Detected \nAdded : %v \nRemoved : %v", msg.Newchain, msg.Oldchain) } + return out } diff --git a/internal/cli/command.go b/internal/cli/command.go index d1851594a7..34f7c4ef12 100644 --- a/internal/cli/command.go +++ b/internal/cli/command.go @@ -9,11 +9,16 @@ import ( "github.com/ethereum/go-ethereum/internal/cli/server" "github.com/ethereum/go-ethereum/internal/cli/server/proto" "github.com/ethereum/go-ethereum/node" + "github.com/mitchellh/cli" "github.com/ryanuber/columnize" "google.golang.org/grpc" ) +const ( + emptyPlaceHolder = "" +) + type MarkDownCommand interface { MarkDown cli.Command @@ -48,6 +53,7 @@ func Run(args []string) int { fmt.Fprintf(os.Stderr, "Error executing CLI: %s\n", err.Error()) return 1 } + return exitCode } @@ -64,6 +70,7 @@ func Commands() map[string]MarkDownCommandFactory { meta := &Meta{ UI: ui, } + return map[string]MarkDownCommandFactory{ "server": func() (MarkDownCommand, error) { return &server.Command{ @@ -180,6 +187,7 @@ func (m *Meta2) NewFlagSet(n string) *flagset.Flagset { Usage: "Address of the grpc endpoint", Default: "127.0.0.1:3131", }) + return f } @@ -188,6 +196,7 @@ func (m *Meta2) Conn() (*grpc.ClientConn, error) { if err != nil { return nil, fmt.Errorf("failed to connect to server: %v", err) } + return conn, nil } @@ -196,6 +205,7 @@ func (m *Meta2) BorConn() (proto.BorClient, error) { if err != nil { return nil, err } + return proto.NewBorClient(conn), nil } @@ -243,18 +253,21 @@ func (m *Meta) GetKeystore() (*keystore.KeyStore, error) { scryptP := keystore.StandardScryptP keys := keystore.NewKeyStore(keydir, scryptN, scryptP) + return keys, nil } func formatList(in []string) string { columnConf := columnize.DefaultConfig() - columnConf.Empty = "" + columnConf.Empty = emptyPlaceHolder + return columnize.Format(in, columnConf) } func formatKV(in []string) string { columnConf := columnize.DefaultConfig() - columnConf.Empty = "" + columnConf.Empty = emptyPlaceHolder columnConf.Glue = " = " + return columnize.Format(in, columnConf) } diff --git a/internal/cli/debug.go b/internal/cli/debug.go index 66e936995c..fb998ee4b9 100644 --- a/internal/cli/debug.go +++ b/internal/cli/debug.go @@ -18,8 +18,9 @@ import ( "github.com/ethereum/go-ethereum/internal/cli/flagset" "github.com/ethereum/go-ethereum/internal/cli/server/proto" - "github.com/golang/protobuf/jsonpb" - gproto "github.com/golang/protobuf/proto" + + "github.com/golang/protobuf/jsonpb" // nolint:staticcheck + gproto "github.com/golang/protobuf/proto" // nolint:staticcheck "github.com/golang/protobuf/ptypes/empty" grpc_net_conn "github.com/mitchellh/go-grpc-net-conn" ) @@ -55,6 +56,7 @@ func (d *DebugCommand) MarkDown() string { d.Flags().MarkDown(), } items = append(items, examples...) + return strings.Join(items, "\n\n") } @@ -112,6 +114,7 @@ func (d *DebugCommand) Run(args []string) int { // User specified output directory tmp = filepath.Join(d.output, stamped) _, err := os.Stat(tmp) + if !os.IsNotExist(err) { d.UI.Error("Output directory already exists") return 1 @@ -139,6 +142,7 @@ func (d *DebugCommand) Run(args []string) int { req := &proto.PprofRequest{ Seconds: int64(d.seconds), } + switch profile { case "cpu": req.Type = proto.PprofRequest_CPU @@ -148,7 +152,9 @@ func (d *DebugCommand) Run(args []string) int { req.Type = proto.PprofRequest_LOOKUP req.Profile = profile } + stream, err := clt.Pprof(ctx, req) + if err != nil { return err } @@ -157,6 +163,7 @@ func (d *DebugCommand) Run(args []string) int { if err != nil { return err } + if _, ok := msg.Event.(*proto.PprofResponse_Open_); !ok { return fmt.Errorf("expected open message") } @@ -179,6 +186,7 @@ func (d *DebugCommand) Run(args []string) int { if _, err := io.Copy(file, conn); err != nil { return err } + return nil } @@ -210,7 +218,7 @@ func (d *DebugCommand) Run(args []string) int { d.UI.Output(err.Error()) return 1 } - if err := ioutil.WriteFile(filepath.Join(tmp, "status.json"), []byte(data), 0644); err != nil { + if err := ioutil.WriteFile(filepath.Join(tmp, "status.json"), []byte(data), 0600); err != nil { d.UI.Output(fmt.Sprintf("Failed to write status: %v", err)) return 1 } @@ -230,6 +238,7 @@ func (d *DebugCommand) Run(args []string) int { } d.UI.Output(fmt.Sprintf("Created debug archive: %s", archiveFile)) + return 0 } diff --git a/internal/cli/markdown_test.go b/internal/cli/markdown_test.go index 13a5c4ece3..30c272a220 100644 --- a/internal/cli/markdown_test.go +++ b/internal/cli/markdown_test.go @@ -7,6 +7,7 @@ import ( ) func TestCodeBlock(t *testing.T) { + t.Parallel() assert := assert.New(t) lines := []string{ diff --git a/internal/cli/peers.go b/internal/cli/peers.go index ee70a747b8..fbbca24fad 100644 --- a/internal/cli/peers.go +++ b/internal/cli/peers.go @@ -21,6 +21,7 @@ func (a *PeersCommand) MarkDown() string { "- [```peers remove```](./peers_remove.md): Disconnects the local client from a connected peer if exists.", "- [```peers status```](./peers_status.md): Display the status of a peer by its id.", } + return strings.Join(items, "\n\n") } diff --git a/internal/cli/peers_add.go b/internal/cli/peers_add.go index ae84f4e0c1..3df1a6b6cb 100644 --- a/internal/cli/peers_add.go +++ b/internal/cli/peers_add.go @@ -22,6 +22,7 @@ func (p *PeersAddCommand) MarkDown() string { "The ```peers add ``` command joins the local client to another remote peer.", p.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -79,5 +80,6 @@ func (c *PeersAddCommand) Run(args []string) int { c.UI.Error(err.Error()) return 1 } + return 0 } diff --git a/internal/cli/peers_list.go b/internal/cli/peers_list.go index 56dfdc317d..4a572447c1 100644 --- a/internal/cli/peers_list.go +++ b/internal/cli/peers_list.go @@ -21,6 +21,7 @@ func (p *PeersListCommand) MarkDown() string { "The ```peers list``` command lists the connected peers.", p.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -60,12 +61,14 @@ func (c *PeersListCommand) Run(args []string) int { req := &proto.PeersListRequest{} resp, err := borClt.PeersList(context.Background(), req) + if err != nil { c.UI.Error(err.Error()) return 1 } c.UI.Output(formatPeers(resp.Peers)) + return 0 } @@ -76,6 +79,7 @@ func formatPeers(peers []*proto.Peer) string { rows := make([]string, len(peers)+1) rows[0] = "ID|Enode|Name|Caps|Static|Trusted" + for i, d := range peers { enode := strings.TrimPrefix(d.Enode, "enode://") @@ -87,5 +91,6 @@ func formatPeers(peers []*proto.Peer) string { d.Static, d.Trusted) } + return formatList(rows) } diff --git a/internal/cli/peers_remove.go b/internal/cli/peers_remove.go index 5cd3796e3c..f53284c40c 100644 --- a/internal/cli/peers_remove.go +++ b/internal/cli/peers_remove.go @@ -22,6 +22,7 @@ func (p *PeersRemoveCommand) MarkDown() string { "The ```peers remove ``` command disconnects the local client from a connected peer if exists.", p.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -79,5 +80,6 @@ func (c *PeersRemoveCommand) Run(args []string) int { c.UI.Error(err.Error()) return 1 } + return 0 } diff --git a/internal/cli/peers_status.go b/internal/cli/peers_status.go index bb8d385291..f5d700a273 100644 --- a/internal/cli/peers_status.go +++ b/internal/cli/peers_status.go @@ -21,6 +21,7 @@ func (p *PeersStatusCommand) MarkDown() string { "The ```peers status ``` command displays the status of a peer by its id.", p.Flags().MarkDown(), } + return strings.Join(items, "\n\n") } @@ -68,12 +69,14 @@ func (c *PeersStatusCommand) Run(args []string) int { Enode: args[0], } resp, err := borClt.PeersStatus(context.Background(), req) + if err != nil { c.UI.Error(err.Error()) return 1 } c.UI.Output(formatPeer(resp.Peer)) + return 0 } @@ -87,5 +90,6 @@ func formatPeer(peer *proto.Peer) string { fmt.Sprintf("Static|%v", peer.Static), fmt.Sprintf("Trusted|%v", peer.Trusted), }) + return base } diff --git a/internal/cli/status.go b/internal/cli/status.go index c4165b0a2e..2a8b7d7470 100644 --- a/internal/cli/status.go +++ b/internal/cli/status.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/ethereum/go-ethereum/internal/cli/server/proto" + "github.com/golang/protobuf/ptypes/empty" ) @@ -20,6 +21,7 @@ func (p *StatusCommand) MarkDown() string { "# Status", "The ```status``` command outputs the status of the client.", } + return strings.Join(items, "\n\n") } @@ -56,6 +58,7 @@ func (c *StatusCommand) Run(args []string) int { } c.UI.Output(printStatus(status)) + return 0 } @@ -69,6 +72,7 @@ func printStatus(status *proto.StatusResponse) string { forks := make([]string, len(status.Forks)+1) forks[0] = "Name|Block|Enabled" + for i, d := range status.Forks { forks[i+1] = fmt.Sprintf("%s|%d|%v", d.Name, d.Block, !d.Disabled) } @@ -92,5 +96,6 @@ func printStatus(status *proto.StatusResponse) string { "\nForks", formatList(forks), } + return strings.Join(full, "\n") } diff --git a/internal/cli/version.go b/internal/cli/version.go index 080cf41221..cd155f43a7 100644 --- a/internal/cli/version.go +++ b/internal/cli/version.go @@ -4,6 +4,7 @@ import ( "strings" "github.com/ethereum/go-ethereum/params" + "github.com/mitchellh/cli" ) @@ -27,6 +28,7 @@ func (d *VersionCommand) MarkDown() string { "The ```bor version``` command outputs the version of the binary.", } items = append(items, examples...) + return strings.Join(items, "\n\n") } From 2e2557a2d004006e72be463d45a3b4a04c397cd2 Mon Sep 17 00:00:00 2001 From: Jerry Date: Tue, 24 May 2022 13:08:45 -0700 Subject: [PATCH 3/3] Enable prealloc check and improve address comparison --- .golangci.yml | 2 +- consensus/bor/api.go | 2 +- consensus/bor/bor.go | 19 ++++++++++--------- consensus/bor/merkle.go | 2 +- consensus/bor/snapshot.go | 3 +-- consensus/bor/validator_set.go | 2 +- internal/cli/attach.go | 5 +++-- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index daea4e1e0b..b063984061 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -38,7 +38,7 @@ linters: - noctx #- nosprintfhostport # TODO: do we use IPv6? - paralleltest - # - prealloc + - prealloc - predeclared #- promlinter #- revive diff --git a/consensus/bor/api.go b/consensus/bor/api.go index 361e439bf7..364fc448b8 100644 --- a/consensus/bor/api.go +++ b/consensus/bor/api.go @@ -61,7 +61,7 @@ type difficultiesKV struct { } func rankMapDifficulties(values map[common.Address]uint64) []difficultiesKV { - var ss []difficultiesKV + ss := make([]difficultiesKV, 0, len(values)) for k, v := range values { ss = append(ss, difficultiesKV{k, v}) } diff --git a/consensus/bor/bor.go b/consensus/bor/bor.go index ada601f838..62336e73f0 100644 --- a/consensus/bor/bor.go +++ b/consensus/bor/bor.go @@ -472,10 +472,9 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainHeaderReader, header *t // nolint: gocognit func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash common.Hash, parents []*types.Header) (*Snapshot, error) { // Search for a snapshot in memory or on disk for checkpoints - var ( - headers []*types.Header - snap *Snapshot - ) + var snap *Snapshot + + headers := make([]*types.Header, 0, 16) //nolint:govet for snap == nil { @@ -549,6 +548,8 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co number, hash = number-1, header.ParentHash } + log.Info("Snapshot has been found in %d headers depth.", len(headers)) + // check if snapshot is nil if snap == nil { return nil, fmt.Errorf("Unknown error while retrieving snapshot at block number %v", number) @@ -701,7 +702,7 @@ func (c *Bor) Prepare(chain consensus.ChainHeaderReader, header *types.Header) e var succession int // if signer is not empty - if !bytes.Equal(c.signer.Bytes(), common.Address{}.Bytes()) { + if c.signer != (common.Address{}) { succession, err = snap.GetSignerSuccessionNumber(c.signer) if err != nil { return err @@ -1146,7 +1147,7 @@ func (c *Bor) fetchAndCommitSpan( } // get validators bytes - var validators []MinimalVal + validators := make([]MinimalVal, 0, len(heimdallSpan.ValidatorSet.Validators)) for _, val := range heimdallSpan.ValidatorSet.Validators { validators = append(validators, val.MinimalVal()) } @@ -1158,7 +1159,7 @@ func (c *Bor) fetchAndCommitSpan( } // get producers bytes - var producers []MinimalVal + producers := make([]MinimalVal, 0, len(heimdallSpan.SelectedProducers)) for _, val := range heimdallSpan.SelectedProducers { producers = append(producers, val.MinimalVal()) } @@ -1401,7 +1402,7 @@ func applyMessage( func validatorContains(a []*Validator, x *Validator) (*Validator, bool) { for _, n := range a { - if bytes.Equal(n.Address.Bytes(), x.Address.Bytes()) { + if n.Address == x.Address { return n, true } } @@ -1413,7 +1414,7 @@ func getUpdatedValidatorSet(oldValidatorSet *ValidatorSet, newVals []*Validator) v := oldValidatorSet oldVals := v.Validators - var changes []*Validator + changes := make([]*Validator, 0, len(oldVals)) for _, ov := range oldVals { if f, ok := validatorContains(newVals, ov); ok { diff --git a/consensus/bor/merkle.go b/consensus/bor/merkle.go index 5c61eb4a12..ef1b4eb87e 100644 --- a/consensus/bor/merkle.go +++ b/consensus/bor/merkle.go @@ -40,7 +40,7 @@ func convertTo32(input []byte) (output [32]byte) { } func convert(input []([32]byte)) [][]byte { - var output [][]byte + output := make([][]byte, 0, len(input)) for _, in := range input { newInput := make([]byte, len(in[:])) diff --git a/consensus/bor/snapshot.go b/consensus/bor/snapshot.go index ef92cc99b4..606c28340b 100644 --- a/consensus/bor/snapshot.go +++ b/consensus/bor/snapshot.go @@ -1,7 +1,6 @@ package bor import ( - "bytes" "encoding/json" lru "github.com/hashicorp/golang-lru" @@ -208,7 +207,7 @@ func (s *Snapshot) signers() []common.Address { // Difficulty returns the difficulty for a particular signer at the current snapshot number func (s *Snapshot) Difficulty(signer common.Address) uint64 { // if signer is empty - if bytes.Equal(signer.Bytes(), common.Address{}.Bytes()) { + if signer == (common.Address{}) { return 1 } diff --git a/consensus/bor/validator_set.go b/consensus/bor/validator_set.go index 892891bbda..ee14ef2cb1 100644 --- a/consensus/bor/validator_set.go +++ b/consensus/bor/validator_set.go @@ -255,7 +255,7 @@ func (vals *ValidatorSet) GetByAddress(address common.Address) (index int, val * idx := sort.Search(len(vals.Validators), func(i int) bool { return bytes.Compare(address.Bytes(), vals.Validators[i].Address.Bytes()) <= 0 }) - if idx < len(vals.Validators) && bytes.Equal(vals.Validators[idx].Address.Bytes(), address.Bytes()) { + if idx < len(vals.Validators) && vals.Validators[idx].Address == address { return idx, vals.Validators[idx].Copy() } diff --git a/internal/cli/attach.go b/internal/cli/attach.go index 1aa888c12b..134a282180 100644 --- a/internal/cli/attach.go +++ b/internal/cli/attach.go @@ -182,9 +182,10 @@ func (c *AttachCommand) makeConsolePreloads() []string { return nil } // Otherwise resolve absolute paths and return them - var preloads []string + splitFlags := strings.Split(c.PreloadJSFlag, ",") + preloads := make([]string, 0, len(splitFlags)) - for _, file := range strings.Split(c.PreloadJSFlag, ",") { + for _, file := range splitFlags { preloads = append(preloads, strings.TrimSpace(file)) }