Skip to content

Commit

Permalink
Merge branch 'feat/epochs' into ph/mbt-epochs
Browse files Browse the repository at this point in the history
  • Loading branch information
@p-offtermatt
p-offtermatt committed Mar 8, 2024
2 parents ccf24af + ff578da commit 9dba765
Show file tree
Hide file tree
Showing 16 changed files with 192 additions and 485 deletions.
116 changes: 17 additions & 99 deletions docs/docs/adrs/adr-001-key-assignment.md
View file
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
View file
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 consumer chains.

## 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 on 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
View file
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.
Assuming we need 6 seconds per block, the default value corresponds to 1 hour and the maximum to 2 hours.
16 changes: 7 additions & 9 deletions tests/e2e/actions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ func (tr TestConfig) submitConsumerAdditionProposal(
bz, err = target.ExecCommand(
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/temp-proposal.json"),
).CombinedOutput()

if err != nil {
log.Fatal(err, "\n", string(bz))
}
Expand All @@ -308,7 +307,6 @@ func (tr TestConfig) submitConsumerAdditionProposal(
`--keyring-backend`, `test`,
`-y`,
).CombinedOutput()

if err != nil {
log.Fatal(err, "\n", string(bz))
}
Expand Down Expand Up @@ -351,7 +349,6 @@ func (tr TestConfig) submitConsumerRemovalProposal(

bz, err = target.ExecCommand(
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/temp-proposal.json")).CombinedOutput()

if err != nil {
log.Fatal(err, "\n", string(bz))
}
Expand All @@ -368,7 +365,6 @@ func (tr TestConfig) submitConsumerRemovalProposal(
`--keyring-backend`, `test`,
`-y`,
).CombinedOutput()

if err != nil {
log.Fatal(err, "\n", string(bz))
}
Expand Down Expand Up @@ -427,7 +423,6 @@ func (tr TestConfig) submitParamChangeProposal(
bz, err = target.ExecCommand(
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/params-proposal.json"),
).CombinedOutput()

if err != nil {
log.Fatal(err, "\n", string(bz))
}
Expand Down Expand Up @@ -564,7 +559,7 @@ func (tr *TestConfig) transformConsumerGenesis(consumerChain ChainID, genesis []
panic(fmt.Sprintf("failed writing ccv consumer file : %v", err))
}
defer file.Close()
err = os.WriteFile(file.Name(), genesis, 0600)
err = os.WriteFile(file.Name(), genesis, 0o600)
if err != nil {
log.Fatalf("Failed writing consumer genesis to file: %v", err)
}
Expand Down Expand Up @@ -875,7 +870,6 @@ func (tr TestConfig) addChainToHermes(
"--chain", string(tr.chainConfigs[action.Chain].ChainId),
"--mnemonic-file", "/root/.hermes/mnemonic.txt",
).CombinedOutput()

if err != nil {
log.Fatal(err, "\n", string(bz))
}
Expand Down Expand Up @@ -1307,6 +1301,9 @@ 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)

Expand Down Expand Up @@ -1334,6 +1331,9 @@ 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)

Expand Down Expand Up @@ -1831,7 +1831,6 @@ func (tr TestConfig) submitChangeRewardDenomsProposal(action SubmitChangeRewardD

bz, err = target.ExecCommand(
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/change-reward-denoms-proposal.json")).CombinedOutput()

if err != nil {
log.Fatal(err, "\n", string(bz))
}
Expand All @@ -1847,7 +1846,6 @@ func (tr TestConfig) submitChangeRewardDenomsProposal(action SubmitChangeRewardD
`--keyring-backend`, `test`,
`-y`,
).CombinedOutput()

if err != nil {
log.Fatal(err, "\n", string(bz))
}
Expand Down
14 changes: 7 additions & 7 deletions tests/e2e/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func SlashThrottleTestConfig() TestConfig {
".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\" | " +
".app_state.provider.params.slash_meter_replenish_fraction = \"0.10\" | " +
".app_state.provider.params.slash_meter_replenish_period = \"20s\" | " +
".app_state.provider.params.blocks_per_epoch = 2",
".app_state.provider.params.blocks_per_epoch = 3",
},
ChainID("consu"): {
ChainId: ChainID("consu"),
Expand Down Expand Up @@ -290,7 +290,7 @@ func DefaultTestConfig() TestConfig {
".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\" | " +
".app_state.provider.params.slash_meter_replenish_fraction = \"1.0\" | " + // This disables slash packet throttling
".app_state.provider.params.slash_meter_replenish_period = \"3s\" | " +
".app_state.provider.params.blocks_per_epoch = 2",
".app_state.provider.params.blocks_per_epoch = 3",
},
ChainID("consu"): {
ChainId: ChainID("consu"),
Expand Down Expand Up @@ -320,7 +320,7 @@ func DemocracyTestConfig(allowReward bool) TestConfig {
".app_state.slashing.params.downtime_jail_duration = \"60s\" | " +
".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\" | " +
".app_state.transfer.params.send_enabled = false | " +
".app_state.provider.params.blocks_per_epoch = 2"
".app_state.provider.params.blocks_per_epoch = 3"

if allowReward {
// This allows the consumer chain to send rewards in the stake denom
Expand Down Expand Up @@ -351,7 +351,7 @@ func DemocracyTestConfig(allowReward bool) TestConfig {
".app_state.slashing.params.downtime_jail_duration = \"60s\" | " +
".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\" | " +
".app_state.provider.params.slash_meter_replenish_fraction = \"1.0\" | " + // This disables slash packet throttling
".app_state.provider.params.blocks_per_epoch = 2",
".app_state.provider.params.blocks_per_epoch = 3",
},
ChainID("democ"): {
ChainId: ChainID("democ"),
Expand Down Expand Up @@ -394,7 +394,7 @@ func MultiConsumerTestConfig() TestConfig {
".app_state.slashing.params.downtime_jail_duration = \"60s\" | " +
".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\" | " +
".app_state.provider.params.slash_meter_replenish_fraction = \"1.0\" | " + // This disables slash packet throttling
".app_state.provider.params.blocks_per_epoch = 2",
".app_state.provider.params.blocks_per_epoch = 3",
},
ChainID("consu"): {
ChainId: ChainID("consu"),
Expand Down Expand Up @@ -454,7 +454,7 @@ func ChangeoverTestConfig() TestConfig {
".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\" | " +
".app_state.provider.params.slash_meter_replenish_fraction = \"1.0\" | " + // This disables slash packet throttling
".app_state.provider.params.slash_meter_replenish_period = \"3s\" | " +
".app_state.provider.params.blocks_per_epoch = 2",
".app_state.provider.params.blocks_per_epoch = 3",
},
ChainID("sover"): {
ChainId: ChainID("sover"),
Expand Down Expand Up @@ -555,7 +555,7 @@ func ConsumerMisbehaviourTestConfig() TestConfig {
".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\" | " +
".app_state.provider.params.slash_meter_replenish_fraction = \"1.0\" | " + // This disables slash packet throttling
".app_state.provider.params.slash_meter_replenish_period = \"3s\" | " +
".app_state.provider.params.blocks_per_epoch = 2",
".app_state.provider.params.blocks_per_epoch = 3",
},
ChainID("consu"): {
ChainId: ChainID("consu"),
Expand Down
Loading

0 comments on commit 9dba765

Please sign in to comment.