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.

Sign up for GitHub

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

Jump to bottom

Prevent staking to a retired pool. #1913

Merged
merged 26 commits into from
Jul 23, 2020
Merged

Prevent staking to a retired pool. #1913

merged 26 commits into from
Jul 23, 2020

Conversation

jonathanknowles
Copy link
Member

@jonathanknowles jonathanknowles commented Jul 16, 2020 •
edited
Loading

Issue Number

#1853

Overview

This PR:

  • Adjusts joinStakePool to return ErrNoSuchPool if the current epoch is later than or equal to the retirement epoch of the specified pool.

Comments

Integration tests specifically designed to test the rejection mechanism are not included in this PR, but will be tackled in a future PR. (See QA section of issue #1853.)

However, existing tests already verify that it is possible to stake to a non-retired pool, so we can be fairly confident we are not breaking existing functionality.

@jonathanknowles

This comment has been minimized.

Sign in to view
@jonathanknowles jonathanknowles self-assigned this Jul 16, 2020
iohk-bors bot added a commit that referenced this pull request Jul 16, 2020
@iohk-bors
Try #1913:
b2e04cb
@jonathanknowles

This comment has been minimized.

Sign in to view
@jonathanknowles

This comment has been minimized.

Sign in to view
iohk-bors bot added a commit that referenced this pull request Jul 16, 2020
@iohk-bors
Try #1913:
35344f1
@iohk-bors

This comment has been minimized.

Sign in to view
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/prevent-joining-retired-stake-pool branch from 99db45a to b361989 Compare July 16, 2020 06:17
@jonathanknowles

This comment has been minimized.

Sign in to view
iohk-bors bot added a commit that referenced this pull request Jul 16, 2020
@iohk-bors
Try #1913:
ad364a4
@jonathanknowles

This comment has been minimized.

Sign in to view
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/prevent-joining-retired-stake-pool branch from b361989 to cba0943 Compare July 16, 2020 06:19
@jonathanknowles

This comment has been minimized.

Sign in to view
iohk-bors bot added a commit that referenced this pull request Jul 16, 2020
@iohk-bors
Try #1913:
9f931ce
@iohk-bors

This comment has been minimized.

Sign in to view
@jonathanknowles

This comment has been minimized.

Sign in to view
iohk-bors bot added a commit that referenced this pull request Jul 16, 2020
@iohk-bors
Try #1913:
e8d5c4e
@jonathanknowles jonathanknowles requested a review from rvl July 16, 2020 07:03
@jonathanknowles jonathanknowles marked this pull request as ready for review July 16, 2020 07:04
@iohk-bors

This comment has been minimized.

Sign in to view
@jonathanknowles

This comment has been minimized.

Sign in to view
iohk-bors bot added a commit that referenced this pull request Jul 16, 2020
@iohk-bors
Try #1913:
a4708e6
@jonathanknowles jonathanknowles mentioned this pull request Jul 16, 2020
Closed
2 tasks
@jonathanknowles jonathanknowles added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Jul 16, 2020
@jonathanknowles jonathanknowles changed the title WIP: Prevent staking to an already-retired staking pool. Prevent staking to an already-retired staking pool. Jul 16, 2020
@iohk-bors

This comment has been minimized.

Sign in to view
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/prevent-joining-retired-stake-pool branch from a214464 to d8c9230 Compare July 16, 2020 08:53
rvl
rvl reviewed Jul 16, 2020
View reviewed changes
Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

Is there a reason that we track retired pools? I don't think we would need info about pools which can't be delegated to. Shouldn't we purge them from the database, and return "pool not found" in this case?

@jonathanknowles jonathanknowles force-pushed the jonathanknowles/prevent-joining-retired-stake-pool branch from 8ba9e1e to fd3fe61 Compare July 22, 2020 09:25
@jonathanknowles jonathanknowles changed the title Prevent staking to an already-retired staking pool. Prevent staking to a retired pool. Jul 22, 2020
@jonathanknowles
Copy link
Member Author

jonathanknowles commented Jul 22, 2020 •
edited
Loading

  1. What error should we return if a user attempts to delegate to a pool that has already retired, but has not yet been garbage collected?

For the record, we decided to return ErrNoSuchPool instead of ErrPoolAlreadyRetired.

Fixed in 76b4ab1.

@jonathanknowles jonathanknowles requested a review from rvl July 22, 2020 09:58
@jonathanknowles

This comment has been minimized.

Sign in to view
iohk-bors bot added a commit that referenced this pull request Jul 23, 2020
@iohk-bors
Try #1913:
0bbba70
@jonathanknowles

This comment has been minimized.

Sign in to view
iohk-bors bot added a commit that referenced this pull request Jul 23, 2020
@iohk-bors @jonathanknowles
Merge #1913
00a6804 
1913: Prevent staking to a retired pool. r=jonathanknowles a=jonathanknowles

# Issue Number

#1853

# Overview

This PR:

- [x] Adjusts `joinStakePool` to return `ErrNoSuchPool` if the current epoch is _later than or equal to_ the retirement epoch of the specificied pool.

# Comments

Integration tests are **not** included in this PR, but will be tackled in a future PR. (See integration tests described in Issue #1853.)

Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>
@iohk-bors

This comment has been minimized.

Sign in to view
@jonathanknowles

This comment has been minimized.

Sign in to view
@iohk-bors

This comment has been minimized.

Sign in to view
@iohk-bors

This comment has been minimized.

Sign in to view
@jonathanknowles
Copy link
Member Author

bors retry

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 23, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 45bff77 into master Jul 23, 2020
@iohk-bors iohk-bors bot deleted the jonathanknowles/prevent-joining-retired-stake-pool branch July 23, 2020 07:52
@jonathanknowles jonathanknowles mentioned this pull request Jul 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvl rvl rvl approved these changes

@Anviking Anviking Awaiting requested review from Anviking

Assignees

@jonathanknowles jonathanknowles

Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Projects
None yet
Milestone
(ADP-313) Track Stakepool Retirements
Development

Successfully merging this pull request may close these issues.
3 participants
@jonathanknowles @Anviking @rvl