Skip to content

Commit

Permalink
Merge master [don't auto-bump]
Browse files Browse the repository at this point in the history
  • Loading branch information
WildFireFlum committed Mar 2, 2023
1 parent 98f5f3b commit a880c00
Show file tree
Hide file tree
Showing 26 changed files with 128 additions and 120 deletions.
13 changes: 8 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ CONCORD_BFT_ADDITIONAL_RUN_PARAMS+=\
--network host
endif

CONCORD_BFT_ADDITIONAL_CTEST_RUN_PARAMS="--output-on-failure"
FAILED_CASES_FILE=/concord-bft/build/apollogs/latest/failed_cases.txt
PRINT_FAILED_APOLLO_CASES=([ -s ${FAILED_CASES_FILE} ] && echo Failed apollo cases: && cat -n ${FAILED_CASES_FILE} && exit 1)

ifneq (${APOLLO_LOG_STDOUT},)
CONCORD_BFT_ADDITIONAL_RUN_PARAMS+=--env APOLLO_LOG_STDOUT=TRUE
CONCORD_BFT_ADDITIONAL_CTEST_RUN_PARAMS+=-V
Expand All @@ -189,8 +193,7 @@ BASIC_RUN_PARAMS?=-it --init --rm --privileged=true \
--name="${CONCORD_BFT_DOCKER_CONTAINER}" \
--workdir=${CONCORD_BFT_TARGET_SOURCE_PATH} \
--mount type=bind,source=${CURDIR},target=${CONCORD_BFT_TARGET_SOURCE_PATH}${CONCORD_BFT_CONTAINER_MOUNT_CONSISTENCY} \
${CONCORD_BFT_ADDITIONAL_RUN_PARAMS} \
${CONCORD_BFT_DOCKER_IMAGE_FULL_PATH}
${CONCORD_BFT_ADDITIONAL_RUN_PARAMS} ${CONCORD_BFT_DOCKER_IMAGE_FULL_PATH}

.DEFAULT_GOAL:=build

Expand Down Expand Up @@ -336,7 +339,7 @@ test: ## Run all tests
${CONCORD_BFT_CONTAINER_SHELL} -c \
"mkdir -p ${CONCORD_BFT_CORE_DIR} && \
cd ${CONCORD_BFT_BUILD_DIR} && \
ctest ${CONCORD_BFT_ADDITIONAL_CTEST_RUN_PARAMS} --timeout ${CONCORD_BFT_CTEST_TIMEOUT} --output-on-failure"
ctest ${CONCORD_BFT_ADDITIONAL_CTEST_RUN_PARAMS} --timeout ${CONCORD_BFT_CTEST_TIMEOUT} || ${PRINT_FAILED_APOLLO_CASES}"

.PHONY: simple-test
simple-test: ## Run Simple Test
Expand All @@ -350,7 +353,7 @@ test-range: ## Run all tests in the range [START,END], inclusive: `make test-ran
${CONCORD_BFT_CONTAINER_SHELL} -c \
"mkdir -p ${CONCORD_BFT_CORE_DIR} && \
cd ${CONCORD_BFT_BUILD_DIR} && \
ctest ${CONCORD_BFT_ADDITIONAL_CTEST_RUN_PARAMS} -I ${START},${END}"
ctest ${CONCORD_BFT_ADDITIONAL_CTEST_RUN_PARAMS} -I ${START},${END} || ${PRINT_FAILED_APOLLO_CASES}"

# ctest allows repeating tests, but not with the exact needed behavior below.
.PHONY: test-single-suite
Expand All @@ -362,7 +365,7 @@ test-single-suite: ## Run a single test `make test-single-suite TEST_NAME=<test
echo "=== Starting iteration $${i}/${NUM_REPEATS__}"; \
docker run ${BASIC_RUN_PARAMS} ${CONCORD_BFT_CONTAINER_SHELL} -c \
"mkdir -p ${CONCORD_BFT_CORE_DIR} && cd ${CONCORD_BFT_BUILD_DIR} && \
ctest ${CONCORD_BFT_ADDITIONAL_CTEST_RUN_PARAMS} -V -R ${TEST_NAME} --timeout ${CONCORD_BFT_CTEST_TIMEOUT} --output-on-failure"; \
ctest ${CONCORD_BFT_ADDITIONAL_CTEST_RUN_PARAMS} -V -R ${TEST_NAME} --timeout ${CONCORD_BFT_CTEST_TIMEOUT}"; || ${PRINT_FAILED_APOLLO_CASES} \
RESULT=$$?; \
if [[ $${RESULT} -ne 0 ]];then \
(( num_failures=num_failures+1 )); \
Expand Down
6 changes: 3 additions & 3 deletions bftengine/include/bftengine/CryptoManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class CryptoManager : public IKeyExchanger, public IMultiSigKeyGenerator {

// IKeyExchanger methods
// onPrivateKeyExchange and onPublicKeyExchange callbacks for a given checkpoint may be called in a different order.
// Therefore the first called will create ca CryptoSys
// Therefore the first called will create a CryptoSys
void onPrivateKeyExchange(const std::string& secretKey,
const std::string& verificationKey,
const SeqNum& sn) override;
Expand All @@ -77,8 +77,8 @@ class CryptoManager : public IKeyExchanger, public IMultiSigKeyGenerator {
* @param secretKey
* @param verificationKey
* @note: Assumes all keys are formatted as hex strings
* @note: TODO: Current implementation is not crash consistent, a ST which was completed after the replica process
* will lose the new private key
* @note: TODO: Current implementation is not crash consistent, a ST which was completed after the termination
* of the replica process will lose the new private key
*/
void syncPrivateKeyAfterST(const std::string& secretKey, const std::string& verificationKey);

Expand Down
1 change: 1 addition & 0 deletions bftengine/src/bcstatetransfer/AsyncStateTransferCRE.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "MsgHandlersRegistrator.hpp"
#include "MsgsCommunicator.hpp"
#include "client/reconfiguration/client_reconfiguration_engine.hpp"
#include "crypto/signer.hpp"

namespace bftEngine::bcst::asyncCRE {
class CreFactory {
Expand Down
4 changes: 2 additions & 2 deletions bftengine/src/bcstatetransfer/BCStateTran.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3431,9 +3431,9 @@ void BCStateTran::processData(bool lastInBatch, uint32_t rvbDigestsSize) {
LOG_ERROR(logger_, "Setting RVB digests into RVB manager failed!");
badDataFromCurrentSourceReplica = true;
} else {
#ifdef ENABLE_ALL_METRICS
#ifdef ENABLE_ALL_METRICS
metrics_.overall_rvb_digest_groups_validated_++;
#endif
#endif
}
}

Expand Down
3 changes: 0 additions & 3 deletions bftengine/src/bftengine/CryptoManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
// file.

#include "ReplicaConfig.hpp"
#include "Logger.hpp"
#include "CryptoManager.hpp"
#include <experimental/map>

Expand Down Expand Up @@ -57,8 +56,6 @@ std::unique_ptr<Cryptosystem>& CryptoManager::getLatestCryptoSystem() const {
*/
concord::crypto::SignatureAlgorithm CryptoManager::getLatestSignatureAlgorithm() const {
const std::unordered_map<std::string, concord::crypto::SignatureAlgorithm> typeToAlgorithm{
{MULTISIG_BLS_SCHEME, concord::crypto::SignatureAlgorithm::BLS},
{THRESHOLD_BLS_SCHEME, concord::crypto::SignatureAlgorithm::BLS},
{MULTISIG_EDDSA_SCHEME, concord::crypto::SignatureAlgorithm::EdDSA},
};
auto currentType = getLatestCryptoSystem()->getType();
Expand Down
4 changes: 2 additions & 2 deletions bftengine/src/bftengine/KeyExchangeManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ void KeyExchangeManager::registerNewKeyPair(uint16_t repID,
candidate_private_keys_.generated.clear();
// erasing seqnum from the map
seq_candidate_map_.erase(sn);
ConcordAssert(private_keys_.key_data().generated.pub == kemsg.pubkey);
ConcordAssert(private_keys_.key_data().generated.pub == pubkey);
private_keys_.onKeyExchange(cid, sn);
for (auto e : registryToExchange_) e->onPrivateKeyExchange(private_keys_.key_data().keys[sn], kemsg.pubkey, sn);
for (auto e : registryToExchange_) e->onPrivateKeyExchange(private_keys_.key_data().keys[sn], pubkey, sn);
metrics_->self_key_exchange_counter++;
}

Expand Down
12 changes: 0 additions & 12 deletions bftengine/src/bftengine/ReplicaBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,6 @@ class ReplicaBase {
}
}

/*bool validateMessage(MessageBase* msg) {
try {
if (config_.debugStatisticsEnabled) DebugStatistics::onReceivedExMessage(msg->type());
msg->validate(*repsInfo);
return true;
} catch (std::exception& e) {
onReportAboutInvalidMessage(msg, e.what());
return false;
}
}*/

protected:
static const uint16_t ALL_OTHER_REPLICAS = UINT16_MAX;

Expand Down
2 changes: 1 addition & 1 deletion bftengine/src/bftengine/ReplicaImp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4437,7 +4437,7 @@ ReplicaImp::ReplicaImp(bool firstTime,
concord::crypto::KeyFormat::PemFormat,
{{repsInfo->getIdOfOperator(),
ReplicaConfig::instance().getOperatorPublicKey(),
concord::crypto::KeyFormat::PemFormat}}
concord::crypto::KeyFormat::PemFormat}},
*repsInfo);
viewsManager = new ViewsManager(repsInfo);
} else {
Expand Down
5 changes: 1 addition & 4 deletions bftengine/src/bftengine/ReplicaImp.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,6 @@ class ReplicaImp : public InternalReplicaApi, public ReplicaForStateTransfer {

void recoverRequests();

template <typename MessageType>
bool validateMessage(MessageType* msg);

std::function<bool(MessageBase*)> getMessageValidator();

// InternalReplicaApi
Expand Down Expand Up @@ -633,7 +630,7 @@ class ReplicaImp : public InternalReplicaApi, public ReplicaForStateTransfer {
void addTimers();
void startConsensusProcess(PrePrepareMsgUPtr pp, bool isCreatedEarlier);
void startConsensusProcess(PrePrepareMsgUPtr pp);
void clearClientRequestQueue();
size_t clearClientRequestQueue();
/**
* Updates both seqNumInfo and slow_path metric
* @param seqNumInfo
Expand Down
35 changes: 18 additions & 17 deletions bftengine/src/bftengine/SigManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,15 @@ SigManager* SigManager::instance() {

void SigManager::reset(std::shared_ptr<SigManager> other) { s_sm = other; }

std::shared_ptr<SigManager> SigManager::init(ReplicaId myId,
const Key& mySigPrivateKey,
const std::set<std::pair<PrincipalId, const std::string>>& publicKeysOfReplicas,
KeyFormat replicasKeysFormat,
const std::set<std::pair<const std::string, std::set<uint16_t>>>* publicKeysOfClients,
KeyFormat clientsKeysFormat,
const std::optional<std::tuple<PrincipalId, Key, concord::crypto::KeyFormat>>& operatorKey,
const ReplicasInfo& replicasInfo) {
std::shared_ptr<SigManager> SigManager::init(
ReplicaId myId,
const Key& mySigPrivateKey,
const std::set<std::pair<PrincipalId, const std::string>>& publicKeysOfReplicas,
KeyFormat replicasKeysFormat,
const std::set<std::pair<const std::string, std::set<uint16_t>>>* publicKeysOfClients,
KeyFormat clientsKeysFormat,
const std::optional<std::tuple<PrincipalId, Key, concord::crypto::KeyFormat>>& operatorKey,
const ReplicasInfo& replicasInfo) {
vector<pair<Key, KeyFormat>> publickeys;
map<PrincipalId, SigManager::KeyIndex> publicKeysMapping;
size_t lowBound, highBound;
Expand Down Expand Up @@ -102,14 +103,14 @@ std::shared_ptr<SigManager> SigManager::init(ReplicaId myId,
}

LOG_INFO(GL, "Done Compute Start ctor for SigManager with " << KVLOG(publickeys.size(), publicKeysMapping.size()));
auto ret = std::shared_ptr<SigManager>{new SigManager(
myId,
make_pair(mySigPrivateKey, replicasKeysFormat),
publickeys,
publicKeysMapping,
((ReplicaConfig::instance().clientTransactionSigningEnabled) && (publicKeysOfClients != nullptr)),
operatorKey,
replicasInfo)};
auto ret = std::shared_ptr<SigManager>{
new SigManager(myId,
make_pair(mySigPrivateKey, replicasKeysFormat),
publickeys,
publicKeysMapping,
((ReplicaConfig::instance().clientTransactionSigningEnabled) && (publicKeysOfClients != nullptr)),
operatorKey,
replicasInfo)};

reset(ret);
return ret;
Expand All @@ -120,7 +121,7 @@ SigManager::SigManager(PrincipalId myId,
const vector<pair<Key, KeyFormat>>& publickeys,
const map<PrincipalId, KeyIndex>& publicKeysMapping,
bool clientTransactionSigningEnabled,
const std::optional<std::tuple<PrincipalId, Key, concord::crypto::KeyFormat>>& operatorKey
const std::optional<std::tuple<PrincipalId, Key, concord::crypto::KeyFormat>>& operatorKey,
const ReplicasInfo& replicasInfo)
: myId_(myId),
clientTransactionSigningEnabled_(clientTransactionSigningEnabled),
Expand Down
2 changes: 1 addition & 1 deletion bftengine/src/bftengine/SigManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
#include "PrimitiveTypes.hpp"
#include "util/assertUtils.hpp"
#include "util/Metrics.hpp"
#include "util/memory.hpp"
#include "crypto/crypto.hpp"
#include "crypto/signer.hpp"
#include "crypto/verifier.hpp"
#include "memory.hpp"
#include "SysConsts.hpp"
#include <utility>
#include <vector>
Expand Down
1 change: 1 addition & 0 deletions bftengine/src/bftengine/ValidationOnlyIdentityManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ ValidationOnlyIdentityManager::ValidationOnlyIdentityManager(
publickeys,
publicKeysMapping,
false,
{},
replicasInfo) {}

bool ValidationOnlyIdentityManager::verifySig(
Expand Down
3 changes: 2 additions & 1 deletion bftengine/src/bftengine/ValidationOnlyIdentityManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ namespace bftEngine::impl {
/* This class is a hack to enable the validation of replica signatures
using fixed keys without initializing a CryptoManager object.
Messages such as CheckpointMsg use a global singleton to expose their validation logic,
thus an instance of this class can be used to succeed in performing the validation.
thus an instance of this class can be registered as a global SigManager to succeed in performing CheckpointMsg
validations.
*/
class ValidationOnlyIdentityManager : public SigManager {
public:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ std::shared_ptr<bftEngine::impl::SigManager> createSigManagerWithTransactionSign
replicasKeysFormat,
publicKeysOfClients,
concord::crypto::KeyFormat::HexaDecimalStrippedFormat,
{},
replicasInfo);
}

Expand Down
3 changes: 2 additions & 1 deletion bftengine/src/preprocessor/tests/preprocessor_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "ReplicaConfig.hpp"
#include "IncomingMsgsStorageImp.hpp"
#include "gtest/gtest.h"
#include "threshsign/eddsa/EdDSAMultisigFactory.h"
#include "crypto/threshsign/eddsa/EdDSAMultisigFactory.h"
#include "CryptoManager.hpp"
#include "tests/messages/helper.hpp"
#include "tests/config/test_comm_config.hpp"
Expand Down Expand Up @@ -216,6 +216,7 @@ void setUpConfiguration_4() {
concord::crypto::KeyFormat::HexaDecimalStrippedFormat,
nullptr,
concord::crypto::KeyFormat::HexaDecimalStrippedFormat,
{},
*replicasInfo[i].get());
cryptoManager[i] =
CryptoManager::init(std::make_unique<TestMultisigCryptoSystem>(i, publicKeysVector, replicaPrivKeys.at(i)));
Expand Down
1 change: 1 addition & 0 deletions bftengine/tests/clientsManager/ClientsManager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ static void resetSigManager() {
kKeyFormatForTesting,
&kInitialPublicKeysOfClientsForTesting,
kKeyFormatForTesting,
{},
*sigManagerReplicasInfoForTesting);
}

Expand Down
2 changes: 1 addition & 1 deletion bftengine/tests/messages/helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class TestSigManager : public bftEngine::impl::SigManager {
const ReplicasInfo& replicasInfo,
bool transactionSigningEnabled = false)
: bftEngine::impl::SigManager(
myId, mySigPrivateKey, publickeys, publicKeysMapping, transactionSigningEnabled, replicasInfo) {}
myId, mySigPrivateKey, publickeys, publicKeysMapping, transactionSigningEnabled, {}, replicasInfo) {}

static std::shared_ptr<TestSigManager> init(size_t myId,
std::string& myPrivateKey,
Expand Down
2 changes: 1 addition & 1 deletion bftengine/tests/messages/helper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "crypto/threshsign/IPublicKey.h"
#include "CryptoManager.hpp"
#include "SigManager.hpp"
#include "threshsign/eddsa/EdDSAMultisigFactory.h"
#include "crypto/threshsign/eddsa/EdDSAMultisigFactory.h"

using bftEngine::impl::ReplicasInfo;

Expand Down
2 changes: 1 addition & 1 deletion libs/crypto/src/threshsign/ThresholdSignaturesTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Cryptosystem::Cryptosystem(const std::string& sysType,
subtype_(sysSubtype),
numSigners_(sysNumSigners),
threshold_(sysThreshold),
signerID_(NID),
signerID_(INVALID_SIGNER_ID),
publicKey_("uninitialized") {
if (!isValidCryptosystemSelection(sysType, sysSubtype, sysNumSigners, sysThreshold)) {
throw std::runtime_error(
Expand Down
3 changes: 0 additions & 3 deletions tests/apollo/test_skvbc_dbsnapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -595,9 +595,6 @@ async def test_db_checkpoint_creation_with_wedge(self, bft_network):
op = operator.Operator(
bft_network.config, client, bft_network.builddir)
await op.wedge()
#from time import sleep
#sleep(1)
#return
await bft_network.wait_for_stable_checkpoint( bft_network.all_replicas(), stable_seqnum = (checkpoint_before + 2) * 150)
await self.validate_stop_on_wedge_point(bft_network, skvbc=skvbc, fullWedge=True)
# verify that snapshot is created on wedge point
Expand Down
15 changes: 7 additions & 8 deletions tests/apollo/test_skvbc_reconfiguration.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ async def test_key_exchange(self, bft_network):
client = bft_network.random_client()
skvbc = kvbc.SimpleKVBCProtocol(bft_network)

await self.send_and_check_key_exchange(target_replica=0, bft_network=bft_network, client=client)
await self.send_and_wait_for_key_exchange_execution(target_replica=0, bft_network=bft_network, client=client)

await skvbc.fill_and_wait_for_checkpoint(initial_nodes=bft_network.all_replicas(),
num_of_checkpoints_to_add=3,
Expand Down Expand Up @@ -667,7 +667,7 @@ async def test_key_exchange_with_restart(self, bft_network):
num_of_checkpoints_to_add=2,
verify_checkpoint_persistency=False)

await self.send_and_check_key_exchange(target_replica=1, bft_network=bft_network, client=client)
await self.send_and_wait_for_key_exchange_execution(target_replica=1, bft_network=bft_network, client=client)

await skvbc.fill_and_wait_for_checkpoint(initial_nodes=bft_network.all_replicas(),
num_of_checkpoints_to_add=2,
Expand Down Expand Up @@ -714,15 +714,15 @@ async def test_key_exchange_with_file_backup(self, bft_network):
num_of_checkpoints_to_add=2,
verify_checkpoint_persistency=False)

await self.send_and_check_key_exchange(target_replica=1, bft_network=bft_network, client=client)
await self.send_and_wait_for_key_exchange_execution(target_replica=1, bft_network=bft_network, client=client)

await skvbc.fill_and_wait_for_checkpoint(initial_nodes=bft_network.all_replicas(),
num_of_checkpoints_to_add=2,
verify_checkpoint_persistency=False)

#backup gen-sec.1 file
copy2(bft_network.testdir + "/gen-sec.1" , bft_network.testdir + "/gen-sec.1.bak")
await self.send_and_check_key_exchange(target_replica=1, bft_network=bft_network, client=client)
await self.send_and_wait_for_key_exchange_execution(target_replica=1, bft_network=bft_network, client=client)
bft_network.stop_replica(1)
# restore gen-sec.1 file
copy2(bft_network.testdir + "/gen-sec.1.bak" , bft_network.testdir + "/gen-sec.1")
Expand All @@ -733,7 +733,7 @@ async def test_key_exchange_with_file_backup(self, bft_network):
verify_checkpoint_persistency=False,
assert_state_transfer_not_started=False)

async def send_and_check_key_exchange(self, target_replica, bft_network, client):
async def send_and_wait_for_key_exchange_execution(self, target_replica, bft_network, client):
with log.start_action(action_type="send_and_check_key_exchange",
target_replica=target_replica):
sent_key_exchange_counter_before = 0
Expand All @@ -745,8 +745,7 @@ async def send_and_check_key_exchange(self, target_replica, bft_network, client)
# public_key_exchange_for_peer_counter_before = await bft_network.metrics.get(0, *["KeyExchangeManager", "Counters", "public_key_exchange_for_peer"])
except:
log.log_message(message_type=f"Replica {target_replica} was unable to query KeyExchangeMetrics, assuming zero")

log.log_message(f"sending key exchange command to replica {target_replica}")

op = operator.Operator(bft_network.config, client, bft_network.builddir)
await op.key_exchange([target_replica])

Expand Down Expand Up @@ -1736,7 +1735,7 @@ async def test_add_nodes_with_failures(self, bft_network):
bft_network.restart_clients(generate_tx_signing_keys=True, restart_replicas=False)

live_replicas = bft_network.all_replicas(without=replicas_for_st.union(crashed_replica))
await bft_network.check_initital_key_exchange(stop_replicas=False, full_key_exchange=False, replicas_to_start=live_replicas)
await bft_network.check_initial_key_exchange(stop_replicas=False, full_key_exchange=False, replicas_to_start=live_replicas)
await skvbc.send_n_kvs_sequentially(601, description=f'New epoch without {crashed_replica.union(replicas_for_st)}')
bft_network.start_replicas(crashed_replica)
await self.validate_initial_key_exchange(bft_network, crashed_replica, metrics_id="self_key_exchange",
Expand Down
Loading

0 comments on commit a880c00

Please sign in to comment.