From bdb80e8c1e1e8533e26a4c53b0c0c5b7900d6250 Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Thu, 4 Apr 2019 15:07:12 +0300 Subject: [PATCH 01/22] Some state of work Signed-off-by: Fedor Muratov --- irohad/consensus/CMakeLists.txt | 1 + .../yac/impl/supermajority_checker_bft.cpp | 5 ++ .../yac/impl/supermajority_checker_bft.hpp | 3 + .../yac/impl/supermajority_checker_cft.cpp | 5 ++ .../yac/impl/supermajority_checker_cft.hpp | 3 + .../consensus/yac/supermajority_checker.hpp | 9 +++ irohad/main/application.cpp | 8 ++- irohad/main/impl/on_demand_ordering_init.cpp | 14 ++++- irohad/main/impl/on_demand_ordering_init.hpp | 9 ++- irohad/ordering/CMakeLists.txt | 1 + .../kick_out_proposal_creation_strategy.cpp | 63 +++++++++++++++++++ .../kick_out_proposal_creation_strategy.hpp | 53 ++++++++++++++++ .../ordering/impl/on_demand_ordering_gate.cpp | 8 ++- .../ordering/impl/on_demand_ordering_gate.hpp | 5 ++ .../impl/on_demand_ordering_service_impl.cpp | 20 +++--- .../impl/on_demand_ordering_service_impl.hpp | 14 ++++- .../impl/on_demand_os_server_grpc.cpp | 7 ++- .../impl/on_demand_os_server_grpc.hpp | 4 +- .../ordering/on_demand_ordering_service.hpp | 13 +++- irohad/ordering/on_demand_os_transport.hpp | 53 ++++++++++++++-- ...ing_service_proposal_creation_strategy.hpp | 56 +++++++++++++++++ libs/common/pointer_utils.hpp | 27 ++++++++ .../interfaces/common_objects/impl/peer.cpp | 5 ++ .../interfaces/common_objects/peer.hpp | 5 ++ .../yac/mock_yac_supermajority_checker.hpp | 1 + test/module/irohad/ordering/CMakeLists.txt | 7 +++ ...ck_out_proposal_creation_strategy_test.cpp | 59 +++++++++++++++++ 27 files changed, 427 insertions(+), 31 deletions(-) create mode 100644 irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp create mode 100644 irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp create mode 100644 irohad/ordering/ordering_service_proposal_creation_strategy.hpp create mode 100644 libs/common/pointer_utils.hpp create mode 100644 test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp diff --git a/irohad/consensus/CMakeLists.txt b/irohad/consensus/CMakeLists.txt index 50cf52ad6b..d743d1ac77 100644 --- a/irohad/consensus/CMakeLists.txt +++ b/irohad/consensus/CMakeLists.txt @@ -8,6 +8,7 @@ add_library(consensus_round ) target_link_libraries(consensus_round boost + shared_model_utils ) add_library(gate_object diff --git a/irohad/consensus/yac/impl/supermajority_checker_bft.cpp b/irohad/consensus/yac/impl/supermajority_checker_bft.cpp index f46b2126bd..7c0558a162 100644 --- a/irohad/consensus/yac/impl/supermajority_checker_bft.cpp +++ b/irohad/consensus/yac/impl/supermajority_checker_bft.cpp @@ -19,6 +19,11 @@ namespace iroha { agreed, all, detail::kSupermajorityCheckerKfPlus1Bft); } + bool SupermajorityCheckerBft::hasMajority(PeersNumberType voted, + PeersNumberType all) const { + return checkKfPlus1Supermajority(voted, all, 2); + } + bool SupermajorityCheckerBft::canHaveSupermajority( const VoteGroups &votes, PeersNumberType all) const { const PeersNumberType largest_group = diff --git a/irohad/consensus/yac/impl/supermajority_checker_bft.hpp b/irohad/consensus/yac/impl/supermajority_checker_bft.hpp index fbba1acb28..1fe4a7401d 100644 --- a/irohad/consensus/yac/impl/supermajority_checker_bft.hpp +++ b/irohad/consensus/yac/impl/supermajority_checker_bft.hpp @@ -23,6 +23,9 @@ namespace iroha { bool hasSupermajority(PeersNumberType current, PeersNumberType all) const override; + bool hasMajority(PeersNumberType voted, + PeersNumberType all) const override; + bool canHaveSupermajority(const VoteGroups &votes, PeersNumberType all) const override; }; diff --git a/irohad/consensus/yac/impl/supermajority_checker_cft.cpp b/irohad/consensus/yac/impl/supermajority_checker_cft.cpp index 4af1123a9e..fcb67e601f 100644 --- a/irohad/consensus/yac/impl/supermajority_checker_cft.cpp +++ b/irohad/consensus/yac/impl/supermajority_checker_cft.cpp @@ -19,6 +19,11 @@ namespace iroha { agreed, all, detail::kSupermajorityCheckerKfPlus1Cft); } + bool SupermajorityCheckerCft::hasMajority(PeersNumberType voted, + PeersNumberType all) const{ + return hasSupermajority(voted, all); + } + bool SupermajorityCheckerCft::canHaveSupermajority( const VoteGroups &votes, PeersNumberType all) const { const PeersNumberType largest_group = diff --git a/irohad/consensus/yac/impl/supermajority_checker_cft.hpp b/irohad/consensus/yac/impl/supermajority_checker_cft.hpp index f9424411ca..e24bf650b5 100644 --- a/irohad/consensus/yac/impl/supermajority_checker_cft.hpp +++ b/irohad/consensus/yac/impl/supermajority_checker_cft.hpp @@ -23,6 +23,9 @@ namespace iroha { bool hasSupermajority(PeersNumberType current, PeersNumberType all) const override; + bool hasMajority(PeersNumberType voted, + PeersNumberType all) const override; + bool canHaveSupermajority(const VoteGroups &votes, PeersNumberType all) const override; }; diff --git a/irohad/consensus/yac/supermajority_checker.hpp b/irohad/consensus/yac/supermajority_checker.hpp index d3acd904b7..c57634b56b 100644 --- a/irohad/consensus/yac/supermajority_checker.hpp +++ b/irohad/consensus/yac/supermajority_checker.hpp @@ -49,6 +49,15 @@ namespace iroha { virtual bool hasSupermajority(PeersNumberType current, PeersNumberType all) const = 0; + /** + * Check if majority of votes is achieved + * @param voted - number of voted peers + * @param all - number of all peers in network + * @return true if majority is reached + */ + virtual bool hasMajority(PeersNumberType voted, + PeersNumberType all) const = 0; + /** * Check if supermajority is possible * @param voted - numbers of peers voted for each option diff --git a/irohad/main/application.cpp b/irohad/main/application.cpp index bb6c7e3287..28c708a1ce 100644 --- a/irohad/main/application.cpp +++ b/irohad/main/application.cpp @@ -35,6 +35,7 @@ #include "multi_sig_transactions/transport/mst_transport_stub.hpp" #include "network/impl/block_loader_impl.hpp" #include "network/impl/peer_communication_service_impl.hpp" +#include "ordering/impl/kick_out_proposal_creation_strategy.hpp" #include "ordering/impl/on_demand_common.hpp" #include "ordering/impl/on_demand_ordering_gate.hpp" #include "pending_txs_storage/impl/pending_txs_storage_impl.hpp" @@ -374,6 +375,10 @@ void Irohad::initOrderingGate() { return reject_delay; }; + std::shared_ptr proposal_strategy = + std::make_shared( + getSupermajorityChecker(kConsensusConsistencyModel)); + ordering_gate = ordering_init.initOrderingGate(max_proposal_size_, proposal_delay_, @@ -387,7 +392,8 @@ void Irohad::initOrderingGate() { proposal_factory, persistent_cache, delay, - log_manager_->getChild("Ordering")); + log_manager_->getChild("Ordering"), + proposal_strategy); log_->info("[Init] => init ordering gate - [{}]", logger::logBool(ordering_gate)); } diff --git a/irohad/main/impl/on_demand_ordering_init.cpp b/irohad/main/impl/on_demand_ordering_init.cpp index 00214a0dcd..7914e13369 100644 --- a/irohad/main/impl/on_demand_ordering_init.cpp +++ b/irohad/main/impl/on_demand_ordering_init.cpp @@ -13,6 +13,7 @@ #include "interfaces/common_objects/types.hpp" #include "logger/logger.hpp" #include "logger/logger_manager.hpp" +#include "ordering/impl/kick_out_proposal_creation_strategy.hpp" #include "ordering/impl/on_demand_common.hpp" #include "ordering/impl/on_demand_connection_manager.hpp" #include "ordering/impl/on_demand_ordering_gate.hpp" @@ -232,12 +233,15 @@ namespace iroha { std::inserter(hashes, hashes.end())); }); return ordering::OnDemandOrderingGate::BlockEvent{ - ordering::nextCommitRound(commit.round), hashes}; + ordering::nextCommitRound(commit.round), + hashes, + commit.ledger_state}; }, [](const auto ¬hing) -> ordering::OnDemandOrderingGate::BlockRoundEventType { return ordering::OnDemandOrderingGate::EmptyEvent{ - ordering::nextRejectRound(nothing.round)}; + ordering::nextRejectRound(nothing.round), + nothing.ledger_state}; }); }; @@ -261,11 +265,13 @@ namespace iroha { std::shared_ptr proposal_factory, std::shared_ptr tx_cache, + std::shared_ptr creation_strategy, const logger::LoggerManagerTreePtr &ordering_log_manager) { return std::make_shared( max_number_of_transactions, std::move(proposal_factory), std::move(tx_cache), + creation_strategy, ordering_log_manager->getChild("Service")->getLogger()); } @@ -294,10 +300,12 @@ namespace iroha { std::shared_ptr tx_cache, std::function delay_func, - logger::LoggerManagerTreePtr ordering_log_manager) { + logger::LoggerManagerTreePtr ordering_log_manager, + std::shared_ptr creation_strategy) { auto ordering_service = createService(max_number_of_transactions, proposal_factory, tx_cache, + creation_strategy, ordering_log_manager); service = std::make_shared( ordering_service, diff --git a/irohad/main/impl/on_demand_ordering_init.hpp b/irohad/main/impl/on_demand_ordering_init.hpp index 554ae4d7ef..1324db166f 100644 --- a/irohad/main/impl/on_demand_ordering_init.hpp +++ b/irohad/main/impl/on_demand_ordering_init.hpp @@ -20,7 +20,7 @@ #include "ordering/impl/on_demand_os_server_grpc.hpp" #include "ordering/impl/ordering_gate_cache/ordering_gate_cache.hpp" #include "ordering/on_demand_ordering_service.hpp" -#include "ordering/on_demand_os_transport.hpp" +#include "ordering/ordering_service_proposal_creation_strategy.hpp" namespace iroha { namespace network { @@ -86,6 +86,7 @@ namespace iroha { std::shared_ptr proposal_factory, std::shared_ptr tx_cache, + std::shared_ptr creation_strategy, const logger::LoggerManagerTreePtr &ordering_log_manager); public: @@ -98,7 +99,8 @@ namespace iroha { /** * Initializes on-demand ordering gate and ordering sevice components * - * @param max_number_of_transactions maximum number of transaction in a proposal + * @param max_number_of_transactions maximum number of transaction in a + * proposal * @param delay timeout for ordering service response on proposal request * @param initial_hashes seeds for peer list permutations for first k * rounds they are required since hash of block i defines round i + k @@ -136,7 +138,8 @@ namespace iroha { std::shared_ptr tx_cache, std::function delay_func, - logger::LoggerManagerTreePtr ordering_log_manager); + logger::LoggerManagerTreePtr ordering_log_manager, + std::shared_ptr creation_strategy); /// gRPC service for ordering service std::shared_ptr service; diff --git a/irohad/ordering/CMakeLists.txt b/irohad/ordering/CMakeLists.txt index 2efd5c8fab..88e3bf7bda 100644 --- a/irohad/ordering/CMakeLists.txt +++ b/irohad/ordering/CMakeLists.txt @@ -11,6 +11,7 @@ target_link_libraries(on_demand_common add_library(on_demand_ordering_service impl/on_demand_ordering_service_impl.cpp + impl/kick_out_proposal_creation_strategy.cpp ) target_link_libraries(on_demand_ordering_service diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp new file mode 100644 index 0000000000..795f71bf51 --- /dev/null +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp @@ -0,0 +1,63 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "ordering/impl/kick_out_proposal_creation_strategy.hpp" + +#include +#include "interfaces/common_objects/peer.hpp" + +using namespace iroha::ordering; + +KickOutProposalCreationStrategy::KickOutProposalCreationStrategy( + std::shared_ptr majority_checker) + : majority_checker_(majority_checker) {} + +bool KickOutProposalCreationStrategy::onCollaborationOutcome( + RoundType round, const PeerList &peers) { + uint64_t counter = 0; + std::for_each(last_requested_.begin(), + last_requested_.end(), + [&round, &counter](const auto &elem) { + if (elem.second >= round) { + counter++; + } + }); + auto has_majority = + majority_checker_->hasMajority(counter, last_requested_.size()); + updateCurrentState(peers); + return has_majority; +} + +boost::optional +KickOutProposalCreationStrategy::onProposal(PeerType who, + RoundType requested_round) { + auto iter = last_requested_.find(who); + if (iter == last_requested_.end()) { + last_requested_.insert({who, requested_round}); + } else { + if (iter->second < requested_round) { + iter->second = requested_round; + } + } + + return boost::none; +} + +void KickOutProposalCreationStrategy::updateCurrentState( + const PeerList &peers) { + last_requested_ = + std::accumulate(peers.begin(), + peers.end(), + RoundCollectionType{}, + [this](auto collection, const auto &peer) { + auto iter = last_requested_.find(peer); + if (iter != last_requested_.end()) { + collection.insert(*iter); + } else { + collection.insert({peer, RoundType{0, 0}}); + } + return std::move(collection); + }); +} diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp new file mode 100644 index 0000000000..2a2875abf0 --- /dev/null +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp @@ -0,0 +1,53 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef IROHA_KICK_OUT_PROPOSAL_CREATION_STRATEGY_HPP +#define IROHA_KICK_OUT_PROPOSAL_CREATION_STRATEGY_HPP + +#include "ordering/ordering_service_proposal_creation_strategy.hpp" + +#include +#include +#include "common/pointer_utils.hpp" +#include "consensus/round.hpp" +#include "consensus/yac/supermajority_checker.hpp" + +namespace iroha { + namespace ordering { + + class KickOutProposalCreationStrategy : public ProposalCreationStrategy { + public: + using SupermajorityCheckerType = + iroha::consensus::yac::SupermajorityChecker; + KickOutProposalCreationStrategy( + std::shared_ptr); + + bool onCollaborationOutcome(RoundType round, + const PeerList &peers) override; + + boost::optional onProposal(PeerType who, + RoundType requested_round) override; + + private: + using RoundCollectionType = std::unordered_map< + PeerType, + RoundType, + DereferenceHash>; + + /** + * Update peers state with new peers. + * Note: the method removes peers which are not participating in consensus + * and adds new with minimal round + * @param peers - list of peers which fetched in the last round + */ + void updateCurrentState(const PeerList &peers); + + std::shared_ptr majority_checker_; + RoundCollectionType last_requested_; + }; + } // namespace ordering +} // namespace iroha + +#endif // IROHA_KICK_OUT_PROPOSAL_CREATION_STRATEGY_HPP diff --git a/irohad/ordering/impl/on_demand_ordering_gate.cpp b/irohad/ordering/impl/on_demand_ordering_gate.cpp index feeb64d8e7..442c1f887e 100644 --- a/irohad/ordering/impl/on_demand_ordering_gate.cpp +++ b/irohad/ordering/impl/on_demand_ordering_gate.cpp @@ -55,8 +55,14 @@ OnDemandOrderingGate::OnDemandOrderingGate( return event.round; }); + auto ledger_state = + visit_in_place(event, [this, ¤t_round](const auto &event) { + return event.ledger_state; + }); + // notify our ordering service about new round - ordering_service_->onCollaborationOutcome(current_round); + ordering_service_->onCollaborationOutcome( + current_round, *ledger_state->ledger_peers); this->sendCachedTransactions(event); diff --git a/irohad/ordering/impl/on_demand_ordering_gate.hpp b/irohad/ordering/impl/on_demand_ordering_gate.hpp index d2a5592636..2a6ee2ca92 100644 --- a/irohad/ordering/impl/on_demand_ordering_gate.hpp +++ b/irohad/ordering/impl/on_demand_ordering_gate.hpp @@ -12,6 +12,7 @@ #include #include +#include "ametsuchi/ledger_state.hpp" #include "interfaces/common_objects/types.hpp" #include "interfaces/iroha_internal/proposal.hpp" #include "interfaces/iroha_internal/unsafe_proposal_factory.hpp" @@ -41,6 +42,8 @@ namespace iroha { /// hashes of processed transactions cache::OrderingGateCache::HashesSetType hashes; + std::shared_ptr ledger_state; + std::string toString() const; }; @@ -51,6 +54,8 @@ namespace iroha { /// next round number consensus::Round round; + std::shared_ptr ledger_state; + std::string toString() const; }; diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index ff11be2141..a4d6a375e2 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -31,6 +31,7 @@ OnDemandOrderingServiceImpl::OnDemandOrderingServiceImpl( std::shared_ptr proposal_factory, std::shared_ptr tx_cache, + std::shared_ptr proposal_creation_strategy, logger::LoggerPtr log, size_t number_of_proposals, const consensus::Round &initial_round) @@ -38,23 +39,25 @@ OnDemandOrderingServiceImpl::OnDemandOrderingServiceImpl( number_of_proposals_(number_of_proposals), proposal_factory_(std::move(proposal_factory)), tx_cache_(std::move(tx_cache)), - log_(std::move(log)) { - onCollaborationOutcome(initial_round); -} + proposal_creation_strategy_(std::move(proposal_creation_strategy)), + log_(std::move(log)) {} // -------------------------| OnDemandOrderingService |------------------------- void OnDemandOrderingServiceImpl::onCollaborationOutcome( - consensus::Round round) { + consensus::Round round, const PeerList &peers) { log_->info("onCollaborationOutcome => {}", round); - packNextProposals(round); + if (proposal_creation_strategy_->onCollaborationOutcome(round, peers)) { + packNextProposals(round); + } tryErase(round); } // ----------------------------| OdOsNotification |----------------------------- -void OnDemandOrderingServiceImpl::onBatches(CollectionType batches) { +void OnDemandOrderingServiceImpl::onBatches(CollectionType batches, + InitiatorPeerType from) { auto unprocessed_batches = boost::adaptors::filter(batches, [this](const auto &batch) { log_->debug("check batch {} for already processed transactions", @@ -73,12 +76,15 @@ void OnDemandOrderingServiceImpl::onBatches(CollectionType batches) { boost::optional< std::shared_ptr> -OnDemandOrderingServiceImpl::onRequestProposal(consensus::Round round) { +OnDemandOrderingServiceImpl::onRequestProposal(consensus::Round round, + InitiatorPeerType requester) { boost::optional< std::shared_ptr> result; { std::shared_lock lock(proposals_mutex_); + // todo add force initialization of proposal + proposal_creation_strategy_->onProposal(requester, round); auto it = proposal_map_.find(round); result = boost::make_optional(it != proposal_map_.end(), it->second); } diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp index 3095dff569..a86b8c1fa2 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp @@ -18,6 +18,7 @@ // TODO 2019-03-15 andrei: IR-403 Separate BatchHashEquality and MstState #include "multi_sig_transactions/state/mst_state.hpp" #include "ordering/impl/on_demand_common.hpp" +#include "ordering/ordering_service_proposal_creation_strategy.hpp" namespace iroha { namespace ametsuchi { @@ -54,20 +55,22 @@ namespace iroha { std::shared_ptr proposal_factory, std::shared_ptr tx_cache, + std::shared_ptr proposal_creation_strategy, logger::LoggerPtr log, size_t number_of_proposals = 3, const consensus::Round &initial_round = {2, kFirstRejectRound}); // --------------------- | OnDemandOrderingService |_--------------------- - void onCollaborationOutcome(consensus::Round round) override; + void onCollaborationOutcome(consensus::Round round, + const PeerList &peers) override; // ----------------------- | OdOsNotification | -------------------------- - void onBatches(CollectionType batches) override; + void onBatches(CollectionType batches, InitiatorPeerType from) override; boost::optional> onRequestProposal( - consensus::Round round) override; + consensus::Round round, InitiatorPeerType requester) override; private: /** @@ -122,6 +125,11 @@ namespace iroha { */ std::shared_ptr tx_cache_; + /** + * + */ + std::shared_ptr proposal_creation_strategy_; + /** * Logger instance */ diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.cpp b/irohad/ordering/impl/on_demand_os_server_grpc.cpp index d72392dc18..8c97fd3a62 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.cpp @@ -18,7 +18,7 @@ using namespace iroha::ordering; using namespace iroha::ordering::transport; OnDemandOsServerGrpc::OnDemandOsServerGrpc( - std::shared_ptr ordering_service, + std::shared_ptr ordering_service, std::shared_ptr transaction_factory, std::shared_ptr batch_parser, @@ -84,7 +84,7 @@ grpc::Status OnDemandOsServerGrpc::SendBatches( return acc; }); - ordering_service_->onBatches(std::move(batches)); + ordering_service_->onBatches(std::move(batches), nullptr); return ::grpc::Status::OK; } @@ -94,7 +94,8 @@ grpc::Status OnDemandOsServerGrpc::RequestProposal( const proto::ProposalRequest *request, proto::ProposalResponse *response) { ordering_service_->onRequestProposal( - {request->round().block_round(), request->round().reject_round()}) + {request->round().block_round(), request->round().reject_round()}, + nullptr) | [&](auto &&proposal) { *response->mutable_proposal() = std::move( static_cast(proposal.get()) diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.hpp b/irohad/ordering/impl/on_demand_os_server_grpc.hpp index 144fd9ab5d..b518b17109 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.hpp @@ -29,7 +29,7 @@ namespace iroha { iroha::protocol::Transaction>; OnDemandOsServerGrpc( - std::shared_ptr ordering_service, + std::shared_ptr ordering_service, std::shared_ptr transaction_factory, std::shared_ptr batch_parser, @@ -53,7 +53,7 @@ namespace iroha { shared_model::interface::types::SharedTxsCollectionType deserializeTransactions(const proto::BatchesRequest *request); - std::shared_ptr ordering_service_; + std::shared_ptr ordering_service_; std::shared_ptr transaction_factory_; std::shared_ptr diff --git a/irohad/ordering/on_demand_ordering_service.hpp b/irohad/ordering/on_demand_ordering_service.hpp index 9c2a3601e2..bfbf1dee17 100644 --- a/irohad/ordering/on_demand_ordering_service.hpp +++ b/irohad/ordering/on_demand_ordering_service.hpp @@ -8,19 +8,28 @@ #include "ordering/on_demand_os_transport.hpp" +#include +#include "interfaces/common_objects/peer.hpp" + namespace iroha { namespace ordering { /** * Ordering Service aka OS which can share proposals by request */ - class OnDemandOrderingService : public transport::OdOsNotification { + class OnDemandOrderingService : public transport::OdNotificationOsSide { public: + /// shortcut for peer type + using PeerType = std::shared_ptr; + /// collection of peers type + using PeerList = std::vector; /** * Method which should be invoked on outcome of collaboration for round * @param round - proposal round which has started + * @param peers - list of peers in new round */ - virtual void onCollaborationOutcome(consensus::Round round) = 0; + virtual void onCollaborationOutcome(consensus::Round round, + const PeerList &peers) = 0; }; } // namespace ordering diff --git a/irohad/ordering/on_demand_os_transport.hpp b/irohad/ordering/on_demand_os_transport.hpp index e235c923df..38777a697c 100644 --- a/irohad/ordering/on_demand_os_transport.hpp +++ b/irohad/ordering/on_demand_os_transport.hpp @@ -25,11 +25,13 @@ namespace iroha { namespace ordering { namespace transport { - /** - * Notification interface of on demand ordering service. - */ - class OdOsNotification { - public: + namespace using_detail { + + /** + * Type of peer identity + */ + using PeerType = shared_model::interface::Peer; + /** * Type of stored proposals */ @@ -45,6 +47,16 @@ namespace iroha { * Type of inserted collections */ using CollectionType = std::vector; + } // namespace using_detail + + /** + * Notification interface of on demand ordering service. + */ + class OdOsNotification { + public: + using ProposalType = using_detail::ProposalType; + using TransactionBatchType = using_detail::TransactionBatchType; + using CollectionType = using_detail::CollectionType; /** * Callback on receiving transactions @@ -76,11 +88,40 @@ namespace iroha { * @return connection represented with OdOsNotification interface */ virtual std::unique_ptr create( - const shared_model::interface::Peer &to) = 0; + const using_detail::PeerType &to) = 0; virtual ~OdOsNotificationFactory() = default; }; + class OdNotificationOsSide { + public: + using InitiatorPeerType = std::shared_ptr; + using ProposalType = using_detail::ProposalType; + using TransactionBatchType = using_detail::TransactionBatchType; + using CollectionType = using_detail::CollectionType; + + /** + * Callback on receiving transactions + * @param batches - vector of passed transaction batches + * @param from - peer which sends batches + */ + virtual void onBatches(CollectionType batches, + InitiatorPeerType from) = 0; + + /** + * Callback on request about proposal + * @param round - number of collaboration round. + * Calculated as block_height + 1 + * @param requester - peer which requests proposal + * @return proposal for requested round + */ + virtual boost::optional> + onRequestProposal(consensus::Round round, + InitiatorPeerType requester) = 0; + + virtual ~OdNotificationOsSide() = default; + }; + } // namespace transport } // namespace ordering } // namespace iroha diff --git a/irohad/ordering/ordering_service_proposal_creation_strategy.hpp b/irohad/ordering/ordering_service_proposal_creation_strategy.hpp new file mode 100644 index 0000000000..38fbb927e6 --- /dev/null +++ b/irohad/ordering/ordering_service_proposal_creation_strategy.hpp @@ -0,0 +1,56 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef IROHA_ORDERING_SERVICE_PROPOSAL_CREATION_STRATEGY_HPP +#define IROHA_ORDERING_SERVICE_PROPOSAL_CREATION_STRATEGY_HPP + +#include +#include + +#include +#include "consensus/round.hpp" +#include "interfaces/common_objects/peer.hpp" + +namespace iroha { + namespace ordering { + + /** + * Class provides a strategy for creation proposals regarding to new rounds + * and requests from other peers + */ + class ProposalCreationStrategy { + public: + /// shortcut for peer type + using PeerType = std::shared_ptr; + /// shortcut for round type + using RoundType = consensus::Round; + /// collection of peers type + using PeerList = std::vector; + + /** + * Indicates the start of new round and provide information about proposal + * creation + * @param round - new consensus round + * @param peers - peers which participate in new round + * @return true, if proposal should be created in new round + */ + virtual bool onCollaborationOutcome(RoundType round, + const PeerList &peers) = 0; + + /** + * Notify the strategy about proposal request + * @param who - peer which requests the proposal + * @param requested_round - in which round proposal is requested + * @return round where proposal is required to be created immediately + */ + virtual boost::optional onProposal( + PeerType who, RoundType requested_round) = 0; + + virtual ~ProposalCreationStrategy() = default; + }; + } // namespace ordering +} // namespace iroha + +#endif // IROHA_ORDERING_SERVICE_PROPOSAL_CREATION_STRATEGY_HPP diff --git a/libs/common/pointer_utils.hpp b/libs/common/pointer_utils.hpp new file mode 100644 index 0000000000..66127c25bf --- /dev/null +++ b/libs/common/pointer_utils.hpp @@ -0,0 +1,27 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef IROHA_POINTER_UTILS_HPP +#define IROHA_POINTER_UTILS_HPP + +#include + +template +class DereferenceLess { + public: + bool operator()(const L &lhs, const R &rhs) const { + return Less()(lhs, rhs); + } +}; + +template +class DereferenceHash { + public: + size_t operator()(const Ptr &lhs) const { + return Hash()(*lhs); + } +}; + +#endif // IROHA_POINTER_UTILS_HPP diff --git a/shared_model/interfaces/common_objects/impl/peer.cpp b/shared_model/interfaces/common_objects/impl/peer.cpp index e09354940d..acd3b185a2 100644 --- a/shared_model/interfaces/common_objects/impl/peer.cpp +++ b/shared_model/interfaces/common_objects/impl/peer.cpp @@ -20,5 +20,10 @@ namespace shared_model { bool Peer::operator==(const ModelType &rhs) const { return address() == rhs.address() and pubkey() == rhs.pubkey(); } + + size_t Peer::PeerHash::operator()( + const shared_model::interface::Peer &t) const { + return std::hash()(t.address() + t.pubkey().toString()); + } } // namespace interface } // namespace shared_model diff --git a/shared_model/interfaces/common_objects/peer.hpp b/shared_model/interfaces/common_objects/peer.hpp index 03ae9f2175..c02e8e6b07 100644 --- a/shared_model/interfaces/common_objects/peer.hpp +++ b/shared_model/interfaces/common_objects/peer.hpp @@ -30,6 +30,11 @@ namespace shared_model { std::string toString() const override; bool operator==(const ModelType &rhs) const override; + + class PeerHash { + public: + size_t operator()(const Peer &) const; + }; }; } // namespace interface } // namespace shared_model diff --git a/test/module/irohad/consensus/yac/mock_yac_supermajority_checker.hpp b/test/module/irohad/consensus/yac/mock_yac_supermajority_checker.hpp index 42ab2a57f4..4c9b8b86ec 100644 --- a/test/module/irohad/consensus/yac/mock_yac_supermajority_checker.hpp +++ b/test/module/irohad/consensus/yac/mock_yac_supermajority_checker.hpp @@ -17,6 +17,7 @@ namespace iroha { class MockSupermajorityChecker : public SupermajorityChecker { public: MOCK_CONST_METHOD2(hasSupermajority, bool(PeersNumberType, PeersNumberType)); + MOCK_CONST_METHOD2(hasMajority, bool(PeersNumberType, PeersNumberType)); MOCK_CONST_METHOD2(canHaveSupermajority, bool(const VoteGroups &, PeersNumberType)); }; diff --git a/test/module/irohad/ordering/CMakeLists.txt b/test/module/irohad/ordering/CMakeLists.txt index 9ab4475f02..469472e14e 100644 --- a/test/module/irohad/ordering/CMakeLists.txt +++ b/test/module/irohad/ordering/CMakeLists.txt @@ -40,3 +40,10 @@ target_link_libraries(on_demand_cache_test on_demand_ordering_gate shared_model_interfaces_factories ) + +addtest(kick_out_proposal_creation_strategy_test + kick_out_proposal_creation_strategy_test.cpp + ) +target_link_libraries(kick_out_proposal_creation_strategy_test + on_demand_ordering_service + ) diff --git a/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp b/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp new file mode 100644 index 0000000000..2c3a1901f1 --- /dev/null +++ b/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp @@ -0,0 +1,59 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "ordering/impl/kick_out_proposal_creation_strategy.hpp" + +#include +#include +#include "module/irohad/consensus/yac/mock_yac_supermajority_checker.hpp" +#include "module/irohad/consensus/yac/yac_test_util.hpp" + +using namespace iroha::ordering; + +using testing::_; +using testing::Return; + +class KickOutProposalCreationStrategyTest : public testing::Test { + public: + void SetUp() override { + for (auto i = 0u; i < number_of_peers; ++i) { + peers.push_back(iroha::consensus::yac::makePeer(std::to_string(i))); + } + + supermajority_checker_ = + std::make_shared(); + strategy_ = std::make_shared( + supermajority_checker_); + } + + std::shared_ptr strategy_; + std::shared_ptr + supermajority_checker_; + + KickOutProposalCreationStrategy::PeerList peers; + size_t number_of_peers = 7; + size_t f = 2; +}; + +/** + * @given initialized kickOutStrategy + * @and onCollaborationOutcome is invoked for the first round + * @when onProposal calls F times with different peers for further rounds + * @then onCollaborationOutcome call returns true + */ +TEST_F(KickOutProposalCreationStrategyTest, OnNonMaliciousCase) { + EXPECT_CALL(*supermajority_checker_, hasMajority(0, 0)) + .WillOnce(Return(false)); + + ASSERT_EQ(false, strategy_->onCollaborationOutcome({1, 0}, peers)); + + for (auto i = 0u; i < f; ++i) { + strategy_->onProposal(peers.at(i), {2, 0}); + } + + EXPECT_CALL(*supermajority_checker_, hasMajority(f, number_of_peers)) + .WillOnce(Return(true)); + ASSERT_EQ(true, strategy_->onCollaborationOutcome({2, 0}, peers)); +} From a92e8c65c9d214983a07d049074a92827e7177c4 Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Fri, 5 Apr 2019 09:01:52 +0300 Subject: [PATCH 02/22] Fix minor issues Signed-off-by: Fedor Muratov --- .../kick_out_proposal_creation_strategy.cpp | 2 +- .../ordering/impl/on_demand_ordering_gate.cpp | 3 +- .../impl/on_demand_ordering_service_impl.cpp | 1 + irohad/ordering/on_demand_os_transport.hpp | 20 +-- .../yac/supermajority_checker_test.cpp | 17 +- ...ck_out_proposal_creation_strategy_test.cpp | 50 +++++- ...mock_on_demand_os_notification_os_side.hpp | 29 ++++ .../mock_proposal_creation_strategy.hpp | 25 +++ .../ordering/on_demand_ordering_gate_test.cpp | 50 ++++-- .../on_demand_os_server_grpc_test.cpp | 15 +- .../irohad/ordering/on_demand_os_test.cpp | 152 +++++++++++++----- .../module/irohad/ordering/ordering_mocks.hpp | 9 +- 12 files changed, 291 insertions(+), 82 deletions(-) create mode 100644 test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp create mode 100644 test/module/irohad/ordering/mock_proposal_creation_strategy.hpp diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp index 795f71bf51..dc110ff022 100644 --- a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp @@ -27,7 +27,7 @@ bool KickOutProposalCreationStrategy::onCollaborationOutcome( auto has_majority = majority_checker_->hasMajority(counter, last_requested_.size()); updateCurrentState(peers); - return has_majority; + return not has_majority; } boost::optional diff --git a/irohad/ordering/impl/on_demand_ordering_gate.cpp b/irohad/ordering/impl/on_demand_ordering_gate.cpp index 442c1f887e..ebaf1a09ac 100644 --- a/irohad/ordering/impl/on_demand_ordering_gate.cpp +++ b/irohad/ordering/impl/on_demand_ordering_gate.cpp @@ -81,7 +81,8 @@ OnDemandOrderingGate::~OnDemandOrderingGate() { events_subscription_.unsubscribe(); } -void OnDemandOrderingGate::propagateBatch( +void OnDemandOrderingGate:: +propagateBatch( std::shared_ptr batch) { cache_->addToBack({batch}); diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index a4d6a375e2..01391b21fa 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -34,6 +34,7 @@ OnDemandOrderingServiceImpl::OnDemandOrderingServiceImpl( std::shared_ptr proposal_creation_strategy, logger::LoggerPtr log, size_t number_of_proposals, + // todo remove initial round const consensus::Round &initial_round) : transaction_limit_(transaction_limit), number_of_proposals_(number_of_proposals), diff --git a/irohad/ordering/on_demand_os_transport.hpp b/irohad/ordering/on_demand_os_transport.hpp index 38777a697c..8deff762b2 100644 --- a/irohad/ordering/on_demand_os_transport.hpp +++ b/irohad/ordering/on_demand_os_transport.hpp @@ -25,7 +25,7 @@ namespace iroha { namespace ordering { namespace transport { - namespace using_detail { + namespace details { /** * Type of peer identity @@ -47,16 +47,16 @@ namespace iroha { * Type of inserted collections */ using CollectionType = std::vector; - } // namespace using_detail + } // namespace details /** * Notification interface of on demand ordering service. */ class OdOsNotification { public: - using ProposalType = using_detail::ProposalType; - using TransactionBatchType = using_detail::TransactionBatchType; - using CollectionType = using_detail::CollectionType; + using ProposalType = details::ProposalType; + using TransactionBatchType = details::TransactionBatchType; + using CollectionType = details::CollectionType; /** * Callback on receiving transactions @@ -88,17 +88,17 @@ namespace iroha { * @return connection represented with OdOsNotification interface */ virtual std::unique_ptr create( - const using_detail::PeerType &to) = 0; + const details::PeerType &to) = 0; virtual ~OdOsNotificationFactory() = default; }; class OdNotificationOsSide { public: - using InitiatorPeerType = std::shared_ptr; - using ProposalType = using_detail::ProposalType; - using TransactionBatchType = using_detail::TransactionBatchType; - using CollectionType = using_detail::CollectionType; + using InitiatorPeerType = std::shared_ptr; + using ProposalType = details::ProposalType; + using TransactionBatchType = details::TransactionBatchType; + using CollectionType = details::CollectionType; /** * Callback on receiving transactions diff --git a/test/module/irohad/consensus/yac/supermajority_checker_test.cpp b/test/module/irohad/consensus/yac/supermajority_checker_test.cpp index d26ad8d963..a2eaaf945c 100644 --- a/test/module/irohad/consensus/yac/supermajority_checker_test.cpp +++ b/test/module/irohad/consensus/yac/supermajority_checker_test.cpp @@ -11,7 +11,6 @@ #include #include "boost/tuple/tuple.hpp" -#include "consensus/yac/supermajority_checker.hpp" #include "logger/logger.hpp" #include "framework/test_logger.hpp" @@ -55,6 +54,10 @@ class SupermajorityCheckerTest return checker->hasSupermajority(current, all); } + bool hasMajority(PeersNumberType voted, PeersNumberType all) const { + return checker->hasMajority(voted, all); + } + bool canHaveSupermajority(const VoteGroups &votes, PeersNumberType all) const override { return checker->canHaveSupermajority(votes, all); @@ -72,13 +75,13 @@ INSTANTIATE_TEST_CASE_P(Instance, ::testing::Values(ConsistencyModel::kCft, ConsistencyModel::kBft), // empty argument for the macro - ); +); INSTANTIATE_TEST_CASE_P(Instance, BftSupermajorityCheckerTest, ::testing::Values(ConsistencyModel::kBft), // empty argument for the macro - ); +); /** * @given 2 participants @@ -88,7 +91,7 @@ INSTANTIATE_TEST_CASE_P(Instance, TEST_P(CftAndBftSupermajorityCheckerTest, SuperMajorityCheckWithSize2) { log_->info("-----------| F(x, 2), x in [0..3] |-----------"); - size_t A = 2; // number of all peers + size_t A = 2; // number of all peers for (size_t i = 0; i < 4; ++i) { if (i >= getSupermajority(A) // enough votes and i <= A // not more than total peers amount @@ -112,7 +115,7 @@ TEST_P(CftAndBftSupermajorityCheckerTest, SuperMajorityCheckWithSize2) { TEST_P(SupermajorityCheckerTest, SuperMajorityCheckWithSize4) { log_->info("-----------| F(x, 4), x in [0..5] |-----------"); - size_t A = 6; // number of all peers + size_t A = 6; // number of all peers for (size_t i = 0; i < 5; ++i) { if (i >= getSupermajority(A) // enough votes and i <= A // not more than total peers amount @@ -153,8 +156,8 @@ TEST_P(CftAndBftSupermajorityCheckerTest, OneLargeAndManySingleVoteGroups) { }; struct Case { - size_t V; // Voted peers - size_t A; // All peers + size_t V; // Voted peers + size_t A; // All peers }; for (const auto &c : std::initializer_list({{6, 7}, {8, 12}})) { diff --git a/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp b/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp index 2c3a1901f1..412563ac03 100644 --- a/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp +++ b/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp @@ -47,13 +47,59 @@ TEST_F(KickOutProposalCreationStrategyTest, OnNonMaliciousCase) { EXPECT_CALL(*supermajority_checker_, hasMajority(0, 0)) .WillOnce(Return(false)); - ASSERT_EQ(false, strategy_->onCollaborationOutcome({1, 0}, peers)); + ASSERT_EQ(true, strategy_->onCollaborationOutcome({1, 0}, peers)); for (auto i = 0u; i < f; ++i) { strategy_->onProposal(peers.at(i), {2, 0}); } EXPECT_CALL(*supermajority_checker_, hasMajority(f, number_of_peers)) - .WillOnce(Return(true)); + .WillOnce(Return(false)); + ASSERT_EQ(true, strategy_->onCollaborationOutcome({2, 0}, peers)); +} + +/** + * @given initialized kickOutStrategy + * @and onCollaborationOutcome is invoked for the first round + * @when onProposal calls F + 1 times with different peers for further rounds + * @then onCollaborationOutcome call returns false + */ +TEST_F(KickOutProposalCreationStrategyTest, OnMaliciousCase) { + EXPECT_CALL(*supermajority_checker_, hasMajority(0, 0)) + .WillOnce(Return(false)); + + ASSERT_EQ(true, strategy_->onCollaborationOutcome({1, 0}, peers)); + + auto requested = f + 1; + for (auto i = 0u; i < requested; ++i) { + strategy_->onProposal(peers.at(i), {2, 0}); + } + + EXPECT_CALL(*supermajority_checker_, hasMajority(requested, number_of_peers)) + .WillOnce(Return(true)); + ASSERT_EQ(false, strategy_->onCollaborationOutcome({2, 0}, peers)); +} + +/** + * @given initialized kickOutStrategy + * @and onCollaborationOutcome is invoked for the first round + * @when onProposal calls F + 1 times with one peer + * @then onCollaborationOutcome call returns true + */ +TEST_F(KickOutProposalCreationStrategyTest, RepeadedRequest) { + EXPECT_CALL(*supermajority_checker_, hasMajority(0, 0)) + .WillOnce(Return(false)); + + ASSERT_EQ(true, strategy_->onCollaborationOutcome({1, 0}, peers)); + + auto requested = f + 1; + for (auto i = 0u; i < requested; ++i) { + strategy_->onProposal(peers.at(0), {2, 0}); + } + EXPECT_CALL(*supermajority_checker_, hasMajority(1, number_of_peers)) + .WillOnce(Return(false)); ASSERT_EQ(true, strategy_->onCollaborationOutcome({2, 0}, peers)); } + + +/// todo test where peer out of the scope diff --git a/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp b/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp new file mode 100644 index 0000000000..149619e4ce --- /dev/null +++ b/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp @@ -0,0 +1,29 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef IROHA_MOCK_ON_DEMAND_OS_NOTIFICATION_OS_SIDE_HPP +#define IROHA_MOCK_ON_DEMAND_OS_NOTIFICATION_OS_SIDE_HPP + +#include "ordering/on_demand_os_transport.hpp" + +#include + +namespace iroha { + namespace ordering { + namespace transport { + + struct MockOdOsNotificationOsSide : public OdNotificationOsSide { + MOCK_METHOD2(onBatches, void(CollectionType, InitiatorPeerType)); + + MOCK_METHOD2(onRequestProposal, + boost::optional>( + consensus::Round, InitiatorPeerType)); + }; + + } // namespace transport + } // namespace ordering +} // namespace iroha + +#endif // IROHA_MOCK_ON_DEMAND_OS_NOTIFICATION_OS_SIDE_HPP diff --git a/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp b/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp new file mode 100644 index 0000000000..36443a306e --- /dev/null +++ b/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp @@ -0,0 +1,25 @@ +/** + * Copyright Soramitsu Co., Ltd. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef IROHA_MOCK_PROPOSAL_CREATION_STRATEGY_HPP +#define IROHA_MOCK_PROPOSAL_CREATION_STRATEGY_HPP + +#include "ordering/ordering_service_proposal_creation_strategy.hpp" + +#include + +namespace iroha { + namespace ordering { + class MockProposalCreationStrategy : public ProposalCreationStrategy { + public: + MOCK_METHOD2(onCollaborationOutcome, bool(RoundType, const PeerList &)); + MOCK_METHOD2(onProposal, + boost::optional(PeerType, + RoundType requested_round)); + }; + } // namespace ordering +} // namespace iroha + +#endif // IROHA_MOCK_PROPOSAL_CREATION_STRATEGY_HPP diff --git a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp index e73a179559..9178772ad6 100644 --- a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp +++ b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp @@ -103,7 +103,10 @@ TEST_F(OnDemandOrderingGateTest, BlockEvent) { ON_CALL(*proposal, transactions()) .WillByDefault(Return(txs | boost::adaptors::indirected)); - EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); + EXPECT_CALL( + *ordering_service, + onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(oproposal)))); @@ -112,7 +115,8 @@ TEST_F(OnDemandOrderingGateTest, BlockEvent) { gate_wrapper.subscribe( [&](auto val) { ASSERT_EQ(proposal, getProposalUnsafe(val).get()); }); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{round, {}}); + rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ + round, {}, std::make_shared()}); ASSERT_TRUE(gate_wrapper.validate()); } @@ -135,7 +139,10 @@ TEST_F(OnDemandOrderingGateTest, EmptyEvent) { ON_CALL(*proposal, transactions()) .WillByDefault(Return(txs | boost::adaptors::indirected)); - EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); + EXPECT_CALL( + *ordering_service, + onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(oproposal)))); @@ -144,7 +151,8 @@ TEST_F(OnDemandOrderingGateTest, EmptyEvent) { gate_wrapper.subscribe( [&](auto val) { ASSERT_EQ(proposal, getProposalUnsafe(val).get()); }); - rounds.get_subscriber().on_next(OnDemandOrderingGate::EmptyEvent{round}); + rounds.get_subscriber().on_next(OnDemandOrderingGate::EmptyEvent{ + round, std::make_shared()}); ASSERT_TRUE(gate_wrapper.validate()); } @@ -159,7 +167,10 @@ TEST_F(OnDemandOrderingGateTest, BlockEventNoProposal) { boost::optional> proposal; - EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); + EXPECT_CALL( + *ordering_service, + onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(proposal)))); @@ -167,7 +178,8 @@ TEST_F(OnDemandOrderingGateTest, BlockEventNoProposal) { make_test_subscriber(ordering_gate->onProposal(), 1); gate_wrapper.subscribe([&](auto val) { ASSERT_FALSE(val.proposal); }); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{round, {}}); + rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ + round, {}, std::make_shared()}); ASSERT_TRUE(gate_wrapper.validate()); } @@ -182,7 +194,10 @@ TEST_F(OnDemandOrderingGateTest, EmptyEventNoProposal) { boost::optional> proposal; - EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); + EXPECT_CALL( + *ordering_service, + onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(proposal)))); @@ -190,7 +205,8 @@ TEST_F(OnDemandOrderingGateTest, EmptyEventNoProposal) { make_test_subscriber(ordering_gate->onProposal(), 1); gate_wrapper.subscribe([&](auto val) { ASSERT_FALSE(val.proposal); }); - rounds.get_subscriber().on_next(OnDemandOrderingGate::EmptyEvent{round}); + rounds.get_subscriber().on_next(OnDemandOrderingGate::EmptyEvent{ + round, std::make_shared()}); ASSERT_TRUE(gate_wrapper.validate()); } @@ -202,7 +218,8 @@ TEST_F(OnDemandOrderingGateTest, EmptyEventNoProposal) { * this transaction */ TEST_F(OnDemandOrderingGateTest, ReplayedTransactionInProposal) { - OnDemandOrderingGate::BlockEvent event = {round, {}}; + OnDemandOrderingGate::BlockEvent event = { + round, {}, std::make_shared()}; // initialize mock transaction auto tx1 = std::make_shared>(); @@ -219,7 +236,10 @@ TEST_F(OnDemandOrderingGateTest, ReplayedTransactionInProposal) { std::move(proposal))); // set expectations for ordering service - EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); + EXPECT_CALL( + *ordering_service, + onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(arriving_proposal)))); EXPECT_CALL(*tx_cache, @@ -277,7 +297,8 @@ TEST_F(OnDemandOrderingGateTest, PopNonEmptyBatchesFromTheCache) { EXPECT_CALL(*notification, onBatches(UnorderedElementsAreArray(collection))) .Times(1); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{round, {}}); + rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ + round, {}, std::make_shared()}); } /** @@ -294,7 +315,8 @@ TEST_F(OnDemandOrderingGateTest, PopEmptyBatchesFromTheCache) { .Times(1); EXPECT_CALL(*notification, onBatches(_)).Times(0); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{round, {}}); + rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ + round, {}, std::make_shared()}); } /** @@ -314,6 +336,6 @@ TEST_F(OnDemandOrderingGateTest, BatchesRemoveFromCache) { EXPECT_CALL(*cache, pop()).Times(1); EXPECT_CALL(*cache, remove(UnorderedElementsAre(hash1, hash2))).Times(1); - rounds.get_subscriber().on_next( - OnDemandOrderingGate::BlockEvent{round, {hash1, hash2}}); + rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ + round, {hash1, hash2}, std::make_shared()}); } diff --git a/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp b/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp index 837115a742..2ccdac97e7 100644 --- a/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp @@ -12,7 +12,7 @@ #include "framework/test_logger.hpp" #include "interfaces/iroha_internal/transaction_batch_impl.hpp" #include "interfaces/iroha_internal/transaction_batch_parser_impl.hpp" -#include "module/irohad/ordering/mock_on_demand_os_notification.hpp" +#include "module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp" #include "module/shared_model/interface/mock_transaction_batch_factory.hpp" #include "module/shared_model/validators/validators.hpp" @@ -28,7 +28,7 @@ using ::testing::Return; struct OnDemandOsServerGrpcTest : public ::testing::Test { void SetUp() override { - notification = std::make_shared(); + notification = std::make_shared(); std::unique_ptr> interface_transaction_validator = @@ -55,14 +55,14 @@ struct OnDemandOsServerGrpcTest : public ::testing::Test { getTestLogger("OdOsServerGrpc")); } - std::shared_ptr notification; + std::shared_ptr notification; std::shared_ptr batch_factory; std::shared_ptr server; consensus::Round round{1, 2}; }; /** - * Separate action required because CollectionType is non-copyable + * Separate action required because CollectionType is non-copy1able */ ACTION_P(SaveArg0Move, var) { *var = std::move(arg0); @@ -93,7 +93,8 @@ TEST_F(OnDemandOsServerGrpcTest, SendBatches) { shared_model::interface::TransactionBatchImpl>( cand)); })); - EXPECT_CALL(*notification, onBatches(_)).WillOnce(SaveArg0Move(&collection)); + EXPECT_CALL(*notification, onBatches(_, _)) + .WillOnce(SaveArg0Move(&collection)); proto::BatchesRequest request; request.add_transactions() ->mutable_payload() @@ -126,7 +127,7 @@ TEST_F(OnDemandOsServerGrpcTest, RequestProposal) { std::shared_ptr iproposal( std::make_shared(proposal)); - EXPECT_CALL(*notification, onRequestProposal(round)) + EXPECT_CALL(*notification, onRequestProposal(round, _)) .WillOnce(Return(ByMove(std::move(iproposal)))); server->RequestProposal(nullptr, &request, &response); @@ -152,7 +153,7 @@ TEST_F(OnDemandOsServerGrpcTest, RequestProposalNone) { request.mutable_round()->set_block_round(round.block_round); request.mutable_round()->set_reject_round(round.reject_round); proto::ProposalResponse response; - EXPECT_CALL(*notification, onRequestProposal(round)) + EXPECT_CALL(*notification, onRequestProposal(round, _)) .WillOnce(Return(ByMove(std::move(boost::none)))); server->RequestProposal(nullptr, &request, &response); diff --git a/test/module/irohad/ordering/on_demand_os_test.cpp b/test/module/irohad/ordering/on_demand_os_test.cpp index a2e0431f66..7cac7d3897 100644 --- a/test/module/irohad/ordering/on_demand_os_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_test.cpp @@ -15,6 +15,7 @@ #include "framework/test_logger.hpp" #include "interfaces/iroha_internal/transaction_batch_impl.hpp" #include "module/irohad/ametsuchi/ametsuchi_mocks.hpp" +#include "module/irohad/ordering/mock_proposal_creation_strategy.hpp" #include "module/shared_model/interface_mocks.hpp" #include "module/shared_model/validators/validators.hpp" #include "ordering/impl/on_demand_common.hpp" @@ -46,6 +47,9 @@ class OnDemandOsTest : public ::testing::Test { commit_round = {3, kFirstRejectRound}, reject_round = {2, kNextRejectRoundConsumer}; NiceMock *mock_cache; + std::shared_ptr proposal_creation_strategy; + ProposalCreationStrategy::PeerList started_list; + ProposalCreationStrategy::PeerType requester_peer; void SetUp() override { // TODO: nickaleks IR-1811 use mock factory @@ -62,10 +66,17 @@ class OnDemandOsTest : public ::testing::Test { _))) .WillByDefault(Return(std::vector{ iroha::ametsuchi::tx_cache_status_responses::Missing()})); + + proposal_creation_strategy = + std::make_shared(); + started_list = {}; + requester_peer = nullptr; + os = std::make_shared( transaction_limit, std::move(factory), std::move(tx_cache), + proposal_creation_strategy, getTestLogger("OdOrderingService"), proposal_limit, initial_round); @@ -76,7 +87,7 @@ class OnDemandOsTest : public ::testing::Test { * @param range - pair of [from, to) */ void generateTransactionsAndInsert(std::pair range) { - os->onBatches(generateTransactions(range)); + os->onBatches(generateTransactions(range), requester_peer); } OnDemandOrderingService::CollectionType generateTransactions( @@ -119,11 +130,16 @@ class OnDemandOsTest : public ::testing::Test { * @then check that previous round doesn't have proposal */ TEST_F(OnDemandOsTest, EmptyRound) { - ASSERT_FALSE(os->onRequestProposal(initial_round)); + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); - os->onCollaborationOutcome(commit_round); + ASSERT_FALSE(os->onRequestProposal(initial_round, requester_peer)); - ASSERT_FALSE(os->onRequestProposal(initial_round)); + os->onCollaborationOutcome(commit_round, started_list); + + ASSERT_FALSE(os->onRequestProposal(initial_round, requester_peer)); } /** @@ -133,11 +149,16 @@ TEST_F(OnDemandOsTest, EmptyRound) { * @then check that previous round has all transaction */ TEST_F(OnDemandOsTest, NormalRound) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome(commit_round); + os->onCollaborationOutcome(commit_round, started_list); - ASSERT_TRUE(os->onRequestProposal(target_round)); + ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); } /** @@ -148,13 +169,20 @@ TEST_F(OnDemandOsTest, NormalRound) { * AND the rest of transactions isn't appeared in next after next round */ TEST_F(OnDemandOsTest, OverflowRound) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + generateTransactionsAndInsert({1, transaction_limit * 2}); - os->onCollaborationOutcome(commit_round); + os->onCollaborationOutcome(commit_round, started_list); - ASSERT_TRUE(os->onRequestProposal(target_round)); + ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); ASSERT_EQ(transaction_limit, - (*os->onRequestProposal(target_round))->transactions().size()); + (*os->onRequestProposal(target_round, requester_peer)) + ->transactions() + .size()); } /** @@ -164,6 +192,11 @@ TEST_F(OnDemandOsTest, OverflowRound) { * @then check that all transactions appear in proposal */ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + auto large_tx_limit = 10000u; auto factory = std::make_unique< shared_model::proto::ProtoProposalFactory>(); @@ -173,6 +206,7 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { large_tx_limit, std::move(factory), std::move(tx_cache), + proposal_creation_strategy, getTestLogger("OdOrderingService"), proposal_limit, initial_round); @@ -187,9 +221,12 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { std::thread two(call, std::make_pair(large_tx_limit / 2, large_tx_limit)); one.join(); two.join(); - os->onCollaborationOutcome(commit_round); + os->onCollaborationOutcome(commit_round, started_list); ASSERT_EQ(large_tx_limit, - os->onRequestProposal(target_round).get()->transactions().size()); + os->onRequestProposal(target_round, requester_peer) + .get() + ->transactions() + .size()); } /** @@ -201,20 +238,27 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { * tryErase */ TEST_F(OnDemandOsTest, Erase) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + generateTransactionsAndInsert({1, 2}); os->onCollaborationOutcome( - {commit_round.block_round, commit_round.reject_round}); + {commit_round.block_round, commit_round.reject_round}, started_list); ASSERT_TRUE(os->onRequestProposal( - {commit_round.block_round + 1, commit_round.reject_round})); + {commit_round.block_round + 1, commit_round.reject_round}, + requester_peer)); for (auto i = commit_round.reject_round + 1; i < (commit_round.reject_round + 1) + (proposal_limit + 2); ++i) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome({commit_round.block_round, i}); + os->onCollaborationOutcome({commit_round.block_round, i}, started_list); } ASSERT_TRUE(os->onRequestProposal( - {commit_round.block_round + 1, commit_round.reject_round})); + {commit_round.block_round + 1, commit_round.reject_round}, + requester_peer)); } /** @@ -223,6 +267,11 @@ TEST_F(OnDemandOsTest, Erase) { * @then check that proposal factory is called and returns a proposal */ TEST_F(OnDemandOsTest, UseFactoryForProposal) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + auto factory = std::make_unique(); auto mock_factory = factory.get(); auto tx_cache = @@ -245,6 +294,7 @@ TEST_F(OnDemandOsTest, UseFactoryForProposal) { transaction_limit, std::move(factory), std::move(tx_cache), + proposal_creation_strategy, getTestLogger("OdOrderingService"), proposal_limit, initial_round); @@ -255,9 +305,9 @@ TEST_F(OnDemandOsTest, UseFactoryForProposal) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome(commit_round); + os->onCollaborationOutcome(commit_round, started_list); - ASSERT_TRUE(os->onRequestProposal(target_round)); + ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); } // Return matcher for batch, which passes it by const & @@ -272,6 +322,11 @@ auto batchRef(const shared_model::interface::TransactionBatch &batch) { * @then the batch is not present in a proposal */ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + auto batches = generateTransactions({1, 2}); auto &batch = *batches.at(0); @@ -279,11 +334,11 @@ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { .WillOnce(Return(std::vector{ iroha::ametsuchi::tx_cache_status_responses::Committed()})); - os->onBatches(batches); + os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round); + os->onCollaborationOutcome(commit_round, started_list); - auto proposal = os->onRequestProposal(initial_round); + auto proposal = os->onRequestProposal(initial_round, requester_peer); EXPECT_FALSE(proposal); } @@ -294,6 +349,11 @@ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { * @then batch is present in a proposal */ TEST_F(OnDemandOsTest, PassMissingTransaction) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + auto batches = generateTransactions({1, 2}); auto &batch = *batches.at(0); @@ -301,11 +361,11 @@ TEST_F(OnDemandOsTest, PassMissingTransaction) { .WillOnce(Return(std::vector{ iroha::ametsuchi::tx_cache_status_responses::Missing()})); - os->onBatches(batches); + os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round); + os->onCollaborationOutcome(commit_round, started_list); - auto proposal = os->onRequestProposal(target_round); + auto proposal = os->onRequestProposal(target_round, requester_peer); // since we only sent one transaction, // if the proposal is present, there is no need to check for that specific tx @@ -318,6 +378,11 @@ TEST_F(OnDemandOsTest, PassMissingTransaction) { * @then 2 new batches are in a proposal and already commited batch is discarded */ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + auto batches = generateTransactions({1, 4}); auto &batch1 = *batches.at(0); auto &batch2 = *batches.at(1); @@ -333,11 +398,11 @@ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { .WillOnce(Return(std::vector{ iroha::ametsuchi::tx_cache_status_responses::Missing()})); - os->onBatches(batches); + os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round); + os->onCollaborationOutcome(commit_round, started_list); - auto proposal = os->onRequestProposal(target_round); + auto proposal = os->onRequestProposal(target_round, requester_peer); const auto &txs = proposal->get()->transactions(); auto &batch2_tx = *batch2.transactions().at(0); @@ -353,14 +418,19 @@ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { * @then the proposal contains the batch once */ TEST_F(OnDemandOsTest, DuplicateTxTest) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + auto now = iroha::time::now(); auto txs1 = generateTransactions({1, 2}, now); - os->onBatches(txs1); + os->onBatches(txs1, requester_peer); auto txs2 = generateTransactions({1, 2}, now); - os->onBatches(txs2); - os->onCollaborationOutcome(commit_round); - auto proposal = os->onRequestProposal(target_round); + os->onBatches(txs2, requester_peer); + os->onCollaborationOutcome(commit_round, started_list); + auto proposal = os->onRequestProposal(target_round, requester_peer); ASSERT_EQ(1, boost::size((*proposal)->transactions())); } @@ -371,21 +441,31 @@ TEST_F(OnDemandOsTest, DuplicateTxTest) { * @then both of them are used for the next proposal */ TEST_F(OnDemandOsTest, RejectCommit) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + auto now = iroha::time::now(); auto txs1 = generateTransactions({1, 2}, now); - os->onBatches(txs1); + os->onBatches(txs1, requester_peer); os->onCollaborationOutcome( - {initial_round.block_round, initial_round.reject_round + 1}); + {initial_round.block_round, initial_round.reject_round + 1}, + started_list); auto txs2 = generateTransactions({1, 2}, now + 1); - os->onBatches(txs2); + os->onBatches(txs2, requester_peer); os->onCollaborationOutcome( - {initial_round.block_round, initial_round.reject_round + 2}); + {initial_round.block_round, initial_round.reject_round + 2}, + started_list); auto proposal = os->onRequestProposal( - {initial_round.block_round, initial_round.reject_round + 3}); + {initial_round.block_round, initial_round.reject_round + 3}, + requester_peer); ASSERT_EQ(2, boost::size((*proposal)->transactions())); - proposal = os->onRequestProposal(commit_round); + proposal = os->onRequestProposal(commit_round, requester_peer); ASSERT_EQ(2, boost::size((*proposal)->transactions())); } + +// todo add test with malicious case in strategy diff --git a/test/module/irohad/ordering/ordering_mocks.hpp b/test/module/irohad/ordering/ordering_mocks.hpp index c867a72768..b1acd9eb7e 100644 --- a/test/module/irohad/ordering/ordering_mocks.hpp +++ b/test/module/irohad/ordering/ordering_mocks.hpp @@ -36,13 +36,14 @@ namespace iroha { } // namespace cache struct MockOnDemandOrderingService : public OnDemandOrderingService { - MOCK_METHOD1(onBatches, void(CollectionType)); + MOCK_METHOD2(onBatches, void(CollectionType, InitiatorPeerType)); - MOCK_METHOD1(onRequestProposal, + MOCK_METHOD2(onRequestProposal, boost::optional>( - consensus::Round)); + consensus::Round, InitiatorPeerType from)); - MOCK_METHOD1(onCollaborationOutcome, void(consensus::Round)); + MOCK_METHOD2(onCollaborationOutcome, + void(consensus::Round, const PeerList &peers)); }; } // namespace ordering From 94da0f2d2b7153ea6b33b248aa100cb783235d02 Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Fri, 5 Apr 2019 09:32:13 +0300 Subject: [PATCH 03/22] fix test Signed-off-by: Fedor Muratov --- .../ordering/on_demand_ordering_gate_test.cpp | 57 ++++++++++++------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp index 9178772ad6..921858d01a 100644 --- a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp +++ b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp @@ -11,6 +11,7 @@ #include "framework/test_subscriber.hpp" #include "interfaces/iroha_internal/transaction_batch_impl.hpp" #include "module/irohad/ametsuchi/mock_tx_presence_cache.hpp" +#include "module/irohad/consensus/yac/yac_test_util.hpp" #include "module/irohad/ordering/mock_on_demand_os_notification.hpp" #include "module/irohad/ordering/ordering_mocks.hpp" #include "module/shared_model/interface_mocks.hpp" @@ -55,6 +56,12 @@ class OnDemandOrderingGateTest : public ::testing::Test { tx_cache, 1000, getTestLogger("OrderingGate")); + using PeerListType = + std::vector>; + PeerListType peers{iroha::consensus::yac::makePeer("1")}; + + ledger_state = std::make_shared( + std::make_shared(peers)); } rxcpp::subjects::subject rounds; @@ -66,6 +73,8 @@ class OnDemandOrderingGateTest : public ::testing::Test { std::shared_ptr cache; + std::shared_ptr ledger_state; + const consensus::Round round = {2, kFirstRejectRound}; }; @@ -105,7 +114,9 @@ TEST_F(OnDemandOrderingGateTest, BlockEvent) { EXPECT_CALL( *ordering_service, - onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + onCollaborationOutcome( + round, + testing::Matcher(_))) .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(oproposal)))); @@ -115,8 +126,8 @@ TEST_F(OnDemandOrderingGateTest, BlockEvent) { gate_wrapper.subscribe( [&](auto val) { ASSERT_EQ(proposal, getProposalUnsafe(val).get()); }); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ - round, {}, std::make_shared()}); + rounds.get_subscriber().on_next( + OnDemandOrderingGate::BlockEvent{round, {}, ledger_state}); ASSERT_TRUE(gate_wrapper.validate()); } @@ -141,7 +152,9 @@ TEST_F(OnDemandOrderingGateTest, EmptyEvent) { EXPECT_CALL( *ordering_service, - onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + onCollaborationOutcome( + round, + testing::Matcher(_))) .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(oproposal)))); @@ -151,8 +164,8 @@ TEST_F(OnDemandOrderingGateTest, EmptyEvent) { gate_wrapper.subscribe( [&](auto val) { ASSERT_EQ(proposal, getProposalUnsafe(val).get()); }); - rounds.get_subscriber().on_next(OnDemandOrderingGate::EmptyEvent{ - round, std::make_shared()}); + rounds.get_subscriber().on_next( + OnDemandOrderingGate::EmptyEvent{round, ledger_state}); ASSERT_TRUE(gate_wrapper.validate()); } @@ -169,7 +182,9 @@ TEST_F(OnDemandOrderingGateTest, BlockEventNoProposal) { EXPECT_CALL( *ordering_service, - onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + onCollaborationOutcome( + round, + testing::Matcher(_))) .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(proposal)))); @@ -178,8 +193,8 @@ TEST_F(OnDemandOrderingGateTest, BlockEventNoProposal) { make_test_subscriber(ordering_gate->onProposal(), 1); gate_wrapper.subscribe([&](auto val) { ASSERT_FALSE(val.proposal); }); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ - round, {}, std::make_shared()}); + rounds.get_subscriber().on_next( + OnDemandOrderingGate::BlockEvent{round, {}, ledger_state}); ASSERT_TRUE(gate_wrapper.validate()); } @@ -196,7 +211,9 @@ TEST_F(OnDemandOrderingGateTest, EmptyEventNoProposal) { EXPECT_CALL( *ordering_service, - onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + onCollaborationOutcome( + round, + testing::Matcher(_))) .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(proposal)))); @@ -205,8 +222,8 @@ TEST_F(OnDemandOrderingGateTest, EmptyEventNoProposal) { make_test_subscriber(ordering_gate->onProposal(), 1); gate_wrapper.subscribe([&](auto val) { ASSERT_FALSE(val.proposal); }); - rounds.get_subscriber().on_next(OnDemandOrderingGate::EmptyEvent{ - round, std::make_shared()}); + rounds.get_subscriber().on_next( + OnDemandOrderingGate::EmptyEvent{round, ledger_state}); ASSERT_TRUE(gate_wrapper.validate()); } @@ -238,7 +255,9 @@ TEST_F(OnDemandOrderingGateTest, ReplayedTransactionInProposal) { // set expectations for ordering service EXPECT_CALL( *ordering_service, - onCollaborationOutcome(round, OnDemandOrderingService::PeerList{})) + onCollaborationOutcome( + round, + testing::Matcher(_))) .Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(arriving_proposal)))); @@ -297,8 +316,8 @@ TEST_F(OnDemandOrderingGateTest, PopNonEmptyBatchesFromTheCache) { EXPECT_CALL(*notification, onBatches(UnorderedElementsAreArray(collection))) .Times(1); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ - round, {}, std::make_shared()}); + rounds.get_subscriber().on_next( + OnDemandOrderingGate::BlockEvent{round, {}, ledger_state}); } /** @@ -315,8 +334,8 @@ TEST_F(OnDemandOrderingGateTest, PopEmptyBatchesFromTheCache) { .Times(1); EXPECT_CALL(*notification, onBatches(_)).Times(0); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ - round, {}, std::make_shared()}); + rounds.get_subscriber().on_next( + OnDemandOrderingGate::BlockEvent{round, {}, ledger_state}); } /** @@ -336,6 +355,6 @@ TEST_F(OnDemandOrderingGateTest, BatchesRemoveFromCache) { EXPECT_CALL(*cache, pop()).Times(1); EXPECT_CALL(*cache, remove(UnorderedElementsAre(hash1, hash2))).Times(1); - rounds.get_subscriber().on_next(OnDemandOrderingGate::BlockEvent{ - round, {hash1, hash2}, std::make_shared()}); + rounds.get_subscriber().on_next( + OnDemandOrderingGate::BlockEvent{round, {hash1, hash2}, ledger_state}); } From e7f984387587326ab4e4c02ad424cb54a33c900c Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Sat, 6 Apr 2019 14:16:01 +0300 Subject: [PATCH 04/22] Fix interfaces in OS Signed-off-by: Fedor Muratov --- .../kick_out_proposal_creation_strategy.cpp | 43 +++++++++---------- .../kick_out_proposal_creation_strategy.hpp | 22 ++++------ .../impl/on_demand_ordering_service_impl.cpp | 37 ++++++++++++---- .../impl/on_demand_ordering_service_impl.hpp | 12 +++++- .../ordering/on_demand_ordering_service.hpp | 4 +- ...ing_service_proposal_creation_strategy.hpp | 17 +++++--- 6 files changed, 80 insertions(+), 55 deletions(-) diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp index dc110ff022..3583a36d6b 100644 --- a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp @@ -14,8 +14,24 @@ KickOutProposalCreationStrategy::KickOutProposalCreationStrategy( std::shared_ptr majority_checker) : majority_checker_(majority_checker) {} -bool KickOutProposalCreationStrategy::onCollaborationOutcome( - RoundType round, const PeerList &peers) { +void KickOutProposalCreationStrategy::onCollaborationOutcome( + const PeerList &peers) { + last_requested_ = + std::accumulate(peers.begin(), + peers.end(), + RoundCollectionType{}, + [this](auto collection, const auto &peer) { + auto iter = last_requested_.find(peer->hex()); + if (iter != last_requested_.end()) { + collection.insert(*iter); + } else { + collection.insert({peer->hex(), RoundType{0, 0}}); + } + return std::move(collection); + }); +} + +bool KickOutProposalCreationStrategy::shouldCreateRound(RoundType round) { uint64_t counter = 0; std::for_each(last_requested_.begin(), last_requested_.end(), @@ -24,18 +40,18 @@ bool KickOutProposalCreationStrategy::onCollaborationOutcome( counter++; } }); + auto has_majority = majority_checker_->hasMajority(counter, last_requested_.size()); - updateCurrentState(peers); return not has_majority; } boost::optional KickOutProposalCreationStrategy::onProposal(PeerType who, RoundType requested_round) { - auto iter = last_requested_.find(who); + auto iter = last_requested_.find(who->hex()); if (iter == last_requested_.end()) { - last_requested_.insert({who, requested_round}); + last_requested_.insert({who->hex(), requested_round}); } else { if (iter->second < requested_round) { iter->second = requested_round; @@ -44,20 +60,3 @@ KickOutProposalCreationStrategy::onProposal(PeerType who, return boost::none; } - -void KickOutProposalCreationStrategy::updateCurrentState( - const PeerList &peers) { - last_requested_ = - std::accumulate(peers.begin(), - peers.end(), - RoundCollectionType{}, - [this](auto collection, const auto &peer) { - auto iter = last_requested_.find(peer); - if (iter != last_requested_.end()) { - collection.insert(*iter); - } else { - collection.insert({peer, RoundType{0, 0}}); - } - return std::move(collection); - }); -} diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp index 2a2875abf0..cbb3c2711a 100644 --- a/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp @@ -24,25 +24,21 @@ namespace iroha { KickOutProposalCreationStrategy( std::shared_ptr); - bool onCollaborationOutcome(RoundType round, - const PeerList &peers) override; - - boost::optional onProposal(PeerType who, - RoundType requested_round) override; - - private: - using RoundCollectionType = std::unordered_map< - PeerType, - RoundType, - DereferenceHash>; - /** * Update peers state with new peers. * Note: the method removes peers which are not participating in consensus * and adds new with minimal round * @param peers - list of peers which fetched in the last round */ - void updateCurrentState(const PeerList &peers); + void onCollaborationOutcome(const PeerList &peers) override; + + bool shouldCreateRound(RoundType round); + + boost::optional onProposal(PeerType who, + RoundType requested_round) override; + + private: + using RoundCollectionType = std::unordered_map; std::shared_ptr majority_checker_; RoundCollectionType last_requested_; diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index 01391b21fa..c9f861a3cf 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -49,15 +49,14 @@ void OnDemandOrderingServiceImpl::onCollaborationOutcome( consensus::Round round, const PeerList &peers) { log_->info("onCollaborationOutcome => {}", round); - if (proposal_creation_strategy_->onCollaborationOutcome(round, peers)) { - packNextProposals(round); - } + packNextProposals(round); + proposal_creation_strategy_->onCollaborationOutcome(peers); tryErase(round); } // ----------------------------| OdOsNotification |----------------------------- -void OnDemandOrderingServiceImpl::onBatches(CollectionType batches, +void OnDemandOrderingServiceImpl::onBatches(BatchesCollectionType batches, InitiatorPeerType from) { auto unprocessed_batches = boost::adaptors::filter(batches, [this](const auto &batch) { @@ -170,7 +169,8 @@ void OnDemandOrderingServiceImpl::packNextProposals( size_t discarded_txs_quantity; auto now = iroha::time::now(); auto generate_proposal = [this, now, &discarded_txs_quantity]( - consensus::Round round, const auto &txs) { + consensus::Round round, + const TransactionsCollectionType &txs) { auto proposal = proposal_factory_->unsafeCreateProposal( round.block_round, now, txs | boost::adaptors::indirected); proposal_map_.erase(round); @@ -187,10 +187,10 @@ void OnDemandOrderingServiceImpl::packNextProposals( if (not pending_batches_.empty()) { auto txs = getTransactions( transaction_limit_, pending_batches_, discarded_txs_quantity); - if (not txs.empty()) { - generate_proposal({round.block_round, round.reject_round + 1}, txs); - generate_proposal({round.block_round + 1, kFirstRejectRound}, txs); - } + tryToCreateProposal( + {round.block_round, round.reject_round + 1}, txs, generate_proposal); + tryToCreateProposal( + {round.block_round + 1, kFirstRejectRound}, txs, generate_proposal); } if (round.reject_round == kFirstRejectRound) { @@ -199,6 +199,25 @@ void OnDemandOrderingServiceImpl::packNextProposals( } } +void OnDemandOrderingServiceImpl::tryToCreateProposal( + iroha::consensus::Round round, + const TransactionsCollectionType &txs, + std::function + proposal_generator) { + std::string log_str = "Proposal in round {} will not created because "; + + if (not txs.empty()) { + if (proposal_creation_strategy_->shouldCreateRound(round)) { + proposal_generator(round, txs); + log_->info("created proposal in round {}", round); + } else { + log_->info(log_str + "round strategy annul creation", round); + } + } else { + log_->info(log_str + "transactions absent", round); + } +} + void OnDemandOrderingServiceImpl::tryErase( const consensus::Round ¤t_round) { // find first round that is not less than current_round diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp index a86b8c1fa2..861b0cf050 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp @@ -67,7 +67,8 @@ namespace iroha { // ----------------------- | OdOsNotification | -------------------------- - void onBatches(CollectionType batches, InitiatorPeerType from) override; + void onBatches(BatchesCollectionType batches, + InitiatorPeerType from) override; boost::optional> onRequestProposal( consensus::Round round, InitiatorPeerType requester) override; @@ -79,6 +80,15 @@ namespace iroha { */ void packNextProposals(const consensus::Round &round); + using TransactionsCollectionType = + std::vector>; + using RoundType = iroha::consensus::Round; + + void tryToCreateProposal( + RoundType, + const TransactionsCollectionType &, + std::function); + /** * Removes last elements if it is required * Method removes the oldest commit or chain of the oldest rejects diff --git a/irohad/ordering/on_demand_ordering_service.hpp b/irohad/ordering/on_demand_ordering_service.hpp index bfbf1dee17..659f579ca1 100644 --- a/irohad/ordering/on_demand_ordering_service.hpp +++ b/irohad/ordering/on_demand_ordering_service.hpp @@ -19,10 +19,8 @@ namespace iroha { */ class OnDemandOrderingService : public transport::OdNotificationOsSide { public: - /// shortcut for peer type - using PeerType = std::shared_ptr; /// collection of peers type - using PeerList = std::vector; + using PeerList = std::vector; /** * Method which should be invoked on outcome of collaboration for round * @param round - proposal round which has started diff --git a/irohad/ordering/ordering_service_proposal_creation_strategy.hpp b/irohad/ordering/ordering_service_proposal_creation_strategy.hpp index 38fbb927e6..25ba6d33ee 100644 --- a/irohad/ordering/ordering_service_proposal_creation_strategy.hpp +++ b/irohad/ordering/ordering_service_proposal_creation_strategy.hpp @@ -11,7 +11,7 @@ #include #include "consensus/round.hpp" -#include "interfaces/common_objects/peer.hpp" +#include "cryptography/public_key.hpp" namespace iroha { namespace ordering { @@ -23,21 +23,24 @@ namespace iroha { class ProposalCreationStrategy { public: /// shortcut for peer type - using PeerType = std::shared_ptr; + using PeerType = std::shared_ptr; /// shortcut for round type using RoundType = consensus::Round; /// collection of peers type using PeerList = std::vector; /** - * Indicates the start of new round and provide information about proposal - * creation - * @param round - new consensus round + * Indicates the start of new round. * @param peers - peers which participate in new round + */ + virtual void onCollaborationOutcome(const PeerList &peers) = 0; + + /** + * + * @param round - new consensus round * @return true, if proposal should be created in new round */ - virtual bool onCollaborationOutcome(RoundType round, - const PeerList &peers) = 0; + virtual bool shouldCreateRound(RoundType round) = 0; /** * Notify the strategy about proposal request From d1dd2ddd4d78d15a58da89e237e86c51b428c37a Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Sat, 6 Apr 2019 17:47:54 +0300 Subject: [PATCH 05/22] fix irohad compilation Signed-off-by: Fedor Muratov --- irohad/main/application.cpp | 29 ++++++++++++-- irohad/main/impl/on_demand_ordering_init.cpp | 11 +++++- irohad/main/impl/on_demand_ordering_init.hpp | 11 +++++- .../kick_out_proposal_creation_strategy.cpp | 4 +- .../impl/on_demand_connection_manager.cpp | 2 +- .../impl/on_demand_connection_manager.hpp | 1 + .../ordering/impl/on_demand_ordering_gate.cpp | 15 ++++++-- .../impl/on_demand_os_client_grpc.cpp | 20 ++++++++-- .../impl/on_demand_os_client_grpc.hpp | 11 +++++- .../impl/on_demand_os_server_grpc.cpp | 38 ++++++++++++++++--- .../impl/on_demand_os_server_grpc.hpp | 11 ++++++ irohad/ordering/on_demand_os_transport.hpp | 16 +++++--- schema/ordering.proto | 3 ++ 13 files changed, 144 insertions(+), 28 deletions(-) diff --git a/irohad/main/application.cpp b/irohad/main/application.cpp index 28c708a1ce..193e39f7a5 100644 --- a/irohad/main/application.cpp +++ b/irohad/main/application.cpp @@ -376,9 +376,14 @@ void Irohad::initOrderingGate() { }; std::shared_ptr proposal_strategy = - std::make_shared( + std::make_shared( getSupermajorityChecker(kConsensusConsistencyModel)); + auto field_validator = + std::make_shared(); + auto self_peer_key = + std::make_shared(keypair.publicKey()); + ordering_gate = ordering_init.initOrderingGate(max_proposal_size_, proposal_delay_, @@ -393,7 +398,9 @@ void Irohad::initOrderingGate() { persistent_cache, delay, log_manager_->getChild("Ordering"), - proposal_strategy); + field_validator, + proposal_strategy, + self_peer_key); log_->info("[Init] => init ordering gate - [{}]", logger::logBool(ordering_gate)); } @@ -685,6 +692,21 @@ Irohad::RunResult Irohad::run() { + e->error); } + auto peer_query = storage->createPeerQuery(); + if (not peer_query) { + return expected::makeError("Failed to create peer query"); + } + auto peer_list = (*peer_query)->getLedgerPeers(); + if (not peer_list) { + return expected::makeError( + "Failed to fetch peers from peer query"); + } + std::shared_ptr state = + std::make_shared(); + state->ledger_peers = std::make_shared< + std::vector>>( + std::move(peer_list.get())); + auto block = boost::get>>(&block_var) ->value; @@ -695,7 +717,8 @@ Irohad::RunResult Irohad::run() { synchronizer::SynchronizationEvent{ rxcpp::observable<>::just(block), SynchronizationOutcomeType::kCommit, - {block->height(), ordering::kFirstRejectRound}}); + {block->height(), ordering::kFirstRejectRound}, + state}); return {}; }, [&](const expected::Error &e) -> RunResult { diff --git a/irohad/main/impl/on_demand_ordering_init.cpp b/irohad/main/impl/on_demand_ordering_init.cpp index 7914e13369..f2fb4e56b7 100644 --- a/irohad/main/impl/on_demand_ordering_init.cpp +++ b/irohad/main/impl/on_demand_ordering_init.cpp @@ -52,12 +52,14 @@ namespace iroha { async_call, std::shared_ptr proposal_transport_factory, std::chrono::milliseconds delay, + std::shared_ptr self_peer, const logger::LoggerManagerTreePtr &ordering_log_manager) { return std::make_shared( std::move(async_call), std::move(proposal_transport_factory), [] { return std::chrono::system_clock::now(); }, delay, + self_peer, ordering_log_manager->getChild("NetworkClient")->getLogger()); } @@ -68,6 +70,7 @@ namespace iroha { std::shared_ptr proposal_transport_factory, std::chrono::milliseconds delay, std::vector initial_hashes, + std::shared_ptr self_peer_key, const logger::LoggerManagerTreePtr &ordering_log_manager) { // since top block will be the first in notifier observable, hashes of // two previous blocks are prepended @@ -195,6 +198,7 @@ namespace iroha { createNotificationFactory(std::move(async_call), std::move(proposal_transport_factory), delay, + self_peer_key, ordering_log_manager), peers, ordering_log_manager->getChild("ConnectionManager")->getLogger()); @@ -301,7 +305,10 @@ namespace iroha { std::function delay_func, logger::LoggerManagerTreePtr ordering_log_manager, - std::shared_ptr creation_strategy) { + std::shared_ptr + field_validator, + std::shared_ptr creation_strategy, + std::shared_ptr self_peer_key) { auto ordering_service = createService(max_number_of_transactions, proposal_factory, tx_cache, @@ -312,6 +319,7 @@ namespace iroha { std::move(transaction_factory), std::move(batch_parser), std::move(transaction_batch_factory), + field_validator, ordering_log_manager->getChild("Server")->getLogger()); return createGate( ordering_service, @@ -320,6 +328,7 @@ namespace iroha { std::move(proposal_transport_factory), delay, std::move(initial_hashes), + self_peer_key, ordering_log_manager), std::make_shared(), std::move(proposal_factory), diff --git a/irohad/main/impl/on_demand_ordering_init.hpp b/irohad/main/impl/on_demand_ordering_init.hpp index 1324db166f..10165033d1 100644 --- a/irohad/main/impl/on_demand_ordering_init.hpp +++ b/irohad/main/impl/on_demand_ordering_init.hpp @@ -45,6 +45,7 @@ namespace iroha { async_call, std::shared_ptr proposal_transport_factory, std::chrono::milliseconds delay, + std::shared_ptr self_peer, const logger::LoggerManagerTreePtr &ordering_log_manager); /** @@ -59,6 +60,7 @@ namespace iroha { std::shared_ptr proposal_transport_factory, std::chrono::milliseconds delay, std::vector initial_hashes, + std::shared_ptr self_peer, const logger::LoggerManagerTreePtr &ordering_log_manager); /** @@ -116,6 +118,10 @@ namespace iroha { * requests to ordering service and processing responses * @param proposal_factory factory required by ordering service to produce * proposals + * @param field_validator - provides a dependency for validating peer keys + * @param creation_strategy - provides a strategy for creaing proposals in + * OS + * @param self_peer_key - the public key of instantiated peer * @return initialized ordering gate */ std::shared_ptr initOrderingGate( @@ -139,7 +145,10 @@ namespace iroha { std::function delay_func, logger::LoggerManagerTreePtr ordering_log_manager, - std::shared_ptr creation_strategy); + std::shared_ptr + field_validator, + std::shared_ptr creation_strategy, + std::shared_ptr self_peer_key); /// gRPC service for ordering service std::shared_ptr service; diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp index 3583a36d6b..d2d83adce8 100644 --- a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp @@ -50,9 +50,7 @@ boost::optional KickOutProposalCreationStrategy::onProposal(PeerType who, RoundType requested_round) { auto iter = last_requested_.find(who->hex()); - if (iter == last_requested_.end()) { - last_requested_.insert({who->hex(), requested_round}); - } else { + if (iter != last_requested_.end()) { if (iter->second < requested_round) { iter->second = requested_round; } diff --git a/irohad/ordering/impl/on_demand_connection_manager.cpp b/irohad/ordering/impl/on_demand_connection_manager.cpp index bff96d5335..5a0274364f 100644 --- a/irohad/ordering/impl/on_demand_connection_manager.cpp +++ b/irohad/ordering/impl/on_demand_connection_manager.cpp @@ -74,7 +74,7 @@ void OnDemandConnectionManager::initializeConnections( const CurrentPeers &peers) { auto create_assign = [this](auto &ptr, auto &peer) { std::lock_guard lock(mutex_); - ptr = factory_->create(*peer); + ptr = factory_->create(peer->address()); }; for (auto &&pair : boost::combine(connections_.peers, peers.peers)) { diff --git a/irohad/ordering/impl/on_demand_connection_manager.hpp b/irohad/ordering/impl/on_demand_connection_manager.hpp index 2d8206ac2c..317e783d78 100644 --- a/irohad/ordering/impl/on_demand_connection_manager.hpp +++ b/irohad/ordering/impl/on_demand_connection_manager.hpp @@ -11,6 +11,7 @@ #include #include +#include "interfaces/common_objects/peer.hpp" #include "logger/logger_fwd.hpp" namespace iroha { diff --git a/irohad/ordering/impl/on_demand_ordering_gate.cpp b/irohad/ordering/impl/on_demand_ordering_gate.cpp index ebaf1a09ac..04e4835f4a 100644 --- a/irohad/ordering/impl/on_demand_ordering_gate.cpp +++ b/irohad/ordering/impl/on_demand_ordering_gate.cpp @@ -13,6 +13,7 @@ #include #include "ametsuchi/tx_presence_cache.hpp" #include "common/visitor.hpp" +#include "cryptography/public_key.hpp" #include "interfaces/iroha_internal/transaction_batch.hpp" #include "interfaces/iroha_internal/transaction_batch_parser_impl.hpp" #include "logger/logger.hpp" @@ -60,9 +61,18 @@ OnDemandOrderingGate::OnDemandOrderingGate( return event.ledger_state; }); + auto peer_keys = [](const auto peers) { + OnDemandOrderingService::PeerList lst; + for (auto peer : peers) { + auto shp_pub_key = + std::make_shared(peer->pubkey()); + lst.push_back(shp_pub_key); + } + return lst; + }; // notify our ordering service about new round ordering_service_->onCollaborationOutcome( - current_round, *ledger_state->ledger_peers); + current_round, peer_keys(*ledger_state->ledger_peers)); this->sendCachedTransactions(event); @@ -81,8 +91,7 @@ OnDemandOrderingGate::~OnDemandOrderingGate() { events_subscription_.unsubscribe(); } -void OnDemandOrderingGate:: -propagateBatch( +void OnDemandOrderingGate::propagateBatch( std::shared_ptr batch) { cache_->addToBack({batch}); diff --git a/irohad/ordering/impl/on_demand_os_client_grpc.cpp b/irohad/ordering/impl/on_demand_os_client_grpc.cpp index dc2a31f15a..9d350fedb3 100644 --- a/irohad/ordering/impl/on_demand_os_client_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_client_grpc.cpp @@ -7,6 +7,7 @@ #include "backend/protobuf/proposal.hpp" #include "backend/protobuf/transaction.hpp" +#include "cryptography/public_key.hpp" #include "interfaces/common_objects/peer.hpp" #include "interfaces/iroha_internal/transaction_batch.hpp" #include "logger/logger.hpp" @@ -23,13 +24,15 @@ OnDemandOsClientGrpc::OnDemandOsClientGrpc( std::shared_ptr proposal_factory, std::function time_provider, std::chrono::milliseconds proposal_request_timeout, + std::shared_ptr self_peer_key, logger::LoggerPtr log) : log_(std::move(log)), stub_(std::move(stub)), async_call_(std::move(async_call)), proposal_factory_(std::move(proposal_factory)), time_provider_(std::move(time_provider)), - proposal_request_timeout_(proposal_request_timeout) {} + proposal_request_timeout_(proposal_request_timeout), + self_peer_key_(self_peer_key) {} void OnDemandOsClientGrpc::onBatches(CollectionType batches) { proto::BatchesRequest request; @@ -41,6 +44,9 @@ void OnDemandOsClientGrpc::onBatches(CollectionType batches) { } } + *request.mutable_peer_key() = + shared_model::crypto::toBinaryString(*self_peer_key_); + log_->debug("Propagating: '{}'", request.DebugString()); async_call_->Call([&](auto context, auto cq) { @@ -53,8 +59,13 @@ OnDemandOsClientGrpc::onRequestProposal(consensus::Round round) { grpc::ClientContext context; context.set_deadline(time_provider_() + proposal_request_timeout_); proto::ProposalRequest request; + request.mutable_round()->set_block_round(round.block_round); request.mutable_round()->set_reject_round(round.reject_round); + + *request.mutable_peer_key() = + shared_model::crypto::toBinaryString(*self_peer_key_); + proto::ProposalResponse response; auto status = stub_->RequestProposal(&context, request, &response); if (not status.ok()) { @@ -85,20 +96,23 @@ OnDemandOsClientGrpcFactory::OnDemandOsClientGrpcFactory( std::shared_ptr proposal_factory, std::function time_provider, OnDemandOsClientGrpc::TimeoutType proposal_request_timeout, + std::shared_ptr self_key_peer, logger::LoggerPtr client_log) : async_call_(std::move(async_call)), proposal_factory_(std::move(proposal_factory)), time_provider_(time_provider), proposal_request_timeout_(proposal_request_timeout), + self_key_peer_(self_key_peer), client_log_(std::move(client_log)) {} std::unique_ptr OnDemandOsClientGrpcFactory::create( - const shared_model::interface::Peer &to) { + const shared_model::interface::types::AddressType &to) { return std::make_unique( - network::createClient(to.address()), + network::createClient(to), async_call_, proposal_factory_, time_provider_, proposal_request_timeout_, + self_key_peer_, client_log_); } diff --git a/irohad/ordering/impl/on_demand_os_client_grpc.hpp b/irohad/ordering/impl/on_demand_os_client_grpc.hpp index 31e506f158..67468027d2 100644 --- a/irohad/ordering/impl/on_demand_os_client_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_client_grpc.hpp @@ -8,6 +8,7 @@ #include "ordering/on_demand_os_transport.hpp" +#include "interfaces/common_objects/types.hpp" #include "interfaces/iroha_internal/abstract_transport_factory.hpp" #include "logger/logger_fwd.hpp" #include "network/impl/async_grpc_client.hpp" @@ -40,6 +41,8 @@ namespace iroha { std::shared_ptr proposal_factory, std::function time_provider, std::chrono::milliseconds proposal_request_timeout, + std::shared_ptr + self_peer_key_, logger::LoggerPtr log); void onBatches(CollectionType batches) override; @@ -55,6 +58,8 @@ namespace iroha { std::shared_ptr proposal_factory_; std::function time_provider_; std::chrono::milliseconds proposal_request_timeout_; + std::shared_ptr + self_peer_key_; }; class OnDemandOsClientGrpcFactory : public OdOsNotificationFactory { @@ -66,6 +71,8 @@ namespace iroha { std::shared_ptr proposal_factory, std::function time_provider, OnDemandOsClientGrpc::TimeoutType proposal_request_timeout, + std::shared_ptr + self_key_peer, logger::LoggerPtr client_log); /** @@ -75,7 +82,7 @@ namespace iroha { * This factory method can be used in production code */ std::unique_ptr create( - const shared_model::interface::Peer &to) override; + const shared_model::interface::types::AddressType &to) override; private: std::shared_ptr> @@ -83,6 +90,8 @@ namespace iroha { std::shared_ptr proposal_factory_; std::function time_provider_; std::chrono::milliseconds proposal_request_timeout_; + std::shared_ptr + self_key_peer_; logger::LoggerPtr client_log_; }; diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.cpp b/irohad/ordering/impl/on_demand_os_server_grpc.cpp index 8c97fd3a62..9c643997d4 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.cpp @@ -9,8 +9,10 @@ #include #include +#include "backend/protobuf/common_objects/peer.hpp" #include "backend/protobuf/proposal.hpp" #include "common/bind.hpp" +#include "cryptography/public_key.hpp" #include "interfaces/iroha_internal/transaction_batch.hpp" #include "logger/logger.hpp" @@ -24,12 +26,15 @@ OnDemandOsServerGrpc::OnDemandOsServerGrpc( batch_parser, std::shared_ptr transaction_batch_factory, + std::shared_ptr field_validator, logger::LoggerPtr log) : ordering_service_(ordering_service), transaction_factory_(std::move(transaction_factory)), batch_parser_(std::move(batch_parser)), batch_factory_(std::move(transaction_batch_factory)), - log_(std::move(log)) {} + field_validator_(std::move(field_validator)), + log_(std::move(log)) { +} shared_model::interface::types::SharedTxsCollectionType OnDemandOsServerGrpc::deserializeTransactions( @@ -84,7 +89,9 @@ grpc::Status OnDemandOsServerGrpc::SendBatches( return acc; }); - ordering_service_->onBatches(std::move(batches), nullptr); + fetchPeer(request->peer_key()) | [this, &batches](auto &&peer) { + ordering_service_->onBatches(std::move(batches), peer); + }; return ::grpc::Status::OK; } @@ -93,13 +100,32 @@ grpc::Status OnDemandOsServerGrpc::RequestProposal( ::grpc::ServerContext *context, const proto::ProposalRequest *request, proto::ProposalResponse *response) { - ordering_service_->onRequestProposal( - {request->round().block_round(), request->round().reject_round()}, - nullptr) - | [&](auto &&proposal) { + fetchPeer(request->peer_key()) | [this, &request, &response](auto &&peer) { + + ordering_service_->onRequestProposal( + {request->round().block_round(), request->round().reject_round()}, peer) + | + [&](auto &&proposal) { *response->mutable_proposal() = std::move( static_cast(proposal.get()) ->getTransport()); }; + }; return ::grpc::Status::OK; } + +OnDemandOsServerGrpc::ParsedPeerType OnDemandOsServerGrpc::fetchPeer( + const std::string &pub_key) const { + std::shared_ptr key = + std::make_shared(pub_key); + shared_model::validation::ReasonsGroupType reason; + field_validator_->validatePubkey(reason, *key); + shared_model::validation::Answer answer; + answer.addReason(std::move(reason)); + if (answer.hasErrors()) { + log_->error("Peer key struct isn't parsed, {}", answer.reason()); + return boost::none; + } + + return key; +} diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.hpp b/irohad/ordering/impl/on_demand_os_server_grpc.hpp index b518b17109..783d6ec5d2 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.hpp @@ -8,11 +8,13 @@ #include "ordering/on_demand_os_transport.hpp" +#include #include "interfaces/iroha_internal/abstract_transport_factory.hpp" #include "interfaces/iroha_internal/transaction_batch_factory.hpp" #include "interfaces/iroha_internal/transaction_batch_parser.hpp" #include "logger/logger_fwd.hpp" #include "ordering.grpc.pb.h" +#include "validators/field_validator.hpp" namespace iroha { namespace ordering { @@ -35,6 +37,8 @@ namespace iroha { batch_parser, std::shared_ptr transaction_batch_factory, + std::shared_ptr + field_validator, logger::LoggerPtr log); grpc::Status SendBatches(::grpc::ServerContext *context, @@ -47,6 +51,11 @@ namespace iroha { proto::ProposalResponse *response) override; private: + using ParsedPeerType = + boost::optional>; + + ParsedPeerType fetchPeer(const std::string &pub_key) const; + /** * Flat map transport transactions to shared model */ @@ -60,6 +69,8 @@ namespace iroha { batch_parser_; std::shared_ptr batch_factory_; + std::shared_ptr + field_validator_; logger::LoggerPtr log_; }; diff --git a/irohad/ordering/on_demand_os_transport.hpp b/irohad/ordering/on_demand_os_transport.hpp index 8deff762b2..8450ae48f8 100644 --- a/irohad/ordering/on_demand_os_transport.hpp +++ b/irohad/ordering/on_demand_os_transport.hpp @@ -12,13 +12,16 @@ #include #include "consensus/round.hpp" +#include "interfaces/common_objects/types.hpp" namespace shared_model { namespace interface { class TransactionBatch; class Proposal; - class Peer; } // namespace interface + namespace crypto { + class PublicKey; + } // namespace crypto } // namespace shared_model namespace iroha { @@ -30,7 +33,7 @@ namespace iroha { /** * Type of peer identity */ - using PeerType = shared_model::interface::Peer; + using PeerType = shared_model::crypto::PublicKey; /** * Type of stored proposals @@ -84,11 +87,11 @@ namespace iroha { /** * Create corresponding OdOsNotification interface for peer * Returned pointer is guaranteed to be not equal to nullptr - * @param peer - peer to connect + * @param to - address of the peer to connect * @return connection represented with OdOsNotification interface */ virtual std::unique_ptr create( - const details::PeerType &to) = 0; + const shared_model::interface::types::AddressType &to) = 0; virtual ~OdOsNotificationFactory() = default; }; @@ -98,14 +101,15 @@ namespace iroha { using InitiatorPeerType = std::shared_ptr; using ProposalType = details::ProposalType; using TransactionBatchType = details::TransactionBatchType; - using CollectionType = details::CollectionType; + using BatchesCollectionType = details::CollectionType; + using PeerStateType = std::vector; /** * Callback on receiving transactions * @param batches - vector of passed transaction batches * @param from - peer which sends batches */ - virtual void onBatches(CollectionType batches, + virtual void onBatches(BatchesCollectionType batches, InitiatorPeerType from) = 0; /** diff --git a/schema/ordering.proto b/schema/ordering.proto index b9d0b2fb13..8e6792c444 100644 --- a/schema/ordering.proto +++ b/schema/ordering.proto @@ -4,6 +4,7 @@ package iroha.ordering.proto; import "transaction.proto"; import "proposal.proto"; import "endpoint.proto"; +import "primitive.proto"; import "google/protobuf/empty.proto"; service OrderingGateTransportGrpc { @@ -21,10 +22,12 @@ message ProposalRound { message BatchesRequest { repeated protocol.Transaction transactions = 1; + string peer_key = 2; } message ProposalRequest { ProposalRound round = 1; + string peer_key = 2; } message ProposalResponse { From a16c936bd581e170adb497cbddcf453d890a7305 Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Sat, 6 Apr 2019 17:48:09 +0300 Subject: [PATCH 06/22] fix kick out strategy tests Signed-off-by: Fedor Muratov --- ...ck_out_proposal_creation_strategy_test.cpp | 49 ++++++++++++------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp b/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp index 412563ac03..0292474536 100644 --- a/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp +++ b/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp @@ -8,7 +8,6 @@ #include #include #include "module/irohad/consensus/yac/mock_yac_supermajority_checker.hpp" -#include "module/irohad/consensus/yac/yac_test_util.hpp" using namespace iroha::ordering; @@ -19,7 +18,8 @@ class KickOutProposalCreationStrategyTest : public testing::Test { public: void SetUp() override { for (auto i = 0u; i < number_of_peers; ++i) { - peers.push_back(iroha::consensus::yac::makePeer(std::to_string(i))); + peers.push_back( + std::make_shared(std::to_string(i))); } supermajority_checker_ = @@ -41,13 +41,15 @@ class KickOutProposalCreationStrategyTest : public testing::Test { * @given initialized kickOutStrategy * @and onCollaborationOutcome is invoked for the first round * @when onProposal calls F times with different peers for further rounds - * @then onCollaborationOutcome call returns true + * @then shouldCreateRound returns true */ TEST_F(KickOutProposalCreationStrategyTest, OnNonMaliciousCase) { - EXPECT_CALL(*supermajority_checker_, hasMajority(0, 0)) + EXPECT_CALL(*supermajority_checker_, hasMajority(0, number_of_peers)) .WillOnce(Return(false)); - ASSERT_EQ(true, strategy_->onCollaborationOutcome({1, 0}, peers)); + strategy_->onCollaborationOutcome(peers); + + ASSERT_EQ(true, strategy_->shouldCreateRound({1, 0})); for (auto i = 0u; i < f; ++i) { strategy_->onProposal(peers.at(i), {2, 0}); @@ -55,20 +57,17 @@ TEST_F(KickOutProposalCreationStrategyTest, OnNonMaliciousCase) { EXPECT_CALL(*supermajority_checker_, hasMajority(f, number_of_peers)) .WillOnce(Return(false)); - ASSERT_EQ(true, strategy_->onCollaborationOutcome({2, 0}, peers)); + ASSERT_EQ(true, strategy_->shouldCreateRound({2, 0})); } /** * @given initialized kickOutStrategy * @and onCollaborationOutcome is invoked for the first round * @when onProposal calls F + 1 times with different peers for further rounds - * @then onCollaborationOutcome call returns false + * @then onCollaborationOutcome returns false */ TEST_F(KickOutProposalCreationStrategyTest, OnMaliciousCase) { - EXPECT_CALL(*supermajority_checker_, hasMajority(0, 0)) - .WillOnce(Return(false)); - - ASSERT_EQ(true, strategy_->onCollaborationOutcome({1, 0}, peers)); + strategy_->onCollaborationOutcome(peers); auto requested = f + 1; for (auto i = 0u; i < requested; ++i) { @@ -77,7 +76,7 @@ TEST_F(KickOutProposalCreationStrategyTest, OnMaliciousCase) { EXPECT_CALL(*supermajority_checker_, hasMajority(requested, number_of_peers)) .WillOnce(Return(true)); - ASSERT_EQ(false, strategy_->onCollaborationOutcome({2, 0}, peers)); + ASSERT_EQ(false, strategy_->shouldCreateRound({2, 0})); } /** @@ -87,10 +86,7 @@ TEST_F(KickOutProposalCreationStrategyTest, OnMaliciousCase) { * @then onCollaborationOutcome call returns true */ TEST_F(KickOutProposalCreationStrategyTest, RepeadedRequest) { - EXPECT_CALL(*supermajority_checker_, hasMajority(0, 0)) - .WillOnce(Return(false)); - - ASSERT_EQ(true, strategy_->onCollaborationOutcome({1, 0}, peers)); + strategy_->onCollaborationOutcome(peers); auto requested = f + 1; for (auto i = 0u; i < requested; ++i) { @@ -98,8 +94,25 @@ TEST_F(KickOutProposalCreationStrategyTest, RepeadedRequest) { } EXPECT_CALL(*supermajority_checker_, hasMajority(1, number_of_peers)) .WillOnce(Return(false)); - ASSERT_EQ(true, strategy_->onCollaborationOutcome({2, 0}, peers)); + ASSERT_EQ(true, strategy_->shouldCreateRound({2, 0})); } +/** + * @given initialized kickOutStrategy + * @and onCollaborationOutcome is invoked for the first round + * @when onProposal calls F times different peers + * @and 1 time with unknown peer + * @then onCollaborationOutcome call returns true + */ +TEST_F(KickOutProposalCreationStrategyTest, UnknownPeerRequestsProposal) { + strategy_->onCollaborationOutcome(peers); -/// todo test where peer out of the scope + for (auto i = 0u; i < f; ++i) { + strategy_->onProposal(peers.at(i), {2, 0}); + } + strategy_->onProposal( + std::make_shared("unknown"), {2, 0}); + EXPECT_CALL(*supermajority_checker_, hasMajority(f, number_of_peers)) + .WillOnce(Return(false)); + ASSERT_EQ(true, strategy_->shouldCreateRound({2, 0})); +} From 023fede9d2674e29735cd6cd879d839b7194d26d Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Sun, 7 Apr 2019 10:15:17 +0300 Subject: [PATCH 07/22] Revert interface of on demand connection factory Signed-off-by: Fedor Muratov --- irohad/main/application.cpp | 2 +- irohad/ordering/impl/on_demand_connection_manager.cpp | 2 +- irohad/ordering/impl/on_demand_os_client_grpc.cpp | 4 ++-- irohad/ordering/impl/on_demand_os_client_grpc.hpp | 2 +- irohad/ordering/on_demand_os_transport.hpp | 5 +++-- test/module/irohad/ordering/ordering_mocks.hpp | 2 +- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/irohad/main/application.cpp b/irohad/main/application.cpp index 193e39f7a5..31558660ef 100644 --- a/irohad/main/application.cpp +++ b/irohad/main/application.cpp @@ -376,7 +376,7 @@ void Irohad::initOrderingGate() { }; std::shared_ptr proposal_strategy = - std::make_shared( + std::make_shared( getSupermajorityChecker(kConsensusConsistencyModel)); auto field_validator = diff --git a/irohad/ordering/impl/on_demand_connection_manager.cpp b/irohad/ordering/impl/on_demand_connection_manager.cpp index 5a0274364f..bff96d5335 100644 --- a/irohad/ordering/impl/on_demand_connection_manager.cpp +++ b/irohad/ordering/impl/on_demand_connection_manager.cpp @@ -74,7 +74,7 @@ void OnDemandConnectionManager::initializeConnections( const CurrentPeers &peers) { auto create_assign = [this](auto &ptr, auto &peer) { std::lock_guard lock(mutex_); - ptr = factory_->create(peer->address()); + ptr = factory_->create(*peer); }; for (auto &&pair : boost::combine(connections_.peers, peers.peers)) { diff --git a/irohad/ordering/impl/on_demand_os_client_grpc.cpp b/irohad/ordering/impl/on_demand_os_client_grpc.cpp index 9d350fedb3..8ccde3b577 100644 --- a/irohad/ordering/impl/on_demand_os_client_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_client_grpc.cpp @@ -106,9 +106,9 @@ OnDemandOsClientGrpcFactory::OnDemandOsClientGrpcFactory( client_log_(std::move(client_log)) {} std::unique_ptr OnDemandOsClientGrpcFactory::create( - const shared_model::interface::types::AddressType &to) { + const shared_model::interface::Peer &to) { return std::make_unique( - network::createClient(to), + network::createClient(to.address()), async_call_, proposal_factory_, time_provider_, diff --git a/irohad/ordering/impl/on_demand_os_client_grpc.hpp b/irohad/ordering/impl/on_demand_os_client_grpc.hpp index 67468027d2..6e78428bee 100644 --- a/irohad/ordering/impl/on_demand_os_client_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_client_grpc.hpp @@ -82,7 +82,7 @@ namespace iroha { * This factory method can be used in production code */ std::unique_ptr create( - const shared_model::interface::types::AddressType &to) override; + const shared_model::interface::Peer &to) override; private: std::shared_ptr> diff --git a/irohad/ordering/on_demand_os_transport.hpp b/irohad/ordering/on_demand_os_transport.hpp index 8450ae48f8..e9604082fe 100644 --- a/irohad/ordering/on_demand_os_transport.hpp +++ b/irohad/ordering/on_demand_os_transport.hpp @@ -18,6 +18,7 @@ namespace shared_model { namespace interface { class TransactionBatch; class Proposal; + class Peer; } // namespace interface namespace crypto { class PublicKey; @@ -87,11 +88,11 @@ namespace iroha { /** * Create corresponding OdOsNotification interface for peer * Returned pointer is guaranteed to be not equal to nullptr - * @param to - address of the peer to connect + * @param peer - peer to connect * @return connection represented with OdOsNotification interface */ virtual std::unique_ptr create( - const shared_model::interface::types::AddressType &to) = 0; + const shared_model::interface::Peer &to) = 0; virtual ~OdOsNotificationFactory() = default; }; diff --git a/test/module/irohad/ordering/ordering_mocks.hpp b/test/module/irohad/ordering/ordering_mocks.hpp index b1acd9eb7e..ac7f3ded6d 100644 --- a/test/module/irohad/ordering/ordering_mocks.hpp +++ b/test/module/irohad/ordering/ordering_mocks.hpp @@ -36,7 +36,7 @@ namespace iroha { } // namespace cache struct MockOnDemandOrderingService : public OnDemandOrderingService { - MOCK_METHOD2(onBatches, void(CollectionType, InitiatorPeerType)); + MOCK_METHOD2(onBatches, void(BatchesCollectionType, InitiatorPeerType)); MOCK_METHOD2(onRequestProposal, boost::optional>( From be546f2c25567341655ab52d006ae48858626169 Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Sun, 7 Apr 2019 17:42:37 +0300 Subject: [PATCH 08/22] Fix reasons adding to answer validator Signed-off-by: Fedor Muratov --- shared_model/validators/answer.hpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/shared_model/validators/answer.hpp b/shared_model/validators/answer.hpp index 55f4e0227d..58d45da25e 100644 --- a/shared_model/validators/answer.hpp +++ b/shared_model/validators/answer.hpp @@ -59,10 +59,13 @@ namespace shared_model { /** * Adds error to map - * @param reasons + * @param reasons - errors for adding */ void addReason(ReasonsGroupType &&reasons) { - reasons_map_.insert(std::move(reasons)); + // checks that reason makes sense + if(reasons.second.size() != 0) { + reasons_map_.insert(std::move(reasons)); + } } std::map getReasonsMap() { From 3b835ebd5f5263092e0091bf9cfe3064479c63f4 Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Sun, 7 Apr 2019 17:43:23 +0300 Subject: [PATCH 09/22] Fix mocks regarding new interface and fix some module tests Signed-off-by: Fedor Muratov --- .../impl/on_demand_os_client_grpc.hpp | 1 + ...mock_on_demand_os_notification_os_side.hpp | 2 +- .../mock_proposal_creation_strategy.hpp | 3 +- .../ordering/on_demand_ordering_gate_test.cpp | 2 +- .../on_demand_os_client_grpc_test.cpp | 9 ++++- .../on_demand_os_server_grpc_test.cpp | 8 ++++ .../irohad/ordering/on_demand_os_test.cpp | 39 ++++++++++++------- .../torii/torii_transport_command_test.cpp | 6 ++- 8 files changed, 51 insertions(+), 19 deletions(-) diff --git a/irohad/ordering/impl/on_demand_os_client_grpc.hpp b/irohad/ordering/impl/on_demand_os_client_grpc.hpp index 6e78428bee..6a0091bc08 100644 --- a/irohad/ordering/impl/on_demand_os_client_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_client_grpc.hpp @@ -8,6 +8,7 @@ #include "ordering/on_demand_os_transport.hpp" +#include "cryptography/public_key.hpp" #include "interfaces/common_objects/types.hpp" #include "interfaces/iroha_internal/abstract_transport_factory.hpp" #include "logger/logger_fwd.hpp" diff --git a/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp b/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp index 149619e4ce..4d8719f33a 100644 --- a/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp +++ b/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp @@ -15,7 +15,7 @@ namespace iroha { namespace transport { struct MockOdOsNotificationOsSide : public OdNotificationOsSide { - MOCK_METHOD2(onBatches, void(CollectionType, InitiatorPeerType)); + MOCK_METHOD2(onBatches, void(BatchesCollectionType, InitiatorPeerType)); MOCK_METHOD2(onRequestProposal, boost::optional>( diff --git a/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp b/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp index 36443a306e..b0c7322535 100644 --- a/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp +++ b/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp @@ -14,7 +14,8 @@ namespace iroha { namespace ordering { class MockProposalCreationStrategy : public ProposalCreationStrategy { public: - MOCK_METHOD2(onCollaborationOutcome, bool(RoundType, const PeerList &)); + MOCK_METHOD1(onCollaborationOutcome, void(const PeerList &)); + MOCK_METHOD1(shouldCreateRound, bool(RoundType)); MOCK_METHOD2(onProposal, boost::optional(PeerType, RoundType requested_round)); diff --git a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp index 921858d01a..2d30fb9281 100644 --- a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp +++ b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp @@ -236,7 +236,7 @@ TEST_F(OnDemandOrderingGateTest, EmptyEventNoProposal) { */ TEST_F(OnDemandOrderingGateTest, ReplayedTransactionInProposal) { OnDemandOrderingGate::BlockEvent event = { - round, {}, std::make_shared()}; + round, {}, ledger_state}; // initialize mock transaction auto tx1 = std::make_shared>(); diff --git a/test/module/irohad/ordering/on_demand_os_client_grpc_test.cpp b/test/module/irohad/ordering/on_demand_os_client_grpc_test.cpp index 815d8ff32d..244fbb42a4 100644 --- a/test/module/irohad/ordering/on_demand_os_client_grpc_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_client_grpc_test.cpp @@ -20,12 +20,12 @@ using namespace iroha; using namespace iroha::ordering; using namespace iroha::ordering::transport; -using grpc::testing::MockClientAsyncResponseReader; using ::testing::_; using ::testing::DoAll; using ::testing::Return; using ::testing::SaveArg; using ::testing::SetArgPointee; +using grpc::testing::MockClientAsyncResponseReader; class OnDemandOsClientGrpcTest : public ::testing::Test { public: @@ -54,12 +54,16 @@ class OnDemandOsClientGrpcTest : public ::testing::Test { proto_proposal_validator = proto_validator.get(); proposal_factory = std::make_shared( std::move(validator), std::move(proto_validator)); + self_key = "self"; + auto self_pub_key = + std::make_shared(self_key); client = std::make_shared(std::move(ustub), async_call, proposal_factory, [&] { return timepoint; }, timeout, + self_pub_key, getTestLogger("OdOsClientGrpc")); } @@ -69,6 +73,7 @@ class OnDemandOsClientGrpcTest : public ::testing::Test { std::chrono::milliseconds timeout{1}; std::shared_ptr client; consensus::Round round{1, 2}; + std::string self_key; MockProposalValidator *proposal_validator; MockProtoProposalValidator *proto_proposal_validator; @@ -104,6 +109,7 @@ TEST_F(OnDemandOsClientGrpcTest, onBatches) { .reduced_payload() .creator_account_id(), creator); + ASSERT_EQ(self_key, request.peer_key()); } /** @@ -143,6 +149,7 @@ TEST_F(OnDemandOsClientGrpcTest, onRequestProposal) { ASSERT_EQ(request.round().reject_round(), round.reject_round); ASSERT_TRUE(proposal); ASSERT_EQ(proposal.value()->transactions()[0].creatorAccountId(), creator); + ASSERT_EQ(self_key, request.peer_key()); } /** diff --git a/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp b/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp index 2ccdac97e7..058bd5d097 100644 --- a/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp @@ -47,11 +47,16 @@ struct OnDemandOsServerGrpcTest : public ::testing::Test { auto batch_parser = std::make_shared(); batch_factory = std::make_shared(); + // todo rework field validator with mock + auto field_validator = + std::make_shared(); + server = std::make_shared(notification, std::move(transaction_factory), std::move(batch_parser), batch_factory, + field_validator, getTestLogger("OdOsServerGrpc")); } @@ -96,6 +101,7 @@ TEST_F(OnDemandOsServerGrpcTest, SendBatches) { EXPECT_CALL(*notification, onBatches(_, _)) .WillOnce(SaveArg0Move(&collection)); proto::BatchesRequest request; + *request.mutable_peer_key() = std::string(32, 'f'); request.add_transactions() ->mutable_payload() ->mutable_reduced_payload() @@ -116,6 +122,7 @@ TEST_F(OnDemandOsServerGrpcTest, SendBatches) { TEST_F(OnDemandOsServerGrpcTest, RequestProposal) { auto creator = "test"; proto::ProposalRequest request; + *request.mutable_peer_key() = std::string(32, 'f'); request.mutable_round()->set_block_round(round.block_round); request.mutable_round()->set_reject_round(round.reject_round); proto::ProposalResponse response; @@ -150,6 +157,7 @@ TEST_F(OnDemandOsServerGrpcTest, RequestProposal) { */ TEST_F(OnDemandOsServerGrpcTest, RequestProposalNone) { proto::ProposalRequest request; + *request.mutable_peer_key() = std::string(32, 'f'); request.mutable_round()->set_block_round(round.block_round); request.mutable_round()->set_reject_round(round.reject_round); proto::ProposalResponse response; diff --git a/test/module/irohad/ordering/on_demand_os_test.cpp b/test/module/irohad/ordering/on_demand_os_test.cpp index 7cac7d3897..e63d54c4de 100644 --- a/test/module/irohad/ordering/on_demand_os_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_test.cpp @@ -90,10 +90,10 @@ class OnDemandOsTest : public ::testing::Test { os->onBatches(generateTransactions(range), requester_peer); } - OnDemandOrderingService::CollectionType generateTransactions( + OnDemandOrderingService::BatchesCollectionType generateTransactions( std::pair range, shared_model::interface::types::TimestampType now = iroha::time::now()) { - OnDemandOrderingService::CollectionType collection; + OnDemandOrderingService::BatchesCollectionType collection; for (auto i = range.first; i < range.second; ++i) { collection.push_back( @@ -130,7 +130,8 @@ class OnDemandOsTest : public ::testing::Test { * @then check that previous round doesn't have proposal */ TEST_F(OnDemandOsTest, EmptyRound) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -149,7 +150,8 @@ TEST_F(OnDemandOsTest, EmptyRound) { * @then check that previous round has all transaction */ TEST_F(OnDemandOsTest, NormalRound) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -169,7 +171,8 @@ TEST_F(OnDemandOsTest, NormalRound) { * AND the rest of transactions isn't appeared in next after next round */ TEST_F(OnDemandOsTest, OverflowRound) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -192,7 +195,8 @@ TEST_F(OnDemandOsTest, OverflowRound) { * @then check that all transactions appear in proposal */ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -238,7 +242,9 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { * tryErase */ TEST_F(OnDemandOsTest, Erase) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)) + .Times(testing::AtLeast(1)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -267,7 +273,8 @@ TEST_F(OnDemandOsTest, Erase) { * @then check that proposal factory is called and returns a proposal */ TEST_F(OnDemandOsTest, UseFactoryForProposal) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -322,7 +329,8 @@ auto batchRef(const shared_model::interface::TransactionBatch &batch) { * @then the batch is not present in a proposal */ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -349,7 +357,8 @@ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { * @then batch is present in a proposal */ TEST_F(OnDemandOsTest, PassMissingTransaction) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -378,7 +387,8 @@ TEST_F(OnDemandOsTest, PassMissingTransaction) { * @then 2 new batches are in a proposal and already commited batch is discarded */ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -418,7 +428,8 @@ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { * @then the proposal contains the batch once */ TEST_F(OnDemandOsTest, DuplicateTxTest) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); @@ -441,7 +452,9 @@ TEST_F(OnDemandOsTest, DuplicateTxTest) { * @then both of them are used for the next proposal */ TEST_F(OnDemandOsTest, RejectCommit) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_, _)) + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)) + .Times(testing::AtLeast(1)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); diff --git a/test/module/irohad/torii/torii_transport_command_test.cpp b/test/module/irohad/torii/torii_transport_command_test.cpp index 705f58babb..abc2844382 100644 --- a/test/module/irohad/torii/torii_transport_command_test.cpp +++ b/test/module/irohad/torii/torii_transport_command_test.cpp @@ -182,7 +182,8 @@ TEST_F(CommandServiceTransportGrpcTest, ListToriiInvalid) { } shared_model::validation::Answer error; - error.addReason(std::make_pair("some error", std::vector{})); + error.addReason( + std::make_pair("some error", std::vector{"error_reason"})); EXPECT_CALL(*proto_tx_validator, validate(_)) .Times(kTimes) .WillRepeatedly(Return(shared_model::validation::Answer{})); @@ -222,7 +223,8 @@ TEST_F(CommandServiceTransportGrpcTest, ListToriiPartialInvalid) { .WillRepeatedly(Invoke([this, &counter, kError](const auto &) mutable { shared_model::validation::Answer res; if (counter++ == kTimes - 1) { - res.addReason(std::make_pair(kError, std::vector{})); + res.addReason( + std::make_pair(kError, std::vector{"error_reason"})); } return res; })); From c29f107f08879474fd3c293d5f40854cdd864429 Mon Sep 17 00:00:00 2001 From: Fedor Muratov Date: Sun, 7 Apr 2019 22:37:57 +0300 Subject: [PATCH 10/22] Clean up branch Signed-off-by: Fedor Muratov --- .../kick_out_proposal_creation_strategy.hpp | 1 - libs/common/pointer_utils.hpp | 27 ------------------- schema/ordering.proto | 1 - .../interfaces/common_objects/impl/peer.cpp | 5 ---- .../interfaces/common_objects/peer.hpp | 5 ---- shared_model/validators/answer.hpp | 2 +- .../irohad/ordering/on_demand_os_test.cpp | 20 +++++++++++++- 7 files changed, 20 insertions(+), 41 deletions(-) delete mode 100644 libs/common/pointer_utils.hpp diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp index cbb3c2711a..b506176e02 100644 --- a/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp @@ -10,7 +10,6 @@ #include #include -#include "common/pointer_utils.hpp" #include "consensus/round.hpp" #include "consensus/yac/supermajority_checker.hpp" diff --git a/libs/common/pointer_utils.hpp b/libs/common/pointer_utils.hpp deleted file mode 100644 index 66127c25bf..0000000000 --- a/libs/common/pointer_utils.hpp +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Copyright Soramitsu Co., Ltd. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -#ifndef IROHA_POINTER_UTILS_HPP -#define IROHA_POINTER_UTILS_HPP - -#include - -template -class DereferenceLess { - public: - bool operator()(const L &lhs, const R &rhs) const { - return Less()(lhs, rhs); - } -}; - -template -class DereferenceHash { - public: - size_t operator()(const Ptr &lhs) const { - return Hash()(*lhs); - } -}; - -#endif // IROHA_POINTER_UTILS_HPP diff --git a/schema/ordering.proto b/schema/ordering.proto index 8e6792c444..be47730b04 100644 --- a/schema/ordering.proto +++ b/schema/ordering.proto @@ -4,7 +4,6 @@ package iroha.ordering.proto; import "transaction.proto"; import "proposal.proto"; import "endpoint.proto"; -import "primitive.proto"; import "google/protobuf/empty.proto"; service OrderingGateTransportGrpc { diff --git a/shared_model/interfaces/common_objects/impl/peer.cpp b/shared_model/interfaces/common_objects/impl/peer.cpp index acd3b185a2..e09354940d 100644 --- a/shared_model/interfaces/common_objects/impl/peer.cpp +++ b/shared_model/interfaces/common_objects/impl/peer.cpp @@ -20,10 +20,5 @@ namespace shared_model { bool Peer::operator==(const ModelType &rhs) const { return address() == rhs.address() and pubkey() == rhs.pubkey(); } - - size_t Peer::PeerHash::operator()( - const shared_model::interface::Peer &t) const { - return std::hash()(t.address() + t.pubkey().toString()); - } } // namespace interface } // namespace shared_model diff --git a/shared_model/interfaces/common_objects/peer.hpp b/shared_model/interfaces/common_objects/peer.hpp index c02e8e6b07..03ae9f2175 100644 --- a/shared_model/interfaces/common_objects/peer.hpp +++ b/shared_model/interfaces/common_objects/peer.hpp @@ -30,11 +30,6 @@ namespace shared_model { std::string toString() const override; bool operator==(const ModelType &rhs) const override; - - class PeerHash { - public: - size_t operator()(const Peer &) const; - }; }; } // namespace interface } // namespace shared_model diff --git a/shared_model/validators/answer.hpp b/shared_model/validators/answer.hpp index 58d45da25e..1eae513c4f 100644 --- a/shared_model/validators/answer.hpp +++ b/shared_model/validators/answer.hpp @@ -62,7 +62,7 @@ namespace shared_model { * @param reasons - errors for adding */ void addReason(ReasonsGroupType &&reasons) { - // checks that reason makes sense + // checks that reasons aren't empty if(reasons.second.size() != 0) { reasons_map_.insert(std::move(reasons)); } diff --git a/test/module/irohad/ordering/on_demand_os_test.cpp b/test/module/irohad/ordering/on_demand_os_test.cpp index e63d54c4de..f3fedc9480 100644 --- a/test/module/irohad/ordering/on_demand_os_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_test.cpp @@ -481,4 +481,22 @@ TEST_F(OnDemandOsTest, RejectCommit) { ASSERT_EQ(2, boost::size((*proposal)->transactions())); } -// todo add test with malicious case in strategy +/** + * @given initialized on-demand OS with a batch inside + * @when creation strategy denies creation of new proposals + * @then check that prposal isn't created + */ +TEST_F(OnDemandOsTest, FailOnCreationStrategy) { + EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); + EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) + .WillRepeatedly(testing::Return(false)); + EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) + .WillRepeatedly(testing::Return(boost::none)); + + generateTransactionsAndInsert({1, 2}); + + os->onCollaborationOutcome(commit_round, started_list); + + ASSERT_FALSE(os->onRequestProposal(target_round, requester_peer)); +} + From 35043b0af0fccf88d3c5c19589e401aa8d714477 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 10:43:01 +0300 Subject: [PATCH 11/22] Move onProposal from OS to Server Signed-off-by: Andrei Lebedev --- irohad/main/impl/on_demand_ordering_init.cpp | 1 + .../impl/on_demand_ordering_service_impl.cpp | 2 -- .../impl/on_demand_os_server_grpc.cpp | 23 ++++++++++--------- .../impl/on_demand_os_server_grpc.hpp | 4 ++++ .../on_demand_os_server_grpc_test.cpp | 7 +++++- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/irohad/main/impl/on_demand_ordering_init.cpp b/irohad/main/impl/on_demand_ordering_init.cpp index f2fb4e56b7..3b8c4ce446 100644 --- a/irohad/main/impl/on_demand_ordering_init.cpp +++ b/irohad/main/impl/on_demand_ordering_init.cpp @@ -320,6 +320,7 @@ namespace iroha { std::move(batch_parser), std::move(transaction_batch_factory), field_validator, + creation_strategy, ordering_log_manager->getChild("Server")->getLogger()); return createGate( ordering_service, diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index c9f861a3cf..9a3e7269ae 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -83,8 +83,6 @@ OnDemandOrderingServiceImpl::onRequestProposal(consensus::Round round, result; { std::shared_lock lock(proposals_mutex_); - // todo add force initialization of proposal - proposal_creation_strategy_->onProposal(requester, round); auto it = proposal_map_.find(round); result = boost::make_optional(it != proposal_map_.end(), it->second); } diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.cpp b/irohad/ordering/impl/on_demand_os_server_grpc.cpp index 9c643997d4..cb4b2dd225 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.cpp @@ -27,14 +27,15 @@ OnDemandOsServerGrpc::OnDemandOsServerGrpc( std::shared_ptr transaction_batch_factory, std::shared_ptr field_validator, + std::shared_ptr proposal_creation_strategy, logger::LoggerPtr log) : ordering_service_(ordering_service), transaction_factory_(std::move(transaction_factory)), batch_parser_(std::move(batch_parser)), batch_factory_(std::move(transaction_batch_factory)), field_validator_(std::move(field_validator)), - log_(std::move(log)) { -} + proposal_creation_strategy_(proposal_creation_strategy), + log_(std::move(log)) {} shared_model::interface::types::SharedTxsCollectionType OnDemandOsServerGrpc::deserializeTransactions( @@ -101,15 +102,15 @@ grpc::Status OnDemandOsServerGrpc::RequestProposal( const proto::ProposalRequest *request, proto::ProposalResponse *response) { fetchPeer(request->peer_key()) | [this, &request, &response](auto &&peer) { - - ordering_service_->onRequestProposal( - {request->round().block_round(), request->round().reject_round()}, peer) - | - [&](auto &&proposal) { - *response->mutable_proposal() = std::move( - static_cast(proposal.get()) - ->getTransport()); - }; + consensus::Round round{request->round().block_round(), + request->round().reject_round()}; + // todo add force initialization of proposal + proposal_creation_strategy_->onProposal(peer, round); + ordering_service_->onRequestProposal(round, peer) | [&](auto &&proposal) { + *response->mutable_proposal() = std::move( + static_cast(proposal.get()) + ->getTransport()); + }; }; return ::grpc::Status::OK; } diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.hpp b/irohad/ordering/impl/on_demand_os_server_grpc.hpp index 783d6ec5d2..61c7c6d7e6 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.hpp @@ -14,6 +14,7 @@ #include "interfaces/iroha_internal/transaction_batch_parser.hpp" #include "logger/logger_fwd.hpp" #include "ordering.grpc.pb.h" +#include "ordering/ordering_service_proposal_creation_strategy.hpp" #include "validators/field_validator.hpp" namespace iroha { @@ -39,6 +40,8 @@ namespace iroha { transaction_batch_factory, std::shared_ptr field_validator, + std::shared_ptr + proposal_creation_strategy, logger::LoggerPtr log); grpc::Status SendBatches(::grpc::ServerContext *context, @@ -71,6 +74,7 @@ namespace iroha { batch_factory_; std::shared_ptr field_validator_; + std::shared_ptr proposal_creation_strategy_; logger::LoggerPtr log_; }; diff --git a/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp b/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp index 058bd5d097..c01dfb10f0 100644 --- a/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp @@ -13,6 +13,7 @@ #include "interfaces/iroha_internal/transaction_batch_impl.hpp" #include "interfaces/iroha_internal/transaction_batch_parser_impl.hpp" #include "module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp" +#include "module/irohad/ordering/mock_proposal_creation_strategy.hpp" #include "module/shared_model/interface/mock_transaction_batch_factory.hpp" #include "module/shared_model/validators/validators.hpp" @@ -50,6 +51,8 @@ struct OnDemandOsServerGrpcTest : public ::testing::Test { // todo rework field validator with mock auto field_validator = std::make_shared(); + proposal_creation_strategy = + std::make_shared(); server = std::make_shared(notification, @@ -57,17 +60,19 @@ struct OnDemandOsServerGrpcTest : public ::testing::Test { std::move(batch_parser), batch_factory, field_validator, + proposal_creation_strategy, getTestLogger("OdOsServerGrpc")); } std::shared_ptr notification; std::shared_ptr batch_factory; + std::shared_ptr proposal_creation_strategy; std::shared_ptr server; consensus::Round round{1, 2}; }; /** - * Separate action required because CollectionType is non-copy1able + * Separate action required because CollectionType is non-copyable */ ACTION_P(SaveArg0Move, var) { *var = std::move(arg0); From 85d7ab028222d58eda03325c84bdbed925cb2997 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 15:17:26 +0300 Subject: [PATCH 12/22] Refactor proposal strategy interface Signed-off-by: Andrei Lebedev --- irohad/main/impl/on_demand_ordering_init.cpp | 3 ++ irohad/main/impl/on_demand_ordering_init.hpp | 1 + .../kick_out_proposal_creation_strategy.cpp | 49 +++++++++-------- .../kick_out_proposal_creation_strategy.hpp | 10 ++-- .../ordering/impl/on_demand_ordering_gate.cpp | 8 ++- .../ordering/impl/on_demand_ordering_gate.hpp | 3 ++ .../impl/on_demand_ordering_service_impl.cpp | 1 - .../impl/on_demand_os_server_grpc.cpp | 28 +++++----- .../impl/on_demand_os_server_grpc.hpp | 6 +-- ...ing_service_proposal_creation_strategy.hpp | 11 ++-- ...ck_out_proposal_creation_strategy_test.cpp | 10 ++-- .../mock_proposal_creation_strategy.hpp | 3 +- .../ordering/on_demand_ordering_gate_test.cpp | 8 ++- .../irohad/ordering/on_demand_os_test.cpp | 54 +++++++------------ 14 files changed, 98 insertions(+), 97 deletions(-) diff --git a/irohad/main/impl/on_demand_ordering_init.cpp b/irohad/main/impl/on_demand_ordering_init.cpp index 3b8c4ce446..906b606fa8 100644 --- a/irohad/main/impl/on_demand_ordering_init.cpp +++ b/irohad/main/impl/on_demand_ordering_init.cpp @@ -211,6 +211,7 @@ namespace iroha { std::shared_ptr proposal_factory, std::shared_ptr tx_cache, + std::shared_ptr creation_strategy, std::function delay_func, size_t max_number_of_transactions, @@ -260,6 +261,7 @@ namespace iroha { std::move(cache), std::move(proposal_factory), std::move(tx_cache), + std::move(creation_strategy), max_number_of_transactions, ordering_log_manager->getChild("Gate")->getLogger()); } @@ -334,6 +336,7 @@ namespace iroha { std::make_shared(), std::move(proposal_factory), std::move(tx_cache), + std::move(creation_strategy), std::move(delay_func), max_number_of_transactions, ordering_log_manager); diff --git a/irohad/main/impl/on_demand_ordering_init.hpp b/irohad/main/impl/on_demand_ordering_init.hpp index 10165033d1..c2fbbee7f2 100644 --- a/irohad/main/impl/on_demand_ordering_init.hpp +++ b/irohad/main/impl/on_demand_ordering_init.hpp @@ -74,6 +74,7 @@ namespace iroha { std::shared_ptr proposal_factory, std::shared_ptr tx_cache, + std::shared_ptr creation_strategy, std::function delay_func, size_t max_number_of_transactions, diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp index d2d83adce8..23b75c37e0 100644 --- a/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.cpp @@ -5,7 +5,7 @@ #include "ordering/impl/kick_out_proposal_creation_strategy.hpp" -#include +#include #include "interfaces/common_objects/peer.hpp" using namespace iroha::ordering; @@ -16,30 +16,28 @@ KickOutProposalCreationStrategy::KickOutProposalCreationStrategy( void KickOutProposalCreationStrategy::onCollaborationOutcome( const PeerList &peers) { - last_requested_ = - std::accumulate(peers.begin(), - peers.end(), - RoundCollectionType{}, - [this](auto collection, const auto &peer) { - auto iter = last_requested_.find(peer->hex()); - if (iter != last_requested_.end()) { - collection.insert(*iter); - } else { - collection.insert({peer->hex(), RoundType{0, 0}}); - } - return std::move(collection); - }); + std::lock_guard guard(mutex_); + RoundCollectionType last_requested; + for (const auto &peer : peers) { + auto iter = last_requested_.find(peer.hex()); + if (iter != last_requested_.end()) { + last_requested.insert(*iter); + } else { + last_requested.insert({peer.hex(), RoundType{0, 0}}); + } + } + last_requested_ = last_requested; } bool KickOutProposalCreationStrategy::shouldCreateRound(RoundType round) { uint64_t counter = 0; - std::for_each(last_requested_.begin(), - last_requested_.end(), - [&round, &counter](const auto &elem) { - if (elem.second >= round) { - counter++; - } - }); + { + std::lock_guard guard(mutex_); + counter = std::count_if( + last_requested_.begin(), + last_requested_.end(), + [&round](const auto &elem) { return elem.second >= round; }); + } auto has_majority = majority_checker_->hasMajority(counter, last_requested_.size()); @@ -47,11 +45,12 @@ bool KickOutProposalCreationStrategy::shouldCreateRound(RoundType round) { } boost::optional -KickOutProposalCreationStrategy::onProposal(PeerType who, +KickOutProposalCreationStrategy::onProposal(const PeerType &who, RoundType requested_round) { - auto iter = last_requested_.find(who->hex()); - if (iter != last_requested_.end()) { - if (iter->second < requested_round) { + { + std::lock_guard guard(mutex_); + auto iter = last_requested_.find(who.hex()); + if (iter != last_requested_.end() and iter->second < requested_round) { iter->second = requested_round; } } diff --git a/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp b/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp index b506176e02..9e6353d993 100644 --- a/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp +++ b/irohad/ordering/impl/kick_out_proposal_creation_strategy.hpp @@ -9,8 +9,9 @@ #include "ordering/ordering_service_proposal_creation_strategy.hpp" #include +#include #include -#include "consensus/round.hpp" + #include "consensus/yac/supermajority_checker.hpp" namespace iroha { @@ -21,7 +22,7 @@ namespace iroha { using SupermajorityCheckerType = iroha::consensus::yac::SupermajorityChecker; KickOutProposalCreationStrategy( - std::shared_ptr); + std::shared_ptr majority_checker); /** * Update peers state with new peers. @@ -31,14 +32,15 @@ namespace iroha { */ void onCollaborationOutcome(const PeerList &peers) override; - bool shouldCreateRound(RoundType round); + bool shouldCreateRound(RoundType round) override; - boost::optional onProposal(PeerType who, + boost::optional onProposal(const PeerType &who, RoundType requested_round) override; private: using RoundCollectionType = std::unordered_map; + std::mutex mutex_; std::shared_ptr majority_checker_; RoundCollectionType last_requested_; }; diff --git a/irohad/ordering/impl/on_demand_ordering_gate.cpp b/irohad/ordering/impl/on_demand_ordering_gate.cpp index 04e4835f4a..498bf5506f 100644 --- a/irohad/ordering/impl/on_demand_ordering_gate.cpp +++ b/irohad/ordering/impl/on_demand_ordering_gate.cpp @@ -43,6 +43,7 @@ OnDemandOrderingGate::OnDemandOrderingGate( std::shared_ptr cache, std::shared_ptr factory, std::shared_ptr tx_cache, + std::shared_ptr proposal_creation_strategy, size_t transaction_limit, logger::LoggerPtr log) : log_(std::move(log)), @@ -71,6 +72,10 @@ OnDemandOrderingGate::OnDemandOrderingGate( return lst; }; // notify our ordering service about new round + proposal_creation_strategy_->onCollaborationOutcome( + *ledger_state->ledger_peers + | boost::adaptors::transformed( + [](auto &peer) -> decltype(auto) { return peer->pubkey(); })); ordering_service_->onCollaborationOutcome( current_round, peer_keys(*ledger_state->ledger_peers)); @@ -85,7 +90,8 @@ OnDemandOrderingGate::OnDemandOrderingGate( })), cache_(std::move(cache)), proposal_factory_(std::move(factory)), - tx_cache_(std::move(tx_cache)) {} + tx_cache_(std::move(tx_cache)), + proposal_creation_strategy_(std::move(proposal_creation_strategy)) {} OnDemandOrderingGate::~OnDemandOrderingGate() { events_subscription_.unsubscribe(); diff --git a/irohad/ordering/impl/on_demand_ordering_gate.hpp b/irohad/ordering/impl/on_demand_ordering_gate.hpp index 2a6ee2ca92..6e224abff5 100644 --- a/irohad/ordering/impl/on_demand_ordering_gate.hpp +++ b/irohad/ordering/impl/on_demand_ordering_gate.hpp @@ -19,6 +19,7 @@ #include "logger/logger_fwd.hpp" #include "ordering/impl/ordering_gate_cache/ordering_gate_cache.hpp" #include "ordering/on_demand_ordering_service.hpp" +#include "ordering/ordering_service_proposal_creation_strategy.hpp" namespace iroha { namespace ametsuchi { @@ -71,6 +72,7 @@ namespace iroha { std::shared_ptr factory, std::shared_ptr tx_cache, + std::shared_ptr proposal_creation_strategy, size_t transaction_limit, logger::LoggerPtr log); @@ -112,6 +114,7 @@ namespace iroha { std::shared_ptr proposal_factory_; std::shared_ptr tx_cache_; + std::shared_ptr proposal_creation_strategy_; rxcpp::subjects::subject proposal_notifier_; }; diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index 9a3e7269ae..bcedf95e91 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -50,7 +50,6 @@ void OnDemandOrderingServiceImpl::onCollaborationOutcome( log_->info("onCollaborationOutcome => {}", round); packNextProposals(round); - proposal_creation_strategy_->onCollaborationOutcome(peers); tryErase(round); } diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.cpp b/irohad/ordering/impl/on_demand_os_server_grpc.cpp index cb4b2dd225..1899c8fde2 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.cpp @@ -91,7 +91,9 @@ grpc::Status OnDemandOsServerGrpc::SendBatches( }); fetchPeer(request->peer_key()) | [this, &batches](auto &&peer) { - ordering_service_->onBatches(std::move(batches), peer); + ordering_service_->onBatches( + std::move(batches), + std::make_shared(peer)); }; return ::grpc::Status::OK; @@ -106,25 +108,27 @@ grpc::Status OnDemandOsServerGrpc::RequestProposal( request->round().reject_round()}; // todo add force initialization of proposal proposal_creation_strategy_->onProposal(peer, round); - ordering_service_->onRequestProposal(round, peer) | [&](auto &&proposal) { - *response->mutable_proposal() = std::move( - static_cast(proposal.get()) - ->getTransport()); - }; + ordering_service_->onRequestProposal( + round, std::make_shared(peer)) + | + [&](auto &&proposal) { + *response->mutable_proposal() = std::move( + static_cast(proposal.get()) + ->getTransport()); + }; }; return ::grpc::Status::OK; } -OnDemandOsServerGrpc::ParsedPeerType OnDemandOsServerGrpc::fetchPeer( - const std::string &pub_key) const { - std::shared_ptr key = - std::make_shared(pub_key); +boost::optional +OnDemandOsServerGrpc::fetchPeer(const std::string &pub_key) const { + shared_model::crypto::PublicKey key{pub_key}; shared_model::validation::ReasonsGroupType reason; - field_validator_->validatePubkey(reason, *key); + field_validator_->validatePubkey(reason, key); shared_model::validation::Answer answer; answer.addReason(std::move(reason)); if (answer.hasErrors()) { - log_->error("Peer key struct isn't parsed, {}", answer.reason()); + log_->error("Failed to parse peer key struct, {}", answer.reason()); return boost::none; } diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.hpp b/irohad/ordering/impl/on_demand_os_server_grpc.hpp index 61c7c6d7e6..1be1526f3e 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.hpp @@ -54,10 +54,8 @@ namespace iroha { proto::ProposalResponse *response) override; private: - using ParsedPeerType = - boost::optional>; - - ParsedPeerType fetchPeer(const std::string &pub_key) const; + boost::optional fetchPeer( + const std::string &pub_key) const; /** * Flat map transport transactions to shared model diff --git a/irohad/ordering/ordering_service_proposal_creation_strategy.hpp b/irohad/ordering/ordering_service_proposal_creation_strategy.hpp index 25ba6d33ee..711496ce55 100644 --- a/irohad/ordering/ordering_service_proposal_creation_strategy.hpp +++ b/irohad/ordering/ordering_service_proposal_creation_strategy.hpp @@ -6,10 +6,8 @@ #ifndef IROHA_ORDERING_SERVICE_PROPOSAL_CREATION_STRATEGY_HPP #define IROHA_ORDERING_SERVICE_PROPOSAL_CREATION_STRATEGY_HPP -#include -#include - #include +#include #include "consensus/round.hpp" #include "cryptography/public_key.hpp" @@ -23,11 +21,12 @@ namespace iroha { class ProposalCreationStrategy { public: /// shortcut for peer type - using PeerType = std::shared_ptr; + using PeerType = shared_model::crypto::PublicKey; /// shortcut for round type using RoundType = consensus::Round; /// collection of peers type - using PeerList = std::vector; + using PeerList = boost:: + any_range; /** * Indicates the start of new round. @@ -49,7 +48,7 @@ namespace iroha { * @return round where proposal is required to be created immediately */ virtual boost::optional onProposal( - PeerType who, RoundType requested_round) = 0; + const PeerType &who, RoundType requested_round) = 0; virtual ~ProposalCreationStrategy() = default; }; diff --git a/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp b/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp index 0292474536..9848968a80 100644 --- a/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp +++ b/test/module/irohad/ordering/kick_out_proposal_creation_strategy_test.cpp @@ -5,6 +5,8 @@ #include "ordering/impl/kick_out_proposal_creation_strategy.hpp" +#include + #include #include #include "module/irohad/consensus/yac/mock_yac_supermajority_checker.hpp" @@ -18,8 +20,7 @@ class KickOutProposalCreationStrategyTest : public testing::Test { public: void SetUp() override { for (auto i = 0u; i < number_of_peers; ++i) { - peers.push_back( - std::make_shared(std::to_string(i))); + peers.emplace_back(std::to_string(i)); } supermajority_checker_ = @@ -32,7 +33,7 @@ class KickOutProposalCreationStrategyTest : public testing::Test { std::shared_ptr supermajority_checker_; - KickOutProposalCreationStrategy::PeerList peers; + std::vector peers; size_t number_of_peers = 7; size_t f = 2; }; @@ -110,8 +111,7 @@ TEST_F(KickOutProposalCreationStrategyTest, UnknownPeerRequestsProposal) { for (auto i = 0u; i < f; ++i) { strategy_->onProposal(peers.at(i), {2, 0}); } - strategy_->onProposal( - std::make_shared("unknown"), {2, 0}); + strategy_->onProposal(shared_model::crypto::PublicKey{"unknown"}, {2, 0}); EXPECT_CALL(*supermajority_checker_, hasMajority(f, number_of_peers)) .WillOnce(Return(false)); ASSERT_EQ(true, strategy_->shouldCreateRound({2, 0})); diff --git a/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp b/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp index b0c7322535..fd484ec869 100644 --- a/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp +++ b/test/module/irohad/ordering/mock_proposal_creation_strategy.hpp @@ -17,8 +17,7 @@ namespace iroha { MOCK_METHOD1(onCollaborationOutcome, void(const PeerList &)); MOCK_METHOD1(shouldCreateRound, bool(RoundType)); MOCK_METHOD2(onProposal, - boost::optional(PeerType, - RoundType requested_round)); + boost::optional(const PeerType &, RoundType)); }; } // namespace ordering } // namespace iroha diff --git a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp index 2d30fb9281..d6d3247014 100644 --- a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp +++ b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp @@ -13,6 +13,7 @@ #include "module/irohad/ametsuchi/mock_tx_presence_cache.hpp" #include "module/irohad/consensus/yac/yac_test_util.hpp" #include "module/irohad/ordering/mock_on_demand_os_notification.hpp" +#include "module/irohad/ordering/mock_proposal_creation_strategy.hpp" #include "module/irohad/ordering/ordering_mocks.hpp" #include "module/shared_model/interface_mocks.hpp" #include "ordering/impl/on_demand_common.hpp" @@ -42,6 +43,8 @@ class OnDemandOrderingGateTest : public ::testing::Test { auto ufactory = std::make_unique>(); factory = ufactory.get(); tx_cache = std::make_shared(); + proposal_creation_strategy = + std::make_shared(); ON_CALL(*tx_cache, check(testing::Matcher(_))) .WillByDefault( @@ -54,6 +57,7 @@ class OnDemandOrderingGateTest : public ::testing::Test { cache, std::move(ufactory), tx_cache, + proposal_creation_strategy, 1000, getTestLogger("OrderingGate")); using PeerListType = @@ -69,6 +73,7 @@ class OnDemandOrderingGateTest : public ::testing::Test { std::shared_ptr notification; NiceMock *factory; std::shared_ptr tx_cache; + std::shared_ptr proposal_creation_strategy; std::shared_ptr ordering_gate; std::shared_ptr cache; @@ -235,8 +240,7 @@ TEST_F(OnDemandOrderingGateTest, EmptyEventNoProposal) { * this transaction */ TEST_F(OnDemandOrderingGateTest, ReplayedTransactionInProposal) { - OnDemandOrderingGate::BlockEvent event = { - round, {}, ledger_state}; + OnDemandOrderingGate::BlockEvent event = {round, {}, ledger_state}; // initialize mock transaction auto tx1 = std::make_shared>(); diff --git a/test/module/irohad/ordering/on_demand_os_test.cpp b/test/module/irohad/ordering/on_demand_os_test.cpp index f3fedc9480..c99bde7624 100644 --- a/test/module/irohad/ordering/on_demand_os_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_test.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include "backend/protobuf/proto_proposal_factory.hpp" @@ -48,8 +49,8 @@ class OnDemandOsTest : public ::testing::Test { reject_round = {2, kNextRejectRoundConsumer}; NiceMock *mock_cache; std::shared_ptr proposal_creation_strategy; - ProposalCreationStrategy::PeerList started_list; - ProposalCreationStrategy::PeerType requester_peer; + std::vector> initial_list; + std::shared_ptr requester_peer; void SetUp() override { // TODO: nickaleks IR-1811 use mock factory @@ -69,8 +70,6 @@ class OnDemandOsTest : public ::testing::Test { proposal_creation_strategy = std::make_shared(); - started_list = {}; - requester_peer = nullptr; os = std::make_shared( transaction_limit, @@ -130,7 +129,6 @@ class OnDemandOsTest : public ::testing::Test { * @then check that previous round doesn't have proposal */ TEST_F(OnDemandOsTest, EmptyRound) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -138,7 +136,7 @@ TEST_F(OnDemandOsTest, EmptyRound) { ASSERT_FALSE(os->onRequestProposal(initial_round, requester_peer)); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); ASSERT_FALSE(os->onRequestProposal(initial_round, requester_peer)); } @@ -150,7 +148,6 @@ TEST_F(OnDemandOsTest, EmptyRound) { * @then check that previous round has all transaction */ TEST_F(OnDemandOsTest, NormalRound) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -158,7 +155,7 @@ TEST_F(OnDemandOsTest, NormalRound) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); } @@ -171,7 +168,6 @@ TEST_F(OnDemandOsTest, NormalRound) { * AND the rest of transactions isn't appeared in next after next round */ TEST_F(OnDemandOsTest, OverflowRound) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -179,7 +175,7 @@ TEST_F(OnDemandOsTest, OverflowRound) { generateTransactionsAndInsert({1, transaction_limit * 2}); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); ASSERT_EQ(transaction_limit, @@ -195,7 +191,6 @@ TEST_F(OnDemandOsTest, OverflowRound) { * @then check that all transactions appear in proposal */ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -225,7 +220,7 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { std::thread two(call, std::make_pair(large_tx_limit / 2, large_tx_limit)); one.join(); two.join(); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); ASSERT_EQ(large_tx_limit, os->onRequestProposal(target_round, requester_peer) .get() @@ -242,8 +237,6 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { * tryErase */ TEST_F(OnDemandOsTest, Erase) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)) - .Times(testing::AtLeast(1)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -251,7 +244,7 @@ TEST_F(OnDemandOsTest, Erase) { generateTransactionsAndInsert({1, 2}); os->onCollaborationOutcome( - {commit_round.block_round, commit_round.reject_round}, started_list); + {commit_round.block_round, commit_round.reject_round}, initial_list); ASSERT_TRUE(os->onRequestProposal( {commit_round.block_round + 1, commit_round.reject_round}, requester_peer)); @@ -260,7 +253,7 @@ TEST_F(OnDemandOsTest, Erase) { i < (commit_round.reject_round + 1) + (proposal_limit + 2); ++i) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome({commit_round.block_round, i}, started_list); + os->onCollaborationOutcome({commit_round.block_round, i}, initial_list); } ASSERT_TRUE(os->onRequestProposal( {commit_round.block_round + 1, commit_round.reject_round}, @@ -273,7 +266,6 @@ TEST_F(OnDemandOsTest, Erase) { * @then check that proposal factory is called and returns a proposal */ TEST_F(OnDemandOsTest, UseFactoryForProposal) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -312,7 +304,7 @@ TEST_F(OnDemandOsTest, UseFactoryForProposal) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); } @@ -329,7 +321,6 @@ auto batchRef(const shared_model::interface::TransactionBatch &batch) { * @then the batch is not present in a proposal */ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -344,7 +335,7 @@ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); auto proposal = os->onRequestProposal(initial_round, requester_peer); @@ -357,7 +348,6 @@ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { * @then batch is present in a proposal */ TEST_F(OnDemandOsTest, PassMissingTransaction) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -372,7 +362,7 @@ TEST_F(OnDemandOsTest, PassMissingTransaction) { os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); auto proposal = os->onRequestProposal(target_round, requester_peer); @@ -387,7 +377,6 @@ TEST_F(OnDemandOsTest, PassMissingTransaction) { * @then 2 new batches are in a proposal and already commited batch is discarded */ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -410,7 +399,7 @@ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); auto proposal = os->onRequestProposal(target_round, requester_peer); const auto &txs = proposal->get()->transactions(); @@ -428,7 +417,6 @@ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { * @then the proposal contains the batch once */ TEST_F(OnDemandOsTest, DuplicateTxTest) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -440,7 +428,7 @@ TEST_F(OnDemandOsTest, DuplicateTxTest) { auto txs2 = generateTransactions({1, 2}, now); os->onBatches(txs2, requester_peer); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); auto proposal = os->onRequestProposal(target_round, requester_peer); ASSERT_EQ(1, boost::size((*proposal)->transactions())); @@ -452,8 +440,6 @@ TEST_F(OnDemandOsTest, DuplicateTxTest) { * @then both of them are used for the next proposal */ TEST_F(OnDemandOsTest, RejectCommit) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)) - .Times(testing::AtLeast(1)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) @@ -464,13 +450,13 @@ TEST_F(OnDemandOsTest, RejectCommit) { os->onBatches(txs1, requester_peer); os->onCollaborationOutcome( {initial_round.block_round, initial_round.reject_round + 1}, - started_list); + initial_list); auto txs2 = generateTransactions({1, 2}, now + 1); os->onBatches(txs2, requester_peer); os->onCollaborationOutcome( {initial_round.block_round, initial_round.reject_round + 2}, - started_list); + initial_list); auto proposal = os->onRequestProposal( {initial_round.block_round, initial_round.reject_round + 3}, requester_peer); @@ -487,16 +473,14 @@ TEST_F(OnDemandOsTest, RejectCommit) { * @then check that prposal isn't created */ TEST_F(OnDemandOsTest, FailOnCreationStrategy) { - EXPECT_CALL(*proposal_creation_strategy, onCollaborationOutcome(_)); EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(false)); + .WillRepeatedly(testing::Return(false)); EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); + .WillRepeatedly(testing::Return(boost::none)); generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome(commit_round, started_list); + os->onCollaborationOutcome(commit_round, initial_list); ASSERT_FALSE(os->onRequestProposal(target_round, requester_peer)); } - From d33bd4f69c2ca79aea0e310b8d255b5110c8f054 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 15:33:56 +0300 Subject: [PATCH 13/22] Remove peers from OS interface Signed-off-by: Andrei Lebedev --- .../ordering/impl/on_demand_ordering_gate.cpp | 12 +------ .../impl/on_demand_ordering_service_impl.cpp | 2 +- .../impl/on_demand_ordering_service_impl.hpp | 3 +- .../ordering/on_demand_ordering_service.hpp | 9 +---- .../ordering/on_demand_ordering_gate_test.cpp | 35 +++---------------- .../irohad/ordering/on_demand_os_test.cpp | 31 ++++++++-------- .../module/irohad/ordering/ordering_mocks.hpp | 3 +- 7 files changed, 24 insertions(+), 71 deletions(-) diff --git a/irohad/ordering/impl/on_demand_ordering_gate.cpp b/irohad/ordering/impl/on_demand_ordering_gate.cpp index 498bf5506f..f3c7079a50 100644 --- a/irohad/ordering/impl/on_demand_ordering_gate.cpp +++ b/irohad/ordering/impl/on_demand_ordering_gate.cpp @@ -62,22 +62,12 @@ OnDemandOrderingGate::OnDemandOrderingGate( return event.ledger_state; }); - auto peer_keys = [](const auto peers) { - OnDemandOrderingService::PeerList lst; - for (auto peer : peers) { - auto shp_pub_key = - std::make_shared(peer->pubkey()); - lst.push_back(shp_pub_key); - } - return lst; - }; // notify our ordering service about new round proposal_creation_strategy_->onCollaborationOutcome( *ledger_state->ledger_peers | boost::adaptors::transformed( [](auto &peer) -> decltype(auto) { return peer->pubkey(); })); - ordering_service_->onCollaborationOutcome( - current_round, peer_keys(*ledger_state->ledger_peers)); + ordering_service_->onCollaborationOutcome(current_round); this->sendCachedTransactions(event); diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index bcedf95e91..2a957e350d 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -46,7 +46,7 @@ OnDemandOrderingServiceImpl::OnDemandOrderingServiceImpl( // -------------------------| OnDemandOrderingService |------------------------- void OnDemandOrderingServiceImpl::onCollaborationOutcome( - consensus::Round round, const PeerList &peers) { + consensus::Round round) { log_->info("onCollaborationOutcome => {}", round); packNextProposals(round); diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp index 861b0cf050..5adfd1d55c 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp @@ -62,8 +62,7 @@ namespace iroha { // --------------------- | OnDemandOrderingService |_--------------------- - void onCollaborationOutcome(consensus::Round round, - const PeerList &peers) override; + void onCollaborationOutcome(consensus::Round round) override; // ----------------------- | OdOsNotification | -------------------------- diff --git a/irohad/ordering/on_demand_ordering_service.hpp b/irohad/ordering/on_demand_ordering_service.hpp index 659f579ca1..59e29e30fc 100644 --- a/irohad/ordering/on_demand_ordering_service.hpp +++ b/irohad/ordering/on_demand_ordering_service.hpp @@ -8,9 +8,6 @@ #include "ordering/on_demand_os_transport.hpp" -#include -#include "interfaces/common_objects/peer.hpp" - namespace iroha { namespace ordering { @@ -19,15 +16,11 @@ namespace iroha { */ class OnDemandOrderingService : public transport::OdNotificationOsSide { public: - /// collection of peers type - using PeerList = std::vector; /** * Method which should be invoked on outcome of collaboration for round * @param round - proposal round which has started - * @param peers - list of peers in new round */ - virtual void onCollaborationOutcome(consensus::Round round, - const PeerList &peers) = 0; + virtual void onCollaborationOutcome(consensus::Round round) = 0; }; } // namespace ordering diff --git a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp index d6d3247014..283d710c62 100644 --- a/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp +++ b/test/module/irohad/ordering/on_demand_ordering_gate_test.cpp @@ -117,12 +117,7 @@ TEST_F(OnDemandOrderingGateTest, BlockEvent) { ON_CALL(*proposal, transactions()) .WillByDefault(Return(txs | boost::adaptors::indirected)); - EXPECT_CALL( - *ordering_service, - onCollaborationOutcome( - round, - testing::Matcher(_))) - .Times(1); + EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(oproposal)))); @@ -155,12 +150,7 @@ TEST_F(OnDemandOrderingGateTest, EmptyEvent) { ON_CALL(*proposal, transactions()) .WillByDefault(Return(txs | boost::adaptors::indirected)); - EXPECT_CALL( - *ordering_service, - onCollaborationOutcome( - round, - testing::Matcher(_))) - .Times(1); + EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(oproposal)))); @@ -185,12 +175,7 @@ TEST_F(OnDemandOrderingGateTest, BlockEventNoProposal) { boost::optional> proposal; - EXPECT_CALL( - *ordering_service, - onCollaborationOutcome( - round, - testing::Matcher(_))) - .Times(1); + EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(proposal)))); @@ -214,12 +199,7 @@ TEST_F(OnDemandOrderingGateTest, EmptyEventNoProposal) { boost::optional> proposal; - EXPECT_CALL( - *ordering_service, - onCollaborationOutcome( - round, - testing::Matcher(_))) - .Times(1); + EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(proposal)))); @@ -257,12 +237,7 @@ TEST_F(OnDemandOrderingGateTest, ReplayedTransactionInProposal) { std::move(proposal))); // set expectations for ordering service - EXPECT_CALL( - *ordering_service, - onCollaborationOutcome( - round, - testing::Matcher(_))) - .Times(1); + EXPECT_CALL(*ordering_service, onCollaborationOutcome(round)).Times(1); EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(arriving_proposal)))); EXPECT_CALL(*tx_cache, diff --git a/test/module/irohad/ordering/on_demand_os_test.cpp b/test/module/irohad/ordering/on_demand_os_test.cpp index c99bde7624..62b3dcec20 100644 --- a/test/module/irohad/ordering/on_demand_os_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_test.cpp @@ -49,7 +49,6 @@ class OnDemandOsTest : public ::testing::Test { reject_round = {2, kNextRejectRoundConsumer}; NiceMock *mock_cache; std::shared_ptr proposal_creation_strategy; - std::vector> initial_list; std::shared_ptr requester_peer; void SetUp() override { @@ -136,7 +135,7 @@ TEST_F(OnDemandOsTest, EmptyRound) { ASSERT_FALSE(os->onRequestProposal(initial_round, requester_peer)); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); ASSERT_FALSE(os->onRequestProposal(initial_round, requester_peer)); } @@ -155,7 +154,7 @@ TEST_F(OnDemandOsTest, NormalRound) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); } @@ -175,7 +174,7 @@ TEST_F(OnDemandOsTest, OverflowRound) { generateTransactionsAndInsert({1, transaction_limit * 2}); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); ASSERT_EQ(transaction_limit, @@ -220,7 +219,7 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { std::thread two(call, std::make_pair(large_tx_limit / 2, large_tx_limit)); one.join(); two.join(); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); ASSERT_EQ(large_tx_limit, os->onRequestProposal(target_round, requester_peer) .get() @@ -244,7 +243,7 @@ TEST_F(OnDemandOsTest, Erase) { generateTransactionsAndInsert({1, 2}); os->onCollaborationOutcome( - {commit_round.block_round, commit_round.reject_round}, initial_list); + {commit_round.block_round, commit_round.reject_round}); ASSERT_TRUE(os->onRequestProposal( {commit_round.block_round + 1, commit_round.reject_round}, requester_peer)); @@ -253,7 +252,7 @@ TEST_F(OnDemandOsTest, Erase) { i < (commit_round.reject_round + 1) + (proposal_limit + 2); ++i) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome({commit_round.block_round, i}, initial_list); + os->onCollaborationOutcome({commit_round.block_round, i}); } ASSERT_TRUE(os->onRequestProposal( {commit_round.block_round + 1, commit_round.reject_round}, @@ -304,7 +303,7 @@ TEST_F(OnDemandOsTest, UseFactoryForProposal) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); } @@ -335,7 +334,7 @@ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); auto proposal = os->onRequestProposal(initial_round, requester_peer); @@ -362,7 +361,7 @@ TEST_F(OnDemandOsTest, PassMissingTransaction) { os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); auto proposal = os->onRequestProposal(target_round, requester_peer); @@ -399,7 +398,7 @@ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { os->onBatches(batches, requester_peer); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); auto proposal = os->onRequestProposal(target_round, requester_peer); const auto &txs = proposal->get()->transactions(); @@ -428,7 +427,7 @@ TEST_F(OnDemandOsTest, DuplicateTxTest) { auto txs2 = generateTransactions({1, 2}, now); os->onBatches(txs2, requester_peer); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); auto proposal = os->onRequestProposal(target_round, requester_peer); ASSERT_EQ(1, boost::size((*proposal)->transactions())); @@ -449,14 +448,12 @@ TEST_F(OnDemandOsTest, RejectCommit) { auto txs1 = generateTransactions({1, 2}, now); os->onBatches(txs1, requester_peer); os->onCollaborationOutcome( - {initial_round.block_round, initial_round.reject_round + 1}, - initial_list); + {initial_round.block_round, initial_round.reject_round + 1}); auto txs2 = generateTransactions({1, 2}, now + 1); os->onBatches(txs2, requester_peer); os->onCollaborationOutcome( - {initial_round.block_round, initial_round.reject_round + 2}, - initial_list); + {initial_round.block_round, initial_round.reject_round + 2}); auto proposal = os->onRequestProposal( {initial_round.block_round, initial_round.reject_round + 3}, requester_peer); @@ -480,7 +477,7 @@ TEST_F(OnDemandOsTest, FailOnCreationStrategy) { generateTransactionsAndInsert({1, 2}); - os->onCollaborationOutcome(commit_round, initial_list); + os->onCollaborationOutcome(commit_round); ASSERT_FALSE(os->onRequestProposal(target_round, requester_peer)); } diff --git a/test/module/irohad/ordering/ordering_mocks.hpp b/test/module/irohad/ordering/ordering_mocks.hpp index ac7f3ded6d..0cb39658cc 100644 --- a/test/module/irohad/ordering/ordering_mocks.hpp +++ b/test/module/irohad/ordering/ordering_mocks.hpp @@ -42,8 +42,7 @@ namespace iroha { boost::optional>( consensus::Round, InitiatorPeerType from)); - MOCK_METHOD2(onCollaborationOutcome, - void(consensus::Round, const PeerList &peers)); + MOCK_METHOD1(onCollaborationOutcome, void(consensus::Round)); }; } // namespace ordering From 3b31d3f4414c1a8677a6192881637f714a76bda9 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 16:00:52 +0300 Subject: [PATCH 14/22] Remove OsSide interface Signed-off-by: Andrei Lebedev --- .../impl/on_demand_ordering_service_impl.cpp | 6 +- .../impl/on_demand_ordering_service_impl.hpp | 5 +- .../impl/on_demand_os_server_grpc.cpp | 19 +++--- .../impl/on_demand_os_server_grpc.hpp | 4 +- .../ordering/on_demand_ordering_service.hpp | 2 +- irohad/ordering/on_demand_os_transport.hpp | 56 ++--------------- ...mock_on_demand_os_notification_os_side.hpp | 29 --------- .../on_demand_os_server_grpc_test.cpp | 13 ++-- .../irohad/ordering/on_demand_os_test.cpp | 61 ++++++++----------- .../module/irohad/ordering/ordering_mocks.hpp | 6 +- 10 files changed, 54 insertions(+), 147 deletions(-) delete mode 100644 test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index 2a957e350d..2565876152 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -55,8 +55,7 @@ void OnDemandOrderingServiceImpl::onCollaborationOutcome( // ----------------------------| OdOsNotification |----------------------------- -void OnDemandOrderingServiceImpl::onBatches(BatchesCollectionType batches, - InitiatorPeerType from) { +void OnDemandOrderingServiceImpl::onBatches(CollectionType batches) { auto unprocessed_batches = boost::adaptors::filter(batches, [this](const auto &batch) { log_->debug("check batch {} for already processed transactions", @@ -75,8 +74,7 @@ void OnDemandOrderingServiceImpl::onBatches(BatchesCollectionType batches, boost::optional< std::shared_ptr> -OnDemandOrderingServiceImpl::onRequestProposal(consensus::Round round, - InitiatorPeerType requester) { +OnDemandOrderingServiceImpl::onRequestProposal(consensus::Round round) { boost::optional< std::shared_ptr> result; diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp index 5adfd1d55c..25f0937220 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp @@ -66,11 +66,10 @@ namespace iroha { // ----------------------- | OdOsNotification | -------------------------- - void onBatches(BatchesCollectionType batches, - InitiatorPeerType from) override; + void onBatches(CollectionType batches) override; boost::optional> onRequestProposal( - consensus::Round round, InitiatorPeerType requester) override; + consensus::Round round) override; private: /** diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.cpp b/irohad/ordering/impl/on_demand_os_server_grpc.cpp index 1899c8fde2..fbf40ebf83 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.cpp @@ -20,7 +20,7 @@ using namespace iroha::ordering; using namespace iroha::ordering::transport; OnDemandOsServerGrpc::OnDemandOsServerGrpc( - std::shared_ptr ordering_service, + std::shared_ptr ordering_service, std::shared_ptr transaction_factory, std::shared_ptr batch_parser, @@ -91,9 +91,7 @@ grpc::Status OnDemandOsServerGrpc::SendBatches( }); fetchPeer(request->peer_key()) | [this, &batches](auto &&peer) { - ordering_service_->onBatches( - std::move(batches), - std::make_shared(peer)); + ordering_service_->onBatches(std::move(batches)); }; return ::grpc::Status::OK; @@ -108,14 +106,11 @@ grpc::Status OnDemandOsServerGrpc::RequestProposal( request->round().reject_round()}; // todo add force initialization of proposal proposal_creation_strategy_->onProposal(peer, round); - ordering_service_->onRequestProposal( - round, std::make_shared(peer)) - | - [&](auto &&proposal) { - *response->mutable_proposal() = std::move( - static_cast(proposal.get()) - ->getTransport()); - }; + ordering_service_->onRequestProposal(round) | [&](auto &&proposal) { + *response->mutable_proposal() = std::move( + static_cast(proposal.get()) + ->getTransport()); + }; }; return ::grpc::Status::OK; } diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.hpp b/irohad/ordering/impl/on_demand_os_server_grpc.hpp index 1be1526f3e..3fab014dc8 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.hpp @@ -32,7 +32,7 @@ namespace iroha { iroha::protocol::Transaction>; OnDemandOsServerGrpc( - std::shared_ptr ordering_service, + std::shared_ptr ordering_service, std::shared_ptr transaction_factory, std::shared_ptr batch_parser, @@ -63,7 +63,7 @@ namespace iroha { shared_model::interface::types::SharedTxsCollectionType deserializeTransactions(const proto::BatchesRequest *request); - std::shared_ptr ordering_service_; + std::shared_ptr ordering_service_; std::shared_ptr transaction_factory_; std::shared_ptr diff --git a/irohad/ordering/on_demand_ordering_service.hpp b/irohad/ordering/on_demand_ordering_service.hpp index 59e29e30fc..9c2a3601e2 100644 --- a/irohad/ordering/on_demand_ordering_service.hpp +++ b/irohad/ordering/on_demand_ordering_service.hpp @@ -14,7 +14,7 @@ namespace iroha { /** * Ordering Service aka OS which can share proposals by request */ - class OnDemandOrderingService : public transport::OdNotificationOsSide { + class OnDemandOrderingService : public transport::OdOsNotification { public: /** * Method which should be invoked on outcome of collaboration for round diff --git a/irohad/ordering/on_demand_os_transport.hpp b/irohad/ordering/on_demand_os_transport.hpp index e9604082fe..e235c923df 100644 --- a/irohad/ordering/on_demand_os_transport.hpp +++ b/irohad/ordering/on_demand_os_transport.hpp @@ -12,7 +12,6 @@ #include #include "consensus/round.hpp" -#include "interfaces/common_objects/types.hpp" namespace shared_model { namespace interface { @@ -20,22 +19,17 @@ namespace shared_model { class Proposal; class Peer; } // namespace interface - namespace crypto { - class PublicKey; - } // namespace crypto } // namespace shared_model namespace iroha { namespace ordering { namespace transport { - namespace details { - - /** - * Type of peer identity - */ - using PeerType = shared_model::crypto::PublicKey; - + /** + * Notification interface of on demand ordering service. + */ + class OdOsNotification { + public: /** * Type of stored proposals */ @@ -51,16 +45,6 @@ namespace iroha { * Type of inserted collections */ using CollectionType = std::vector; - } // namespace details - - /** - * Notification interface of on demand ordering service. - */ - class OdOsNotification { - public: - using ProposalType = details::ProposalType; - using TransactionBatchType = details::TransactionBatchType; - using CollectionType = details::CollectionType; /** * Callback on receiving transactions @@ -97,36 +81,6 @@ namespace iroha { virtual ~OdOsNotificationFactory() = default; }; - class OdNotificationOsSide { - public: - using InitiatorPeerType = std::shared_ptr; - using ProposalType = details::ProposalType; - using TransactionBatchType = details::TransactionBatchType; - using BatchesCollectionType = details::CollectionType; - using PeerStateType = std::vector; - - /** - * Callback on receiving transactions - * @param batches - vector of passed transaction batches - * @param from - peer which sends batches - */ - virtual void onBatches(BatchesCollectionType batches, - InitiatorPeerType from) = 0; - - /** - * Callback on request about proposal - * @param round - number of collaboration round. - * Calculated as block_height + 1 - * @param requester - peer which requests proposal - * @return proposal for requested round - */ - virtual boost::optional> - onRequestProposal(consensus::Round round, - InitiatorPeerType requester) = 0; - - virtual ~OdNotificationOsSide() = default; - }; - } // namespace transport } // namespace ordering } // namespace iroha diff --git a/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp b/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp deleted file mode 100644 index 4d8719f33a..0000000000 --- a/test/module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp +++ /dev/null @@ -1,29 +0,0 @@ -/** - * Copyright Soramitsu Co., Ltd. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -#ifndef IROHA_MOCK_ON_DEMAND_OS_NOTIFICATION_OS_SIDE_HPP -#define IROHA_MOCK_ON_DEMAND_OS_NOTIFICATION_OS_SIDE_HPP - -#include "ordering/on_demand_os_transport.hpp" - -#include - -namespace iroha { - namespace ordering { - namespace transport { - - struct MockOdOsNotificationOsSide : public OdNotificationOsSide { - MOCK_METHOD2(onBatches, void(BatchesCollectionType, InitiatorPeerType)); - - MOCK_METHOD2(onRequestProposal, - boost::optional>( - consensus::Round, InitiatorPeerType)); - }; - - } // namespace transport - } // namespace ordering -} // namespace iroha - -#endif // IROHA_MOCK_ON_DEMAND_OS_NOTIFICATION_OS_SIDE_HPP diff --git a/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp b/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp index c01dfb10f0..2be184e46a 100644 --- a/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_server_grpc_test.cpp @@ -12,7 +12,7 @@ #include "framework/test_logger.hpp" #include "interfaces/iroha_internal/transaction_batch_impl.hpp" #include "interfaces/iroha_internal/transaction_batch_parser_impl.hpp" -#include "module/irohad/ordering/mock_on_demand_os_notification_os_side.hpp" +#include "module/irohad/ordering/mock_on_demand_os_notification.hpp" #include "module/irohad/ordering/mock_proposal_creation_strategy.hpp" #include "module/shared_model/interface/mock_transaction_batch_factory.hpp" #include "module/shared_model/validators/validators.hpp" @@ -29,7 +29,7 @@ using ::testing::Return; struct OnDemandOsServerGrpcTest : public ::testing::Test { void SetUp() override { - notification = std::make_shared(); + notification = std::make_shared(); std::unique_ptr> interface_transaction_validator = @@ -64,7 +64,7 @@ struct OnDemandOsServerGrpcTest : public ::testing::Test { getTestLogger("OdOsServerGrpc")); } - std::shared_ptr notification; + std::shared_ptr notification; std::shared_ptr batch_factory; std::shared_ptr proposal_creation_strategy; std::shared_ptr server; @@ -103,8 +103,7 @@ TEST_F(OnDemandOsServerGrpcTest, SendBatches) { shared_model::interface::TransactionBatchImpl>( cand)); })); - EXPECT_CALL(*notification, onBatches(_, _)) - .WillOnce(SaveArg0Move(&collection)); + EXPECT_CALL(*notification, onBatches(_)).WillOnce(SaveArg0Move(&collection)); proto::BatchesRequest request; *request.mutable_peer_key() = std::string(32, 'f'); request.add_transactions() @@ -139,7 +138,7 @@ TEST_F(OnDemandOsServerGrpcTest, RequestProposal) { std::shared_ptr iproposal( std::make_shared(proposal)); - EXPECT_CALL(*notification, onRequestProposal(round, _)) + EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(iproposal)))); server->RequestProposal(nullptr, &request, &response); @@ -166,7 +165,7 @@ TEST_F(OnDemandOsServerGrpcTest, RequestProposalNone) { request.mutable_round()->set_block_round(round.block_round); request.mutable_round()->set_reject_round(round.reject_round); proto::ProposalResponse response; - EXPECT_CALL(*notification, onRequestProposal(round, _)) + EXPECT_CALL(*notification, onRequestProposal(round)) .WillOnce(Return(ByMove(std::move(boost::none)))); server->RequestProposal(nullptr, &request, &response); diff --git a/test/module/irohad/ordering/on_demand_os_test.cpp b/test/module/irohad/ordering/on_demand_os_test.cpp index 62b3dcec20..ea24356e33 100644 --- a/test/module/irohad/ordering/on_demand_os_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_test.cpp @@ -49,7 +49,6 @@ class OnDemandOsTest : public ::testing::Test { reject_round = {2, kNextRejectRoundConsumer}; NiceMock *mock_cache; std::shared_ptr proposal_creation_strategy; - std::shared_ptr requester_peer; void SetUp() override { // TODO: nickaleks IR-1811 use mock factory @@ -85,13 +84,13 @@ class OnDemandOsTest : public ::testing::Test { * @param range - pair of [from, to) */ void generateTransactionsAndInsert(std::pair range) { - os->onBatches(generateTransactions(range), requester_peer); + os->onBatches(generateTransactions(range)); } - OnDemandOrderingService::BatchesCollectionType generateTransactions( + OnDemandOrderingService::CollectionType generateTransactions( std::pair range, shared_model::interface::types::TimestampType now = iroha::time::now()) { - OnDemandOrderingService::BatchesCollectionType collection; + OnDemandOrderingService::CollectionType collection; for (auto i = range.first; i < range.second; ++i) { collection.push_back( @@ -133,11 +132,11 @@ TEST_F(OnDemandOsTest, EmptyRound) { EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) .WillRepeatedly(testing::Return(boost::none)); - ASSERT_FALSE(os->onRequestProposal(initial_round, requester_peer)); + ASSERT_FALSE(os->onRequestProposal(initial_round)); os->onCollaborationOutcome(commit_round); - ASSERT_FALSE(os->onRequestProposal(initial_round, requester_peer)); + ASSERT_FALSE(os->onRequestProposal(initial_round)); } /** @@ -156,7 +155,7 @@ TEST_F(OnDemandOsTest, NormalRound) { os->onCollaborationOutcome(commit_round); - ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); + ASSERT_TRUE(os->onRequestProposal(target_round)); } /** @@ -176,11 +175,9 @@ TEST_F(OnDemandOsTest, OverflowRound) { os->onCollaborationOutcome(commit_round); - ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); + ASSERT_TRUE(os->onRequestProposal(target_round)); ASSERT_EQ(transaction_limit, - (*os->onRequestProposal(target_round, requester_peer)) - ->transactions() - .size()); + (*os->onRequestProposal(target_round))->transactions().size()); } /** @@ -221,10 +218,7 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { two.join(); os->onCollaborationOutcome(commit_round); ASSERT_EQ(large_tx_limit, - os->onRequestProposal(target_round, requester_peer) - .get() - ->transactions() - .size()); + os->onRequestProposal(target_round).get()->transactions().size()); } /** @@ -245,8 +239,7 @@ TEST_F(OnDemandOsTest, Erase) { os->onCollaborationOutcome( {commit_round.block_round, commit_round.reject_round}); ASSERT_TRUE(os->onRequestProposal( - {commit_round.block_round + 1, commit_round.reject_round}, - requester_peer)); + {commit_round.block_round + 1, commit_round.reject_round})); for (auto i = commit_round.reject_round + 1; i < (commit_round.reject_round + 1) + (proposal_limit + 2); @@ -255,8 +248,7 @@ TEST_F(OnDemandOsTest, Erase) { os->onCollaborationOutcome({commit_round.block_round, i}); } ASSERT_TRUE(os->onRequestProposal( - {commit_round.block_round + 1, commit_round.reject_round}, - requester_peer)); + {commit_round.block_round + 1, commit_round.reject_round})); } /** @@ -305,7 +297,7 @@ TEST_F(OnDemandOsTest, UseFactoryForProposal) { os->onCollaborationOutcome(commit_round); - ASSERT_TRUE(os->onRequestProposal(target_round, requester_peer)); + ASSERT_TRUE(os->onRequestProposal(target_round)); } // Return matcher for batch, which passes it by const & @@ -332,11 +324,11 @@ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { .WillOnce(Return(std::vector{ iroha::ametsuchi::tx_cache_status_responses::Committed()})); - os->onBatches(batches, requester_peer); + os->onBatches(batches); os->onCollaborationOutcome(commit_round); - auto proposal = os->onRequestProposal(initial_round, requester_peer); + auto proposal = os->onRequestProposal(initial_round); EXPECT_FALSE(proposal); } @@ -359,11 +351,11 @@ TEST_F(OnDemandOsTest, PassMissingTransaction) { .WillOnce(Return(std::vector{ iroha::ametsuchi::tx_cache_status_responses::Missing()})); - os->onBatches(batches, requester_peer); + os->onBatches(batches); os->onCollaborationOutcome(commit_round); - auto proposal = os->onRequestProposal(target_round, requester_peer); + auto proposal = os->onRequestProposal(target_round); // since we only sent one transaction, // if the proposal is present, there is no need to check for that specific tx @@ -396,11 +388,11 @@ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { .WillOnce(Return(std::vector{ iroha::ametsuchi::tx_cache_status_responses::Missing()})); - os->onBatches(batches, requester_peer); + os->onBatches(batches); os->onCollaborationOutcome(commit_round); - auto proposal = os->onRequestProposal(target_round, requester_peer); + auto proposal = os->onRequestProposal(target_round); const auto &txs = proposal->get()->transactions(); auto &batch2_tx = *batch2.transactions().at(0); @@ -423,12 +415,12 @@ TEST_F(OnDemandOsTest, DuplicateTxTest) { auto now = iroha::time::now(); auto txs1 = generateTransactions({1, 2}, now); - os->onBatches(txs1, requester_peer); + os->onBatches(txs1); auto txs2 = generateTransactions({1, 2}, now); - os->onBatches(txs2, requester_peer); + os->onBatches(txs2); os->onCollaborationOutcome(commit_round); - auto proposal = os->onRequestProposal(target_round, requester_peer); + auto proposal = os->onRequestProposal(target_round); ASSERT_EQ(1, boost::size((*proposal)->transactions())); } @@ -446,21 +438,20 @@ TEST_F(OnDemandOsTest, RejectCommit) { auto now = iroha::time::now(); auto txs1 = generateTransactions({1, 2}, now); - os->onBatches(txs1, requester_peer); + os->onBatches(txs1); os->onCollaborationOutcome( {initial_round.block_round, initial_round.reject_round + 1}); auto txs2 = generateTransactions({1, 2}, now + 1); - os->onBatches(txs2, requester_peer); + os->onBatches(txs2); os->onCollaborationOutcome( {initial_round.block_round, initial_round.reject_round + 2}); auto proposal = os->onRequestProposal( - {initial_round.block_round, initial_round.reject_round + 3}, - requester_peer); + {initial_round.block_round, initial_round.reject_round + 3}); ASSERT_EQ(2, boost::size((*proposal)->transactions())); - proposal = os->onRequestProposal(commit_round, requester_peer); + proposal = os->onRequestProposal(commit_round); ASSERT_EQ(2, boost::size((*proposal)->transactions())); } @@ -479,5 +470,5 @@ TEST_F(OnDemandOsTest, FailOnCreationStrategy) { os->onCollaborationOutcome(commit_round); - ASSERT_FALSE(os->onRequestProposal(target_round, requester_peer)); + ASSERT_FALSE(os->onRequestProposal(target_round)); } diff --git a/test/module/irohad/ordering/ordering_mocks.hpp b/test/module/irohad/ordering/ordering_mocks.hpp index 0cb39658cc..c867a72768 100644 --- a/test/module/irohad/ordering/ordering_mocks.hpp +++ b/test/module/irohad/ordering/ordering_mocks.hpp @@ -36,11 +36,11 @@ namespace iroha { } // namespace cache struct MockOnDemandOrderingService : public OnDemandOrderingService { - MOCK_METHOD2(onBatches, void(BatchesCollectionType, InitiatorPeerType)); + MOCK_METHOD1(onBatches, void(CollectionType)); - MOCK_METHOD2(onRequestProposal, + MOCK_METHOD1(onRequestProposal, boost::optional>( - consensus::Round, InitiatorPeerType from)); + consensus::Round)); MOCK_METHOD1(onCollaborationOutcome, void(consensus::Round)); }; From 842980536754c4116a2efd9eb6e43723d61f31c9 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 16:37:03 +0300 Subject: [PATCH 15/22] Rename public key fields Signed-off-by: Andrei Lebedev --- irohad/main/application.cpp | 4 +-- irohad/main/impl/on_demand_ordering_init.cpp | 12 ++++---- irohad/main/impl/on_demand_ordering_init.hpp | 8 +++--- .../impl/on_demand_os_client_grpc.cpp | 16 +++++------ .../impl/on_demand_os_client_grpc.hpp | 12 +++----- .../on_demand_os_client_grpc_test.cpp | 28 +++++++++---------- 6 files changed, 35 insertions(+), 45 deletions(-) diff --git a/irohad/main/application.cpp b/irohad/main/application.cpp index 31558660ef..ac8e3a726f 100644 --- a/irohad/main/application.cpp +++ b/irohad/main/application.cpp @@ -381,8 +381,6 @@ void Irohad::initOrderingGate() { auto field_validator = std::make_shared(); - auto self_peer_key = - std::make_shared(keypair.publicKey()); ordering_gate = ordering_init.initOrderingGate(max_proposal_size_, @@ -400,7 +398,7 @@ void Irohad::initOrderingGate() { log_manager_->getChild("Ordering"), field_validator, proposal_strategy, - self_peer_key); + keypair.publicKey()); log_->info("[Init] => init ordering gate - [{}]", logger::logBool(ordering_gate)); } diff --git a/irohad/main/impl/on_demand_ordering_init.cpp b/irohad/main/impl/on_demand_ordering_init.cpp index 906b606fa8..4e445a78f9 100644 --- a/irohad/main/impl/on_demand_ordering_init.cpp +++ b/irohad/main/impl/on_demand_ordering_init.cpp @@ -52,14 +52,14 @@ namespace iroha { async_call, std::shared_ptr proposal_transport_factory, std::chrono::milliseconds delay, - std::shared_ptr self_peer, + shared_model::crypto::PublicKey my_key, const logger::LoggerManagerTreePtr &ordering_log_manager) { return std::make_shared( std::move(async_call), std::move(proposal_transport_factory), [] { return std::chrono::system_clock::now(); }, delay, - self_peer, + my_key, ordering_log_manager->getChild("NetworkClient")->getLogger()); } @@ -70,7 +70,7 @@ namespace iroha { std::shared_ptr proposal_transport_factory, std::chrono::milliseconds delay, std::vector initial_hashes, - std::shared_ptr self_peer_key, + shared_model::crypto::PublicKey my_key, const logger::LoggerManagerTreePtr &ordering_log_manager) { // since top block will be the first in notifier observable, hashes of // two previous blocks are prepended @@ -198,7 +198,7 @@ namespace iroha { createNotificationFactory(std::move(async_call), std::move(proposal_transport_factory), delay, - self_peer_key, + my_key, ordering_log_manager), peers, ordering_log_manager->getChild("ConnectionManager")->getLogger()); @@ -310,7 +310,7 @@ namespace iroha { std::shared_ptr field_validator, std::shared_ptr creation_strategy, - std::shared_ptr self_peer_key) { + shared_model::crypto::PublicKey my_key) { auto ordering_service = createService(max_number_of_transactions, proposal_factory, tx_cache, @@ -331,7 +331,7 @@ namespace iroha { std::move(proposal_transport_factory), delay, std::move(initial_hashes), - self_peer_key, + my_key, ordering_log_manager), std::make_shared(), std::move(proposal_factory), diff --git a/irohad/main/impl/on_demand_ordering_init.hpp b/irohad/main/impl/on_demand_ordering_init.hpp index c2fbbee7f2..8aafdbe673 100644 --- a/irohad/main/impl/on_demand_ordering_init.hpp +++ b/irohad/main/impl/on_demand_ordering_init.hpp @@ -45,7 +45,7 @@ namespace iroha { async_call, std::shared_ptr proposal_transport_factory, std::chrono::milliseconds delay, - std::shared_ptr self_peer, + shared_model::crypto::PublicKey my_key, const logger::LoggerManagerTreePtr &ordering_log_manager); /** @@ -60,7 +60,7 @@ namespace iroha { std::shared_ptr proposal_transport_factory, std::chrono::milliseconds delay, std::vector initial_hashes, - std::shared_ptr self_peer, + shared_model::crypto::PublicKey my_key, const logger::LoggerManagerTreePtr &ordering_log_manager); /** @@ -122,7 +122,7 @@ namespace iroha { * @param field_validator - provides a dependency for validating peer keys * @param creation_strategy - provides a strategy for creaing proposals in * OS - * @param self_peer_key - the public key of instantiated peer + * @param my_key - public key of instantiated peer * @return initialized ordering gate */ std::shared_ptr initOrderingGate( @@ -149,7 +149,7 @@ namespace iroha { std::shared_ptr field_validator, std::shared_ptr creation_strategy, - std::shared_ptr self_peer_key); + shared_model::crypto::PublicKey my_key); /// gRPC service for ordering service std::shared_ptr service; diff --git a/irohad/ordering/impl/on_demand_os_client_grpc.cpp b/irohad/ordering/impl/on_demand_os_client_grpc.cpp index 8ccde3b577..413f28ea7d 100644 --- a/irohad/ordering/impl/on_demand_os_client_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_client_grpc.cpp @@ -24,7 +24,7 @@ OnDemandOsClientGrpc::OnDemandOsClientGrpc( std::shared_ptr proposal_factory, std::function time_provider, std::chrono::milliseconds proposal_request_timeout, - std::shared_ptr self_peer_key, + shared_model::interface::types::PubkeyType my_key, logger::LoggerPtr log) : log_(std::move(log)), stub_(std::move(stub)), @@ -32,7 +32,7 @@ OnDemandOsClientGrpc::OnDemandOsClientGrpc( proposal_factory_(std::move(proposal_factory)), time_provider_(std::move(time_provider)), proposal_request_timeout_(proposal_request_timeout), - self_peer_key_(self_peer_key) {} + my_key_(std::move(my_key)) {} void OnDemandOsClientGrpc::onBatches(CollectionType batches) { proto::BatchesRequest request; @@ -44,8 +44,7 @@ void OnDemandOsClientGrpc::onBatches(CollectionType batches) { } } - *request.mutable_peer_key() = - shared_model::crypto::toBinaryString(*self_peer_key_); + *request.mutable_peer_key() = shared_model::crypto::toBinaryString(my_key_); log_->debug("Propagating: '{}'", request.DebugString()); @@ -63,8 +62,7 @@ OnDemandOsClientGrpc::onRequestProposal(consensus::Round round) { request.mutable_round()->set_block_round(round.block_round); request.mutable_round()->set_reject_round(round.reject_round); - *request.mutable_peer_key() = - shared_model::crypto::toBinaryString(*self_peer_key_); + *request.mutable_peer_key() = shared_model::crypto::toBinaryString(my_key_); proto::ProposalResponse response; auto status = stub_->RequestProposal(&context, request, &response); @@ -96,13 +94,13 @@ OnDemandOsClientGrpcFactory::OnDemandOsClientGrpcFactory( std::shared_ptr proposal_factory, std::function time_provider, OnDemandOsClientGrpc::TimeoutType proposal_request_timeout, - std::shared_ptr self_key_peer, + shared_model::interface::types::PubkeyType my_key, logger::LoggerPtr client_log) : async_call_(std::move(async_call)), proposal_factory_(std::move(proposal_factory)), time_provider_(time_provider), proposal_request_timeout_(proposal_request_timeout), - self_key_peer_(self_key_peer), + my_key_(std::move(my_key)), client_log_(std::move(client_log)) {} std::unique_ptr OnDemandOsClientGrpcFactory::create( @@ -113,6 +111,6 @@ std::unique_ptr OnDemandOsClientGrpcFactory::create( proposal_factory_, time_provider_, proposal_request_timeout_, - self_key_peer_, + my_key_, client_log_); } diff --git a/irohad/ordering/impl/on_demand_os_client_grpc.hpp b/irohad/ordering/impl/on_demand_os_client_grpc.hpp index 6a0091bc08..053078a3c8 100644 --- a/irohad/ordering/impl/on_demand_os_client_grpc.hpp +++ b/irohad/ordering/impl/on_demand_os_client_grpc.hpp @@ -42,8 +42,7 @@ namespace iroha { std::shared_ptr proposal_factory, std::function time_provider, std::chrono::milliseconds proposal_request_timeout, - std::shared_ptr - self_peer_key_, + shared_model::interface::types::PubkeyType my_key, logger::LoggerPtr log); void onBatches(CollectionType batches) override; @@ -59,8 +58,7 @@ namespace iroha { std::shared_ptr proposal_factory_; std::function time_provider_; std::chrono::milliseconds proposal_request_timeout_; - std::shared_ptr - self_peer_key_; + shared_model::interface::types::PubkeyType my_key_; }; class OnDemandOsClientGrpcFactory : public OdOsNotificationFactory { @@ -72,8 +70,7 @@ namespace iroha { std::shared_ptr proposal_factory, std::function time_provider, OnDemandOsClientGrpc::TimeoutType proposal_request_timeout, - std::shared_ptr - self_key_peer, + shared_model::interface::types::PubkeyType my_key, logger::LoggerPtr client_log); /** @@ -91,8 +88,7 @@ namespace iroha { std::shared_ptr proposal_factory_; std::function time_provider_; std::chrono::milliseconds proposal_request_timeout_; - std::shared_ptr - self_key_peer_; + shared_model::interface::types::PubkeyType my_key_; logger::LoggerPtr client_log_; }; diff --git a/test/module/irohad/ordering/on_demand_os_client_grpc_test.cpp b/test/module/irohad/ordering/on_demand_os_client_grpc_test.cpp index 244fbb42a4..46e0593bb3 100644 --- a/test/module/irohad/ordering/on_demand_os_client_grpc_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_client_grpc_test.cpp @@ -20,12 +20,12 @@ using namespace iroha; using namespace iroha::ordering; using namespace iroha::ordering::transport; +using grpc::testing::MockClientAsyncResponseReader; using ::testing::_; using ::testing::DoAll; using ::testing::Return; using ::testing::SaveArg; using ::testing::SetArgPointee; -using grpc::testing::MockClientAsyncResponseReader; class OnDemandOsClientGrpcTest : public ::testing::Test { public: @@ -54,17 +54,15 @@ class OnDemandOsClientGrpcTest : public ::testing::Test { proto_proposal_validator = proto_validator.get(); proposal_factory = std::make_shared( std::move(validator), std::move(proto_validator)); - self_key = "self"; - auto self_pub_key = - std::make_shared(self_key); - client = - std::make_shared(std::move(ustub), - async_call, - proposal_factory, - [&] { return timepoint; }, - timeout, - self_pub_key, - getTestLogger("OdOsClientGrpc")); + my_key = "self"; + client = std::make_shared( + std::move(ustub), + async_call, + proposal_factory, + [&] { return timepoint; }, + timeout, + shared_model::crypto::PublicKey{my_key}, + getTestLogger("OdOsClientGrpc")); } proto::MockOnDemandOrderingStub *stub; @@ -73,7 +71,7 @@ class OnDemandOsClientGrpcTest : public ::testing::Test { std::chrono::milliseconds timeout{1}; std::shared_ptr client; consensus::Round round{1, 2}; - std::string self_key; + std::string my_key; MockProposalValidator *proposal_validator; MockProtoProposalValidator *proto_proposal_validator; @@ -109,7 +107,7 @@ TEST_F(OnDemandOsClientGrpcTest, onBatches) { .reduced_payload() .creator_account_id(), creator); - ASSERT_EQ(self_key, request.peer_key()); + ASSERT_EQ(my_key, request.peer_key()); } /** @@ -149,7 +147,7 @@ TEST_F(OnDemandOsClientGrpcTest, onRequestProposal) { ASSERT_EQ(request.round().reject_round(), round.reject_round); ASSERT_TRUE(proposal); ASSERT_EQ(proposal.value()->transactions()[0].creatorAccountId(), creator); - ASSERT_EQ(self_key, request.peer_key()); + ASSERT_EQ(my_key, request.peer_key()); } /** From 981f1faee5bfb379f48f6b7cc3ab3566f9b63990 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 16:57:58 +0300 Subject: [PATCH 16/22] Make Answer usage consistent with shared model Signed-off-by: Andrei Lebedev --- irohad/ordering/impl/on_demand_os_server_grpc.cpp | 7 +++---- shared_model/validators/answer.hpp | 7 ++----- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.cpp b/irohad/ordering/impl/on_demand_os_server_grpc.cpp index fbf40ebf83..b57f8aaa1e 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.cpp @@ -120,12 +120,11 @@ OnDemandOsServerGrpc::fetchPeer(const std::string &pub_key) const { shared_model::crypto::PublicKey key{pub_key}; shared_model::validation::ReasonsGroupType reason; field_validator_->validatePubkey(reason, key); - shared_model::validation::Answer answer; - answer.addReason(std::move(reason)); - if (answer.hasErrors()) { + if (not reason.second.empty()) { + shared_model::validation::Answer answer; + answer.addReason(std::move(reason)); log_->error("Failed to parse peer key struct, {}", answer.reason()); return boost::none; } - return key; } diff --git a/shared_model/validators/answer.hpp b/shared_model/validators/answer.hpp index 1eae513c4f..55f4e0227d 100644 --- a/shared_model/validators/answer.hpp +++ b/shared_model/validators/answer.hpp @@ -59,13 +59,10 @@ namespace shared_model { /** * Adds error to map - * @param reasons - errors for adding + * @param reasons */ void addReason(ReasonsGroupType &&reasons) { - // checks that reasons aren't empty - if(reasons.second.size() != 0) { - reasons_map_.insert(std::move(reasons)); - } + reasons_map_.insert(std::move(reasons)); } std::map getReasonsMap() { From 13141ce69cf845140c5c6c26bfa353f0b382349c Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 18:00:12 +0300 Subject: [PATCH 17/22] Simplify method calls in OS Signed-off-by: Andrei Lebedev --- .../impl/on_demand_ordering_service_impl.cpp | 82 +++++-------------- .../impl/on_demand_ordering_service_impl.hpp | 9 +- 2 files changed, 24 insertions(+), 67 deletions(-) diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index 2565876152..dbcf2e6fc8 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -80,6 +80,7 @@ OnDemandOrderingServiceImpl::onRequestProposal(consensus::Round round) { result; { std::shared_lock lock(proposals_mutex_); + // todo add force initialization of proposal auto it = proposal_map_.find(round); result = boost::make_optional(it != proposal_map_.end(), it->second); } @@ -130,62 +131,15 @@ getTransactions(size_t requested_tx_amount, void OnDemandOrderingServiceImpl::packNextProposals( const consensus::Round &round) { - /* - * The possible cases can be visualised as a diagram, where: - * o - current round, x - next round, v - target round - * - * 0 1 2 - * 0 o x v - * 1 x v . - * 2 v . . - * - * Reject case: - * - * 0 1 2 3 - * 0 . o x v - * 1 x v . . - * 2 v . . . - * - * (0,1) - current round. Round (0,2) is closed for transactions. - * Round (0,3) is now receiving transactions. - * Rounds (1,) and (2,) do not change. - * - * Commit case: - * - * 0 1 2 - * 0 . . . - * 1 o x v - * 2 x v . - * 3 v . . - * - * (1,0) - current round. The diagram is similar to the initial case. - */ - - size_t discarded_txs_quantity; - auto now = iroha::time::now(); - auto generate_proposal = [this, now, &discarded_txs_quantity]( - consensus::Round round, - const TransactionsCollectionType &txs) { - auto proposal = proposal_factory_->unsafeCreateProposal( - round.block_round, now, txs | boost::adaptors::indirected); - proposal_map_.erase(round); - proposal_map_.emplace(round, std::move(proposal)); - log_->debug( - "packNextProposal: data has been fetched for {}. " - "Number of transactions in proposal = {}. Discarded {} " - "transactions.", - round, - txs.size(), - discarded_txs_quantity); - }; - if (not pending_batches_.empty()) { + size_t discarded_txs_quantity; auto txs = getTransactions( transaction_limit_, pending_batches_, discarded_txs_quantity); - tryToCreateProposal( - {round.block_round, round.reject_round + 1}, txs, generate_proposal); - tryToCreateProposal( - {round.block_round + 1, kFirstRejectRound}, txs, generate_proposal); + log_->debug("Discarded {} transactions", discarded_txs_quantity); + auto now = iroha::time::now(); + // create proposals for the next commit and reject rounds + tryCreateProposal({round.block_round, round.reject_round + 1}, txs, now); + tryCreateProposal({round.block_round + 1, kFirstRejectRound}, txs, now); } if (round.reject_round == kFirstRejectRound) { @@ -194,22 +148,26 @@ void OnDemandOrderingServiceImpl::packNextProposals( } } -void OnDemandOrderingServiceImpl::tryToCreateProposal( +void OnDemandOrderingServiceImpl::tryCreateProposal( iroha::consensus::Round round, const TransactionsCollectionType &txs, - std::function - proposal_generator) { - std::string log_str = "Proposal in round {} will not created because "; - + shared_model::interface::types::TimestampType created_time) { if (not txs.empty()) { if (proposal_creation_strategy_->shouldCreateRound(round)) { - proposal_generator(round, txs); - log_->info("created proposal in round {}", round); + auto proposal = proposal_factory_->unsafeCreateProposal( + round.block_round, created_time, txs | boost::adaptors::indirected); + proposal_map_.erase(round); + proposal_map_.emplace(round, std::move(proposal)); + log_->debug( + "packNextProposal: data has been fetched for {}. " + "Number of transactions in proposal = {}.", + round, + txs.size()); } else { - log_->info(log_str + "round strategy annul creation", round); + log_->debug("Proposal for {} not created by the strategy", round); } } else { - log_->info(log_str + "transactions absent", round); + log_->debug("No transactions to create a proposal for {}", round); } } diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp index 25f0937220..c493c337ef 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp @@ -80,12 +80,11 @@ namespace iroha { using TransactionsCollectionType = std::vector>; - using RoundType = iroha::consensus::Round; - void tryToCreateProposal( - RoundType, - const TransactionsCollectionType &, - std::function); + void tryCreateProposal( + consensus::Round round, + const TransactionsCollectionType &txs, + shared_model::interface::types::TimestampType created_time); /** * Removes last elements if it is required From 6a1164d2163d77890eab205aa1a12c9eb1d5e194 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 18:10:23 +0300 Subject: [PATCH 18/22] Remove initial round from OS Signed-off-by: Andrei Lebedev --- irohad/ordering/impl/on_demand_ordering_service_impl.cpp | 4 +--- irohad/ordering/impl/on_demand_ordering_service_impl.hpp | 5 +---- test/module/irohad/ordering/on_demand_os_test.cpp | 9 +++------ 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp index dbcf2e6fc8..e62c40f756 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.cpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.cpp @@ -33,9 +33,7 @@ OnDemandOrderingServiceImpl::OnDemandOrderingServiceImpl( std::shared_ptr tx_cache, std::shared_ptr proposal_creation_strategy, logger::LoggerPtr log, - size_t number_of_proposals, - // todo remove initial round - const consensus::Round &initial_round) + size_t number_of_proposals) : transaction_limit_(transaction_limit), number_of_proposals_(number_of_proposals), proposal_factory_(std::move(proposal_factory)), diff --git a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp index c493c337ef..e3d6f7e968 100644 --- a/irohad/ordering/impl/on_demand_ordering_service_impl.hpp +++ b/irohad/ordering/impl/on_demand_ordering_service_impl.hpp @@ -47,8 +47,6 @@ namespace iroha { * @param log to print progress * @param number_of_proposals - number of stored proposals, older will be * removed. Default value is 3 - * @param initial_round - first round of agreement. - * Default value is {2, kFirstRejectRound} since genesis block height is 1 */ OnDemandOrderingServiceImpl( size_t transaction_limit, @@ -57,8 +55,7 @@ namespace iroha { std::shared_ptr tx_cache, std::shared_ptr proposal_creation_strategy, logger::LoggerPtr log, - size_t number_of_proposals = 3, - const consensus::Round &initial_round = {2, kFirstRejectRound}); + size_t number_of_proposals = 3); // --------------------- | OnDemandOrderingService |_--------------------- diff --git a/test/module/irohad/ordering/on_demand_os_test.cpp b/test/module/irohad/ordering/on_demand_os_test.cpp index ea24356e33..b9a60fadc0 100644 --- a/test/module/irohad/ordering/on_demand_os_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_test.cpp @@ -75,8 +75,7 @@ class OnDemandOsTest : public ::testing::Test { std::move(tx_cache), proposal_creation_strategy, getTestLogger("OdOrderingService"), - proposal_limit, - initial_round); + proposal_limit); } /** @@ -203,8 +202,7 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { std::move(tx_cache), proposal_creation_strategy, getTestLogger("OdOrderingService"), - proposal_limit, - initial_round); + proposal_limit); auto call = [this](auto bounds) { for (auto i = bounds.first; i < bounds.second; ++i) { @@ -286,8 +284,7 @@ TEST_F(OnDemandOsTest, UseFactoryForProposal) { std::move(tx_cache), proposal_creation_strategy, getTestLogger("OdOrderingService"), - proposal_limit, - initial_round); + proposal_limit); EXPECT_CALL(*mock_factory, unsafeCreateProposal(_, _, _)) .WillOnce(Return(ByMove(makeMockProposal()))) From f6f1427cea79bbfaecb452763b53fdeb8a2e5bef Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 18:27:24 +0300 Subject: [PATCH 19/22] Simplify OS test Signed-off-by: Andrei Lebedev --- .../impl/on_demand_os_server_grpc.cpp | 1 - .../irohad/ordering/on_demand_os_test.cpp | 59 +------------------ 2 files changed, 2 insertions(+), 58 deletions(-) diff --git a/irohad/ordering/impl/on_demand_os_server_grpc.cpp b/irohad/ordering/impl/on_demand_os_server_grpc.cpp index b57f8aaa1e..a7a964410e 100644 --- a/irohad/ordering/impl/on_demand_os_server_grpc.cpp +++ b/irohad/ordering/impl/on_demand_os_server_grpc.cpp @@ -104,7 +104,6 @@ grpc::Status OnDemandOsServerGrpc::RequestProposal( fetchPeer(request->peer_key()) | [this, &request, &response](auto &&peer) { consensus::Round round{request->round().block_round(), request->round().reject_round()}; - // todo add force initialization of proposal proposal_creation_strategy_->onProposal(peer, round); ordering_service_->onRequestProposal(round) | [&](auto &&proposal) { *response->mutable_proposal() = std::move( diff --git a/test/module/irohad/ordering/on_demand_os_test.cpp b/test/module/irohad/ordering/on_demand_os_test.cpp index b9a60fadc0..27f4d6eff5 100644 --- a/test/module/irohad/ordering/on_demand_os_test.cpp +++ b/test/module/irohad/ordering/on_demand_os_test.cpp @@ -68,6 +68,8 @@ class OnDemandOsTest : public ::testing::Test { proposal_creation_strategy = std::make_shared(); + ON_CALL(*proposal_creation_strategy, shouldCreateRound(_)) + .WillByDefault(Return(true)); os = std::make_shared( transaction_limit, @@ -126,11 +128,6 @@ class OnDemandOsTest : public ::testing::Test { * @then check that previous round doesn't have proposal */ TEST_F(OnDemandOsTest, EmptyRound) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - ASSERT_FALSE(os->onRequestProposal(initial_round)); os->onCollaborationOutcome(commit_round); @@ -145,11 +142,6 @@ TEST_F(OnDemandOsTest, EmptyRound) { * @then check that previous round has all transaction */ TEST_F(OnDemandOsTest, NormalRound) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - generateTransactionsAndInsert({1, 2}); os->onCollaborationOutcome(commit_round); @@ -165,11 +157,6 @@ TEST_F(OnDemandOsTest, NormalRound) { * AND the rest of transactions isn't appeared in next after next round */ TEST_F(OnDemandOsTest, OverflowRound) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - generateTransactionsAndInsert({1, transaction_limit * 2}); os->onCollaborationOutcome(commit_round); @@ -186,11 +173,6 @@ TEST_F(OnDemandOsTest, OverflowRound) { * @then check that all transactions appear in proposal */ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - auto large_tx_limit = 10000u; auto factory = std::make_unique< shared_model::proto::ProtoProposalFactory>(); @@ -228,11 +210,6 @@ TEST_F(OnDemandOsTest, DISABLED_ConcurrentInsert) { * tryErase */ TEST_F(OnDemandOsTest, Erase) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - generateTransactionsAndInsert({1, 2}); os->onCollaborationOutcome( {commit_round.block_round, commit_round.reject_round}); @@ -255,11 +232,6 @@ TEST_F(OnDemandOsTest, Erase) { * @then check that proposal factory is called and returns a proposal */ TEST_F(OnDemandOsTest, UseFactoryForProposal) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - auto factory = std::make_unique(); auto mock_factory = factory.get(); auto tx_cache = @@ -309,11 +281,6 @@ auto batchRef(const shared_model::interface::TransactionBatch &batch) { * @then the batch is not present in a proposal */ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - auto batches = generateTransactions({1, 2}); auto &batch = *batches.at(0); @@ -336,11 +303,6 @@ TEST_F(OnDemandOsTest, AlreadyProcessedProposalDiscarded) { * @then batch is present in a proposal */ TEST_F(OnDemandOsTest, PassMissingTransaction) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - auto batches = generateTransactions({1, 2}); auto &batch = *batches.at(0); @@ -365,11 +327,6 @@ TEST_F(OnDemandOsTest, PassMissingTransaction) { * @then 2 new batches are in a proposal and already commited batch is discarded */ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - auto batches = generateTransactions({1, 4}); auto &batch1 = *batches.at(0); auto &batch2 = *batches.at(1); @@ -405,11 +362,6 @@ TEST_F(OnDemandOsTest, SeveralTransactionsOneCommited) { * @then the proposal contains the batch once */ TEST_F(OnDemandOsTest, DuplicateTxTest) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - auto now = iroha::time::now(); auto txs1 = generateTransactions({1, 2}, now); os->onBatches(txs1); @@ -428,11 +380,6 @@ TEST_F(OnDemandOsTest, DuplicateTxTest) { * @then both of them are used for the next proposal */ TEST_F(OnDemandOsTest, RejectCommit) { - EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) - .WillRepeatedly(testing::Return(true)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); - auto now = iroha::time::now(); auto txs1 = generateTransactions({1, 2}, now); os->onBatches(txs1); @@ -460,8 +407,6 @@ TEST_F(OnDemandOsTest, RejectCommit) { TEST_F(OnDemandOsTest, FailOnCreationStrategy) { EXPECT_CALL(*proposal_creation_strategy, shouldCreateRound(_)) .WillRepeatedly(testing::Return(false)); - EXPECT_CALL(*proposal_creation_strategy, onProposal(_, _)) - .WillRepeatedly(testing::Return(boost::none)); generateTransactionsAndInsert({1, 2}); From 09554efb71c574d4d4b6eeed173e07437696e366 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 19:03:03 +0300 Subject: [PATCH 20/22] Revert torii_transport_command_test Signed-off-by: Andrei Lebedev --- test/module/irohad/torii/torii_transport_command_test.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/module/irohad/torii/torii_transport_command_test.cpp b/test/module/irohad/torii/torii_transport_command_test.cpp index abc2844382..705f58babb 100644 --- a/test/module/irohad/torii/torii_transport_command_test.cpp +++ b/test/module/irohad/torii/torii_transport_command_test.cpp @@ -182,8 +182,7 @@ TEST_F(CommandServiceTransportGrpcTest, ListToriiInvalid) { } shared_model::validation::Answer error; - error.addReason( - std::make_pair("some error", std::vector{"error_reason"})); + error.addReason(std::make_pair("some error", std::vector{})); EXPECT_CALL(*proto_tx_validator, validate(_)) .Times(kTimes) .WillRepeatedly(Return(shared_model::validation::Answer{})); @@ -223,8 +222,7 @@ TEST_F(CommandServiceTransportGrpcTest, ListToriiPartialInvalid) { .WillRepeatedly(Invoke([this, &counter, kError](const auto &) mutable { shared_model::validation::Answer res; if (counter++ == kTimes - 1) { - res.addReason( - std::make_pair(kError, std::vector{"error_reason"})); + res.addReason(std::make_pair(kError, std::vector{})); } return res; })); From 07652ad4f89ac99a3c14a88a8e2e58e0d24e34b8 Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Fri, 12 Apr 2019 22:31:59 +0300 Subject: [PATCH 21/22] Fix proto schema Signed-off-by: Andrei Lebedev --- schema/ordering.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/schema/ordering.proto b/schema/ordering.proto index be47730b04..1ab400f291 100644 --- a/schema/ordering.proto +++ b/schema/ordering.proto @@ -21,12 +21,12 @@ message ProposalRound { message BatchesRequest { repeated protocol.Transaction transactions = 1; - string peer_key = 2; + bytes peer_key = 2; } message ProposalRequest { ProposalRound round = 1; - string peer_key = 2; + bytes peer_key = 2; } message ProposalResponse { From c02883f4d6ce372aaec7bfb77b05f1f4f4ad0f3f Mon Sep 17 00:00:00 2001 From: Andrei Lebedev Date: Sun, 14 Apr 2019 20:13:51 +0300 Subject: [PATCH 22/22] Fix hasMajority Signed-off-by: Andrei Lebedev --- .../yac/impl/supermajority_checker_bft.cpp | 5 ++-- .../yac/impl/supermajority_checker_cft.cpp | 5 ++-- .../yac/impl/supermajority_checker_kf1.hpp | 9 +++++++ .../yac/supermajority_checker_test.cpp | 24 ++++++++++++++++++- 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/irohad/consensus/yac/impl/supermajority_checker_bft.cpp b/irohad/consensus/yac/impl/supermajority_checker_bft.cpp index 7c0558a162..0251d635d0 100644 --- a/irohad/consensus/yac/impl/supermajority_checker_bft.cpp +++ b/irohad/consensus/yac/impl/supermajority_checker_bft.cpp @@ -20,8 +20,9 @@ namespace iroha { } bool SupermajorityCheckerBft::hasMajority(PeersNumberType voted, - PeersNumberType all) const { - return checkKfPlus1Supermajority(voted, all, 2); + PeersNumberType all) const { + return checkKfPlus1Majority( + voted, all, detail::kSupermajorityCheckerKfPlus1Bft); } bool SupermajorityCheckerBft::canHaveSupermajority( diff --git a/irohad/consensus/yac/impl/supermajority_checker_cft.cpp b/irohad/consensus/yac/impl/supermajority_checker_cft.cpp index fcb67e601f..5c24884248 100644 --- a/irohad/consensus/yac/impl/supermajority_checker_cft.cpp +++ b/irohad/consensus/yac/impl/supermajority_checker_cft.cpp @@ -20,8 +20,9 @@ namespace iroha { } bool SupermajorityCheckerCft::hasMajority(PeersNumberType voted, - PeersNumberType all) const{ - return hasSupermajority(voted, all); + PeersNumberType all) const { + return checkKfPlus1Majority( + voted, all, detail::kSupermajorityCheckerKfPlus1Cft); } bool SupermajorityCheckerCft::canHaveSupermajority( diff --git a/irohad/consensus/yac/impl/supermajority_checker_kf1.hpp b/irohad/consensus/yac/impl/supermajority_checker_kf1.hpp index ef82b3731d..1d270c43e1 100644 --- a/irohad/consensus/yac/impl/supermajority_checker_kf1.hpp +++ b/irohad/consensus/yac/impl/supermajority_checker_kf1.hpp @@ -34,6 +34,15 @@ namespace iroha { return agreed * k >= (k - 1) * (all - 1) + k; } + inline bool checkKfPlus1Majority(PeersNumberType agreed, + PeersNumberType all, + unsigned int k) { + if (agreed > all) { + return false; + } + return agreed * k > all - 1; + } + } // namespace yac } // namespace consensus } // namespace iroha diff --git a/test/module/irohad/consensus/yac/supermajority_checker_test.cpp b/test/module/irohad/consensus/yac/supermajority_checker_test.cpp index a2eaaf945c..e307173819 100644 --- a/test/module/irohad/consensus/yac/supermajority_checker_test.cpp +++ b/test/module/irohad/consensus/yac/supermajority_checker_test.cpp @@ -44,6 +44,10 @@ class SupermajorityCheckerTest return total_peers - getAllowedFaultyPeers(total_peers); } + size_t getMajority(size_t total_peers) const { + return getAllowedFaultyPeers(total_peers) + 1; + } + std::string modelToString() const { return "`" + std::to_string(getK()) + " * f + 1' " + (GetParam() == ConsistencyModel::kBft ? "BFT" : "CFT") + " model"; @@ -112,7 +116,7 @@ TEST_P(CftAndBftSupermajorityCheckerTest, SuperMajorityCheckWithSize2) { * @when check range of voted participants * @then correct result */ -TEST_P(SupermajorityCheckerTest, SuperMajorityCheckWithSize4) { +TEST_P(CftAndBftSupermajorityCheckerTest, SuperMajorityCheckWithSize4) { log_->info("-----------| F(x, 4), x in [0..5] |-----------"); size_t A = 6; // number of all peers @@ -131,6 +135,24 @@ TEST_P(SupermajorityCheckerTest, SuperMajorityCheckWithSize4) { } } +/** + * @given 4 participants + * @when check range of voted participants + * @then correct result + */ +TEST_P(CftAndBftSupermajorityCheckerTest, MajorityWithSize4) { + size_t A = 4; + for (size_t i = 0; i < 5; ++i) { + if (i >= getMajority(A) and i <= A) { + ASSERT_TRUE(hasMajority(i, A)) + << i << " votes out of " << A << " in " << modelToString(); + } else { + ASSERT_FALSE(hasMajority(i, A)) + << i << " votes out of " << A << " in " << modelToString(); + } + } +} + /** * \attention this test does not address possible supermajority on other peers * due to malicious peers sending then votes for other hashes