core: remove Governance.NotifyRoundHeight (#431)

12 files changed, 49 insertions, 46 deletions

diff --git a/core/consensus.go b/core/consensus.go
index 67a8b05..e8d1d61 100644
--- a/core/consensus.go
+++ b/core/consensus.go
@@ -388,7 +388,7 @@ type Consensus struct {
 	dMoment                    time.Time
 	nodeSetCache               *utils.NodeSetCache
 	round                      uint64
-	roundToNotify              uint64
+	roundForNewConfig          uint64
 	lock                       sync.RWMutex
 	ctx                        context.Context
 	ctxCancel                  context.CancelFunc
@@ -586,7 +586,7 @@ func newConsensusForRound(
 func (con *Consensus) prepare(initBlock *types.Block) error {
 	// The block past from full node should be delivered already or known by
 	// full node. We don't have to notify it.
-	con.roundToNotify = initBlock.Position.Round + 1
+	con.roundForNewConfig = initBlock.Position.Round + 1
 	initRound := initBlock.Position.Round
 	initConfig := utils.GetConfigWithPanic(con.gov, initRound, con.logger)
 	// Setup context.
@@ -1185,13 +1185,13 @@ func (con *Consensus) deliverBlock(b *types.Block) {
 	con.cfgModule.untouchTSigHash(b.Hash)
 	con.logger.Debug("Calling Application.BlockDelivered", "block", b)
 	con.app.BlockDelivered(b.Hash, b.Position, b.Finalization.Clone())
-	if b.Position.Round == con.roundToNotify {
+	if b.Position.Round == con.roundForNewConfig {
 		// Get configuration for the round next to next round. Configuration
 		// for that round should be ready at this moment and is required for
 		// lattice module. This logic is related to:
 		//  - roundShift
 		//  - notifyGenesisRound
-		futureRound := con.roundToNotify + 1
+		futureRound := con.roundForNewConfig + 1
 		futureConfig := utils.GetConfigWithPanic(con.gov, futureRound, con.logger)
 		con.logger.Debug("Append Config", "round", futureRound)
 		if err := con.lattice.AppendConfig(
@@ -1201,14 +1201,7 @@ func (con *Consensus) deliverBlock(b *types.Block) {
 				"error", err)
 			panic(err)
 		}
-		// Only the first block delivered of that round would
-		// trigger this noitification.
-		con.logger.Debug("Calling Governance.NotifyRoundHeight",
-			"round", con.roundToNotify,
-			"height", b.Finalization.Height)
-		con.gov.NotifyRoundHeight(
-			con.roundToNotify, b.Finalization.Height)
-		con.roundToNotify++
+		con.roundForNewConfig++
 	}
 	if con.debugApp != nil {
 		con.debugApp.BlockReady(b.Hash)
diff --git a/core/consensus_test.go b/core/consensus_test.go
index d340c4e..ddaf731 100644
--- a/core/consensus_test.go
+++ b/core/consensus_test.go
@@ -207,7 +207,7 @@ func (s *ConsensusTestSuite) prepareConsensus(
 	conn *networkConnection) (
 	*test.App, *Consensus) {
 
-	app := test.NewApp(nil)
+	app := test.NewApp(0, nil)
 	dbInst, err := db.NewMemBackedDB()
 	s.Require().NoError(err)
 	nID := types.NewNodeID(prvKey.PublicKey())
diff --git a/core/interfaces.go b/core/interfaces.go
index a77ec93..408343f 100644
--- a/core/interfaces.go
+++ b/core/interfaces.go
@@ -115,10 +115,6 @@ type Governance interface {
 	// Return the genesis node set if round == 0.
 	NodeSet(round uint64) []crypto.PublicKey
 
-	// NotifyRoundHeight notifies governance contract the consensus height of
-	// the first block of the given round.
-	NotifyRoundHeight(targetRound, consensusHeight uint64)
-
 	//// DKG-related methods.
 
 	// AddDKGComplaint adds a DKGComplaint.
diff --git a/core/lattice_test.go b/core/lattice_test.go
index 488bef5..b847fad 100644
--- a/core/lattice_test.go
+++ b/core/lattice_test.go
@@ -110,7 +110,7 @@ func (s *LatticeTestSuite) newTestLatticeMgr(
 		pubKeys, cfg.LambdaBA, logger, true), ConfigRoundShift)
 	req.NoError(err)
 	// Setup application.
-	app := test.NewApp(gov.State())
+	app := test.NewApp(0, gov)
 	// Setup compaction chain.
 	cc := newCompactionChain(gov)
 	cc.init(&types.Block{})
diff --git a/core/test/app.go b/core/test/app.go
index dbee059..515ed23 100644
--- a/core/test/app.go
+++ b/core/test/app.go
@@ -110,21 +110,28 @@ type App struct {
 	DeliverSequence       common.Hashes
 	deliveredLock         sync.RWMutex
 	state                 *State
+	gov                   *Governance
 	lastPendingHeightLock sync.RWMutex
 	LastPendingHeight     uint64
+	roundToNotify         uint64
 }
 
 // NewApp constructs a TestApp instance.
-func NewApp(state *State) *App {
-	return &App{
+func NewApp(initRound uint64, gov *Governance) (app *App) {
+	app = &App{
 		Confirmed:            make(map[common.Hash]*types.Block),
 		LastConfirmedHeights: make(map[uint32]uint64),
 		TotalOrdered:         []*AppTotalOrderRecord{},
 		TotalOrderedByHash:   make(map[common.Hash]*AppTotalOrderRecord),
 		Delivered:            make(map[common.Hash]*AppDeliveredRecord),
 		DeliverSequence:      common.Hashes{},
-		state:                state,
+		gov:                  gov,
+		roundToNotify:        initRound,
 	}
+	if gov != nil {
+		app.state = gov.State()
+	}
+	return app
 }
 
 // PreparePayload implements Application interface.
@@ -274,6 +281,12 @@ func (app *App) BlockDelivered(
 				panic(err)
 			}
 		}
+		if app.roundToNotify == pos.Round {
+			if app.gov != nil {
+				app.gov.NotifyRound(app.roundToNotify)
+				app.roundToNotify++
+			}
+		}
 	}()
 }
 
diff --git a/core/test/app_test.go b/core/test/app_test.go
index 21c9a81..5ed562a 100644
--- a/core/test/app_test.go
+++ b/core/test/app_test.go
@@ -91,12 +91,12 @@ func (s *AppTestSuite) deliverBlock(
 func (s *AppTestSuite) TestCompare() {
 	req := s.Require()
 
-	app1 := NewApp(nil)
+	app1 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app1, s.to1)
 	s.setupAppByTotalOrderDeliver(app1, s.to2)
 	s.setupAppByTotalOrderDeliver(app1, s.to3)
 	// An App with different deliver sequence.
-	app2 := NewApp(nil)
+	app2 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app2, s.to1)
 	s.setupAppByTotalOrderDeliver(app2, s.to2)
 	hash := common.NewRandomHash()
@@ -105,7 +105,7 @@ func (s *AppTestSuite) TestCompare() {
 	s.deliverBlockWithTimeFromSequenceLength(app2, hash)
 	req.Equal(ErrMismatchBlockHashSequence, app1.Compare(app2))
 	// An App with different consensus time for the same block.
-	app3 := NewApp(nil)
+	app3 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app3, s.to1)
 	s.setupAppByTotalOrderDeliver(app3, s.to2)
 	for _, h := range s.to3.BlockHashes {
@@ -120,7 +120,7 @@ func (s *AppTestSuite) TestCompare() {
 	req.Equal(ErrMismatchConsensusTime, app1.Compare(app3))
 	req.Equal(ErrMismatchConsensusTime, app3.Compare(app1))
 	// An App without any delivered blocks.
-	app4 := NewApp(nil)
+	app4 := NewApp(0, nil)
 	req.Equal(ErrEmptyDeliverSequence, app4.Compare(app1))
 	req.Equal(ErrEmptyDeliverSequence, app1.Compare(app4))
 }
@@ -129,7 +129,7 @@ func (s *AppTestSuite) TestVerify() {
 	req := s.Require()
 
 	// An OK App instance.
-	app1 := NewApp(nil)
+	app1 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app1, s.to1)
 	s.setupAppByTotalOrderDeliver(app1, s.to2)
 	s.setupAppByTotalOrderDeliver(app1, s.to3)
@@ -139,7 +139,7 @@ func (s *AppTestSuite) TestVerify() {
 		uint64(len(app1.DeliverSequence)))
 	req.Equal(ErrDeliveredBlockNotConfirmed, app1.Verify())
 	// The consensus time is out of order.
-	app2 := NewApp(nil)
+	app2 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app2, s.to1)
 	for _, h := range s.to2.BlockHashes {
 		app2.BlockConfirmed(types.Block{Hash: h})
@@ -149,14 +149,14 @@ func (s *AppTestSuite) TestVerify() {
 		uint64(len(app2.DeliverSequence)+1))
 	req.Equal(ErrConsensusTimestampOutOfOrder, app2.Verify())
 	// A delivered block is not found in total ordering delivers.
-	app3 := NewApp(nil)
+	app3 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app3, s.to1)
 	hash := common.NewRandomHash()
 	app3.BlockConfirmed(types.Block{Hash: hash})
 	s.deliverBlockWithTimeFromSequenceLength(app3, hash)
 	req.Equal(ErrMismatchTotalOrderingAndDelivered, app3.Verify())
 	// A delivered block is not found in total ordering delivers.
-	app4 := NewApp(nil)
+	app4 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app4, s.to1)
 	for _, h := range s.to2.BlockHashes {
 		app4.BlockConfirmed(types.Block{Hash: h})
@@ -167,10 +167,10 @@ func (s *AppTestSuite) TestVerify() {
 	app4.TotalOrderingDelivered(common.Hashes{hash}, core.TotalOrderingModeNormal)
 	s.deliverBlockWithTimeFromSequenceLength(app4, hash)
 	// Witness ack on unknown block.
-	app5 := NewApp(nil)
+	app5 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app5, s.to1)
 	// The conensus height is out of order.
-	app6 := NewApp(nil)
+	app6 := NewApp(0, nil)
 	s.setupAppByTotalOrderDeliver(app6, s.to1)
 	for _, h := range s.to2.BlockHashes {
 		app6.BlockConfirmed(types.Block{Hash: h})
@@ -181,7 +181,7 @@ func (s *AppTestSuite) TestVerify() {
 		uint64(len(app6.DeliverSequence)+2))
 	req.Equal(ErrConsensusHeightOutOfOrder, app6.Verify())
 	// Test the acking block doesn't delivered.
-	app7 := NewApp(nil)
+	app7 := NewApp(0, nil)
 	// Patch a block's acks.
 	b7 := &types.Block{
 		Hash: common.NewRandomHash(),
@@ -199,7 +199,7 @@ func (s *AppTestSuite) TestVerify() {
 
 func (s *AppTestSuite) TestWitness() {
 	// Deliver several blocks, there is only one chain only.
-	app := NewApp(nil)
+	app := NewApp(0, nil)
 	deliver := func(b *types.Block) {
 		app.BlockConfirmed(*b)
 		app.BlockDelivered(b.Hash, b.Position, b.Finalization)
diff --git a/core/test/governance.go b/core/test/governance.go
index 5cf9732..81ced54 100644
--- a/core/test/governance.go
+++ b/core/test/governance.go
@@ -93,8 +93,9 @@ func (g *Governance) CRS(round uint64) common.Hash {
 	return g.stateModule.CRS(round)
 }
 
-// NotifyRoundHeight notifies governace contract to snapshot config.
-func (g *Governance) NotifyRoundHeight(round, height uint64) {
+// NotifyRound notifies governace contract to snapshot config, and broadcast
+// pending state change requests for next round if any.
+func (g *Governance) NotifyRound(round uint64) {
 	// Snapshot configuration for the shifted round, this behavior is synced with
 	// full node's implementation.
 	shiftedRound := round + g.roundShift
diff --git a/core/test/governance_test.go b/core/test/governance_test.go
index 30f1469..cef2aea 100644
--- a/core/test/governance_test.go
+++ b/core/test/governance_test.go
@@ -87,13 +87,13 @@ func (s *GovernanceTestSuite) TestRegisterChange() {
 	req.NoError(g.RegisterConfigChange(7, StateChangeNumChains, uint32(40)))
 	// In local mode, state for round 6 would be ready after notified with
 	// round 2.
-	g.NotifyRoundHeight(2, 0)
-	g.NotifyRoundHeight(3, 0)
+	g.NotifyRound(2)
+	g.NotifyRound(3)
 	// In local mode, state for round 7 would be ready after notified with
 	// round 6.
-	g.NotifyRoundHeight(4, 0)
+	g.NotifyRound(4)
 	// Notify governance to take a snapshot for round 7's configuration.
-	g.NotifyRoundHeight(5, 0)
+	g.NotifyRound(5)
 	req.Equal(g.Configuration(6).NumChains, uint32(32))
 	req.Equal(g.Configuration(7).NumChains, uint32(40))
 }
diff --git a/core/test/stopper_test.go b/core/test/stopper_test.go
index d296727..758a0e4 100644
--- a/core/test/stopper_test.go
+++ b/core/test/stopper_test.go
@@ -70,7 +70,7 @@ func (s *StopperTestSuite) TestStopByConfirmedBlocks() {
 		nodes   = GenerateRandomNodeIDs(2)
 	)
 	for _, nID := range nodes {
-		apps[nID] = NewApp(nil)
+		apps[nID] = NewApp(0, nil)
 		dbInst, err := db.NewMemBackedDB()
 		req.NoError(err)
 		dbInsts[nID] = dbInst
@@ -118,7 +118,7 @@ func (s *StopperTestSuite) TestStopByRound() {
 		nodes   = GenerateRandomNodeIDs(2)
 	)
 	for _, nID := range nodes {
-		apps[nID] = NewApp(nil)
+		apps[nID] = NewApp(0, nil)
 		dbInst, err := db.NewMemBackedDB()
 		req.NoError(err)
 		dbInsts[nID] = dbInst
diff --git a/integration_test/consensus_test.go b/integration_test/consensus_test.go
index fc6bb47..8af118b 100644
--- a/integration_test/consensus_test.go
+++ b/integration_test/consensus_test.go
@@ -76,9 +76,9 @@ func (s *ConsensusTestSuite) setupNodes(
 		)
 		gov := seedGov.Clone()
 		gov.SwitchToRemoteMode(networkModule)
-		gov.NotifyRoundHeight(0, 0)
+		gov.NotifyRound(0)
 		networkModule.AddNodeSetCache(utils.NewNodeSetCache(gov))
-		app := test.NewApp(gov.State())
+		app := test.NewApp(1, gov)
 		nID := types.NewNodeID(k.PublicKey())
 		nodes[nID] = &node{nID, nil, app, gov, dbInst, networkModule}
 		go func() {
diff --git a/integration_test/node.go b/integration_test/node.go
index c5109af..732e54a 100644
--- a/integration_test/node.go
+++ b/integration_test/node.go
@@ -105,7 +105,7 @@ func newNode(
 		return nil, err
 	}
 	// Setup test.App
-	app := test.NewApp(copiedGov.State())
+	app := test.NewApp(0, copiedGov)
 	// Setup lattice instance.
 	lattice := core.NewLattice(
 		dMoment,
@@ -275,7 +275,7 @@ func (n *Node) checkRoundSwitch(b *types.Block) (evts []*test.Event) {
 		return
 	}
 	// Handle round switching logic.
-	n.govModule.NotifyRoundHeight(n.roundToNotify, b.Finalization.Height)
+	n.govModule.NotifyRound(n.roundToNotify)
 	if n.roundToNotify == uint64(len(n.roundEndTimes)) {
 		config := n.govModule.Configuration(n.roundToNotify)
 		if config == nil {
diff --git a/simulation/node.go b/simulation/node.go
index 437d4b7..7c02991 100644
--- a/simulation/node.go
+++ b/simulation/node.go
@@ -226,7 +226,7 @@ func (n *node) prepareConfigs() {
 		prepareConfigs(i, n.cfg.Node.Changes, n.gov)
 	}
 	// This notification is implictly called in full node.
-	n.gov.NotifyRoundHeight(0, 0)
+	n.gov.NotifyRound(0)
 	// Setup of configuration is ready, can be switched to remote mode.
 	n.gov.SwitchToRemoteMode(n.netModule)
 }