fix 7600 double confirm after changing sign key

plugins/producer_plugin/producer_plugin.cpp

@@ -280,28 +280,13 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
             }
          } ) );
 
-         // since the watermark has to be set before a block is created, we are looking into the future to
-         // determine the new schedule to identify producers that have become active
-         chain::controller& chain = chain_plug->chain();
-         const auto hbn = bsp->block_num;
-         auto new_pbhs = bsp->next(bsp->header.timestamp.next(), 0);
-
-         // for newly installed producers we can set their watermarks to the block they became active
-         if( bsp->active_schedule.version != new_pbhs.active_schedule.version ) {
-            flat_set<account_name> new_producers;
-            new_producers.reserve(new_pbhs.active_schedule.producers.size());
-            for( const auto& p: new_pbhs.active_schedule.producers) {
-               if (_producers.count(p.producer_name) > 0)
-                  new_producers.insert(p.producer_name);
-            }
-
-            for( const auto& p: bsp->active_schedule.producers) {
-               new_producers.erase(p.producer_name);
-            }
-
-            for (const auto& new_producer: new_producers) {
-               _producer_watermarks[new_producer] = hbn;
-            }
+         // simplify handling of watermark in on_block
+         auto block_producer = bsp->header.producer;
+         auto watermark_itr = _producer_watermarks.find( block_producer );
+         if( watermark_itr != _producer_watermarks.end() ) {
+            watermark_itr->second = bsp->block_num;
+         } else if( _producers.count( block_producer ) > 0 ) {
+            _producer_watermarks.emplace( block_producer, bsp->block_num );
          }
       }
 
@@ -1352,9 +1337,12 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() {
          if (currrent_watermark_itr != _producer_watermarks.end()) {
             auto watermark = currrent_watermark_itr->second;
             if (watermark < hbs->block_num) {
-               blocks_to_confirm = std::min<uint16_t>(std::numeric_limits<uint16_t>::max(), (uint16_t)(hbs->block_num - watermark));
+               blocks_to_confirm = (uint16_t)(std::min<uint32_t>(std::numeric_limits<uint16_t>::max(), (uint32_t)(hbs->block_num - watermark)));
             }
          }
+
+         // can not confirm irreversible blocks
+         blocks_to_confirm = (uint16_t)(std::min<uint32_t>(blocks_to_confirm, (uint32_t)(hbs->block_num - hbs->dpos_irreversible_blocknum)));
       }
 
       chain.abort_block();

programs/eosio-launcher/main.cpp

@@ -346,7 +346,7 @@ enum allowed_connection : char {
 
 class producer_names {
 public:
-   static string producer_name(unsigned int producer_number);
+   static string producer_name(unsigned int producer_number, bool shared_producer = false);
 private:
    static const int total_chars = 12;
    static const char slot_chars[];
@@ -358,8 +358,9 @@ const char producer_names::valid_char_range = sizeof(producer_names::slot_chars)
 
 // for 26 or fewer total producers create "defproducera" .. "defproducerz"
 // above 26 produce  "defproducera" .. "defproducerz",  "defproduceaa" .. "defproducerb", etc.
-string producer_names::producer_name(unsigned int producer_number) {
+string producer_names::producer_name(unsigned int producer_number, bool shared_producer) {
    // keeping legacy "defproducer[a-z]", but if greater than valid_char_range, will use "defpraaaaaaa"
+   // shared_producer will appear in all nodes' config
    char prod_name[] = "defproducera";
    if (producer_number > valid_char_range) {
       for (int current_char_loc = 5; current_char_loc < total_chars; ++current_char_loc) {
@@ -380,6 +381,12 @@ string producer_names::producer_name(unsigned int producer_number) {
    // make sure we haven't cycled back to the first 26 names (some time after 26^6)
    if (string(prod_name) == "defproducera" && producer_number != 0)
       throw std::runtime_error( "launcher not designed to handle numbers this large " );
+
+   if (shared_producer) {
+      prod_name[0] = 's';
+      prod_name[1] = 'h';
+      prod_name[2] = 'r';
+   }
    return prod_name;
 }
 
@@ -389,6 +396,7 @@ struct launcher_def {
    size_t unstarted_nodes;
    size_t prod_nodes;
    size_t producers;
+   size_t shared_producers;
    size_t next_node;
    string shape;
    allowed_connection allowed_connections = PC_NONE;
@@ -479,7 +487,8 @@ launcher_def::set_options (bpo::options_description &cfg) {
     ("nodes,n",bpo::value<size_t>(&total_nodes)->default_value(1),"total number of nodes to configure and launch")
     ("unstarted-nodes",bpo::value<size_t>(&unstarted_nodes)->default_value(0),"total number of nodes to configure, but not launch")
     ("pnodes,p",bpo::value<size_t>(&prod_nodes)->default_value(1),"number of nodes that contain one or more producers")
-    ("producers",bpo::value<size_t>(&producers)->default_value(21),"total number of non-bios producer instances in this network")
+    ("producers",bpo::value<size_t>(&producers)->default_value(21),"total number of non-bios and non-shared producer instances in this network")
+    ("shared-producers",bpo::value<size_t>(&shared_producers)->default_value(0),"total number of shared producers on each non-bios nodes")
     ("mode,m",bpo::value<vector<string>>()->multitoken()->default_value({"any"}, "any"),"connection mode, combination of \"any\", \"producers\", \"specified\", \"none\"")
     ("shape,s",bpo::value<string>(&shape)->default_value("star"),"network topology, use \"star\" \"mesh\" or give a filename for custom")
     ("genesis,g",bpo::value<string>()->default_value("./genesis.json"),"set the path to genesis.json")
@@ -901,6 +910,11 @@ launcher_def::bind_nodes () {
                  producer_set.schedule.push_back({prodname,pubkey});
                  ++producer_number;
               }
+              for (unsigned j = 0; j < shared_producers; ++j) {
+                 const auto prodname = producer_names::producer_name(j, true);
+                 node.producers.push_back(prodname);
+                 producer_set.schedule.push_back({prodname,pubkey});                 
+              }
            }
            node.dont_start = i >= to_not_start_node;
         }

tests/CMakeLists.txt

@@ -48,6 +48,7 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/launcher_test.py ${CMAKE_CURRENT_BINA
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/db_modes_test.sh ${CMAKE_CURRENT_BINARY_DIR}/db_modes_test.sh COPYONLY)
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/prod_preactivation_test.py ${CMAKE_CURRENT_BINARY_DIR}/prod_preactivation_test.py COPYONLY)
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/version-label.sh ${CMAKE_CURRENT_BINARY_DIR}/version-label.sh COPYONLY)
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/nodeos_producer_watermark_test.py ${CMAKE_CURRENT_BINARY_DIR}/nodeos_producer_watermark_test.py COPYONLY)
 
 #To run plugin_test with all log from blockchain displayed, put --verbose after --, i.e. plugin_test -- --verbose
 add_test(NAME plugin_test COMMAND plugin_test --report_level=detailed --color_output)
@@ -118,6 +119,10 @@ add_test(NAME nodeos_multiple_version_protocol_feature_mv_test COMMAND tests/nod
          -v --clean-run --dump-error-detail --alternate-version-labels-file ${ALTERNATE_VERSION_LABELS_FILE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
 set_property(TEST nodeos_multiple_version_protocol_feature_mv_test PROPERTY LABELS mixed_version_tests)
 
+add_test(NAME nodeos_producer_watermark_lr_test COMMAND tests/nodeos_producer_watermark_test.py -v --clean-run --dump-error-detail WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
+set_property(TEST nodeos_producer_watermark_lr_test PROPERTY LABELS long_running_tests)
+
+
 if(ENABLE_COVERAGE_TESTING)
 
   set(Coverage_NAME ${PROJECT_NAME}_coverage)

tests/Cluster.py

@@ -143,7 +143,7 @@ def setAlternateVersionLabels(self, file):
     # pylint: disable=too-many-branches
     # pylint: disable=too-many-statements
     def launch(self, pnodes=1, unstartedNodes=0, totalNodes=1, prodCount=1, topo="mesh", delay=1, onlyBios=False, dontBootstrap=False,
-               totalProducers=None, extraNodeosArgs=None, useBiosBootFile=True, specificExtraNodeosArgs=None, onlySetProds=False,
+               totalProducers=None, sharedProducers=0, extraNodeosArgs=None, useBiosBootFile=True, specificExtraNodeosArgs=None, onlySetProds=False,
                pfSetupPolicy=PFSetupPolicy.FULL, alternateVersionLabelsFile=None, associatedNodeLabels=None, loadSystemContract=True):
         """Launch cluster.
         pnodes: producer nodes count
@@ -199,6 +199,9 @@ def launch(self, pnodes=1, unstartedNodes=0, totalNodes=1, prodCount=1, topo="me
             assert(isinstance(totalProducers, (str,int)))
             producerFlag="--producers %s" % (totalProducers)
 
+        if sharedProducers > 0:
+            producerFlag += (" --shared-producers %d" % (sharedProducers))
+
         self.setAlternateVersionLabels(alternateVersionLabelsFile)
 
         tries = 30
@@ -384,7 +387,7 @@ def connectGroup(group, producerNodes, bridgeNodes) :
         Cluster.__LauncherCmdArr = cmdArr.copy()
 
         s=" ".join(cmdArr)
-        if Utils.Debug: Utils.Print("cmd: %s" % (s))
+        Utils.Print("cmd: %s" % (s))
         if 0 != subprocess.call(cmdArr):
             Utils.Print("ERROR: Launcher failed to launch. failed cmd: %s" % (s))
             return False
@@ -430,7 +433,7 @@ def connectGroup(group, producerNodes, bridgeNodes) :
         if not loadSystemContract:
             useBiosBootFile=False  #ensure we use Cluster.bootstrap
         if onlyBios or not useBiosBootFile:
-            self.biosNode=self.bootstrap(biosNode, startedNodes, prodCount, totalProducers, pfSetupPolicy, onlyBios, onlySetProds, loadSystemContract)
+            self.biosNode=self.bootstrap(biosNode, startedNodes, prodCount + sharedProducers, totalProducers, pfSetupPolicy, onlyBios, onlySetProds, loadSystemContract)
             if self.biosNode is None:
                 Utils.Print("ERROR: Bootstrap failed.")
                 return False

tests/Node.py

@@ -49,6 +49,7 @@ def __init__(self, host, port, pid=None, cmd=None, walletMgr=None, enableMongo=F
         self.infoValid=None
         self.lastRetrievedHeadBlockNum=None
         self.lastRetrievedLIB=None
+        self.lastRetrievedHeadBlockProducer=""
         self.transCache={}
         self.walletMgr=walletMgr
         self.missingTransaction=False
@@ -1173,6 +1174,7 @@ def getInfo(self, silentErrors=False, exitOnError=False):
             self.infoValid=True
             self.lastRetrievedHeadBlockNum=int(info["head_block_num"])
             self.lastRetrievedLIB=int(info["last_irreversible_block_num"])
+            self.lastRetrievedHeadBlockProducer=info["head_block_producer"]
         return info
 
     def getBlockFromDb(self, idx):
@@ -1316,9 +1318,12 @@ def getBlockProducer(self, timeout=None, waitForBlock=True, exitOnError=True, bl
         return blockProducer
 
     def getNextCleanProductionCycle(self, trans):
-        transId=Node.getTransId(trans)
         rounds=21*12*2  # max time to ensure that at least 2/3+1 of producers x blocks per producer x at least 2 times
-        self.waitForTransFinalization(transId, timeout=rounds/2)
+        if trans is not None:
+            transId=Node.getTransId(trans)
+            self.waitForTransFinalization(transId, timeout=rounds/2)
+        else:
+            transId="Null"
         irreversibleBlockNum=self.getIrreversibleBlockNum()
 
         # The voted schedule should be promoted now, then need to wait for that to become irreversible