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

Divide logic between enum variants #5488

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Divide logic between enum variants #5488

merged 2 commits into from
Dec 11, 2024

Conversation

kylezs
Copy link
Contributor

@kylezs kylezs commented Dec 11, 2024

No description provided.

@kylezs kylezs requested a review from MxmUrw December 11, 2024 05:48
Comment on lines +182 to +192
ChainProgress::Reorg(reorg_range) => {
// Delete any elections that are ongoing for any blocks in the reorg range.
for (i, election_identifier) in election_identifiers.into_iter().enumerate() {
let election = ElectoralAccess::election_mut(election_identifier);
let properties = election.properties()?;
if properties.0.into_range_inclusive() == *reorg_range {
election.delete();
open_elections = open_elections.saturating_sub(1);
remaining_election_identifiers.remove(i);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, when I detect a reorg, I have two ranges: removed and added. Depending on circumstances either one could be "longer" then the other. removed are the headers which I have previously witnessed and forwarded to you and are now invalid due to the reorg, added are the replacement headers now valid after the reorg.

It seems to me that since you want to delete the previously created elections, and recreate elections for the new blocks, you actually want to get both ranges. So I propose to change the variant to be ChainProgress::Reorg { removed_range: InclusiveRange<>, added_range: InclusiveRange<> }

@MxmUrw MxmUrw marked this pull request as ready for review December 11, 2024 16:58
@MxmUrw MxmUrw requested a review from dandanlen as a code owner December 11, 2024 16:58
@MxmUrw MxmUrw merged commit 5c06446 into feat/block-witnesser-es Dec 11, 2024
@MxmUrw MxmUrw deleted the kyle/wip branch December 11, 2024 16:59
kylezs added a commit that referenced this pull request Dec 17, 2024
* wip

* divide logic between enum variants
MxmUrw pushed a commit that referenced this pull request Jan 27, 2025
* wip

* divide logic between enum variants
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