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

Fixing marking blocks as bad & SyncMessage bugs + small client refactoring. #503

Merged
merged 9 commits into from Feb 26, 2016

Conversation

tomusdrw
Copy link
Collaborator

No description provided.

let bad_contains_parent = bad_blocks.contains(&header.parent_hash);

let mark_block_as_bad = || {
self.block_queue.write().unwrap().mark_as_bad(&header_hash);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is it really necessary to mark the blocks in queue in the loop? Could it be done like with mark_as_good(&[H256]) ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not necessary

NewChainBlock(Bytes), //TODO: use Cow
NewChainBlocks {
/// Hashes of blocks imported to blockchain
good: Vec<H256>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will be (probably) needed by transaction queue.

@tomusdrw tomusdrw changed the title Fixing mark_as_bad & SyncMessage bugs + small client refactoring. Fixing marking blocks as bad & SyncMessage bugs + small client refactoring. Feb 23, 2016
@tomusdrw tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Feb 23, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 24, 2016
@@ -285,18 +285,24 @@ impl BlockQueue {
}

/// Mark given block and all its children as bad. Stops verification.
pub fn mark_as_bad(&mut self, hash: &H256) {
pub fn mark_as_bad(&mut self, hashes: &[H256]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should not clear what hashes means in this context. consider renaming to bad_blocks or bad_hashes, if that's what is meant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, altough I renamed to block_hashes (because we already have bad in fn name), is it ok?

gavofyork pushed a commit that referenced this pull request Feb 26, 2016
Fixing marking blocks as bad & SyncMessage bugs + small client refactoring.
@gavofyork gavofyork merged commit e64293d into master Feb 26, 2016
@gavofyork gavofyork deleted the client_bugs branch February 26, 2016 20:10
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants