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

Asset update issuer operation #599

Merged
merged 15 commits into from
Mar 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions libraries/app/impacted.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ struct get_impacted_account_visitor
_impacted.insert( *(op.new_issuer) );
}

void operator()( const asset_update_issuer_operation& op ) {
_impacted.insert( op.new_issuer );
}

void operator()( const asset_update_bitasset_operation& op ) {}
void operator()( const asset_update_feed_producers_operation& op ) {}

Expand Down
70 changes: 56 additions & 14 deletions libraries/chain/asset_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,23 @@ void_result asset_fund_fee_pool_evaluator::do_apply(const asset_fund_fee_pool_op
return void_result();
} FC_CAPTURE_AND_RETHROW( (o) ) }

static void validate_new_issuer( const database& d, const asset_object& a, account_id_type new_issuer )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the static for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That avoids exposing the symbol outside the compilation unit.

{ try {
FC_ASSERT(d.find_object(new_issuer));
if( a.is_market_issued() && new_issuer == GRAPHENE_COMMITTEE_ACCOUNT )
{
const asset_object& backing = a.bitasset_data(d).options.short_backing_asset(d);
if( backing.is_market_issued() )
{
const asset_object& backing_backing = backing.bitasset_data(d).options.short_backing_asset(d);
FC_ASSERT( backing_backing.get_id() == asset_id_type(),
"May not create a blockchain-controlled market asset which is not backed by CORE.");
} else
FC_ASSERT( backing.get_id() == asset_id_type(),
"May not create a blockchain-controlled market asset which is not backed by CORE.");
}
} FC_CAPTURE_AND_RETHROW( (a)(new_issuer) ) }

void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
{ try {
database& d = db();
Expand All @@ -262,19 +279,9 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)

if( o.new_issuer )
{
FC_ASSERT(d.find_object(*o.new_issuer));
if( a.is_market_issued() && *o.new_issuer == GRAPHENE_COMMITTEE_ACCOUNT )
{
const asset_object& backing = a.bitasset_data(d).options.short_backing_asset(d);
if( backing.is_market_issued() )
{
const asset_object& backing_backing = backing.bitasset_data(d).options.short_backing_asset(d);
FC_ASSERT( backing_backing.get_id() == asset_id_type(),
"May not create a blockchain-controlled market asset which is not backed by CORE.");
} else
FC_ASSERT( backing.get_id() == asset_id_type(),
"May not create a blockchain-controlled market asset which is not backed by CORE.");
}
FC_ASSERT( d.head_block_time() < HARDFORK_CORE_199_TIME,
"Since Hardfork #199, updating issuer requires the use of asset_update_issuer_operation.");
validate_new_issuer( d, a, *o.new_issuer );
}

if( (d.head_block_time() < HARDFORK_572_TIME) || (a.dynamic_asset_data_id(d).current_supply != 0) )
Expand All @@ -289,7 +296,9 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
"Flag change is forbidden by issuer permissions");

asset_to_update = &a;
FC_ASSERT( o.issuer == a.issuer, "", ("o.issuer", o.issuer)("a.issuer", a.issuer) );
FC_ASSERT( o.issuer == a.issuer,
"Incorrect issuer for asset! (${o.issuer} != ${a.issuer})",
("o.issuer", o.issuer)("a.issuer", a.issuer) );

const auto& chain_parameters = d.get_global_properties().parameters;

Expand Down Expand Up @@ -328,6 +337,39 @@ void_result asset_update_evaluator::do_apply(const asset_update_operation& o)
return void_result();
} FC_CAPTURE_AND_RETHROW( (o) ) }

void_result asset_update_issuer_evaluator::do_evaluate(const asset_update_issuer_operation& o)
{ try {
database& d = db();

const asset_object& a = o.asset_to_update(d);

validate_new_issuer( d, a, o.new_issuer );

asset_to_update = &a;
FC_ASSERT( o.issuer == a.issuer,
"Incorrect issuer for asset! (${o.issuer} != ${a.issuer})",
("o.issuer", o.issuer)("a.issuer", a.issuer) );

if( d.head_block_time() < HARDFORK_CORE_199_TIME )
{ // TODO: remove after HARDFORK_CORE_199_TIME has passed
graphene::chain::impl::hf_199_visitor hf_199;
hf_199( o );
}


return void_result();
} FC_CAPTURE_AND_RETHROW((o)) }

void_result asset_update_issuer_evaluator::do_apply(const asset_update_issuer_operation& o)
{ try {
database& d = db();
d.modify(*asset_to_update, [&](asset_object& a) {
a.issuer = o.new_issuer;
});

return void_result();
} FC_CAPTURE_AND_RETHROW( (o) ) }

