Skip to content

Conversation

@theoreticalbts
Copy link
Contributor

PR for #1175

@theoreticalbts theoreticalbts requested a review from mvandeberg June 8, 2017 19:11
@theoreticalbts theoreticalbts force-pushed the 1175-refactor-branch-walk branch from 14868ad to 4ad2512 Compare June 8, 2017 19:23
witness_time_pairs.push_back( std::make_pair( b->data.witness, b->data.timestamp ) );
}

ilog( "Encountered a block num collision due to a fork. Walking the current fork to determine the correct block. block_num:${n}", ("n", height) );
Copy link
Contributor

Choose a reason for hiding this comment

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

We should combine these messages and make the logging accurate. Something like, "Encountered block num collision at block ${n} produced by witnesses ${w}" This also removes the statement that we are walking the fork, which we are not doing at this point in the code.

@theoreticalbts
Copy link
Contributor Author

Still not building. Will fix the build and address the above comment momentarily.

@theoreticalbts theoreticalbts force-pushed the 1175-refactor-branch-walk branch from 4ad2512 to 1f80f5f Compare June 8, 2017 21:01
return result;
}

void database::_maybe_warn_multiple_production( const database& db, uint32_t height )const
Copy link
Contributor

Choose a reason for hiding this comment

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

If this method is a member of database, why pass a const reference to itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, originally it wasn't, but then I saw _fork_db was private.

@theoreticalbts theoreticalbts force-pushed the 1175-refactor-branch-walk branch from 0422f28 to 33ecf12 Compare June 8, 2017 22:31
@mvandeberg mvandeberg merged commit 3fa962e into master Jun 9, 2017
@mvandeberg mvandeberg deleted the 1175-refactor-branch-walk branch June 9, 2017 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants