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!: introduce epochs #1691

Merged
merged 9 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Introduce epochs (i.e., send a VSCPacket every X blocks instead of in every
block) so that we reduce the cost of relaying IBC packets needed for ICS.
([\#1516](https://github.com/cosmos/interchain-security/pull/1516))
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Introduce epochs (i.e., send a VSCPacket every X blocks instead of in every
block) so that we reduce the cost of relaying IBC packets needed for ICS.
([\#1516](https://github.com/cosmos/interchain-security/pull/1516))
6 changes: 2 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,8 @@ test-trace:
# Note: this is *not* using the Quint models to test the system,
# this tests/verifies the Quint models *themselves*.
verify-models:
quint test tests/mbt/model/ccv_test.qnt;\
quint test tests/mbt/model/ccv_model.qnt;\
quint run --invariant "all{ValidatorUpdatesArePropagatedInv,ValidatorSetHasExistedInv,SameVscPacketsInv,MatureOnTimeInv,EventuallyMatureOnProviderInv}" tests/mbt/model/ccv_model.qnt --max-steps 200 --max-samples 200;\
quint run --invariant "all{ValidatorUpdatesArePropagatedKeyAssignmentInv,ValidatorSetHasExistedKeyAssignmentInv,SameVscPacketsKeyAssignmentInv,MatureOnTimeInv,EventuallyMatureOnProviderInv,KeyAssignmentRulesInv}" tests/mbt/model/ccv_model.qnt --step stepKeyAssignment --max-steps 200 --max-samples 200
cd tests/mbt/model;\
../run_invariants.sh



Expand Down
116 changes: 17 additions & 99 deletions docs/docs/adrs/adr-001-key-assignment.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ title: Key Assignment

## Changelog
* 2022-12-01: Initial Draft
* 2024-03-01: Updated to take into account they key-assigment-replacement deprecation.

## Status

Expand All @@ -30,10 +31,6 @@ ConsumerValidatorsBytePrefix | len(chainID) | chainID | providerConsAddress -> c
```golang
ValidatorsByConsumerAddrBytePrefix | len(chainID) | chainID | consumerConsAddress -> providerConsAddress
```
- `KeyAssignmentReplacements` - Stores the key assignments that need to be replaced in the current block. Needed to apply the key assignments received in a block to the validator updates sent to the consumer chains.
```golang
KeyAssignmentReplacementsBytePrefix | len(chainID) | chainID | providerConsAddress -> abci.ValidatorUpdate{PubKey: oldConsumerKey, Power: currentPower},
```
- `ConsumerAddrsToPrune` - Stores the mapping from VSC ids to consumer validators addresses. Needed for pruning `ValidatorByConsumerAddr`.
```golang
ConsumerAddrsToPruneBytePrefix | len(chainID) | chainID | vscID -> []consumerConsAddresses
Expand Down Expand Up @@ -67,20 +64,6 @@ if _, consumerRegistered := GetConsumerClientId(chainID); consumerRegistered {
oldConsumerAddr := utils.TMCryptoPublicKeyToConsAddr(oldConsumerKey)
vscID := GetValidatorSetUpdateId()
AppendConsumerAddrsToPrune(chainID, vscID, oldConsumerAddr)
} else {
// the validator had no key assigned on this consumer chain
oldConsumerKey := validator.TmConsPublicKey()
}

// check whether the validator is valid, i.e., its power is positive
if currentPower := stakingKeeper.GetLastValidatorPower(providerAddr); currentPower > 0 {
// to enable multiple calls of AssignConsumerKey in the same block by the same validator
// the key assignment replacement should not be overwritten
if _, found := GetKeyAssignmentReplacement(chainID, providerConsAddr); !found {
// store old key and power for modifying the valset update in EndBlock
oldKeyAssignment := abci.ValidatorUpdate{PubKey: oldConsumerKey, Power: currentPower}
SetKeyAssignmentReplacement(chainID, providerConsAddr, oldKeyAssignment)
}
}
} else {
// if the consumer chain is not registered, then remove the previous reverse mapping
Expand Down Expand Up @@ -129,89 +112,24 @@ func (k Keeper) MakeConsumerGenesis(chainID string) (gen consumertypes.GenesisSt
}
```

On `EndBlock` while queueing `VSCPacket`s to send to registered consumer chains:
Note that key assignment works hand-in-hand with [epochs](https://github.com/cosmos/interchain-security/blob/main/docs/docs/adrs/adr-014-epochs.md).
For each consumer chain, we store the consumer validator set that is currently (i.e., in this epoch) validating the consumer chain.
Specifically, for each validator in the set we store among others, the public key that it is using on the consumer chain during the current (i.e., ongoing) epoch.
At the end of every epoch, if there were validator set changes on the provider, then for every consumer chain, we construct a `VSCPacket`
with all the validator updates and add it to the list of `PendingVSCPacket`s. We compute the validator updates needed by a consumer chain by
comparing the stored list of consumer validators with the current bonded validators on the provider, with something similar to this:
```golang
func QueueVSCPackets() {
valUpdateID := GetValidatorSetUpdateId()
// get the validator updates from the staking module
valUpdates := stakingKeeper.GetValidatorUpdates()

IterateConsumerChains(func(chainID, clientID string) (stop bool) {
// apply the key assignment to the validator updates
valUpdates := ApplyKeyAssignmentToValUpdates(chainID, valUpdates)
// ..
})
// ...
}

func ApplyKeyAssignmentToValUpdates(
chainID string,
valUpdates []abci.ValidatorUpdate,
) (newUpdates []abci.ValidatorUpdate) {
for _, valUpdate := range valUpdates {
providerAddr := utils.TMCryptoPublicKeyToConsAddr(valUpdate.PubKey)

// if a key assignment replacement is found, then
// remove the valupdate with the old consumer key
// and create two new valupdates
prevConsumerKey, _, found := GetKeyAssignmentReplacement(chainID, providerAddr)
if found {
// set the old consumer key's power to 0
newUpdates = append(newUpdates, abci.ValidatorUpdate{
PubKey: prevConsumerKey,
Power: 0,
})
// set the new consumer key's power to the power in the update
newConsumerKey := GetValidatorConsumerPubKey(chainID, providerAddr)
newUpdates = append(newUpdates, abci.ValidatorUpdate{
PubKey: newConsumerKey,
Power: valUpdate.Power,
})
// delete key assignment replacement
DeleteKeyAssignmentReplacement(chainID, providerAddr)
} else {
// there is no key assignment replacement;
// check if the validator's key is assigned
consumerKey, found := k.GetValidatorConsumerPubKey(ctx, chainID, providerAddr)
if found {
// replace the update containing the provider key
// with an update containing the consumer key
newUpdates = append(newUpdates, abci.ValidatorUpdate{
PubKey: consumerKey,
Power: valUpdate.Power,
})
} else {
// keep the same update
newUpdates = append(newUpdates, valUpdate)
}
}
}

// iterate over the remaining key assignment replacements
IterateKeyAssignmentReplacements(chainID, func(
pAddr sdk.ConsAddress,
prevCKey tmprotocrypto.PublicKey,
power int64,
) (stop bool) {
// set the old consumer key's power to 0
newUpdates = append(newUpdates, abci.ValidatorUpdate{
PubKey: prevCKey,
Power: 0,
})
// set the new consumer key's power to the power in key assignment replacement
newConsumerKey := GetValidatorConsumerPubKey(chainID, pAddr)
newUpdates = append(newUpdates, abci.ValidatorUpdate{
PubKey: newConsumerKey,
Power: power,
})
return false
})

// remove all the key assignment replacements

return newUpdates
}
// get the valset that has been validating the consumer chain during this epoch
currentValidators := GetConsumerValSet(consumerChain)
// generate the validator updates needed to be sent through a `VSCPacket` by comparing the current validators
// in the epoch with the latest bonded validators
valUpdates := DiffValidators(currentValidators, stakingmodule.GetBondedValidators())
// update the current validators set for the upcoming epoch to be the latest bonded validators instead
SetConsumerValSet(stakingmodule.GetBondedValidators())
```
where `DiffValidators` internally checks if the consumer public key for a validator has changed since the last
epoch and if so generates a validator update. This way, a validator can change its consumer public key for a consumer
chain an arbitrary amount of times and only the last set consumer public key would be taken into account.

On receiving a `SlashPacket` from a consumer chain with id `chainID` for a infraction of a validator `data.Validator`:
```golang
Expand Down
48 changes: 32 additions & 16 deletions docs/docs/adrs/adr-014-epochs.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ title: Epochs
# ADR 014: Epochs

## Changelog
* 2024-01-105: Proposed, first draft of ADR.
* 2024-01-05: Proposed, first draft of ADR.
* 2024-02-29: Updated so that it describes the implementation where we store the whole consumer validator set.

## Status

Expand All @@ -22,37 +23,52 @@ As a matter of fact, this already happens due to relaying delays.
As a solution, this ADR introduces the concept of _epochs_.
An epoch consists of multiple blocks.
The provider sends `VSCPacket`s once per epoch.
A `VSCPacket` contains all the valset changes that occurred throughout the epoch.
A `VSCPacket` contains all the validator updates that are needed by a consumer chain.

## Decision

The implementation of epochs requires the following changes:

- Add a param that sets the number of blocks in an epoch, i.e., `BlocksPerEpoch`.
We can use `BlockHeight() % BlocksPerEpoch == 0` to decide when an epoch is over.
Note that `BlocksPerEpoch` can also be a hardcoded constant as it's unlikely that it will change often.
- In every provider `EndBlock()`, instead of queueing `VSCPacket` data for every consumer chain, we accumulate the validator changes (similarly to how is done on the consumer, see `AccumulateChanges`).
- Modify the key assignment logic to allow for `MustApplyKeyAssignmentToValUpdates` to be called once per epoch.
Currently, this method is called in every block before queueing a `VSCPacket`.
Also, the method uses the `KeyAssignmentReplacement` state, which is pruned at the end of every block.
This needs to be done once per epoch instead.
- At the end of every epoch, if there were validator set changes on the provider, then for every consumer chain, construct a `VSCPacket` with all the accumulated validator changes and add it to the list of `PendingVSCPackets`.

As an optional change, to better accommodate [the Partial Set Security design](https://informalsystems.notion.site/Partial-Set-Security-398ca9a1453740068be5c7964a4059bb), the validator changes should be accumulated per consumer chain.
Like this, it would make it easier to have validators opting out from certain consumer chains.
- For each consumer chain, we store the consumer validator set that is currently (i.e., in this epoch) validating the
consumer chain. For each validator in the set we store i) its voting power, and ii) the public key that it is
using on the consumer chain during the current (i.e., ongoing) epoch.
The initial consumer validator set for a chain is set during the creation of the consumer genesis.
- We introduce the `BlocksPerEpoch` param that sets the number of blocks in an epoch. By default, `BlocksPerEpoch` is
set to be 600 which corresponds to 1 hour, assuming 6 seconds per block. This param can be changed through
a _governance proposal_ to be anywhere between `[1, MaxBlocksPerEpoch]` where `MaxBlocksPerEpoch` can be up to 1200
(2 hours if we assume 6 seconds per block). In the provider `EndBlock` we check `BlockHeight() % BlocksPerEpoch() == 0`
to decide when an epoch has ended.
- At the end of every epoch, if there were validator set changes on the provider, then for every consumer chain, we
construct a `VSCPacket` with all the validator updates and add it to the list of `PendingVSCPackets`. We compute the
validator updates needed by a consumer chain by comparing the stored list of consumer validators with the current
bonded validators on the provider, with something similar to this:
```go
// get the valset that has been validating the consumer chain during this epoch
currentValidators := GetConsumerValSet(consumerChain)
// generate the validator updates needed to be sent through a `VSCPacket` by comparing the current validators
// in the epoch with the latest bonded validators
valUpdates := DiffValidators(currentValidators, stakingmodule.GetBondedValidators())
// update the current validators set for the upcoming epoch to be the latest bonded validators instead
SetConsumerValSet(stakingmodule.GetBondedValidators())
```
Note that a validator can change its consumer public key for a specific consumer chain an arbitrary amount of times during
a block and during an epoch. Then, when we generate the validator updates in `DiffValidators`, we have to check whether
the current consumer public key (retrieved by calling `GetValidatorConsumerPubKey`) is different from the consumer public
key the validator was using in the current epoch.

## Consequences

### Positive

- Reduce the cost of relaying.
- Reduce the amount of IBC packets needed for ICS.
- Simplifies [key-assignment code](https://github.com/cosmos/interchain-security/blob/main/docs/docs/adrs/adr-001-key-assignment.md) because
we only need to check if the `consumer_public_key` has been modified since the last epoch to generate an update.

### Negative

- Additional logic on the provider side as valset changes need to be accumulated.
- The changes might impact the key-assignment logic so special care is needed to avoid introducing bugs.
- Increase the delay in the propagation of validator set changes (but for reasonable epoch lengths on the order of ~hours or less, this is unlikely to be significant).

### Neutral

N/A
Expand Down
7 changes: 7 additions & 0 deletions docs/docs/introduction/params.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,10 @@ This param would allow provider binaries to panic deterministically in the event

`RetryDelayPeriod` exists on the consumer for **ICS versions >= v3.2.0** (introduced by the implementation of [ADR-008](../adrs/adr-008-throttle-retries.md)) and is the period at which the consumer retries to send a `SlashPacket` that was rejected by the provider.


## Epoch Parameters

### BlocksPerEpoch
`BlocksPerEpoch` exists on the provider for **ICS versions >= 3.3.0** (introduced by the implementation of [ADR-014](../adrs/adr-014-epochs.md))
and corresponds to the number of blocks that constitute an epoch. This param is set to 600 by default and cannot exceed 1200.
Copy link
Contributor

Choose a reason for hiding this comment

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

wasn't there a plan to get rid of the maximum/make it much larger?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm in favor of keeping this upper limit. It just adds an additional check that in the best case will never be used and in the worst case will prevent from a potential bogus parameter.
Also, I do not believe that it is a very conservative upper limit. If the time we need for a block to be committed drops to something less than 6 seconds, then nothing bad will happen besides sending a greater number of VSCPackets. However, if the time to commit blocks increases (e.g., to 1 minute) then 1200 blocks correspond to 20 hours. If we add to this a potential multi-hour upgrade delay, then we might start having "unbonding pausing" issues with Neutron that has an unbonding period of 20 days.

We could get rid of this upper limit if we also looked at the BFT times when deciding when to send a VSCPacket, but this is not something we currently do, so I feel keeping an upper limit here is justified.

Copy link
Contributor

Choose a reason for hiding this comment

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

imo, it seems silly to me that, if for some reason we can't think of yet governance wants the parameter greater than 1200 blocks, there would first have to be an upgrade to Gaia itself before it could be done. keep in mind that for any param changes, there will need to be a vote anyways. I would recommend just giving clear guidance on appropriate values for the param in the docs instead of having the cap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just the removed the upper limit.

Assuming we need 6 seconds per block, the default value corresponds to 1 hour and the maximum to 2 hours.
14 changes: 14 additions & 0 deletions proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ message Params {
// The fee required to be paid to add a reward denom
cosmos.base.v1beta1.Coin consumer_reward_denom_registration_fee = 9
[ (gogoproto.nullable) = false ];

// The number of blocks that comprise an epoch.
int64 blocks_per_epoch = 10;
}

// SlashAcks contains cons addresses of consumer chain validators
Expand Down Expand Up @@ -295,3 +298,14 @@ message ConsumerAddrsToPrune {
uint64 vsc_id = 2;
AddressList consumer_addrs = 3;
}

// ConsumerValidator is used to facilitate epoch-based transitions. It contains relevant info for
// a validator that is expected to validate on a consumer chain during an epoch.
message ConsumerValidator {
// validator's consensus address on the provider chain
bytes provider_cons_addr = 1;
// voting power the validator has during this epoch
int64 power = 2;
// public key the validator uses on the consumer chain during this epoch
tendermint.crypto.PublicKey consumer_public_key = 3;
}
14 changes: 13 additions & 1 deletion tests/e2e/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,12 @@ func (tr TestConfig) relayPacketsGorelayer(
target ExecutionTarget,
verbose bool,
) {
// Because `.app_state.provider.params.blocks_per_epoch` is set to 3 in the E2E tests, we wait 3 blocks
// before relaying the packets to guarantee that at least one epoch passes and hence any `VSCPacket`s get
// queued and are subsequently relayed.
tr.waitBlocks(action.ChainA, 3, 90*time.Second)
tr.waitBlocks(action.ChainB, 3, 90*time.Second)

pathName := tr.GetPathNameForGorelayer(action.ChainA, action.ChainB)

// rly transact relay-packets [path-name] --channel [channel-id]
Expand All @@ -1455,6 +1461,12 @@ func (tr TestConfig) relayPacketsHermes(
target ExecutionTarget,
verbose bool,
) {
// Because `.app_state.provider.params.blocks_per_epoch` is set to 3 in the E2E tests, we wait 3 blocks
// before relaying the packets to guarantee that at least one epoch passes and hence any `VSCPacket`s get
// queued and are subsequently relayed.
tr.waitBlocks(action.ChainA, 3, 90*time.Second)
tr.waitBlocks(action.ChainB, 3, 90*time.Second)

// hermes clear packets ibc0 transfer channel-13
cmd := target.ExecCommand("hermes", "clear", "packets",
"--chain", string(tr.chainConfigs[action.ChainA].ChainId),
Expand Down Expand Up @@ -1763,7 +1775,7 @@ func (tr TestConfig) invokeDowntimeSlash(action DowntimeSlashAction, target Exec
// Bring validator down
tr.setValidatorDowntime(action.Chain, action.Validator, true, target, verbose)
// Wait appropriate amount of blocks for validator to be slashed
tr.waitBlocks(action.Chain, 10, 3*time.Minute)
tr.waitBlocks(action.Chain, 11, 3*time.Minute)
// Bring validator back up
tr.setValidatorDowntime(action.Chain, action.Validator, false, target, verbose)
}
Expand Down
Loading
Loading