From a0765e2cf2efcdfece249aa104c869790696078c Mon Sep 17 00:00:00 2001 From: Daniel Larimer Date: Wed, 24 Jun 2015 16:38:56 -0400 Subject: [PATCH] Removing unnecessary indexing from account history plugin, it can now focus on just tracking operation history --- .../account_history_plugin.cpp | 324 +----------------- .../account_history_plugin.hpp | 28 -- tests/common/database_fixture.cpp | 3 + 3 files changed, 4 insertions(+), 351 deletions(-) diff --git a/libraries/plugins/account_history/account_history_plugin.cpp b/libraries/plugins/account_history/account_history_plugin.cpp index f632d8ae..4f5e763b 100644 --- a/libraries/plugins/account_history/account_history_plugin.cpp +++ b/libraries/plugins/account_history/account_history_plugin.cpp @@ -34,69 +34,15 @@ namespace graphene { namespace account_history { namespace detail { -class account_create_observer : public graphene::chain::evaluation_observer -{ - public: - account_create_observer( account_history_plugin& plugin ) - : _plugin( plugin ) {} - virtual ~account_create_observer(); - - virtual void post_evaluate( - const transaction_evaluation_state& eval_state, - const operation& op, - bool apply, - generic_evaluator* ge, - const operation_result& result ) override; - - account_history_plugin& _plugin; -}; - -class account_update_observer : public graphene::chain::evaluation_observer -{ - public: - account_update_observer( account_history_plugin& plugin ) - : _plugin( plugin ) - { - _pre_account_keys.reserve( GRAPHENE_DEFAULT_MAX_AUTHORITY_MEMBERSHIP * 2 + 2 ); - } - virtual ~account_update_observer(); - - virtual void pre_evaluate( - const transaction_evaluation_state& eval_state, - const operation& op, - bool apply, - generic_evaluator* ge ) override; - - virtual void post_evaluate( - const transaction_evaluation_state& eval_state, - const operation& op, - bool apply, - generic_evaluator* ge, - const operation_result& result ) override; - - virtual void evaluation_failed( - const transaction_evaluation_state& eval_state, - const operation& op, - bool apply, - generic_evaluator* ge, - const operation_result& result ) override; - - account_history_plugin& _plugin; - flat_set< key_id_type > _pre_account_keys; -}; class account_history_plugin_impl { public: account_history_plugin_impl(account_history_plugin& _plugin) - : _self( _plugin ), - _create_observer( _plugin ), - _update_observer( _plugin ) + : _self( _plugin ) { } virtual ~account_history_plugin_impl(); - void rebuild_key_account_index(); - flat_set get_keys_for_account( const account_id_type& account_id ); @@ -104,7 +50,6 @@ class account_history_plugin_impl * and will process/index all operations that were applied in the block. */ void update_account_histories( const signed_block& b ); - void index_account_keys( const account_id_type& account_id ); graphene::chain::database& database() { @@ -112,8 +57,6 @@ class account_history_plugin_impl } account_history_plugin& _self; - account_create_observer _create_observer; - account_update_observer _update_observer; flat_set _tracked_accounts; }; @@ -248,271 +191,13 @@ struct operation_get_impacted_accounts {} }; -account_create_observer::~account_create_observer() -{ - return; -} - -void account_create_observer::post_evaluate( - const transaction_evaluation_state& eval_state, - const operation& op, - bool apply, - generic_evaluator* ge, - const operation_result& result - ) -{ - assert( op.which() == operation::tag::value ); - - if( !apply ) - return; - - // if we only care about given accounts, then key -> account mapping - // is not maintained - if( _plugin.my->_tracked_accounts.size() > 0 ) - return; - - account_id_type account_id = result.get< object_id_type >(); - _plugin.my->index_account_keys( account_id ); - return; -} - -account_update_observer::~account_update_observer() -{ - return; -} - -void account_update_observer::pre_evaluate( - const transaction_evaluation_state& eval_state, - const operation& op, - bool apply, - generic_evaluator* ge - ) -{ - assert( op.which() == operation::tag::value ); - - if( !apply ) - return; - - // if we only care about given accounts, then key -> account mapping - // is not maintained - if( _plugin.my->_tracked_accounts.size() > 0 ) - return; - - // is this a tx which affects a key? - // switch( op.which() ) - // { - // see note in configure() why account_create_operation handling is unnecessary - // case operation::tag::value: - // const account_create_operation& create_op = op.get< account_create_operation >(); - // break; - // case operation::tag::value: - // const account_update_operation& update_op = op.get< account_update_operation >(); - // _pre_account_keys.clear(); - // get_updatable_account_keys( update_op, _pre_account_keys ); - // break; - // default: - // FC_ASSERT( false, "account_update_observer got unexpected operation type" ); - //} - - const account_update_operation& update_op = op.get< account_update_operation >(); - _pre_account_keys = _plugin.my->get_keys_for_account( update_op.account ); - return; -} - -void account_update_observer::post_evaluate( - const transaction_evaluation_state& eval_state, - const operation& op, - bool apply, - generic_evaluator* ge, - const operation_result& result - ) -{ - assert( op.which() == operation::tag::value ); - - if( !apply ) - return; - - graphene::chain::database& db = _plugin.my->database(); - - // if we only care about given accounts, then key -> account mapping - // is not maintained - if( _plugin.my->_tracked_accounts.size() > 0 ) - return; - - // wild back-of-envelope heuristic: most account update ops - // don't add more than two keys - flat_set post_account_keys; - post_account_keys.reserve( _pre_account_keys.size() + 2 ); - - vector removed_account_keys; - removed_account_keys.reserve( _pre_account_keys.size() ); - - const account_update_operation& update_op = op.get< account_update_operation >(); - post_account_keys = _plugin.my->get_keys_for_account( update_op.account ); - - std::set_difference( - _pre_account_keys.begin(), _pre_account_keys.end(), - post_account_keys.begin(), post_account_keys.end(), - std::back_inserter( removed_account_keys ) - ); - - // - // If a key_id is in _pre_account_keys but not in post_account_keys, - // then it is placed in removed_account_keys by set_difference(). - // - // Note, the *address* corresponding to this key may still exist - // in the account, because it may be aliased to multiple key_id's. - // - // We delete the key_account_object for all removed_account_keys. - // This correctly deletes addresses which were removed - // from the account by the update_op. - // - // Unfortunately, in the case of aliased keys, it deletes - // key_account_object if *any* of the aliases was removed from - // the account. We want it to delete only if *all* of the aliases - // were removed from the account. - // - // However, we need to run index_account_keys() afterwards anyway. - // It will re-add to the index any addresses which had been - // deleted -- but only if they still exist in the account under - // at least one alias. - // - // This is precisely the correct behavior. - // - - for( const key_id_type& key_id : removed_account_keys ) - { - auto& index = db.get_index_type().indices().get(); - auto it = index.find( key_id(db).key_address() ); - assert( it != index.end() ); - - db.modify( *it, [&]( key_account_object& ka ) - { - ka.account_ids.erase( update_op.account ); - }); - } - - _plugin.my->index_account_keys( update_op.account ); - - return; -} - -void account_update_observer::evaluation_failed( - const transaction_evaluation_state& eval_state, - const operation& op, - bool apply, - generic_evaluator* ge, - const operation_result& result - ) -{ - if( !apply ) - return; - - // if we only care about given accounts, then key -> account mapping - // is not maintained - if( _plugin.my->_tracked_accounts.size() > 0 ) - return; - - // cleaning up here is not strictly necessary, but good "hygiene" - _pre_account_keys.clear(); - return; -} account_history_plugin_impl::~account_history_plugin_impl() { return; } -void account_history_plugin_impl::rebuild_key_account_index() -{ - // TODO: We should really delete the index before we re-create it. - // TODO: Building and sorting a vector of tuples is probably more efficient - const graphene::chain::database& db = database(); - vector< pair< account_id_type, address > > tuples_from_db; - const auto& primary_account_idx = db.get_index_type().indices().get(); - for( const account_object& acct : primary_account_idx ) - index_account_keys( acct.id ); - return; -} - -flat_set account_history_plugin_impl::get_keys_for_account( const account_id_type& account_id ) -{ - const graphene::chain::database& db = database(); - const account_object& acct = account_id(db); - - const flat_map& owner_auths = - acct.owner.auths; - const flat_map& active_auths = - acct.active.auths; - - flat_set key_id_set; - key_id_set.reserve(owner_auths.size() + active_auths.size() + 2); - - key_id_set.insert(acct.options.memo_key); - - // we don't use get_keys() here to avoid an intermediate copy operation - for( const pair& item : active_auths ) - { - if( item.first.type() == key_object_type ) - key_id_set.insert( item.first ); - } - - for( const pair& item : owner_auths ) - { - if( item.first.type() == key_object_type ) - key_id_set.insert( item.first ); - } - - return key_id_set; -} - -void account_history_plugin_impl::index_account_keys( const account_id_type& account_id ) -{ - // for each key in account authority... get address, modify(..) - graphene::chain::database& db = database(); - - flat_set key_id_set = get_keys_for_account( account_id ); - - flat_set
address_set; - - // - // we pass the addresses through another flat_set because the - // blockchain doesn't force de-duplication of addresses - // (multiple key_id's might refer to the same address) - // - address_set.reserve( key_id_set.size() ); - - for( const key_id_type& key_id : key_id_set ) - address_set.insert( key_id(db).key_address() ); - - // add mappings for the given account - for( const address& addr : address_set ) - { - auto& idx = db.get_index_type().indices().get(); - auto it = idx.find( addr ); - if( it == idx.end() ) - { - // if unknown, we need to create a new object - db.create( [&]( key_account_object& ka ) - { - ka.key = addr; - ka.account_ids.insert( account_id ); - }); - } - else - { - // if known, we need to add to existing object - db.modify( *it, - [&]( key_account_object& ka ) - { - ka.account_ids.insert( account_id ); - }); - } - } - - return; -} void account_history_plugin_impl::update_account_histories( const signed_block& b ) { @@ -555,8 +240,6 @@ void account_history_plugin_impl::update_account_histories( const signed_block& { if( impacted.find( account_id ) != impacted.end() ) { - index_account_keys( account_id ); - // add history const auto& stats_obj = account_id(db).statistics(db); const auto& ath = db.create( [&]( account_transaction_history_object& obj ){ @@ -603,17 +286,12 @@ void account_history_plugin::plugin_initialize(const boost::program_options::var database().applied_block.connect( [&]( const signed_block& b){ my->update_account_histories(b); } ); database().add_index< primary_index< simple_index< operation_history_object > > >(); database().add_index< primary_index< simple_index< account_transaction_history_object > > >(); - database().add_index< primary_index< key_account_index >>(); - - database().register_evaluation_observer( my->_create_observer ); - database().register_evaluation_observer< graphene::chain::account_update_evaluator >( my->_update_observer ); LOAD_VALUE_SET(options, "tracked-accounts", my->_tracked_accounts, graphene::chain::account_id_type); } void account_history_plugin::plugin_startup() { - my->rebuild_key_account_index(); } flat_set account_history_plugin::tracked_accounts() const diff --git a/libraries/plugins/account_history/include/graphene/account_history/account_history_plugin.hpp b/libraries/plugins/account_history/include/graphene/account_history/account_history_plugin.hpp index fbaf44c7..3734a715 100644 --- a/libraries/plugins/account_history/include/graphene/account_history/account_history_plugin.hpp +++ b/libraries/plugins/account_history/include/graphene/account_history/account_history_plugin.hpp @@ -45,29 +45,6 @@ enum account_history_object_type bucket_object_type = 1 ///< used in market_history_plugin }; -class key_account_object : public abstract_object -{ - public: - static const uint8_t space_id = ACCOUNT_HISTORY_SPACE_ID; - static const uint8_t type_id = key_account_object_type; - - key_account_object() {} - key_account_object( const address& a ) : key(a) {} - - address key; - flat_set account_ids; -}; - -struct by_key{}; -typedef multi_index_container< - key_account_object, - indexed_by< - hashed_unique< tag, member< object, object_id_type, &object::id > >, - ordered_unique< tag, member< key_account_object, address, &key_account_object::key > > - > -> key_account_object_multi_index_type; - -typedef generic_index key_account_index; namespace detail { @@ -95,8 +72,3 @@ class account_history_plugin : public graphene::app::plugin } } //graphene::account_history -FC_REFLECT_DERIVED( graphene::account_history::key_account_object, - (graphene::db::object), - (key) - (account_ids) - ) diff --git a/tests/common/database_fixture.cpp b/tests/common/database_fixture.cpp index e34bae4e..43464b59 100644 --- a/tests/common/database_fixture.cpp +++ b/tests/common/database_fixture.cpp @@ -188,6 +188,7 @@ void database_fixture::verify_asset_supplies( )const void database_fixture::verify_account_history_plugin_index( )const { + return; if( skip_key_index_test ) return; @@ -219,6 +220,7 @@ void database_fixture::verify_account_history_plugin_index( )const tuples_from_db.emplace_back( account_id, addr ); } + /* vector< pair< account_id_type, address > > tuples_from_index; tuples_from_index.reserve( tuples_from_db.size() ); const auto& key_account_idx = @@ -263,6 +265,7 @@ void database_fixture::verify_account_history_plugin_index( )const bool account_history_plugin_index_ok = is_equal; BOOST_CHECK( account_history_plugin_index_ok ); + */ } return; }