Merge pull request #1148 from hyperledger/develop

Release 0.26.2

Author: Silas Davis

CHANGELOG.md

@@ -2,6 +2,11 @@
 ## [Unreleased]
 
 
+## [0.26.2] - 2019-06-19
+### Fixed
+- [Blockchain] Persist LastBlockTime in Blockchain - before this patch LastBlockTime would only be set correctly after the first block had been received after a node is restarted - this can lead to non-determinism in the EVM via the TIMESTAMP opcode that use the LastBlockTime which is itself sourced from Tendermint's block header (from their implementation of BFT time). Implementing no empty blocks made observing this bug more likely by increasing the amount of time spent in a bad state (LastBlockTime is initially set to GenesisTime).
+
+
 ## [0.26.1] - 2019-06-16
 ### Changed
 - [CLI] 'burrow dump' renamed 'burrow dump remote'
@@ -502,7 +507,8 @@ This release marks the start of Eris-DB as the full permissioned blockchain node
   - [Blockchain] Fix getBlocks to respect block height cap.
 
 
-[Unreleased]: https://github.com/hyperledger/burrow/compare/v0.26.1...HEAD
+[Unreleased]: https://github.com/hyperledger/burrow/compare/v0.26.2...HEAD
+[0.26.2]: https://github.com/hyperledger/burrow/compare/v0.26.1...v0.26.2
 [0.26.1]: https://github.com/hyperledger/burrow/compare/v0.26.0...v0.26.1
 [0.26.0]: https://github.com/hyperledger/burrow/compare/v0.25.1...v0.26.0
 [0.25.1]: https://github.com/hyperledger/burrow/compare/v0.25.0...v0.25.1

NOTES.md

@@ -1,15 +1,3 @@
-### Changed
-- [CLI] 'burrow dump' renamed 'burrow dump remote'
-- [Consensus] By default Burrow no longer creates empty blocks at the end of a round - though does make on every 5 minutes by default. Set CreateEmptyBlocks to "never" or omit to create no blocks unless there are transactions, or "always" to generate blocks even when there are no transactions.
-- [State] Burrow state does not store empty blocks in the execution event store even when Tendermint creates them.
-- [Build] 'make install_burrow' is now just 'make install'
-
 ### Fixed
-- [Deploy] Always read TxExecution exception in Burrow deploy to avoid panics later on
-- [Restore] Set restore transaction hash to non-zero (sha256 of original ChainID + Height)
-- [Vent] --txs and --blocks now actually enable their respective tables in the Vent database
-- [Consensus] Tendermint config CreateEmptyBlocks, CreateEmptyBlocksInterval now work as intended and prevent empty blocks being produced (except when needed for proof purposes) or when the interval expires (when set)
-
-### Added
-- [Dump] burrow dump now has local variant that produces a dump directly from a compatible burrow directory rather than over GRPC. If dumping/restoring between state-incompatible versions use burrow dump remote. 
+- [Blockchain] Persist LastBlockTime in Blockchain - before this patch LastBlockTime would only be set correctly after the first block had been received after a node is restarted - this can lead to non-determinism in the EVM via the TIMESTAMP opcode that use the LastBlockTime which is itself sourced from Tendermint's block header (from their implementation of BFT time). Implementing no empty blocks made observing this bug more likely by increasing the amount of time spent in a bad state (LastBlockTime is initially set to GenesisTime).
 

bcm/block.go

@@ -17,15 +17,16 @@ func NewBlock(txDecoder txs.Decoder, block *types.Block) *Block {
 	}
 }
 
