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.

Sign up for GitHub

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

[3.2] log integrity hash on start/stop with integrity-hash-on-start & integrity-hash-on-stop options #395

Merged
merged 4 commits into from
Jun 21, 2022
Merged

[3.2] log integrity hash on start/stop with integrity-hash-on-start & integrity-hash-on-stop options #395

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a70d6d5
log db hash on start/stop with integrity-hash-on-start & integrity-ha…
spoonincode Jun 15, 2022
3019dcd
just use a bool to track when it's acceptable to log exit hash
spoonincode Jun 17, 2022
0f9ac1b
missed staging a rename in last commit
spoonincode Jun 17, 2022
2d01629
move integrity-hash-on options from CLI to Cfg
spoonincode Jun 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions libraries/chain/controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,8 +569,6 @@ struct controller_impl {
"Snapshot is invalid." );
blog.reset( chain_id, lib_num + 1 );
}
const auto hash = calculate_integrity_hash();
ilog( "database initialized with hash: ${hash}", ("hash", hash) );

init(check_shutdown);
ilog( "Finished initialization from snapshot" );
Expand Down Expand Up @@ -696,6 +694,9 @@ struct controller_impl {
dm_logger->on_startup(db, head->block_num);
}

if( conf.integrity_hash_on_start )
ilog( "chain database started with hash: ${hash}", ("hash", calculate_integrity_hash()) );

replay( check_shutdown ); // replay any irreversible and reversible blocks ahead of current head

if( check_shutdown() ) return;
Expand Down Expand Up @@ -723,6 +724,9 @@ struct controller_impl {
~controller_impl() {
thread_pool.stop();
pending.reset();
//using the presence of a row in datebase_header index to indicate controller had a successful startup()
Copy link
Member Author

Choose a reason for hiding this comment

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

eh something better?

Copy link
Member

Choose a reason for hiding this comment

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

Move the chain_plugin where you know if started successfully?

Copy link
Member Author

@spoonincode spoonincode Jun 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I can't log the startup in chain_plugin because I need to to do the log before replay but after snapshot is loaded. So, it felt weird to do the startup log in controller but the shutdown log in chain_plugin.

I'm not sure doing it in chain_plugin really helps either since there are so many exceptions that can start unrolling the world at any time.

Actually I'm not really even sure this here works well enough: what if an exception is thrown half way through loading a snapshot. Then once everything unrolls the dtor is fired and if datebase_header made it then it does an integrity computation with a half initialized state? That's what I want to avoid.

But I also wanted to avoid adding a bool yep_controller_started_up. It's kinda wild there isn't a clear indicator anywhere 🤔

Copy link
Member

Choose a reason for hiding this comment

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

In chain_plugin::plugin_startup you could add a catch(...) after the database_guard_exception catch and add a my->chain.reset();. Then in chain_plugin::plugin_shutdown() you could check my->chain to know if it is valid, if so then log the integrity.

Copy link
Member Author

Choose a reason for hiding this comment

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

I capitulated and just added a bool 😕

if(db.get_index<database_header_multi_index>().indices().get<by_id>().size() && conf.integrity_hash_on_stop)
ilog( "chain database stopped with hash: ${hash}", ("hash", calculate_integrity_hash()) );
}

void add_indices() {
Expand Down
2 changes: 2 additions & 0 deletions libraries/chain/include/eosio/chain/controller.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ namespace eosio { namespace chain {
uint32_t maximum_variable_signature_length = chain::config::default_max_variable_signature_length;
bool disable_all_subjective_mitigations = false; //< for developer & testing purposes, can be configured using `disable-all-subjective-mitigations` when `EOSIO_DEVELOPER` build option is provided
uint32_t terminate_at_block = 0; //< primarily for testing purposes
bool integrity_hash_on_start= false;
bool integrity_hash_on_stop = false;

wasm_interface::vm_type wasm_runtime = chain::config::default_wasm_runtime;
eosvmoc::config eosvmoc_config;
Expand Down
5 changes: 5 additions & 0 deletions plugins/chain_plugin/chain_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,8 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip
("terminate-at-block", bpo::value<uint32_t>()->default_value(0),
"terminate after reaching this block number (if set to a non-zero number)")
("snapshot", bpo::value<bfs::path>(), "File to read Snapshot State from")
("integrity-hash-on-start", bpo::bool_switch(), "Log the state integrity hash on startup")
("integrity-hash-on-stop", bpo::bool_switch(), "Log the state integrity hash on shutdown")
heifner marked this conversation as resolved.
Show resolved Hide resolved
;

}
Expand Down Expand Up @@ -1079,6 +1081,9 @@ void chain_plugin::plugin_initialize(const variables_map& options) {

my->account_queries_enabled = options.at("enable-account-queries").as<bool>();

my->chain_config->integrity_hash_on_start = options.at("integrity-hash-on-start").as<bool>();
my->chain_config->integrity_hash_on_stop = options.at("integrity-hash-on-stop").as<bool>();

my->chain.emplace( *my->chain_config, std::move(pfs), *chain_id );

if( options.count( "transaction-retry-max-storage-size-gb" )) {
Expand Down