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

extend debug_dump checks #928

Merged
merged 4 commits into from
May 17, 2018
Merged

Conversation

oxarbitrage
Copy link
Member

for issue #136

@pmconrad
Copy link
Contributor

Question: does it make sense that this is called at the end of init_genesis?

  • We're not going to change genesis anytime soon.
  • Running that check only increases startup time.
  • For the unit tests, database_fixture::verify_asset_supplies is much more relevant.
  • The name is misleading, it doesn't actually dump anything.
  • Logging an error is is not helpful because nobody really looks at the logs. I agree with the OP that this should be considered a fatal error.

Apparently it was intended that this function can be used in different places, e. g. at https://github.com/bitshares/bitshares-core/blob/master/libraries/chain/db_market.cpp#L47 . Adding genesis as a parameter would defeat that purpose.

@oxarbitrage
Copy link
Member Author

oxarbitrage commented May 16, 2018

@pmconrad i think you are right, was not seeing the full picture.

in this commit: a0f7560

  • i removed the genesis parameter from the call.
  • removed genesis balance checks.
  • i decided to keep the vesting balance and settle checks.
  • commented call in db_init.cpp
  • marked as fatal with exit(0), not sure if it is the best.

if( total_balances[asset_id_type()].value != core_asset_data.current_supply.value )
{
edump( (total_balances[asset_id_type()].value)(core_asset_data.current_supply.value ));
exit(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't call exit(). Better throw. (And if you do call exit, use a non-zero exit value to indicate an error. But don't call it.)

Copy link
Member Author

Choose a reason for hiding this comment

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

changed it, thanks.

@@ -96,7 +96,7 @@ void database::debug_dump()
if( total_balances[asset_id_type()].value != core_asset_data.current_supply.value )
{
edump( (total_balances[asset_id_type()].value)(core_asset_data.current_supply.value ));
exit(0);
throw;
Copy link
Contributor

Choose a reason for hiding this comment

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

Uh, at least use FC_THROW with an explanatory message. Perhaps I should have been more explicit. :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry, changing now :)

@oxarbitrage oxarbitrage merged commit d6f9db5 into bitshares:develop May 17, 2018
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