Merge pull request #10245 from EOSIO/bdj__privacy-apply-block-fix_EPE…

…-832

Fix for Security Group on nodes just applying the block that was produced

libraries/chain/controller.cpp

@@ -1442,7 +1442,9 @@ struct controller_impl {
                }
 
                ++bb._pending_block_header_state.security_group.version;
-               bb._pending_block_header_state.security_group.participants = std::move(gpo.proposed_security_group_participants);
+               bb._pending_block_header_state.security_group.participants = {
+                  gpo.proposed_security_group_participants.begin(),
+                  gpo.proposed_security_group_participants.end()};
 
                db.modify(gpo, [&](auto& gp) { 
                   gp.proposed_security_group_block_num = 0; 
@@ -1755,6 +1757,9 @@ struct controller_impl {
          if( !use_bsp_cached ) {
             bsp->set_trxs_metas( std::move( ab._trx_metas ), !skip_auth_checks );
          }
+
+         auto& pbsh = ab._pending_block_header_state;
+         bsp->set_security_group_info(std::move(pbsh.security_group));
          // create completed_block with the existing block_state as we just verified it is the same as assembled_block
          pending->_block_stage = completed_block{ bsp };
 
@@ -2260,8 +2265,9 @@ struct controller_impl {
       }
 
       flat_set<account_name> proposed_participants = gpo.proposed_security_group_block_num == 0
-                                                         ? self.active_security_group().participants
-                                                         : gpo.proposed_security_group_participants;
+                              ? self.active_security_group().participants
+                              : flat_set<account_name>{gpo.proposed_security_group_participants.begin(),
+                                                       gpo.proposed_security_group_participants.end()};
 
       auto orig_participants_size = proposed_participants.size();
 
@@ -2274,7 +2280,8 @@ struct controller_impl {
 
       db.modify(gpo, [&proposed_participants, cur_block_num](auto& gp) {
          gp.proposed_security_group_block_num    = cur_block_num;
-         gp.proposed_security_group_participants = std::move(proposed_participants);
+         gp.set_proposed_security_group_participants(proposed_participants.begin(),
+                                                     proposed_participants.end());
       });
 
       return 0;
@@ -2876,8 +2883,9 @@ const security_group_info_t& controller::active_security_group() const {
        my->pending->_block_stage);
 }
 
-const flat_set<account_name>& controller::proposed_security_group_participants() const {
-   return get_global_properties().proposed_security_group_participants;
+flat_set<account_name> controller::proposed_security_group_participants() const {
+   return {get_global_properties().proposed_security_group_participants.begin(),
+           get_global_properties().proposed_security_group_participants.end()};
 }
 
 int64_t controller::add_security_group_participants(const flat_set<account_name>& participants) {

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

@@ -294,7 +294,7 @@ namespace eosio { namespace chain {
          int64_t set_proposed_producers( vector<producer_authority> producers );
 
          const security_group_info_t& active_security_group() const;
-         const flat_set<account_name>& proposed_security_group_participants() const;
+         flat_set<account_name> proposed_security_group_participants() const;
 
          int64_t add_security_group_participants(const flat_set<account_name>& participants);
          int64_t remove_security_group_participants(const flat_set<account_name>& participants);

libraries/chain/include/eosio/chain/global_property_object.hpp

@@ -62,7 +62,7 @@ namespace eosio { namespace chain {
     */
    class global_property_object : public chainbase::object<global_property_object_type, global_property_object>
    {
-      OBJECT_CTOR(global_property_object, (proposed_schedule))
+      OBJECT_CTOR(global_property_object, (proposed_schedule)(proposed_security_group_participants)(transaction_hooks))
 
    public:
       id_type                             id;
@@ -73,8 +73,9 @@ namespace eosio { namespace chain {
       kv_database_config                  kv_configuration;
       wasm_config                         wasm_configuration;
       block_num_type                      proposed_security_group_block_num = 0;
-      flat_set<account_name>              proposed_security_group_participants;
-      vector<transaction_hook>            transaction_hooks;
+      // members that are containers need to be shared_* containers, since this object is stored in a multi-index container
+      shared_set<account_name>            proposed_security_group_participants;
+      shared_vector<transaction_hook>     transaction_hooks;
 
       void initalize_from(const legacy::snapshot_global_property_object_v2& legacy, const chain_id_type& chain_id_val,
                           const kv_database_config& kv_config_val, const wasm_config& wasm_config_val) {
@@ -103,6 +104,18 @@ namespace eosio { namespace chain {
          kv_configuration = legacy.kv_configuration;
          wasm_configuration = legacy.wasm_configuration;
       }
+
+      template<typename Iter>
+      void set_proposed_security_group_participants(Iter begin, Iter end) {
+         proposed_security_group_participants = {begin, end,
+                                                 proposed_security_group_participants.key_comp(),
+                                                 proposed_security_group_participants.get_allocator()};
+      }
+
+      template<typename Iter>
+      void set_transaction_hooks(Iter begin, Iter end) {
+         transaction_hooks = {begin, end, transaction_hooks.get_allocator()};
+      }
    };
 
 
@@ -128,10 +141,10 @@ namespace eosio { namespace chain {
       struct extension_v0 {
          // libstdc++ requires the following two constructors to work. 
          extension_v0(){};
-         extension_v0(block_num_type num, const flat_set<account_name>& participants, const vector<transaction_hook> trx_hooks)
+         extension_v0(block_num_type num, flat_set<account_name> participants, vector<transaction_hook> trx_hooks)
              : proposed_security_group_block_num(num)
-             , proposed_security_group_participants(participants)
-             , transaction_hooks(trx_hooks) {}
+             , proposed_security_group_participants(std::move(participants))
+             , transaction_hooks(std::move(trx_hooks)) {}
 
          block_num_type                      proposed_security_group_block_num = 0;
          flat_set<account_name>              proposed_security_group_participants;
@@ -153,16 +166,19 @@ namespace eosio { namespace chain {
 
    inline snapshot_global_property_object::extension_t get_gpo_extension(const global_property_object& gpo) {
       return snapshot_global_property_object::extension_v0{
-          gpo.proposed_security_group_block_num, gpo.proposed_security_group_participants, gpo.transaction_hooks};
+          gpo.proposed_security_group_block_num,
+          {gpo.proposed_security_group_participants.begin(), gpo.proposed_security_group_participants.end()},
+          {gpo.transaction_hooks.begin(), gpo.transaction_hooks.end()}};
    }
 
    inline void set_gpo_extension(global_property_object&                        gpo,
                                  snapshot_global_property_object::extension_t&& extension) {
       std::visit(
           [&gpo](auto& ext) {
              gpo.proposed_security_group_block_num    = ext.proposed_security_group_block_num;
-             gpo.proposed_security_group_participants = std::move(ext.proposed_security_group_participants);
-             gpo.transaction_hooks                    = std::move(ext.transaction_hooks);
+             gpo.set_proposed_security_group_participants(ext.proposed_security_group_participants.begin(),
+                                                          ext.proposed_security_group_participants.end());
+             gpo.set_transaction_hooks(ext.transaction_hooks.begin(), ext.transaction_hooks.end());
           },
           extension);
    }

pipeline.jsonc

@@ -52,7 +52,7 @@
         "test":
         [
             {
-                "commit": "fa9581c9c50b8123d3f86055e8b9cafa3ef36f5b"
+                "commit": "85ac2aae5f52c68b6b1d764b997bb020f3c496f8"
             }
         ]
     }

plugins/net_plugin/net_plugin.cpp

@@ -3678,10 +3678,7 @@ namespace eosio {
       //
       for(auto& connection : connections) {
          const auto& participant = connection->participant_name();
-         if(!participant) {
-            continue;
-         }
-         if(security_group.is_in_security_group(participant.value())) {
+         if(participant && security_group.is_in_security_group(participant.value())) {
             if(!connection->is_participating()) {
                connection->set_participating(true);
                connection->send_handshake();

tests/CMakeLists.txt

@@ -62,6 +62,7 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/nodeos_contrl_c_test.py ${CMAKE_CURRE
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/blockvault_tests.py ${CMAKE_CURRENT_BINARY_DIR}/blockvault_tests.py COPYONLY)
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/generate-certificates.sh ${CMAKE_CURRENT_BINARY_DIR}/generate-certificates.sh COPYONLY)
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/privacy_startup_network.py ${CMAKE_CURRENT_BINARY_DIR}/privacy_startup_network.py COPYONLY)
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/privacy_simple_network.py ${CMAKE_CURRENT_BINARY_DIR}/privacy_simple_network.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,8 @@ add_test(NAME eosio_blocklog_prune_test COMMAND tests/eosio_blocklog_prune_test.
 set_property(TEST eosio_blocklog_prune_test PROPERTY LABELS nonparallelizable_tests)
 add_test(NAME privacy_startup_network COMMAND tests/privacy_startup_network.py -p 1 -v --clean-run --dump-error-detail WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
 set_property(TEST privacy_startup_network PROPERTY LABELS nonparallelizable_tests)
+add_test(NAME privacy_simple_network COMMAND tests/privacy_simple_network.py -p 2 -n 3 -v --clean-run --dump-error-detail WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
+set_property(TEST privacy_simple_network PROPERTY LABELS nonparallelizable_tests)
 
 # Long running tests
 add_test(NAME nodeos_sanity_lr_test COMMAND tests/nodeos_run_test.py -v --sanity-test --clean-run --dump-error-detail WORKING_DIRECTORY ${CMAKE_BINARY_DIR})

tests/Cluster.py

@@ -245,27 +245,9 @@ def insertSpecificExtraNodeosArgs(node, insertStr):
 
             certAuth = os.path.join(privacyDir, "CA_cert.pem")
             def getArguments(number):
-                # this function converts number to eos name string
-                # eos name can have only numbers 1-5
-                # e.g. 0 -> 1, 6 -> 5.1, 12 -> 5.5.2
-                def normalizeNumber(number):
-                    assert(number > 0)
-                    if number <= 5:
-                        return str(number)
-                    cnt = number
-                    ret = "5"
-                    while cnt > 5:
-                        cnt = cnt - 5
-                        if cnt > 5:
-                            ret = "{}.5".format(ret)
-                        else:
-                            ret = "{}.{}".format(ret, cnt)
-                        assert(len(ret) <= 13)
-
-                    return ret
-
-                nodeCert = os.path.join(privacyDir, "node{}.crt".format(normalizeNumber(number+1)))
-                nodeKey = os.path.join(privacyDir, "node{}_key.pem".format(normalizeNumber(number+1)))
+                participantName = Node.participantName(number)
+                nodeCert = os.path.join(privacyDir, "{}.crt".format(participantName))
+                nodeKey = os.path.join(privacyDir, "{}_key.pem".format(participantName))
                 return "--p2p-tls-own-certificate-file {} --p2p-tls-private-key-file {} --p2p-tls-security-group-ca-file {}".format(nodeCert, nodeKey, certAuth)
 
             for node in range(totalNodes):
@@ -370,9 +352,11 @@ def normalizeNumber(number):
 
         def createDefaultShapeFile(newFile, cmdArr):
             cmdArrForOutput=copy.deepcopy(cmdArr)
+            cmdArrForOutput.append("--shape")
+            cmdArrForOutput.append("ring")
             cmdArrForOutput.append("--output")
             cmdArrForOutput.append(newFile)
-            s=" ".join(cmdArrForOutput)
+            s=" ".join([("'{0}'".format(element) if (' ' in element) else element) for element in cmdArrForOutput.copy()])
             if Utils.Debug: Utils.Print("cmd: %s" % (s))
             if 0 != subprocess.call(cmdArrForOutput):
                 Utils.Print("ERROR: Launcher failed to create shape file \"{}\".".format(newFile))
@@ -1838,3 +1822,58 @@ def stripValues(lowestMaxes,greaterThan):
         while len(lowestMaxes)>0 and compareCommon(blockLogs, blockNameExtensions, first, lowestMaxes[0]):
             first=lowestMaxes[0]+1
             lowestMaxes=stripValues(lowestMaxes,lowestMaxes[0])
+
+    def getAllNodes(self):
+        nodes = []
+        nodes.extend(self.getNodes())
+        if self.biosNode is not None:
+            nodes.append(self.biosNode)
+        return nodes
+
+    def reportInfo(self):
+        Utils.Print("\n\n\n*****************************")
+        Utils.Print("All Nodes current info:")
+        for node in self.getAllNodes():
+            Utils.Print("Info: {}".format(json.dumps(node.getInfo(), indent=4, sort_keys=True)))
+        Utils.Print("\n*****************************")
+
+    def verifyInSync(self, sourceNodeNum=0, specificNodes=None):
+        assert isinstance(sourceNodeNum, int)
+        desc = "provided " if specificNodes else ""
+
+        if specificNodes is None:
+            specificNodes = self.getAllNodes()
+        assert sourceNodeNum < len(specificNodes)
+        Utils.Print("Ensure all {}nodes are in-sync".format(desc))
+        source = specificNodes[sourceNodeNum]
+        lib = source.getInfo()["last_irreversible_block_num"]
+        headBlockNum = source.getBlockNum()
+        headBlock = source.getBlock(headBlockNum)
+        Utils.Print("headBlock: {}".format(json.dumps(headBlock, indent=4, sort_keys=True)))
+        headBlockId = headBlock["id"]
+        error = None
+        for node in specificNodes:
+            if node.waitForBlock(headBlockNum, reportInterval = 1) is None:
+                error = "Node failed to get block number {}. Current node info: {}".format(headBlockNum, json.dumps(node.getInfo(), indent=4, sort_keys=True))
+                break
+
+            if node.waitForNextBlock() is None:
+                error = "Node failed to advance head. Current node info: {}".format(json.dumps(node.getInfo(), indent=4, sort_keys=True))
+                break
+
+            if node.getBlock(headBlockId) is None:
+                error = "Producer node has block number: {}, but it is not id: {}. Block: {}".format(headBlockNum, headBlockId, json.dumps(node.getBlock(headBlockNum), indent=4, sort_keys=True))
+                break
+
+            if node.waitForBlock(lib, blockType=BlockType.lib) is None:
+                error = "Producer node is failing to advance its lib ({}) with producer {} ({})".format(node.getInfo()["last_irreversible_block_num"], producerNum, lib)
+                break
+
+            Utils.Print("Ensure all nodes are advancing lib")
+            if node.waitForBlock(lib + 1, blockType=BlockType.lib, reportInterval = 1) == None:
+                error = "Producer node failed to advance lib ahead one block to: {}".format(lib + 1)
+                break
+
+        if error:
+            self.reportInfo()
+            Utils.errorExit(error)