From 12ab0f0240625f890355d4c29d523bbfeafc7478 Mon Sep 17 00:00:00 2001 From: Jerry Date: Thu, 19 May 2022 17:55:23 -0700 Subject: [PATCH] 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 }