Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove protoc dep, use new MerkleTree methods, fix Agreement. #32

Merged
merged 2 commits into from
May 18, 2018

Conversation

afck
Copy link
Collaborator

@afck afck commented May 17, 2018

No description provided.

@afck afck requested a review from DrPeterVanNostrand May 17, 2018 14:52
@afck afck requested a review from vkomenda May 17, 2018 15:39
@afck afck changed the title Remove protoc dep, use new MerkleTree methods. Remove protoc dep, use new MerkleTree methods, fix Agreement. May 17, 2018
if message.epoch() < self.epoch {
return Ok(()); // Message is obsolete: We are already in a later epoch.
}
if message.epoch() > self.epoch {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be else if.

.push_back(AgreementMessage::BVal(self.epoch, b));
// Receive the BVAL message locally.
let our_uid = self.uid.clone();
self.handle_bval(&our_uid, b)
Copy link
Contributor

Choose a reason for hiding this comment

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

Well spotted!

@@ -178,32 +190,35 @@ impl<NodeUid: Clone + Debug + Eq + Hash> Agreement<NodeUid> {
self.messages
.push_back(AgreementMessage::Aux(self.epoch, b));
// Receive the AUX message locally.
self.received_aux.insert(self.uid.clone(), b);
let our_uid = self.uid.clone();
self.handle_aux(&our_uid, b)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense too.

self.send_bval(b)?;
let queued_msgs = mem::replace(&mut self.incoming_queue, Vec::new());
for (sender_id, msg) in queued_msgs {
self.handle_message(&sender_id, msg)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is one possible place for introducing spam control. I've no idea though how to avoid spam apart from using a set instead of Vec of queued messages and filtering out messages with unknown sender IDs. The latter is already done at the top level.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's true. Basically we just need to store per epoch: who sent us which BVals and which Aux.

We still might want to limit the number of future epochs somehow. On the other hand, that wouldn't be strictly correct…

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I created a new issue for this: #43

@vkomenda vkomenda merged commit e4843b4 into master May 18, 2018
@afck afck deleted the afck-no-protoc branch May 28, 2018 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants