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

added counter for individual rewards #450

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions frame/dapps-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub struct EraStakingPoints<AccountId: Ord, Balance: HasCompact> {
/// Era when this contract was staked last time before this one.
/// In case only a single staking era exists, it will be set to that one. This indicates the final element in the chain.
former_staked_era: EraIndex,
paidout_rewards: Balance,
Copy link
Member

Choose a reason for hiding this comment

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

paid_out_rewards?

Or just 'claimed_rewards'?

Also add documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

renamed.
Added doc.

}

/// Multi-VM pointer to smart contract instance.
Expand All @@ -86,12 +87,7 @@ pub enum SmartContract<AccountId> {
/// The ledger of a (bonded) stash.
#[derive(PartialEq, Eq, Clone, Encode, Decode, Default, RuntimeDebug)]
pub struct StakingLedger<Balance: HasCompact + Default> {
/// The total amount of the stash's balance that we are currently accounting for.
/// It's just `active` plus all the `unlocking` balances.
/// The total amount of the staker's balance that we are currently accounting for.
Copy link
Member

Choose a reason for hiding this comment

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

It no longer makes sense to keep this as struct.

Copy link
Member Author

Choose a reason for hiding this comment

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

removed struct

#[codec(compact)]
pub total: Balance,
/// The total amount of the stash's balance that will be at stake in any forthcoming
/// rounds.
#[codec(compact)]
pub active: Balance,
}
52 changes: 38 additions & 14 deletions frame/dapps-staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,11 @@ pub mod pallet {
/// Number of eras to keep in history.
///
/// Information is kept for eras in `[current_era - history_depth; current_era]`.
///
/// Must be more than the number of eras delayed by session otherwise. I.e. active era must
/// always be in history. I.e. `active_era > current_era - history_depth` must be
/// guaranteed.
#[pallet::storage]
#[pallet::getter(fn history_depth)]
pub(crate) type HistoryDepth<T> = StorageValue<_, u32, ValueQuery, HistoryDepthOnEmpty>;

/// The current era index.
///
/// This is the latest planned era, depending on how the Session pallet queues the validator
/// set, it might be active or not.
#[pallet::storage]
#[pallet::getter(fn current_era)]
pub type CurrentEra<T> = StorageValue<_, EraIndex, ValueQuery>;
Expand Down Expand Up @@ -132,6 +125,19 @@ pub mod pallet {
pub(crate) type EraRewardsAndStakes<T: Config> =
StorageMap<_, Twox64Concat, EraIndex, EraRewardAndStake<BalanceOf<T>>>;

/// Reward counter for individual stakers and the developer
#[pallet::storage]
#[pallet::getter(fn reward_counter)]
pub(crate) type IndividualRewardCounter<T: Config> = StorageDoubleMap<
Copy link
Member

Choose a reason for hiding this comment

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

Just a suggestion, but I'd replace the name. You're not 'counting' rewards, e.g. how many times rewards were given.
The name is just misleading, at least to me.

I'd suggest something like 'TotalRewardsEarned' (or claimed).

Copy link
Member Author

Choose a reason for hiding this comment

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

it is actually not total since one staker can stake on several contracts.
It is renamed to RewardsClaimed

_,
Twox64Concat,
Copy link
Member

Choose a reason for hiding this comment

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

Since both key types are supplied from the outside, shouldn't we use secure hashing?

Also applies to the rest of the maps in dapps staking pallet.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's take this out of the scope of this PR.
I have created new todo for this
https://github.com/PlasmNetwork/Astar/projects/5#card-69947102

SmartContract<T::AccountId>,
Twox64Concat,
T::AccountId,
BalanceOf<T>,
ValueQuery,
>;

/// Stores amount staked and stakers for a contract per era
#[pallet::storage]
#[pallet::getter(fn contract_era_stake)]
Expand Down Expand Up @@ -393,7 +399,6 @@ pub mod pallet {

// update the ledger value by adding the newly bonded funds
ledger.total += value_to_stake;
ledger.active += value_to_stake;

// Get the latest era staking point info or create it if contract hasn't been staked yet so far.
let era_when_contract_last_staked = Self::contract_last_staked(&contract_id);
Expand All @@ -407,6 +412,7 @@ pub mod pallet {
total: Zero::zero(),
stakers: BTreeMap::<T::AccountId, BalanceOf<T>>::new(),
former_staked_era: 0 as EraIndex,
paidout_rewards: BalanceOf::<T>::default(),
}
};

Expand Down Expand Up @@ -527,6 +533,8 @@ pub mod pallet {
// if staked value would fall below threshold, unstake everything
era_staking_points.stakers.remove(&staker);
value_to_unstake = staked_value;
// remove reward counter
IndividualRewardCounter::<T>::remove(&contract_id, &staker);
} else {
era_staking_points
.stakers
Expand All @@ -539,7 +547,6 @@ pub mod pallet {
// Get the staking ledger and update it
let mut ledger = Self::ledger(&staker).ok_or(Error::<T>::UnexpectedState)?;
ledger.total = ledger.total.saturating_sub(value_to_unstake);
ledger.active = ledger.active.saturating_sub(value_to_unstake);
Self::update_ledger(&staker, &ledger);

let current_era = Self::current_era();
Expand Down Expand Up @@ -672,10 +679,18 @@ pub mod pallet {
// continue and process the next era interval
}

// send rewards to stakers
Self::payout_stakers(&rewards_for_stakers_map);
// send rewards to developer
// send rewards to stakers' accounts and update reward counter individual staker
let reward_for_stakers = Self::payout_stakers(&contract_id, &rewards_for_stakers_map);

// send rewards to developer's account and update reward counter for developer's account
T::Currency::deposit_into_existing(&developer, reward_for_developer).ok();
IndividualRewardCounter::<T>::mutate(&contract_id, &developer, |balance| {
*balance += reward_for_developer
});

// updated counter for total rewards paid to the contract
contract_staking_info.paidout_rewards += reward_for_stakers + reward_for_developer;

// if !unclaimed_rewards.is_zero() { TODO!
// T::Currency::deposit_into_existing(&treasury, unclaimed_rewards).ok();
// }
Expand Down Expand Up @@ -773,7 +788,7 @@ pub mod pallet {
/// Update the ledger for a staker. This will also update the stash lock.
/// This lock will lock the entire funds except paying for further transactions.
fn update_ledger(staker: &T::AccountId, ledger: &StakingLedger<BalanceOf<T>>) {
if ledger.active.is_zero() {
if ledger.total.is_zero() {
Ledger::<T>::remove(&staker);
T::Currency::remove_lock(STAKING_ID, &staker);
} else {
Expand Down Expand Up @@ -811,10 +826,19 @@ pub mod pallet {
}

/// Execute payout for stakers
Copy link
Member

Choose a reason for hiding this comment

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

Now it does more than that.

payout_stakers_and_return_total_reward?

Doc should also be updated.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

fn payout_stakers(staker_map: &BTreeMap<T::AccountId, BalanceOf<T>>) {
fn payout_stakers(
contract: &SmartContract<T::AccountId>,
staker_map: &BTreeMap<T::AccountId, BalanceOf<T>>,
) -> BalanceOf<T> {
let mut reward_for_stakers = Zero::zero();

for (s, b) in staker_map {
IndividualRewardCounter::<T>::mutate(contract, s, |balance| *balance += *b);
T::Currency::deposit_into_existing(&s, *b).ok();
reward_for_stakers += *b;
}

reward_for_stakers
}

/// The block rewards are accumulated on the pallets's account during an era.
Expand Down
34 changes: 33 additions & 1 deletion frame/dapps-staking/src/testing_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ pub(crate) fn verify_ledger(staker_id: AccountId, staked_value: Balance) {
// Verify that ledger storage values are as expected.
let ledger = Ledger::<TestRuntime>::get(staker_id).unwrap();
assert_eq!(staked_value, ledger.total);
assert_eq!(staked_value, ledger.active);
}

/// Used to verify era staking points content.
Expand Down Expand Up @@ -159,3 +158,36 @@ pub(crate) fn calc_expected_developer_reward(
let contract_reward = Perbill::from_rational(contract_stake, era_stake) * era_reward;
Perbill::from_percent(DEVELOPER_REWARD_PERCENTAGE) * contract_reward
}

/// Check Balance after reward distribution and check reward counter
pub(crate) fn check_rewards_and_counter(
contract: &SmartContract<mock::AccountId>,
user: &AccountId,
free_balance: mock::Balance,
eras: EraIndex,
expected_era_reward: mock::Balance,
) {
println!("check_rewards_and_counter {:?}, user={:?}", contract, &user);
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

removed

let total_reward_per_era = expected_era_reward * eras as u128;
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(user),
free_balance + total_reward_per_era
);
assert_eq!(
mock::DappsStaking::reward_counter(contract, user),
total_reward_per_era
);
}

pub(crate) fn check_paidout_rewards_for_contract(
contract: &SmartContract<mock::AccountId>,
expected_contract_reward: mock::Balance,
) {
let era_last_claimed = mock::DappsStaking::contract_last_claimed(contract).unwrap_or(0);
let contract_staking_info =
mock::DappsStaking::contract_era_stake(contract, era_last_claimed).unwrap_or_default();
assert_eq!(
contract_staking_info.paidout_rewards,
expected_contract_reward
)
}
149 changes: 103 additions & 46 deletions frame/dapps-staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1154,14 +1154,23 @@ fn claim_one_contract_one_staker() {
calc_expected_developer_reward(total_era_dapps_reward, INITIAL_STAKE, INITIAL_STAKE);

// check balances to see if the rewards are paid out
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&staker1),
free_balance_staker1 + eras_eligible_for_reward * expected_staker1_reward
);
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&developer),
free_developer_balance + eras_eligible_for_reward * expected_developer_reward as u128
);
check_rewards_and_counter(
&contract,
&staker1,
free_balance_staker1,
eras_eligible_for_reward as u32,
Copy link
Member

Choose a reason for hiding this comment

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

'as EraIndex' ?

Copy link
Member Author

Choose a reason for hiding this comment

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

replaced all instances

expected_staker1_reward,
);
check_rewards_and_counter(
&contract,
&developer,
free_developer_balance,
eras_eligible_for_reward as u32,
expected_developer_reward,
);
let expected_contract_reward =
eras_eligible_for_reward * (expected_staker1_reward + expected_developer_reward);
check_paidout_rewards_for_contract(&contract, expected_contract_reward);
})
}

Expand Down Expand Up @@ -1219,18 +1228,30 @@ fn claim_one_contract_two_stakers() {

// check balances to see if the rewards are paid out
let eras_eligible_for_reward = (claim_era - start_era) as u128;
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&staker1),
free_balance_staker1 + eras_eligible_for_reward * expected_staker1_reward
);
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&staker2),
free_balance_staker2 + eras_eligible_for_reward * expected_staker2_reward
);
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&developer),
free_developer_balance + eras_eligible_for_reward * expected_developer_reward as u128
);
check_rewards_and_counter(
&contract,
&staker1,
free_balance_staker1,
eras_eligible_for_reward as u32,
expected_staker1_reward,
);
check_rewards_and_counter(
&contract,
&staker2,
free_balance_staker2,
eras_eligible_for_reward as u32,
expected_staker2_reward,
);
check_rewards_and_counter(
&contract,
&developer,
free_developer_balance,
eras_eligible_for_reward as u32,
expected_developer_reward,
);
let expected_contract_reward = eras_eligible_for_reward
* (expected_staker1_reward + expected_staker2_reward + expected_developer_reward);
check_paidout_rewards_for_contract(&contract, expected_contract_reward);
})
}

Expand Down Expand Up @@ -1347,28 +1368,42 @@ fn claim_two_contracts_three_stakers() {
let expected_c1_dev1_e2_reward =
calc_expected_developer_reward(expected_era_reward, ERA_STAKED2, CONTRACT1_STAKE);

assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&staker1),
free_balance_staker1
+ eras_eligible_for_reward1 * expected_c1_staker1_e1_reward
+ eras_eligible_for_reward2 * expected_c1_staker1_e2_reward
let expected_c1_staker1_reward_total = eras_eligible_for_reward1
* expected_c1_staker1_e1_reward
+ eras_eligible_for_reward2 * expected_c1_staker1_e2_reward;
check_rewards_and_counter(
&contract1,
&staker1,
free_balance_staker1,
1 as u32, // use 1 since the multiplication with era is alreday done
expected_c1_staker1_reward_total,
);

// staker2 staked on both contracts. Memorize this reward for staker2 on contract1
let expected_c1_staker2_reward = eras_eligible_for_reward1 * expected_c1_staker2_e1_reward
let expected_c1_staker2_reward_total = eras_eligible_for_reward1
* expected_c1_staker2_e1_reward
+ eras_eligible_for_reward2 * expected_c1_staker2_e2_reward;

// check balances to see if the rewards are paid out
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&staker2),
free_balance_staker2 + expected_c1_staker2_reward
);
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&developer1),
free_balance_developer1
+ eras_eligible_for_reward1 * expected_c1_dev1_e1_reward as u128
+ eras_eligible_for_reward2 * expected_c1_dev1_e2_reward as u128
);
check_rewards_and_counter(
&contract1,
&staker2,
free_balance_staker2,
1 as u32, // use 1 since the multiplication with era is alreday done
expected_c1_staker2_reward_total,
);

