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

consensus: handle EBBs in rewindHeaderState #1691

Merged
merged 1 commit into from
Feb 25, 2020
Merged

Conversation

nfrisby
Copy link
Contributor

@nfrisby nfrisby commented Feb 24, 2020

Fixes #1690.

I believe this bug was masked by Issue #1489; I discovered it and confirmed that this patch fixes it on my related WIP branch. This PR does not include a repro for this bug, but that branch does, and PR #1506 will as soon as I'm able to update it with that WIP branch -- that's my current focus.

@nfrisby nfrisby added bug Something isn't working consensus issues related to ouroboros-consensus labels Feb 24, 2020
@nfrisby nfrisby requested a review from edsko February 24, 2020 20:25
@nfrisby nfrisby changed the title consensus: handle EBBs in rewindHeaderState DO NOT MERGE consensus: handle EBBs in rewindHeaderState Feb 24, 2020
@nfrisby nfrisby added the WIP Do not merge, work in progress. label Feb 24, 2020
@nfrisby nfrisby removed the WIP Do not merge, work in progress. label Feb 25, 2020
@nfrisby nfrisby changed the title DO NOT MERGE consensus: handle EBBs in rewindHeaderState consensus: handle EBBs in rewindHeaderState Feb 25, 2020
, headerStateAnchor = headerStateAnchor
}
where
rolledBack :: AnnTip blk -> Bool
rolledBack t = annTipPoint t > p
rollBack :: AnnTip blk -> Bool
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name change was accidental, but doesn't seem worth another force push :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you see #1690 (comment)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did now. I've adopted it.

@edsko
Copy link
Contributor

edsko commented Feb 25, 2020

@nfrisby feel free to bors this once you feel confident it solves the problem :)

@nfrisby
Copy link
Contributor Author

nfrisby commented Feb 25, 2020

My big update to PR #1506 is still a work-in-progress, but I do think it is "feature complete" with respect to PBFT and RealPBFT. In particular the commit on this PR passed hundreds of thousands of tests (though I can't be sure how many exercised it), including a couple repros derived from this PR's Issue.

So I'm going to merge. Thanks.

@nfrisby
Copy link
Contributor Author

nfrisby commented Feb 25, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 25, 2020

@iohk-bors iohk-bors bot merged commit 3d2f492 into master Feb 25, 2020
@iohk-bors iohk-bors bot deleted the nfrisby/issue-1690 branch February 25, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug in rewindHeaderState triggered by EBBs
2 participants