Skip to content
This repository has been archived by the owner on Jun 25, 2021. It is now read-only.

Don't suppress parsec prune when relocation is in progress #1993

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/chain/chain.rs
Expand Up @@ -661,10 +661,9 @@ impl Chain {
self.state.split_in_progress
}

/// Returns whether a membership change is in progress (a node leaving, joining or being
/// relocated).
pub fn membership_change_in_progress(&self) -> bool {
self.churn_in_progress || self.relocation_in_progress
/// Returns whether a churn (elders change) is in progress.
pub fn churn_in_progress(&self) -> bool {
self.churn_in_progress
}

/// Neighbour infos signed by our section
Expand Down
37 changes: 17 additions & 20 deletions src/states/elder/mod.rs
Expand Up @@ -413,7 +413,13 @@ impl Elder {
self.parsec_map
.our_unpolled_observations()
.filter_map(|obs| match obs {
parsec::Observation::OpaquePayload(event) => Some(event),
parsec::Observation::OpaquePayload(event) => {
if !completed_events.contains(&event.payload) {
Some(event)
} else {
None
}
}

parsec::Observation::Genesis { .. }
| parsec::Observation::Add { .. }
Expand All @@ -432,7 +438,7 @@ impl Elder {
.iter()
.filter(|event| match &event.payload {
// Events to re-process
AccumulatingEvent::Online(_) => !completed_events.contains(&event.payload),
AccumulatingEvent::Online(_) => true,
// Events to re-insert
AccumulatingEvent::Offline(_)
| AccumulatingEvent::AckMessage(_)
Expand All @@ -453,26 +459,20 @@ impl Elder {
.into_iter()
.filter(|event| {
match event.payload {
// Only re-vote not yet accumulated events and still relevant to our new prefix.
// Only re-vote if still relevant to our new prefix.
AccumulatingEvent::Online(ref payload) => {
our_pfx.matches(payload.p2p_node.name())
&& !completed_events.contains(&event.payload)
}
AccumulatingEvent::Offline(pub_id) => {
our_pfx.matches(pub_id.name()) && !completed_events.contains(&event.payload)
}
AccumulatingEvent::Offline(pub_id) => our_pfx.matches(pub_id.name()),
AccumulatingEvent::AckMessage(ref payload) => {
our_pfx.matches(&payload.dst_name)
&& !completed_events.contains(&event.payload)
}

AccumulatingEvent::Relocate(ref details)
| AccumulatingEvent::RelocatePrepare(ref details, _) => {
our_pfx.matches(details.pub_id.name())
}
// Drop: no longer relevant after prefix change.
// TODO: verify this is really the case. Some/all of these might still make sense
// to carry over. In case it does not, add a comment explaining why.
AccumulatingEvent::StartDkg(_)
| AccumulatingEvent::ParsecPrune
| AccumulatingEvent::Relocate(_)
| AccumulatingEvent::RelocatePrepare(_, _) => false,
AccumulatingEvent::StartDkg(_) | AccumulatingEvent::ParsecPrune => false,

// Keep: Additional signatures for neighbours for sec-msg-relay.
AccumulatingEvent::SectionInfo(ref elders_info, _)
Expand Down Expand Up @@ -1672,11 +1672,8 @@ impl Approved for Elder {
);
return Ok(());
}
if self.chain.membership_change_in_progress() {
trace!(
"{} - ignore ParsecPrune - membership change in progress.",
self
);
if self.chain.churn_in_progress() {
trace!("{} - ignore ParsecPrune - churn in progress.", self);
return Ok(());
}

Expand Down