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 support for proposal transaction to database_api_impl::get_required_fees #434

Closed
jcalfee opened this issue Nov 4, 2015 · 8 comments
Closed
Assignees
Milestone

Comments

@jcalfee
Copy link

jcalfee commented Nov 4, 2015

I'm passing a full transaction with one operation: proposal_create

proposal_create has proposed_ops (array operation) .. It looks like it gets to this array and expects an object:

{"code":1,"message":"7 bad_cast_exception: Bad Cast\nInvalid cast from type 'array_type' to Object\n {\"type\":\"array_type\"}\n th_a variant.cpp:568 get_object","data":{"code":7,"name":"bad_cast_exception","message":"Bad Cast","stack":[{"context":{"level":"error","file":"variant.cpp","line":568,"method":"get_object","hostname":"","thread_name":"th_a","timestamp":"2015-11-05T16:15:22"},"format":"Invalid cast from type '${type}' to Object","data":{"type":"array_type"}}]}}

It would be reasonable to recurse into this array but simply append to the return vector<asset> .. The caller knows the object graph and can still place the fees correctly back into the transaction.

@jcalfee
Copy link
Author

jcalfee commented Nov 5, 2015

I have added a recursive call to go into a proposal_object and calculate the fees for its operations, I simply want to flatten all the fee objects and return one list...

I think there is only 1 issue: op (source is not a pointer)

vector<asset> database_api_impl::get_required_fees( const vector<operation>& ops, asset_id_type id )const
{
   vector<asset> result;
   // dynamic size (I hope) ?
   //result.reserve(ops.size()); 
   const asset_object&  a = id(_db);
   for( const auto& op : ops ) {
      const auto& pop = dynamic_cast<const proposal_object*>(op);//  op (source is not a pointer)
      if( pop != nullptr ) {
          result.push_back( _db.current_fee_schedule().calculate_fee( op, a.options.core_exchange_rate ) );
          for( const auto& asset : get_required_fees( pop.proposed_ops, id ) )
             result.push_back( asset ); // new, but this should work 
      } else {
          result.push_back( _db.current_fee_schedule().calculate_fee( op, a.options.core_exchange_rate ) );
      }
   }
   return result;
}

(edited)

jcalfee [9:08 AM]
original:

vector<asset> database_api_impl::get_required_fees( const vector<operation>& ops, asset_id_type id )const
{
vector<asset> result;
result.reserve(ops.size());
const asset_object& a = id(_db);
for( const auto& op : ops )
result.push_back( _db.current_fee_schedule().calculate_fee( op, a.options.core_exchange_rate ) );
return result;
}

@theoreticalbts
Copy link
Contributor

Proposal does not take a vector of operation, rather it takes a vector of op_wrapper, where op_wrapper is a class containing a single member called op of type operation.

@theoreticalbts
Copy link
Contributor

Merged to develop, awaiting review.

@jcalfee
Copy link
Author

jcalfee commented Nov 9, 2015

theoretical [4:29 PM]
note in particular how proposed_ops is different from what you have -- i have "proposed_ops":[{"op":[0,...]}] and you have "proposed_ops":[[0,...]]

jcalfee
Is it easy to remove the "op":? That's going to save the client from having to clone and recursively change the transaction.

I would like to look into this in the c++ side as soon as I have a chance. This is just the finishing touches. I'm sure your very busy with other stuff.

@jcalfee
Copy link
Author

jcalfee commented Nov 9, 2015

The above is not an issues, it is in the serializer format. I'll verify (review) after fixing this: #439

@jcalfee
Copy link
Author

jcalfee commented Nov 9, 2015

reviewed, it works

@theoreticalbts
Copy link
Contributor

Still needs merged.

@theoreticalbts theoreticalbts added this to the next milestone Nov 23, 2015
@theoreticalbts theoreticalbts modified the milestones: next2, next Jan 4, 2016
@theoreticalbts
Copy link
Contributor

Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants