BSIP87 hardfork checks in proposal evaluator.

And other minor changes.
christophersanborn · May 5, 2020 · cd4753b · cd4753b
1 parent 453b30a
commit cd4753b
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 36 deletions.
diff --git a/libraries/chain/asset_evaluator.cpp b/libraries/chain/asset_evaluator.cpp
@@ -56,7 +56,16 @@ namespace detail {
       }
    }
 
-   void check_asset_claim_fees_hardfork_87_74_collatfee(const fc::time_point_sec& block_time, const asset_claim_fees_operation& op)
+   void check_asset_options_hf_bsip87(const fc::time_point_sec& block_time, const asset_options& options)
+   {
+      // HF_REMOVABLE: Following hardfork check should be removable after hardfork date passes:
+      FC_ASSERT( !options.extensions.value.force_settle_fee_percent.valid()
+                 || block_time >= HARDFORK_CORE_BSIP87_TIME,
+                 "A BitAsset's FSFP cannot be set before Hardfork BSIP87" );
+   }
+
+   void check_asset_claim_fees_hardfork_87_74_collatfee(const fc::time_point_sec& block_time,
+                                                        const asset_claim_fees_operation& op)
    {
       // HF_REMOVABLE: Following hardfork check should be removable after hardfork date passes:
       FC_ASSERT( !op.extensions.value.claim_from_asset_id.valid() ||
@@ -70,13 +79,17 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o
 { try {
 
    const database& d = db();
+   const time_point_sec now = d.head_block_time();
+
+   // Hardfork Checks:
+   detail::check_asset_options_hf_1774(now, op.common_options);
+   detail::check_asset_options_hf_bsip81(now, op.common_options);
+   detail::check_asset_options_hf_bsip87(now, op.common_options); // HF_REMOVABLE
 
    const auto& chain_parameters = d.get_global_properties().parameters;
    FC_ASSERT( op.common_options.whitelist_authorities.size() <= chain_parameters.maximum_asset_whitelist_authorities );
    FC_ASSERT( op.common_options.blacklist_authorities.size() <= chain_parameters.maximum_asset_whitelist_authorities );
 
-   detail::check_asset_options_hf_1774(d.head_block_time(), op.common_options);
-
    // Check that all authorities do exist
    for( auto id : op.common_options.whitelist_authorities )
       d.get_object(id);
@@ -87,8 +100,6 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o
    auto asset_symbol_itr = asset_indx.find( op.symbol );
    FC_ASSERT( asset_symbol_itr == asset_indx.end() );
 
-   // Define now from the current block time
-   const time_point_sec now = d.head_block_time();
    // This must remain due to "BOND.CNY" being allowed before this HF
    if( now > HARDFORK_385_TIME )
    {
@@ -123,19 +134,12 @@ void_result asset_create_evaluator::do_evaluate( const asset_create_operation& o
                  op.bitasset_opts->force_settlement_delay_sec > chain_parameters.block_interval );
    }
 
-   FC_ASSERT( d.head_block_time() >= HARDFORK_CORE_BSIP87_TIME
-              || !op.common_options.extensions.value.force_settle_fee_percent.valid(),
-              "A BitAsset's FSFP cannot be set before Hardfork BSIP87" );
-
    if( op.is_prediction_market )
    {
       FC_ASSERT( op.bitasset_opts );
       FC_ASSERT( op.precision == op.bitasset_opts->short_backing_asset(d).precision );
    }
 
-   // Check the taker fee percent
-   detail::check_asset_options_hf_bsip81(now, op.common_options);
-
    return void_result();
 } FC_CAPTURE_AND_RETHROW( (op) ) }
 
@@ -299,6 +303,11 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
    const database& d = db();
    const time_point_sec now = d.head_block_time();
 
+   // Hardfork Checks:
+   detail::check_asset_options_hf_1774(now, o.new_options);
+   detail::check_asset_options_hf_bsip81(now, o.new_options);
+   detail::check_asset_options_hf_bsip87(now, o.new_options); // HF_REMOVABLE
+
    const asset_object& a = o.asset_to_update(d);
    auto a_copy = a;
    a_copy.options = o.new_options;
@@ -311,8 +320,6 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
       validate_new_issuer( d, a, *o.new_issuer );
    }
 
-   detail::check_asset_options_hf_1774(d.head_block_time(), o.new_options);
-
    if( a.dynamic_asset_data_id(d).current_supply != 0 )
    {
       // new issuer_permissions must be subset of old issuer permissions
@@ -329,10 +336,6 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
               "Incorrect issuer for asset! (${o.issuer} != ${a.issuer})",
               ("o.issuer", o.issuer)("a.issuer", a.issuer) );
 
-   FC_ASSERT( d.head_block_time() >= HARDFORK_CORE_BSIP87_TIME
-              || !o.new_options.extensions.value.force_settle_fee_percent.valid(),
-              "A BitAsset's FSFP cannot be set before Hardfork BSIP87" );
-
    const auto& chain_parameters = d.get_global_properties().parameters;
 
    FC_ASSERT( o.new_options.whitelist_authorities.size() <= chain_parameters.maximum_asset_whitelist_authorities );
@@ -342,9 +345,6 @@ void_result asset_update_evaluator::do_evaluate(const asset_update_operation& o)
    for( auto id : o.new_options.blacklist_authorities )
       d.get_object(id);
 
-   // Check the taker fee percent
-   detail::check_asset_options_hf_bsip81(now, o.new_options);
-
    return void_result();
 } FC_CAPTURE_AND_RETHROW((o)) }
 

diff --git a/libraries/chain/db_market.cpp b/libraries/chain/db_market.cpp
@@ -1011,9 +1011,6 @@ bool database::fill_settle_order( const force_settlement_object& settle, const a
 
    auto total_collateral_denominated_fees = market_fees + force_settle_fees;
 
-   // TODO: Do we need a something-for-nothing check here? Or does rounding guarantee fees
-   // strictly less than receives?
-
    // If we don't consume entire settle order:
    if( pays < settle.balance )
    {
@@ -1027,7 +1024,8 @@ bool database::fill_settle_order( const force_settlement_object& settle, const a
    adjust_balance(settle.owner, receives - total_collateral_denominated_fees);
 
    assert( pays.asset_id != receives.asset_id );
-   push_applied_operation( fill_order_operation( settle.id, settle.owner, pays, receives, total_collateral_denominated_fees, fill_price, is_maker ) );
+   push_applied_operation( fill_order_operation( settle.id, settle.owner, pays, receives,
+                                                 total_collateral_denominated_fees, fill_price, is_maker ) );
 
    if (filled)
       remove(settle);
@@ -1401,7 +1399,7 @@ asset database::pay_market_fees(const account_object* seller, const asset_object
 
 /***
  * @brief Calculate force-settlement fee and give it to issuer of the settled asset
- * @param collecting_asset the smart asset object which should receive the the fee
+ * @param collecting_asset the smart asset object which should receive the fee
  * @param collat_receives the amount of collateral the settler would expect to receive absent this fee
  *     (fee is computed as a percentage of this amount)
  * @return asset denoting the amount of fee collected
@@ -1414,7 +1412,9 @@ asset database::pay_force_settle_fees(const asset_object& collecting_asset, cons
    if( !collecting_asset.options.extensions.value.force_settle_fee_percent.valid()
          || *collecting_asset.options.extensions.value.force_settle_fee_percent == 0 )
       return collat_asset.amount(0);
-   auto value = detail::calculate_percent(collat_receives.amount, *collecting_asset.options.extensions.value.force_settle_fee_percent);
+
+   auto value = detail::calculate_percent(collat_receives.amount,
+                                          *collecting_asset.options.extensions.value.force_settle_fee_percent);
    asset settle_fee = collat_asset.amount(value);
 
    // Deposit fee in asset's dynamic data object:

diff --git a/libraries/chain/proposal_evaluator.cpp b/libraries/chain/proposal_evaluator.cpp
@@ -29,9 +29,12 @@
 namespace graphene { namespace chain {
 
 namespace detail {
-   void check_asset_options_hf_1774(const fc::time_point_sec& block_time, const asset_options& options); 
+   void check_asset_options_hf_1774(const fc::time_point_sec& block_time, const asset_options& options);
    void check_asset_options_hf_bsip81(const fc::time_point_sec& block_time, const asset_options& options);
-   void check_asset_claim_fees_hardfork_87_74_collatfee(const fc::time_point_sec& block_time, const asset_claim_fees_operation& op);
+   void check_asset_options_hf_bsip87(const fc::time_point_sec& block_time,
+                                      const asset_options& options); // HF_REMOVABLE
+   void check_asset_claim_fees_hardfork_87_74_collatfee(const fc::time_point_sec& block_time,
+                                                        const asset_claim_fees_operation& op); // HF_REMOVABLE
 }
 
 struct proposal_operation_hardfork_visitor
@@ -48,22 +51,25 @@ struct proposal_operation_hardfork_visitor
    void operator()(const T &v) const {}
 
    void operator()(const graphene::chain::asset_create_operation &v) const {
-      // hf_1774
-      detail::check_asset_options_hf_1774(block_time, v.common_options);
 
-      // HARDFORK_BSIP_81
+      detail::check_asset_options_hf_1774(block_time, v.common_options);
       detail::check_asset_options_hf_bsip81(block_time, v.common_options);
+      detail::check_asset_options_hf_bsip87(block_time, v.common_options); // HF_REMOVABLE
+
    }
+
    void operator()(const graphene::chain::asset_update_operation &v) const {
-      // hf_1774
-      detail::check_asset_options_hf_1774(block_time, v.new_options);
 
-      // HARDFORK_BSIP_81
+      detail::check_asset_options_hf_1774(block_time, v.new_options);
       detail::check_asset_options_hf_bsip81(block_time, v.new_options);
+      detail::check_asset_options_hf_bsip87(block_time, v.new_options); // HF_REMOVABLE
+
    }
 
    void operator()(const graphene::chain::asset_claim_fees_operation &v) const {
+
       detail::check_asset_claim_fees_hardfork_87_74_collatfee(block_time, v); // HF_REMOVABLE
+
    }
 
    void operator()(const graphene::chain::committee_member_update_global_parameters_operation &op) const {

diff --git a/libraries/protocol/include/graphene/protocol/asset_ops.hpp b/libraries/protocol/include/graphene/protocol/asset_ops.hpp
@@ -563,7 +563,8 @@ FC_REFLECT( graphene::protocol::bitasset_options,
             (extensions)
           )
 
-FC_REFLECT( graphene::protocol::additional_asset_options, (reward_percent)(whitelist_market_fee_sharing)(taker_fee_percent)(force_settle_fee_percent) )
+FC_REFLECT( graphene::protocol::additional_asset_options,
+            (reward_percent)(whitelist_market_fee_sharing)(taker_fee_percent)(force_settle_fee_percent) )
 FC_REFLECT( graphene::protocol::asset_create_operation::fee_parameters_type, (symbol3)(symbol4)(long_symbol)(price_per_kbyte) )
 FC_REFLECT( graphene::protocol::asset_global_settle_operation::fee_parameters_type, (fee) )
 FC_REFLECT( graphene::protocol::asset_settle_operation::fee_parameters_type, (fee) )