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.

Sign up for GitHub

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

Jump to bottom

Fixing and updating QA plan #375

Merged
merged 14 commits into from
Oct 18, 2022
Merged

Fixing and updating QA plan #375

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b765163
fixing and updating QA plan
mpoke Oct 5, 2022
fd40bcc
add link to 4.07 e2e test
mpoke Oct 7, 2022
c70dff9
add link to issue
shaspitz Oct 7, 2022
33d3704
Merge branch 'main' into marius/qa-update
shaspitz Oct 7, 2022
8ecdea3
fix typo
mpoke Oct 11, 2022
cad1339
Merge branch 'main' into marius/qa-update
mpoke Oct 17, 2022
c7381b8
updating links
mpoke Oct 17, 2022
704aa16
Update docs/quality_assurance.md
mpoke Oct 18, 2022
3376533
remove ambigous concern
mpoke Oct 18, 2022
048d09b
Merge branch 'main' into marius/qa-update
mpoke Oct 18, 2022
d469ac2
Patches qa plan with diff testing changes (#399)
danwt Oct 18, 2022
b68dde8
markdown linter
mpoke Oct 18, 2022
cad6a68
fix merge conflicts
mpoke Oct 18, 2022
094f3d4
formatting
mpoke Oct 18, 2022
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
75 changes: 37 additions & 38 deletions docs/quality_assurance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ The verification of Interchain Security is split across the following concerns:
- The correct integration with IBC (i.e., [ibc-go](https://github.com/cosmos/ibc-go/tree/v3.0.0)).
- The correct integration with Cosmos SDK (i.e., [cosmos-sdk](https://github.com/cosmos/cosmos-sdk/tree/v0.45.6)).
- The correctness of the provider chain, i.e., the provider CCV module does not break the liveness or the safety of the provider chain.
- The correctness of the Interchain Security protocol, i.e., the protocol follows the [specification](https://github.com/cosmos/ibc/blob/master/spec/appics-028-cross-chain-validation/README.md).
- The correctness of the Interchain Security protocol, i.e., the protocol follows the [specification](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/README.md).
- The correctness of the consumer chain, i.e., both liveness or safety hold.

For an overview of the Interchain Security workflow, have a look at [the diagrams](#interchain-security-workflow) at the end of this document.
Expand Down Expand Up @@ -43,26 +43,25 @@ IBC packets:

| ID | Concern | Code Review | Unit Testing | E2E Testing | Diff. Testing | Testnet |
| -- | ------- | ----------- | ------------ | ----------- | ------------- | ------- |
| 2.01 | Create IBC clients | `Scheduled` (ibc-go team) | `Done` | `??` | `Future work` | `Scheduled` |
| 2.02 | Getting consumer `UnbondingPeriod` from IBC client | `Scheduled` (ibc-go team) | `Done`, see TestUnbondingTime` | `??` | `NA` | `NA` |
| 2.03 | Create CCV channel (handshake) | `Scheduled` (ibc-go team) | `Done` | `NA` | `Future work` | `Scheduled` |
| 2.04 | Sending IBC packets <br /> - see `x/ccv/utils/utils.go:SendIBCPacket()` | `Scheduled` (ibc-go team) | `??` | `Done` | `Done` | `Scheduled` |
| 2.05 | Handling acknowledgments | `Scheduled` (ibc-go team) | `Partial Coverage` | `Partial coverage` | `Scheduled` | `Scheduled` |
| 2.06 | Handling timeouts | `Scheduled` (ibc-go team) | `??` |`??` | `Future work` | `Scheduled` |
| 2.07 | **Handling IBC client expiration** | `Scheduled` (ibc-go team) <br /> high priority | `??` | `??` | `Future work` | `Scheduled` |
| 2.08 | ICS-20 channel creation | `Scheduled` (ibc-go team) | `??` | `??` |`Future work` | `Scheduled` |
| 2.09 | ICS-20 transfer | `Scheduled` (ibc-go team) | `??` | `??` | `NA` | `Scheduled` |
| 2.10 | Changes in IBC-GO testing suite | `Scheduled` (ibc-go team) | `NA` | `??` | `Partial coverage` | `NA` |
| 2.01 | Create IBC clients | `Scheduled` (ibc-go team) | `Done` (TODO: link) | `??` | `Future work` | `Scheduled` |
| 2.02 | Create CCV channel (handshake) | `Scheduled` (ibc-go team) | `Done` [provider](../x/ccv/provider/ibc_module_test.go) and [consumer](../x/ccv/consumer/ibc_module_test.go) | `NA` | `Future work` | `Scheduled` |
| 2.03 | Sending IBC packets <br /> - see `x/ccv/utils/utils.go:SendIBCPacket()` | `Scheduled` (ibc-go team) | `??` | `Done` (TODO: link) | `Done` | `Scheduled` |
| 2.04 | Handling acknowledgments | `Scheduled` (ibc-go team) | `Partial Coverage` (TODO: link) | `Partial coverage` (TODO: link) | `Scheduled` | `Scheduled` |
| 2.05 | Handling timeouts | `Scheduled` (ibc-go team) | `??` |`??` | `Future work` | `Scheduled` |
| 2.06 | **Handling IBC client expiration** | `Scheduled` (ibc-go team) <br /> high priority | `??` | `??` | `Future work` | `Scheduled` |
| 2.07 | ICS-20 channel creation | `Scheduled` (ibc-go team) | `??` | `??` |`Future work` | `Scheduled` |
| 2.08 | ICS-20 transfer | `Scheduled` (ibc-go team) | `??` | `??` | `NA` | `Scheduled` |
| 2.09 | Changes in IBC-GO testing suite | `Scheduled` (ibc-go team) | `NA` | `??` | `Partial coverage` | `NA` |

### Integration with Cosmos SDK

A prerequisite of the code review is to open a PR with all the [SDK changes](https://github.com/cosmos/cosmos-sdk/tree/interchain-security-rebase) needed by Interchain Security.
- [x] A prerequisite of the code review is to open a PR with all the [SDK changes](https://github.com/cosmos/cosmos-sdk/tree/interchain-security-rebase) needed by Interchain Security.

| ID | Concern | Code Review | Unit Testing | E2E Testing | Diff. Testing | Testnet |
| -- | ------- | ----------- | ------------ | ----------- | ------------- | ------- |
| 3.01 | Changes to staking module | `Scheduled` (sdk team) | `??` | `Partial coverage` <br /> see [unbonding_test.go](../tests/e2e/unbonding_test.go) <br /> redelegation could be expanded, validator unbonding missing | `Partial coverage` | `Scheduled` |
| 3.02 | Changes to slashing module | `Scheduled` (sdk team) | `??` | `Done` <br /> see [TestValidatorDowntime](../tests/e2e/slashing_test.go#L502) <br /> | `NA` | `Scheduled` |
| 3.03 | Changes to evidence module | `Scheduled` (sdk team) | `??` | `Done` <br /> see [TestValidatorDoubleSigning](../tests/e2e/slashing_test.go#L584) <br /> | `NA` | `Scheduled` |
| 3.01 | Changes to staking module | `Done` | `??` | `Partial coverage` <br /> see [unbonding_test.go](../tests/e2e/unbonding_test.go) <br /> redelegation could be expanded, validator unbonding missing | `Partial coverage` | `Scheduled` |
| 3.02 | Changes to slashing module | `Done` | `??` | `Done` <br /> see [TestValidatorDowntime](../tests/e2e/slashing_test.go#L502) <br /> | `NA` | `Scheduled` |
| 3.03 | Changes to evidence module | `Done` | `??` | `Done` <br /> see [TestValidatorDoubleSigning](../tests/e2e/slashing_test.go#L584) <br /> | `NA` | `Scheduled` |

### Provider Chain Correctness

Expand All @@ -73,27 +72,27 @@ The main concern addressed in this section is the correctness of the provider ch

| ID | Concern | Code Review | Unit | E2e | Diff. Testing | Testnet | Protocol audit |
| -- | ------- | ----------- | ---- | --- | ------------- | ------- | -------------- |
| 4.01 | Liveness of undelegations <br /> - unbonding delegation entries are eventually removed from `UnbondingDelegation` | `Scheduled` | `NA` | `Done` <br /> see [here](../tests/e2e/unbonding_test.go) | `Done` | `Scheduled` | `Scheduled` |
| 4.02 | Liveness of redelegations <br /> - redelegations entries are eventually removed from `Redelegations` | `NA` | `Scheduled` | `Scheduled` | `Scheduled` | `Scheduled` | `Scheduled` |
| 4.03 | Liveness of validator unbondings <br /> - unbonding validators with no delegations are eventually removed from `Validators` | `NA` | `Scheduled` | `Scheduled` | `Done` | `Scheduled` | `Scheduled` |
| 4.04 | Unbonding operations (undelegations, redelegations, validator unbondings) should eventually complete even if the CCV channel is never established (due to error) <br /> - expected outcome: the pending VSC packets eventually timeout, which leads to the consumer chain removal | `Scheduled` | `NA` | `??` | `Future work` | `Scheduled` | `Scheduled` <br /> high priority |
| 4.05 | Unbonding operations (undelegations, redelegations, validator unbondings) should eventually complete even if one of the clients expire <br /> - expected outcome: the pending VSC packets eventually timeout, which leads to the consumer chain removal | `Scheduled` | `??` | `??` | `Future work` | `Scheduled` | `Scheduled` <br /> high priority |
| 4.06 | A validator cannot get slashed more than once for double signing, regardless of how many times it double signs on different chains (consumers or provider) | `Scheduled` | `NA` |`Done` <br /> see [here](../tests/e2e/slashing_test.go#L317) | `Done` | `Scheduled` | `NA` |
| 4.07 | A validator cannot get slashed multiple times for downtime on the same consumer chain without requesting to `Unjail` itself on the provider chain in between | `Scheduled` | `NA` | `Done` <br /> see [here](../tests/e2e/slashing_test.go#642)| `Partial coverage` | `Scheduled` | `NA` |
| 4.01 | Liveness of undelegations <br /> - unbonding delegation entries are eventually removed from `UnbondingDelegation` | `Scheduled` | `NA` | `Done` <br /> see [unbonding_test.go](../tests/e2e/unbonding_test.go) | `Done` | `Scheduled` | `NA` |
| 4.02 | Liveness of redelegations <br /> - redelegations entries are eventually removed from `Redelegations` | `Scheduled` | `NA` | `Scheduled` | `Scheduled` | `Scheduled` | `NA` |
| 4.03 | Liveness of validator unbondings <br /> - unbonding validators with no delegations are eventually removed from `Validators` | `Scheduled` | `NA` | `NA` | `Done` | `Scheduled` | `NA` |
| 4.04 | Unbonding operations (undelegations, redelegations, validator unbondings) should eventually complete even if the CCV channel is never established (due to error) <br /> - expected outcome: the pending VSC packets eventually timeout, which leads to the consumer chain removal <br /> - requires https://github.com/cosmos/interchain-security/issues/278 | `Scheduled` | `NA` | `Scheduled` | `Future work` | `Scheduled` | `Done` |
| 4.05 | Unbonding operations (undelegations, redelegations, validator unbondings) should eventually complete even if one of the clients expire <br /> - expected outcome: the pending VSC packets eventually timeout, which leads to the consumer chain removal <br /> - requires https://github.com/cosmos/interchain-security/issues/283 | `Scheduled` | `NA` | `Scheduled` | `Future work` | `Scheduled` | `NA` |
| 4.06 | A validator cannot get slashed more than once for double signing, regardless of how many times it double signs on different chains (consumers or provider) | `Scheduled` | `NA` |`Done` <br /> see [TestHandleSlashPacketErrors](../tests/e2e/slashing_test.go#L317) | `Done` | `Scheduled` | `NA` |
| 4.07 | A validator cannot get slashed multiple times for downtime on the same consumer chain without requesting to `Unjail` itself on the provider chain in between | `Scheduled` | `NA` | `Done` <br /> (TODO: link) | `Partial coverage` | `Scheduled` | `NA` |
mpoke marked this conversation as resolved.
Show resolved Hide resolved
| 4.08 | A validator can be slashed multiple times for downtime on different chains | `Scheduled` | `NA` | `Future work` | `NA` | `Scheduled` | `NA` |
| 4.09 | The provider chain can easily be restarted with IS enabled <br /> - `ExportGenesis` & `InitGenesis` | `Scheduled` | `??` | `Future work` | `Future work` | `Scheduled` | `NA` |
| 4.10 | The provider chain's correctness is not affected by a consumer chain shutting down | `Scheduled` | `NA` | `Future work` | `Future work` | `Scheduled` | `NA` |
| 4.11 | The provider chain can graciously handle a CCV packet timing out (without shuting down) <br /> - expected outcome: consumer chain shuts down and its state in provider CCV module is removed | `Scheduled` | `??` | `Future work` | `Future work` | `Scheduled` | `NA` |
| 4.12 | The provider chain can graciously handle a `ConsumerRemovalProposal` <br /> - expected outcome: consumer chain shuts down and its state in provider CCV module is removed | `Scheduled` | `Done` <br /> see [here](../x/ccv/provider/keeper/proposal_test.go#L313) | `NA` | `Future work` | `Scheduled` | `NA` |
| 4.13 | The provider chain can graciously handle a `ConsumerAdditionProposal` <br /> - expected outcome: a consumer chain is registered and a client is created | `Scheduled` |`Done` <br /> see [here](../x/ccv/provider/keeper/proposal_test.go#L31) | `NA` | `Future work` | `Scheduled` | `NA` |
| 4.09 | The provider chain can easily be restarted with IS enabled <br /> - `ExportGenesis` & `InitGenesis` | `Scheduled` | `??` (Simon?) | `Future work` (Simon?) | `Future work` | `Scheduled` | `NA` |
mpoke marked this conversation as resolved.
Show resolved Hide resolved
| 4.10 | The provider chain's correctness is not affected by a consumer chain shutting down | `Scheduled` | `NA` (Simon?) | `Future work` (Simon?) | `Future work` | `Scheduled` | `NA` |
| 4.11 | The provider chain can graciously handle a CCV packet timing out (without shuting down) <br /> - expected outcome: consumer chain shuts down and its state in provider CCV module is removed | `Scheduled` | `??` (Simon?) | `Future work` (Simon?) | `Future work` | `Scheduled` | `NA` |
| 4.12 | The provider chain can graciously handle a `ConsumerRemovalProposal` <br /> - expected outcome: consumer chain shuts down and its state in provider CCV module is removed | `Scheduled` | `Done` <br /> see [TestHandleConsumerRemovalProposal](../x/ccv/provider/keeper/proposal_test.go#L313) | `NA` | `Future work` | `Scheduled` | `NA` |
| 4.13 | The provider chain can graciously handle a `ConsumerAdditionProposal` <br /> - expected outcome: a consumer chain is registered and a client is created | `Scheduled` |`Done` <br /> see [TestHandleConsumerAdditionProposal](../x/ccv/provider/keeper/proposal_test.go#L31) | `NA` | `Future work` | `Scheduled` | `NA` |

### Interchain Security Protocol Correctness

The main concern addressed in this section is the correctness of the Interchain Security protocol. In other words, the implementation should be aligned with the Interchain Security [specification](https://github.com/cosmos/ibc/blob/master/spec/appics-028-cross-chain-validation/README.md).
The main concern addressed in this section is the correctness of the Interchain Security protocol. In other words, the implementation should be aligned with the Interchain Security [specification](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/README.md).

The implementation MUST guarantee the *Channel Uniqueness* property, i.e., the channel between the provider chain and a consumer chain MUST be unique.

In addition, the implementation MUST guarantee the following [system properties](https://github.com/cosmos/ibc/blob/master/spec/appics-028-cross-chain-validation/system_model_and_properties.md#system-properties):
In addition, the implementation MUST guarantee the following [system properties](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/system_model_and_properties.md#system-properties):
- *Validator Set Replication*
- *Bond-Based Consumer Voting Power*
- *Slashable Consumer Misbehavior*
Expand All @@ -103,32 +102,32 @@ In addition, the implementation MUST guarantee the following [system properties]

| ID | Concern re. *Channel Uniqueness* | Code Review | Unit Testing | E2e Testing | Diff. Testing | Testnet | Protocol audit |
| -- | -------------------------------- | ----------- | ------------ | ----------- | ------------- | ------- | -------------- |
| 5.01 | `HandleConsumerAdditionProposal()` should fail if a consumer with `chainId` is already registered | `Scheduled` | `DONE` see [here](../x/ccv/provider/keeper/proposal_test.go#L138) | `??` | `NA` | `Scheduled` | `Scheduled` <br /> high priority |
| 5.02 | The channel handshake for a consumer with `chainId` should fail if there is already an established CCV channel for `chainId` | `Scheduled` | `DONE` see [here](../x/ccv/provider/ibc_module_test.go#L103) and [here](../x/ccv/consumer/ibc_module_test.go#L59) | `??` | `NA` | `Scheduled` | `Scheduled` <br /> high priority |
| 5.03 | *Channel Uniqueness* should hold even if a consumer chain restarts | `Scheduled` | `NA` | `??` | `NA` | `Scheduled` | `NA` |
| 5.04 | *Channel Uniqueness* should hold even when a client expires | `Scheduled` | `??` | `NA` | `NA` | `Scheduled` | `NA` |
| 5.01 | `HandleConsumerAdditionProposal()` should fail if a consumer with `chainId` is already registered | `Scheduled` | `Done` see [TestCreateConsumerClient](../x/ccv/provider/keeper/proposal_test.go#L116) | `NA` | `NA` | `Scheduled` | `NA` |
| 5.02 | The channel handshake for a consumer with `chainId` should fail if there is already an established CCV channel for `chainId` | `Scheduled` | `Done` see [TestOnChanOpenTry](../x/ccv/provider/ibc_module_test.go#L103) and [TestOnChanOpenInit](../x/ccv/consumer/ibc_module_test.go#L59) | `NA` | `NA` | `Scheduled` | `NA` |
| 5.03 | *Channel Uniqueness* should hold even if a consumer chain restarts | `Scheduled` | `NA` | `Scheduled` | `NA` | `Scheduled` | `NA` |
| 5.04 | *Channel Uniqueness* should hold even when a client expires | `Scheduled` | `NA` | `Scheduled` | `NA` | `Scheduled` | `NA` |

---

| ID | Concern re. *Validator Set Replication* | Code Review | Unit Testing | E2e Testing | Diff. testing | Testnet | Protocol audit |
| -- | --------------------------------------- | ----------- | ------------ | ----------- | ------------- | ------- | -------------- |
| 6.01 | Every validator set on any consumer chain MUST either be or have been a validator set on the provider chain. | `Scheduled` | `NA` | `NA` | `Done` | `Scheduled` | `Scheduled` |
| 6.02 | Any update in the power of a validator `val` on the provider, as a result of <br /> - (increase) `Delegate()` / `Redelegate()` to `val` <br /> - (increase) `val` joining the provider validator set <br /> - (decrease) `Undelegate()` / `Redelegate()` from `val` <br /> - (decrease) `Slash(val)` <br /> - (decrease) `val` leaving the provider validator set <br /> MUST be present in a `ValidatorSetChangePacket` that is sent to all registered consumer chains | `Scheduled` | `NA` | `NA` | `Done` | `Scheduled` | `Scheduled` |
| 6.03 | Every consumer chain receives the same sequence of `ValidatorSetChangePacket`s in the same order. | `Scheduled` | `NA` | `NA` | `NA` | `Scheduled` | `Scheduled` <br /> high priority |
| 6.03 | Every consumer chain receives the same sequence of `ValidatorSetChangePacket`s in the same order. | `Scheduled` | `NA` | `NA` | `NA` (Dan?) | `Scheduled` | `Scheduled` <br /> high priority |

---

| ID | Concern re. *Bond-Based Consumer Voting Power* | Code Review | Unit Testing | E2e Testing | Diff. Testing | Testnet | Protocol audit |
| -- | ---------------------------------------------- | ----------- | ------------ | ----------- | ------------- | ------- | -------------- |
| 7.01 | For every `ValidatorSetChangePacket` received by a consumer chain at time `t`, a `MaturedVSCPacket` is sent back to the provider in the first block with a timestamp `>= t + UnbondingPeriod` | `Scheduled` | `??` | `Scheduled` | `Done` | `Scheduled` | `Scheduled` |
| 7.02 | If an unbonding operation resulted in a `ValidatorSetChangePacket` sent to all registered consumer chains, then it cannot complete before receiving matching `MaturedVSCPacket`s from these consumer chains (unless some of these consumer chains are removed) | `Scheduled` | `??` | `Scheduled` | `Done` | `Scheduled` | `Scheduled` |
| 7.01 | For every `ValidatorSetChangePacket` received by a consumer chain at time `t`, a `MaturedVSCPacket` is sent back to the provider in the first block with a timestamp `>= t + UnbondingPeriod` | `Scheduled` | `NA` | `NA` | `Done` | `Scheduled` | `Scheduled` |
| 7.02 | If an unbonding operation resulted in a `ValidatorSetChangePacket` sent to all registered consumer chains, then it cannot complete before receiving matching `MaturedVSCPacket`s from these consumer chains (unless some of these consumer chains are removed) | `Scheduled` | `NA` | `NA` | `Done` | `Scheduled` | `Scheduled` |

---

| ID | Concern re. *Slashable Consumer Misbehavior* | Code Review | Unit Testing | E2e Testing | Diff. testing | Testnet | Protocol audit |
| -- | -------------------------------------------- | ----------- | ------------ | ----------- | ------------- | ------- | -------------- |
| 8.01 | Multiple downtime infractions committed by the same validator `val` on the same consumer chain without `val` requesting to `Unjail` itself result in a single `SlashPacket` | `Scheduled` | `??` | `??` | `??` | `Done` | `Scheduled` | `Scheduled` |
| 8.02 | If evidence of misbehavior is submitted on a consumer chain within the unbonding period targeting an amount `x` of staked tokens, the amount `x` cannot be unlocked on the provider before the corresponding `SlashPacket` is received <br /> - `SlashPacket` will not arrive after the corresponding `MaturedVSCPacket`s | `Scheduled` | `??` | `??` | `??` | `Done` | `Scheduled` | `Scheduled` |
| 8.01 | Multiple downtime infractions committed by the same validator `val` on the same consumer chain without `val` requesting to `Unjail` itself result in a single `SlashPacket` | `Scheduled` | `NA` | `NA` | `Done` | `Scheduled` | `NA` |
| 8.02 | If evidence of misbehavior is submitted on a consumer chain within the unbonding period targeting an amount `x` of staked tokens, the amount `x` cannot be unlocked on the provider before the corresponding `SlashPacket` is received <br /> - `SlashPacket` will not arrive after the corresponding `MaturedVSCPacket`s | `Scheduled` | `NA` | `NA` | `Done` | `Scheduled` | `NA` |

---

Expand Down