-
Notifications
You must be signed in to change notification settings - Fork 4
Conversation
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
|
||
let shuffledActiveValidatorIndices = activeValidatorIndices.map { | ||
activeValidatorIndices[getPermutedIndex(index: Int($0), listSize: activeValidatorIndices.count, seed: seed)] | ||
} | ||
|
||
return shuffledActiveValidatorIndices.split(count: committeesPerEpoch) | ||
return shuffledActiveValidatorIndices.split(count: getEpochCommitteeCount(activeValidatorCount: validators.count)) |
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.
Should use the active validator count when getting the committee count
proofOfPossession: deposit.depositData.depositInput.proofOfPossession, | ||
withdrawalCredentials: deposit.depositData.depositInput.withdrawalCredentials | ||
) | ||
processDeposit(state: &state, deposit: deposit) |
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.
asking why process_deposit
wasn't just passing in deposit
was maybe the first thing you asked me about 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.
indeed it was, and i changed it twice in my code
@@ -31,5 +31,5 @@ struct BeaconState { | |||
|
|||
var latestEth1Data: Eth1Data | |||
var eth1DataVotes: [Eth1DataVote] | |||
let depositIndex: UInt64 | |||
var depositIndex: UInt64 |
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.
Why sometimes var
and sometimes let
?
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.
Seems moderately inconsistent throughout
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.
let
can't be mutated
whatup @gakonst |
No description provided.