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

404 add pooled reward pallet #415

Merged
merged 50 commits into from
Nov 2, 2023
Merged

404 add pooled reward pallet #415

merged 50 commits into from
Nov 2, 2023

Conversation

gianfra-t
Copy link
Contributor

@gianfra-t gianfra-t commented Oct 10, 2023

Issue #404 and #406, part of this pendulum task.

Adds the pooled-rewards pallet, that will allow to distribute the static native currency rewards for the nominators/operators of vaults, as well as the fee distribution in arbitrary currency.

This pallet is taken from interlay .
This will also replace the current rewards pallet that is used to distribute fees currently.

At any point the stake of each vault_id on the pooled-rewards pallet for it's corresponding collateral currency pool is maintained to be equal to that vault_id's total_staking amount from the staking pallet. See for example here.

This update is performed whenever the stake for the vault_id changes, and is managed by the pool_staking_manager interface:

This PR will also merge changes for #406, where reward percentage per collateral pool is calculated (see here), and exposes a function used for fee pallet to distribute arbitrary rewards.

Additions to reward-distribution pallet.

  • An on_initialize hook, that calculates the block specific reward, and updates it if required.
  • A distrubute_reward function that calculates the percentages in USD of the total stake, and distributes to each collateral pool a proportional amount. This function will be used both from external pallets (eg Fee) and to distribute the per-block rewards in native token.
  • Trait interfaces for this function to be called from other pallets.
  • A new extrinsic to collect the rewards for a given vault_id, account_id pair.

The collect_reward extrinsic replaces the extrinsic previously located in the fee pallet which only allowed to withdraw the reward for the wrapped currency and native( see here)

@gianfra-t gianfra-t linked an issue Oct 10, 2023 that may be closed by this pull request
@gianfra-t gianfra-t requested review from b-yap and ebma and removed request for b-yap October 10, 2023 13:12
@gianfra-t gianfra-t marked this pull request as draft October 10, 2023 19:30
pallets/fee/src/mock.rs Outdated Show resolved Hide resolved
pallets/issue/src/mock.rs Outdated Show resolved Hide resolved
pallets/oracle/src/lib.rs Outdated Show resolved Hide resolved
pallets/pooled-rewards/src/mock.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

The latest implementation looks good to me overall. I mainly added nit-picking comments.

It's probably not obvious but doc comments on specific pallet items are super useful because they will be extracted into the metadata and tools like polkadot.js use them to show some extra info when eg calling extrinsics.

pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/staking/src/lib.rs Outdated Show resolved Hide resolved
pallets/vault-registry/src/lib.rs Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
pallets/reward-distribution/src/benchmarking.rs Outdated Show resolved Hide resolved
@ebma
Copy link
Member

ebma commented Oct 27, 2023

For some reason, the checks are disabled for this PR. I don't see an option to turn them back on nor do I see a reason why they don't run. Maybe it's because the PR has conflicts with the base branch that must be resolved? @gianfra-t can you resolve these conflicts so we see if this prevents the CI actions from running?
Otherwise, please run all the relevant checks contained here locally so we know if there are any issues.

@gianfra-t
Copy link
Contributor Author

gianfra-t commented Oct 27, 2023

Hey @ebma, does this error:

thread 'test_shutdown' panicked at 'Expected Ok(_). Got Err(
    SubxtRuntimeError(
        Metadata(
            IncompatibleCallMetadata(
                "Sudo",
                "sudo",
            ),
        ),
    ),

mean we need to regenerate the metadata?

@ebma
Copy link
Member

ebma commented Oct 30, 2023

Yes exactly, regenerating the metadata should do the trick 👍

@gianfra-t
Copy link
Contributor Author

I just merged with the latest changes in main, but the tests are passing. As soon as they pass again should we merge this? Or do you want to have another look @TorstenStueber?

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Looks good to me now. Thanks for your great efforts with all of this @gianfra-t! 👍

Let's still wait for the review of @TorstenStueber before merging though

pallets/reward-distribution/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@TorstenStueber TorstenStueber left a comment

Choose a reason for hiding this comment

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

I looked thoroughly at the code again and everything looks good.

Great job!

@ebma
Copy link
Member

ebma commented Nov 2, 2023

@gianfra-t feel free to push the squash and merge button then.

@gianfra-t gianfra-t merged commit e5c6acd into main Nov 2, 2023
2 checks passed
@gianfra-t gianfra-t deleted the 404-add-pooled-reward-pallet branch November 2, 2023 13:55
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.

Add pooled reward pallet
3 participants