Merge pull request #2468 from bitshares/pr-973-asset-auths

Add asset authorization checks for some operations

libraries/chain/asset_evaluator.cpp

@@ -1010,6 +1010,15 @@ void_result asset_settle_evaluator::do_evaluate(const asset_settle_evaluator::op
       FC_THROW_EXCEPTION(insufficient_feeds, "Cannot force settle with no price feed.");
    FC_ASSERT( d.get_balance( op.account, op.amount.asset_id ) >= op.amount, "Insufficient balance" );
 
+   // Since hard fork core-973, check asset authorization limitations
+   if( HARDFORK_CORE_973_PASSED(d.head_block_time()) )
+   {
+      FC_ASSERT( is_authorized_asset( d, *fee_paying_account, *asset_to_settle ),
+                 "The account is not allowed to settle the asset" );
+      FC_ASSERT( is_authorized_asset( d, *fee_paying_account, bitasset.options.short_backing_asset(d) ),
+                 "The account is not allowed to receive the backing asset" );
+   }
+
    return void_result();
 } FC_CAPTURE_AND_RETHROW( (op) ) }
 
@@ -1240,6 +1249,8 @@ void_result asset_claim_fees_evaluator::do_evaluate( const asset_claim_fees_oper
                  "backed by (${fid}). Asset DDO: ${ddo}. Fee claim: ${claim}.", ("a",container_asset->symbol)
                  ("id",container_asset->id)("fid",o.amount_to_claim.asset_id)("ddo",*container_ddo)
                  ("claim",o.amount_to_claim) );
+      // Note: asset authorization check on (account, collateral asset) is skipped here,
+      //       because it is fine to allow the funds to be moved to account balance
    }
 
    return void_result();

libraries/chain/hardfork.d/CORE_973.hf

@@ -0,0 +1,6 @@
+// bitshares-core issue #973 check asset authorizations for operations
+#ifndef HARDFORK_CORE_973_TIME
+// Jan 1 2030, midnight; this is a dummy date until a hardfork date is scheduled
+#define HARDFORK_CORE_973_TIME (fc::time_point_sec( 1893456000 ))
+#define HARDFORK_CORE_973_PASSED(now) (now >= HARDFORK_CORE_973_TIME)
+#endif

libraries/chain/include/graphene/chain/market_evaluator.hpp

@@ -80,7 +80,6 @@ namespace graphene { namespace chain {
 
          bool _closing_order = false;
          const asset_object* _debt_asset = nullptr;
-         const account_object* _paying_account = nullptr;
          const call_order_object* _order = nullptr;
          const asset_bitasset_data_object* _bitasset_data = nullptr;
          const asset_dynamic_data_object*  _dynamic_data_obj = nullptr;
@@ -96,7 +95,6 @@ namespace graphene { namespace chain {
 
          const asset_object* _debt_asset = nullptr;
          const asset_bitasset_data_object* _bitasset_data = nullptr;
-         const account_object* _paying_account = nullptr;
          const collateral_bid_object* _bid = nullptr;
    };
 

libraries/chain/market_evaluator.cpp

@@ -184,7 +184,7 @@ void_result call_order_update_evaluator::do_evaluate(const call_order_update_ope
 
    auto next_maintenance_time = d.get_dynamic_global_properties().next_maintenance_time;
 
-   _paying_account = &o.funding_account(d);
+   // Note: funding_account is the fee payer thus exists in the database
    _debt_asset     = &o.delta_debt.asset_id(d);
    FC_ASSERT( _debt_asset->is_market_issued(), "Unable to cover ${sym} as it is not a collateralized asset.",
               ("sym", _debt_asset->symbol) );
@@ -220,6 +220,15 @@ void_result call_order_update_evaluator::do_evaluate(const call_order_update_ope
    else if( _bitasset_data->current_feed.settlement_price.is_null() )
       FC_THROW_EXCEPTION(insufficient_feeds, "Cannot borrow asset with no price feed.");
 
+   // Since hard fork core-973, check asset authorization limitations
+   if( HARDFORK_CORE_973_PASSED(d.head_block_time()) )
+   {
+      FC_ASSERT( is_authorized_asset( d, *fee_paying_account, *_debt_asset ),
+                 "The account is not allowed to transact the debt asset" );
+      FC_ASSERT( is_authorized_asset( d, *fee_paying_account, _bitasset_data->options.short_backing_asset(d) ),
+                 "The account is not allowed to transact the collateral asset" );
+   }
+
    // Note: there was code here checking whether the account has enough balance to increase delta collateral,
    //       which is now removed since the check is implicitly done later by `adjust_balance()` in `do_apply()`.
 
@@ -248,7 +257,7 @@ object_id_type call_order_update_evaluator::do_apply(const call_order_update_ope
       // Adjust the total core in orders accodingly
       if( o.delta_collateral.asset_id == asset_id_type() )
       {
-         d.modify(_paying_account->statistics(d), [&](account_statistics_object& stats) {
+         d.modify( d.get_account_stats_by_owner( o.funding_account ), [&o](account_statistics_object& stats) {
                stats.total_core_in_orders += o.delta_collateral.amount;
          });
       }
@@ -398,7 +407,7 @@ void_result bid_collateral_evaluator::do_evaluate(const bid_collateral_operation
 
    FC_ASSERT( d.head_block_time() > HARDFORK_CORE_216_TIME, "Not yet!" );
 
-   _paying_account = &o.bidder(d);
+   // Note: bidder is the fee payer thus exists in the database
    _debt_asset     = &o.debt_covered.asset_id(d);
    FC_ASSERT( _debt_asset->is_market_issued(), "Unable to cover ${sym} as it is not a collateralized asset.",
               ("sym", _debt_asset->symbol) );
@@ -421,18 +430,27 @@ void_result bid_collateral_evaluator::do_evaluate(const bid_collateral_operation
 
    if( o.additional_collateral.amount > 0 )
    {
+      auto collateral_balance = d.get_balance( o.bidder, _bitasset_data->options.short_backing_asset );
       if( _bid && d.head_block_time() >= HARDFORK_CORE_1692_TIME ) // TODO: see if HF check can be removed after HF
       {
          asset delta = o.additional_collateral - _bid->get_additional_collateral();
-         FC_ASSERT( d.get_balance(*_paying_account, _bitasset_data->options.short_backing_asset(d)) >= delta,
+         FC_ASSERT( collateral_balance >= delta,
                     "Cannot increase bid from ${oc} to ${nc} collateral when payer only has ${b}",
                     ("oc", _bid->get_additional_collateral().amount)("nc", o.additional_collateral.amount)
-                    ("b", d.get_balance(*_paying_account, o.additional_collateral.asset_id(d)).amount) );
+                    ("b", collateral_balance.amount) );
       } else
-         FC_ASSERT( d.get_balance( *_paying_account,
-                                   _bitasset_data->options.short_backing_asset(d) ) >= o.additional_collateral,
+         FC_ASSERT( collateral_balance >= o.additional_collateral,
                     "Cannot bid ${c} collateral when payer only has ${b}", ("c", o.additional_collateral.amount)
-                    ("b", d.get_balance(*_paying_account, o.additional_collateral.asset_id(d)).amount) );
+                    ("b", collateral_balance.amount) );
+   }
+
+   // Since hard fork core-973, check asset authorization limitations
+   if( HARDFORK_CORE_973_PASSED(d.head_block_time()) )
+   {
+      FC_ASSERT( is_authorized_asset( d, *fee_paying_account, *_debt_asset ),
+                 "The account is not allowed to transact the debt asset" );
+      FC_ASSERT( is_authorized_asset( d, *fee_paying_account, _bitasset_data->options.short_backing_asset(d) ),
+                 "The account is not allowed to transact the collateral asset" );
    }
 
    return void_result();

libraries/chain/transfer_evaluator.cpp

@@ -99,12 +99,16 @@ void_result override_transfer_evaluator::do_evaluate( const override_transfer_op
    const account_object& from_account    = op.from(d);
    const account_object& to_account      = op.to(d);
 
-   FC_ASSERT( is_authorized_asset( d, to_account, asset_type ) );
+   FC_ASSERT( is_authorized_asset( d, to_account, asset_type ),
+              "The to_account is not allowed to transact the asset" );
    // Since hard fork core-2295, do not check asset authorization limitations on from_account for override_transfer
    // TODO code cleanup: if applicable (could be false due to proposals),
    //      remove the check and the assertion below after the hard fork time, keep the comment about reasoning above
    if( !HARDFORK_CORE_2295_PASSED(d.head_block_time()) )
-      FC_ASSERT( is_authorized_asset( d, from_account, asset_type ) );
+   {
+      FC_ASSERT( is_authorized_asset( d, from_account, asset_type ),
+                 "The from_account is not allowed to transact the asset" );
+   }
 
    FC_ASSERT( d.get_balance( from_account, asset_type ).amount >= op.amount.amount,
               "", ("total_transfer",op.amount)("balance",d.get_balance(from_account, asset_type).amount) );

libraries/chain/vesting_balance_evaluator.cpp

@@ -27,21 +27,29 @@
 #include <graphene/chain/vesting_balance_evaluator.hpp>
 #include <graphene/chain/vesting_balance_object.hpp>
 #include <graphene/chain/hardfork.hpp>
+#include <graphene/chain/is_authorized_asset.hpp>
 
 namespace graphene { namespace chain {
 
 void_result vesting_balance_create_evaluator::do_evaluate( const vesting_balance_create_operation& op )
 { try {
    const database& d = db();
 
-   const account_object& creator_account = op.creator( d );
-   /* const account_object& owner_account = */ op.owner( d );
+   const account_object& creator_account = *fee_paying_account;
+   const account_object& owner_account = op.owner( d );
 
-   // TODO: Check asset authorizations and withdrawals
+   const asset_object& asset_obj = op.amount.asset_id( d );
 
-   FC_ASSERT( op.amount.amount > 0 );
-   FC_ASSERT( d.get_balance( creator_account.id, op.amount.asset_id ) >= op.amount );
-   FC_ASSERT( !op.amount.asset_id(d).is_transfer_restricted() );
+   FC_ASSERT( !asset_obj.is_transfer_restricted(), "Asset has transfer_restricted flag enabled" );
+
+   // Since hard fork core-973, check asset authorization limitations
+   if( HARDFORK_CORE_973_PASSED(d.head_block_time()) )
+   {
+      FC_ASSERT( is_authorized_asset( d, creator_account, asset_obj ),
+                 "The creator account is not allowed to transact the asset" );
+      FC_ASSERT( is_authorized_asset( d, owner_account, asset_obj ),
+                 "The owner account is not allowed to transact the asset" );
+   }
 
    return void_result();
 } FC_CAPTURE_AND_RETHROW( (op) ) }
@@ -90,7 +98,6 @@ object_id_type vesting_balance_create_evaluator::do_apply( const vesting_balance
    database& d = db();
    const time_point_sec now = d.head_block_time();
 
-   FC_ASSERT( d.get_balance( op.creator, op.amount.asset_id ) >= op.amount );
    d.adjust_balance( op.creator, -op.amount );
 
    const vesting_balance_object& vbo = d.create< vesting_balance_object >( [&]( vesting_balance_object& obj )
@@ -116,8 +123,8 @@ void_result vesting_balance_withdraw_evaluator::do_evaluate( const vesting_balan
    FC_ASSERT( vbo.is_withdraw_allowed( now, op.amount ), "", ("now", now)("op", op)("vbo", vbo) );
    assert( op.amount <= vbo.balance );      // is_withdraw_allowed should fail before this check is reached
 
-   /* const account_object& owner_account = */ op.owner( d );
-   // TODO: Check asset authorizations and withdrawals
+   // Note: asset authorization check is skipped here,
+   //       because it is fine to allow the funds to be moved to account balance
    return void_result();
 } FC_CAPTURE_AND_RETHROW( (op) ) }
 
@@ -139,7 +146,6 @@ void_result vesting_balance_withdraw_evaluator::do_apply( const vesting_balance_
 
    d.adjust_balance( op.owner, op.amount );
 
-   // TODO: Check asset authorizations and withdrawals
    return void_result();
 } FC_CAPTURE_AND_RETHROW( (op) ) }