Redirect pruning signature generation/verification to SigManager

Author: WildFireFlum

.github/workflows/build_and_test.yml

@@ -27,6 +27,7 @@ jobs:
           run: |
             mkdir build
             cd build
+            echo $(date +%y-%m-%d_%H-%M-%S) > timestamp
             cmake \
               ${{ matrix.comm_type }} \
               -DCMAKE_CXX_COMPILER=${{ matrix.compiler }} \
@@ -39,6 +40,11 @@ jobs:
             cd build
             # many tests require administrative priviliges for running iptables or nc
             sudo ctest --output-on-failure
+        - name: Print failed cases
+          if: ${{ failure() }}
+          run: |
+            FAILED_CASES_FILE=${PWD}/build/apollogs/latest/failed_cases.txt
+            test -f ${FAILED_CASES_FILE} && echo Failed apollo cases: && cat -n ${FAILED_CASES_FILE}
         - name: Prepare artifacts
           if: ${{ failure() }}
           run: |

Makefile

@@ -49,34 +49,17 @@ else
    TCP_ENABLED__:=OFF
 endif
 
-CONCORD_BFT_CMAKE_CXX_FLAGS_RELEASE?='-O3 -g'
-CONCORD_BFT_CMAKE_USE_LOG4CPP?=ON
-CONCORD_BFT_CMAKE_BUILD_UTT?=TRUE
-CONCORD_BFT_CMAKE_BUILD_ROCKSDB_STORAGE?=TRUE
-CONCORD_BFT_CMAKE_USE_S3_OBJECT_STORE?=TRUE
-CONCORD_BFT_CMAKE_USE_OPENTRACING?=TRUE
-CONCORD_BFT_CMAKE_USE_PROMETHEUS?=TRUE
-CONCORD_BFT_CMAKE_USE_JAEGER?=TRUE
-CONCORD_BFT_CMAKE_USE_JSON?=TRUE
-CONCORD_BFT_CMAKE_USE_HTTPLIB?=TRUE
-CONCORD_BFT_CMAKE_EXPORT_COMPILE_COMMANDS?=TRUE
-CONCORD_BFT_CMAKE_OMIT_TEST_OUTPUT?=FALSE
-CONCORD_BFT_CMAKE_KEEP_APOLLO_LOGS?=TRUE
-CONCORD_BFT_CMAKE_RUN_APOLLO_TESTS?=TRUE
-CONCORD_BFT_CMAKE_TRANSACTION_SIGNING_ENABLED?=TRUE
-CONCORD_BFT_CMAKE_BUILD_SLOWDOWN?=FALSE
-# Only useful with CONCORD_BFT_CMAKE_BUILD_TYPE:=Release
-CONCORD_BFT_CMAKE_BUILD_KVBC_BENCH?=TRUE
-# Only usefull with CONCORD_BFT_CMAKE_CXX_FLAGS_RELEASE=-O0 -g
-CONCORD_BFT_CMAKE_ASAN?=FALSE
-CONCORD_BFT_CMAKE_TSAN?=FALSE
-CONCORD_BFT_CMAKE_UBSAN?=FALSE
-CONCORD_BFT_CMAKE_HEAPTRACK?=FALSE
-CONCORD_BFT_CMAKE_CODECOVERAGE?=FALSE
-CONCORD_BFT_CMAKE_CCACHE?=TRUE
-CONCORD_BFT_CMAKE_USE_FAKE_CLOCK_IN_TIME_SERVICE?=FALSE
-ENABLE_RESTART_RECOVERY_TESTS?=FALSE
-CONCORD_ENABLE_ALL_METRICS?=FALSE
+CONCORD_BFT_CMAKE_BUILD_UTT?=ON
+CONCORD_BFT_CMAKE_OMIT_TEST_OUTPUT?=OFF
+CONCORD_BFT_CMAKE_KEEP_APOLLO_LOGS?=ON
+CONCORD_BFT_CMAKE_RUN_APOLLO_TESTS?=ON
+CONCORD_BFT_CMAKE_ASAN?=OFF
+CONCORD_BFT_CMAKE_TSAN?=OFF
+CONCORD_BFT_CMAKE_UBSAN?=OFF
+CONCORD_BFT_CMAKE_HEAPTRACK?=OFF
+CONCORD_BFT_CMAKE_CODECOVERAGE?=OFF
+CONCORD_BFT_CMAKE_CCACHE?=ON
+ENABLE_RESTART_RECOVERY_TESTS?=OFF
 
 # Our CMake logic won't allow more one of these flags to be raised, so having this if/else logic makes sense
 ifeq (${CONCORD_BFT_CMAKE_ASAN},ON)

bftengine/include/bftengine/ReplicaConfig.hpp

@@ -306,11 +306,11 @@ class ReplicaConfig : public concord::serialize::SerializableFactory<ReplicaConf
     if (auto it = config_params_.find(param); it != config_params_.end()) return concord::util::to<T>(it->second);
     return defaultValue;
   }
-  inline std::set<std::pair<const std::string, std::set<uint16_t>>>* getPublicKeysOfClients() {
+  inline const std::set<std::pair<const std::string, std::set<uint16_t>>>* getPublicKeysOfClients() const {
     return (clientTransactionSigningEnabled || !clientsKeysPrefix.empty()) ? &publicKeysOfClients : nullptr;
   }
 
-  std::string getOperatorPublicKey() {
+  std::string getOperatorPublicKey() const {
     std::ifstream op_key_file(pathToOperatorPublicKey_);
     if (!op_key_file.fail()) {
       std::stringstream buffer;

bftengine/src/bftengine/ReplicaBase.hpp

@@ -112,7 +112,7 @@ class ReplicaBase {
   static const uint16_t ALL_OTHER_REPLICAS = UINT16_MAX;
 
   const ReplicaConfig& config_;
-  ReplicasInfo* repsInfo = nullptr;
+  const ReplicasInfo* repsInfo = nullptr;
   std::shared_ptr<MsgsCommunicator> msgsCommunicator_;
   std::shared_ptr<MsgHandlersRegistrator> msgHandlers_;
   std::shared_ptr<IRequestsHandler> bftRequestsHandler_;

bftengine/src/bftengine/ReplicaImp.cpp

@@ -4434,16 +4434,7 @@ ReplicaImp::ReplicaImp(bool firstTime,
 
   if (firstTime) {
     repsInfo = new ReplicasInfo(config_, dynamicCollectorForPartialProofs, dynamicCollectorForExecutionProofs);
-    sigManager_ = SigManager::init(config_.replicaId,
-                                   config_.replicaPrivateKey,
-                                   config_.publicKeysOfReplicas,
-                                   concord::crypto::KeyFormat::HexaDecimalStrippedFormat,
-                                   ReplicaConfig::instance().getPublicKeysOfClients(),
-                                   concord::crypto::KeyFormat::PemFormat,
-                                   {{repsInfo->getIdOfOperator(),
-                                     ReplicaConfig::instance().getOperatorPublicKey(),
-                                     concord::crypto::KeyFormat::PemFormat}},
-                                   *repsInfo);
+    sigManager_ = SigManager::owningInstance();
     viewsManager = new ViewsManager(repsInfo);
   } else {
     repsInfo = replicasInfo;

bftengine/src/bftengine/ReplicaLoader.cpp

@@ -47,25 +47,14 @@ namespace impl {
 namespace {
 
 ReplicaLoader::ErrorCode loadConfig(LoadedReplicaData &ld) {
-  ld.repsInfo = new ReplicasInfo(ld.repConfig, dynamicCollectorForPartialProofs, dynamicCollectorForExecutionProofs);
   auto &config = ld.repConfig;
-  ld.sigManager = SigManager::init(config.replicaId,
-                                   config.replicaPrivateKey,
-                                   config.publicKeysOfReplicas,
-                                   concord::crypto::KeyFormat::HexaDecimalStrippedFormat,
-                                   ReplicaConfig::instance().getPublicKeysOfClients(),
-                                   concord::crypto::KeyFormat::PemFormat,
-                                   {{ld.repsInfo->getIdOfOperator(),
-                                     ReplicaConfig::instance().getOperatorPublicKey(),
-                                     concord::crypto::KeyFormat::PemFormat}},
-                                   *ld.repsInfo);
-
-  std::unique_ptr<Cryptosystem> cryptoSys = std::make_unique<Cryptosystem>(ld.repConfig.thresholdSystemType_,
-                                                                           ld.repConfig.thresholdSystemSubType_,
-                                                                           ld.repConfig.numReplicas,
-                                                                           ld.repConfig.numReplicas);
-  cryptoSys->loadKeys(ld.repConfig.thresholdPublicKey_, ld.repConfig.thresholdVerificationKeys_);
-  cryptoSys->loadPrivateKey(ld.repConfig.replicaId, ld.repConfig.thresholdPrivateKey_);
+  ld.repsInfo = new ReplicasInfo(config, dynamicCollectorForPartialProofs, dynamicCollectorForExecutionProofs);
+  ld.sigManager = SigManager::owningInstance();
+
+  std::unique_ptr<Cryptosystem> cryptoSys = std::make_unique<Cryptosystem>(
+      config.thresholdSystemType_, config.thresholdSystemSubType_, config.numReplicas, config.numReplicas);
+  cryptoSys->loadKeys(config.thresholdPublicKey_, config.thresholdVerificationKeys_);
+  cryptoSys->loadPrivateKey(config.replicaId, config.thresholdPrivateKey_);
   bftEngine::CryptoManager::init(std::move(cryptoSys));
 
   return Succ;

bftengine/src/bftengine/ReplicasInfo.cpp

@@ -146,11 +146,12 @@ ReplicasInfo::ReplicasInfo(const ReplicaConfig& config,
         }
         if (start != end) LOG_INFO(GL, "Principal ids in _idsOfInternalClients: " << start << " to " << end - 1);
         return ret;
-      }()} {
-  _operator_id = config.operatorEnabled_
-                     ? static_cast<PrincipalId>(config.numReplicas + config.numRoReplicas + config.numOfClientProxies +
-                                                config.numOfExternalClients + config.numOfClientServices - 1)
-                     : 0;
+      }()},
+
+      _operator_id{static_cast<PrincipalId>(
+          config.operatorEnabled_ ? config.numReplicas + config.numRoReplicas + config.numOfClientProxies +
+                                        config.numOfExternalClients + config.numOfClientServices - 1
+                                  : 0)} {
   ConcordAssert(_numberOfReplicas == (3 * _fVal + 2 * _cVal + 1));
 }
 

bftengine/src/bftengine/ReplicasInfo.hpp

@@ -20,6 +20,9 @@ class ReplicaConfig;
 
 namespace impl {
 
+/**
+ * An immutable class holding the the ids of all the participants in the network
+ */
 class ReplicasInfo {
  public:
   ReplicasInfo(const ReplicaConfig&, bool dynamicCollectorForPartialProofs, bool dynamicCollectorForExecutionProofs);
@@ -110,7 +113,7 @@ class ReplicasInfo {
   const std::set<PrincipalId> _idsOfInternalClients;
 
   // Currently we support only a single operator entity in the system
-  PrincipalId _operator_id;
+  const PrincipalId _operator_id = 0;
 };
 }  // namespace impl
 }  // namespace bftEngine

bftengine/src/bftengine/SigManager.cpp

@@ -47,6 +47,11 @@ SigManager* SigManager::instance() {
   return s_sm.get();
 }
 
+std::shared_ptr<SigManager> SigManager::owningInstance() {
+  ConcordAssertNE(s_sm.get(), nullptr);
+  return s_sm;
+}
+
 void SigManager::reset(std::shared_ptr<SigManager> other) { s_sm = other; }
 
 std::shared_ptr<SigManager> SigManager::init(
@@ -457,6 +462,7 @@ SeqNum SigManager::getReplicaLastExecutedSeq() const {
   ConcordAssert(replicasInfo_.isIdOfReplica(myId_) || replicasInfo_.isRoReplica());
   return replicaLastExecutedSeq_;
 }
+const ReplicasInfo& SigManager::getReplicasInfo() const { return replicasInfo_; }
 
 }  // namespace impl
 }  // namespace bftEngine

bftengine/src/bftengine/SigManager.hpp

@@ -19,6 +19,7 @@
 #include "crypto/signer.hpp"
 #include "crypto/verifier.hpp"
 #include "SysConsts.hpp"
+#include "ReplicasInfo.hpp"
 #include <utility>
 #include <vector>
 #include <map>
@@ -35,15 +36,14 @@ class IThresholdVerifier;
 namespace bftEngine {
 namespace impl {
 
-class ReplicasInfo;
-
 class SigManager {
  public:
   using Key = std::string;
   using KeyIndex = uint16_t;
 
   virtual ~SigManager() = default;
   static SigManager* instance();
+  static std::shared_ptr<SigManager> owningInstance();
   static void reset(std::shared_ptr<SigManager> other);
 
   // It is the caller responsibility to deallocate (delete) the object
@@ -129,6 +129,8 @@ class SigManager {
   const concord::crypto::IVerifier& extractVerifierFromMultisig(std::shared_ptr<IThresholdVerifier> thresholdVerifier,
                                                                 PrincipalId id) const;
 
+  const ReplicasInfo& getReplicasInfo() const;
+
  protected:
   static constexpr uint16_t updateMetricsAggregatorThresh = 1000;
 
@@ -153,7 +155,7 @@ class SigManager {
   std::unique_ptr<concord::crypto::ISigner> mySigner_;
   std::map<PrincipalId, std::shared_ptr<concord::crypto::IVerifier>> verifiers_;
   bool clientTransactionSigningEnabled_ = true;
-  const ReplicasInfo& replicasInfo_;
+  const ReplicasInfo replicasInfo_;
 
   // The ownership model of a SigManager object depends on its use
   static std::shared_ptr<SigManager> s_sm;