Skip to content

Commit 74c8ac2

Browse files
MBoldyrevlebdron
authored andcommitted
mst incoming state logic fix
Signed-off-by: Mikhail Boldyrev <miboldyrev@gmail.com>
1 parent 442a083 commit 74c8ac2

File tree

8 files changed

+41
-13
lines changed

8 files changed

+41
-13
lines changed

irohad/multi_sig_transactions/impl/mst_processor_impl.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,12 @@ namespace iroha {
8686
// -------------------| MstTransportNotification override |-------------------
8787

8888
void FairMstProcessor::onNewState(const shared_model::crypto::PublicKey &from,
89-
ConstRefState new_state) {
89+
MstState new_state) {
9090
log_->info("Applying new state");
9191
auto current_time = time_provider_->getCurrentTime();
9292

93+
// no need to add already expired batches to local state
94+
new_state.eraseExpired(current_time);
9395
auto state_update = storage_->apply(from, new_state);
9496

9597
// updated batches
@@ -101,7 +103,8 @@ namespace iroha {
101103
completedBatchesNotify(*state_update.completed_state_);
102104

103105
// expired batches
104-
expiredBatchesNotify(storage_->getDiffState(from, current_time));
106+
// not nesessary to do it right here, just use the occasion to clean storage
107+
expiredBatchesNotify(storage_->extractExpiredTransactions(current_time));
105108
}
106109

107110
// -----------------------------| private api |-----------------------------

irohad/multi_sig_transactions/mst_processor_impl.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ namespace iroha {
5555
// ------------------| MstTransportNotification override |------------------
5656

5757
void onNewState(const shared_model::crypto::PublicKey &from,
58-
ConstRefState new_state) override;
58+
MstState new_state) override;
5959

6060
// ----------------------------| end override |-----------------------------
6161

irohad/network/mst_transport.hpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,8 @@ namespace iroha {
2323
* @param from - key of the peer emitted the state
2424
* @param new_state - state propagated from peer
2525
*/
26-
virtual void onNewState(
27-
const shared_model::crypto::PublicKey &from,
28-
const MstState &new_state) = 0;
26+
virtual void onNewState(const shared_model::crypto::PublicKey &from,
27+
MstState new_state) = 0;
2928

3029
virtual ~MstTransportNotification() = default;
3130
};

test/framework/integration_framework/fake_peer/network/mst_message.hpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,8 @@
1212
namespace integration_framework {
1313
namespace fake_peer {
1414
struct MstMessage final {
15-
MstMessage(const shared_model::crypto::PublicKey &f,
16-
const iroha::MstState &s)
17-
: from(f), state(s) {}
15+
MstMessage(const shared_model::crypto::PublicKey &f, iroha::MstState s)
16+
: from(f), state(std::move(s)) {}
1817
shared_model::crypto::PublicKey from;
1918
iroha::MstState state;
2019
};

test/framework/integration_framework/fake_peer/network/mst_network_notifier.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ namespace integration_framework {
1010

1111
void MstNetworkNotifier::onNewState(
1212
const shared_model::crypto::PublicKey &from,
13-
const iroha::MstState &new_state) {
13+
iroha::MstState new_state) {
1414
std::lock_guard<std::mutex> guard(mst_subject_mutex_);
1515
mst_subject_.get_subscriber().on_next(
16-
std::make_shared<MstMessage>(from, new_state));
16+
std::make_shared<MstMessage>(from, std::move(new_state)));
1717
}
1818

1919
rxcpp::observable<std::shared_ptr<MstMessage>>

test/framework/integration_framework/fake_peer/network/mst_network_notifier.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace integration_framework {
2121
: public iroha::network::MstTransportNotification {
2222
public:
2323
void onNewState(const shared_model::crypto::PublicKey &from,
24-
const iroha::MstState &new_state) override;
24+
iroha::MstState new_state) override;
2525

2626
rxcpp::observable<std::shared_ptr<MstMessage>> getObservable();
2727

test/module/irohad/multi_sig_transactions/mst_mocks.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace iroha {
3333
public:
3434
MOCK_METHOD2(onNewState,
3535
void(const shared_model::crypto::PublicKey &from,
36-
const MstState &state));
36+
MstState state));
3737
};
3838

3939
/**

test/module/irohad/multi_sig_transactions/mst_processor_test.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,3 +319,30 @@ TEST_F(MstProcessorTest, emptyStatePropagation) {
319319
another_peer};
320320
propagation_subject.get_subscriber().on_next(peers);
321321
}
322+
323+
/**
324+
* @given initialized mst processor with empty state
325+
*
326+
* @when received other peer's state containing an outdated batch
327+
*
328+
* @then check that transport was not invoked
329+
* @and queues are not pushed to
330+
* @and the batch does not get into our state
331+
*/
332+
TEST_F(MstProcessorTest, receivedOutdatedState) {
333+
// ---------------------------------| then |----------------------------------
334+
EXPECT_CALL(*transport, sendState(_, _)).Times(0);
335+
auto observers = initObservers(mst_processor, 0, 0, 0);
336+
337+
// ---------------------------------| when |----------------------------------
338+
shared_model::crypto::PublicKey another_peer_key("another_pubkey");
339+
auto transported_state = MstState::empty(getTestLogger("MstState"),
340+
std::make_shared<TestCompleter>());
341+
const auto expired_batch = makeTestBatch(txBuilder(1, time_before, 3));
342+
transported_state += addSignaturesFromKeyPairs(expired_batch, 0, makeKey());
343+
mst_processor->onNewState(another_peer_key, transported_state);
344+
345+
// ---------------------------------| then |----------------------------------
346+
EXPECT_FALSE(storage->batchInStorage(expired_batch));
347+
check(observers);
348+
}

0 commit comments

Comments
 (0)