Skip to content

Commit

Permalink
Merge branch 'master' into piersy/fix-epoch-block-marshaling
Browse files Browse the repository at this point in the history
  • Loading branch information
piersy authored Sep 24, 2021
2 parents 5393345 + 00a9a9d commit 37ed12d
Show file tree
Hide file tree
Showing 9 changed files with 218 additions and 20 deletions.
48 changes: 48 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,50 @@ jobs:
- run: go run build/ci.go test -coverage
- run: bash <(curl -s https://codecov.io/bash)

istanbul-e2e-coverage:
executor: golang
resource_class: medium+
steps:
- attach_workspace:
at: ~/repos
- restore_cache:
keys:
- go-mod-v1-{{ checksum "go.sum" }}
# Run the tests with coverage parse the coverage and output the summary
- run:
name: Run tests and print coverage summary
command: |
go test -coverprofile cov.out -coverpkg ./consensus/istanbul/... ./e2e_test
go run tools/parsecov/main.go -packagePrefix github.com/celo-org/celo-blockchain/ cov.out > summary
cat summary
- run:
name: Post summary comment on PR
command: |
# Only post on PR if this build is running on a PR. If this build
# is running on a PR then CIRCLE_PULL_REQUEST contains the link to
# the PR.
if [[ ! -z ${CIRCLE_PULL_REQUEST} ]] ; then
# Build comment
echo "Coverage from tests in \`./e2e_test/...\` for \`./consensus/istanbul/...\` at commit ${CIRCLE_SHA1}" > comment
echo "\`\`\`" >> comment
cat summary >> comment
echo "\`\`\`" >> comment
# This command is quite involved, its posting the comment on the
# associated PR.
# ${CIRCLE_PULL_REQUEST##*/} expands to just the pr number at the
# end of the PR link.
# "{\"body\":\"`awk -v ORS='\\\\n' '1' comment`\"}" evaluates to
# a json object with comment as the content of body with newlines
# replaced by '\n'. Using backtics causes there to be a round of
# backslash processing on the command before execution, so we
# need to double the backslashes in the awk command.
curl -u piersy:${PR_COMMENT_TOKEN} -X POST \
https://api.github.com/repos/celo-org/celo-blockchain/issues/${CIRCLE_PULL_REQUEST##*/}/comments \
-d "{\"body\":\"`awk -v ORS='\\\\n' '1' comment`\"}" ;
fi
lint:
executor: golang
steps:
Expand Down Expand Up @@ -338,6 +382,10 @@ workflows:
requires:
- build-geth
- prepare-system-contracts
- istanbul-e2e-coverage:
requires:
- build-geth
- prepare-system-contracts
- android
- ios
- publish-mobile-client:
Expand Down
4 changes: 2 additions & 2 deletions consensus/istanbul/backend/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func (api *API) GetCurrentRoundState() (*core.RoundStateSummary, error) {
api.istanbul.coreMu.RLock()
defer api.istanbul.coreMu.RUnlock()

if !api.istanbul.coreStarted {
if !api.istanbul.isCoreStarted() {
return nil, istanbul.ErrStoppedEngine
}
return api.istanbul.core.CurrentRoundState().Summary(), nil
Expand All @@ -203,7 +203,7 @@ func (api *API) ForceRoundChange() (bool, error) {
api.istanbul.coreMu.RLock()
defer api.istanbul.coreMu.RUnlock()

if !api.istanbul.coreStarted {
if !api.istanbul.isCoreStarted() {
return false, istanbul.ErrStoppedEngine
}
api.istanbul.core.ForceRoundChange()
Expand Down
23 changes: 18 additions & 5 deletions consensus/istanbul/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,15 @@ func New(config *istanbul.Config, db ethdb.Database) consensus.Istanbul {
logger.Crit("Failed to create recent snapshots cache", "err", err)
}

coreStarted := atomic.Value{}
coreStarted.Store(false)
backend := &Backend{
config: config,
istanbulEventMux: new(event.TypeMux),
logger: logger,
db: db,
recentSnapshots: recentSnapshots,
coreStarted: false,
coreStarted: coreStarted,
announceRunning: false,
gossipCache: NewLRUGossipCache(inmemoryPeers, inmemoryMessages),
announceThreadWg: new(sync.WaitGroup),
Expand Down Expand Up @@ -221,7 +223,16 @@ type Backend struct {
validateState func(block *types.Block, statedb *state.StateDB, receipts types.Receipts, usedGas uint64) error
onNewConsensusBlock func(block *types.Block, receipts []*types.Receipt, logs []*types.Log, state *state.StateDB)

coreStarted bool
// We need this to be an atomic value so that we can access it in a lock
// free way from IsValidating. This is required because StartValidating
// makes a call to RefreshValPeers while holding coreMu and RefreshValPeers
// waits for all validator peers to be deleted and then reconnects to known
// validators. If any of those peers has called IsValidating before
// RefreshValPeers tries to delete them the system gets stuck in a
// deadlock, the peer will never acquire coreMu because it is held by
// StartValidating, and StartValidating will never return because it is
// waiting for all peers to disconnect.
coreStarted atomic.Value
coreMu sync.RWMutex

// Snapshots for recent blocks to speed up reorgs
Expand Down Expand Up @@ -325,6 +336,10 @@ type Backend struct {
abortCommitHook func(result *istanbulCore.StateProcessResult) bool // Method to call upon committing a proposal
}

func (sb *Backend) isCoreStarted() bool {
return sb.coreStarted.Load().(bool)
}

// IsProxy returns true if instance has proxy flag
func (sb *Backend) IsProxy() bool {
return sb.config.Proxy
Expand All @@ -350,9 +365,7 @@ func (sb *Backend) GetProxiedValidatorEngine() proxy.ProxiedValidatorEngine {
// IsValidating return true if instance is validating
func (sb *Backend) IsValidating() bool {
// TODO: Maybe a little laggy, but primary / replica should track the core
sb.coreMu.RLock()
defer sb.coreMu.RUnlock()
return sb.coreStarted
return sb.isCoreStarted()
}

// IsValidator return if instance is a validator (either proxied or standalone)
Expand Down
12 changes: 6 additions & 6 deletions consensus/istanbul/backend/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ func (sb *Backend) updateReplicaStateLoop(bc *ethCore.BlockChain) {
select {
case chainEvent := <-chainEventCh:
sb.coreMu.RLock()
if !sb.coreStarted && sb.replicaState != nil {
if !sb.isCoreStarted() && sb.replicaState != nil {
consensusBlock := new(big.Int).Add(chainEvent.Block.Number(), common.Big1)
sb.replicaState.NewChainHead(consensusBlock)
}
Expand All @@ -649,7 +649,7 @@ func (sb *Backend) SetCallBacks(hasBadBlock func(common.Hash) bool,
onNewConsensusBlock func(block *types.Block, receipts []*types.Receipt, logs []*types.Log, state *state.StateDB)) error {
sb.coreMu.RLock()
defer sb.coreMu.RUnlock()
if sb.coreStarted {
if sb.isCoreStarted() {
return istanbul.ErrStartedEngine
}

Expand All @@ -664,7 +664,7 @@ func (sb *Backend) SetCallBacks(hasBadBlock func(common.Hash) bool,
func (sb *Backend) StartValidating() error {
sb.coreMu.Lock()
defer sb.coreMu.Unlock()
if sb.coreStarted {
if sb.isCoreStarted() {
return istanbul.ErrStartedEngine
}

Expand All @@ -684,7 +684,7 @@ func (sb *Backend) StartValidating() error {
sb.UpdateAnnounceVersion()
}

sb.coreStarted = true
sb.coreStarted.Store(true)

// coreStarted must be true by this point for validator peers to be successfully added
if !sb.config.Proxied {
Expand All @@ -700,14 +700,14 @@ func (sb *Backend) StartValidating() error {
func (sb *Backend) StopValidating() error {
sb.coreMu.Lock()
defer sb.coreMu.Unlock()
if !sb.coreStarted {
if !sb.isCoreStarted() {
return istanbul.ErrStoppedEngine
}
sb.logger.Info("Stopping istanbul.Engine validating")
if err := sb.core.Stop(); err != nil {
return err
}
sb.coreStarted = false
sb.coreStarted.Store(false)

return nil
}
Expand Down
2 changes: 1 addition & 1 deletion consensus/istanbul/backend/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (sb *Backend) NewWork() error {

sb.coreMu.RLock()
defer sb.coreMu.RUnlock()
if !sb.coreStarted {
if !sb.isCoreStarted() {
return istanbul.ErrStoppedEngine
}

Expand Down
2 changes: 1 addition & 1 deletion node/rpcstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func (h *httpServer) wsAllowed() bool {
// isWebsocket checks the header of an http request for a websocket upgrade request.
func isWebsocket(r *http.Request) bool {
return strings.ToLower(r.Header.Get("Upgrade")) == "websocket" &&
strings.ToLower(r.Header.Get("Connection")) == "upgrade"
strings.Contains(strings.ToLower(r.Header.Get("Connection")), "upgrade")
}

// NewHTTPHandlerStack returns wrapped http-related handlers
Expand Down
15 changes: 15 additions & 0 deletions node/rpcstack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,21 @@ func TestWebsocketOrigins(t *testing.T) {
assert.Error(t, err)
}

// TestIsWebsocket tests if an incoming websocket upgrade request is handled properly.
func TestIsWebsocket(t *testing.T) {
r, _ := http.NewRequest("GET", "/", nil)

assert.False(t, isWebsocket(r))
r.Header.Set("upgrade", "websocket")
assert.False(t, isWebsocket(r))
r.Header.Set("connection", "upgrade")
assert.True(t, isWebsocket(r))
r.Header.Set("connection", "upgrade,keep-alive")
assert.True(t, isWebsocket(r))
r.Header.Set("connection", " UPGRADE,keep-alive")
assert.True(t, isWebsocket(r))
}

func createAndStartServer(t *testing.T, conf httpConfig, ws bool, wsConf wsConfig) *httpServer {
t.Helper()

Expand Down
7 changes: 2 additions & 5 deletions test/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,9 @@ func NewNetwork(accounts *env.AccountsConfig, gc *genesis.Config, ec *eth.Config
// each other nodes don't start sending consensus messages to another node
// until they have received an enode certificate from that node.
for i, en := range enodes {
for j, n := range network {
if j == i {
continue
}
// Connect to the remaining nodes
for _, n := range network[i+1:] {
n.Server().AddPeer(en, p2p.ValidatorPurpose)
n.Server().AddTrustedPeer(en, p2p.ValidatorPurpose)
}
}

Expand Down
125 changes: 125 additions & 0 deletions tools/parsecov/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package main

import (
"bufio"
"fmt"
"log"
"os"
"path"
"sort"
"strconv"
"strings"

"gopkg.in/urfave/cli.v1"
)

var (
packagePrefixFlagName = "packagePrefix"
)

type count struct {
totalStatements int
coveredStatements int
}

func (c count) percentage() float32 {
return 100 * float32(c.coveredStatements) / float32(c.totalStatements)
}

func parse(c *cli.Context) error {
packageCoverage := make(map[string]count)
profile, err := os.Open(c.Args()[0])
if err != nil {
return err
}
defer profile.Close()

// First line is "mode: foo", where foo is "set", "count", or "atomic".
// Rest of file is in the format
// encoding/base64/base64.go:34.44,37.40 3 1
// where the fields are: name.go:line.column,line.column numberOfStatements count

// This program only handles set mode
s := bufio.NewScanner(profile)
s.Scan()
line := s.Text()
if line != "mode: set" {
return fmt.Errorf("invalid coverage mode, expecting 'mode: set' as the first line, but got %q", line)
}
linenum := 1
for s.Scan() {
line := s.Text()
linenum++
lastColon := strings.LastIndex(line, ":")
if lastColon == -1 {
return fmt.Errorf("line %d invalid: %q", linenum, line)
}

packageName := path.Dir(line[:lastColon])
numStatements, covered, err := getStats(line[lastColon+1:])
if err != nil {
return fmt.Errorf("line %d invalid: %q", linenum, line)
}
c := packageCoverage[packageName]
c.totalStatements += numStatements
if covered {
c.coveredStatements += numStatements
}
packageCoverage[packageName] = c
}

packageNames := make([]string, 0, len(packageCoverage))
for name := range packageCoverage {
packageNames = append(packageNames, name)
}
sort.Strings(packageNames)

var totalCount count
for _, v := range packageCoverage {
totalCount.totalStatements += v.totalStatements
totalCount.coveredStatements += v.coveredStatements
}
fmt.Printf("coverage: %5.1f%% of statements across all listed packages\n", totalCount.percentage())

for _, name := range packageNames {
cov := packageCoverage[name].percentage()
fmt.Printf("coverage: %5.1f%% of statements in %s\n", cov, strings.TrimPrefix(name, c.String(packagePrefixFlagName)))
}

return nil
}

// Gets the stats from the end of the line, how many statements and were they
// covered?
//
// E.G line -> encoding/base64/base64.go:34.44,37.40 3 1
// End of line -> 34.44,37.40 3 1
// Would return 3 true
func getStats(lineEnd string) (numStatements int, covered bool, err error) {
parts := strings.Split(lineEnd, " ")
if len(parts) != 3 {
return 0, false, fmt.Errorf("invalid line end")
}
numStatements, err = strconv.Atoi(parts[1])
return numStatements, parts[2] == "1", err
}

func main() {
app := cli.NewApp()
app.Name = "parsecov"
app.Usage = `parses coverage files outputting a per package breakdown
of coverage as well as a total for all the packages.`
app.Flags = []cli.Flag{
cli.StringFlag{
Name: packagePrefixFlagName,
Usage: `a common prefix that is stripped from the front
of all packages in order to make output more concise.`,
},
}
app.Action = parse

err := app.Run(os.Args)
if err != nil {
log.Fatalf("Failed to parse coverage: %v", err)
}
}

0 comments on commit 37ed12d

Please sign in to comment.