-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
spec updates #1263
Conversation
3948484
to
95b81db
Compare
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. |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarification suggestions
docs/spec/slashing/end_block.md
Outdated
|
||
Currently, such messages include only the following: | ||
|
||
- prevotes by the same validator for more than one BlockID at the same |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
docs/spec/slashing/end_block.md
Outdated
- 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 |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
basically yes. will clarify
6821f6b
to
2219c3b
Compare
Codecov Report
@@ Coverage Diff @@
## develop #1263 +/- ##
========================================
Coverage 65.26% 65.26%
========================================
Files 102 102
Lines 5519 5519
========================================
Hits 3602 3602
Misses 1708 1708
Partials 209 209 |
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.