Skip to content

Commit

Permalink
more cleaning up
Browse files Browse the repository at this point in the history
  • Loading branch information
@insumity
insumity committed Sep 6, 2023
1 parent 65141e4 commit c20c0c2
Showing 1 changed file with 28 additions and 59 deletions.
87 changes: 28 additions & 59 deletions docs/docs/adrs/adr-013-equivocation-slashing.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,13 @@ In Cosmos Hub, the `MaxAgeNumBlocks` is set to 1000000 (i.e., ~70 days if we ass

### Slashing on the provider
We see that in the single-chain slashing case, we use the `infractionHeight` and the voting `power` to be able to slash.
In order to slash the provider chain for a consumer chain equivocation we need to have the provider's `infractionHeight`
In order to slash the provider for a consumer equivocation we need to have the provider's `infractionHeight`
and voting `power`. However, we do **not** have those values. We only have the `infractionHeight` in the consumer chain
(that we can extract from the votes), but we do not know to what provider height this `infractionHeight` corresponds.
Unless we have a way to find the corresponding `infractionHeight`
and `power` in the provider chain, we cannot directly slash in the same way as we slash in the single-chain case.
(that we can extract from the equivocated votes), but we do not know to what provider height this `infractionHeight` corresponds.
Unless we have a way to find the corresponding `infractionHeight` and `power` in the provider chain, we cannot slash the provider in the same way as we would slash in the single-chain case.

Someone might think that the problem of figuring out the corresponding `infractionHeight` and `power` values in
the provider chain is easy because we could have the consumer chain send us this information. However, we
consider that the application on the consumer chain could be _malicious_ and hence we
cannot really trust anything that stems from the _application state_ of the consumer chain.
Someone might think that the problem of figuring out the corresponding `infractionHeight` and `power` values in the provider chain is easy because we could have the consumer chain send us this information.
However, we consider that the application on the consumer chain could be _malicious_ and hence we cannot really trust anything that stems from the _application state_ of the consumer chain.

Note that when a relayer or a user sends evidence through a [MsgSubmitConsumerDoubleVoting](https://github.com/cosmos/interchain-security/pull/1232/files) message, we get what is contained in the [DuplicateVoteEvidence](https://github.com/cometbft/cometbft/blob/v0.37.0/types/evidence.go#L35):
```protobuf
Expand All @@ -74,12 +71,11 @@ type DuplicateVoteEvidence struct {
Timestamp time.Time
}
```
The "abci specific information" cannot be trusted because they are not signed. Therefore,
we cannot use the `ValidatorPower` in any way for slashing in the provider chain. We can get the `infractionHeight`
from the votes but this corresponds to the infraction height on the consumer chain. Furthermore, note that the [Timestamp](https://github.com/cometbft/cometbft/blob/v0.37.0/types/vote.go#L55) in the votes is just the [BFT time](https://github.com/cometbft/cometbft/blob/v0.37.0/spec/consensus/bft-time.md), and the time could be anything since a misbehaving validator could have included any time in the votes.
The "abci specific information" fields cannot be trusted because they are not signed. Therefore,
we cannot use `ValidatorPower` in any way for slashing in the provider chain. We can get the `infractionHeight` from the votes, but as already mentioned, this `infractionHeight` corresponds to the infraction height on the consumer and **not** on the provider chain.
Furthermore, note that the [Timestamp](https://github.com/cometbft/cometbft/blob/v0.37.0/types/vote.go#L55) in the votes is just the [BFT time](https://github.com/cometbft/cometbft/blob/v0.37.0/spec/consensus/bft-time.md), and the time could be anything since a misbehaving validator could have included any time in the votes.

Finally, note that in the single-chain case, we trust the underlying consensus engine (CometBFT) and hence when we receive
evidence from CometBFT we can act on it. In the case of provider and consumer chains however we cannot trust the evidence as is.
Finally, note that in the single-chain case, we trust the underlying consensus engine (CometBFT) and hence when we receive evidence from CometBFT we can act on it. In the case of provider and consumer chains however we cannot trust the evidence as is (e.g., evidence is submitted by a relayer that could set arbitrary values to the "abci specific information" fields).



Expand All @@ -88,35 +84,25 @@ evidence from CometBFT we can act on it. In the case of provider and consumer ch

Conceptually, we have 2 different chains and there’s no guarantee that the clock drift between them is bounded.
One could have BFT time of 2023 on one chain and on the other chain a BFT time of 2013.
Nevertheless, the chains communicate over IBC and
hence some timing constraints are imposed by IBC otherwise the chains could not communicate (e.g., light clients could expire).
Nevertheless, the chains communicate over IBC and hence some timing constraints are imposed by IBC otherwise the chains could not communicate (e.g., light clients could expire).
IBC clients contain a `maxClockDrift` parameter but this is [only used](https://github.com/cometbft/cometbft/blob/v0.37.0/light/verifier.go#L176)
to reject headers that come slightly from the future. For example, assume a chain `A` that has a light client `lc` that
tracks chain `B` and that has `maxClockDrift` of 10 seconds. If chain `A` has time `12:58:31` and
then `lc` receives a header with time `12:58:42` (that is 11 seconds from the future), then `lc` would reject his client update.
However, even with this `maxClockDrift` constraint we still do not know the time at the chain `B`, e.g., `B`'s BFT time could be 5 years ahead
compared to chain's `A` BFT time.
to reject headers that come slightly from the future. For example, assume a chain `A` that has a light client `lc` that tracks chain `B` and that has `maxClockDrift` of 10 seconds. If chain `A` has time `12:58:31` and then `lc` receives a header with time `12:58:42` (that is 11 seconds from the future), then `lc` would reject his client update.
However, even with this `maxClockDrift` constraint we still do not know the time at the chain `B`, e.g., `B`'s BFT time could be 5 years ahead compared to chain's `A` BFT time.

(TODO: add more on the `trustingPeriod`) only tells us from the provider side that we have received a message from the other chain in the
last `trustingPeriod` (2 weeks in Cosmos Hub) before ... so doesn't add much either ...
(TODO: add more on the `trustingPeriod`) only tells us from the provider side that we have received a message from the other chain in the last `trustingPeriod` (2 weeks in Cosmos Hub) before ... so doesn't add much either ...

We currently investigate on whether we can bound the clock drift between the provider and the consumer chain.

## Proposed solution
As a first iteration, in order to implement the slashing functionality as soon as possible, we propose an aggressive (i.e.,
would slash more delegators than in the single-chain case) slashing approach because this allows for a straight-forward implementation.
As a first iteration, in order to implement the slashing functionality as soon as possible, we propose an aggressive (i.e., would slash more delegators than in the single-chain case) slashing approach because this allows for a straight-forward implementation.
In this approach, at the moment we receive evidence, at _evidence height_, we:
1. slash all the undelegations and redelegations using the `slashFactor`;
2. slash all delegations using as voting `power` the sum of the voting power of the misbehaving validator and the
power of all the ongoing undelegations and redelegations.
1. slash all the undelegations and redelegations using `slashFactor`;
2. slash all delegations using as voting `power` the sum of the voting power of the misbehaving validator and the power of all the ongoing undelegations and redelegations.

**Evidence expiration:** Additionally, in this approach and because of what we explained earlier, because at the moment
we cannot infer the actual time the evidence was created (i.e., timestamp in evidence could contain anything), we do not consider
_evidence expiration_ and hence we can receive and act on evidence from a time in the past (e.g., 3 years ago).
**Evidence expiration:** Additionally, in this approach and because we cannot infer the actual time of the evidence (i.e., timestamp in evidence could contain anything), we do not consider _evidence expiration_ and hence old evidence is never ignored (e.g., we would act on 3 year-old evidence).

### Implementation
The aggressive approach allows for multiple simplifications. Specifically, we can introduce the following snippet
in the [HandleConsumerDoubleVoting](https://github.com/cosmos/interchain-security/pull/1232) method:
The aggressive approach allows for multiple simplifications. Specifically, we intend to introduce the following logic in the [HandleConsumerDoubleVoting](https://github.com/cosmos/interchain-security/pull/1232) method:
```go
undelegationsTotalAmount := sdk.NewInt(0)
for _, v := range k.stakingKeeper.GetUnbondingDelegationsFromValidator(ctx, validatorAddress) {
Expand All @@ -139,26 +125,15 @@ slashFraction := k.slashingKeeper.SlashFractionDoubleSign(ctx)
k.stakingKeeper.Slash(ctx, validatorConsAddress, infractionHeight, totalPower, slashFraction, DoubleSign)
```

**Infraction height:** We provide a zero `infractionHeight` to the [Slash](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0/x/staking/keeper/slash.go#L33) method in order to slash all ongoing undelegations and redelegations (see checks in [Slash](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0/x/staking/keeper/slash.go#L92),
[SlashUnbondingDelegation](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0/x/staking/keeper/slash.go#L195), and
[SlashRedelegation](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0/x/staking/keeper/slash.go#L249)).
**Infraction height:** We provide a zero `infractionHeight` to the [Slash](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0/x/staking/keeper/slash.go#L33) method in order to slash all ongoing undelegations and redelegations (see checks in [Slash](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0/x/staking/keeper/slash.go#L92), [SlashUnbondingDelegation](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0/x/staking/keeper/slash.go#L195), and [SlashRedelegation](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0/x/staking/keeper/slash.go#L249)).

**Power:** We pass the sum of the voting power of the misbehaving validator and the
power of all the ongoing undelegations and redelegations. This is a slightly more aggressive approach than just providing
the voting `power` at the evidence height. If we assume that the `slashFactor` is 5%, then the `power` we pass
is `validatorPower + totalPower(undelegations) + totalPower(redelegations)`. Hence, when the `Slash`
method slashes all the undelegations and redelegations it would end up with
`0.05 * validatorPower + 0.05 * totalPower(undelegations) + 0.05 * totalPower(redelegations) - 0.05 * totalPower(undelegations) - 0.05 * totalPower(redelegations) = 0.05 * validatorPower`
and hence it would slash 5% of the `validatorPower` at evidence height.

**Storing evidence:** As mentioned, we do not expire evidence with this aggresive approach. However, because we do not want to
slash the same validator more than once for the same infraction, we have to store the evidence in some _cache_ in order
to check if we have already slashed based on received evidence. We could do something similar to the following:
**Power:** We pass the sum of the voting power of the misbehaving validator and the power of all the ongoing undelegations and redelegations. This is a slightly more aggressive approach than just providing the voting `power` at the evidence height. If we assume that the `slashFactor` is 5%, then the `power` we pass is `validatorPower + totalPower(undelegations) + totalPower(redelegations)`.
Hence, when the `Slash` method slashes all the undelegations and redelegations it would end up with `0.05 * validatorPower + 0.05 * totalPower(undelegations) + 0.05 * totalPower(redelegations) - 0.05 * totalPower(undelegations) - 0.05 * totalPower(redelegations) = 0.05 * validatorPower` and hence it would slash 5% of the `validatorPower` at evidence height.

**Storing evidence:** As mentioned, we do not expire evidence with this aggressive approach. However, because we do not want to slash the same validator more than once for the same infraction, we have to store the evidence in some _cache_ in order to check if we have already slashed that validator. We intend to do the following:
```go

// hashEvidence should only use the signed parts of the evidence to generate the hash
// therefore, it should only use VoteA and VoteB to avoid someone changing other parts DuplicateVoteEvidence
// therefore, it should only use VoteA and VoteB to avoid someone changing other parts of DuplicateVoteEvidence
// and resubmitting
key := hashEvidence(dve DuplicateVoteEvidence)

Expand All @@ -169,22 +144,16 @@ if key not in cache {
// do not slash
}
```
To prevent this cache from growing arbitrarily big, we can introduce some additional checks. For example, if the evidence
is for a validator that is not in the active validator set we can skip from storing it in the cache, etc.
We can also periodically clean up the cache by checking if the validator `validatorAddr` of a cache entry
is still in the active set and if not we remove that entry from the cache.
To prevent this cache from growing arbitrarily big, we introduce additional checks. If the evidence is for a validator that is not in the active validator set we skip from storing it in the cache, etc.
We also periodically clean up the cache by checking if the validator `validatorAddr` of a cache entry is still in the active set and if not we remove that entry from the cache.

### Positive
With the proposed approach we can quickly implement slashing functionality on the provider chain for consumer chain equivocations.
This approach does not need any changes in the staking module and therefore does not change in any way how slashing is performed
today for a single chain.
This approach does not need to change the staking module and therefore does not change in any way how slashing is performed today for a single chain.

### Negative
We _definitely_ slash more when it comes to undelegations and redelegations because we slash for all of them without
considering an `infractionHeight`.
We _potentially_ slash more than what we would have slashed if we knew the corresponding `votingPower` in the provider
chain.

We _definitely_ slash more when it comes to undelegations and redelegations because we slash for all of them without considering an `infractionHeight`.
We _potentially_ slash more than what we would have slashed if we knew the voting `power` at the corresponding `infractionHeight` in the provider chain.


## Can we expire the evidence?
Expand Down

0 comments on commit c20c0c2

Please sign in to comment.