Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

EPM and staking events improvement #13035

Merged
merged 9 commits into from
Jan 9, 2023
Merged

EPM and staking events improvement #13035

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
45940b2
EPM and staking events improvement
gpestana Dec 29, 2022
9d1a25f
Uses RawOrigin in ElectionCompute event
gpestana Dec 30, 2022
73dcbd6
Refactors new phase events to PhaseTransition event
gpestana Jan 2, 2023
1685ac1
PhaseTransitioned and remove RawOrigin from event
gpestana Jan 3, 2023
d682484
Adds helpers for epm phase transition and staking force new
gpestana Jan 3, 2023
dd45479
addresses review comments
gpestana Jan 4, 2023
26f7f65
nit: removes unecessary clone
gpestana Jan 4, 2023
641b287
fixes benchmarks
gpestana Jan 9, 2023
ca10f7b
Merge remote-tracking branch 'origin/master' into gpestana/13026-epm-…
Jan 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 97 additions & 26 deletions frame/election-provider-multi-phase/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,7 @@ pub mod pallet {
<QueuedSolution<T>>::put(ready);
Self::deposit_event(Event::SolutionStored {
compute: ElectionCompute::Unsigned,
account_id: None,
prev_ejected: ejected_a_solution,
});

Expand Down Expand Up @@ -983,6 +984,7 @@ pub mod pallet {

Self::deposit_event(Event::SolutionStored {
compute: ElectionCompute::Emergency,
account_id: None,
gpestana marked this conversation as resolved.
Show resolved Hide resolved
prev_ejected: QueuedSolution::<T>::exists(),
});

Expand Down Expand Up @@ -1060,6 +1062,7 @@ pub mod pallet {
signed_submissions.put();
Self::deposit_event(Event::SolutionStored {
compute: ElectionCompute::Signed,
account_id: Some(who),
prev_ejected: ejected_a_solution,
});
Ok(())
Expand Down Expand Up @@ -1102,6 +1105,7 @@ pub mod pallet {

Self::deposit_event(Event::SolutionStored {
compute: ElectionCompute::Fallback,
account_id: None,
prev_ejected: QueuedSolution::<T>::exists(),
});

Expand All @@ -1118,8 +1122,14 @@ pub mod pallet {
/// If the solution is signed, this means that it hasn't yet been processed. If the
/// solution is unsigned, this means that it has also been processed.
///
/// The `bool` is `true` when a previous solution was ejected to make room for this one.
SolutionStored { compute: ElectionCompute, prev_ejected: bool },
/// If `account_id` is `None` if the solution is stored during the unsigned phase or by
/// `T::ForceOrigin`. The `bool` is `true` when a previous solution was ejected to make
/// room for this one.
SolutionStored {
compute: ElectionCompute,
account_id: Option<T::AccountId>,
prev_ejected: bool,
},
/// The election has been finalized, with the given computation and score.
ElectionFinalized { compute: ElectionCompute, score: ElectionScore },
/// An election failed.
Expand All @@ -1134,6 +1144,10 @@ pub mod pallet {
SignedPhaseStarted { round: u32 },
gpestana marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes might break some UIs depending on these events. May be we should label it as breaking changes (E5?, B7 ) with some release notes in description.

@kianenigma might have better insights.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think any UI reads these events, so it should be okay.

Nonetheless, I would love to see Ankan's suggestions in PRs that actually do break UIs that we care about.

The broader meta-discussion here is what degree of release-note do we want to have for breaking frame changes.

I think there is a LOT LOT more that we can do here, but the current labels are barely enough to reflect changes that are relevant to parachain teams and polkadot, and for this audience I don't think this matters.

Copy link
Contributor

Choose a reason for hiding this comment

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

hi,
currently the only thing you can do to highlight a code change on the release notes is elevating its upgrade criticality with the C* labels. This will change though, we're slowly implementing new labels on polkadot/substrate/cumulus that have a clear description and in future the C* labels should be used in combination with other labels so that you can distinguish what it is about upfront. Here is the documentation of the new labels explaining how they should be used: https://github.com/paritytech/labels/blob/main/docs/doc_cumulus.md

/// The unsigned phase of the given round has started.
UnsignedPhaseStarted { round: u32 },
/// The `Phase::Off` of the given round has started.
OffPhaseStarted { round: u32 },
/// The emergency phase has started in the given round.
EmergencyPhaseStarted { round: u32 },
}

/// Error of the pallet that can be returned in response to dispatches.
Expand Down Expand Up @@ -1573,6 +1587,8 @@ impl<T: Config> Pallet<T> {
// Phase is off now.
<CurrentPhase<T>>::put(Phase::Off);

Self::deposit_event(Event::OffPhaseStarted { round: Self::round() });
gpestana marked this conversation as resolved.
Show resolved Hide resolved

// Kill snapshots.
Self::kill_snapshot();
}
Expand Down Expand Up @@ -1653,6 +1669,7 @@ impl<T: Config> ElectionProvider for Pallet<T> {
Err(why) => {
log!(error, "Entering emergency mode: {:?}", why);
<CurrentPhase<T>>::put(Phase::Emergency);
Self::deposit_event(Event::EmergencyPhaseStarted { round: Self::round() });
Err(why)
},
}
Expand Down Expand Up @@ -1959,6 +1976,7 @@ mod tests {
sum_stake_squared: 0
}
},
Event::OffPhaseStarted { round: 2 },
Event::SignedPhaseStarted { round: 2 },
Event::UnsignedPhaseStarted { round: 2 }
]
Expand Down Expand Up @@ -1998,7 +2016,8 @@ mod tests {
sum_stake: 0,
sum_stake_squared: 0
}
}
},
Event::OffPhaseStarted { round: 2 },
]
);
});
Expand Down Expand Up @@ -2036,7 +2055,8 @@ mod tests {
sum_stake: 0,
sum_stake_squared: 0
}
}
},
Event::OffPhaseStarted { round: 2 },
]
)
});
Expand Down Expand Up @@ -2064,10 +2084,17 @@ mod tests {

assert_eq!(
multi_phase_events(),
vec![Event::ElectionFinalized {
compute: ElectionCompute::Fallback,
score: ElectionScore { minimal_stake: 0, sum_stake: 0, sum_stake_squared: 0 }
}]
vec![
Event::ElectionFinalized {
compute: ElectionCompute::Fallback,
score: ElectionScore {
minimal_stake: 0,
sum_stake: 0,
sum_stake_squared: 0
}
},
Event::OffPhaseStarted { round: 2 },
]
);
});
}
Expand All @@ -2094,7 +2121,8 @@ mod tests {
Event::ElectionFinalized {
compute: ElectionCompute::Fallback,
score: Default::default()
}
},
Event::OffPhaseStarted { round: 2 },
],
);
// All storage items must be cleared.
Expand Down Expand Up @@ -2145,11 +2173,31 @@ mod tests {
multi_phase_events(),
vec![
Event::SignedPhaseStarted { round: 1 },
Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
Event::SolutionStored {
compute: ElectionCompute::Signed,
account_id: Some(99),
prev_ejected: false
},
Event::SolutionStored {
compute: ElectionCompute::Signed,
account_id: Some(99),
prev_ejected: false
},
Event::SolutionStored {
compute: ElectionCompute::Signed,
account_id: Some(99),
prev_ejected: false
},
Event::SolutionStored {
compute: ElectionCompute::Signed,
account_id: Some(99),
prev_ejected: false
},
Event::SolutionStored {
compute: ElectionCompute::Signed,
account_id: Some(99),
prev_ejected: false
},
Event::Slashed { account: 99, value: 5 },
Event::Slashed { account: 99, value: 5 },
Event::Slashed { account: 99, value: 5 },
Expand All @@ -2162,7 +2210,8 @@ mod tests {
sum_stake: 0,
sum_stake_squared: 0
}
}
},
Event::OffPhaseStarted { round: 2 },
]
);
})
Expand All @@ -2187,7 +2236,11 @@ mod tests {
multi_phase_events(),
vec![
Event::SignedPhaseStarted { round: 1 },
Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
Event::SolutionStored {
compute: ElectionCompute::Signed,
account_id: Some(99),
prev_ejected: false
},
Event::Rewarded { account: 99, value: 7 },
Event::UnsignedPhaseStarted { round: 1 },
Event::ElectionFinalized {
Expand All @@ -2197,7 +2250,8 @@ mod tests {
sum_stake: 100,
sum_stake_squared: 5200
}
}
},
Event::OffPhaseStarted { round: 2 },
],
);
})
Expand Down Expand Up @@ -2234,6 +2288,7 @@ mod tests {
Event::UnsignedPhaseStarted { round: 1 },
Event::SolutionStored {
compute: ElectionCompute::Unsigned,
account_id: None,
prev_ejected: false
},
Event::ElectionFinalized {
Expand All @@ -2243,7 +2298,8 @@ mod tests {
sum_stake: 100,
sum_stake_squared: 5200
}
}
},
Event::OffPhaseStarted { round: 2 },
],
);
})
Expand Down Expand Up @@ -2279,7 +2335,8 @@ mod tests {
sum_stake: 0,
sum_stake_squared: 0
}
}
},
Event::OffPhaseStarted { round: 2 },
]
);
});
Expand All @@ -2301,7 +2358,8 @@ mod tests {
vec![
Event::SignedPhaseStarted { round: 1 },
Event::UnsignedPhaseStarted { round: 1 },
Event::ElectionFailed
Event::ElectionFailed,
Event::EmergencyPhaseStarted { round: 1 },
]
);
})
Expand Down Expand Up @@ -2342,14 +2400,17 @@ mod tests {
Event::SignedPhaseStarted { round: 1 },
Event::UnsignedPhaseStarted { round: 1 },
Event::ElectionFailed,
Event::EmergencyPhaseStarted { round: 1 },
Event::SolutionStored {
compute: ElectionCompute::Fallback,
account_id: None,
prev_ejected: false
},
Event::ElectionFinalized {
compute: ElectionCompute::Fallback,
score: Default::default()
}
},
Event::OffPhaseStarted { round: 2 },
]
);
})
Expand All @@ -2375,10 +2436,17 @@ mod tests {

assert_eq!(
multi_phase_events(),
vec![Event::ElectionFinalized {
compute: ElectionCompute::Fallback,
score: ElectionScore { minimal_stake: 0, sum_stake: 0, sum_stake_squared: 0 }
}]
vec![
Event::ElectionFinalized {
compute: ElectionCompute::Fallback,
score: ElectionScore {
minimal_stake: 0,
sum_stake: 0,
sum_stake_squared: 0
}
},
Event::OffPhaseStarted { round: 2 },
]
);
});
}
Expand All @@ -2404,7 +2472,10 @@ mod tests {
assert_eq!(err, ElectionError::Fallback("NoFallback."));
assert_eq!(MultiPhase::current_phase(), Phase::Emergency);

assert_eq!(multi_phase_events(), vec![Event::ElectionFailed]);
assert_eq!(
multi_phase_events(),
vec![Event::ElectionFailed, Event::EmergencyPhaseStarted { round: 1 }]
);
});
}

Expand Down
Loading