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

feat: integrate staking pallet into vault-registry and nomination #165

Merged
merged 2 commits into from Jul 8, 2021

Conversation

gregdhill
Copy link
Member

@gregdhill gregdhill commented Jul 5, 2021

Signed-off-by: Gregory Hill gregorydhill@outlook.com

Integrates the new staking pallet into the vault-registry, nomination pallet and runtime. The main purpose of this is to centralize the stake accounting - many of the fields on the Vault struct have been removed and the Nominator struct is no longer required. Another reason is for reward distribution since rewards should not be accrued on slashed stake.

@nud3l nud3l marked this pull request as draft July 6, 2021 07:55
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
@gregdhill gregdhill marked this pull request as ready for review July 6, 2021 10:49
@gregdhill gregdhill requested review from sander2 and nud3l July 6, 2021 10:49
Copy link
Member

@nud3l nud3l left a comment

Choose a reason for hiding this comment

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

To fully review this, I need to understand what the desired outcome of the system is.

Open questions for me are:

Staking pallet

  • What does the staking pallet do?
  • Is this our way of tracking collateral across vaults and nominators?
  • Can we use this with multiple currencies as well?
  • What exactly is the stake? Is that just the amount of e.g., DOT deployed by the vault?

Rewards

  • What are the implications on the rewards? In our current docs, fees are distributed to vaults 90% based on issued_tokens and 10% based on locked collateral (see https://docs.interlay.io/#/vault/overview?id=pool-based-fee-distribution). It looks like to me rewards are currently only paid based on locked collateral rather than issued_tokens?
  • In the future, we will still want to be able to pay rewards on both issued tokens and on collateral locked. In case this is currently not implemented, we need to add this again.

Specification

We need to add a specification for the reward and staking pallet.

Tests

Overall, this changes a lot of logic without adding tests. In the reward crate, a lot of tests are deleted. Please add tests for checking that the assumptions for the reward distributions hold.

crates/fee/src/lib.rs Outdated Show resolved Hide resolved
crates/nomination/src/tests.rs Show resolved Hide resolved
crates/staking/src/lib.rs Outdated Show resolved Hide resolved
crates/vault-registry/src/lib.rs Show resolved Hide resolved
crates/vault-registry/src/types.rs Show resolved Hide resolved
@gregdhill
Copy link
Member Author

Staking pallet

  • The staking pallet tracks stake / collateral for Vaults and Nominators.
  • It currently indexes balances by a currency, but there are efficiency issues with multi-currency staking so I would prefer not to do this.
  • The stake here is the collateral for a Vault, but in the rewards pallet this is the unbounded SLA.

Rewards

  • This PR has no discernible impact on rewards from what is currently implemented.
  • It does however ensure that slashed Vaults do not gain rewards on slashed stake.
  • The rewards pallet distributes based on collateral locked + issuance (out-of-scope of this PR).

Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
@sander2 sander2 merged commit 2dbb0b9 into interlay:master Jul 8, 2021
@gregdhill gregdhill deleted the feat/staking-rewards branch July 8, 2021 14:23
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.

None yet

3 participants