Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

update/0.4.0 #59

Merged
merged 8 commits into from
Feb 26, 2019
Merged

update/0.4.0 #59

merged 8 commits into from
Feb 26, 2019

Conversation

decanus
Copy link
Collaborator

@decanus decanus commented Feb 25, 2019

No description provided.

Copy link

@gakonst gakonst left a 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))
Copy link

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)
Copy link

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

Copy link
Collaborator Author

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
Copy link

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?

Copy link

Choose a reason for hiding this comment

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

Seems moderately inconsistent throughout

Copy link
Collaborator Author

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

@djrtwo
Copy link

djrtwo commented Feb 25, 2019

whatup @gakonst

@decanus decanus merged commit 59a0641 into master Feb 26, 2019
@decanus decanus deleted the update/0.4.0 branch February 26, 2019 02:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants