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

Jump to bottom

Use BlockNo for Byron transition #2626

Merged
merged 2 commits into from Sep 28, 2020
Merged

Use BlockNo for Byron transition #2626

merged 2 commits into from Sep 28, 2020

Conversation

edsko
Copy link
Contributor

@edsko edsko commented Sep 22, 2020

The first part of this PR is #2621.

This PR proper addresses #2455. In particular, the change in computation is justified theoretically in #2455 (comment) . Comparing the log before and after this PR confirms that it does what that comment set out to do. Before, as mentioned in #2455 (comment) ,

BlockNo  SlotNo   Event
--------------------------------------------------------------------------------
4469128  4471417  2.0.0 registered
4469130  4471419  2.0.0 confirmed
4473450  4475739  2.0.0 stably confirmed, endorsements: 0
4476955  4479244  2.0.0 stably confirmed, endorsements: 1
4476960  4479249  2.0.0 stably confirmed, endorsements: 2
4476964  4479253  2.0.0 stably confirmed, endorsements: 3
4476965  4479254  2.0.0 candidate
4481285  4483574  2.0.0 stable candidate, scheduled for 208
                  Hard fork from Byron to Shelley confirmed for epoch 208
4490511  4492800  Hard fork from Byron to Shelley complete

after

BlockNo  SlotNo   Event
--------------------------------------------------------------------------------
4469128  4471417  2.0.0 registered
4469130  4471419  2.0.0 confirmed
4473450  4475739  2.0.0 stably confirmed; endorsements: 0
4476955  4479244  2.0.0 stably confirmed; endorsements: 1
4476960  4479249  2.0.0 stably confirmed; endorsements: 2
4476964  4479253  2.0.0 stably confirmed; endorsements: 3
4476965  4479254  2.0.0 candidate; tentatively scheduled for epoch 208
4479125  4481414  2.0.0 hard fork from Byron to Shelley confirmed for epoch 208
4481285  4483574  2.0.0 stable candidate, scheduled for epoch 208
4481285  4492800  hard fork from Byron to Shelley complete

Notice that the only difference is that the HFC is already informed about the hard fork in block 4479125, wich

4479125 - 4476965 = 2160

indeed is k blocks after the proposal became a candidate, and hence is stable.

(I think checking for k, rather than k + 1 is correct: if we set k = 0, no rollback is possible at all, so the moment we see a block adopted that has the candidate in it, we know it will happen.)

@edsko edsko requested a review from mrBliss September 22, 2020 10:14
@edsko edsko mentioned this pull request Sep 22, 2020
Merged
mrBliss
mrBliss approved these changes Sep 28, 2020
View reviewed changes
Copy link
Contributor

@mrBliss mrBliss left a comment

Choose a reason for hiding this comment

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

Nice

@mrBliss mrBliss added the consensus issues related to ouroboros-consensus label Sep 28, 2020
@mrBliss mrBliss linked an issue Sep 28, 2020 that may be closed by this pull request
Byron should use block no, not slot no, to decide whether or not to report transition to HFC #2455
Closed
@edsko
Have db-analyser log to stderr, and flush buffer
dede2db
@edsko
Use BlockNo for stability of B-to-S transition
b578de2 
In order to be able to do this, we must record some additional HFC-required
transition into in the Byron state.

Closes #2455
@edsko edsko force-pushed the edsko/use-blockno-for-byron-transition branch from be44baa to b578de2 Compare September 28, 2020 13:18
@edsko
Copy link
Contributor Author

edsko commented Sep 28, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 28, 2020

Build succeeded:

@iohk-bors iohk-bors bot merged commit bbbe59c into master Sep 28, 2020
@iohk-bors iohk-bors bot deleted the edsko/use-blockno-for-byron-transition branch September 28, 2020 13:32
iohk-bors bot added a commit that referenced this pull request Sep 28, 2020
@iohk-bors @edsko
Merge #2627
66195ab 
2627: InspectLedger instance for Shelley r=edsko a=edsko

This PR include #2621 and #2626.

Replaces #2508. 

Co-authored-by: Edsko de Vries <edsko@well-typed.com>
iohk-bors bot added a commit that referenced this pull request Sep 28, 2020
@iohk-bors @edsko
Merge #2627
aa723b8 
2627: InspectLedger instance for Shelley r=edsko a=edsko

This PR include #2621 and #2626.

Replaces #2508. 

Co-authored-by: Edsko de Vries <edsko@well-typed.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrBliss mrBliss mrBliss approved these changes

Assignees
No one assigned
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Byron should use block no, not slot no, to decide whether or not to report transition to HFC
2 participants
@edsko @mrBliss