-
Notifications
You must be signed in to change notification settings - Fork 261
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 #20
spec updates #20
Conversation
* move consts to top * move some functions to validator.nim * add some validator functoin smoke tests
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.
LGTM, pending our type janitor @tersec.
Regarding attempting to call undeclared routine init
, this comes nimcrypto when Nim tries to resolve the symbol in digest too early. The rootcause is that the proc is a generic proc and symbol resolution is too early in generics. See also the meta Nim issue: nim-lang/Nim#8677.
Regarding odd shuffling, tracked in #1
beacon_chain/private/helpers.nim
Outdated
# Re-hash the `source` to obtain a new pattern of bytes | ||
source = blake2_256.digest source.data | ||
# Iterate through the `source` bytes in 3-byte chunks | ||
# XXX "attempting to call undeclared routine init" |
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.
You probably need a mixin init
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.
Cf: https://github.com/cheatfate/nimcrypto/blob/master/nimcrypto/hash.nim#L60-L80 I think Nim tries to resolve the symbol too early (nim-lang/Nim#8677)
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.
mixin init
here doesn't seem to help - added a comment for now, will dig into it later
status: ACTIVE | ||
), 1024) | ||
|
||
# XXX the shuffling looks really odd, probably buggy |
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.
Repurposed #1 (comment) for shuffling test implementations using Sigp tests as reference - https://github.com/sigp/lighthouse/blob/ba548e49a52687a655c61b443b6835d79c6d4236/beacon_chain/validator_shuffling/src/shuffle.rs
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.
thanks! added links to code, to keep track
* link to resources for shuffling * describe get_beacon_proposer * hints for solving init issue
* Update file headers, copyright date * Rename files and hopefully fix nimble * Forgot to change path in tests * Update readme
random stuff encountered while reading spec