void_result asset_update_bitasset_evaluator::do_evaluate(const asset_update_bitasset_operation& o)
{ try {
database& d = db();
Expand Down
1 change: 1 addition & 0 deletions libraries/chain/db_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ void database::initialize_evaluators()
register_evaluator<transfer_from_blind_evaluator>();
register_evaluator<blind_transfer_evaluator>();
register_evaluator<asset_claim_fees_evaluator>();
register_evaluator<asset_update_issuer_evaluator>();
register_evaluator<asset_claim_pool_evaluator>();
}

Expand Down
5 changes: 4 additions & 1 deletion libraries/chain/db_notify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ struct get_impacted_account_visitor
if( op.new_issuer )
_impacted.insert( *(op.new_issuer) );
}

void operator()( const asset_update_issuer_operation& op )
{
_impacted.insert( op.new_issuer );
}
void operator()( const asset_update_bitasset_operation& op ) {}
void operator()( const asset_update_feed_producers_operation& op ) {}

Expand Down
4 changes: 4 additions & 0 deletions libraries/chain/hardfork.d/CORE_199.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// bitshares-core #199 Require owner key for change of asset-issuer (new operation)
#ifndef HARDFORK_CORE_199_TIME
#define HARDFORK_CORE_199_TIME (fc::time_point_sec( 1600000000 ))
#endif
33 changes: 30 additions & 3 deletions libraries/chain/include/graphene/chain/asset_evaluator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ namespace graphene { namespace chain {
const asset_object* asset_to_update = nullptr;
};

class asset_update_issuer_evaluator : public evaluator<asset_update_issuer_evaluator>
{
public:
typedef asset_update_issuer_operation operation_type;

void_result do_evaluate( const asset_update_issuer_operation& o );
void_result do_apply( const asset_update_issuer_operation& o );

const asset_object* asset_to_update = nullptr;
};

class asset_update_bitasset_evaluator : public evaluator<asset_update_bitasset_evaluator>
{
public:
Expand Down Expand Up @@ -161,8 +172,8 @@ namespace graphene { namespace chain {
void_result do_apply( const asset_claim_pool_operation& o );
};

namespace impl { // TODO: remove after HARDFORK_CORE_188_TIME has passed
class hf_188_visitor {
namespace impl {
class hf_188_visitor { // TODO: remove after HARDFORK_CORE_188_TIME has passed
public:
typedef void result_type;

Expand All @@ -178,6 +189,22 @@ namespace graphene { namespace chain {
op.op.visit( *this );
}
};
}

class hf_199_visitor { // TODO: remove after HARDFORK_CORE_199_TIME has passed
public:
typedef void result_type;

template<typename T>
void operator()( const T& v )const {}

void operator()( const graphene::chain::asset_update_issuer_operation& v )const {
FC_ASSERT( false, "Not allowed until hardfork 199" );
}

void operator()( const graphene::chain::proposal_create_operation& v )const {
for( const op_wrapper& op : v.proposed_ops )
op.op.visit( *this );
}
};
}
} } // graphene::chain
42 changes: 41 additions & 1 deletion libraries/chain/include/graphene/chain/protocol/asset_ops.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,39 @@ namespace graphene { namespace chain {
void validate()const;
};

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is redundant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shit, it seems I must be much much more careful when merging to resolve conflicts. On it ..

* @brief Update issuer of an asset
* @ingroup operations
*
* An issuer has general administrative power of an asset and in some cases
* also its shares issued to individuals. Thus, changing the issuer today
* requires the use of a separate operation that needs to be signed by the
* owner authority.
*
*/
struct asset_update_issuer_operation : public base_operation
{
struct fee_parameters_type {
uint64_t fee = 20 * GRAPHENE_BLOCKCHAIN_PRECISION;
};

asset fee;
account_id_type issuer;
asset_id_type asset_to_update;
account_id_type new_issuer;
extensions_type extensions;

account_id_type fee_payer()const { return issuer; }
void validate()const;

void get_required_owner_authorities( flat_set<account_id_type>& a )const
{ a.insert( issuer ); }

void get_required_active_authorities( flat_set<account_id_type>& a )const
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can (IMHO) remove the get_required_active_authorities part, but the get_required_owner_authorities is crucial as we only want owner key to be able to sign that transction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, not a big deal if keep it.

{ }

};

/**
* @brief Transfers BTS from the fee pool of a specified asset back to the issuer's balance

Expand Down Expand Up @@ -471,7 +504,6 @@ namespace graphene { namespace chain {
void validate()const;
};


} } // graphene::chain

FC_REFLECT( graphene::chain::asset_claim_fees_operation, (fee)(issuer)(amount_to_claim)(extensions) )
Expand Down Expand Up @@ -510,6 +542,7 @@ FC_REFLECT( graphene::chain::asset_settle_operation::fee_parameters_type, (fee)
FC_REFLECT( graphene::chain::asset_settle_cancel_operation::fee_parameters_type, )
FC_REFLECT( graphene::chain::asset_fund_fee_pool_operation::fee_parameters_type, (fee) )
FC_REFLECT( graphene::chain::asset_update_operation::fee_parameters_type, (fee)(price_per_kbyte) )
FC_REFLECT( graphene::chain::asset_update_issuer_operation::fee_parameters_type, (fee) )
FC_REFLECT( graphene::chain::asset_update_bitasset_operation::fee_parameters_type, (fee) )
FC_REFLECT( graphene::chain::asset_update_feed_producers_operation::fee_parameters_type, (fee) )
FC_REFLECT( graphene::chain::asset_publish_feed_operation::fee_parameters_type, (fee) )
Expand All @@ -535,6 +568,13 @@ FC_REFLECT( graphene::chain::asset_update_operation,
(new_options)
(extensions)
)
FC_REFLECT( graphene::chain::asset_update_issuer_operation,
(fee)
(issuer)
(asset_to_update)
(new_issuer)
(extensions)
)
FC_REFLECT( graphene::chain::asset_update_bitasset_operation,
(fee)
(issuer)
Expand Down
15 changes: 15 additions & 0 deletions libraries/chain/include/graphene/chain/protocol/fee_schedule.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,21 @@ namespace graphene { namespace chain {
}
};

template<>
class fee_helper<asset_update_issuer_operation> {
public:
const asset_update_issuer_operation::fee_parameters_type& cget(const flat_set<fee_parameters>& parameters)const
{
auto itr = parameters.find( asset_update_issuer_operation::fee_parameters_type() );
if ( itr != parameters.end() )
return itr->get<asset_update_issuer_operation::fee_parameters_type>();

static asset_update_issuer_operation::fee_parameters_type dummy;
dummy.fee = fee_helper<asset_update_operation>().cget(parameters).fee;
return dummy;
}
};

template<>
class fee_helper<asset_claim_pool_operation> {
public:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ namespace graphene { namespace chain {
fba_distribute_operation, // VIRTUAL
bid_collateral_operation,
execute_bid_operation, // VIRTUAL
asset_claim_pool_operation
asset_claim_pool_operation,
asset_update_issuer_operation
> operation;

/// @} // operations group
Expand Down
7 changes: 7 additions & 0 deletions libraries/chain/proposal_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <graphene/chain/protocol/fee_schedule.hpp>
#include <graphene/chain/exceptions.hpp>
#include <graphene/chain/asset_evaluator.hpp>
#include <graphene/chain/hardfork.hpp>


#include <fc/smart_ref_impl.hpp>
Expand Down Expand Up @@ -77,6 +78,12 @@ void_result proposal_create_evaluator::do_evaluate(const proposal_create_operati
_proposed_trx.operations.push_back(op.op);
_proposed_trx.validate();

if( d.head_block_time() < HARDFORK_CORE_199_TIME )
{ // TODO: remove after HARDFORK_CORE_199_TIME has passed
graphene::chain::impl::hf_199_visitor hf_199;
hf_199( o );
}

if( d.head_block_time() < HARDFORK_CORE_188_TIME )
{ // TODO: remove after HARDFORK_CORE_188_TIME has passed
graphene::chain::impl::hf_188_visitor hf_188;
Expand Down
6 changes: 6 additions & 0 deletions libraries/chain/protocol/asset_ops.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ void asset_update_operation::validate()const
FC_ASSERT(dummy.asset_id == asset_id_type());
}

void asset_update_issuer_operation::validate()const
{
FC_ASSERT( fee.amount >= 0 );
FC_ASSERT( issuer != new_issuer );
}

share_type asset_update_operation::calculate_fee(const asset_update_operation::fee_parameters_type& k)const
{
return k.fee + calculate_data_fee( fc::raw::pack_size(*this), k.price_per_kbyte );
Expand Down
16 changes: 16 additions & 0 deletions libraries/wallet/include/graphene/wallet/wallet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,21 @@ class wallet_api
asset_options new_options,
bool broadcast = false);

/** Update the issuer of an asset
* Since this call requires the owner authority of the current issuer to sign the transaction,
* a separated operation is used to change the issuer. This call simplifies the use of this action.
*
* @note This operation requires the owner key to be available in the wallet.
*
* @param symbol the name or id of the asset to update
* @param new_issuer if changing the asset's issuer, the name or id of the new issuer.
* @param broadcast true to broadcast the transaction on the network
* @returns the signed transaction updating the asset
*/
signed_transaction update_asset_issuer(string symbol,
string new_issuer,
bool broadcast = false);

/** Update the options specific to a BitAsset.
*
* BitAssets have some options which are not relevant to other asset types. This operation is used to update those
Expand Down Expand Up @@ -1679,6 +1694,7 @@ FC_API( graphene::wallet::wallet_api,
(get_transaction_id)
(create_asset)
(update_asset)
(update_asset_issuer)
(update_bitasset)
(update_asset_feed_producers)
(publish_asset_feed)
Expand Down
Loading