Justified revision (#802)

* feature: add justified revision for block * fix: missing err declaration * docs: add justified revision to thor.yaml * feat: storing justified blockId in engine.data * a non-persist way to implement jutified * use store point to restore quality * refactor: rename CommitLevel interface * refactor: change approach to non-persist the justified block * bft: add justified tests * fix: rename file name and simplified error check in test * improve bft package * add comments --------- Co-authored-by: tony <[email protected]> Co-authored-by: otherview <[email protected]> Co-authored-by: Darren Kelly <[email protected]>
vechain · Sep 6, 2024 · dddcdd8 · dddcdd8
1 parent a57bf94
commit dddcdd8
Show file tree

Hide file tree

Showing 12 changed files with 360 additions and 68 deletions.
diff --git a/api/accounts/accounts.go b/api/accounts/accounts.go
@@ -31,15 +31,15 @@ type Accounts struct {
 	stater       *state.Stater
 	callGasLimit uint64
 	forkConfig   thor.ForkConfig
-	bft          bft.Finalizer
+	bft          bft.Committer
 }
 
 func New(
 	repo *chain.Repository,
 	stater *state.Stater,
 	callGasLimit uint64,
 	forkConfig thor.ForkConfig,
-	bft bft.Finalizer,
+	bft bft.Committer,
 ) *Accounts {
 	return &Accounts{
 		repo,

diff --git a/api/api.go b/api/api.go
@@ -38,7 +38,7 @@ func New(
 	stater *state.Stater,
 	txPool *txpool.TxPool,
 	logDB *logdb.LogDB,
-	bft bft.Finalizer,
+	bft bft.Committer,
 	nw node.Network,
 	forkConfig thor.ForkConfig,
 	allowedOrigins string,

diff --git a/api/blocks/blocks.go b/api/blocks/blocks.go
@@ -19,10 +19,10 @@ import (
 
 type Blocks struct {
 	repo *chain.Repository
-	bft  bft.Finalizer
+	bft  bft.Committer
 }
 
-func New(repo *chain.Repository, bft bft.Finalizer) *Blocks {
+func New(repo *chain.Repository, bft bft.Committer) *Blocks {
 	return &Blocks{
 		repo,
 		bft,

diff --git a/api/blocks/blocks_test.go b/api/blocks/blocks_test.go
@@ -20,6 +20,7 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/gorilla/mux"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/vechain/thor/v2/api/blocks"
 	"github.com/vechain/thor/v2/block"
 	"github.com/vechain/thor/v2/chain"
@@ -52,6 +53,7 @@ func TestBlock(t *testing.T) {
 		"testGetBlockByHeight":                  testGetBlockByHeight,
 		"testGetBestBlock":                      testGetBestBlock,
 		"testGetFinalizedBlock":                 testGetFinalizedBlock,
+		"testGetJustifiedBlock":                 testGetJustifiedBlock,
 		"testGetBlockWithRevisionNumberTooHigh": testGetBlockWithRevisionNumberTooHigh,
 	} {
 		t.Run(name, tt)
@@ -99,6 +101,16 @@ func testGetFinalizedBlock(t *testing.T) {
 	assert.Equal(t, genesisBlock.Header().ID(), finalized.ID)
 }
 
+func testGetJustifiedBlock(t *testing.T) {
+	res, statusCode := httpGet(t, ts.URL+"/blocks/justified")
+	justified := new(blocks.JSONCollapsedBlock)
+	require.NoError(t, json.Unmarshal(res, &justified))
+
+	assert.Equal(t, http.StatusOK, statusCode)
+	assert.Equal(t, uint32(0), justified.Number)
+	assert.Equal(t, genesisBlock.Header().ID(), justified.ID)
+}
+
 func testGetBlockById(t *testing.T) {
 	res, statusCode := httpGet(t, ts.URL+"/blocks/"+blk.Header().ID().String())
 	rb := new(blocks.JSONCollapsedBlock)

diff --git a/api/debug/debug.go b/api/debug/debug.go
@@ -43,7 +43,7 @@ type Debug struct {
 	forkConfig        thor.ForkConfig
 	callGasLimit      uint64
 	allowCustomTracer bool
-	bft               bft.Finalizer
+	bft               bft.Committer
 	allowedTracers    map[string]struct{}
 	skipPoA           bool
 }
@@ -54,7 +54,7 @@ func New(
 	forkConfig thor.ForkConfig,
 	callGaslimit uint64,
 	allowCustomTracer bool,
-	bft bft.Finalizer,
+	bft bft.Committer,
 	allowedTracers []string,
 	soloMode bool) *Debug {
 

diff --git a/api/doc/thor.yaml b/api/doc/thor.yaml
@@ -2252,15 +2252,15 @@ components:
     RevisionInQuery:
       name: revision
       in: query
-      description: Specify either `best`, `finalized`, a block number or block ID. If omitted, the `best` block is assumed.
+      description: Specify either `best`, `justified`, `finalized`, a block number or block ID. If omitted, the `best` block is assumed.
       schema:
         type: string
 
     CallCodeRevisionInQuery:
       name: revision
       in: query
       description: |
-        Specify either `best`,`next`, `finalized`, a block number or block ID. If omitted, the `best` block is assumed.
+        Specify either `best`, `next`, `justified`, `finalized`, a block number or block ID. If omitted, the `best` block is assumed.
         
         If the `next` block is specified, the call code will be executed on the next block, with the following:
         - The block number is the `best` block number plus one.
@@ -2279,6 +2279,7 @@ components:
         - a block ID (hex string)
         - a block number (integer)
         - `best` stands for latest block
+        - `justified` stands for the justified block
         - `finalized` stands for the finalized block
       required: true
       schema:

diff --git a/api/utils/revisions.go b/api/utils/revisions.go
@@ -21,6 +21,7 @@ const (
 	revBest      int64 = -1
 	revFinalized int64 = -2
 	revNext      int64 = -3
+	revJustified int64 = -4
 )
 
 type Revision struct {
@@ -41,6 +42,10 @@ func ParseRevision(revision string, allowNext bool) (*Revision, error) {
 		return &Revision{revFinalized}, nil
 	}
 
+	if revision == "justified" {
+		return &Revision{revJustified}, nil
+	}
+
 	if revision == "next" {
 		if !allowNext {
 			return nil, errors.New("invalid revision: next is not allowed")
@@ -67,7 +72,7 @@ func ParseRevision(revision string, allowNext bool) (*Revision, error) {
 
 // GetSummary returns the block summary for the given revision,
 // revision required to be a deterministic block other than "next".
-func GetSummary(rev *Revision, repo *chain.Repository, bft bft.Finalizer) (sum *chain.BlockSummary, err error) {
+func GetSummary(rev *Revision, repo *chain.Repository, bft bft.Committer) (sum *chain.BlockSummary, err error) {
 	var id thor.Bytes32
 	switch rev := rev.val.(type) {
 	case thor.Bytes32:
@@ -83,6 +88,11 @@ func GetSummary(rev *Revision, repo *chain.Repository, bft bft.Finalizer) (sum *
 			id = repo.BestBlockSummary().Header.ID()
 		case revFinalized:
 			id = bft.Finalized()
+		case revJustified:
+			id, err = bft.Justified()
+			if err != nil {
+				return nil, err
+			}
 		}
 	}
 	if id.IsZero() {
@@ -97,7 +107,7 @@ func GetSummary(rev *Revision, repo *chain.Repository, bft bft.Finalizer) (sum *
 
 // GetSummaryAndState returns the block summary and state for the given revision,
 // this function supports the "next" revision.
-func GetSummaryAndState(rev *Revision, repo *chain.Repository, bft bft.Finalizer, stater *state.Stater) (*chain.BlockSummary, *state.State, error) {
+func GetSummaryAndState(rev *Revision, repo *chain.Repository, bft bft.Committer, stater *state.Stater) (*chain.BlockSummary, *state.State, error) {
 	if rev.IsNext() {
 		best := repo.BestBlockSummary()
 

diff --git a/api/utils/revisions_test.go b/api/utils/revisions_test.go
@@ -41,6 +41,11 @@ func TestParseRevision(t *testing.T) {
 			err:      nil,
 			expected: &Revision{revBest},
 		},
+		{
+			revision: "justified",
+			err:      nil,
+			expected: &Revision{revJustified},
+		},
 		{
 			revision: "finalized",
 			err:      nil,
@@ -122,6 +127,11 @@ func TestGetSummary(t *testing.T) {
 			revision: &Revision{uint32(1234)},
 			err:      errors.New("not found"),
 		},
+		{
+			name:     "justified",
+			revision: &Revision{revJustified},
+			err:      nil,
+		},
 		{
 			name:     "finalized",
 			revision: &Revision{revFinalized},

diff --git a/bft/engine.go b/bft/engine.go
@@ -25,8 +25,14 @@ const dataStoreName = "bft.engine"
 
 var finalizedKey = []byte("finalized")
 
-type Finalizer interface {
+type Committer interface {
 	Finalized() thor.Bytes32
+	Justified() (thor.Bytes32, error)
+}
+
+type justified struct {
+	search thor.Bytes32
+	value  thor.Bytes32
 }
 
 // BFTEngine tracks all votes of blocks, computes the finalized checkpoint.
@@ -39,6 +45,7 @@ type BFTEngine struct {
 	master     thor.Address
 	casts      casts
 	finalized  atomic.Value
+	justified  atomic.Value
 	caches     struct {
 		state     *lru.Cache
 		quality   *lru.Cache
@@ -60,6 +67,7 @@ func NewEngine(repo *chain.Repository, mainDB *muxdb.MuxDB, forkConfig thor.Fork
 	engine.caches.quality, _ = lru.New(16)
 	engine.caches.justifier = cache.NewPrioCache(16)
 
+	// Restore finalized block, if any
 	if val, err := engine.data.Get(finalizedKey); err != nil {
 		if !engine.data.IsNotFound(err) {
 			return nil, err
@@ -77,6 +85,54 @@ func (engine *BFTEngine) Finalized() thor.Bytes32 {
 	return engine.finalized.Load().(thor.Bytes32)
 }
 
+// Justified returns the justified checkpoint.
+func (engine *BFTEngine) Justified() (thor.Bytes32, error) {
+	head := engine.repo.BestBlockSummary().Header
+	finalized := engine.Finalized()
+
+	// if head is in the first epoch and not concluded yet
+	if head.Number() < getCheckPoint(engine.forkConfig.FINALITY)+thor.CheckpointInterval-1 {
+		return finalized, nil
+	}
+
+	// find the recent concluded checkpoint
+	concluded := getCheckPoint(head.Number())
+	if head.Number() < getStorePoint(head.Number()) {
+		concluded -= thor.CheckpointInterval
+	}
+
+	headChain := engine.repo.NewChain(head.ID())
+
+	// storeID is the block id where an epoch concluded
+	storeID, err := headChain.GetBlockID(getStorePoint(concluded))
+	if err != nil {
+		return thor.Bytes32{}, err
+	}
+
+	if val := engine.justified.Load(); val != nil && storeID == val.(justified).search {
+		return val.(justified).value, nil
+	}
+
+	quality, err := engine.getQuality(storeID)
+	if err != nil {
+		return thor.Bytes32{}, err
+	}
+
+	// if the quality is 0, then the epoch is not justified
+	// this is possible for the starting epochs
+	if quality == 0 {
+		return finalized, nil
+	}
+
+	checkpoint, err := engine.findCheckpointByQuality(quality, finalized, storeID)
+	if err != nil {
+		return thor.Bytes32{}, err
+	}
+
+	engine.justified.Store(justified{search: storeID, value: checkpoint})
+	return checkpoint, nil
+}
+
 // Accepts checks if the given block is on the same branch of finalized checkpoint.
 func (engine *BFTEngine) Accepts(parentID thor.Bytes32) (bool, error) {
 	finalized := engine.Finalized()
@@ -123,7 +179,7 @@ func (engine *BFTEngine) CommitBlock(header *block.Header, isPacking bool) error
 		engine.caches.quality.Add(header.ID(), state.Quality)
 
 		if state.Committed && state.Quality > 1 {
-			id, err := engine.findCheckpointByQuality(state.Quality-1, engine.Finalized(), header.ParentID())
+			id, err := engine.findCheckpointByQuality(state.Quality-1, engine.Finalized(), header.ID())
 			if err != nil {
 				return err
 			}
@@ -182,17 +238,23 @@ func (engine *BFTEngine) ShouldVote(parentID thor.Bytes32) (bool, error) {
 
 	headQuality := st.Quality
 	finalized := engine.Finalized()
+	chain := engine.repo.NewChain(parentID)
 	// most recent justified checkpoint
 	var recentJC thor.Bytes32
 	if st.Justified {
 		// if justified in this round, use this round's checkpoint
-		checkpoint, err := engine.repo.NewChain(parentID).GetBlockID(getCheckPoint(block.Number(parentID)))
+		checkpoint, err := chain.GetBlockID(getCheckPoint(block.Number(parentID)))
 		if err != nil {
 			return false, err
 		}
 		recentJC = checkpoint
 	} else {
-		checkpoint, err := engine.findCheckpointByQuality(headQuality, finalized, parentID)
+		// if current round is not justified, find the most recent justified checkpoint
+		prev, err := chain.GetBlockID(getStorePoint(block.Number(parentID) - thor.CheckpointInterval))
+		if err != nil {
+			return false, err
+		}
+		checkpoint, err := engine.findCheckpointByQuality(headQuality, finalized, prev)
 		if err != nil {
 			return false, err
 		}
@@ -223,7 +285,7 @@ func (engine *BFTEngine) ShouldVote(parentID thor.Bytes32) (bool, error) {
 	return true, nil
 }
 
-// computeState computes the bft state regarding the given block header.
+// computeState computes the bft state regarding the given block header to the closest checkpoint.
 func (engine *BFTEngine) computeState(header *block.Header) (*bftState, error) {
 	if cached, ok := engine.caches.state.Get(header.ID()); ok {
 		return cached.(*bftState), nil
@@ -278,7 +340,8 @@ func (engine *BFTEngine) computeState(header *block.Header) (*bftState, error) {
 }
 
 // findCheckpointByQuality finds the first checkpoint reaches the given quality.
-func (engine *BFTEngine) findCheckpointByQuality(target uint32, finalized, parentID thor.Bytes32) (blockID thor.Bytes32, err error) {
+// It is caller's responsibility to ensure the epoch that headID belongs to is concluded.
+func (engine *BFTEngine) findCheckpointByQuality(target uint32, finalized, headID thor.Bytes32) (blockID thor.Bytes32, err error) {
 	defer func() {
 		if e := recover(); e != nil {
 			err = e.(error)
@@ -291,7 +354,7 @@ func (engine *BFTEngine) findCheckpointByQuality(target uint32, finalized, paren
 		searchStart = getCheckPoint(engine.forkConfig.FINALITY)
 	}
 
-	c := engine.repo.NewChain(parentID)
+	c := engine.repo.NewChain(headID)
 	get := func(i int) (uint32, error) {
 		id, err := c.GetBlockID(getStorePoint(searchStart + uint32(i)*thor.CheckpointInterval))
 		if err != nil {
@@ -300,7 +363,8 @@ func (engine *BFTEngine) findCheckpointByQuality(target uint32, finalized, paren
 		return engine.getQuality(id)
 	}
 
-	n := int((block.Number(parentID) + 1 - searchStart) / thor.CheckpointInterval)
+	// sort.Search searches from [0, n)
+	n := int((block.Number(headID)-searchStart)/thor.CheckpointInterval) + 1
 	num := sort.Search(n, func(i int) bool {
 		quality, err := get(i)
 		if err != nil {
@@ -310,6 +374,7 @@ func (engine *BFTEngine) findCheckpointByQuality(target uint32, finalized, paren
 		return quality >= target
 	})
 
+	// n means not found for sort.Search
 	if num == n {
 		return thor.Bytes32{}, errors.New("failed find the block by quality")
 	}