From fe382c6233335bcf3d80d2db9958e0ef9fa13deb Mon Sep 17 00:00:00 2001 From: Bart Wyatt Date: Wed, 17 Oct 2018 10:57:54 -0400 Subject: [PATCH 1/2] integrity hashes were not using the determinisitic snapshot row types which meant internal implementation differences were invalidating it for some types of rows --- libraries/chain/authorization_manager.cpp | 5 +++-- libraries/chain/controller.cpp | 10 ++++++---- libraries/chain/resource_limits.cpp | 5 +++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/libraries/chain/authorization_manager.cpp b/libraries/chain/authorization_manager.cpp index e03783c8f9d..2f0fd26496e 100644 --- a/libraries/chain/authorization_manager.cpp +++ b/libraries/chain/authorization_manager.cpp @@ -37,8 +37,9 @@ namespace eosio { namespace chain { void authorization_manager::calculate_integrity_hash( fc::sha256::encoder& enc ) const { authorization_index_set::walk_indices([this, &enc]( auto utils ){ - decltype(utils)::walk(_db, [&enc]( const auto &row ) { - fc::raw::pack(enc, row); + decltype(utils)::walk(_db, [this, &enc]( const auto &row ) { + using row_type = std::decay_t; + fc::raw::pack(enc, detail::snapshot_row_traits::to_snapshot_row(row, _db)); }); }); } diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index e03fae1a80f..2153236b37f 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -429,8 +429,9 @@ struct controller_impl { auto tid_key = boost::make_tuple(table_row.id); auto next_tid_key = boost::make_tuple(table_id_object::id_type(table_row.id._id + 1)); - decltype(utils)::template walk_range(db, tid_key, next_tid_key, [&enc](const auto& row){ - fc::raw::pack(enc, row); + decltype(utils)::template walk_range(db, tid_key, next_tid_key, [this, &enc](const auto& row){ + using row_type = std::decay_t; + fc::raw::pack(enc, detail::snapshot_row_traits::to_snapshot_row(row, db)); }); }); }); @@ -501,8 +502,9 @@ struct controller_impl { return; } - decltype(utils)::walk(db, [&enc]( const auto &row ) { - fc::raw::pack(enc, row); + decltype(utils)::walk(db, [this, &enc]( const auto &row ) { + using row_type = std::decay_t; + fc::raw::pack(enc, detail::snapshot_row_traits::to_snapshot_row(row, db)); }); }); diff --git a/libraries/chain/resource_limits.cpp b/libraries/chain/resource_limits.cpp index d090631b9c4..a84fcbc6888 100644 --- a/libraries/chain/resource_limits.cpp +++ b/libraries/chain/resource_limits.cpp @@ -67,8 +67,9 @@ void resource_limits_manager::initialize_database() { void resource_limits_manager::calculate_integrity_hash( fc::sha256::encoder& enc ) const { resource_index_set::walk_indices([this, &enc]( auto utils ){ - decltype(utils)::walk(_db, [&enc]( const auto &row ) { - fc::raw::pack(enc, row); + decltype(utils)::walk(_db, [this, &enc]( const auto &row ) { + using row_type = std::decay_t; + fc::raw::pack(enc, detail::snapshot_row_traits::to_snapshot_row(row, _db)); }); }); } From 21f0129bbe35bee096b914b08efe284e1b04de97 Mon Sep 17 00:00:00 2001 From: Bart Wyatt Date: Wed, 17 Oct 2018 12:02:09 -0400 Subject: [PATCH 2/2] route integrity hashes through snapshot system to prevent future desyncs and reduce code that requires maintenance --- libraries/chain/authorization_manager.cpp | 9 ---- libraries/chain/controller.cpp | 51 ++++--------------- .../eosio/chain/authorization_manager.hpp | 1 - .../include/eosio/chain/resource_limits.hpp | 1 - .../chain/include/eosio/chain/snapshot.hpp | 26 +++++++++- libraries/chain/resource_limits.cpp | 9 ---- libraries/chain/snapshot.cpp | 22 ++++++++ 7 files changed, 57 insertions(+), 62 deletions(-) diff --git a/libraries/chain/authorization_manager.cpp b/libraries/chain/authorization_manager.cpp index 2f0fd26496e..832f69c71cd 100644 --- a/libraries/chain/authorization_manager.cpp +++ b/libraries/chain/authorization_manager.cpp @@ -35,15 +35,6 @@ namespace eosio { namespace chain { _db.create([](auto&){}); /// reserve perm 0 (used else where) } - void authorization_manager::calculate_integrity_hash( fc::sha256::encoder& enc ) const { - authorization_index_set::walk_indices([this, &enc]( auto utils ){ - decltype(utils)::walk(_db, [this, &enc]( const auto &row ) { - using row_type = std::decay_t; - fc::raw::pack(enc, detail::snapshot_row_traits::to_snapshot_row(row, _db)); - }); - }); - } - namespace detail { template<> struct snapshot_row_traits { diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 2153236b37f..9a6c4d0f958 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -419,24 +419,6 @@ struct controller_impl { }); } - void calculate_contract_tables_integrity_hash( sha256::encoder& enc ) const { - index_utils::walk(db, [this, &enc]( const table_id_object& table_row ){ - fc::raw::pack(enc, table_row); - - contract_database_index_set::walk_indices([this, &enc, &table_row]( auto utils ) { - using value_t = typename decltype(utils)::index_t::value_type; - using by_table_id = object_to_table_id_tag_t; - - auto tid_key = boost::make_tuple(table_row.id); - auto next_tid_key = boost::make_tuple(table_id_object::id_type(table_row.id._id + 1)); - decltype(utils)::template walk_range(db, tid_key, next_tid_key, [this, &enc](const auto& row){ - using row_type = std::decay_t; - fc::raw::pack(enc, detail::snapshot_row_traits::to_snapshot_row(row, db)); - }); - }); - }); - } - void add_contract_tables_to_snapshot( const snapshot_writer_ptr& snapshot ) const { snapshot->write_section("contract_tables", [this]( auto& section ) { index_utils::walk(db, [this, §ion]( const table_id_object& table_row ){ @@ -492,29 +474,6 @@ struct controller_impl { }); } - sha256 calculate_integrity_hash() const { - sha256::encoder enc; - controller_index_set::walk_indices([this, &enc]( auto utils ){ - using value_t = typename decltype(utils)::index_t::value_type; - - // skip the table_id_object as its inlined with contract tables section - if (std::is_same::value) { - return; - } - - decltype(utils)::walk(db, [this, &enc]( const auto &row ) { - using row_type = std::decay_t; - fc::raw::pack(enc, detail::snapshot_row_traits::to_snapshot_row(row, db)); - }); - }); - - calculate_contract_tables_integrity_hash(enc); - - authorization.calculate_integrity_hash(enc); - resource_limits.calculate_integrity_hash(enc); - return enc.result(); - } - void add_to_snapshot( const snapshot_writer_ptr& snapshot ) const { snapshot->write_section([this]( auto §ion ){ section.add_row(chain_snapshot_header(), db); @@ -595,6 +554,16 @@ struct controller_impl { db.set_revision( head->block_num ); } + sha256 calculate_integrity_hash() const { + sha256::encoder enc; + auto hash_writer = std::make_shared(enc); + add_to_snapshot(hash_writer); + hash_writer->finalize(); + + return enc.result(); + } + + /** * Sets fork database head to the genesis state. */ diff --git a/libraries/chain/include/eosio/chain/authorization_manager.hpp b/libraries/chain/include/eosio/chain/authorization_manager.hpp index 52f211de374..9a75b5f80b1 100644 --- a/libraries/chain/include/eosio/chain/authorization_manager.hpp +++ b/libraries/chain/include/eosio/chain/authorization_manager.hpp @@ -28,7 +28,6 @@ namespace eosio { namespace chain { void add_indices(); void initialize_database(); - void calculate_integrity_hash( fc::sha256::encoder& enc ) const; void add_to_snapshot( const snapshot_writer_ptr& snapshot ) const; void read_from_snapshot( const snapshot_reader_ptr& snapshot ); diff --git a/libraries/chain/include/eosio/chain/resource_limits.hpp b/libraries/chain/include/eosio/chain/resource_limits.hpp index b7fb7b24c29..4b0c58beeb0 100644 --- a/libraries/chain/include/eosio/chain/resource_limits.hpp +++ b/libraries/chain/include/eosio/chain/resource_limits.hpp @@ -44,7 +44,6 @@ namespace eosio { namespace chain { namespace resource_limits { void add_indices(); void initialize_database(); - void calculate_integrity_hash( fc::sha256::encoder& enc ) const; void add_to_snapshot( const snapshot_writer_ptr& snapshot ) const; void read_from_snapshot( const snapshot_reader_ptr& snapshot ); diff --git a/libraries/chain/include/eosio/chain/snapshot.hpp b/libraries/chain/include/eosio/chain/snapshot.hpp index b6c7a81bf0a..47f69dd07e2 100644 --- a/libraries/chain/include/eosio/chain/snapshot.hpp +++ b/libraries/chain/include/eosio/chain/snapshot.hpp @@ -74,6 +74,7 @@ namespace eosio { namespace chain { struct abstract_snapshot_row_writer { virtual void write(ostream_wrapper& out) const = 0; + virtual void write(fc::sha256::encoder& out) const = 0; virtual variant to_variant() const = 0; virtual std::string row_type_name() const = 0; }; @@ -83,10 +84,19 @@ namespace eosio { namespace chain { explicit snapshot_row_writer( const T& data ) :data(data) {} - void write(ostream_wrapper& out) const override { + template + void write_stream(DataStream& out) const { fc::raw::pack(out, data); } + void write(ostream_wrapper& out) const override { + write_stream(out); + } + + void write(fc::sha256::encoder& out) const override { + write_stream(out); + } + fc::variant to_variant() const override { variant var; fc::to_variant(data, var); @@ -356,4 +366,18 @@ namespace eosio { namespace chain { uint64_t cur_row; }; + class integrity_hash_snapshot_writer : public snapshot_writer { + public: + explicit integrity_hash_snapshot_writer(fc::sha256::encoder& enc); + + void write_start_section( const std::string& section_name ) override; + void write_row( const detail::abstract_snapshot_row_writer& row_writer ) override; + void write_end_section( ) override; + void finalize(); + + private: + fc::sha256::encoder& enc; + + }; + }} diff --git a/libraries/chain/resource_limits.cpp b/libraries/chain/resource_limits.cpp index a84fcbc6888..fa38f76a1e2 100644 --- a/libraries/chain/resource_limits.cpp +++ b/libraries/chain/resource_limits.cpp @@ -65,15 +65,6 @@ void resource_limits_manager::initialize_database() { }); } -void resource_limits_manager::calculate_integrity_hash( fc::sha256::encoder& enc ) const { - resource_index_set::walk_indices([this, &enc]( auto utils ){ - decltype(utils)::walk(_db, [this, &enc]( const auto &row ) { - using row_type = std::decay_t; - fc::raw::pack(enc, detail::snapshot_row_traits::to_snapshot_row(row, _db)); - }); - }); -} - void resource_limits_manager::add_to_snapshot( const snapshot_writer_ptr& snapshot ) const { resource_index_set::walk_indices([this, &snapshot]( auto utils ){ snapshot->write_section([this]( auto& section ){ diff --git a/libraries/chain/snapshot.cpp b/libraries/chain/snapshot.cpp index fb119b40a3a..e226a4f4886 100644 --- a/libraries/chain/snapshot.cpp +++ b/libraries/chain/snapshot.cpp @@ -336,4 +336,26 @@ void istream_snapshot_reader::clear_section() { cur_row = 0; } +integrity_hash_snapshot_writer::integrity_hash_snapshot_writer(fc::sha256::encoder& enc) +:enc(enc) +{ +} + +void integrity_hash_snapshot_writer::write_start_section( const std::string& ) +{ + // no-op for structural details +} + +void integrity_hash_snapshot_writer::write_row( const detail::abstract_snapshot_row_writer& row_writer ) { + row_writer.write(enc); +} + +void integrity_hash_snapshot_writer::write_end_section( ) { + // no-op for structural details +} + +void integrity_hash_snapshot_writer::finalize() { + // no-op for structural details +} + }} \ No newline at end of file