From 7f966583b043f43601bebfe3e17eee50afa81930 Mon Sep 17 00:00:00 2001 From: Tarunkumar Banda Date: Mon, 24 Apr 2023 09:36:23 +0000 Subject: [PATCH] Refactoring the ST changes in FullNode --- .../bcstatetransfer/AsyncStateTransferCRE.cpp | 4 +-- bftengine/src/bcstatetransfer/BCStateTran.cpp | 6 ++--- bftengine/src/bftengine/FullNodeReplica.cpp | 26 +++++++++---------- bftengine/src/bftengine/FullNodeReplica.hpp | 9 ++++--- bftengine/src/bftengine/ReadOnlyReplica.hpp | 1 + bftengine/src/bftengine/ReplicaBase.hpp | 2 ++ bftengine/src/bftengine/ReplicaFactory.cpp | 2 +- bftengine/src/bftengine/ReplicaImp.hpp | 2 ++ bftengine/src/preprocessor/PreProcessor.cpp | 3 +-- examples/replica/src/SetupReplica.cpp | 4 +-- kvbc/src/Replica.cpp | 4 +-- kvbc/src/reconfiguration_kvbc_handler.cpp | 8 ++---- tests/simpleKVBC/TesterReplica/setup.cpp | 4 +-- 13 files changed, 37 insertions(+), 38 deletions(-) diff --git a/bftengine/src/bcstatetransfer/AsyncStateTransferCRE.cpp b/bftengine/src/bcstatetransfer/AsyncStateTransferCRE.cpp index fe0d8d63ca..ddb1cab14e 100644 --- a/bftengine/src/bcstatetransfer/AsyncStateTransferCRE.cpp +++ b/bftengine/src/bcstatetransfer/AsyncStateTransferCRE.cpp @@ -148,9 +148,7 @@ std::shared_ptr CreFactory::create(std::shared_ptr< for (uint16_t i = 0; i < repConfig.numReplicas; i++) { bftClientConf.all_replicas.emplace(bft::client::ReplicaId{i}); } - for (uint16_t i = repConfig.numReplicas; - i < repConfig.numReplicas + repConfig.numRoReplicas + repConfig.numFnReplicas; - i++) { + for (uint16_t i = repConfig.numReplicas; i < repConfig.numReplicas + repConfig.numRoReplicas; i++) { bftClientConf.ro_replicas.emplace(bft::client::ReplicaId{i}); } bftClientConf.replicas_master_key_folder_path = std::nullopt; diff --git a/bftengine/src/bcstatetransfer/BCStateTran.cpp b/bftengine/src/bcstatetransfer/BCStateTran.cpp index b146959889..b4d3372529 100644 --- a/bftengine/src/bcstatetransfer/BCStateTran.cpp +++ b/bftengine/src/bcstatetransfer/BCStateTran.cpp @@ -3971,13 +3971,13 @@ void BCStateTran::computeDigestOfPage( if (checkpointNumber > 0) { digestGenerator.update(page, pageSize); } - digestGenerator.writeDigest(reinterpret_cast(&outDigest)); + digestGenerator.writeDigest(outDigest.getForUpdate()); } void BCStateTran::computeDigestOfPagesDescriptor(const DataStore::ResPagesDescriptor *pagesDesc, Digest &outDigest) { DigestGenerator digestGenerator; digestGenerator.update(reinterpret_cast(pagesDesc), pagesDesc->size()); - digestGenerator.writeDigest(reinterpret_cast(&outDigest)); + digestGenerator.writeDigest(outDigest.getForUpdate()); } void BCStateTran::computeDigestOfBlockImpl(const uint64_t blockNum, @@ -3996,7 +3996,7 @@ void BCStateTran::computeDigestOfBlock(const uint64_t blockNum, const char *block, const uint32_t blockSize, Digest *outDigest) { - computeDigestOfBlockImpl(blockNum, block, blockSize, reinterpret_cast(outDigest)); + computeDigestOfBlockImpl(blockNum, block, blockSize, outDigest->getForUpdate()); } BlockDigest BCStateTran::computeDigestOfBlock(const uint64_t blockNum, const char *block, const uint32_t blockSize) { diff --git a/bftengine/src/bftengine/FullNodeReplica.cpp b/bftengine/src/bftengine/FullNodeReplica.cpp index 4151d793b8..f71a4ee511 100755 --- a/bftengine/src/bftengine/FullNodeReplica.cpp +++ b/bftengine/src/bftengine/FullNodeReplica.cpp @@ -44,12 +44,12 @@ FullNodeReplica::FullNodeReplica(const ReplicaConfig &config, concordUtil::Timers &timers, MetadataStorage *metadataStorage) : ReplicaForStateTransfer(config, requestsHandler, stateTransfer, msgComm, msgHandlerReg, true, timers), - ro_metrics_{metrics_.RegisterCounter("receivedCheckpointMsgs"), + fn_metrics_{metrics_.RegisterCounter("receivedCheckpointMsgs"), metrics_.RegisterCounter("sentAskForCheckpointMsgs"), metrics_.RegisterCounter("receivedInvalidMsgs"), metrics_.RegisterGauge("lastExecutedSeqNum", lastExecutedSeqNum)}, metadataStorage_{metadataStorage} { - LOG_INFO(GL, "Initialising ReadOnly Replica"); + LOG_INFO(GL, "Initialising Full Node Replica"); repsInfo = new ReplicasInfo(config, dynamicCollectorForPartialProofs, dynamicCollectorForExecutionProofs); msgHandlers_->registerMsgHandler( MsgCode::Checkpoint, std::bind(&FullNodeReplica::messageHandler, this, std::placeholders::_1)); @@ -72,7 +72,7 @@ FullNodeReplica::FullNodeReplica(const ReplicaConfig &config, concord::crypto::KeyFormat::PemFormat}}, *repsInfo); - // Register status handler for Read-Only replica + // Register status handler for Full Node replica registerStatusHandlers(); bft::communication::StateControl::instance().setGetPeerPubKeyMethod( [&](uint32_t id) { return SigManager::instance()->getPublicKeyOfVerifier(id); }); @@ -98,18 +98,18 @@ void FullNodeReplica::stop() { void FullNodeReplica::onTransferringCompleteImp(uint64_t newStateCheckpoint) { lastExecutedSeqNum = newStateCheckpoint * checkpointWindowSize; - ro_metrics_.last_executed_seq_num_.Get().Set(lastExecutedSeqNum); + fn_metrics_.last_executed_seq_num_.Get().Set(lastExecutedSeqNum); last_executed_seq_num_ = lastExecutedSeqNum; } void FullNodeReplica::onReportAboutInvalidMessage(MessageBase *msg, const char *reason) { - ro_metrics_.received_invalid_msg_++; + fn_metrics_.received_invalid_msg_++; LOG_WARN(GL, "Node " << config_.replicaId << " received invalid message from Node " << msg->senderId() << " type=" << msg->type() << " reason: " << reason); } void FullNodeReplica::sendAskForCheckpointMsg() { - ro_metrics_.sent_ask_for_checkpoint_msg_++; + fn_metrics_.sent_ask_for_checkpoint_msg_++; LOG_INFO(GL, "sending AskForCheckpointMsg"); auto msg = std::make_unique(config_.replicaId); for (auto id : repsInfo->idsOfPeerReplicas()) send(msg.get(), id); @@ -125,7 +125,7 @@ void FullNodeReplica::onMessage(std::unique_ptr ms if (isCollectingState()) { return; } - ro_metrics_.received_checkpoint_msg_++; + fn_metrics_.received_checkpoint_msg_++; LOG_INFO(GL, KVLOG(msg->senderId(), msg->idOfGeneratedReplica(), @@ -208,12 +208,12 @@ void FullNodeReplica::onMessage(std::unique_ptrgetCid()); span.setTag("seq_num", reqSeqNum); - // A read only replica can handle only reconfiguration requests. Those requests are signed by the operator and + // A full node replica can handle only reconfiguration requests. Those requests are signed by the operator and // the validation is done in the reconfiguration engine. Thus, we don't need to check the client validity as in // the committers if (reconfig_flag) { - LOG_INFO(GL, "ro replica has received a reconfiguration request"); + LOG_INFO(GL, "FN replica has received a reconfiguration request"); executeReadOnlyRequest(span, *(msg.get())); return; } @@ -221,7 +221,7 @@ void FullNodeReplica::onMessage(std::unique_ptr metadataStorage_; std::atomic last_executed_seq_num_{0}; private: // This function serves as an ReplicaStatusHandlers alternative for FullNodeReplica. The reason to use this function - // is that regular and read-only replicas expose different metrics and the status handlers are not interchangeable. - // The read-only replica also hasn't got an implementation for InternalMessages which are used by the + // is that regular and full node replicas expose different metrics and the status handlers are not interchangeable. + // The full node replica also hasn't got an implementation for InternalMessages which are used by the // ReplicaStatusHandler. void registerStatusHandlers(); }; diff --git a/bftengine/src/bftengine/ReadOnlyReplica.hpp b/bftengine/src/bftengine/ReadOnlyReplica.hpp index 5d1951b173..4bc0b5461a 100644 --- a/bftengine/src/bftengine/ReadOnlyReplica.hpp +++ b/bftengine/src/bftengine/ReadOnlyReplica.hpp @@ -34,6 +34,7 @@ class ReadOnlyReplica : public ReplicaForStateTransfer { void start() override; void stop() override; virtual bool isReadOnly() const override { return true; } + virtual bool isFullNode() const override { return false; } protected: void sendAskForCheckpointMsg(); diff --git a/bftengine/src/bftengine/ReplicaBase.hpp b/bftengine/src/bftengine/ReplicaBase.hpp index 7799fb4820..f1650969b9 100644 --- a/bftengine/src/bftengine/ReplicaBase.hpp +++ b/bftengine/src/bftengine/ReplicaBase.hpp @@ -50,6 +50,8 @@ class ReplicaBase { virtual bool isReadOnly() const = 0; + virtual bool isFullNode() const = 0; + std::shared_ptr getMsgsCommunicator() const { return msgsCommunicator_; } std::shared_ptr getMsgHandlersRegistrator() const { return msgHandlers_; } concordUtil::Timers* getTimers() { return &timers_; } diff --git a/bftengine/src/bftengine/ReplicaFactory.cpp b/bftengine/src/bftengine/ReplicaFactory.cpp index 1bf85d2873..c8879fb368 100644 --- a/bftengine/src/bftengine/ReplicaFactory.cpp +++ b/bftengine/src/bftengine/ReplicaFactory.cpp @@ -96,7 +96,7 @@ void ReplicaInternal::restartForDebug(uint32_t delayMillisec) { } } - if (!replica_->isReadOnly()) { + if (!replica_->isReadOnly() && !replica_->isFullNode()) { auto replicaImp = dynamic_cast(replica_.get()); shared_ptr persistentStorage(replicaImp->getPersistentStorage()); diff --git a/bftengine/src/bftengine/ReplicaImp.hpp b/bftengine/src/bftengine/ReplicaImp.hpp index 5f443b9232..4d79f4311b 100644 --- a/bftengine/src/bftengine/ReplicaImp.hpp +++ b/bftengine/src/bftengine/ReplicaImp.hpp @@ -366,6 +366,8 @@ class ReplicaImp : public InternalReplicaApi, public ReplicaForStateTransfer { virtual bool isReadOnly() const override { return false; } + virtual bool isFullNode() const override { return false; } + shared_ptr getPersistentStorage() const { return ps_; } std::shared_ptr getSecretsManager() { return sm_; } diff --git a/bftengine/src/preprocessor/PreProcessor.cpp b/bftengine/src/preprocessor/PreProcessor.cpp index 062933ac42..bdc3e86da5 100644 --- a/bftengine/src/preprocessor/PreProcessor.cpp +++ b/bftengine/src/preprocessor/PreProcessor.cpp @@ -407,8 +407,7 @@ PreProcessor::PreProcessor(shared_ptr &msgsCommunicator, myReplicaId_(myReplica.getReplicaConfig().replicaId), maxExternalMsgSize_(myReplica.getReplicaConfig().maxExternalMessageSize), responseSizeInternalOverhead_{/* for conflict detection BlockId */ sizeof(uint64_t)}, - numOfReplicas_(myReplica.getReplicaConfig().numReplicas + myReplica.getReplicaConfig().numRoReplicas + - myReplica.getReplicaConfig().numFnReplicas), + numOfReplicas_(myReplica.getReplicaConfig().numReplicas + myReplica.getReplicaConfig().numRoReplicas), numOfClientProxies_(myReplica.getReplicaConfig().numOfClientProxies), clientBatchingEnabled_(myReplica.getReplicaConfig().clientBatchingEnabled), threadPool_("PreProcessor::threadPool"), diff --git a/examples/replica/src/SetupReplica.cpp b/examples/replica/src/SetupReplica.cpp index 485e78ba65..1833a29e43 100644 --- a/examples/replica/src/SetupReplica.cpp +++ b/examples/replica/src/SetupReplica.cpp @@ -202,8 +202,8 @@ bft::communication::ICommunication* SetupReplica::createCommunication( logging::Logger logger = getLogger(); TestCommConfig testCommConfig(logger); testCommConfig.GetReplicaConfig(replicaConfig.replicaId, keysFilePrefix, &replicaConfig); - uint16_t numOfReplicas = (uint16_t)(3 * replicaConfig.fVal + 2 * replicaConfig.cVal + 1 + - replicaConfig.numRoReplicas + replicaConfig.numFnReplicas); + uint16_t numOfReplicas = + (uint16_t)(3 * replicaConfig.fVal + 2 * replicaConfig.cVal + 1 + replicaConfig.numRoReplicas); auto numOfClients = replicaConfig.numOfClientProxies ? replicaConfig.numOfClientProxies : replicaConfig.numOfExternalClients; #ifdef USE_COMM_PLAIN_TCP diff --git a/kvbc/src/Replica.cpp b/kvbc/src/Replica.cpp index 94c6229f41..227a12c227 100644 --- a/kvbc/src/Replica.cpp +++ b/kvbc/src/Replica.cpp @@ -64,9 +64,9 @@ Status Replica::initInternals() { bftEngine::ReplicaConfig::instance().pathToOperatorPublicKey_, bftEngine::ReplicaConfig::instance().operatorMsgSigningAlgo, *this)); - if (replicaConfig_.isReadOnly || replicaConfig_.isFullNode) { + if (replicaConfig_.isReadOnly) { LOG_INFO(logger, - "Read Replica Status:" << KVLOG(getLastBlockNum(), getLastBlockId(), getLastReachableBlockNum())); + "ReadOnly Replica Status:" << KVLOG(getLastBlockNum(), getLastBlockId(), getLastReachableBlockNum())); m_replicaPtr = bftEngine::ReplicaFactory::createRoReplica( replicaConfig_, requestHandler, m_stateTransfer, m_ptrComm.get(), m_metadataStorage); } else { diff --git a/kvbc/src/reconfiguration_kvbc_handler.cpp b/kvbc/src/reconfiguration_kvbc_handler.cpp index c4f72f7489..7e2a284cd8 100644 --- a/kvbc/src/reconfiguration_kvbc_handler.cpp +++ b/kvbc/src/reconfiguration_kvbc_handler.cpp @@ -464,8 +464,7 @@ bool KvbcClientReconfigurationHandler::handle(const concord::messages::ClientRec const std::optional& ts, concord::messages::ReconfigurationResponse& rres) { concord::messages::ClientReconfigurationStateReply rep; - uint16_t first_client_id = ReplicaConfig::instance().numReplicas + ReplicaConfig::instance().numRoReplicas + - ReplicaConfig::instance().numFnReplicas; + uint16_t first_client_id = ReplicaConfig::instance().numReplicas + ReplicaConfig::instance().numRoReplicas; if (sender_id > first_client_id) { for (uint8_t i = kvbc::keyTypes::CLIENT_COMMAND_TYPES::start_ + 1; i < kvbc::keyTypes::CLIENT_COMMAND_TYPES::end_; i++) { @@ -699,10 +698,7 @@ bool ReconfigurationHandler::handle(const concord::messages::AddRemoveWithWedgeC auto execute_key_prefix = std::string{kvbc::keyTypes::reconfiguration_client_data_prefix, static_cast(kvbc::keyTypes::CLIENT_COMMAND_TYPES::CLIENT_SCALING_EXECUTE_COMMAND)}; - - for (auto i = 0; i < ReplicaConfig::instance().numReplicas + ReplicaConfig::instance().numRoReplicas + - ReplicaConfig::instance().numFnReplicas; - i++) { + for (uint64_t i = 0; i < ReplicaConfig::instance().numReplicas + ReplicaConfig::instance().numRoReplicas; i++) { concord::messages::ClientsAddRemoveExecuteCommand cmd; cmd.config_descriptor = command.config_descriptor; if (token.find(i) == token.end()) continue; diff --git a/tests/simpleKVBC/TesterReplica/setup.cpp b/tests/simpleKVBC/TesterReplica/setup.cpp index 4d74a34c41..af8e1620df 100644 --- a/tests/simpleKVBC/TesterReplica/setup.cpp +++ b/tests/simpleKVBC/TesterReplica/setup.cpp @@ -370,8 +370,8 @@ std::unique_ptr TestSetup::ParseArgs(int argc, char** argv) { TestCommConfig testCommConfig(logger); testCommConfig.GetReplicaConfig(replicaConfig.replicaId, keysFilePrefix, &replicaConfig); - uint16_t numOfReplicas = (uint16_t)(3 * replicaConfig.fVal + 2 * replicaConfig.cVal + 1 + - replicaConfig.numRoReplicas + replicaConfig.numFnReplicas); + uint16_t numOfReplicas = + (uint16_t)(3 * replicaConfig.fVal + 2 * replicaConfig.cVal + 1 + replicaConfig.numRoReplicas); auto numOfClients = replicaConfig.numOfClientProxies ? replicaConfig.numOfClientProxies : replicaConfig.numOfExternalClients; std::shared_ptr sm_ =