-
Notifications
You must be signed in to change notification settings - Fork 26
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
refactor: Validator Staking Adjustments #458
Conversation
WalkthroughThe changes enhance the Validator Staking functionality by introducing the ability to specify the amount for unstaking tokens and enabling rewards to be sent to other ADOs using the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- contracts/finance/andromeda-validator-staking/src/contract.rs (9 hunks)
- contracts/finance/andromeda-validator-staking/src/mock.rs (5 hunks)
- contracts/finance/andromeda-validator-staking/src/testing/tests.rs (3 hunks)
- packages/andromeda-finance/src/validator_staking.rs (2 hunks)
- tests-integration/tests/validator_staking.rs (8 hunks)
Additional comments not posted (12)
packages/andromeda-finance/src/validator_staking.rs (2)
23-23
: The addition of theamount
parameter to theUnstake
variant is consistent with the PR objectives and allows users to specify the amount to unstake.
27-27
: The change of therecipient
type in theClaim
variant fromAndrAddr
toRecipient
is consistent with the PR objectives and allows rewards to be sent to another ADO.contracts/finance/andromeda-validator-staking/src/mock.rs (2)
98-99
: The addition of theamount
parameter to themock_execute_unstake
function is consistent with the changes in theExecuteMsg
enum and ensures that the mock function reflects the new functionality.
104-104
: The change of therecipient
type in themock_execute_claim_reward
function fromAndrAddr
toRecipient
is consistent with the changes in theExecuteMsg
enum and ensures that the mock function reflects the new functionality.contracts/finance/andromeda-validator-staking/src/testing/tests.rs (2)
148-148
: The addition of theamount
parameter to theExecuteMsg::Unstake
message in thetest_unauthorized_unstake
test case is consistent with the changes in theExecuteMsg
enum and ensures that the test case reflects the new functionality.
173-173
: The change of therecipient
type in theExecuteMsg::Claim
message fromAndrAddr
toRecipient
in thetest_unauthorized_claim
test case is consistent with the changes in theExecuteMsg
enum and ensures that the test case reflects the new functionality.contracts/finance/andromeda-validator-staking/src/contract.rs (4)
82-82
: The addition of theamount
parameter to theExecuteMsg::Unstake
variant is consistent with the PR objectives and allows users to specify the amount to unstake.
202-202
: The change of therecipient
type in theExecuteMsg::Claim
variant fromAndrAddr
toRecipient
is consistent with the PR objectives and allows rewards to be sent to another ADO.
Line range hint
142-192
: The update to theexecute_unstake
function to handle theamount
parameter ensures that the function properly handles the new parameter, allowing users to specify the amount to unstake. The logic includes validation to ensure the amount is not zero and does not exceed the staked amount.
Line range hint
202-260
: The update to theexecute_claim
function to handle theRecipient
type ensures that the function properly handles the new type, allowing rewards to be sent to another ADO. The logic includes validation to ensure the recipient is the contract owner.tests-integration/tests/validator_staking.rs (2)
316-326
: The addition of theamount
parameter to theexecute_unstake
function calls in the integration tests is consistent with the changes in theExecuteMsg
enum and ensures that the integration tests reflect the new functionality.
99-99
: The change of therecipient
type in theexecute_claim_reward
function calls fromAndrAddr
toRecipient
is consistent with the changes in theExecuteMsg
enum and ensures that the integration tests reflect the new functionality.
if let Some(recipient) = recipient { | ||
if recipient.msg.is_some() { | ||
res.clone() | ||
.add_submessage(recipient.generate_direct_msg(&deps.as_ref(), info.funds)?); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the amount being sent here not be the amount that is being claimed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The more I think about this the more difficult I think this implementation might be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a discussion about that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's go ahead and undo it, we can come back to this in a future release. Keep it as AndrAddr
for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- contracts/finance/andromeda-validator-staking/src/contract.rs (9 hunks)
Files skipped from review as they are similar to previous changes (1)
- contracts/finance/andromeda-validator-staking/src/contract.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- contracts/finance/andromeda-validator-staking/src/contract.rs (7 hunks)
- contracts/finance/andromeda-validator-staking/src/mock.rs (3 hunks)
- contracts/finance/andromeda-validator-staking/src/testing/tests.rs (1 hunks)
- packages/andromeda-finance/src/validator_staking.rs (2 hunks)
- tests-integration/tests/validator_staking.rs (5 hunks)
Files skipped from review as they are similar to previous changes (5)
- contracts/finance/andromeda-validator-staking/src/contract.rs
- contracts/finance/andromeda-validator-staking/src/mock.rs
- contracts/finance/andromeda-validator-staking/src/testing/tests.rs
- packages/andromeda-finance/src/validator_staking.rs
- tests-integration/tests/validator_staking.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional comments not posted (1)
CHANGELOG.md (1)
17-17
: The changelog entry accurately reflects the new feature added by the PR and is correctly linked to the PR. Good job on maintaining clear and precise documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Motivation
Resolves: #453
Implementation
Users now can specify the amount of funds they'd like to unstake.
Testing
An integration test was created for unstaking a specific amount.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests