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.

Sign up for GitHub

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

Update dependencies #1542

Merged
merged 3 commits into from
Jan 31, 2020
Merged

Update dependencies #1542

merged 3 commits into from
Jan 31, 2020

Conversation

erikd
Copy link
Contributor

@erikd erikd commented Jan 30, 2020

No description provided.

dcoutts
dcoutts reviewed Jan 30, 2020
View reviewed changes
ouroboros-consensus/src/Ouroboros/Consensus/Protocol/Praos.hs
Just signature -> do
-- TODO : We should not update the key on each signing, but X slots
-- (for configurable param X)
newKey <- fromMaybe (error "mkOutoborosPayload: updateKES failed") <$> updateKES () keyKES
Copy link
Contributor

Choose a reason for hiding this comment

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

@nc6 can you have a look at this. The KES API has been fixed and so this bit here needs updating. We need: 1. to separate key evolution from block signing, we only evolve the key based on slots (slots equivalent to 24 hrs in Shelley), and 2. KES evolutions are finite, when we run out we can not longer sign blocks, and we need a new operational cert.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this is the mock Praos implementation, not the real one.

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 have no idea what I am doing this far down the stack.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, of course, I'd not expect anyone to be able to apply a real update in the underlying crypto interface to make changes to a specific (and non-trivial) consensus protocol :)

dcoutts
dcoutts approved these changes Jan 30, 2020
View reviewed changes
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

Will need updates in the Praos code for the KES API changes, but that's still work in progress, so this is fine.

@erikd erikd force-pushed the erikd/deps-20200131 branch from 7574c05 to f3be3ae Compare January 31, 2020 00:06
@erikd erikd mentioned this pull request Jan 31, 2020
Merged
@erikd erikd force-pushed the erikd/deps-20200131 branch from f3be3ae to 3bb910d Compare January 31, 2020 00:54
edsko
edsko approved these changes Jan 31, 2020
View reviewed changes
Copy link
Contributor

@edsko edsko left a comment

Choose a reason for hiding this comment

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

Pushed a commit that disables the Praos tests for now. We need to re-enable them of course but they are not needed for Byron and we need to unblock this.

@edsko
Copy link
Contributor

edsko commented Jan 31, 2020

Created a ticket for re-enabling these tests at #1545 .

@edsko
Copy link
Contributor

edsko commented Jan 31, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jan 31, 2020
@iohk-bors @erikd @edsko
Merge #1542
d38e10b 
1542: Update dependencies r=edsko a=erikd



Co-authored-by: Erik de Castro Lopo <erikd@mega-nerd.com>
Co-authored-by: Edsko de Vries <edsko@well-typed.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jan 31, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 1ca4a94 into master Jan 31, 2020
@iohk-bors iohk-bors bot deleted the erikd/deps-20200131 branch January 31, 2020 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edsko edsko edsko approved these changes

@dcoutts dcoutts dcoutts approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@erikd @edsko @dcoutts