Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Replay prevention on proposal generation #1857

Merged

Conversation

nickaleks
Copy link
Contributor

Description of the Change

This PR adds replay prevention during proposal generation in on demand ordering service.

Benefits

Replay prevention

Possible Drawbacks

All batches must be checked via cache

Signed-off-by: Nikita Alekseev <[email protected]>
},
[this](const auto &status) {
log_->warn(
"Received already processed batch. Duplicate transaction: {}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the log it will be unclear, where messages about one batch end and another starts. I propose to add some splitters to the messages, so it would be:
Batch 1:
Received already processed batch. Duplicate transaction 395gjfr8g49
Received already processed batch. Duplicate transaction fc409mir43k
Batch 42:
Received already processed batch. Duplicate transaction fk4590owfje

Of course, the information can be extracted from tx hashes, but for usability it will be better, if it were understood from the first glance

@@ -13,7 +13,7 @@
#include <unordered_map>

#include <tbb/concurrent_queue.h>

#include "ametsuchi/tx_presence_cache.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't forward declaration here and include in .cpp work?

Copy link
Contributor

@igor-egorov igor-egorov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Nikita Alekseev <[email protected]>
Signed-off-by: Nikita Alekseev <[email protected]>
@nickaleks nickaleks merged commit 7638737 into trunk/tx_persistent_cache Nov 16, 2018
@l4l l4l deleted the feature/replay_proposal_generation branch December 26, 2018 21:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants