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

Chain scoring #4218

Merged
merged 2 commits into from
Jan 23, 2017
Merged

Chain scoring #4218

merged 2 commits into from
Jan 23, 2017

Conversation

keorn
Copy link

@keorn keorn commented Jan 19, 2017

Not sure about renaming difficulty to weight yet since the overall usage is still Ethash centric.

@keorn keorn added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jan 19, 2017
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 380f5b6 on chain-score into ** on master**.

@@ -209,7 +207,9 @@ impl Engine for AuthorityRound {
}

fn populate_from_parent(&self, header: &mut Header, parent: &Header, gas_floor_target: U256, _gas_ceil_target: U256) {
header.set_difficulty(parent.difficulty().clone());
// Chain scoring: total weight is sqrt(U256::max_value())*height - step
let new_difficulty = U256::from(U128::max_value()) + header_step(parent).expect("Header has been verified; qed").into() - self.step.load(AtomicOrdering::SeqCst).into();
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like + height instead of * height

Copy link
Author

Choose a reason for hiding this comment

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

The formula above is for total weight, populating the block gives weight to an individual block. Summing blocks gives the total.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably use saturating_add to avoid overflows.

Copy link
Author

Choose a reason for hiding this comment

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

Consensus is broken if any of these overflow, so it does not matter.

Copy link
Author

Choose a reason for hiding this comment

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

But no need to worry, releasing billion blocks every second it will last us until the death of the galaxy many times anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, just noticed that it's basically a block number and it's u64 anyway :)

@@ -397,7 +395,9 @@ impl Engine for Tendermint {
}

fn populate_from_parent(&self, header: &mut Header, parent: &Header, gas_floor_target: U256, _gas_ceil_target: U256) {
header.set_difficulty(parent.difficulty().clone());
// Chain scoring: total weight is sqrt(U256::max_value())*height - round
let new_difficulty = U256::from(U128::max_value()) + consensus_round(parent).expect("Header has been verified; qed").into() - self.round.load(AtomicOrdering::SeqCst).into();
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Copy link
Author

Choose a reason for hiding this comment

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

as above

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably use saturating_add to avoid overflows.

@@ -867,7 +862,7 @@ impl BlockChain {
let number = header.number();
let parent_hash = header.parent_hash();
let parent_details = self.block_details(&parent_hash).unwrap_or_else(|| panic!("Invalid parent hash: {:?}", parent_hash));
let is_new_best = self.engine.is_new_best_block(self.best_block_total_difficulty(), self.best_block_header().view(), &parent_details, header);
let is_new_best = parent_details.total_difficulty + header.difficulty() > self.best_block_total_difficulty();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably use saturating_add to avoid overflows.

@@ -397,7 +395,9 @@ impl Engine for Tendermint {
}

fn populate_from_parent(&self, header: &mut Header, parent: &Header, gas_floor_target: U256, _gas_ceil_target: U256) {
header.set_difficulty(parent.difficulty().clone());
// Chain scoring: total weight is sqrt(U256::max_value())*height - round
let new_difficulty = U256::from(U128::max_value()) + consensus_round(parent).expect("Header has been verified; qed").into() - self.round.load(AtomicOrdering::SeqCst).into();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably use saturating_add to avoid overflows.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 20, 2017
@NikVolf NikVolf merged commit b7f9b30 into master Jan 23, 2017
@NikVolf NikVolf deleted the chain-score branch January 23, 2017 14:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants