move confirm block count to the start_block call, make tester track its own...

move confirm block count to the start_block call, make tester track its own watermarks the same way any producer plugin would, fix off-by-one caused by not accounting for a block confirming itself; fix off-by-one caused by propagating the value of dpos-lib before it was updated; decoupled dpos-lib from bft-lib though controller still returns the max of them

libraries/chain/block_header_state.cpp

@@ -85,20 +85,16 @@ namespace eosio { namespace chain {
     result.block_num                             = block_num + 1;
     result.producer_to_last_produced             = producer_to_last_produced;
     result.producer_to_last_produced[prokey.producer_name] = result.block_num;
-//    result.dpos_last_irreversible_blocknum       = result.calc_dpos_last_irreversible();
+//    result.dpos_irreversible_blocknum       = result.calc_dpos_last_irreversible();
     result.blockroot_merkle = blockroot_merkle;
     result.blockroot_merkle.append( id );
 
     auto block_mroot = result.blockroot_merkle.get_root();
 
-    result.active_schedule  = active_schedule;
-    result.pending_schedule = pending_schedule;
-    result.dpos2_lib        = dpos2_lib;
-
-    result.dpos_last_irreversible_blocknum = dpos2_lib;
-    result.bft_irreversible_blocknum             =
-                std::max(bft_irreversible_blocknum,result.dpos_last_irreversible_blocknum);
-
+    result.active_schedule                   = active_schedule;
+    result.pending_schedule                  = pending_schedule;
+    result.dpos_irreversible_blocknum        = dpos_irreversible_blocknum;
+    result.bft_irreversible_blocknum         = bft_irreversible_blocknum;
 
     /// grow the confirmed count
     if( confirm_count.size() < 1024 ) {
@@ -114,7 +110,7 @@ namespace eosio { namespace chain {
 
 
     if( result.pending_schedule.producers.size() &&
-        result.dpos_last_irreversible_blocknum >= pending_schedule_lib_num ) {
+        result.dpos_irreversible_blocknum >= pending_schedule_lib_num ) {
       result.active_schedule = move( result.pending_schedule );
 
       flat_map<account_name,uint32_t> new_producer_to_last_produced;
@@ -123,7 +119,7 @@ namespace eosio { namespace chain {
         if( existing != producer_to_last_produced.end() ) {
           new_producer_to_last_produced[pro.producer_name] = existing->second;
         } else {
-          new_producer_to_last_produced[pro.producer_name] = result.dpos_last_irreversible_blocknum;
+          new_producer_to_last_produced[pro.producer_name] = result.dpos_irreversible_blocknum;
         }
       }
       result.producer_to_last_produced = move( new_producer_to_last_produced );
@@ -202,23 +198,28 @@ namespace eosio { namespace chain {
      */
      header.confirmed = num_prev_blocks;
 
-     int32_t i = confirm_count.size() - 2;
-     while( i >= 0 && num_prev_blocks ) {
+     auto i = confirm_count.size() - 1;
+     uint32_t blocks_to_confirm = num_prev_blocks + 1; /// confirm the head block too
+     while( i >= 0 && blocks_to_confirm ) {
         ++confirm_count[i];
         //idump((confirm_count[i]));
         if( confirm_count[i] > active_schedule.producers.size()*2/3 )
         {
-           uint32_t block_num_for_i = block_num - (confirm_count.size() - 1 - i);
-           dpos2_lib = block_num_for_i;
-           idump((dpos2_lib)(block_num)(dpos_last_irreversible_blocknum));
+           uint32_t block_num_for_i = block_num - (uint32_t)(confirm_count.size() - 1 - i);
+           dpos_irreversible_blocknum = block_num_for_i;
+           //idump((dpos2_lib)(block_num)(dpos_irreversible_blocknum));
+
+           if (i == confirm_count.size() - 1) {
+              confirm_count.resize(0);
+           } else {
+              memmove( &confirm_count[0], &confirm_count[i + 1], confirm_count.size() - i  - 1);
+              confirm_count.resize( confirm_count.size() - i - 1 );
+           }
 
-           memmove( &confirm_count[0], &confirm_count[i], confirm_count.size() -i );
-      //     wdump((confirm_count.size()-i));
-           confirm_count.resize( confirm_count.size() - i );
            return;
         }
         --i;
-        --num_prev_blocks;
+        --blocks_to_confirm;
      }
   }
 

libraries/chain/controller.cpp

@@ -626,7 +626,7 @@ struct controller_impl {
    } FC_CAPTURE_AND_RETHROW() } /// push_transaction
 
 
-   void start_block( block_timestamp_type when ) {
+   void start_block( block_timestamp_type when, uint16_t confirm_block_count ) {
       FC_ASSERT( !pending );
 
       FC_ASSERT( db.revision() == head->block_num, "",
@@ -640,7 +640,7 @@ struct controller_impl {
 
       const auto& gpo = db.get<global_property_object>();
       if( gpo.proposed_schedule_block_num.valid() && // if there is a proposed schedule that was proposed in a block ...
-          ( *gpo.proposed_schedule_block_num <= pending->_pending_block_state->dpos_last_irreversible_blocknum ) && // ... that has now become irreversible ...
+          ( *gpo.proposed_schedule_block_num <= pending->_pending_block_state->dpos_irreversible_blocknum ) && // ... that has now become irreversible ...
           pending->_pending_block_state->pending_schedule.producers.size() == 0 && // ... and there is room for a new pending schedule ...
           head->pending_schedule.producers.size() == 0 // ... and not just because it was promoted to active at the start of this block, then:
         )
@@ -648,7 +648,7 @@ struct controller_impl {
          // Promote proposed schedule to pending schedule.
          ilog( "promoting proposed schedule (set in block ${proposed_num}) to pending; current block: ${n} lib: ${lib} schedule: ${schedule} ",
                ("proposed_num", *gpo.proposed_schedule_block_num)("n", pending->_pending_block_state->block_num)
-               ("lib", pending->_pending_block_state->dpos_last_irreversible_blocknum)
+               ("lib", pending->_pending_block_state->dpos_irreversible_blocknum)
                ("schedule", static_cast<producer_schedule_type>(gpo.proposed_schedule) ) );
          pending->_pending_block_state->set_new_producers( gpo.proposed_schedule );
          db.modify( gpo, [&]( auto& gp ) {
@@ -657,6 +657,8 @@ struct controller_impl {
          });
       }
 
+      pending->_pending_block_state->set_confirmed(confirm_block_count);
+
       try {
          auto onbtrx = std::make_shared<transaction_metadata>( get_on_block_transaction() );
          push_transaction( onbtrx, fc::time_point::maximum(), true );
@@ -678,8 +680,7 @@ struct controller_impl {
 
    void apply_block( const signed_block_ptr& b ) { try {
       try {
-         start_block( b->timestamp );
-         self.pending_block_state()->set_confirmed( b->confirmed );
+         start_block( b->timestamp, b->confirmed );
 
          for( const auto& receipt : b->transactions ) {
             if( receipt.trx.contains<packed_transaction>() ) {
@@ -835,7 +836,7 @@ struct controller_impl {
             ("p",pending->_pending_block_state->header.producer)
             ("signing_key", pending->_pending_block_state->block_signing_key)
             ("v",pending->_pending_block_state->header.schedule_version)
-            ("lib",pending->_pending_block_state->dpos_last_irreversible_blocknum)
+            ("lib",pending->_pending_block_state->dpos_irreversible_blocknum)
             ("ndtrxs",db.get_index<generated_transaction_multi_index,by_trx_id>().size())
             ("np",pending->_pending_block_state->header.new_producers)
             );
@@ -1004,8 +1005,8 @@ void controller::startup() {
 chainbase::database& controller::db()const { return my->db; }
 
 
-void controller::start_block( block_timestamp_type when ) {
-   my->start_block(when);
+void controller::start_block( block_timestamp_type when, uint16_t confirm_block_count ) {
+   my->start_block(when, confirm_block_count);
 }
 
 void controller::finalize_block() {
@@ -1098,11 +1099,11 @@ time_point controller::pending_block_time()const {
 }
 
 uint32_t controller::last_irreversible_block_num() const {
-   return my->head->bft_irreversible_blocknum;
+   return std::max(my->head->bft_irreversible_blocknum, my->head->dpos_irreversible_blocknum);
 }
 
 block_id_type controller::last_irreversible_block_id() const {
-   auto lib_num = my->head->bft_irreversible_blocknum;
+   auto lib_num = last_irreversible_block_num();
    const auto& tapos_block_summary = db().get<block_summary_object>((uint16_t)lib_num);
 
    if( block_header::num_from_id(tapos_block_summary.block_id) == lib_num )
@@ -1132,7 +1133,7 @@ void controller::log_irreversible_blocks() {
       my->blog.read_head();
 
    const auto& log_head = my->blog.head();
-   auto lib = my->head->dpos_last_irreversible_blocknum;
+   auto lib = my->head->dpos_irreversible_blocknum;
 
 
    if( lib > 2 ) {

libraries/chain/fork_database.cpp

@@ -32,7 +32,7 @@ namespace eosio { namespace chain {
          >,
          ordered_non_unique< tag<by_lib_block_num>,
             composite_key< block_header_state,
-                member<block_header_state,uint32_t,&block_header_state::dpos_last_irreversible_blocknum>,
+                member<block_header_state,uint32_t,&block_header_state::dpos_irreversible_blocknum>,
                 member<block_header_state,uint32_t,&block_header_state::bft_irreversible_blocknum>,
                 member<block_header_state,uint32_t,&block_header_state::block_num>
             >,
@@ -98,7 +98,7 @@ namespace eosio { namespace chain {
       /// we cannot normally prune the lib if it is the head block because
       /// the next block needs to build off of the head block. We are exiting
       /// now so we can prune this block as irreversible before exiting.
-      auto lib    = my->head->dpos_last_irreversible_blocknum;
+      auto lib    = my->head->dpos_irreversible_blocknum;
       auto oldest = *my->index.get<by_block_num>().begin();
       if( oldest->block_num <= lib ) {
          prune( oldest );
@@ -131,7 +131,7 @@ namespace eosio { namespace chain {
 
       my->head = *my->index.get<by_lib_block_num>().begin();
 
-      auto lib    = my->head->dpos_last_irreversible_blocknum;
+      auto lib    = my->head->dpos_irreversible_blocknum;
       auto oldest = *my->index.get<by_block_num>().begin();
 
       if( oldest->block_num < lib ) {

libraries/chain/include/eosio/chain/block_header_state.hpp

@@ -12,7 +12,8 @@ struct block_header_state {
     block_id_type                     id;
     uint32_t                          block_num = 0;
     signed_block_header               header;
-    uint32_t                          dpos_last_irreversible_blocknum = 0;
+    uint32_t                          dpos_irreversible_blocknum = 0;
+    uint32_t                          bft_irreversible_blocknum = 0;
     uint32_t                          pending_schedule_lib_num = 0; /// last irr block num
     digest_type                       pending_schedule_hash;
     producer_schedule_type            pending_schedule;
@@ -22,10 +23,6 @@ struct block_header_state {
     public_key_type                   block_signing_key;
     vector<uint8_t>                   confirm_count;
 
-
-    uint32_t                     bft_irreversible_blocknum = 0;
-    uint32_t                     dpos2_lib = 0;
-
     block_header_state   next( const signed_block_header& h )const;
     block_header_state   generate_next( block_timestamp_type when )const;
 
@@ -60,7 +57,7 @@ struct block_header_state {
 } } /// namespace eosio::chain
 
 FC_REFLECT( eosio::chain::block_header_state,
-            (id)(block_num)(header)(dpos_last_irreversible_blocknum)
+            (id)(block_num)(header)(dpos_irreversible_blocknum)
             (pending_schedule_lib_num)(pending_schedule_hash)
             (pending_schedule)(active_schedule)(blockroot_merkle)
             (producer_to_last_produced)(block_signing_key)

libraries/chain/include/eosio/chain/controller.hpp

@@ -59,7 +59,7 @@ namespace eosio { namespace chain {
           * Starts a new pending block session upon which new transactions can
           * be pushed.
           */
-         void start_block( block_timestamp_type time = block_timestamp_type() );
+         void start_block( block_timestamp_type time = block_timestamp_type(), uint16_t confirm_block_count = 0 );
 
          void  abort_block();
 

libraries/testing/include/eosio/testing/tester.hpp

@@ -221,6 +221,7 @@ namespace eosio { namespace testing {
 
       protected:
          signed_block_ptr _produce_block( fc::microseconds skip_time, bool skip_pending_trxs = false, uint32_t skip_flag = 0 );
+         void             _start_block(fc::time_point block_time);
 
       // Fields:
       protected:
@@ -232,6 +233,7 @@ namespace eosio { namespace testing {
       protected:
          controller::config                            cfg;
          map<transaction_id_type, transaction_receipt> chain_transactions;
+         map<account_name, block_id_type>              last_produced_block;
    };
 
    class tester : public base_tester {

libraries/testing/tester.cpp

@@ -85,6 +85,12 @@ namespace eosio { namespace testing {
    signed_block_ptr base_tester::push_block(signed_block_ptr b) {
       control->abort_block();
       control->push_block(b);
+
+      auto itr = last_produced_block.find(b->producer);
+      if (itr == last_produced_block.end() || block_header::num_from_id(b->id()) > block_header::num_from_id(itr->second)) {
+         last_produced_block[b->producer] = b->id();
+      }
+
       return b;
    }
 
@@ -93,12 +99,8 @@ namespace eosio { namespace testing {
       auto head_time = control->head_block_time();
       auto next_time = head_time + skip_time;
 
-      if( !control->pending_block_state() ) {
-         control->start_block( next_time );
-      } else if( control->pending_block_state()->header.timestamp != next_time ) {
-         wlog( "aborting current pending block and starting a new one" );
-         control->abort_block();
-         control->start_block( next_time );
+      if( !control->pending_block_state() || control->pending_block_state()->header.timestamp != next_time ) {
+         _start_block( next_time );
       }
 
       auto producer = control->head_block_state()->get_scheduled_producer(next_time);
@@ -119,16 +121,8 @@ namespace eosio { namespace testing {
             while( control->push_next_scheduled_transaction( fc::time_point::maximum() ) );
       }
 
-      auto hb = control->head_block_state();
-      auto pb = control->pending_block_state();
-      const auto& lpp_map = hb->producer_to_last_produced;
-      auto pitr = lpp_map.find( pb->header.producer );
-      if( pitr != lpp_map.end() ) {
-         if( pb->block_num == pitr->second ) {
-            wdump((pb->block_num));
-         }
-         control->pending_block_state()->set_confirmed( pb->block_num - pitr->second );
-      }
+
+
       control->finalize_block();
       control->sign_block( [&]( digest_type d ) {
                     return priv_key.sign(d);
@@ -136,10 +130,27 @@ namespace eosio { namespace testing {
 
       control->commit_block();
       control->log_irreversible_blocks();
-      control->start_block( next_time + fc::microseconds(config::block_interval_us));
+      last_produced_block[control->head_block_state()->header.producer] = control->head_block_state()->id;
+
+      _start_block( next_time + fc::microseconds(config::block_interval_us));
       return control->head_block_state()->block;
    }
 
+   void base_tester::_start_block(fc::time_point block_time) {
+      auto head_block_number = control->head_block_num();
+      auto producer = control->head_block_state()->get_scheduled_producer(block_time);
+
+      auto last_produced_block_num = control->last_irreversible_block_num();
+      auto itr = last_produced_block.find(producer.producer_name);
+      if (itr != last_produced_block.end()) {
+         last_produced_block_num = block_header::num_from_id(itr->second);
+      }
+
+      control->abort_block();
+      control->start_block( block_time, head_block_number - last_produced_block_num );
+   }
+
+
    void base_tester::produce_blocks( uint32_t n, bool empty ) {
       if( empty ) {
          for( uint32_t i = 0; i < n; ++i )
@@ -196,7 +207,7 @@ namespace eosio { namespace testing {
 
    transaction_trace_ptr base_tester::push_transaction( packed_transaction& trx, uint32_t skip_flag, fc::time_point deadline ) { try {
       if( !control->pending_block_state() )
-         control->start_block();
+         _start_block(control->head_block_time() + fc::microseconds(config::block_interval_us));
       auto r = control->sync_push( std::make_shared<transaction_metadata>(trx), deadline );
       if( r->hard_except_ptr ) std::rethrow_exception( r->hard_except_ptr );
       if( r->soft_except_ptr ) std::rethrow_exception( r->soft_except_ptr );
@@ -207,7 +218,7 @@ namespace eosio { namespace testing {
 
    transaction_trace_ptr base_tester::push_transaction( signed_transaction& trx, uint32_t skip_flag, fc::time_point deadline ) { try {
       if( !control->pending_block_state() )
-         control->start_block();
+         _start_block(control->head_block_time() + fc::microseconds(config::block_interval_us));
       auto r = control->sync_push( std::make_shared<transaction_metadata>(trx), deadline );
       if( r->hard_except_ptr ) std::rethrow_exception( r->hard_except_ptr );
       if( r->soft_except_ptr ) std::rethrow_exception( r->soft_except_ptr );

plugins/producer_plugin/producer_plugin.cpp

@@ -66,8 +66,12 @@ class producer_plugin_impl {
 
          const auto& active_producer_to_signing_key = bsp->active_schedule.producers;
 
-         auto active_producers = boost::adaptors::keys( boost::make_iterator_range( bsp->producer_to_last_produced.begin(),
-                                                                                    bsp->producer_to_last_produced.end()   ) );
+         flat_set<account_name> active_producers;
+         active_producers.reserve(bsp->active_schedule.producers.size());
+         for (const auto& p: bsp->active_schedule.producers) {
+            active_producers.insert(p.producer_name);
+         }
+
          std::set_intersection( _producers.begin(), _producers.end(),
                                 active_producers.begin(), active_producers.end(),
                                 boost::make_function_output_iterator( [&]( const chain::account_name& producer )

unittests/database_tests.cpp

@@ -78,7 +78,7 @@ BOOST_AUTO_TEST_SUITE(database_tests)
 
          // Check the last irreversible block number is set correctly
          const auto expected_last_irreversible_block_number = calc_exp_last_irr_block_num(num_of_blocks_to_prod) + 1;
-         BOOST_TEST(test.control->head_block_state()->dpos_last_irreversible_blocknum == expected_last_irreversible_block_number);
+         BOOST_TEST(test.control->head_block_state()->dpos_irreversible_blocknum == expected_last_irreversible_block_number);
          // Check that block 201 cannot be found (only 20 blocks exist)
          BOOST_TEST(test.control->fetch_block_by_number(num_of_blocks_to_prod + 1 + 1) == nullptr);
 
@@ -89,7 +89,7 @@ BOOST_AUTO_TEST_SUITE(database_tests)
          const auto next_expected_last_irreversible_block_number = calc_exp_last_irr_block_num(
                  num_of_blocks_to_prod + next_num_of_blocks_to_prod) + 1;
          // Check the last irreversible block number is updated correctly
-         BOOST_TEST(test.control->head_block_state()->dpos_last_irreversible_blocknum == next_expected_last_irreversible_block_number);
+         BOOST_TEST(test.control->head_block_state()->dpos_irreversible_blocknum == next_expected_last_irreversible_block_number);
          // Check that block 201 can now be found
          BOOST_CHECK_NO_THROW(test.control->fetch_block_by_number(num_of_blocks_to_prod + 1));
          // Check the latest head block match

unittests/forked_tests.cpp

@@ -82,8 +82,7 @@ BOOST_AUTO_TEST_CASE( forking ) try {
    wlog( "push c1 blocks to c2" );
    while( c2.control->head_block_num() < c.control->head_block_num() ) {
       auto fb = c.control->fetch_block_by_number( c2.control->head_block_num()+1 );
-      c2.control->abort_block();
-      c2.control->push_block( fb );
+      c2.push_block( fb );
    }
    wlog( "end push c1 blocks to c2" );
 
@@ -108,8 +107,7 @@ BOOST_AUTO_TEST_CASE( forking ) try {
    wlog( "push c1 blocks to c2" );
    while( c2.control->head_block_num() < c.control->head_block_num() ) {
       auto fb = c.control->fetch_block_by_number( c2.control->head_block_num()+1 );
-      c2.control->abort_block();
-      c2.control->push_block( fb );
+      c2.push_block( fb );
    }
    wlog( "end push c1 blocks to c2" );
 
@@ -135,9 +133,9 @@ BOOST_AUTO_TEST_CASE( forking ) try {
    // dan on chain 1 now gets all of the blocks from chain 2 which should cause fork switch
    wlog( "push c2 blocks to c1" );
    for( uint32_t start = fork_block_num + 1, end = c2.control->head_block_num(); start <= end; ++start ) {
+      wdump((start));
       auto fb = c2.control->fetch_block_by_number( start );
-      c.control->abort_block();
-      c.control->push_block( fb );
+      c.push_block( fb );
    }
    wlog( "end push c2 blocks to c1" );
 
@@ -156,8 +154,7 @@ BOOST_AUTO_TEST_CASE( forking ) try {
    wlog( "push c1 blocks to c2" );
    while( c2.control->head_block_num() < c.control->head_block_num() ) {
       auto fb = c.control->fetch_block_by_number( c2.control->head_block_num()+1 );
-      c2.control->abort_block();
-      c2.control->push_block( fb );
+      c2.push_block( fb );
    }
    wlog( "end push c1 blocks to c2" );
 
@@ -184,8 +181,7 @@ BOOST_AUTO_TEST_CASE( forking ) try {
    wlog( "push c2 blocks (except for the last block by dan) to c1" );
    for( uint32_t start = fork_block_num + 1, end = c2.control->head_block_num() - 1; start <= end; ++start ) {
       auto fb = c2.control->fetch_block_by_number( start );
-      c.control->abort_block();
-      c.control->push_block( fb );
+      c.push_block( fb );
    }
    wlog( "end push c2 blocks to c1" );
    wlog( "now push dan's block to c1 but first corrupt it so it is a bad block" );