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

Ensure unbonding ops cannot complete during initialization #27

Closed
mpoke opened this issue Dec 24, 2021 · 2 comments · Fixed by #163
Closed

Ensure unbonding ops cannot complete during initialization #27

mpoke opened this issue Dec 24, 2021 · 2 comments · Fixed by #163
Assignees
Labels
type: bug Issues that need priority attention -- something isn't working

Comments

@mpoke
Copy link
Contributor

mpoke commented Dec 24, 2021

During the initialization period, the consumer chain has the same initial validator set V1. To keep the security model of Tendermint, none of the validators in V1 should be able to unbond any amount of their stake during this period. In other words, no unbonding delegation would complete during the CCV initialization protocol.

This feature would enable CCV to guarantee the following invariant that would preserve the security model of Tendermint on the consumer chains:

  • Let t be an instance of time and power(T) be the function that calculates the voting power granted to some validators given an amount T of tokens bonded by these validators on the provider chain. Then, the voting power granted to validators on any consumer chain at time t MUST NOT exceed power(T_t), where T_t is the amount of tokens bonded by these validators on the provider chain at time t.

This feature can be easily added to the current implementation.

  • First, in UnbondingDelegationEntryCreated we need to iterate over all consumer chains, not just the ones with established channels, i.e.,
    h.k.IterateBabyChains(ctx, func(ctx sdk.Context, chainID string) (stop bool) {
    This will block the completion of any unbonding delegation until ACKs are received from all consumer chains.
  • Second, once the CCV channel is set to VALIDATING, the consumer chain still has the initial validator set V1. In the meantime, the provider chain may have gone through multiple updates, e.g., V1V2 → ... → V10. Thus, the first packet sent to the consumer chain should contain all the validator set changes e.g., from V1 to V10. As in the previous step, we need to iterate over all consumer chains, i.e.,
    am.keeper.IterateBabyChains(ctx, func(ctx sdk.Context, chainID string) (stop bool) {
    and if there is no established channel, then accumulate all validator updates; otherwise, send a ValidatorSetChange packet with all accumulated validator updates.
    Note that this step needs to be done regardless of the completion of unbonding operations.
  • Third, we need to enable the completion of unbonding (delegation) operations. At the moment, the UBDEs are mapped to ValidatorSetUpdateIds , i.e.,
    ubde := ccv.UnbondingDelegationEntry{
    For each packet sent, ValidatorSetUpdateId is incremented and it's added to the packet. When receiving a packet ACK, the associated ValidatorSetUpdateId is used to get the associated UBDEs that have matured, i.e.,
    UBDEs, _ := k.GetUBDEsFromIndex(ctx, chainID, data.ValsetUpdateId)
    For this functionality to be preserved when sending multiple validator set changes in a single packet, we need to find another way to map UBDEs to ValidatorSetUpdateIds, i.e., a packet will be associated with multiple ValidatorSetUpdateIds.
@mpoke mpoke changed the title Ensure unbonding ops cannot complete during initialisation Ensure unbonding ops cannot complete during initialization Dec 24, 2021
@mpoke mpoke added type: bug Issues that need priority attention -- something isn't working type: feature-request New feature or request improvement labels Feb 7, 2022
@mpoke
Copy link
Contributor Author

mpoke commented Feb 17, 2022

A fix for this issue is specified in cosmos/ibc#646

@mpoke mpoke moved this to Todo in Replicated Security Apr 11, 2022
@mpoke mpoke assigned mpoke and unassigned mpoke Apr 30, 2022
@mpoke mpoke moved this from Todo to In Progress in Replicated Security May 13, 2022
@mpoke mpoke moved this from In Progress to Todo in Replicated Security May 13, 2022
@mpoke mpoke moved this from Todo to Next in Replicated Security May 13, 2022
@mpoke mpoke moved this from Next to In Progress in Replicated Security Jun 1, 2022
@mpoke mpoke added mvcc and removed type: feature-request New feature or request improvement labels Jun 8, 2022
@mpoke mpoke moved this from In Progress to Waiting for review in Replicated Security Jun 23, 2022
@danwt
Copy link
Contributor

danwt commented Jun 26, 2022

Nice work. I have a one thought

  • we need to find another way to map UBDEs to ValidatorSetUpdateIds, i.e., a packet will be associated with multiple ValidatorSetUpdateIds.

Could we just send multiple packets instead?

EDIT: I see in the code it is done with multiple packets 👍

Repository owner moved this from Waiting for review to Done in Replicated Security Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Issues that need priority attention -- something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants