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

multi currency incentive rewards #1396

Merged
merged 15 commits into from
Sep 15, 2021
Merged

multi currency incentive rewards #1396

merged 15 commits into from
Sep 15, 2021

Conversation

ermalkaleci
Copy link
Contributor

@ermalkaleci ermalkaleci commented Sep 4, 2021

Stash

@ermalkaleci ermalkaleci requested a review from xlc September 4, 2021 23:07
@xlc xlc requested a review from wangjj9219 September 5, 2021 07:36
Copy link
Member

@xlc xlc left a comment

Choose a reason for hiding this comment

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

Also need to add the migration for orml-rewards to karura runtime

modules/incentives/src/lib.rs Show resolved Hide resolved
Copy link
Member

@xlc xlc left a comment

Choose a reason for hiding this comment

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

Need migration for orml-rewards

Copy link
Member

@wangjj9219 wangjj9219 left a comment

Choose a reason for hiding this comment

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

plan to use HomaValidatorAllowance to implement the multi-currency reward? But reward accumulation for HomaValidatorAllowance is not periodic.

The definition for PoolId is no longer appropriate. PooId implies the reward currency id(native and stable or other allowance) and accumulation method (periodly by fixed amount ,or the multiple of the base amount and fixed rate, or transfer allowance non-periodicly)

Now, orml_rewards supports to record multi-currency rewards amount with single PoolId,
And the pool share of DexIncentives is equal to DexSaving for the same trading pair.
it's better to refactor PoolId to DEX, Loans and others, and need new storage
map Vec<(reward_currency_id, reward_amount_per_period)>
to record periodic fixed amount reward, and define new enum to imply base amount source, add another storage
map PoolId => Vec<(reward_currency_id, fixed_rate, BaseAmountSourceEnum)> to record the fixed rate and the base amount source. Aggregate these configurations when accumulating periodic multi rewards.

@xlc What do you think?
If need to do, more migrations are required, and I am afraid that testing and front-end modification cannot be completed before this Thursday’s runtime upgrade.

@wangjj9219 wangjj9219 marked this pull request as ready for review September 15, 2021 03:21
modules/incentives/src/lib.rs Show resolved Hide resolved
@xlc xlc merged commit ce28190 into master Sep 15, 2021
@xlc xlc deleted the incentives_multi_currency branch September 15, 2021 10:36
xlc pushed a commit that referenced this pull request Sep 19, 2021
* stash: multi currency incentive rewards

* update

* add migration

* refactor

* update

* fix tests

* fix benchmarking

* update incentives migration

* add migration

* update

* fix clippy

* migration limit

* fix

Co-authored-by: wangjj9219 <183318287@qq.com>
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.

3 participants