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

spec updates #1263

Merged
merged 7 commits into from
Jun 16, 2018
Merged

spec updates #1263

merged 7 commits into from
Jun 16, 2018

Conversation

ebuchman
Copy link
Member

just opening PR for the changes we made today going through the spec with @cwgoes and @rigelrozanski and @liamsi

this is rough notes, should be fixed up so we give proper names instead of mapX, and better sentences, etc.

@ebuchman ebuchman added the wip label Jun 15, 2018
@ebuchman ebuchman force-pushed the bucky/staking-spec-updates branch 3 times, most recently from 3948484 to 95b81db Compare June 15, 2018 09:47
@ebuchman ebuchman changed the title WIP spec updates spec updates Jun 15, 2018
@ebuchman
Copy link
Member Author

Ok, cleaned this up.

The key/value pairs in the store are much clearer now.

Also removed a bunch of unecessary fields from structs (ie. fields that are part of the keys) and other fields that we don't need anymore (ie. because we do active slashing by iterating). Also clarified what some indices are used for.

@ebuchman
Copy link
Member Author

One thing to note is the key prefixes are a little scattered and could probably use cleanup.

Also there are keys in the keys.go file that are not reflected in the spec. At least TendermintUpdates Key can be removed. Not sure about Cliff related keys though ...

So seems like we still need to do a take through the codebase and ensure everything (and only these things) that are persisted in a store are reflected correctly in the spec

@ebuchman ebuchman mentioned this pull request Jun 15, 2018
1 task
Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Clarification suggestions


Currently, such messages include only the following:

- prevotes by the same validator for more than one BlockID at the same
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we either define BlockID here or reference the Tendermint spec? Block ID = app hash + header info?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep lets reference the TM spec

- precommits by the same validator for more than one BlockID at the same
Height and Round

We call any such pair of conflicting votes `Evidence`. Full nodes in the network prioritize the
Copy link
Contributor

Choose a reason for hiding this comment

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

Do conflicting precommits and conflicting prevotes have the same effect on the safety of the protocol? (we should explain why we treat them as equivalent for the purpose of slashing)

Copy link
Member Author

Choose a reason for hiding this comment

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

basically yes. will clarify

@cwgoes cwgoes mentioned this pull request Jun 16, 2018
6 tasks
@ebuchman ebuchman changed the base branch from rigel/unbonding to develop June 16, 2018 07:20
@codecov
Copy link

codecov bot commented Jun 16, 2018

Codecov Report

Merging #1263 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #1263   +/-   ##
========================================
  Coverage    65.26%   65.26%           
========================================
  Files          102      102           
  Lines         5519     5519           
========================================
  Hits          3602     3602           
  Misses        1708     1708           
  Partials       209      209

@ebuchman ebuchman merged commit d083287 into develop Jun 16, 2018
@ebuchman ebuchman deleted the bucky/staking-spec-updates branch June 16, 2018 23:55
cwgoes added a commit that referenced this pull request Jun 30, 2018
Implement semifinal Gaia slashing spec (#1263), less #1348, #1378, and #1440 which are TBD.
adrianbrink pushed a commit that referenced this pull request Jul 2, 2018
Implement semifinal Gaia slashing spec (#1263), less #1348, #1378, and #1440 which are TBD.
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