let expected_c1_developer1_reward_total = eras_eligible_for_reward1
* expected_c1_dev1_e1_reward
+ eras_eligible_for_reward2 * expected_c1_dev1_e2_reward;
check_rewards_and_counter(
&contract1,
&developer1,
free_balance_developer1,
1 as u32, // use 1 since the multiplication with era is alreday done
expected_c1_developer1_reward_total,
);
let expected_contract1_reward = expected_c1_staker1_reward_total
+ expected_c1_staker2_reward_total
+ expected_c1_developer1_reward_total;
check_paidout_rewards_for_contract(&contract1, expected_contract1_reward);

// claim rewards for contract2 4 eras later
// era=11
Expand Down Expand Up @@ -1404,19 +1439,41 @@ fn claim_two_contracts_three_stakers() {
let eras_eligible_for_reward = (claim_era - start_staking_era_for_c2) as u128; // all skipped eras plus era when it was last claimed

// check balances to see if the rewards are paid out
let expected_c2_staker2_reward_total =
eras_eligible_for_reward * expected_c2_staker2_e2_reward;
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&staker2),
free_balance_staker2
+ eras_eligible_for_reward * expected_c2_staker2_e2_reward
+ expected_c1_staker2_reward
+ expected_c2_staker2_reward_total
+ expected_c1_staker2_reward_total
);

// we do not use check_rewards_and_counter() here since
// this counter check is for the contract2 only.
// It does not include reward for the contract1
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&staker3),
free_balance_staker3 + eras_eligible_for_reward * expected_c2_staker3_e2_reward
mock::DappsStaking::reward_counter(contract2, staker2),
expected_c2_staker2_reward_total
);
assert_eq!(
<mock::TestRuntime as Config>::Currency::free_balance(&developer2),
free_balance_developer2 + eras_eligible_for_reward * expected_c2_dev2_e2_reward as u128

check_rewards_and_counter(
&contract2,
&staker3,
free_balance_staker3,
eras_eligible_for_reward as u32,
expected_c2_staker3_e2_reward,
);

check_rewards_and_counter(
&contract2,
&developer2,
free_balance_developer2,
eras_eligible_for_reward as u32,
expected_c2_dev2_e2_reward,
);
let expected_contract2_reward = eras_eligible_for_reward
* (expected_c2_staker3_e2_reward + expected_c2_dev2_e2_reward)
+ expected_c2_staker2_reward_total;
check_paidout_rewards_for_contract(&contract2, expected_contract2_reward);
})
}