Merge branch 'main' into GH-986-retry-with-oc

libraries/chain/controller.cpp

@@ -5325,7 +5325,6 @@ transaction_trace_ptr controller::push_transaction( const transaction_metadata_p
                                                     uint32_t billed_cpu_time_us, bool explicit_billed_cpu_time,
                                                     int64_t subjective_cpu_bill_us ) {
    validate_db_available_size();
-   EOS_ASSERT( get_read_mode() != db_read_mode::IRREVERSIBLE, transaction_type_exception, "push transaction not allowed in irreversible mode" );
    EOS_ASSERT( trx && !trx->implicit() && !trx->scheduled(), transaction_type_exception, "Implicit/Scheduled transaction not allowed" );
    return my->push_transaction(trx, block_deadline, max_transaction_time, billed_cpu_time_us, explicit_billed_cpu_time, subjective_cpu_bill_us );
 }
@@ -5334,7 +5333,6 @@ transaction_trace_ptr controller::push_scheduled_transaction( const transaction_
                                                               fc::time_point block_deadline, fc::microseconds max_transaction_time,
                                                               uint32_t billed_cpu_time_us, bool explicit_billed_cpu_time )
 {
-   EOS_ASSERT( get_read_mode() != db_read_mode::IRREVERSIBLE, transaction_type_exception, "push scheduled transaction not allowed in irreversible mode" );
    validate_db_available_size();
    return my->push_scheduled_transaction( trxid, block_deadline, max_transaction_time, billed_cpu_time_us, explicit_billed_cpu_time );
 }

libraries/chain/fork_database.cpp

@@ -192,10 +192,11 @@ namespace eosio::chain {
 
    template<class BSP>
    void fork_database_impl<BSP>::reset_root_impl( const bsp_t& root_bsp ) {
-      index.clear();
       assert(root_bsp);
       root = root_bsp;
       root->set_valid(true);
+      pending_savanna_lib_id = block_id_type{};
+      index.clear();
    }
 
    template<class BSP>

plugins/chain_plugin/chain_plugin.cpp

@@ -942,12 +942,6 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) {
       }
       api_accept_transactions = options.at( "api-accept-transactions" ).as<bool>();
 
-      if( chain_config->read_mode == db_read_mode::IRREVERSIBLE ) {
-         if( api_accept_transactions ) {
-            api_accept_transactions = false;
-            wlog( "api-accept-transactions set to false due to read-mode: irreversible" );
-         }
-      }
       if( api_accept_transactions ) {
          enable_accept_transactions();
       }
