Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: ensure the canonical block is written before the canonical hash is set #2873

Merged
merged 1 commit into from
Aug 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,14 @@ func (self *BlockChain) WriteBlock(block *types.Block) (status WriteStatus, err
localTd := self.GetTd(self.currentBlock.Hash(), self.currentBlock.NumberU64())
externTd := new(big.Int).Add(block.Difficulty(), ptd)

// Irrelevant of the canonical status, write the block itself to the database
if err := self.hc.WriteTd(block.Hash(), block.NumberU64(), externTd); err != nil {
glog.Fatalf("failed to write block total difficulty: %v", err)
}
if err := WriteBlock(self.chainDb, block); err != nil {
glog.Fatalf("failed to write block contents: %v", err)
}

// If the total difficulty is higher than our known, add it to the canonical chain
// Second clause in the if statement reduces the vulnerability to selfish mining.
// Please refer to http://www.cs.cornell.edu/~ie53/publications/btcProcFC.pdf
Expand All @@ -788,19 +796,11 @@ func (self *BlockChain) WriteBlock(block *types.Block) (status WriteStatus, err
return NonStatTy, err
}
}
// Insert the block as the new head of the chain
self.insert(block)
self.insert(block) // Insert the block as the new head of the chain
status = CanonStatTy
} else {
status = SideStatTy
}
// Irrelevant of the canonical status, write the block itself to the database
if err := self.hc.WriteTd(block.Hash(), block.NumberU64(), externTd); err != nil {
glog.Fatalf("failed to write block total difficulty: %v", err)
}
if err := WriteBlock(self.chainDb, block); err != nil {
glog.Fatalf("failed to write block contents: %v", err)
}

self.futureBlocks.Remove(block.Hash())

Expand Down
38 changes: 38 additions & 0 deletions core/blockchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1090,3 +1090,41 @@ done:
}

}

// Tests if the canonical block can be fetched from the database during chain insertion.
func TestCanonicalBlockRetrieval(t *testing.T) {
var (
db, _ = ethdb.NewMemDatabase()
genesis = WriteGenesisBlockForTesting(db)
)

evmux := &event.TypeMux{}
blockchain, _ := NewBlockChain(db, testChainConfig(), FakePow{}, evmux)

chain, _ := GenerateChain(nil, genesis, db, 10, func(i int, gen *BlockGen) {})

for i, _ := range chain {
go func(block *types.Block) {
// try to retrieve a block by its canonical hash and see if the block data can be retrieved.
for {
ch := GetCanonicalHash(db, block.NumberU64())
if ch == (common.Hash{}) {
continue // busy wait for canonical hash to be written
}
if ch != block.Hash() {
t.Fatalf("unknown canonical hash, want %s, got %s", block.Hash().Hex(), ch.Hex())
}
fb := GetBlock(db, ch, block.NumberU64())
if fb == nil {
t.Fatalf("unable to retrieve block %d for canonical hash: %s", block.NumberU64(), ch.Hex())
}
if fb.Hash() != block.Hash() {
t.Fatalf("invalid block hash for block %d, want %s, got %s", block.NumberU64(), block.Hash().Hex(), fb.Hash().Hex())
}
return
}
}(chain[i])

blockchain.InsertChain(types.Blocks{chain[i]})
}
}
6 changes: 5 additions & 1 deletion core/database_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,11 @@ func GetTd(db ethdb.Database, hash common.Hash, number uint64) *big.Int {
}

// GetBlock retrieves an entire block corresponding to the hash, assembling it
// back from the stored header and body.
// back from the stored header and body. If either the header or body could not
// be retrieved nil is returned.
//
// Note, due to concurrent download of header and block body the header and thus
// canonical hash can be stored in the database but the body data not (yet).
func GetBlock(db ethdb.Database, hash common.Hash, number uint64) *types.Block {
// Retrieve the block header and body contents
header := GetHeader(db, hash, number)
Expand Down
18 changes: 11 additions & 7 deletions core/headerchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er
localTd := hc.GetTd(hc.currentHeaderHash, hc.currentHeader.Number.Uint64())
externTd := new(big.Int).Add(header.Difficulty, ptd)

// Irrelevant of the canonical status, write the td and header to the database
if err := hc.WriteTd(hash, number, externTd); err != nil {
glog.Fatalf("failed to write header total difficulty: %v", err)
}
if err := WriteHeader(hc.chainDb, header); err != nil {
glog.Fatalf("failed to write header contents: %v", err)
}

// If the total difficulty is higher than our known, add it to the canonical chain
// Second clause in the if statement reduces the vulnerability to selfish mining.
// Please refer to http://www.cs.cornell.edu/~ie53/publications/btcProcFC.pdf
Expand All @@ -176,26 +184,22 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er
headNumber = headHeader.Number.Uint64() - 1
headHeader = hc.GetHeader(headHash, headNumber)
}

// Extend the canonical chain with the new header
if err := WriteCanonicalHash(hc.chainDb, hash, number); err != nil {
glog.Fatalf("failed to insert header number: %v", err)
}
if err := WriteHeadHeaderHash(hc.chainDb, hash); err != nil {
glog.Fatalf("failed to insert head header hash: %v", err)
}

hc.currentHeaderHash, hc.currentHeader = hash, types.CopyHeader(header)

status = CanonStatTy
} else {
status = SideStatTy
}
// Irrelevant of the canonical status, write the header itself to the database
if err := hc.WriteTd(hash, number, externTd); err != nil {
glog.Fatalf("failed to write header total difficulty: %v", err)
}
if err := WriteHeader(hc.chainDb, header); err != nil {
glog.Fatalf("failed to write header contents: %v", err)
}

hc.headerCache.Add(hash, header)
hc.numberCache.Add(hash, number)

Expand Down
9 changes: 6 additions & 3 deletions eth/filters/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ func (self *Filter) SetTopics(topics [][]common.Hash) {
func (self *Filter) Find() vm.Logs {
latestHash := core.GetHeadBlockHash(self.db)
latestBlock := core.GetBlock(self.db, latestHash, core.GetBlockNumber(self.db, latestHash))
if latestBlock == nil {
return vm.Logs{}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this after the database write-order fixes? The panic is a nice way to catch the invariant and not let it slip by undetected.

var beginBlockNo uint64 = uint64(self.begin)
if self.begin == -1 {
beginBlockNo = latestBlock.NumberU64()
Expand Down Expand Up @@ -123,13 +126,13 @@ func (self *Filter) mipFind(start, end uint64, depth int) (logs vm.Logs) {
}

func (self *Filter) getLogs(start, end uint64) (logs vm.Logs) {
var block *types.Block

for i := start; i <= end; i++ {
var block *types.Block
hash := core.GetCanonicalHash(self.db, i)
if hash != (common.Hash{}) {
block = core.GetBlock(self.db, hash, i)
} else { // block not found
}
if block == nil { // block not found/written
return logs
}

Expand Down