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

update/0.3.0 #58

Merged
merged 17 commits into from
Feb 25, 2019
Merged

update/0.3.0 #58

merged 17 commits into from
Feb 25, 2019

Conversation

decanus
Copy link
Collaborator

@decanus decanus commented Feb 24, 2019

No description provided.

@decanus decanus closed this Feb 24, 2019
@decanus decanus reopened this Feb 24, 2019
Copy link

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

Did a very quick read. Looks good!
Left a couple of comments. only potential bug was the withdrawal credentials range

func isActive(epoch: EpochNumber) -> Bool {
return self.activationEpoch <= epoch && epoch < self.exitEpoch
func isActive(epoch: Epoch) -> Bool {
return activationEpoch <= epoch && epoch < exitEpoch
Copy link

Choose a reason for hiding this comment

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

Fancy. Don't need self.?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, art.

@@ -254,17 +254,17 @@ extension StateTransition {
}

static func exits(state: inout BeaconState, block: BeaconBlock) {
Copy link

Choose a reason for hiding this comment

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

Might consider renaming voluntary_exits


assert(state.slot == transfer.slot)
assert(BeaconChain.getCurrentEpoch(state: state) >= state.validatorRegistry[Int(transfer.from)].withdrawableEpoch)
// @todo does this work (-1)?
Copy link

Choose a reason for hiding this comment

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

Does it?

Copy link

Choose a reason for hiding this comment

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

If it does work, wouldn't it be 1 ..<= -1 so you include the last element?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -297,7 +340,7 @@ extension StateTransition {
extension StateTransition {

static func processEpoch(state: inout BeaconState) {
assert(state.slot + 1 % EPOCH_LENGTH == 0) // @todo not sure if this should be here
assert(state.slot + 1 % SLOTS_PER_EPOCH == 0) // @todo not sure if this should be here
Copy link

Choose a reason for hiding this comment

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

You should only call this function if that condition holds. Seems a little redundant to assert it here but not necessarily incorrect

@decanus decanus merged commit 393ea82 into master Feb 25, 2019
@decanus decanus deleted the update/0.3.0 branch February 25, 2019 18:08
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.

2 participants