-func (b *Block) Transactions(iter func(*txs.Envelope) (stop bool)) (stopped bool, err error) {
+func (b *Block) Transactions(iter func(*txs.Envelope) error) error {
 	for i := 0; i < len(b.Txs); i++ {
 		tx, err := b.txDecoder.DecodeTx(b.Txs[i])
 		if err != nil {
-			return false, err
+			return err
 		}
-		if iter(tx) {
-			return true, nil
+		err = iter(tx)
+		if err != nil {
+			return err
 		}
 	}
-	return false, nil
+	return nil
 }

bcm/block_store.go

@@ -52,9 +52,9 @@ func (bs *BlockStore) BlockMeta(height int64) (_ *types.BlockMeta, err error) {
 }
 
 // Iterate over blocks between start (inclusive) and end (exclusive)
-func (bs *BlockStore) Blocks(start, end int64, iter func(*Block) (stop bool)) (stopped bool, err error) {
+func (bs *BlockStore) Blocks(start, end int64, iter func(*Block) error) error {
 	if end > 0 && start >= end {
-		return false, fmt.Errorf("end height must be strictly greater than start height")
+		return fmt.Errorf("end height must be strictly greater than start height")
 	}
 	if start <= 0 {
 		// From first block
@@ -68,12 +68,13 @@ func (bs *BlockStore) Blocks(start, end int64, iter func(*Block) (stop bool)) (s
 	for height := start; height <= end; height++ {
 		block, err := bs.Block(height)
 		if err != nil {
-			return false, err
+			return err
 		}
-		if iter(block) {
-			return true, nil
+		err = iter(block)
+		if err != nil {
+			return err
 		}
 	}
 
-	return false, nil
+	return nil
 }

bcm/blockchain.go

@@ -48,60 +48,63 @@ type BlockchainInfo interface {
 
 type Blockchain struct {
 	sync.RWMutex
-	db                    dbm.DB
-	genesisHash           []byte
-	genesisDoc            genesis.GenesisDoc
-	chainID               string
-	lastBlockHeight       uint64
-	lastBlockTime         time.Time
-	lastBlockHash         []byte
-	lastCommitTime        time.Time
-	lastCommitDuration    time.Duration
-	appHashAfterLastBlock []byte
-	blockStore            *BlockStore
+	persistedState PersistedState
+	// Non-persisted state
+	db                 dbm.DB
+	blockStore         *BlockStore
+	genesisDoc         genesis.GenesisDoc
+	lastBlockHash      []byte
+	lastCommitTime     time.Time
+	lastCommitDuration time.Duration
 }
 
 var _ BlockchainInfo = &Blockchain{}
 
 type PersistedState struct {
 	AppHashAfterLastBlock []byte
+	LastBlockTime         time.Time
 	LastBlockHeight       uint64
 	GenesisHash           []byte
 }
 
 // LoadOrNewBlockchain returns true if state already exists
-func LoadOrNewBlockchain(db dbm.DB, genesisDoc *genesis.GenesisDoc, logger *logging.Logger) (bool, *Blockchain, error) {
+func LoadOrNewBlockchain(db dbm.DB, genesisDoc *genesis.GenesisDoc, logger *logging.Logger) (_ *Blockchain, exists bool, _ error) {
 	logger = logger.WithScope("LoadOrNewBlockchain")
 	logger.InfoMsg("Trying to load blockchain state from database",
 		"database_key", stateKey)
 	bc, err := loadBlockchain(db, genesisDoc)
 	if err != nil {
-		return false, nil, fmt.Errorf("error loading blockchain state from database: %v", err)
+		return nil, false, fmt.Errorf("error loading blockchain state from database: %v", err)
 	}
 	if bc != nil {
-		dbHash := bc.genesisHash
+		dbHash := bc.GenesisHash()
 		argHash := genesisDoc.Hash()
 		if !bytes.Equal(dbHash, argHash) {
-			return false, nil, fmt.Errorf("GenesisDoc passed to LoadOrNewBlockchain has hash: 0x%X, which does not "+
+			return nil, false, fmt.Errorf("GenesisDoc passed to LoadOrNewBlockchain has hash: 0x%X, which does not "+
 				"match the one found in database: 0x%X, database genesis:\n%v\npassed genesis:\n%v\n",
 				argHash, dbHash, bc.genesisDoc.JSONString(), genesisDoc.JSONString())
 		}
-		return true, bc, nil
+		if bc.LastBlockTime().Before(genesisDoc.GenesisTime) {
+			return nil, false, fmt.Errorf("LastBlockTime %v from loaded Blockchain is before GenesisTime %v",
+				bc.LastBlockTime(), genesisDoc.GenesisTime)
+		}
+		return bc, true, nil
 	}
 
 	logger.InfoMsg("No existing blockchain state found in database, making new blockchain")
-	return false, NewBlockchain(db, genesisDoc), nil
+	return NewBlockchain(db, genesisDoc), false, nil
 }
 
 // NewBlockchain returns a pointer to blockchain state initialised from genesis
 func NewBlockchain(db dbm.DB, genesisDoc *genesis.GenesisDoc) *Blockchain {
 	bc := &Blockchain{
-		db:                    db,
-		genesisHash:           genesisDoc.Hash(),
-		genesisDoc:            *genesisDoc,
-		chainID:               genesisDoc.ChainID(),
-		lastBlockTime:         genesisDoc.GenesisTime,
-		appHashAfterLastBlock: genesisDoc.Hash(),
+		db: db,
+		persistedState: PersistedState{
+			AppHashAfterLastBlock: genesisDoc.Hash(),
+			GenesisHash:           genesisDoc.Hash(),
+			LastBlockTime:         genesisDoc.GenesisTime,
+		},
+		genesisDoc: *genesisDoc,
 	}
 	return bc
 }
@@ -131,7 +134,7 @@ func loadBlockchain(db dbm.DB, genesisDoc *genesis.GenesisDoc) (*Blockchain, err
 }
 
 func (bc *Blockchain) CommitBlock(blockTime time.Time, blockHash, appHash []byte) error {
-	return bc.CommitBlockAtHeight(blockTime, blockHash, appHash, bc.lastBlockHeight+1)
+	return bc.CommitBlockAtHeight(blockTime, blockHash, appHash, bc.persistedState.LastBlockHeight+1)
 }
 
 func (bc *Blockchain) CommitBlockAtHeight(blockTime time.Time, blockHash, appHash []byte, height uint64) error {
@@ -144,17 +147,17 @@ func (bc *Blockchain) CommitBlockAtHeight(blockTime time.Time, blockHash, appHas
 	if err != nil {
 		return err
 	}
-	bc.lastCommitDuration = blockTime.Sub(bc.lastBlockTime)
-	bc.lastBlockHeight = height
-	bc.lastBlockTime = blockTime
+	bc.lastCommitDuration = blockTime.Sub(bc.persistedState.LastBlockTime)
 	bc.lastBlockHash = blockHash
-	bc.appHashAfterLastBlock = appHash
+	bc.persistedState.LastBlockHeight = height
+	bc.persistedState.LastBlockTime = blockTime
+	bc.persistedState.AppHashAfterLastBlock = appHash
 	bc.lastCommitTime = time.Now().UTC()
 	return nil
 }
 
 func (bc *Blockchain) CommitWithAppHash(appHash []byte) error {
-	bc.appHashAfterLastBlock = appHash
+	bc.persistedState.AppHashAfterLastBlock = appHash
 	bc.Lock()
 	defer bc.Unlock()
 
@@ -175,43 +178,32 @@ func (bc *Blockchain) save() error {
 var cdc = amino.NewCodec()
 
 func (bc *Blockchain) Encode() ([]byte, error) {
-	persistedState := &PersistedState{
-		GenesisHash:           bc.genesisDoc.Hash(),
-		AppHashAfterLastBlock: bc.appHashAfterLastBlock,
-		LastBlockHeight:       bc.lastBlockHeight,
-	}
-	encodedState, err := cdc.MarshalBinaryBare(persistedState)
+	encodedState, err := cdc.MarshalBinaryBare(bc.persistedState)
 	if err != nil {
 		return nil, err
 	}
 	return encodedState, nil
 }
 
 func decodeBlockchain(encodedState []byte, genesisDoc *genesis.GenesisDoc) (*Blockchain, error) {
-	persistedState := new(PersistedState)
-	err := cdc.UnmarshalBinaryBare(encodedState, persistedState)
+	bc := NewBlockchain(nil, genesisDoc)
+	err := cdc.UnmarshalBinaryBare(encodedState, &bc.persistedState)
 	if err != nil {
 		return nil, err
 	}
-
-	bc := NewBlockchain(nil, genesisDoc)
-	bc.genesisHash = persistedState.GenesisHash
-	//bc.lastBlockHeight = persistedState.LastBlockHeight
-	bc.lastBlockHeight = persistedState.LastBlockHeight
-	bc.appHashAfterLastBlock = persistedState.AppHashAfterLastBlock
 	return bc, nil
 }
 
 func (bc *Blockchain) GenesisHash() []byte {
-	return bc.genesisHash
+	return bc.persistedState.GenesisHash
 }
 
 func (bc *Blockchain) GenesisDoc() genesis.GenesisDoc {
 	return bc.genesisDoc
 }
 
 func (bc *Blockchain) ChainID() string {
-	return bc.chainID
+	return bc.genesisDoc.ChainID()
 }
 
 func (bc *Blockchain) LastBlockHeight() uint64 {
@@ -220,13 +212,13 @@ func (bc *Blockchain) LastBlockHeight() uint64 {
 	}
 	bc.RLock()
 	defer bc.RUnlock()
-	return bc.lastBlockHeight
+	return bc.persistedState.LastBlockHeight
 }
 
 func (bc *Blockchain) LastBlockTime() time.Time {
 	bc.RLock()
 	defer bc.RUnlock()
-	return bc.lastBlockTime
+	return bc.persistedState.LastBlockTime
 }
 
 func (bc *Blockchain) LastCommitTime() time.Time {
@@ -250,7 +242,7 @@ func (bc *Blockchain) LastBlockHash() []byte {
 func (bc *Blockchain) AppHashAfterLastBlock() []byte {
 	bc.RLock()
 	defer bc.RUnlock()
-	return bc.appHashAfterLastBlock
+	return bc.persistedState.AppHashAfterLastBlock
 }
 
 // Tendermint block access

bcm/blockchain_test.go

@@ -0,0 +1,82 @@
+package bcm
+
+import (
+	"testing"
+	"time"
+
+	"github.com/hyperledger/burrow/crypto/sha3"
+	"github.com/hyperledger/burrow/genesis"
+	"github.com/hyperledger/burrow/logging"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	dbm "github.com/tendermint/tendermint/libs/db"
+)
+
+func TestLoadOrNewBlockchain(t *testing.T) {
+	// Initial load - fresh chain
+	genesisDoc := newGenesisDoc()
+	db := dbm.NewMemDB()
+	blockchain, exists, err := LoadOrNewBlockchain(db, genesisDoc, logging.NewNoopLogger())
+	require.NoError(t, err)
+	assert.False(t, exists)
+	assert.Equal(t, genesisDoc.GenesisTime, blockchain.LastBlockTime())
+	assert.Equal(t, uint64(0), blockchain.LastBlockHeight())
+	assert.Equal(t, genesisDoc.Hash(), blockchain.AppHashAfterLastBlock())
+
+	// First block
+	blockTime1 := genesisDoc.GenesisTime.Add(time.Second * 10)
+	blockHash1 := sha3.Sha3([]byte("blockHash"))
+	appHash1 := sha3.Sha3([]byte("appHash"))
+	err = blockchain.CommitBlock(blockTime1, blockHash1, appHash1)
+	require.NoError(t, err)
+	assertState(t, blockchain, 1, blockTime1, appHash1)
+
+	// Second block
+	blockTime2a := blockTime1.Add(time.Second * 30)
+	blockHash2a := sha3.Sha3(append(blockHash1, 2))
+	appHash2a := sha3.Sha3(append(appHash1, 2))
+	err = blockchain.CommitBlock(blockTime2a, blockHash2a, appHash2a)
+	require.NoError(t, err)
+	assertState(t, blockchain, 2, blockTime2a, appHash2a)
+
+	// Load at checkpoint (i.e. first block)
+	blockchain, exists, err = LoadOrNewBlockchain(db, genesisDoc, logging.NewNoopLogger())
+	require.NoError(t, err)
+	assert.True(t, exists)
+	// Assert first block values
+	assertState(t, blockchain, 1, blockTime1, appHash1)
+
+	// Commit (overwriting previous block 2 pointer
+	blockTime2b := blockTime1.Add(time.Second * 30)
+	blockHash2b := sha3.Sha3(append(blockHash1, 2))
+	appHash2b := sha3.Sha3(append(appHash1, 2))
+	err = blockchain.CommitBlock(blockTime2b, blockHash2b, appHash2b)
+	require.NoError(t, err)
+	assertState(t, blockchain, 2, blockTime2b, appHash2b)
+
+	// Commit again to check things are okay
+	blockTime3 := blockTime2b.Add(time.Second * 30)
+	blockHash3 := sha3.Sha3(append(blockHash2b, 2))
+	appHash3 := sha3.Sha3(append(appHash2b, 2))
+	err = blockchain.CommitBlock(blockTime3, blockHash3, appHash3)
+	require.NoError(t, err)
+	assertState(t, blockchain, 3, blockTime3, appHash3)
+
+	// Load at checkpoint (i.e. block 2b)
+	blockchain, exists, err = LoadOrNewBlockchain(db, genesisDoc, logging.NewNoopLogger())
+	require.NoError(t, err)
+	assert.True(t, exists)
+	// Assert first block values
+	assertState(t, blockchain, 2, blockTime2b, appHash2b)
+}
+
+func assertState(t *testing.T, blockchain *Blockchain, height uint64, blockTime time.Time, appHash []byte) {
+	assert.Equal(t, height, blockchain.LastBlockHeight())
+	assert.Equal(t, blockTime, blockchain.LastBlockTime())
+	assert.Equal(t, appHash, blockchain.AppHashAfterLastBlock())
+}
+
+func newGenesisDoc() *genesis.GenesisDoc {
+	genesisDoc, _, _ := genesis.NewDeterministicGenesis(3450976).GenesisDoc(23, true, 4, 10, true, 4)
+	return genesisDoc
+}