Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Insert PROOF messages for some cases in blockchain #9141

Merged
merged 5 commits into from
Jul 25, 2018
Merged

Insert PROOF messages for some cases in blockchain #9141

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6413019
Insert PROOF messages for some cases in blockchain
sorpaas Jul 17, 2018
5a63bbb
Break expect to its own line to avoid things being too long
sorpaas Jul 17, 2018
b36efe2
Be more specific for all low-level database error cases
sorpaas Jul 17, 2018
7be9f8a
Fix BranchBecomingCanonChain expect
sorpaas Jul 17, 2018
0c82fbb
ethcore: fix typo in expect proof message
andresilva Jul 25, 2018
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
25 changes: 18 additions & 7 deletions ethcore/src/blockchain/blockchain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,9 @@ impl BlockChain {
};

// load best block
let best_block_hash = match bc.db.key_value().get(db::COL_EXTRA, b"best").unwrap() {
let best_block_hash = match bc.db.key_value().get(db::COL_EXTRA, b"best")
.expect("Low level database error. Some issue with disk?")
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could mention that we're trying to get the best block?
"Low-level database error when fetching 'best' block." ?

{
Some(best) => {
H256::from_slice(&best)
}
Expand Down Expand Up @@ -571,8 +573,11 @@ impl BlockChain {

{
// Fetch best block details
let best_block_total_difficulty = bc.block_details(&best_block_hash).unwrap().total_difficulty;
let best_block_rlp = bc.block(&best_block_hash).unwrap();
let best_block_total_difficulty = bc.block_details(&best_block_hash)
.expect("Best block is from a known block hash; a known block hash always comes with a known block detail; qed")
.total_difficulty;
let best_block_rlp = bc.block(&best_block_hash)
.expect("Best block is from a known block hash; qed");

// and write them
let mut best_block = bc.best_block.write();
Expand All @@ -586,8 +591,12 @@ impl BlockChain {
{
let best_block_number = bc.best_block.read().header.number();
// Fetch first and best ancient block details
let raw_first = bc.db.key_value().get(db::COL_EXTRA, b"first").unwrap().map(|v| v.into_vec());
let mut best_ancient = bc.db.key_value().get(db::COL_EXTRA, b"ancient").unwrap().map(|h| H256::from_slice(&h));
let raw_first = bc.db.key_value().get(db::COL_EXTRA, b"first")
.expect("Low level database error. Some issue with disk?")
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, we can mention that it's the "first" block and the "best ancient" block below.

.map(|v| v.into_vec());
let mut best_ancient = bc.db.key_value().get(db::COL_EXTRA, b"ancient")
.expect("Low level database error. Some issue with disk?")
.map(|h| H256::from_slice(&h));
let best_ancient_number;
if best_ancient.is_none() && best_block_number > 1 && bc.block_hash(1).is_none() {
best_ancient = Some(bc.genesis_hash());
Expand Down Expand Up @@ -1350,11 +1359,13 @@ impl BlockChain {
}
},
BlockLocation::BranchBecomingCanonChain(ref data) => {
let ancestor_number = self.block_number(&data.ancestor).unwrap();
let ancestor_number = self.block_number(&data.ancestor)
.expect("Inserted block's ancestor number always exists; qed");
Copy link
Contributor

Choose a reason for hiding this comment

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

"hash belongs to an ancestor of an inserted block; ancestors of an inserted block are always available; block number of an inserted block is always available; qed"

Actually while writing this I realised that this could fail because the second condition doesn't always hold true (because of warp sync) right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah looks like that's indeed. This function is called by insert_unordered_block which can get unordered?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems that this is fine, although all the logic is not really apparent (so we may consider to do some refactorings for this): In wrap sync, the location is always set to BlockLocation::CanonChain so we will never reach this match statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for looking this up. Yes, it's not clear from just looking at this code that invariant holds. But it makes sense that we do consider the warped block to be canon (what else can we do if we already trusted the warp?).

let start_number = ancestor_number + 1;

let mut blooms: Vec<Bloom> = data.enacted.iter()
.map(|hash| self.block_header_data(hash).unwrap())
.map(|hash| self.block_header_data(hash)
.expect("Inserted block's header data always exists; qed"))
Copy link
Contributor

Choose a reason for hiding this comment

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

"hash belongs to an inserted block; block header data of an inserted block is always available; qed"

.map(|h| h.log_bloom())
.collect();

Expand Down