@@ -1138,8 +1132,6 @@ void chain_plugin::plugin_initialize(const variables_map& options) {
 
 void chain_plugin_impl::plugin_startup()
 { try {
-   EOS_ASSERT( chain_config->read_mode != db_read_mode::IRREVERSIBLE || !accept_transactions, plugin_config_exception,
-               "read-mode = irreversible. transactions should not be enabled by enable_accept_transactions" );
    try {
       auto shutdown = [](){ return app().quit(); };
       auto check_shutdown = [](){ return app().is_quiting(); };

plugins/net_plugin/net_plugin.cpp

@@ -4277,14 +4277,7 @@ namespace eosio {
 
          chain_id = chain_plug->get_chain_id();
          fc::rand_pseudo_bytes( node_id.data(), node_id.data_size());
-         const controller& cc = chain_plug->chain();
 
-         if( cc.get_read_mode() == db_read_mode::IRREVERSIBLE ) {
-            if( p2p_accept_transactions ) {
-               p2p_accept_transactions = false;
-               fc_wlog( logger, "p2p-accept-transactions set to false due to read-mode: irreversible" );
-            }
-         }
          if( p2p_accept_transactions ) {
             chain_plug->enable_accept_transactions();
          }

plugins/producer_plugin/producer_plugin.cpp

@@ -687,6 +687,7 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
    std::map<chain::public_key_type, signature_provider_type> _signature_providers;
    chain::bls_pub_priv_key_map_t                     _finalizer_keys; // public, private
    std::set<chain::account_name>                     _producers;
+   chain::db_read_mode                               _db_read_mode = db_read_mode::HEAD;
    boost::asio::deadline_timer                       _timer;
    block_timing_util::producer_watermarks            _producer_watermarks;
    pending_block_mode                                _pending_block_mode = pending_block_mode::speculating;
@@ -833,6 +834,10 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
       return !_producers.empty();
    }
 
+   bool irreversible_mode() const {
+      return _db_read_mode == db_read_mode::IRREVERSIBLE;
+   }
+
    void on_accepted_block(const signed_block_ptr& block, const block_id_type& id) {
       auto& chain  = chain_plug->chain();
       auto  before = _unapplied_transactions.size();
@@ -1532,10 +1537,12 @@ void producer_plugin_impl::plugin_startup() {
       dlog("producer plugin:  plugin_startup() begin");
 
       chain::controller& chain = chain_plug->chain();
-      EOS_ASSERT(!is_configured_producer() || chain.get_read_mode() != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception,
+      _db_read_mode = chain.get_read_mode();
+
+      EOS_ASSERT(!is_configured_producer() || !irreversible_mode(), plugin_config_exception,
                  "node cannot have any producer-name configured because block production is impossible when read_mode is \"irreversible\"");
 
-      EOS_ASSERT(_finalizer_keys.empty() || chain.get_read_mode() != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception,
+      EOS_ASSERT(_finalizer_keys.empty() || !irreversible_mode(), plugin_config_exception,
                  "node cannot have any finalizers configured because finalization is impossible when read_mode is \"irreversible\"");
 
       EOS_ASSERT(!is_configured_producer() || chain.get_validation_mode() == chain::validation_mode::FULL, plugin_config_exception,
@@ -1587,6 +1594,10 @@ void producer_plugin_impl::plugin_startup() {
          _irreversible_block_time = fc::time_point::maximum();
       }
 
+      if (!_is_savanna_active && irreversible_mode() && chain_plug->accept_transactions()) {
+         wlog("Legacy consensus active. Accepting speculative transaction execution not recommended in read-mode=irreversible");
+      }
+
       if (is_configured_producer()) {
          ilog("Launching block production for ${n} producers at ${time}.", ("n", _producers.size())("time", fc::time_point::now()));
 
@@ -1978,8 +1989,11 @@ bool producer_plugin_impl::should_interrupt_start_block(const fc::time_point& de
    if (in_producing_mode()) {
       return deadline <= fc::time_point::now();
    }
-   // if we can produce then honor deadline so production starts on time
-   return (is_configured_producer() && deadline <= fc::time_point::now()) || (_received_block >= pending_block_num);
+   // if we can produce then honor deadline so production starts on time.
+   // if in irreversible mode then a received block should not interrupt since the incoming block is not processed until
+   // it becomes irreversible. We could check if LIB changed, but doesn't seem like the extra complexity is worth it.
+   return (is_configured_producer() && deadline <= fc::time_point::now())
+          || (!irreversible_mode() && _received_block >= pending_block_num);
 }
 
 producer_plugin_impl::start_block_result producer_plugin_impl::start_block() {
@@ -2081,7 +2095,7 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() {
       // Determine if we are syncing: if we have recently started an old block then assume we are syncing
       if (last_start_block_time < now + fc::microseconds(config::block_interval_us)) {
          auto head_block_age = now - chain.head().block_time();
-         if (head_block_age > fc::seconds(5))
+         if (head_block_age > fc::minutes(5))
             return start_block_result::waiting_for_block; // if syncing no need to create a block just to immediately abort it
       }
       last_start_block_time = now;

tests/CMakeLists.txt

@@ -228,8 +228,6 @@ set_property(TEST get_account_test PROPERTY LABELS nonparallelizable_tests)
 
 add_test(NAME distributed-transactions-test COMMAND tests/distributed-transactions-test.py -d 2 -p 4 -n 6 -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
 set_property(TEST distributed-transactions-test PROPERTY LABELS nonparallelizable_tests)
-add_test(NAME distributed-transactions-speculative-test COMMAND tests/distributed-transactions-test.py -d 2 -p 4 -n 6 --speculative -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
-set_property(TEST distributed-transactions-speculative-test PROPERTY LABELS nonparallelizable_tests)
 add_test(NAME distributed-transactions-if-test COMMAND tests/distributed-transactions-test.py -d 2 -p 4 -n 6 --activate-if -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
 set_property(TEST distributed-transactions-if-test PROPERTY LABELS nonparallelizable_tests)
 add_test(NAME restart-scenarios-test-resync COMMAND tests/restart-scenarios-test.py -c resync -p4 -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})

tests/TestHarness/Node.py

@@ -685,15 +685,45 @@ def linesInLog(self, searchStr):
                         lines.append(line)
         return lines
 
-    def countInLog(self, searchStr) -> int:
+    # Verfify that in during synching, unlinkable blocks are expected if
+    # the number of each group of consecutive unlinkable blocks is less than sync fetch span
+    def verifyUnlinkableBlocksExpected(self, syncFetchSpan) -> bool:
         dataDir=Utils.getNodeDataDir(self.nodeId)
         files=Node.findStderrFiles(dataDir)
-        count = 0
+
+        # A sample of unique line of unlinkable_block in logging file looks like:
+        # debug 2024-11-06T16:28:21.216 net-0 net_plugin.cpp:3744 operator() unlinkable_block 144 : 0000009063379d966646fede5662c76c970dd53ea3a3a38d4311625b72971b07, previous 143 : 0000008f172a24dd573825702ff7bdeec92ea6c2c3b22a5303a27cc367ee5a52
+        pattern = re.compile(r"unlinkable_block\s(\d+)")
+
         for file in files:
+            blocks = []
             with open(file, 'r') as f:
-                contents = f.read()
-                count += contents.count(searchStr)
-        return count
+                for line in f:
+                    match = pattern.search(line)
+                    if match:
+                        try:
+                            blockNum = int(match.group(1))
+                            blocks.append(blockNum)
+                        except ValueError:
+                            Utils.Print(f"unlinkable block number cannot be converted into integer: in {line.strip()} of {f}")
+                            return False
+                blocks.sort() # blocks from multiple connections might be out of order
+                Utils.Print(f"Unlinkable blocks: {blocks}")
+                numConsecutiveUnlinkableBlocks = 0 if len(blocks) == 0 else 1 # numConsecutiveUnlinkableBlocks is at least 1 if len(blocks) > 0
+                for i in range(1, len(blocks)):
+                    if blocks[i] == blocks[i - 1] or blocks[i] == blocks[i - 1] + 1: # look for consecutive blocks, including duplicate
+                        if blocks[i] == blocks[i - 1] + 1: # excluding duplicate
+                            ++numConsecutiveUnlinkableBlocks
+                    else: # start a new group of consecutive blocks
+                        if numConsecutiveUnlinkableBlocks > syncFetchSpan:
+                            Utils.Print(f"the number of a group of unlinkable blocks {numConsecutiveUnlinkableBlocks} greater than syncFetchSpan {syncFetchSpan} in {f}")
+                            return False
+                        numConsecutiveUnlinkableBlocks = 1
+        if numConsecutiveUnlinkableBlocks > syncFetchSpan:
+            Utils.Print(f"the number of a group of unlinkable blocks {numConsecutiveUnlinkableBlocks} greater than syncFetchSpan {syncFetchSpan} in {f}")
+            return False
+        else:
+            return True
 
     # Verify that we have only one "Starting block" in the log for any block number unless:
     # - the block was restarted because it was exhausted,

tests/distributed-transactions-test.py

@@ -12,7 +12,7 @@
 # Performs currency transfers between N accounts sent to http endpoints of
 # N nodes and verifies, after a steady state is reached, that the accounts
 # balances are correct
-# if called with --nodes-file it will will load a json description of nodes
+# if called with --nodes-file it will load a json description of nodes
 # that are already running and run distributed test against them (not
 # currently testing this feature)
 #
@@ -22,20 +22,19 @@
 errorExit=Utils.errorExit
 
 appArgs = AppArgs()
-extraArgs = appArgs.add_bool(flag="--speculative", help="Run nodes in read-mode=speculative")
-args=TestHelper.parse_args({"-p","-n","-d","-s","--nodes-file","--seed", "--speculative", "--activate-if"
+args=TestHelper.parse_args({"-p","-n","-d","-s","--nodes-file","--seed", "--activate-if"
                            ,"--dump-error-details","-v","--leave-running","--keep-logs","--unshared"}, applicationSpecificArgs=appArgs)
 
 pnodes=args.p
 topo=args.s
 delay=args.d
 total_nodes = pnodes if args.n < pnodes else args.n
+total_nodes = total_nodes if total_nodes > pnodes + 3 else pnodes + 3
 debug=args.v
 nodesFile=args.nodes_file
 dontLaunch=nodesFile is not None
 seed=args.seed
 dumpErrorDetails=args.dump_error_details
-speculative=args.speculative
 activateIF=args.activate_if
 
 Utils.Debug=debug
@@ -64,11 +63,13 @@
                (pnodes, total_nodes-pnodes, topo, delay))
 
         Print("Stand up cluster")
-        extraNodeosArgs = ""
-        if speculative:
-           extraNodeosArgs = " --read-mode speculative "
+        specificExtraNodeosArgs = {}
+        specificExtraNodeosArgs[total_nodes-1] = f' --read-mode head '
+        if activateIF: # irreversible mode speculative trx execution not recommended in legacy mode
+            specificExtraNodeosArgs[total_nodes-2] = f' --read-mode irreversible '
+        specificExtraNodeosArgs[total_nodes-3] = f' --read-mode speculative '
 
-        if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, topo=topo, delay=delay, extraNodeosArgs=extraNodeosArgs, activateIF=activateIF) is False:
+        if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, topo=topo, delay=delay, specificExtraNodeosArgs=specificExtraNodeosArgs, activateIF=activateIF) is False:
             errorExit("Failed to stand up eos cluster.")
 
         Print ("Wait for Cluster stabilization")

tests/nodeos_startup_catchup.py

@@ -253,12 +253,8 @@ def waitForNodeStarted(node):
         # See https://github.com/AntelopeIO/spring/issues/81 for fix to reduce the number of expected unlinkable blocks
         # Test verifies LIB is advancing, check to see that not too many unlinkable block exceptions are generated
         # while syncing up to head.
-        numUnlinkable = catchupNode.countInLog("unlinkable_block")
-        numUnlinkableAllowed = 500
-        Print(f"Node{catchupNodeNum} has {numUnlinkable} unlinkable_block in {catchupNode.data_dir}")
-        if numUnlinkable > numUnlinkableAllowed:
-            errorExit(f"Node{catchupNodeNum} has {numUnlinkable} which is more than the configured "
-                      f"allowed {numUnlinkableAllowed} unlinkable blocks: {catchupNode.data_dir}.")
+        if not catchupNode.verifyUnlinkableBlocksExpected(sync_fetch_span):
+            errorExit(f"unlinkable blocks are not expected") # details already logged in verifyUnlinkableBlocksExpected
 
     testSuccessful=True