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.

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

Add fee_payer to ALL operations in get_impacted_account_visitor #776

Closed
abitmore opened this issue Mar 23, 2018 · 3 comments
Closed

Add fee_payer to ALL operations in get_impacted_account_visitor #776

abitmore opened this issue Mar 23, 2018 · 3 comments

Comments

@abitmore
Copy link
Member

notify_changed_objects() calls get_relevant_accounts() which calls transaction_get_impacted_accounts() or operation_get_impacted_accounts() for some objects:

  • proposal_object_type (1.10.x)
  • operation_history_object_type (1.11.x)
  • impl_transaction_object_type (2.7.x)

Since fee_payer is not added to some operations in get_impacted_account_visitor in db_nofity.cpp, clients won't get notified when changes happened on those objects.

// TODO:  Review all of these, especially no-ops
struct get_impacted_account_visitor
...
   void operator()( const transfer_operation& op )
   {
      _impacted.insert( op.to );
   }

Other related code:

      operation_get_required_authorities( op.op, impacted, impacted, other ); // fee_payer is added here

      if( op.op.which() == operation::tag< account_create_operation >::value )
         impacted.insert( op.result.get<object_id_type>() );
      else
         graphene::app::operation_get_impacted_accounts( op.op, impacted );

      for( auto& a : other )
         for( auto& item : a.account_auths )
            impacted.insert( item.first );
   template<typename T>
   void operator()( const T& v )const 
   { 
      active.insert( v.fee_payer() );
// TODO:  Review all of these, especially no-ops
struct get_impacted_account_visitor
...
   void operator()( const transfer_operation& op )
   {
      _impacted.insert( op.to );
   }

Thanks to @xiangxn for reporting.

@abitmore abitmore added the bug label Mar 23, 2018
@abitmore abitmore added this to the Next Non-Consensus-Changing Release milestone Mar 24, 2018
@oxarbitrage
Copy link
Member

Related code also in elasticsearch plugin that uses the same code as account_history plugin:
https://github.com/bitshares/bitshares-core/blob/master/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp#L122-L131

@abitmore
Copy link
Member Author

Moved this issue to consensus-changing project/milestone to avoid missing changes for new operations.

@abitmore
Copy link
Member Author

Done with #955.

201806 Protocol Upgrade Release (Hard Fork) automation moved this from In progress to Done May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

2 participants