Remove istanbul.v1 code (#1999)

* Remove v1 types

* Add backlog tests (remove v1)

* Remove v1 references from istanbul core core.go

* Remove v1 code from handler & handler_test

* remove v1 code from preprepare files

* remove v1 code from roundchange files

* Remove most of v1 code

* Remove isConsensusFork

* Remove all mentions to V2Block

* Remove useless testing suite function

Author: hbandura

cmd/geth/config.go

@@ -161,9 +161,6 @@ func makeFullNode(ctx *cli.Context) (*node.Node, ethapi.Backend) {
 	if ctx.GlobalIsSet(utils.OverrideEHardforkFlag.Name) {
 		cfg.Eth.OverrideEHardfork = new(big.Int).SetUint64(ctx.GlobalUint64(utils.OverrideEHardforkFlag.Name))
 	}
-	if ctx.GlobalIsSet(utils.OverrideV2IstanbulForkFlag.Name) {
-		cfg.Eth.OverrideV2IstanbulFork = new(big.Int).SetUint64(ctx.GlobalUint64(utils.OverrideV2IstanbulForkFlag.Name))
-	}
 	backend, _ := utils.RegisterEthService(stack, &cfg.Eth)
 
 	// Configure GraphQL if requested

cmd/geth/main.go

@@ -73,7 +73,6 @@ var (
 		utils.USBFlag,
 		// utils.SmartCardDaemonPathFlag,
 		utils.OverrideEHardforkFlag,
-		utils.OverrideV2IstanbulForkFlag,
 		utils.TxPoolLocalsFlag,
 		utils.TxPoolNoLocalsFlag,
 		utils.TxPoolJournalFlag,

cmd/utils/flags.go

@@ -241,12 +241,6 @@ var (
 		Usage: "Manually specify the espresso fork block, overriding the bundled setting",
 	}
 
-	// V2 Istanbul fork activation overrides
-	OverrideV2IstanbulForkFlag = cli.Uint64Flag{
-		Name:  "override.v2istanbul",
-		Usage: "Manually specify the v2 istanbul consensus fork block, overriding the bundled setting",
-	}
-
 	BloomFilterSizeFlag = cli.Uint64Flag{
 		Name:  "bloomfilter.size",
 		Usage: "Megabytes of memory allocated to bloom-filter for pruning",

consensus/istanbul/config.go

@@ -18,7 +18,6 @@ package istanbul
 
 import (
 	"fmt"
-	"math/big"
 
 	"github.com/celo-org/celo-blockchain/common"
 	"github.com/celo-org/celo-blockchain/p2p/enode"
@@ -69,8 +68,6 @@ type Config struct {
 	AnnounceAggressiveQueryEnodeGossipOnEnablement bool   `toml:",omitempty"` // Specifies if this node should aggressively query enodes on announce enablement
 	AnnounceAdditionalValidatorsToGossip           int64  `toml:",omitempty"` // Specifies the number of additional non-elected validators to gossip an announce
 
-	V2Block *big.Int `toml:",omitempty"` // Activation block for the V2 istanbul consensus fork (nil = no fork, 0 = already activated)
-
 	// Load test config
 	LoadTestCSVFile string `toml:",omitempty"` // If non-empty, specifies the file to write out csv metrics about the block production cycle to.
 }
@@ -107,9 +104,6 @@ var DefaultConfig = &Config{
 
 //ApplyParamsChainConfigToConfig applies the istanbul config values from params.chainConfig to the istanbul.Config config
 func ApplyParamsChainConfigToConfig(chainConfig *params.ChainConfig, config *Config) error {
-	if chainConfig.Istanbul.V2Block != nil {
-		config.V2Block = chainConfig.Istanbul.V2Block
-	}
 	if chainConfig.Istanbul.Epoch != 0 {
 		if chainConfig.Istanbul.Epoch < MinEpochSize {
 			return fmt.Errorf("istanbul.Epoch must be greater than %d", MinEpochSize-1)

consensus/istanbul/core/backlog.go

@@ -30,9 +30,8 @@ import (
 
 var (
 	// msgPriority is defined for calculating processing priority to speedup consensus
-	// istanbul.MsgPreprepare > istanbul.MsgCommit > istanbul.MsgPrepare
+	// istanbul.MsgPreprepareV2 > istanbul.MsgCommit > istanbul.MsgPrepare
 	msgPriority = map[uint64]int{
-		istanbul.MsgPreprepare:   1,
 		istanbul.MsgPreprepareV2: 1,
 		istanbul.MsgCommit:       2,
 		istanbul.MsgPrepare:      3,
@@ -328,16 +327,12 @@ func toPriority(msgCode uint64, view *istanbul.View) int64 {
 
 func extractMessageView(msg *istanbul.Message) *istanbul.View {
 	switch msg.Code {
-	case istanbul.MsgPreprepare:
-		return msg.Preprepare().View
 	case istanbul.MsgPreprepareV2:
 		return msg.PreprepareV2().View
 	case istanbul.MsgPrepare:
 		return msg.Prepare().View
 	case istanbul.MsgCommit:
 		return msg.Commit().Subject.View
-	case istanbul.MsgRoundChange:
-		return msg.RoundChange().View
 	case istanbul.MsgRoundChangeV2:
 		return &msg.RoundChangeV2().Request.View
 	default:

consensus/istanbul/core/backlog_test.go

@@ -43,18 +43,18 @@ func TestCheckMessage(t *testing.T) {
 		current: newRoundState(&istanbul.View{
 			Sequence: big.NewInt(2),
 			Round:    big.NewInt(2),
-		}, valSet, valSet.GetByIndex(0), false),
+		}, valSet, valSet.GetByIndex(0)),
 	}
 
 	t.Run("invalid view format", func(t *testing.T) {
-		err := c.checkMessage(istanbul.MsgPreprepare, nil)
+		err := c.checkMessage(istanbul.MsgPreprepareV2, nil)
 		if err != errInvalidMessage {
 			t.Errorf("error mismatch: have %v, want %v", err, errInvalidMessage)
 		}
 	})
 
 	testStates := []State{StateAcceptRequest, StatePreprepared, StatePrepared, StateCommitted, StateWaitingForNewRound}
-	testCodes := []uint64{istanbul.MsgPreprepare, istanbul.MsgPrepare, istanbul.MsgCommit, istanbul.MsgRoundChange}
+	testCodes := []uint64{istanbul.MsgPreprepareV2, istanbul.MsgPrepare, istanbul.MsgCommit, istanbul.MsgRoundChangeV2}
 
 	// accept Commits from sequence, round matching LastSubject
 	t.Run("Rejects all other older rounds", func(t *testing.T) {
@@ -116,7 +116,7 @@ func TestCheckMessage(t *testing.T) {
 			for _, testCode := range testCodes {
 				c.current.(*roundStateImpl).state = testState
 				err := c.checkMessage(testCode, v)
-				if testCode == istanbul.MsgRoundChange {
+				if testCode == istanbul.MsgRoundChangeV2 {
 					if err != nil {
 						t.Errorf("error mismatch: have %v, want nil", err)
 					}
@@ -133,11 +133,11 @@ func TestCheckMessage(t *testing.T) {
 
 		for _, testCode := range testCodes {
 			err := c.checkMessage(testCode, v)
-			if testCode == istanbul.MsgRoundChange {
+			if testCode == istanbul.MsgRoundChangeV2 {
 				if err != nil {
 					t.Errorf("error mismatch: have %v, want nil", err)
 				}
-			} else if testCode == istanbul.MsgPreprepare {
+			} else if testCode == istanbul.MsgPreprepareV2 {
 				if err != nil {
 					t.Errorf("error mismatch: have %v, want nil", err)
 				}
@@ -154,7 +154,7 @@ func TestCheckMessage(t *testing.T) {
 		c.current.(*roundStateImpl).state = StatePreprepared
 		for _, testCode := range testCodes {
 			err := c.checkMessage(testCode, v)
-			if testCode == istanbul.MsgRoundChange {
+			if testCode == istanbul.MsgRoundChangeV2 {
 				if err != nil {
 					t.Errorf("error mismatch: have %v, want nil", err)
 				}
@@ -169,7 +169,7 @@ func TestCheckMessage(t *testing.T) {
 		c.current.(*roundStateImpl).state = StatePrepared
 		for _, testCode := range testCodes {
 			err := c.checkMessage(testCode, v)
-			if testCode == istanbul.MsgRoundChange {
+			if testCode == istanbul.MsgRoundChangeV2 {
 				if err != nil {
 					t.Errorf("error mismatch: have %v, want nil", err)
 				}
@@ -184,7 +184,7 @@ func TestCheckMessage(t *testing.T) {
 		c.current.(*roundStateImpl).state = StateCommitted
 		for _, testCode := range testCodes {
 			err := c.checkMessage(testCode, v)
-			if testCode == istanbul.MsgRoundChange {
+			if testCode == istanbul.MsgRoundChangeV2 {
 				if err != nil {
 					t.Errorf("error mismatch: have %v, want nil", err)
 				}
@@ -199,7 +199,7 @@ func TestCheckMessage(t *testing.T) {
 		c.current.(*roundStateImpl).state = StateWaitingForNewRound
 		for _, testCode := range testCodes {
 			err := c.checkMessage(testCode, v)
-			if testCode == istanbul.MsgRoundChange || testCode == istanbul.MsgPreprepare {
+			if testCode == istanbul.MsgRoundChangeV2 || testCode == istanbul.MsgPreprepareV2 {
 				if err != nil {
 					t.Errorf("error mismatch: have %v, want nil", err)
 				}
@@ -231,8 +231,8 @@ func TestStoreBacklog(t *testing.T) {
 	p1 := validator.New(common.BytesToAddress([]byte("12345667890")), blscrypto.SerializedPublicKey{})
 	p2 := validator.New(common.BytesToAddress([]byte("47324349949")), blscrypto.SerializedPublicKey{})
 
-	mPreprepare := istanbul.NewPreprepareMessage(
-		&istanbul.Preprepare{View: v10, Proposal: makeBlock(10)},
+	mPreprepare := istanbul.NewPreprepareV2Message(
+		&istanbul.PreprepareV2{View: v10, Proposal: makeBlock(10)},
 		p1.Address(),
 	)
 	backlog.store(mPreprepare)
@@ -246,8 +246,8 @@ func TestStoreBacklog(t *testing.T) {
 		&istanbul.Subject{View: v10, Digest: common.BytesToHash([]byte("1234567890"))},
 		p1.Address(),
 	)
-	mPreprepare2 := istanbul.NewPreprepareMessage(
-		&istanbul.Preprepare{View: v11, Proposal: makeBlock(11)},
+	mPreprepare2 := istanbul.NewPreprepareV2Message(
+		&istanbul.PreprepareV2{View: v11, Proposal: makeBlock(11)},
 		p2.Address(),
 	)
 
@@ -300,8 +300,8 @@ func TestClearBacklogForSequence(t *testing.T) {
 	// The backlog's state is sequence number 1, round 0.  Store future messages with sequence number 2
 	p1 := validator.New(common.BytesToAddress([]byte("12345667890")), blscrypto.SerializedPublicKey{})
 
-	mPreprepare := istanbul.NewPreprepareMessage(
-		&istanbul.Preprepare{
+	mPreprepare := istanbul.NewPreprepareV2Message(
+		&istanbul.PreprepareV2{
 			View:     &istanbul.View{Round: big.NewInt(0), Sequence: big.NewInt(2)},
 			Proposal: makeBlock(2),
 		},
@@ -360,15 +360,21 @@ func TestProcessFutureBacklog(t *testing.T) {
 	backlog.store(mFuture)
 
 	// push a message from the past and check we expire it
-	mPast := istanbul.NewRoundChangeMessage(&istanbul.RoundChange{
-		View: &istanbul.View{
-			Round:    big.NewInt(0),
-			Sequence: oldSequence,
-		},
-		PreparedCertificate: istanbul.PreparedCertificate{
-			Proposal: makeBlock(0),
+	addr := valSet.GetByIndex(1).Address()
+	roundChangeV2 := &istanbul.RoundChangeV2{
+		Request: istanbul.RoundChangeRequest{
+			Address: addr,
+			View: istanbul.View{
+				Round:    big.NewInt(0),
+				Sequence: oldSequence,
+			},
+			PreparedCertificateV2: istanbul.PCV2FromPCV1(istanbul.PreparedCertificate{
+				Proposal: makeBlock(0),
+			}),
 		},
-	}, valSet.GetByIndex(1).Address())
+	}
+	// No need to sign the RoundChangeRequest for this test
+	mPast := istanbul.NewRoundChangeV2Message(roundChangeV2, addr)
 
 	backlog.store(mPast)
 
@@ -404,17 +410,18 @@ func TestProcessBacklog(t *testing.T) {
 	address := common.BytesToAddress([]byte("0xce10ce10"))
 
 	msgs := []*istanbul.Message{
-		istanbul.NewPreprepareMessage(
-			&istanbul.Preprepare{View: v, Proposal: makeBlock(1)},
+		istanbul.NewPreprepareV2Message(
+			&istanbul.PreprepareV2{View: v, Proposal: makeBlock(1)},
 			address,
 		),
 		istanbul.NewPrepareMessage(subject, address),
 		istanbul.NewCommitMessage(
 			&istanbul.CommittedSubject{Subject: subject, CommittedSeal: []byte{0x63, 0x65, 0x6C, 0x6F}},
 			address,
 		),
-		istanbul.NewRoundChangeMessage(
-			&istanbul.RoundChange{View: v, PreparedCertificate: istanbul.EmptyPreparedCertificate()},
+		istanbul.NewRoundChangeV2Message(
+			&istanbul.RoundChangeV2{
+				Request: istanbul.RoundChangeRequest{View: *v}, PreparedProposal: makeBlock(1)},
 			address,
 		),
 	}

consensus/istanbul/core/commit_test.go

@@ -56,7 +56,7 @@ func TestHandleCommit(t *testing.T) {
 				for i, backend := range sys.backends {
 					c := backend.engine.(*core)
 					// same view as the expected one to everyone
-					c.current = newTestRoundState(
+					c.current = newTestRoundStateV2(
 						expectedSubject.View,
 						backend.peers,
 					)
@@ -80,13 +80,13 @@ func TestHandleCommit(t *testing.T) {
 					c := backend.engine.(*core)
 					if i == 0 {
 						// replica 0 is the proposer
-						c.current = newTestRoundState(
+						c.current = newTestRoundStateV2(
 							expectedSubject.View,
 							backend.peers,
 						)
 						c.current.(*roundStateImpl).state = StatePreprepared
 					} else {
-						c.current = newTestRoundState(
+						c.current = newTestRoundStateV2(
 							&istanbul.View{
 								Round: big.NewInt(0),
 								// proposal from 1 round in the future
@@ -111,13 +111,13 @@ func TestHandleCommit(t *testing.T) {
 
 					if i == 0 {
 						// replica 0 is the proposer
-						c.current = newTestRoundState(
+						c.current = newTestRoundStateV2(
 							expectedSubject.View,
 							backend.peers,
 						)
 						c.current.(*roundStateImpl).state = StatePreprepared
 					} else {
-						c.current = newTestRoundState(
+						c.current = newTestRoundStateV2(
 							&istanbul.View{
 								Round: big.NewInt(0),
 								// we're 2 blocks before so this is indeed a
@@ -141,7 +141,7 @@ func TestHandleCommit(t *testing.T) {
 
 				for i, backend := range sys.backends {
 					c := backend.engine.(*core)
-					c.current = newTestRoundState(
+					c.current = newTestRoundStateV2(
 						&istanbul.View{
 							Round:    big.NewInt(0),
 							Sequence: proposal.Number(),
@@ -174,13 +174,13 @@ func TestHandleCommit(t *testing.T) {
 					c := backend.engine.(*core)
 					if i == 0 {
 						// replica 0 is the proposer
-						c.current = newTestRoundState(
+						c.current = newTestRoundStateV2(
 							expectedSubject.View,
 							backend.peers,
 						)
 						c.current.(*roundStateImpl).state = StatePrepared
 					} else {
-						c.current = newTestRoundState(
+						c.current = newTestRoundStateV2(
 							&istanbul.View{
 								Round:    big.NewInt(1),
 								Sequence: big.NewInt(0).Sub(proposal.Number(), common.Big1),
@@ -297,7 +297,7 @@ func TestVerifyCommit(t *testing.T) {
 					Digest: newTestProposal().Hash(),
 				},
 			},
-			roundState: newTestRoundState(
+			roundState: newTestRoundStateV2(
 				&istanbul.View{Round: big.NewInt(0), Sequence: big.NewInt(0)},
 				valSet,
 			),
@@ -311,7 +311,7 @@ func TestVerifyCommit(t *testing.T) {
 					Digest: newTestProposal().Hash(),
 				},
 			},
-			roundState: newTestRoundState(
+			roundState: newTestRoundStateV2(
 				&istanbul.View{Round: big.NewInt(1), Sequence: big.NewInt(1)},
 				valSet,
 			),
@@ -325,7 +325,7 @@ func TestVerifyCommit(t *testing.T) {
 					Digest: common.BytesToHash([]byte("1234567890")),
 				},
 			},
-			roundState: newTestRoundState(
+			roundState: newTestRoundStateV2(
 				&istanbul.View{Round: big.NewInt(1), Sequence: big.NewInt(1)},
 				valSet,
 			),
@@ -339,7 +339,7 @@ func TestVerifyCommit(t *testing.T) {
 					Digest: newTestProposal().Hash(),
 				},
 			},
-			roundState: newTestRoundState(
+			roundState: newTestRoundStateV2(
 				&istanbul.View{Round: big.NewInt(1), Sequence: big.NewInt(1)},
 				valSet,
 			),
@@ -353,7 +353,7 @@ func TestVerifyCommit(t *testing.T) {
 					Digest: newTestProposal().Hash(),
 				},
 			},
-			roundState: newTestRoundState(
+			roundState: newTestRoundStateV2(
 				&istanbul.View{Round: big.NewInt(0), Sequence: big.NewInt(0)},
 				valSet,
 			),
@@ -367,7 +367,7 @@ func TestVerifyCommit(t *testing.T) {
 					Digest: newTestProposal().Hash(),
 				},
 			},
-			roundState: newTestRoundState(
+			roundState: newTestRoundStateV2(
 				&istanbul.View{Round: big.NewInt(0), Sequence: big.NewInt(0)},
 				valSet,
 			),
@@ -404,7 +404,7 @@ func BenchmarkHandleCommit(b *testing.B) {
 	for i, backend := range sys.backends {
 		c := backend.engine.(*core)
 		// same view as the expected one to everyone
-		c.current = newTestRoundState(
+		c.current = newTestRoundStateV2(
 			expectedSubject.View,
 			backend.peers,
 		)