From 0238768870e64d47f5cc5990401315e1d5e4535c Mon Sep 17 00:00:00 2001 From: Mikhail Boldyrev Date: Thu, 4 Apr 2019 17:02:25 +0300 Subject: [PATCH 1/3] fixed expired batches notification Signed-off-by: Mikhail Boldyrev --- irohad/multi_sig_transactions/impl/mst_processor_impl.cpp | 6 ++---- irohad/multi_sig_transactions/mst_processor_impl.hpp | 2 +- irohad/network/mst_transport.hpp | 5 ++--- test/module/irohad/multi_sig_transactions/mst_mocks.hpp | 2 +- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/irohad/multi_sig_transactions/impl/mst_processor_impl.cpp b/irohad/multi_sig_transactions/impl/mst_processor_impl.cpp index e5bdbdcc40..65d6d66aee 100644 --- a/irohad/multi_sig_transactions/impl/mst_processor_impl.cpp +++ b/irohad/multi_sig_transactions/impl/mst_processor_impl.cpp @@ -92,10 +92,11 @@ namespace iroha { // -------------------| MstTransportNotification override |------------------- void FairMstProcessor::onNewState(const shared_model::crypto::PublicKey &from, - ConstRefState new_state) { + MstState new_state) { log_->info("Applying new state"); auto current_time = time_provider_->getCurrentTime(); + new_state.eraseExpired(current_time); auto state_update = storage_->apply(from, new_state); // updated batches @@ -105,9 +106,6 @@ namespace iroha { // completed batches completedBatchesNotify(*state_update.completed_state_); - - // expired batches - expiredBatchesNotify(storage_->getDiffState(from, current_time)); } // -----------------------------| private api |----------------------------- diff --git a/irohad/multi_sig_transactions/mst_processor_impl.hpp b/irohad/multi_sig_transactions/mst_processor_impl.hpp index 6cf3aca34f..d16130448e 100644 --- a/irohad/multi_sig_transactions/mst_processor_impl.hpp +++ b/irohad/multi_sig_transactions/mst_processor_impl.hpp @@ -55,7 +55,7 @@ namespace iroha { // ------------------| MstTransportNotification override |------------------ void onNewState(const shared_model::crypto::PublicKey &from, - ConstRefState new_state) override; + MstState new_state) override; // ----------------------------| end override |----------------------------- diff --git a/irohad/network/mst_transport.hpp b/irohad/network/mst_transport.hpp index f238f40d49..dc6d8aa6d6 100644 --- a/irohad/network/mst_transport.hpp +++ b/irohad/network/mst_transport.hpp @@ -23,9 +23,8 @@ namespace iroha { * @param from - key of the peer emitted the state * @param new_state - state propagated from peer */ - virtual void onNewState( - const shared_model::crypto::PublicKey &from, - const MstState &new_state) = 0; + virtual void onNewState(const shared_model::crypto::PublicKey &from, + MstState new_state) = 0; virtual ~MstTransportNotification() = default; }; diff --git a/test/module/irohad/multi_sig_transactions/mst_mocks.hpp b/test/module/irohad/multi_sig_transactions/mst_mocks.hpp index fbdbfee445..0e9bfbeae3 100644 --- a/test/module/irohad/multi_sig_transactions/mst_mocks.hpp +++ b/test/module/irohad/multi_sig_transactions/mst_mocks.hpp @@ -33,7 +33,7 @@ namespace iroha { public: MOCK_METHOD2(onNewState, void(const shared_model::crypto::PublicKey &from, - const MstState &state)); + MstState state)); }; /** From f74603f04c50a495d57ce4574835e3715811a78e Mon Sep 17 00:00:00 2001 From: Mikhail Boldyrev Date: Thu, 4 Apr 2019 17:02:32 +0300 Subject: [PATCH 2/3] added test Signed-off-by: Mikhail Boldyrev --- .../mst_processor_test.cpp | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/module/irohad/multi_sig_transactions/mst_processor_test.cpp b/test/module/irohad/multi_sig_transactions/mst_processor_test.cpp index 4e4aacd004..0fee0420f1 100644 --- a/test/module/irohad/multi_sig_transactions/mst_processor_test.cpp +++ b/test/module/irohad/multi_sig_transactions/mst_processor_test.cpp @@ -319,3 +319,30 @@ TEST_F(MstProcessorTest, emptyStatePropagation) { another_peer}; propagation_subject.get_subscriber().on_next(peers); } + +/** + * @given initialized mst processor with empty state + * + * @when received other peer's state containing an outdated batch + * + * @then check that transport was not invoked + * @and queues are not pushed to + * @and the batch does not get into our state + */ +TEST_F(MstProcessorTest, receivedOutdatedState) { + // ---------------------------------| then |---------------------------------- + EXPECT_CALL(*transport, sendState(_, _)).Times(0); + auto observers = initObservers(mst_processor, 0, 0, 0); + + // ---------------------------------| when |---------------------------------- + shared_model::crypto::PublicKey another_peer_key("another_pubkey"); + auto transported_state = MstState::empty(getTestLogger("MstState"), + std::make_shared()); + const auto expired_batch = makeTestBatch(txBuilder(1, time_before, 3)); + transported_state += addSignaturesFromKeyPairs(expired_batch, 0, makeKey()); + mst_processor->onNewState(another_peer_key, transported_state); + + // ---------------------------------| then |---------------------------------- + EXPECT_FALSE(storage->batchInStorage(expired_batch)); + check(observers); +} From 8bd0f007d3581b733a5dda076549704d53244e63 Mon Sep 17 00:00:00 2001 From: Mikhail Boldyrev Date: Wed, 10 Apr 2019 18:02:07 +0300 Subject: [PATCH 3/3] pr issues Signed-off-by: Mikhail Boldyrev --- irohad/multi_sig_transactions/impl/mst_processor_impl.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/irohad/multi_sig_transactions/impl/mst_processor_impl.cpp b/irohad/multi_sig_transactions/impl/mst_processor_impl.cpp index 65d6d66aee..8ec61d6811 100644 --- a/irohad/multi_sig_transactions/impl/mst_processor_impl.cpp +++ b/irohad/multi_sig_transactions/impl/mst_processor_impl.cpp @@ -96,6 +96,7 @@ namespace iroha { log_->info("Applying new state"); auto current_time = time_provider_->getCurrentTime(); + // no need to add already expired batches to local state new_state.eraseExpired(current_time); auto state_update = storage_->apply(from, new_state); @@ -106,6 +107,10 @@ namespace iroha { // completed batches completedBatchesNotify(*state_update.completed_state_); + + // expired batches + // not nesessary to do it right here, just use the occasion to clean storage + expiredBatchesNotify(storage_->extractExpiredTransactions(current_time)); } // -----------------------------| private api |-----------------------------