Kevjue/restore snapshot parents (ethereum#227)

* restored the parents variable in the snapshot function

* added more logging

* fixed the minParentsBlockNumber

* added comment for the 'parents' parameter

consensus/istanbul/backend/api.go

@@ -42,7 +42,7 @@ func (api *API) GetSnapshot(number *rpc.BlockNumber) (*Snapshot, error) {
 	if header == nil {
 		return nil, errUnknownBlock
 	}
-	return api.istanbul.snapshot(api.chain, header.Number.Uint64(), header.Hash())
+	return api.istanbul.snapshot(api.chain, header.Number.Uint64(), header.Hash(), nil)
 }
 
 // GetSnapshotAtHash retrieves the state snapshot at a given block.
@@ -51,7 +51,7 @@ func (api *API) GetSnapshotAtHash(hash common.Hash) (*Snapshot, error) {
 	if header == nil {
 		return nil, errUnknownBlock
 	}
-	return api.istanbul.snapshot(api.chain, header.Number.Uint64(), header.Hash())
+	return api.istanbul.snapshot(api.chain, header.Number.Uint64(), header.Hash(), nil)
 }
 
 // GetValidators retrieves the list of authorized validators at the specified block.
@@ -67,7 +67,7 @@ func (api *API) GetValidators(number *rpc.BlockNumber) ([]common.Address, error)
 	if header == nil {
 		return nil, errUnknownBlock
 	}
-	snap, err := api.istanbul.snapshot(api.chain, header.Number.Uint64(), header.Hash())
+	snap, err := api.istanbul.snapshot(api.chain, header.Number.Uint64(), header.Hash(), nil)
 	if err != nil {
 		return nil, err
 	}
@@ -80,7 +80,7 @@ func (api *API) GetValidatorsAtHash(hash common.Hash) ([]common.Address, error)
 	if header == nil {
 		return nil, errUnknownBlock
 	}
-	snap, err := api.istanbul.snapshot(api.chain, header.Number.Uint64(), header.Hash())
+	snap, err := api.istanbul.snapshot(api.chain, header.Number.Uint64(), header.Hash(), nil)
 	if err != nil {
 		return nil, err
 	}

consensus/istanbul/backend/backend.go

@@ -363,7 +363,7 @@ func (sb *Backend) ParentValidators(proposal istanbul.Proposal) istanbul.Validat
 }
 
 func (sb *Backend) getValidators(number uint64, hash common.Hash) istanbul.ValidatorSet {
-	snap, err := sb.snapshot(sb.chain, number, hash)
+	snap, err := sb.snapshot(sb.chain, number, hash, nil)
 	if err != nil {
 		return validator.NewSet(nil, sb.config.ProposerPolicy)
 	}

consensus/istanbul/backend/engine.go

@@ -53,7 +53,7 @@ var (
 	// errInvalidSignature is returned when given signature is not signed by given
 	// address.
 	errInvalidSignature = errors.New("invalid signature")
-	// errUnknownBlock is returned when the list of validators is requested for a block
+	// errUnknownBlock is returned when the list of validators or header is requested for a block
 	// that is not part of the local blockchain.
 	errUnknownBlock = errors.New("unknown block")
 	// errUnauthorized is returned if a header is signed by a non authorized entity.
@@ -253,7 +253,7 @@ func (sb *Backend) verifySigner(chain consensus.ChainReader, header *types.Heade
 	}
 
 	// Retrieve the snapshot needed to verify this header and cache it
-	snap, err := sb.snapshot(chain, number-1, header.ParentHash)
+	snap, err := sb.snapshot(chain, number-1, header.ParentHash, parents)
 	if err != nil {
 		return err
 	}
@@ -280,7 +280,7 @@ func (sb *Backend) verifyCommittedSeals(chain consensus.ChainReader, header *typ
 	}
 
 	// Retrieve the snapshot needed to verify this header and cache it
-	snap, err := sb.snapshot(chain, number-1, header.ParentHash)
+	snap, err := sb.snapshot(chain, number-1, header.ParentHash, parents)
 	if err != nil {
 		return err
 	}
@@ -382,7 +382,7 @@ func (sb *Backend) UpdateValSetDiff(chain consensus.ChainReader, header *types.H
 
 		} else {
 			// Get the last epoch's validator set
-			snap, err := sb.snapshot(chain, header.Number.Uint64()-1, header.ParentHash)
+			snap, err := sb.snapshot(chain, header.Number.Uint64()-1, header.ParentHash, nil)
 			if err != nil {
 				return err
 			}
@@ -444,7 +444,7 @@ func (sb *Backend) Seal(chain consensus.ChainReader, block *types.Block, results
 	number := header.Number.Uint64()
 
 	// Bail out if we're unauthorized to sign a block
-	snap, err := sb.snapshot(chain, number-1, header.ParentHash)
+	snap, err := sb.snapshot(chain, number-1, header.ParentHash, nil)
 	if err != nil {
 		return err
 	}
@@ -601,10 +601,12 @@ func (sb *Backend) Stop() error {
 //
 // hash - The requested snapshot's block's hash
 // number - The requested snapshot's block number
-func (sb *Backend) snapshot(chain consensus.ChainReader, number uint64, hash common.Hash) (*Snapshot, error) {
+// parents - (Optional argument) An array of headers from directly previous blocks.
+func (sb *Backend) snapshot(chain consensus.ChainReader, number uint64, hash common.Hash, parents []*types.Header) (*Snapshot, error) {
 	// Search for a snapshot in memory or on disk
 	var (
 		headers   []*types.Header
+		header    *types.Header
 		snap      *Snapshot
 		blockHash common.Hash
 	)
@@ -687,9 +689,25 @@ func (sb *Backend) snapshot(chain consensus.ChainReader, number uint64, hash com
 
 	// Calculate the returned snapshot by applying epoch headers' val set diffs to the intermediate snapshot (the one that is retreived/created from above).
 	// This will involve retrieving all of those headers into an array, and then call snapshot.apply on that array and the intermediate snapshot.
+	// Note that the callee of this method may have passed in a set of previous headers, so we may be able to use some of them.
+	minParentsBlockNumber := number - uint64(len(parents)) + 1
 	for numberIter+sb.config.Epoch <= number {
 		numberIter += sb.config.Epoch
-		headers = append(headers, chain.GetHeaderByNumber(numberIter))
+
+		log.Trace("Retrieving ancestor header", "number", number, "numberIter", numberIter, "minParentsBlockNumber", minParentsBlockNumber, "parents size", len(parents))
+
+		if len(parents) > 0 && numberIter >= minParentsBlockNumber {
+			header = parents[numberIter-minParentsBlockNumber]
+			log.Trace("Retrieved header from parents param", "header num", header.Number.Uint64())
+		} else {
+			header = chain.GetHeaderByNumber(numberIter)
+			if header == nil {
+				log.Error("The header retrieved from the chain is nil", "block num", numberIter)
+				return nil, errUnknownBlock
+			}
+		}
+
+		headers = append(headers, header)
 	}
 
 	if len(headers) > 0 {

consensus/istanbul/backend/engine_test.go

@@ -52,7 +52,7 @@ func newBlockChain(n int, isFullChain bool) (*core.BlockChain, *Backend) {
 		panic(err)
 	}
 	b.Start(blockchain, blockchain.CurrentBlock, blockchain.HasBadBlock, nil, nil, nil)
-	snap, err := b.snapshot(blockchain, 0, common.Hash{})
+	snap, err := b.snapshot(blockchain, 0, common.Hash{}, nil)
 	if err != nil {
 		panic(err)
 	}

consensus/istanbul/backend/snapshot_test.go

@@ -266,7 +266,7 @@ func TestValSetChange(t *testing.T) {
 
 			prevHeader = header
 		}
-		snap, err := engine.snapshot(chain, prevHeader.Number.Uint64(), prevHeader.Hash())
+		snap, err := engine.snapshot(chain, prevHeader.Number.Uint64(), prevHeader.Hash(), nil)
 		if err != tt.err {
 			t.Errorf("test %d: error mismatch:  have %v, want %v", i, err, tt.err)
 			continue