Defer MCR update to next maintenance interval

libraries/chain/asset_evaluator.cpp

@@ -463,11 +463,28 @@ static bool update_bitasset_object_options(
 
    if( should_update_feeds )
    {
-      const auto old_feed_price = bdo.current_feed.settlement_price;
+      const auto old_feed = bdo.current_feed;
       bdo.update_median_feeds( db.head_block_time() );
 
+      // TODO review and refactor / cleanup after hard fork:
+      //      1. if hf_core_868_890 and core-935 occurred at same time
+      //      2. if wlog did not actually get called
+
+      // We need to call check_call_orders if the price feed changes after hardfork core-935
+      if( next_maint_time > HARDFORK_CORE_935_TIME )
+         return ( !( old_feed == bdo.current_feed ) );
+
       // We need to call check_call_orders if the settlement price changes after hardfork core-868-890
-      return after_hf_core_868_890 && old_feed_price != bdo.current_feed.settlement_price;
+      if( after_hf_core_868_890 )
+      {
+         if( old_feed.settlement_price != bdo.current_feed.settlement_price )
+            return true;
+         else
+         {
+            if( !( old_feed == bdo.current_feed ) )
+               wlog( "Settlement price did not change but current_feed changed at block ${b}", ("b",db.head_block_num()) );
+         }
+      }
    }
 
    return false;

libraries/chain/db_maint.cpp

@@ -856,7 +856,7 @@ void database::process_bitassets()
  */
 // TODO: for better performance, this function can be removed if it actually updated nothing at hf time.
 //       * Also need to update related test cases
-//       * NOTE: perhaps the removal can't be applied to testnet
+//       * NOTE: the removal can't be applied to testnet
 void process_hf_868_890( database& db, bool skip_check_call_orders )
 {
    auto head_time = db.head_block_time();
@@ -875,8 +875,8 @@ void process_hf_868_890( database& db, bool skip_check_call_orders )
 
       // for each feed
       const asset_bitasset_data_object& bitasset_data = current_asset.bitasset_data(db);
-      // NOTE: We'll only need old_price if HF343 hasn't rolled out yet
-      auto old_price = bitasset_data.current_feed.settlement_price;
+      // NOTE: We'll only need old_feed if HF343 hasn't rolled out yet
+      auto old_feed = bitasset_data.current_feed;
       bool feeds_changed = false; // did any feed change
       auto itr = bitasset_data.feeds.begin();
       while( itr != bitasset_data.feeds.end() )
@@ -920,20 +920,71 @@ void process_hf_868_890( database& db, bool skip_check_call_orders )
          obj.update_median_feeds( head_time );
       });
 
-      bool median_changed = ( old_price != bitasset_data.current_feed.settlement_price );
-      if( median_changed )
+      bool median_changed = ( old_feed.settlement_price != bitasset_data.current_feed.settlement_price );
+      bool median_feed_changed = ( !( old_feed == bitasset_data.current_feed ) );
+      if( median_feed_changed )
       {
          wlog( "Median feed for asset ${asset_sym} (${asset_id}) changed during hardfork core-868-890",
                ("asset_sym", current_asset.symbol)("asset_id", current_asset.id) );
       }
 
+      // Note: due to bitshares-core issue #935, this check below (using median_changed) is incorrect.
+      //       However, `skip_check_call_orders` will likely be true in both testnet and mainnet,
+      //         so effectively the incorrect code won't make a difference.
+      // TODO cleanup after hard fork
       if( !skip_check_call_orders && median_changed ) // check_call_orders should be called
       {
          db.check_call_orders( current_asset );
       }
    } // for each market issued asset
 }
 
+/******
+ * @brief one-time data process for hard fork core-935
+ *
+ * Prior to hardfork 935, `check_call_orders` may be unintendedly skipped when
+ * median price feed has changed. This method will run at the hardfork time, and
+ * call `check_call_orders` for all markets.
+ * https://github.com/bitshares/bitshares-core/issues/935
+ *
+ * @param db the database
+ */
+// TODO: for better performance, this function can be removed if it actually updated nothing at hf time.
+//       * Also need to update related test cases
+//       * NOTE: perhaps the removal can't be applied to testnet
+void process_hf_935( database& db )
+{
+   bool changed_something = false;
+   const asset_bitasset_data_object* bitasset = nullptr;
+   bool settled_before_check_call;
+   bool settled_after_check_call;
+   // for each market issued asset
+   const auto& asset_idx = db.get_index_type<asset_index>().indices().get<by_type>();
+   for( auto asset_itr = asset_idx.lower_bound(true); asset_itr != asset_idx.end(); ++asset_itr )
+   {
+      const auto& current_asset = *asset_itr;
+
+      if( !changed_something )
+      {
+         bitasset = &current_asset.bitasset_data( db );
+         settled_before_check_call = bitasset->has_settlement(); // whether already force settled
+      }
+
+      bool called_some = db.check_call_orders( current_asset );
+
+      if( !changed_something )
+      {
+         settled_after_check_call = bitasset->has_settlement(); // whether already force settled
+
+         if( settled_before_check_call != settled_after_check_call || called_some )
+         {
+            changed_something = true;
+            wlog( "process_hf_935 changed something" );
+         }
+      }
+   }
+}
+
 void database::perform_chain_maintenance(const signed_block& next_block, const global_property_object& global_props)
 {
    const auto& gpo = get_global_properties();
@@ -1094,6 +1145,11 @@ void database::perform_chain_maintenance(const signed_block& next_block, const g
    if( (dgpo.next_maintenance_time <= HARDFORK_CORE_868_890_TIME) && (next_maintenance_time > HARDFORK_CORE_868_890_TIME) )
       process_hf_868_890( *this, to_update_and_match_call_orders );
 
+   // Explicitly call check_call_orders of all markets
+   if( (dgpo.next_maintenance_time <= HARDFORK_CORE_935_TIME) && (next_maintenance_time > HARDFORK_CORE_935_TIME)
+         && !to_update_and_match_call_orders )
+      process_hf_935( *this );
+
    modify(dgpo, [next_maintenance_time](dynamic_global_property_object& d) {
       d.next_maintenance_time = next_maintenance_time;
       d.accounts_registered_this_interval = 0;

libraries/chain/hardfork.d/CORE_935.hf

@@ -0,0 +1,4 @@
+// bitshares-core issue #935 Call check_call_orders not only when settlement_price changed
+#ifndef HARDFORK_CORE_935_TIME
+#define HARDFORK_CORE_935_TIME (fc::time_point_sec( 1537625300 ))
+#endif

tests/common/database_fixture.cpp

@@ -351,20 +351,25 @@ void database_fixture::generate_blocks( uint32_t block_count )
       generate_block();
 }
 
-void database_fixture::generate_blocks(fc::time_point_sec timestamp, bool miss_intermediate_blocks, uint32_t skip)
+uint32_t database_fixture::generate_blocks(fc::time_point_sec timestamp, bool miss_intermediate_blocks, uint32_t skip)
 {
    if( miss_intermediate_blocks )
    {
       generate_block(skip);
       auto slots_to_miss = db.get_slot_at_time(timestamp);
       if( slots_to_miss <= 1 )
-         return;
+         return 1;
       --slots_to_miss;
       generate_block(skip, init_account_priv_key, slots_to_miss);
-      return;
+      return 2;
    }
+   uint32_t blocks = 0;
    while( db.head_block_time() < timestamp )
+   {
       generate_block(skip);
+      ++blocks;
+   }
+   return blocks;
 }
 
 account_create_operation database_fixture::make_account(

tests/common/database_fixture.hpp

@@ -184,8 +184,9 @@ struct database_fixture {
    /**
     * @brief Generates blocks until the head block time matches or exceeds timestamp
     * @param timestamp target time to generate blocks until
+    * @return number of blocks generated
     */
-   void generate_blocks(fc::time_point_sec timestamp, bool miss_intermediate_blocks = true, uint32_t skip = ~0);
+   uint32_t generate_blocks(fc::time_point_sec timestamp, bool miss_intermediate_blocks = true, uint32_t skip = ~0);
 
    account_create_operation make_account(
       const std::string& name = "nathan",

tests/tests/bitasset_tests.cpp

@@ -434,10 +434,8 @@ BOOST_AUTO_TEST_CASE( hf_890_test )
 
       if( i == 1 ) // go beyond hard fork
       {
-         generate_blocks(HARDFORK_CORE_868_890_TIME - mi, true, skip);
-         blocks += 2;
-         generate_blocks(db.get_dynamic_global_properties().next_maintenance_time, true, skip);
-         blocks += 2;
+         blocks += generate_blocks(HARDFORK_CORE_868_890_TIME - mi, true, skip);
+         blocks += generate_blocks(db.get_dynamic_global_properties().next_maintenance_time, true, skip);
       }
       set_expiration( db, trx );
 
@@ -486,8 +484,7 @@ BOOST_AUTO_TEST_CASE( hf_890_test )
       // settlement price = 100 USD / 10 CORE, mssp = 100/11 USD/CORE
 
       // let the feed expire
-      generate_blocks( db.head_block_time() + 1200, true, skip );
-      blocks += 2;
+      blocks += generate_blocks( db.head_block_time() + 1200, true, skip );
       set_expiration( db, trx );
 
       // check: median feed should be null
@@ -519,10 +516,8 @@ BOOST_AUTO_TEST_CASE( hf_890_test )
          BOOST_CHECK( db.find<limit_order_object>( sell_id ) );
 
          // go beyond hard fork
-         generate_blocks(HARDFORK_CORE_868_890_TIME - mi, true, skip);
-         blocks += 2;
-         generate_blocks(db.get_dynamic_global_properties().next_maintenance_time, true, skip);
-         blocks += 2;
+         blocks += generate_blocks(HARDFORK_CORE_868_890_TIME - mi, true, skip);
+         blocks += generate_blocks(db.get_dynamic_global_properties().next_maintenance_time, true, skip);
       }
 
       // after hard fork, median feed should become valid, and the limit order should be filled
@@ -542,6 +537,178 @@ BOOST_AUTO_TEST_CASE( hf_890_test )
    }
 }
 
+/*********
+ * @brief Call check_call_orders after current_feed changed but not only settlement_price changed.
+ */
+BOOST_AUTO_TEST_CASE( hf_935_test )
+{ try {
+   uint32_t skip = database::skip_witness_signature
+                 | database::skip_transaction_signatures
+                 | database::skip_transaction_dupe_check
+                 | database::skip_block_size_check
+                 | database::skip_tapos_check
+                 | database::skip_authority_check
+                 | database::skip_merkle_check
+                 ;
+   generate_blocks( HARDFORK_615_TIME, true, skip ); // get around Graphene issue #615 feed expiration bug
+   generate_blocks( db.get_dynamic_global_properties().next_maintenance_time, true, skip );
+   generate_block( skip );
+
+   for( int i = 0; i < 3; ++i )
+   {
+      idump( (i) );
+      int blocks = 0;
+      auto mi = db.get_global_properties().parameters.maintenance_interval;
+
+      if( i == 1 ) // go beyond hard fork 890
+      {
+         generate_blocks( HARDFORK_CORE_868_890_TIME - mi, true, skip );
+         generate_blocks( db.get_dynamic_global_properties().next_maintenance_time, true, skip );
+      }
+      else if( i == 2 ) // go beyond hard fork 935
+      {
+         generate_blocks( HARDFORK_CORE_935_TIME - mi, true, skip );
+         generate_blocks( db.get_dynamic_global_properties().next_maintenance_time, true, skip );
+      }
+      set_expiration( db, trx );
+
+      ACTORS( (seller)(borrower)(feedproducer)(feedproducer2)(feedproducer3) );
+
+      int64_t init_balance( 1000000 );
+
+      transfer( committee_account, borrower_id, asset(init_balance) );
+
+      const auto& bitusd = create_bitasset( "USDBIT", feedproducer_id );
+      asset_id_type usd_id = bitusd.id;
+
+      {
+         // change feed lifetime (2x maintenance interval)
+         const asset_object& asset_to_update = usd_id(db);
+         asset_update_bitasset_operation ba_op;
+         ba_op.asset_to_update = usd_id;
+         ba_op.issuer = asset_to_update.issuer;
+         ba_op.new_options = asset_to_update.bitasset_data(db).options;
+         ba_op.new_options.feed_lifetime_sec = 300;
+         trx.operations.push_back(ba_op);
+         PUSH_TX(db, trx, ~0);
+         trx.clear();
+      }
+
+      // set feed producers
+      flat_set<account_id_type> producers;
+      producers.insert( feedproducer_id );
+      producers.insert( feedproducer2_id );
+      producers.insert( feedproducer3_id );
+      update_feed_producers( usd_id(db), producers );
+
+      // prepare feed data
+      price_feed current_feed;
+      current_feed.maintenance_collateral_ratio = 3500;
+      current_feed.maximum_short_squeeze_ratio = 1100;
+
+      // set 2 price feeds with 350% MCR
+      current_feed.settlement_price = asset(100, usd_id) / asset(5);
+      publish_feed( usd_id, feedproducer_id, current_feed );
+      publish_feed( usd_id, feedproducer2_id, current_feed );
+
+      // check median, MCR should be 350%
+      BOOST_CHECK( usd_id(db).bitasset_data(db).current_feed.settlement_price == current_feed.settlement_price );
+      BOOST_CHECK_EQUAL( usd_id(db).bitasset_data(db).current_feed.maintenance_collateral_ratio, 3500 );
+
+      // generate some blocks, let the feeds expire
+      blocks += generate_blocks( db.head_block_time() + 360, true, skip );
+      set_expiration( db, trx );
+
+      // check median, should be null
+      BOOST_CHECK( usd_id(db).bitasset_data(db).current_feed.settlement_price.is_null() );
+
+      // publish a new feed with 175% MCR, new median MCR would be 175%
+      current_feed.maintenance_collateral_ratio = 1750;
+      publish_feed( usd_id, feedproducer3_id, current_feed );
+
+      // check median, MCR should be 175%
+      BOOST_CHECK( usd_id(db).bitasset_data(db).current_feed.settlement_price == current_feed.settlement_price );
+      BOOST_CHECK_EQUAL( usd_id(db).bitasset_data(db).current_feed.maintenance_collateral_ratio, 1750 );
+
+      // Place some collateralized orders
+      // start out with 300% collateral, call price is 15/175 CORE/USD = 60/700
+      borrow( borrower_id, asset(100, usd_id), asset(15) );
+
+      transfer( borrower_id, seller_id, asset(100, usd_id) );
+
+      // place a sell order, it won't be matched with the call order now.
+      // when median MCR changed to 350%, the call order with 300% collateral will be in margin call territory,
+      // then this limit order should be filled
+      limit_order_id_type sell_id = create_sell_order( seller_id, asset(20, usd_id), asset(1) )->id;
+
+      {
+         // change feed lifetime to longer, let all 3 feeds be valid
+         const asset_object& asset_to_update = usd_id(db);
+         asset_update_bitasset_operation ba_op;
+         ba_op.asset_to_update = usd_id;
+         ba_op.issuer = asset_to_update.issuer;
+         ba_op.new_options = asset_to_update.bitasset_data(db).options;
+         ba_op.new_options.feed_lifetime_sec = HARDFORK_CORE_935_TIME.sec_since_epoch()
+                                             - db.head_block_time().sec_since_epoch()
+                                             + mi * 3;
+         trx.operations.push_back(ba_op);
+         PUSH_TX(db, trx, ~0);
+         trx.clear();
+      }
+
+      // check
+      if( i == 0 ) // before hard fork 890
+      {
+         // median feed won't change (issue 890)
+         BOOST_CHECK( usd_id(db).bitasset_data(db).current_feed.settlement_price == current_feed.settlement_price );
+         BOOST_CHECK_EQUAL( usd_id(db).bitasset_data(db).current_feed.maintenance_collateral_ratio, 1750 );
+         // limit order is still there
+         BOOST_CHECK( db.find<limit_order_object>( sell_id ) );
+
+         // go beyond hard fork 890
+         blocks += generate_blocks(HARDFORK_CORE_868_890_TIME - mi, true, skip);
+         blocks += generate_blocks(db.get_dynamic_global_properties().next_maintenance_time, true, skip);
+      }
+
+      // after hard fork 890, if it's before hard fork 935
+      if( db.get_dynamic_global_properties().next_maintenance_time <= HARDFORK_CORE_935_TIME )
+      {
+         // median should have changed
+         BOOST_CHECK( usd_id(db).bitasset_data(db).current_feed.settlement_price == current_feed.settlement_price );
+         BOOST_CHECK_EQUAL( usd_id(db).bitasset_data(db).current_feed.maintenance_collateral_ratio, 3500 );
+         // but the limit order is still there, because `check_call_order` was incorrectly skipped
+         BOOST_CHECK( db.find<limit_order_object>( sell_id ) );
+
+         // go beyond hard fork 935
+         blocks += generate_blocks(HARDFORK_CORE_935_TIME - mi, true, skip);
+         blocks += generate_blocks(db.get_dynamic_global_properties().next_maintenance_time, true, skip);
+      }
+
+      // after hard fork 935, the limit order should be filled
+      {
+         // median MCR should be 350%
+         BOOST_CHECK( usd_id(db).bitasset_data(db).current_feed.settlement_price == current_feed.settlement_price );
+         BOOST_CHECK_EQUAL( usd_id(db).bitasset_data(db).current_feed.maintenance_collateral_ratio, 3500 );
+         // the limit order is still there, because `check_call_order` is skipped
+         BOOST_CHECK( !db.find<limit_order_object>( sell_id ) );
+         if( db.find<limit_order_object>( sell_id ) )
+         {
+            idump( (sell_id(db)) );
+         }
+      }
+
+
+      // undo above tx's and reset
+      generate_block( skip );
+      ++blocks;
+      while( blocks > 0 )
+      {
+         db.pop_block();
+         --blocks;
+      }
+   }
+} FC_LOG_AND_RETHROW() }
+
 /*****
  * @brief make sure feeds work correctly after changing from non-witness-fed to witness-fed before the 868 fork
  * NOTE: This test case is a different issue than what is currently being worked on, and fails. Hopefully it