-
Notifications
You must be signed in to change notification settings - Fork 650
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix #436 object_database created outside of witness data directory #687
Conversation
You mean #436 I suppose |
that makes much more sense, thanks @pmconrad . also, the .gitignore changes are unrelated to the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO you shouldn't introduce an "_opened" flag without
- specifying what it means that the database is "open", and
- consistently checking the flag wherever it makes sense.
For example, what happens if you create objects in a database that is not "open", and then call open(). Doesn't make sense, should be forbidden.
It seems that database::wipe is only ever called from application::startup. before database::open is called. A minimalist fix would be to simply remove the call to database::close() in database::wipe(). I haven't checked if close has any other relevant side effects though.
.gitignore
Outdated
libraries/egenesis/egenesis_brief.cpp | ||
libraries/egenesis/egenesis_full.cpp | ||
libraries/egenesis/embed_genesis | ||
tests/generate_empty_blocks/generate_empty_blocks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing .gitignore is unrelated, please undo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the changes are good. However, better to do it with another PR.
Thanks for the contribution anyway. According to the Git Flow, features that are not consensus-related need to be merged into |
sorry I deleted my former branch by mistake, I have to request another pr #689, bother moving to there... |
No description provided.