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

Improve rewards withdrawal process #123

Closed
Tracked by #85
maurolacy opened this issue Sep 13, 2023 · 2 comments
Closed
Tracked by #85

Improve rewards withdrawal process #123

maurolacy opened this issue Sep 13, 2023 · 2 comments
Assignees

Comments

@maurolacy
Copy link
Collaborator

maurolacy commented Sep 13, 2023

The current rewards withdrawal process would break if a validator was slashed or fell out of the validator set.

See

/**
* This is called once per epoch to withdraw all rewards and rebalance the bonded tokens.
* Note: the current implementation may (repeatedly) fail if any validator was slashed or fell out
* of the active set.
*
* The basic logic for calculating rebalance is:
* 1. Get all bond requests
* 2. Sum the total amount
* 3. If the sum <= max_cap then use collected requests as is
* 4. If the sum > max_cap,
* a. calculate multiplier Decimal(max_cap / sum)
* b. multiply every element of the collected requests in place.
* 5. Find diff between collected (normalized) requests and last bonding amounts (which go up, which down).
* 6. Transform diff into unbond and bond requests, sorting so all unbond happen first
*/
fn handle_epoch(
&self,
mut deps: DepsMut<VirtualStakeCustomQuery>,
env: Env,
) -> Result<Response<VirtualStakeCustomMsg>, ContractError> {

And this comment

/**
* This is called every time there's a change of the active validator set.
*
*/
fn handle_valset_update(
&self,
deps: DepsMut<VirtualStakeCustomQuery>,
additions: &[Validator],
removals: &[Validator],
tombstones: &[Validator],
) -> Result<Response<VirtualStakeCustomMsg>, ContractError> {
// TODO: Store/process removals (and additions) locally, so that they are filtered out from
// the `bonded` list

Fixing this requiring keeping track of the active validator set on the Consumer (i.e. on the virtual-staking contract).

This is similar to the way the valset is being kept up-to-date in Tgrade. You can take a look at tgrade-valset for inspiration / ideas.

@uint
Copy link
Contributor

uint commented Oct 12, 2023

I think this was dealt with in #137. @maurolacy Do we close this, or is there more to do?

@maurolacy
Copy link
Collaborator Author

maurolacy commented Oct 12, 2023

There's more to do (FIXMEs / TODOs in the virtual-staking code / tests), but will be dealt with during full valset state implementation (#118). Closing.

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

No branches or pull requests

2 participants