From d97bc5e57595f3586d4b5064458dfeb10b040e8c Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Tue, 23 Jun 2020 14:38:16 +0200 Subject: [PATCH 1/6] Initial ADR --- .../adr-026-ibc-recovery-mechanisms.md | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 docs/architecture/adr-026-ibc-recovery-mechanisms.md diff --git a/docs/architecture/adr-026-ibc-recovery-mechanisms.md b/docs/architecture/adr-026-ibc-recovery-mechanisms.md new file mode 100644 index 000000000000..3b9b023f31fd --- /dev/null +++ b/docs/architecture/adr-026-ibc-recovery-mechanisms.md @@ -0,0 +1,65 @@ +# ADR 026: IBC Client Recovery Mechanisms + +## Changelog + +- 2020/06/23: Initial version + +## Status + +*Proposed* + +## Context + +### Summary + +At launch, IBC will be a novel protocol, without an experienced user-base. At the protocol layer, it is not possible to distinguish between client expiry or misbehaviour due to genuine faults (Byzantine behavior) and client expiry or misbehaviour due to user mistakes (failing to update a client, or accidentally double-signing). In the base IBC protocol and ICS 20 fungible token transfer implementation, if a client can no longer be updated, funds in that channel will be permanently locked and can no longer be transferred. To the degree that it is safe to do so, it would be preferable to provide users with a recovery mechanism which can be utilised in these exceptional cases. + +### Exceptional cases + +The state of concern is where a client associated with connection(s) and channel(s) can no longer be updated. This can happen for several reasons: + +1. The chain which the client is following has halted and is no longer producing blocks/headers, so no updates can be made to the client +1. The chain which the client is following has continued to operate, but no relayer has submitted a new header within the unbonding period, and the client has expired + 1. This could be due to real misbehaviour (intentional Byzantine behaviour) or merely a mistake by validators, but the client cannot distinguish these two cases +1. The chain which the client is following has experienced a misbehaviour event, and the client has been frozen & thus can no longer be updated + +### Security model + +Two-thirds of the validator set (the quorum for governance, module participation) can already sign arbitrary data, so allowing governance to manually force-update a client with a new header after a delay period does not substantially alter the security model. + +## Decision + +We elect not to deal with chains which have actually halted, which is necessarily Byzantine behaviour and in which case token recovery is not likely possible anyways (in-flight packets cannot be timed-out, but the relative impact of that is minor). + +1. Require Tendermint light clients (ICS 07) to be created with the following additional flags + 1. `allow_governance_override_after_expiry` (boolean) +1. Require Tendermint light clients (ICS 07) to expose the following additional internal query functions + 1. `Expired() boolean`, which returns whether or not the client has passed the unbonding period since the last update +1. Require Tendermint light clients (ICS 07) & solo machine clients (ICS 06) to be created with the following additional flags + 1. `allow_governance_override_after_misbehaviour` (boolean) +1. Add a new governance proposal type, `ClientUpdateProposal`, in the `x/ibc` module + 1. Extend the base `Proposal` with a client identifier (`string`) and a header (`bytes`, encoded in a client-type-specific format) + 1. If this governance proposal passes, the client is updated with the provided header, if and only if: + 1. `allow_governance_override_after_expiry` is true and the client has expired (`Expired()` returns true) + 1. `allow_governance_override_after_misbehaviour` is true and the client has been frozen (`Frozen()` returns true) + +## Consequences + +### Positive + +- Establishes a mechanism for client recovery in the case of expiry +- Establishes a mechanism for client recovery in the case of misbehaviour +- Clients can elect to disallow this recovery mechanism if they do not wish to allow for it + +### Negative + +- Additional complexity in client creation which must be understood by the user +- Governance participants must pick a new header, which is a bit different from their usual tasks + +### Neutral + +No neutral consequences. + +## References + +- [Prior discussion](https://github.com/cosmos/ics/issues/421) From 9c2282dc99b73c3e34558c8cacd2fc0fd285caca Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Tue, 23 Jun 2020 14:42:58 +0200 Subject: [PATCH 2/6] Move file, add to README.md --- docs/architecture/README.md | 1 + ...y-mechanisms.md => adr-026-ibc-client-recovery-mechanisms.md} | 0 2 files changed, 1 insertion(+) rename docs/architecture/{adr-026-ibc-recovery-mechanisms.md => adr-026-ibc-client-recovery-mechanisms.md} (100%) diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 3889315c39bf..9a7fb183e482 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -50,3 +50,4 @@ Please add a entry below in your Pull Request for an ADR. - [ADR 022: Custom baseapp panic handling](./adr-022-custom-panic-handling.md) - [ADR 023: Protocol Buffer Naming and Versioning](./adr-023-protobuf-naming.md) - [ADR 024: Coin Metadata](./adr-024-coin-metadata.md) +- [ADR 026: IBC Client Recovery Mechanisms](./adr-026-ibc-client-recovery-mechanisms.md) diff --git a/docs/architecture/adr-026-ibc-recovery-mechanisms.md b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md similarity index 100% rename from docs/architecture/adr-026-ibc-recovery-mechanisms.md rename to docs/architecture/adr-026-ibc-client-recovery-mechanisms.md From 8d6eab6d02aacf660fb37385b9aa36f6b4f02f94 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Tue, 23 Jun 2020 14:43:43 +0200 Subject: [PATCH 3/6] Fix indent --- .../adr-026-ibc-client-recovery-mechanisms.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md index 3b9b023f31fd..369f087227d9 100644 --- a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md +++ b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md @@ -20,7 +20,7 @@ The state of concern is where a client associated with connection(s) and channel 1. The chain which the client is following has halted and is no longer producing blocks/headers, so no updates can be made to the client 1. The chain which the client is following has continued to operate, but no relayer has submitted a new header within the unbonding period, and the client has expired - 1. This could be due to real misbehaviour (intentional Byzantine behaviour) or merely a mistake by validators, but the client cannot distinguish these two cases + 1. This could be due to real misbehaviour (intentional Byzantine behaviour) or merely a mistake by validators, but the client cannot distinguish these two cases 1. The chain which the client is following has experienced a misbehaviour event, and the client has been frozen & thus can no longer be updated ### Security model @@ -32,16 +32,16 @@ Two-thirds of the validator set (the quorum for governance, module participation We elect not to deal with chains which have actually halted, which is necessarily Byzantine behaviour and in which case token recovery is not likely possible anyways (in-flight packets cannot be timed-out, but the relative impact of that is minor). 1. Require Tendermint light clients (ICS 07) to be created with the following additional flags - 1. `allow_governance_override_after_expiry` (boolean) + 1. `allow_governance_override_after_expiry` (boolean) 1. Require Tendermint light clients (ICS 07) to expose the following additional internal query functions - 1. `Expired() boolean`, which returns whether or not the client has passed the unbonding period since the last update + 1. `Expired() boolean`, which returns whether or not the client has passed the unbonding period since the last update 1. Require Tendermint light clients (ICS 07) & solo machine clients (ICS 06) to be created with the following additional flags - 1. `allow_governance_override_after_misbehaviour` (boolean) + 1. `allow_governance_override_after_misbehaviour` (boolean) 1. Add a new governance proposal type, `ClientUpdateProposal`, in the `x/ibc` module - 1. Extend the base `Proposal` with a client identifier (`string`) and a header (`bytes`, encoded in a client-type-specific format) - 1. If this governance proposal passes, the client is updated with the provided header, if and only if: - 1. `allow_governance_override_after_expiry` is true and the client has expired (`Expired()` returns true) - 1. `allow_governance_override_after_misbehaviour` is true and the client has been frozen (`Frozen()` returns true) + 1. Extend the base `Proposal` with a client identifier (`string`) and a header (`bytes`, encoded in a client-type-specific format) + 1. If this governance proposal passes, the client is updated with the provided header, if and only if: + 1. `allow_governance_override_after_expiry` is true and the client has expired (`Expired()` returns true) + 1. `allow_governance_override_after_misbehaviour` is true and the client has been frozen (`Frozen()` returns true) ## Consequences From 9686184c032a19d0561077adba404efdb2c5693a Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 25 Jun 2020 15:38:33 +0200 Subject: [PATCH 4/6] Clarify a few points --- docs/architecture/adr-026-ibc-client-recovery-mechanisms.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md index 369f087227d9..ce398ad766f1 100644 --- a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md +++ b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md @@ -32,11 +32,11 @@ Two-thirds of the validator set (the quorum for governance, module participation We elect not to deal with chains which have actually halted, which is necessarily Byzantine behaviour and in which case token recovery is not likely possible anyways (in-flight packets cannot be timed-out, but the relative impact of that is minor). 1. Require Tendermint light clients (ICS 07) to be created with the following additional flags - 1. `allow_governance_override_after_expiry` (boolean) + 1. `allow_governance_override_after_expiry` (boolean, default false) 1. Require Tendermint light clients (ICS 07) to expose the following additional internal query functions - 1. `Expired() boolean`, which returns whether or not the client has passed the unbonding period since the last update + 1. `Expired() boolean`, which returns whether or not the client has passed the trusting period since the last update (in which case no headers can be validated) 1. Require Tendermint light clients (ICS 07) & solo machine clients (ICS 06) to be created with the following additional flags - 1. `allow_governance_override_after_misbehaviour` (boolean) + 1. `allow_governance_override_after_misbehaviour` (boolean, default false) 1. Add a new governance proposal type, `ClientUpdateProposal`, in the `x/ibc` module 1. Extend the base `Proposal` with a client identifier (`string`) and a header (`bytes`, encoded in a client-type-specific format) 1. If this governance proposal passes, the client is updated with the provided header, if and only if: From 297c48cf4f0e998d7cb90da66af024477b2afd9b Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 25 Jun 2020 15:40:58 +0200 Subject: [PATCH 5/6] Notes on Unfreeze(), header validity --- docs/architecture/adr-026-ibc-client-recovery-mechanisms.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md index ce398ad766f1..144381813092 100644 --- a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md +++ b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md @@ -35,6 +35,8 @@ We elect not to deal with chains which have actually halted, which is necessaril 1. `allow_governance_override_after_expiry` (boolean, default false) 1. Require Tendermint light clients (ICS 07) to expose the following additional internal query functions 1. `Expired() boolean`, which returns whether or not the client has passed the trusting period since the last update (in which case no headers can be validated) +1. Require Tendermint light clients (ICS 07) to expose the following additional state mutation functions + 1. `Unfreeze()`, which unfreezes a light client after misbehaviour and clears any frozen height previously set 1. Require Tendermint light clients (ICS 07) & solo machine clients (ICS 06) to be created with the following additional flags 1. `allow_governance_override_after_misbehaviour` (boolean, default false) 1. Add a new governance proposal type, `ClientUpdateProposal`, in the `x/ibc` module @@ -42,6 +44,9 @@ We elect not to deal with chains which have actually halted, which is necessaril 1. If this governance proposal passes, the client is updated with the provided header, if and only if: 1. `allow_governance_override_after_expiry` is true and the client has expired (`Expired()` returns true) 1. `allow_governance_override_after_misbehaviour` is true and the client has been frozen (`Frozen()` returns true) + 1. In this case, additionally, the client is unfrozen by calling `Unfreeze()` + +Note additionally that the header submitted by governance must be new enough that it will be possible to update the light client after the new header is inserted into the client state (which will only happen after the governance proposal has passed). ## Consequences From e767deed453eb148bd36c770e4edc7aa7246c025 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 6 Aug 2020 16:57:37 +0200 Subject: [PATCH 6/6] Reference upgrade discussions --- docs/architecture/adr-026-ibc-client-recovery-mechanisms.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md index 144381813092..d3ffd34d85eb 100644 --- a/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md +++ b/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md @@ -3,6 +3,7 @@ ## Changelog - 2020/06/23: Initial version +- 2020/08/06: Revisions per review & to reference epoch ## Status @@ -48,6 +49,8 @@ We elect not to deal with chains which have actually halted, which is necessaril Note additionally that the header submitted by governance must be new enough that it will be possible to update the light client after the new header is inserted into the client state (which will only happen after the governance proposal has passed). +This ADR does not address planned upgrades, which are handled separately as per the [specification](https://github.com/cosmos/ics/tree/master/spec/ics-007-tendermint-client#upgrades). + ## Consequences ### Positive @@ -68,3 +71,5 @@ No neutral consequences. ## References - [Prior discussion](https://github.com/cosmos/ics/issues/421) +- [Epoch number discussion](https://github.com/cosmos/ics/issues/439) +- [Upgrade plan discussion](https://github.com/cosmos/ics/issues/445)