Skip to content
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

Move all waiting list related code from staking.go to stakingWaitingList.go #3866

Merged
merged 12 commits into from Mar 21, 2022

Conversation

mariusmihaic
Copy link
Contributor

@mariusmihaic mariusmihaic commented Mar 2, 2022

  • All code from staking.go which is related to the staking queue/waiting list, which is going to be obsolete since staking v4 has been moved to a new file: stakingWaitingList.go
  • No code has been changed/altered

@mariusmihaic mariusmihaic changed the title FEAT: Move all waiting list code from staking.go Move all waiting list related code from staking.go to stakingWaitingList.go Mar 2, 2022
@mariusmihaic mariusmihaic self-assigned this Mar 3, 2022
@mariusmihaic mariusmihaic marked this pull request as ready for review March 11, 2022 14:11
@ssd04 ssd04 self-requested a review March 16, 2022 11:48
@ssd04
Copy link
Contributor

ssd04 commented Mar 16, 2022

there seems to be some tests that are directly related to waitingList, and i think they can be moved to another test file, or maybe you already planned to handle unit tests (if needed) in another PR

ssd04
ssd04 previously approved these changes Mar 16, 2022
return vmcommon.Ok
}

func (s *stakingSC) getTotalNumberOfRegisteredNodes(args *vmcommon.ContractCallInput) vmcommon.ReturnCode {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not old code - this can be moved back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, this is not old, it just used the waiting list, but it is not legacy. Moved it back

return vmcommon.Ok
}

func (s *stakingSC) unStake(args *vmcommon.ContractCallInput) vmcommon.ReturnCode {
Copy link
Contributor

Choose a reason for hiding this comment

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

unStake should be in the base file - not here.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe make a refactor - unStakeV1 and unStakeV2 -> and call them depending on flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion with unStakeV1 & unStakeV2. Applied it !

return nil
}

func (s *stakingSC) unStakeAtEndOfEpoch(args *vmcommon.ContractCallInput) vmcommon.ReturnCode {
Copy link
Contributor

Choose a reason for hiding this comment

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

this again should be in the base file - this is not legacy - this will work with stakingV4 as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, when I started this refactor, I just had in mind "files with/without waiting list", not legacy. Will move it back

afterLastJailed bool
}

func (s *stakingSC) processStake(blsKey []byte, registrationData *StakedDataV2_0, addFirst bool) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

make processStakeV1 and V2 - one with legacy only with v4 in mind. in v4 you need to set the registernonce - line31 and the last 2 lines - like 51 and 52.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, thank you, applied it !

Base automatically changed from EN-116622-ignore-staking-queue-v1 to feat/liquid-staking March 18, 2022 11:29
return nil
}

if !s.flagStakingV4.IsSet() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not needed - this is duplicated check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, deleted it, thank you

@mariusmihaic mariusmihaic merged commit 837d416 into feat/liquid-staking Mar 21, 2022
@mariusmihaic mariusmihaic deleted the move-waiting-list-from-staking branch March 21, 2022 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants