-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: create route to withdraw rewards #146
Conversation
Warning Review failedThe pull request is closed. WalkthroughThis update introduces the ability to claim rewards within the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrokerStakingContract
participant RewardsDistribution
User ->> BrokerStakingContract: Execute ClaimRewards
BrokerStakingContract ->> RewardsDistribution: Request Rewards Distribution
RewardsDistribution -->> BrokerStakingContract: Distribute Rewards
BrokerStakingContract -->> User: Acknowledge Claim
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 (
|
deps.querier.query_all_delegations(&env.contract.address)?; | ||
|
||
let mut messages: Vec<CosmosMsg> = vec![]; | ||
for delegation in delegations.iter() { |
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.
There's a CLI command to do withdraw all but it's internally just doing this logic. There's no actual message to withdraw from all delegations in the comsos sdk
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 Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
artifacts/broker_bank.wasm
is excluded by!**/*.wasm
,!**/*.wasm
artifacts/broker_staking.wasm
is excluded by!**/*.wasm
,!**/*.wasm
Files selected for processing (4)
- artifacts/checksums.txt (1 hunks)
- contracts/broker-staking/src/contract.rs (2 hunks)
- contracts/broker-staking/src/msg.rs (1 hunks)
- contracts/broker-staking/src/testing.rs (2 hunks)
Additional comments not posted (6)
artifacts/checksums.txt (1)
1-2
: Checksum updates forbroker_bank.wasm
andbroker_staking.wasm
are aligned with the PR objectives.contracts/broker-staking/src/msg.rs (1)
34-34
: The addition of theClaimRewards
variant to theExecuteMsg
enum correctly implements the new feature for withdrawing rewards.contracts/broker-staking/src/contract.rs (2)
62-62
: Correctly integrated handling forClaimRewards
in theexecute
function.
66-92
: Theclaim_rewards
function is correctly implemented to allow operators to claim delegator rewards.contracts/broker-staking/src/testing.rs (2)
7-8
: Correctly replacedDistributionMsg
withStakingMsg
to align with the changes in the contract logic.
756-831
: Thetest_withdraw_rewards
function correctly tests the newClaimRewards
functionality under various scenarios.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
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/broker-staking/src/testing.rs (2 hunks)
Additional comments not posted (1)
contracts/broker-staking/src/testing.rs (1)
757-831
: Comprehensive test coverage forClaimRewards
.The new test function
test_withdraw_rewards
is well-structured and covers various scenarios for claiming rewards, including success and failure cases for different senders. This ensures that theClaimRewards
functionality is thoroughly tested.However, consider adding the following improvements:
- Edge Cases: Include tests for edge cases such as when there are no rewards to claim or when the contract is halted.
- Assertions: Ensure that the state changes (e.g., rewards balance) are asserted after the operation to verify the correctness of the functionality.
Example:
// Edge case: No rewards to claim TestCaseExec { to_addrs: to_addrs.to_vec(), opers: opers.to_vec(), sender: "owner", exec_msg: ExecuteMsg::ClaimRewards {}, err: Some("no rewards to claim"), contract_funds_start: None, resp_msgs: vec![], }, // Edge case: Contract is halted IS_HALTED.save(deps.as_mut().storage, &true)?; let res = execute(deps.as_mut(), env.clone(), info, ExecuteMsg::ClaimRewards {}); assert!(res.is_err(), "contract is halted");
Summary by CodeRabbit
New Features
Tests