Improve update_expired_feeds performance #1093 #1180
Conversation
The old bug is cryptonomex/graphene#615 . Due to the bug, `update_median_feeds()` and `check_call_orders()` will be called when a feed is not actually expired, normally this should not affect consensus since calling them should not change any data in the state. However, the logging indicates that `check_call_orders()` did change some data under certain circumstances, specifically, when multiple limit order matching issue (#453) occurred at same block. * #453
* changed an `assert()` to `FC_ASSERT()` * replaced one `db.get(asset_id_type())` with `db.get_core_asset()` * capture only required variables for lambda
libraries/chain/asset_evaluator.cpp
Outdated
| a.current_supply = 0; | ||
| a.fee_pool = core_fee_paid - (hf_429 ? 1 : 0); | ||
| }); | ||
| if( fee_is_odd && !hf_429 ) | ||
| { | ||
| const auto& core_dd = db().get<asset_object>( asset_id_type() ).dynamic_data( db() ); | ||
| db().modify( core_dd, [=]( asset_dynamic_data_object& dd ) { | ||
| const auto& core_dd = db().get_core_asset().dynamic_data( db() ); |
There was a problem hiding this comment.
Use db().get_core_dynamic_data() instead
libraries/chain/db_maint.cpp
Outdated
| const auto last_budget_time = get_dynamic_global_properties().last_budget_time; | ||
| const auto passed_time_ms = head_time - last_budget_time; | ||
| const bool passed_time_is_a_day = ( passed_time_ms == fc::days(1) ); | ||
| // the variable above is more likely false on BitShares mainnet, so do calculations below anyway |
There was a problem hiding this comment.
IMO can simplify the code and remove the passed_time_is_a_day check because it's unlikely to ever be true
| else | ||
| { | ||
| _p_core_asset_obj = &get( asset_id_type() ); | ||
| _p_core_dynamic_data_obj = &get( asset_dynamic_data_id_type() ); |
There was a problem hiding this comment.
There is no FC_ASSERT that ensures that _p_core_asset_obj->dynamic_asset_data_id == asset_dynamic_data_id_type(), better do a regular lookup on _p_core_asset_obj->dynamic_asset_data_id
There was a problem hiding this comment.
asset_dynamic_data_id_type() was used in account_object::process_fees():
bitshares-core/libraries/chain/account_object.cpp
Lines 77 to 79 in 68a0ffa
IMHO we need to add a FC_ASSERT. Update: added 4d40398
| _p_global_prop_obj = &get( global_property_id_type() ); | ||
| _p_chain_property_obj = &get( chain_property_id_type() ); | ||
| _p_dyn_global_prop_obj = &get( dynamic_global_property_id_type() ); | ||
| _p_witness_schedule_obj = &get( witness_schedule_id_type() ); |
There was a problem hiding this comment.
I get compilation errors on lines 172, 174, 175?!
invalid static_cast from type ‘const graphene::db::object’ to type ‘const graphene::chain::chain_property_object&’
invalid static_cast from type ‘const graphene::db::object’ to type ‘const graphene::chain::special_assets_meta_object&’
invalid static_cast from type ‘const graphene::db::object’ to type ‘const graphene::chain::witness_schedule_object&’
Apparently need to
#include <graphene/chain/chain_property_object.hpp>
#include <graphene/chain/witness_schedule_object.hpp>
#include <graphene/chain/special_authority_object.hpp>
There was a problem hiding this comment.
What's your compiler version? I don't know where to add these header includes. Perhaps you can submit a PR?
There was a problem hiding this comment.
gcc-5.3.1 - please add them at the top of db_management.cpp
|
Travis-CI reported:
Update: restarted Travis-CI job and it passed.
|
PR for #1093, rebased from #1113.