`claim_staking_rewards` extrinsic #2080

shannonwells · 2024-07-15T22:52:09Z

Goal

The goal of this PR is to implement the claim_staking_rewards extrinsic, its benchmarks, and tests.

Closes #1970

Discussion

Also removes a trait function that was complicating things and didn't really belong there.

Checklist

Design doc(s) updated
Tests added
Benchmarks added
Weights updated

…l refactor

shannonwells · 2024-07-15T23:28:25Z

pallets/capacity/src/tests/change_staking_target_tests.rs

@@ -40,7 +40,7 @@ fn do_retarget_happy_path() {
 		let from_amount = 600u64;
 		let to_amount = 300u64;
 		let to_msa: MessageSourceId = 2;
-		let staking_type = ProviderBoost;
+		let staking_type = StakingType::ProviderBoost;


This was about silencing linter warnings.

shannonwells · 2024-07-16T23:41:41Z

pallets/capacity/src/benchmarking.rs

-			assert_ok!(new_chunk.try_insert(era.into(), (1000u32 * era).into()));
-		}
-		ProviderBoostRewardPools::<T>::set(i, Some(new_chunk));
+fn fill_reward_pool_chunks<T: Config>(current_era: RewardEra) {


The previous version of this function, while it worked for the provider_boost benchmark, did not correctly fill up the reward_pool_chunks, so it caused an EraOutOfRange error in the claim_staking_reward benchmark.

shannonwells · 2024-07-16T23:42:54Z

pallets/capacity/src/lib.rs

@@ -402,6 +410,10 @@ pub mod pallet {
 		MaxRetargetsExceeded,
 		/// Tried to exceed bounds of a some Bounded collection
 		CollectionBoundExceeded,
+		/// This origin has nothing staked for ProviderBoost.
+		NotAProviderBoostAccount,


NotAStakingAccount was getting a bit overloaded.

shannonwells · 2024-07-16T23:50:02Z

common/primitives/src/capacity.rs

+	/// When this reward expires, i.e. can no longer be claimed
+	pub expires_at_block: BlockNumber,
+	/// The total staked in this era as of the current block
+	pub staked_amount: Balance,


It makes sense to have the amount they staked in each Reward Info.

shannonwells · 2024-07-16T23:53:07Z

pallets/capacity/src/types.rs

-		// have to save this because calling self.count() after self.0.get_mut produces compiler error
-		let current_count = self.count();
+
+		let current_staking_amount = self.get_last_staking_amount();


some code golf.

shannonwells · 2024-07-16T23:53:34Z

runtime/common/src/fee.rs

@@ -77,7 +77,7 @@ mod test {

 		assert_eq!(p, 100_000);

-		assert!(q >= 65_000_000);
+		assert!(q >= 50_000_000);


pulled in from main b/c benchmarks were failing due to too low weight.

shannonwells · 2024-07-17T17:00:47Z

pallets/capacity/src/weights.rs

+		// Proof Size summary in bytes:
+		//  Measured:  `1592`
+		//  Estimated: `20591`
+		// Minimum execution time: 90_000_000 picoseconds.


This is worst case scenario. Claiming rewards may be able to have some tuning, but also the benchmark should include variation. Created issue #2082 to address this

aramikm

Overall looks good, just some nits and questions!

common/primitives/src/capacity.rs

pallets/capacity/src/lib.rs

aramikm

Read through the code and looked good to me!

enddynayn · 2024-07-18T14:11:14Z

pallets/capacity/src/lib.rs

+			);
+			let total_to_mint = Self::do_claim_rewards(&staker)?;
+			Self::deposit_event(Event::ProviderBoostRewardClaimed {
+				account: staker.clone(),


Can you please check if the clone can be removed from here? It seems at first sight that we might not need to do a clone.

enddynayn · 2024-07-18T14:16:13Z

pallets/capacity/src/lib.rs

@@ -1024,37 +1053,42 @@ impl<T: Config> Pallet<T> {
 			Some(provider_boost_history) => {
 				match provider_boost_history.count() {
 					0usize => false,
-					// they staked before the current era, so they have unclaimed rewards.
-					1usize => provider_boost_history.get_entry_for_era(&current_era).is_none(),
+					1usize => {


I think removing a match statement makes the code a bit more readable:

pub(crate) fn has_unclaimed_rewards(account: &T::AccountId) -> bool { let current_era = Self::get_current_era().era_index; if let Some(provider_boost_history) = Self::get_staking_history_for(account) { let entry_count = provider_boost_history.count(); match entry_count { 0 => false, 1 => { let previous_era = current_era.saturating_sub(1); provider_boost_history.get_entry_for_era(&previous_era).is_none() && provider_boost_history.get_entry_for_era(&current_era).is_none() }, _ => true } } else { false } }

enddynayn · 2024-07-18T14:18:55Z

pallets/capacity/src/lib.rs

+			.fold(zero_balance, |acc, reward_info| acc.saturating_add(reward_info.earned_amount))
+			.into();
+		ensure!(total_to_mint.gt(&Zero::zero()), Error::<T>::NothingToClaim);
+		let _minted_unused = T::Currency::mint_into(&staker, total_to_mint)?;


Do we need let _minted_unused if we are not using it?

enddynayn

👍 amazing!

The goal of this PR is to implement the claim_staking_rewards extrinsic, its benchmarks, and tests. Closes #1970

updates to design docs to use a capacity rewards interface Prototypes, make check working refinements to design doc Implement Staking Reward Eras basics (#1589) Implement the basic functionality of tracking and rotating Reward Era. Closes #1567 Does not include anything to do with the Reward Pool. Feat/staking rewards rewards provider #1572 (#1598) The goal of this PR is to implement a really basic version of the StakingRewardsProvider in the Capacity pallet and in the test mock, neither of which is actively used. Closes #1572 Does not include anything to do with setting and storing RewardPoolInfo when each new Era starts. change staking target extrinsic, closes #1570 (#1623) comments, capacity boost fn added to StakingRewardsProvider trait * Refactor staking type to go in StakingTargetDetails * Make some functions pass by reference (performance) * fix broken tests & calls use capacity_boost and StakingType to adjust capacity generation, add/adjust tests * Check from/to aren't the same when retargeting * Performance: move non-db checks to top when retargeting * Lots more tests * Fix a bug where we weren't setting the staking type on a retarget * Remove staking type from StakingAccountDetails * Fix broken tests from last commit Feat/reward pool history (#1710) Closes #1710 Feat/split stake extrinsic #1699 (#1717) The goal of this PR is to split the `stake` extrinsic into two: `stake` and `provider_boost` Closes #1707 Feat/split storage #1726 (#1744) The goal of this PR is to split up storage of boosting and maximized staking accounts, as well as store retarget history separately, which can store retargeting events for any type of staking. Closes #1726 fix e2e tests, correction to implementation design doc initialize storage for ProviderBoost on runtime upgrade Set ProviderBoost capacity generated and fix tests (#1947) * set the amount of capacity generated by a provider boost to the final amount, 50% of what is generated by MaximizedCapacity staking. * Also Fixes some tests broken from the last rebase with main. Closes #1569 Update reward pool on `provider_boost` or `unstake` #1699 (#1948) The goal of this PR is to update the StakingRewardPool on a `provider_boost` or `unstake` extrinsic call. Closes #1699 Implement rewards calculation formula #1941 (#1956) The goal of this PR is to implement (but not really use yet) the chosen formula for calculation of a reward in a single Provider Boost Reward Era. Closes #1941 updates after rebase upsert staking history #1699 (#1963) The goal of this PR is to add and use storage for individual staking history so that rewards can be calculated and paid out. Benchmarks run to update capacity weights. Relates to #1699 Chores/update capacity benchmarks #1949 (#1966) The goal of this PR is to update the benchmark for `on_initialize` to include the weight when a new RewardEra must be created. Closes #1949 Feat/check unclaimed rewards 1969 (#1972) The goal of this PR is to implement `list_unclaimed_rewards`, and also one that is lighter weight, `has_unclaimed_rewards`, which returns a `bool` and which `unstake` extrinsic uses. Unstake now fails if there are any unclaimed rewards. Closes #1969 Closes #1578 Feat/reward pool refactor #1976 (#2005) The goal of this PR is to implement a "chunk" version of the overall reward pool history to reduce read/write load and hence weight for transactions and `on_initialize` when a new `RewardEra` needs to start. Part of #1976 Co-authored-by: Wil Wade <[email protected]> Revise Provider Boost implementation design doc #2016 (#2020) The goal of this PR is to review and update the implementation design doc in light of the chosen economic model, and to reflect some changes in behavior. Review of the design doc also fed back into the code itself. Some code is no longer needed. Closes #2016 Co-authored-by: Aramik <[email protected]> E2e for new extrinsics (#2067) The goal of this PR is to add some e2e tests for the `provider_boost` extrinsic, and update the `change_staking_target` extrinsic after a rebase with main. Refactor reward era (#2069) The goal of this PR is primarily to pull RewardEra out of the Capacity Config and make it the same type everywhere. `claim_staking_rewards` extrinsic (#2080) Closes #1970 Capacity runtime api with list_unclaimed_rewards endpoint (#2088) The goal of this PR is to implement a capacity runtime api for the `list_unclaimed_rewards` endpoint. Closes #1698 Update Capacity README address the lint failure. updates after rebase with main Remove TODO since it's already been addressed. Add a minor test case to unstaking tests Add new extrinsics to Capacity README

claim_staking_rewards extrinsic + tests plus UnclaimedRewardInfo smal…

cb36af5

…l refactor

shannonwells changed the base branch from main to feat/capacity-staking-rewards-impl July 15, 2024 23:26

shannonwells commented Jul 15, 2024

View reviewed changes

claim_staking_rewards benchmark + updated weights.rs

c299b37

shannonwells commented Jul 16, 2024

View reviewed changes

shannonwells added 2 commits July 16, 2024 17:06

cleanup for PR

bf7f004

weights file

8c89ce5

shannonwells commented Jul 17, 2024

View reviewed changes

shannonwells marked this pull request as ready for review July 17, 2024 17:02

shannonwells requested a review from wilwade as a code owner July 17, 2024 17:02

shannonwells requested review from a team, mattheworris, enddynayn, aramikm, claireolmstead and JoeCap08055 and removed request for a team July 17, 2024 20:01

aramikm reviewed Jul 17, 2024

View reviewed changes

common/primitives/src/capacity.rs Outdated Show resolved Hide resolved

pallets/capacity/src/lib.rs Outdated Show resolved Hide resolved

pallets/capacity/src/lib.rs Show resolved Hide resolved

shannonwells added 2 commits July 17, 2024 14:17

correct benchmark

57b22f1

address PR comments

c5d713b

shannonwells requested a review from aramikm July 17, 2024 23:12

aramikm approved these changes Jul 17, 2024

View reviewed changes

enddynayn reviewed Jul 18, 2024

View reviewed changes

enddynayn approved these changes Jul 18, 2024

View reviewed changes

shannonwells merged commit ad63363 into feat/capacity-staking-rewards-impl Jul 18, 2024

shannonwells deleted the extrinsic-claim-rewards branch July 18, 2024 16:42

This was referenced Jul 18, 2024

Implement claim_staking_reward extrinsic #1571

Closed

Integrate economic model into StakingRewardsProvider trait implementation #1747

Closed

shannonwells added a commit that referenced this pull request Jul 18, 2024

claim_staking_rewards extrinsic (#2080)

ec2b7ac

The goal of this PR is to implement the claim_staking_rewards extrinsic, its benchmarks, and tests. Closes #1970

shannonwells added the Provider Boosting label Jul 19, 2024

shannonwells added a commit that referenced this pull request Jul 23, 2024

claim_staking_rewards extrinsic (#2080)

6ed6217

The goal of this PR is to implement the claim_staking_rewards extrinsic, its benchmarks, and tests. Closes #1970

shannonwells added a commit that referenced this pull request Oct 9, 2024

claim_staking_rewards extrinsic (#2080)

c6746a0

The goal of this PR is to implement the claim_staking_rewards extrinsic, its benchmarks, and tests. Closes #1970

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`claim_staking_rewards` extrinsic #2080

`claim_staking_rewards` extrinsic #2080

shannonwells commented Jul 15, 2024 •

edited

Loading

shannonwells Jul 15, 2024

shannonwells Jul 16, 2024

shannonwells Jul 16, 2024

shannonwells Jul 16, 2024

shannonwells Jul 16, 2024

shannonwells Jul 16, 2024

shannonwells Jul 17, 2024 •

edited

Loading

aramikm left a comment

aramikm left a comment

enddynayn Jul 18, 2024 •

edited

Loading

enddynayn Jul 18, 2024 •

edited

Loading

enddynayn Jul 18, 2024

enddynayn left a comment

claim_staking_rewards extrinsic #2080

claim_staking_rewards extrinsic #2080

Conversation

shannonwells commented Jul 15, 2024 • edited Loading

Goal

Discussion

Checklist

shannonwells Jul 15, 2024

Choose a reason for hiding this comment

shannonwells Jul 16, 2024

Choose a reason for hiding this comment

shannonwells Jul 16, 2024

Choose a reason for hiding this comment

shannonwells Jul 16, 2024

Choose a reason for hiding this comment

shannonwells Jul 16, 2024

Choose a reason for hiding this comment

shannonwells Jul 16, 2024

Choose a reason for hiding this comment

shannonwells Jul 17, 2024 • edited Loading

Choose a reason for hiding this comment

aramikm left a comment

Choose a reason for hiding this comment

aramikm left a comment

Choose a reason for hiding this comment

enddynayn Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

enddynayn Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

enddynayn Jul 18, 2024

Choose a reason for hiding this comment

enddynayn left a comment

Choose a reason for hiding this comment

`claim_staking_rewards` extrinsic #2080

`claim_staking_rewards` extrinsic #2080

shannonwells commented Jul 15, 2024 •

edited

Loading

shannonwells Jul 17, 2024 •

edited

Loading

enddynayn Jul 18, 2024 •

edited

Loading

enddynayn Jul 18, 2024 •

edited

Loading