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

[mtg-578] Add both restrict and allow claim instructions with tests #28

Merged
merged 11 commits into from
Sep 3, 2024

Conversation

kstepanovdev
Copy link
Collaborator

No description provided.

@kstepanovdev kstepanovdev force-pushed the penalties/block-the-clam-operation branch 2 times, most recently from 20d7cb5 to f32a952 Compare August 30, 2024 11:58
@kstepanovdev kstepanovdev requested review from StanChe and rwwwx August 30, 2024 12:17
@kstepanovdev kstepanovdev self-assigned this Aug 30, 2024
@kstepanovdev kstepanovdev force-pushed the penalties/block-the-clam-operation branch from f32a952 to 7617203 Compare August 30, 2024 14:46
@kstepanovdev kstepanovdev force-pushed the penalties/block-the-clam-operation branch from 7617203 to 1750838 Compare August 30, 2024 14:51
Copy link
Contributor

@StanChe StanChe left a comment

Choose a reason for hiding this comment

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

We're restricting the reward claims, and not withdrawals here? I had an impression we were planning to restrict withdrawals.

use anchor_lang::prelude::*;
use mplx_staking_states::error::MplStakingError;

/// Restricts claiming rewards from the specified mining account.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Restricts claiming rewards from the specified mining account.
/// Allows claiming rewards from the specified mining account.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My idea was to restrict claiming and withdraws at the same time because you can't withdraw if you have some unclaimed rewards. Albeit, a user might have no rewards and still violate the rules. Presumably, restricting withdrawal (while still restricting claiming) is a more sensible approach. wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, the logic is good. The comments are a bit off and need fixing

Copy link
Collaborator Author

@kstepanovdev kstepanovdev Sep 2, 2024

Choose a reason for hiding this comment

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

I've committed your suggestion, haven't I? As well as renamed methods completely to remove ambiguity.

@StanChe
Copy link
Contributor

StanChe commented Sep 2, 2024

Got it with the restrictions. It makes sense to update comments to reflect the restriction applies to both claiming the rewards and withdrawing funds.

@kstepanovdev kstepanovdev changed the title [mtg-578] Add both restrit and allow claim instructions with tests [mtg-578] Add both restrict and allow claim instructions with tests Sep 2, 2024
@kstepanovdev kstepanovdev requested a review from StanChe September 2, 2024 12:36
@kstepanovdev kstepanovdev merged commit a739a3c into master Sep 3, 2024
2 checks passed
@kstepanovdev kstepanovdev deleted the penalties/block-the-clam-operation branch September 3, 2024 08:21
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.

2 participants