core: clean TODOs (#539)

* core: fix block timestamp (#529)

* Remove TODO

dMoment is still required when the block timestamp of
the genesis block is still need to be verified.

* Refine timestamp when preparing blocks
* Add timestamp checking in sanity check
* Revert code to patch position when preparing
* Remove TODOs that seems meaningless now
* Remove TODOs related to refactoring

* core: remove finalization (#531)

- Remove types.FinalizationResult, randomness
  field would be moved to `types.Block` directly.
- Add a placeholder for types.Block.Randomness
  field for blocks proposed from
  round < DKGDelayRound. (refer to core.NoRand)
- Make the height of the genesis block starts
  from 1. (refer to types.GenesisHeight)
- The fullnode's behavior of
  core.Governance.GetRoundHeight is (assume
  round-length is 100):
  - round: 0 -> 0 (we need to workaround this)
  - round: 1 -> 101
  - round: 2 -> 201
  - test.Governance already simulate this
    behavior, and the workaround is wrapped at
    utils.GetRoundHeight.

* core: fix issues (#536)

fixing code in these condition:
- assigning position without initializing them
  and expected it's for genesis
- compare height with 0

1 files changed, 113 insertions, 125 deletions

diff --git a/core/test/app_test.go b/core/test/app_test.go
index 0a68f5e..574604c 100644
--- a/core/test/app_test.go
+++ b/core/test/app_test.go
@@ -104,112 +104,97 @@ func (s *AppTestSuite) proposeFinalize(
 	}
 }
 
-func (s *AppTestSuite) deliverBlockWithTimeFromSequenceLength(
-	app *App, hash common.Hash) {
-
-	s.deliverBlock(app, hash, time.Time{}.Add(
-		time.Duration(len(app.DeliverSequence))*time.Second),
-		uint64(len(app.DeliverSequence)+1))
-}
-
-func (s *AppTestSuite) deliverBlock(
-	app *App, hash common.Hash, timestamp time.Time, height uint64) {
-
-	app.BlockDelivered(hash, types.Position{}, types.FinalizationResult{
-		Timestamp: timestamp,
-		Height:    height,
-	})
-}
-
 func (s *AppTestSuite) TestCompare() {
 	var (
-		now = time.Now().UTC()
-		b0  = types.Block{Hash: common.Hash{}}
-		b1  = types.Block{
-			Hash:     common.NewRandomHash(),
-			Position: types.Position{Height: 1},
+		b0 = types.Block{
+			Hash:       common.Hash{},
+			Position:   types.Position{Height: types.GenesisHeight},
+			Randomness: []byte("b0")}
+		b1 = types.Block{
+			Hash:       common.NewRandomHash(),
+			Position:   types.Position{Height: types.GenesisHeight + 1},
+			Randomness: []byte("b1"),
 		}
 	)
 	// Prepare an OK App instance.
 	app1 := NewApp(0, nil, nil)
 	app1.BlockConfirmed(b0)
 	app1.BlockConfirmed(b1)
-	app1.BlockDelivered(b0.Hash, b0.Position, types.FinalizationResult{
-		Height:    1,
-		Timestamp: now,
-	})
-	app1.BlockDelivered(b1.Hash, b1.Position, types.FinalizationResult{
-		Height:    2,
-		Timestamp: now.Add(1 * time.Second),
-	})
+	app1.BlockDelivered(b0.Hash, b0.Position, b0.Randomness)
+	app1.BlockDelivered(b1.Hash, b1.Position, b1.Randomness)
 	app2 := NewApp(0, nil, nil)
-	s.Require().Equal(ErrEmptyDeliverSequence.Error(),
+	s.Require().EqualError(ErrEmptyDeliverSequence,
 		app1.Compare(app2).Error())
 	app2.BlockConfirmed(b0)
-	app2.BlockDelivered(b0.Hash, b0.Position, types.FinalizationResult{
-		Height:    1,
-		Timestamp: now,
-	})
+	app2.BlockDelivered(b0.Hash, b0.Position, b0.Randomness)
 	b1Bad := types.Block{
-		Hash:     common.NewRandomHash(),
-		Position: types.Position{Height: 1},
+		Hash:       common.NewRandomHash(),
+		Position:   types.Position{Height: types.GenesisHeight + 1},
+		Randomness: []byte("b1Bad"),
 	}
 	app2.BlockConfirmed(b1Bad)
-	app2.BlockDelivered(b1Bad.Hash, b1Bad.Position, types.FinalizationResult{
-		Height:    1,
-		Timestamp: now,
-	})
-	s.Require().Equal(ErrMismatchBlockHashSequence.Error(),
+	app2.BlockDelivered(b1Bad.Hash, b1Bad.Position, b1Bad.Randomness)
+	s.Require().EqualError(ErrMismatchBlockHashSequence,
 		app1.Compare(app2).Error())
 	app2 = NewApp(0, nil, nil)
 	app2.BlockConfirmed(b0)
-	app2.BlockDelivered(b0.Hash, b0.Position, types.FinalizationResult{
-		Height:    1,
-		Timestamp: now.Add(1 * time.Second),
-	})
-	s.Require().Equal(ErrMismatchConsensusTime.Error(),
-		app1.Compare(app2).Error())
+	app2.BlockDelivered(b0.Hash, b0.Position, []byte("b0-another"))
+	s.Require().EqualError(ErrMismatchRandomness, app1.Compare(app2).Error())
 }
 
 func (s *AppTestSuite) TestVerify() {
 	var (
 		now = time.Now().UTC()
-		b0  = types.Block{Hash: common.Hash{}}
-		b1  = types.Block{
-			Hash:     common.NewRandomHash(),
-			Position: types.Position{Height: 1},
+		b0  = types.Block{
+			Hash:       common.Hash{},
+			Position:   types.Position{Height: types.GenesisHeight},
+			Randomness: []byte("b0"),
+			Timestamp:  now,
+		}
+		b1 = types.Block{
+			Hash:       common.NewRandomHash(),
+			Position:   types.Position{Height: types.GenesisHeight + 1},
+			Randomness: []byte("b1"),
+			Timestamp:  now.Add(1 * time.Second),
 		}
 	)
+	// ErrDeliveredBlockNotConfirmed
 	app := NewApp(0, nil, nil)
 	s.Require().Equal(ErrEmptyDeliverSequence.Error(), app.Verify().Error())
-	app.BlockDelivered(b0.Hash, b0.Position, types.FinalizationResult{})
-	app.BlockDelivered(b1.Hash, b1.Position, types.FinalizationResult{Height: 1})
-	s.Require().Equal(
-		ErrDeliveredBlockNotConfirmed.Error(), app.Verify().Error())
+	app.BlockDelivered(b0.Hash, b0.Position, b0.Randomness)
+	app.BlockDelivered(b1.Hash, b1.Position, b1.Randomness)
+	s.Require().EqualError(ErrDeliveredBlockNotConfirmed, app.Verify().Error())
+	// ErrTimestampOutOfOrder.
+	app = NewApp(0, nil, nil)
+	now = time.Now().UTC()
+	b0Bad := *(b0.Clone())
+	b0Bad.Timestamp = now
+	b1Bad := *(b1.Clone())
+	b1Bad.Timestamp = now.Add(-1 * time.Second)
+	app.BlockConfirmed(b0Bad)
+	app.BlockDelivered(b0Bad.Hash, b0Bad.Position, b0Bad.Randomness)
+	app.BlockConfirmed(b1Bad)
+	app.BlockDelivered(b1Bad.Hash, b1Bad.Position, b1Bad.Randomness)
+	s.Require().EqualError(ErrTimestampOutOfOrder, app.Verify().Error())
+	// ErrInvalidHeight.
+	app = NewApp(0, nil, nil)
+	b0Bad = *(b0.Clone())
+	b0Bad.Position.Height = 0
+	s.Require().Panics(func() { app.BlockConfirmed(b0Bad) })
+	b0Bad.Position.Height = 2
+	s.Require().Panics(func() { app.BlockConfirmed(b0Bad) })
+	// ErrEmptyRandomness
 	app = NewApp(0, nil, nil)
 	app.BlockConfirmed(b0)
-	app.BlockDelivered(b0.Hash, b0.Position, types.FinalizationResult{
-		Height:    1,
-		Timestamp: now,
-	})
-	app.BlockConfirmed(b1)
-	app.BlockDelivered(b1.Hash, b1.Position, types.FinalizationResult{
-		Height:    2,
-		Timestamp: now.Add(-1 * time.Second),
-	})
-	s.Require().Equal(ErrConsensusTimestampOutOfOrder.Error(),
-		app.Verify().Error())
+	app.BlockDelivered(b0.Hash, b0.Position, []byte{})
+	s.Require().EqualError(ErrEmptyRandomness, app.Verify().Error())
+	// OK.
 	app = NewApp(0, nil, nil)
 	app.BlockConfirmed(b0)
 	app.BlockConfirmed(b1)
-	app.BlockDelivered(b0.Hash, b0.Position, types.FinalizationResult{
-		Height:    1,
-		Timestamp: now,
-	})
-	app.BlockDelivered(b1.Hash, b1.Position, types.FinalizationResult{
-		Height:    1,
-		Timestamp: now.Add(1 * time.Second),
-	})
+	app.BlockDelivered(b0.Hash, b0.Position, b0.Randomness)
+	app.BlockDelivered(b1.Hash, b1.Position, b1.Randomness)
+	s.Require().NoError(app.Verify())
 }
 
 func (s *AppTestSuite) TestWitness() {
@@ -217,36 +202,30 @@ func (s *AppTestSuite) TestWitness() {
 	app := NewApp(0, nil, nil)
 	deliver := func(b *types.Block) {
 		app.BlockConfirmed(*b)
-		app.BlockDelivered(b.Hash, b.Position, b.Finalization)
+		app.BlockDelivered(b.Hash, b.Position, b.Randomness)
 	}
 	b00 := &types.Block{
-		Hash: common.NewRandomHash(),
-		Finalization: types.FinalizationResult{
-			Height:    1,
-			Timestamp: time.Now().UTC(),
-		}}
+		Hash:       common.NewRandomHash(),
+		Position:   types.Position{Height: 1},
+		Timestamp:  time.Now().UTC(),
+		Randomness: common.GenerateRandomBytes(),
+	}
 	b01 := &types.Block{
-		Hash:     common.NewRandomHash(),
-		Position: types.Position{Height: 1},
-		Finalization: types.FinalizationResult{
-			ParentHash: b00.Hash,
-			Height:     2,
-			Timestamp:  time.Now().UTC(),
-		},
+		Hash:       common.NewRandomHash(),
+		Position:   types.Position{Height: 2},
+		Timestamp:  time.Now().UTC(),
+		Randomness: common.GenerateRandomBytes(),
 		Witness: types.Witness{
-			Height: 1,
+			Height: b00.Position.Height,
 			Data:   b00.Hash.Bytes(),
 		}}
 	b02 := &types.Block{
-		Hash:     common.NewRandomHash(),
-		Position: types.Position{Height: 2},
-		Finalization: types.FinalizationResult{
-			ParentHash: b01.Hash,
-			Height:     3,
-			Timestamp:  time.Now().UTC(),
-		},
+		Hash:       common.NewRandomHash(),
+		Position:   types.Position{Height: 3},
+		Timestamp:  time.Now().UTC(),
+		Randomness: common.GenerateRandomBytes(),
 		Witness: types.Witness{
-			Height: 1,
+			Height: b00.Position.Height,
 			Data:   b00.Hash.Bytes(),
 		}}
 	deliver(b00)
@@ -257,24 +236,32 @@ func (s *AppTestSuite) TestWitness() {
 		Witness: types.Witness{Height: 4}}))
 	// Mismatched witness height and data, should return invalid.
 	s.Require().Equal(types.VerifyInvalidBlock, app.VerifyBlock(&types.Block{
-		Witness: types.Witness{Height: 1, Data: b01.Hash.Bytes()}}))
+		Witness: types.Witness{
+			Height: 1,
+			Data:   b01.Hash.Bytes(),
+		}}))
 	// We can only verify a block followed last confirmed block.
 	s.Require().Equal(types.VerifyRetryLater, app.VerifyBlock(&types.Block{
-		Witness:  types.Witness{Height: 2, Data: b01.Hash.Bytes()},
-		Position: types.Position{Height: 4}}))
+		Witness: types.Witness{
+			Height: b01.Position.Height,
+			Data:   b01.Hash.Bytes()},
+		Position: types.Position{Height: 5}}))
 	// It's the OK case.
 	s.Require().Equal(types.VerifyOK, app.VerifyBlock(&types.Block{
-		Witness:  types.Witness{Height: 2, Data: b01.Hash.Bytes()},
-		Position: types.Position{Height: 3}}))
+		Witness: types.Witness{
+			Height: b01.Position.Height,
+			Data:   b01.Hash.Bytes()},
+		Position: types.Position{Height: 4}}))
 	// Check current last pending height.
-	s.Require().Equal(app.LastPendingHeight, uint64(3))
+	_, lastRec := app.LastDeliveredRecordNoLock()
+	s.Require().Equal(lastRec.Pos.Height, uint64(3))
 	// We can only prepare witness for what've delivered.
 	_, err := app.PrepareWitness(4)
 	s.Require().Equal(err.Error(), ErrLowerPendingHeight.Error())
 	// It should be ok to prepare for height that already delivered.
 	w, err := app.PrepareWitness(3)
 	s.Require().NoError(err)
-	s.Require().Equal(w.Height, b02.Finalization.Height)
+	s.Require().Equal(w.Height, b02.Position.Height)
 	s.Require().Equal(0, bytes.Compare(w.Data, b02.Hash[:]))
 }
 
@@ -292,10 +279,10 @@ func (s *AppTestSuite) TestAttachedWithRoundEvent() {
 	for r := uint64(2); r <= uint64(20); r++ {
 		gov.ProposeCRS(r, getCRS(r, 0))
 	}
-	for r := uint64(0); r <= uint64(19); r++ {
-		gov.NotifyRound(r, r*roundLength)
+	for r := uint64(1); r <= uint64(19); r++ {
+		gov.NotifyRound(r, utils.GetRoundHeight(gov, r-1)+roundLength)
 	}
-	gov.NotifyRound(20, 2200)
+	gov.NotifyRound(20, 2201)
 	// Reset round#20 twice, then make it done DKG preparation.
 	gov.ResetDKG(getCRS(20, 1))
 	gov.ResetDKG(getCRS(20, 2))
@@ -311,8 +298,8 @@ func (s *AppTestSuite) TestAttachedWithRoundEvent() {
 	s.proposeMPK(gov, 22, 0, 3)
 	s.proposeFinalize(gov, 22, 0, 3)
 	// Prepare utils.RoundEvent, starts from round#19, reset(for round#20)#1.
-	rEvt, err := utils.NewRoundEvent(context.Background(), gov, s.logger, 19,
-		2019, core.ConfigRoundShift)
+	rEvt, err := utils.NewRoundEvent(context.Background(), gov, s.logger,
+		types.Position{Round: 19, Height: 2019}, core.ConfigRoundShift)
 	s.Require().NoError(err)
 	// Register a handler to collects triggered events.
 	evts := make(chan evtParamToCheck, 3)
@@ -331,30 +318,31 @@ func (s *AppTestSuite) TestAttachedWithRoundEvent() {
 	deliver := func(round, start, end uint64) {
 		for i := start; i <= end; i++ {
 			b := &types.Block{
-				Hash:         common.NewRandomHash(),
-				Position:     types.Position{Round: round, Height: i},
-				Finalization: types.FinalizationResult{Height: i},
+				Hash:       common.NewRandomHash(),
+				Position:   types.Position{Round: round, Height: i},
+				Randomness: common.GenerateRandomBytes(),
 			}
 			app.BlockConfirmed(*b)
-			app.BlockDelivered(b.Hash, b.Position, b.Finalization)
+			app.BlockDelivered(b.Hash, b.Position, b.Randomness)
 		}
 	}
-	// Deliver blocks from height=2020 to height=2081.
+	// Deliver blocks from height=2020 to height=2092.
 	for r := uint64(0); r <= uint64(19); r++ {
-		deliver(r, r*roundLength, (r+1)*roundLength-1)
+		begin := utils.GetRoundHeight(gov, r)
+		deliver(r, begin, begin+roundLength-1)
 	}
-	deliver(19, 2000, 2091)
-	s.Require().Equal(<-evts, evtParamToCheck{19, 1, 2000, gov.CRS(19)})
-	s.Require().Equal(<-evts, evtParamToCheck{19, 2, 2100, gov.CRS(19)})
-	s.Require().Equal(<-evts, evtParamToCheck{20, 0, 2200, gov.CRS(20)})
+	deliver(19, 2001, 2092)
+	s.Require().Equal(<-evts, evtParamToCheck{19, 1, 2001, gov.CRS(19)})
+	s.Require().Equal(<-evts, evtParamToCheck{19, 2, 2101, gov.CRS(19)})
+	s.Require().Equal(<-evts, evtParamToCheck{20, 0, 2201, gov.CRS(20)})
 	// Deliver blocks from height=2082 to height=2281.
-	deliver(19, 2092, 2199)
-	deliver(20, 2200, 2291)
-	s.Require().Equal(<-evts, evtParamToCheck{21, 0, 2300, gov.CRS(21)})
+	deliver(19, 2093, 2200)
+	deliver(20, 2201, 2292)
+	s.Require().Equal(<-evts, evtParamToCheck{21, 0, 2301, gov.CRS(21)})
 	// Deliver blocks from height=2282 to height=2381.
-	deliver(20, 2292, 2299)
-	deliver(21, 2300, 2391)
-	s.Require().Equal(<-evts, evtParamToCheck{22, 0, 2400, gov.CRS(22)})
+	deliver(20, 2293, 2300)
+	deliver(21, 2301, 2392)
+	s.Require().Equal(<-evts, evtParamToCheck{22, 0, 2401, gov.CRS(22)})
 }
 
 func TestApp(t *testing.T) {