diff --git a/docs/architecture/adr-040-storage-and-smt-state-commitments.md b/docs/architecture/adr-040-storage-and-smt-state-commitments.md index f60e3adcfff9..72d10bb7e8c9 100644 --- a/docs/architecture/adr-040-storage-and-smt-state-commitments.md +++ b/docs/architecture/adr-040-storage-and-smt-state-commitments.md @@ -19,12 +19,11 @@ Currently, Cosmos SDK uses IAVL for both state [commitments](https://cryptograph IAVL has effectively become an orphaned project within the Cosmos ecosystem and it's proven to be an inefficient state commitment data structure. In the current design, IAVL is used for both data storage and as a Merkle Tree for state commitments. IAVL is meant to be a standalone Merkelized key/value database, however it's using a KV DB engine to store all tree nodes. So, each node is stored in a separate record in the KV DB. This causes many inefficiencies and problems: -* Each object query requires a tree traversal from the root. Subsequent queries for the same object are cached on the Cosmos SDK level. -* Each edge traversal requires a DB query. -* Creating snapshots is [expensive](https://github.com/cosmos/cosmos-sdk/issues/7215#issuecomment-684804950). It takes about 30 seconds to export less than 100 MB of state (as of March 2020). -* Updates in IAVL may trigger tree reorganization and possible O(log(n)) hashes re-computation, which can become a CPU bottleneck. -* The node structure is pretty expensive - it contains a standard tree node elements (key, value, left and right element) and additional metadata such as height, version (which is not required by the Cosmos SDK). The entire node is hashed, and that hash is used as the key in the underlying database, [ref](https://github.com/cosmos/iavl/blob/master/docs/node/node.md -). +- Each object query requires a tree traversal from the root. Subsequent queries for the same object are cached on the Cosmos SDK level. +- Each edge traversal requires a DB query. +- Creating snapshots is [expensive](https://github.com/cosmos/cosmos-sdk/issues/7215#issuecomment-684804950). It takes about 30 seconds to export less than 100 MB of state (as of March 2020). +- Updates in IAVL may trigger tree reorganization and possible O(log(n)) hashes re-computation, which can become a CPU bottleneck. +- The node structure is pretty expensive - it contains a standard tree node elements (key, value, left and right element) and additional metadata such as height, version (which is not required by the Cosmos SDK). The entire node is hashed, and that hash is used as the key in the underlying database, [ref](https://github.com/cosmos/iavl/blob/master/docs/node/node.md). Moreover, the IAVL project lacks support and a maintainer and we already see better and well-established alternatives. Instead of optimizing the IAVL, we are looking into other solutions for both storage and state commitments. @@ -54,18 +53,18 @@ We propose to use a KV database for both `SS` and `SC`. The store interface will State Storage requirements: -* range queries -* quick (key, value) access -* creating a snapshot -* historical versioning -* pruning (garbage collection) +- range queries +- quick (key, value) access +- creating a snapshot +- historical versioning +- pruning (garbage collection) State Commitment requirements: -* fast updates -* tree path should be short -* query historical commitment proofs using ICS-23 standard -* pruning (garbage collection) +- fast updates +- tree path should be short +- query historical commitment proofs using ICS-23 standard +- pruning (garbage collection) ### SMT for State Commitment @@ -73,11 +72,11 @@ A Sparse Merkle tree is based on the idea of a complete Merkle tree of an intrac The full specification can be found at [Celestia](https://github.com/celestiaorg/celestia-specs/blob/ec98170398dfc6394423ee79b00b71038879e211/src/specs/data_structures.md#sparse-merkle-tree). In summary: -* The SMT consists of a binary Merkle tree, constructed in the same fashion as described in [Certificate Transparency (RFC-6962)](https://tools.ietf.org/html/rfc6962), but using as the hashing function SHA-2-256 as defined in [FIPS 180-4](https://doi.org/10.6028/NIST.FIPS.180-4). -* Leaves and internal nodes are hashed differently: the one-byte `0x00` is prepended for leaf nodes while `0x01` is prepended for internal nodes. -* Default values are given to leaf nodes with empty leaves. -* While the above rule is sufficient to pre-compute the values of intermediate nodes that are roots of empty subtrees, a further simplification is to extend this default value to all nodes that are roots of empty subtrees. The 32-byte zero is used as the default value. This rule takes precedence over the above one. -* An internal node that is the root of a subtree that contains exactly one non-empty leaf is replaced by that leaf's leaf node. +- The SMT consists of a binary Merkle tree, constructed in the same fashion as described in [Certificate Transparency (RFC-6962)](https://tools.ietf.org/html/rfc6962), but using as the hashing function SHA-2-256 as defined in [FIPS 180-4](https://doi.org/10.6028/NIST.FIPS.180-4). +- Leaves and internal nodes are hashed differently: the one-byte `0x00` is prepended for leaf nodes while `0x01` is prepended for internal nodes. +- Empty leafs have value set to empty bytes (`[]byte{}`). +- While the above rule is sufficient to pre-compute the values of intermediate nodes that are roots of empty subtrees, a further simplification is to pre-define a value of empty sub-trees without creating that subtrees. Empty subtree of height n is compressed to a node with [value](https://github.com/celestiaorg/smt/blob/6e634fe4424005b4bce55a56a7a559412b4152b3/treehasher.go#L18) of k zero bytes (`[]byte{0, 0, ..., 0}`), where k is a byte length of the hash function range. For sha256, `k = 256 / 8 = 32`. +- An internal node that is the root of a subtree that contains exactly one non-empty leaf is replaced by that leaf's leaf node. ### Snapshots for storage sync and state versioning @@ -88,7 +87,7 @@ Some DB engines support snapshotting. Hence, we propose to reuse that functional One of the Stargate core features is a _snapshot sync_ delivered in the `/snapshot` package. It provides a way to trustlessly sync a blockchain without repeating all transactions from the genesis. This feature is implemented in Cosmos SDK and requires storage support. Currently IAVL is the only supported backend. It works by streaming to a client a snapshot of a `SS` at a certain version together with a header chain. -A new database snapshot will be created in every `EndBlocker` and identified by a block height. The `root` store keeps track of the available snapshots to offer `SS` at a certain version. The `root` store implements the `RootStore` interface described below. In essence, `RootStore` encapsulates a `Committer` interface. `Committer` has a `Commit`, `SetPruning`, `GetPruning` functions which will be used for creating and removing snapshots. The `rootStore.Commit` function creates a new snapshot and increments the version on each call, and checks if it needs to remove old versions. We will need to update the SMT interface to implement the `Committer` interface. +A new database snapshot will be created in every `EndBlocker` and identified by a block height. The `root` store keeps track of the available snapshots to offer `SS` at a certain version. The `root` store implements the `MultiStore` interface described below. In essence, `MultiStore` extends the `Committer` interface. `Committer` has `Commit`, `SetPruning`, and `GetPruning` functions which will be used for creating and removing snapshots. The `rootStore.Commit` function creates a new snapshot and increments the version on each call, and checks if it needs to remove old versions. We will need to update the SMT interface to implement the `Committer` interface. NOTE: `Commit` must be called exactly once per block. Otherwise we risk going out of sync for the version number and block height. NOTE: For the Cosmos SDK storage, we may consider splitting that interface into `Committer` and `PruningCommitter` - only the multiroot should implement `PruningCommitter` (cache and prefix store don't need pruning). @@ -100,7 +99,7 @@ To manage the active snapshots we will either use a DB _max number of snapshots_ #### Accessing old state versions -One of the functional requirements is to access old state. This is done through `abci.RequestQuery` structure. The version is specified by a block height (so we query for an object by a key `K` at block height `H`). The number of old versions supported for `abci.RequestQuery` is configurable. Accessing an old state is done by using available snapshots. +One of the functional requirements is to access old state. This is done through an `abci.RequestQuery` structure. The version is specified by a block height (so we query for an object by a key `K` at block height `H`). The number of old versions supported for `abci.RequestQuery` is configurable. Accessing an old state is done by using available snapshots. `abci.RequestQuery` doesn't need old state of `SC` unless the `prove=true` parameter is set. The SMT merkle proof must be included in the `abci.ResponseQuery` only if both `SC` and `SS` have a snapshot for requested version. Moreover, Cosmos SDK could provide a way to directly access a historical state. However, a state machine shouldn't do that - since the number of snapshots is configurable, it would lead to nondeterministic execution. @@ -113,48 +112,100 @@ For any object stored in State Store (SS), we have corresponding object in `SC`. ### Rollbacks -We need to be able to process transactions and roll-back state updates if a transaction fails. This can be done in the following way: during transaction processing, we keep all state change requests (writes) in a `CacheWrapper` abstraction (as it's done today). Once we finish the block processing, in the `Endblocker`, we commit a root store - at that time, all changes are written to the SMT and to the `SS` and a snapshot is created. +We need to be able to process transactions and roll-back state updates if a transaction fails. This can be done in the following way: during transaction processing, we keep all state change requests (writes) in a `CacheWrapper` abstraction (as it's done today). Once we finish the block processing, in the `Endblocker`, we commit a root store - at that time, all changes are written to the SMT and to the `SS` and a snapshot is created. ### Committing to an object without saving it We identified use-cases, where modules will need to save an object commitment without storing an object itself. Sometimes clients are receiving complex objects, and they have no way to prove a correctness of that object without knowing the storage layout. For those use cases it would be easier to commit to the object without storing it directly. -### Refactor MultiStore +### Direct SS and SC access -The Stargate `/store` implementation (store/v1) adds an additional layer in the SDK store construction - the `MultiStore` structure. The multistore exists to support the modularity of the Cosmos SDK - each module is using its own instance of IAVL, but in the current implementation, all instances share the same database. The latter indicates, however, that the implementation doesn't provide true modularity. Instead it causes problems related to race condition and atomic DB commits (see: [\#6370](https://github.com/cosmos/cosmos-sdk/issues/6370) and [discussion](https://github.com/cosmos/cosmos-sdk/discussions/8297#discussioncomment-757043)). +Modules should be able to commit a value fully managed by the module itself. For example, a module can manage its own special database and commit its state by setting a value only to `SC`. +Similarly, a module can save a value without committing it - this is useful for ORM module or secondary indexes (eg x/staking `UnbondingDelegationKey` and `UnbondingDelegationByValIndexKey`). -We propose to reduce the multistore concept from the SDK, and to use a single instance of `SC` and `SS` in a `RootStore` object. To avoid confusion, we should rename the `MultiStore` interface to `RootStore`. The `RootStore` will have the following interface; the methods for configuring tracing and listeners are omitted for brevity. +We consider 2 options for accessing `SS` and `SC` while working on this ADR. + +#### Option 1: StoreAccess interface + +Currently, a module can access a store only through `sdk.Context`. We add the following methods to the `sdk.Context`: + +``` +type StoreAccess inteface { + KVStore(key []byte) KVStore // the existing method in sdk.Context, reads and writes both to SS & SC stores in a combined namespace. + SCStore(key []byte) KVStore // reads and writes only to the SC in reserved SC namespace + SSStore(key []byte) KVStore // reads and writes only to the SS in reserved SS namespace +} +``` + +`KVStore`, `SCStore` an `SSStore` will operate in a distinct namespace and will be registered as a separate store in the MultiStore object. So, the records in each store will not collide (Eg: `KVStore(a).Set(k, v)` won't collide with `SSStore(a).Set(k, v)`). + +`KVStore(key)` will provide access to the combined `SS` and `SC` store: + +- `Get` will return `SS` value +- `Has` will return true if value key is present in `SS` +- `Set` will store a value in both `SS` and `SC`. It panics when key or value are nil +- `Delete` will delete key both from `SS` and `SC`. It panics when key is nil +- `Iterator` will iterate over `SS` +- `ReverseIterator` will iterate over `SS` + +and will be implemented on a cache level with the following helper structure: + +```go +type CombinedKVStore { + ss KVStore + sc KVStore +} +``` + +`SCStore()` and `SSStore()` returns a KVStore with access and operations only for `SC` and `SS` respectively. Moreover, they will use a unique namespace to avoid conflicts with `KVStore`. Naive implementation could cause race conditions (when someone writes to the combined `KVStore` and later writes to `SSStore` in the same transaction). + +The Cache store must be aware if writes happen to a combined `KVStore` or `SCStore` only. +The proposed solution is to return different cache instances for each method of `StoreAccess` interface. More specifically, when starting a transaction, we will create create 3 cache instances (for CombinedKVStore, SS and SC). + +#### Option 2: SCStoreKey and SSStoreKey types + +Cosmos SDK manages prefix keys for various storage types for modules. Module requests a store key for specified store type (permanent, transient, memory). We can extend that mechanism and introduce two new sore key types: `SCStoreKey` and `SSStoreKey`. +If an app module will need to write some data only to `SC` store and some other data to a general store, then it will request both `SCStoreKey` and `KVStoreKey` during module initialization. Stores are accessed using the existing mechanism: `kvstore := Context.KVStore(storeKey)`. +Store manager will assign different prefix keys for stores with different store key, so data stored and accessed with each key will be in a different namespace (opposite to the option 1 approach). Specifically, if `(key, valule)` is written using `Context.KVStore(moduleStoreKey)` it won't be available when querying using `Context.KVStore(moduleSCStoreKey)`. + +### MultiStore Refactor + +The Stargate `/store` implementation (store/v1) has an additional layer in the SDK store construction - the `MultiStore` structure. The multistore exists to support the modularity of the Cosmos SDK - each module is using its own instance of IAVL with independent commit phase. It causes problems related to race condition and atomic DB commits (see: [\#6370](https://github.com/cosmos/cosmos-sdk/issues/6370) and [discussion](https://github.com/cosmos/cosmos-sdk/discussions/8297#discussioncomment-757043)). + +We propose to simplify the multistore concept in the Cosmos SDK: ++ As in store/v1, MultiStore allows to mount substores. Each substore has exactly one instance of `SS` and `SC`. This provides expected modularity for modules. ++ Multistore maintains a mapping (we call it _scheme_) between substore key (usually a module store key) and a compressed key prefix - see _Optimization: compress module key prefixes_ section below. ++ Multistore is responsible for creating and maintaining the substores. User should not be able to create and mount substore by his own. All stores managed by Multistore use the same underlying database preserving atomic operations. + +The following interfaces are proposed; the methods for configuring tracing and listeners are omitted for brevity. ```go // Used where read-only access to versions is needed. -type BasicRootStore interface { - Store +type BasicMultiStore interface { + // returns a substore GetKVStore(StoreKey) KVStore - CacheRootStore() CacheRootStore } // Used as the main app state, replacing CommitMultiStore. -type CommitRootStore interface { - BasicRootStore +type CommitMultiStore interface { + BasicMultiStore Committer Snapshotter - GetVersion(uint64) (BasicRootStore, error) + GetVersion(uint64) (BasicMultiStore, error) + CacheMultiStore() CacheMultiStore SetInitialVersion(uint64) error - - ... // Trace and Listen methods } // Replaces CacheMultiStore for branched state. -type CacheRootStore interface { - BasicRootStore +type CacheMultiStore interface { + BasicMultiStore Write() - - ... // Trace and Listen methods + CacheMultiStore() CacheMultiStore } // Example of constructor parameters for the concrete type. -type RootStoreConfig struct { +type MultiStoreConfig struct { Upgrades *StoreUpgrades InitialVersion uint64 @@ -162,39 +213,34 @@ type RootStoreConfig struct { } ``` - - - -In contrast to `MultiStore`, `RootStore` doesn't allow to dynamically mount sub-stores or provide an arbitrary backing DB for individual sub-stores. - -NOTE: modules will be able to use a special commitment and their own DBs. For example: a module which will use ZK proofs for state can store and commit this proof in the `RootStore` (usually as a single record) and manage the specialized store privately or using the `SC` low level interface. +NOTE: modules will be able to use a special commitment and their own DBs. For example: a module which will use ZK proofs for state can store and commit this proof in the `MultiStore` (usually as a single record) and manage the specialized store privately or using the `SC` low level interface. #### Compatibility support -To ease the transition to this new interface for users, we can create a shim which wraps a `CommitMultiStore` but provides a `CommitRootStore` interface, and expose functions to safely create and access the underlying `CommitMultiStore`. +Cosmos SDK users should be only concerned about the module interface, which currently relies on the `KVStore`. We don't change this interface, so the proposed store/v2 is 100% compatible with existing modules. -The new `RootStore` and supporting types can be implemented in a `store/v2alpha1` package to avoid breaking existing code. +The new `RootStore` and supporting types can be implemented in a `store/v2alpha1` package to avoid breaking existing code and keep the support for IAVL stores using `store` (v1) package. #### Merkle Proofs and IBC -Currently, an IBC (v1.0) Merkle proof path consists of two elements (`["", ""]`), with each key corresponding to a separate proof. These are each verified according to individual [ICS-23 specs](https://github.com/cosmos/ibc-go/blob/f7051429e1cf833a6f65d51e6c3df1609290a549/modules/core/23-commitment/types/merkle.go#L17), and the result hash of each step is used as the committed value of the next step, until a root commitment hash is obtained. -The root hash of the proof for `""` is hashed with the `""` to validate against the App Hash. - -This is not compatible with the `RootStore`, which stores all records in a single Merkle tree structure, and won't produce separate proofs for the store- and record-key. Ideally, the store-key component of the proof could just be omitted, and updated to use a "no-op" spec, so only the record-key is used. However, because the IBC verification code hardcodes the `"ibc"` prefix and applies it to the SDK proof as a separate element of the proof path, this isn't possible without a breaking change. Breaking this behavior would severely impact the Cosmos ecosystem which already widely adopts the IBC module. Requesting an update of the IBC module across the chains is a time consuming effort and not easily feasible. - -As a workaround, the `RootStore` will have to use two separate SMTs (they could use the same underlying DB): one for IBC state and one for everything else. A simple Merkle map that reference these SMTs will act as a Merkle Tree to create a final App hash. The Merkle map is not stored in a DBs - it's constructed in the runtime. The IBC substore key must be `"ibc"`. +The IBC v1.0 Merkle proof is influenced by the MultiStore design: it is a path of two elements (`["", ""]`), with each key corresponding to a separate sub-proof. `` is a key in a substore identified by ``. The x/ibc module implementation requires that the `` is not empty and assumes that the proofs are broken down according to the [ICS-23 specs](https://github.com/cosmos/ibc-go/blob/f7051429e1cf833a6f65d51e6c3df1609290a549/modules/core/23-commitment/types/merkle.go#L17). +IBC verification has two steps: firstly we make a standard Merkle proof verification for the ``. In the second step, we hash the the `` with the root hash of the first step and validate it against the App Hash. -The workaround can still guarantee atomic syncs: the [proposed DB backends](#evaluated-kv-databases) support atomic transactions and efficient rollbacks, which will be used in the commit phase. +The IBC client is configured with a proof spec that defines how to hash individual elements on a proof path. +An IBC proof spec for the SMT is required to support the IBC client. -The presented workaround can be used until the IBC module is fully upgraded to supports single-element commitment proofs. +The x/ibc module client hardcodes `"ibc"` as the `` (IBC store-key component proof could be omitted if a "no-op" spec was defined in the x/ibc client). +Breaking this behavior would severely impact the Cosmos ecosystem which already widely adopts the IBC module. Requesting an update of the IBC module across the chains is a time consuming effort and not easily feasible. +We want to support ICS-23 for all modules. This means that all modules must use a separate SMT instance. +This functionality is preserved in the `MultiStore` implementation. ### Optimization: compress module key prefixes We consider a compression of prefix keys by creating a mapping from module key to an integer, and serializing the integer using varint coding. Varint coding assures that different values don't have common byte prefix. For Merkle Proofs we can't use prefix compression - so it should only apply for the `SS` keys. Moreover, the prefix compression should be only applied for the module namespace. More precisely: -* each module has it's own namespace; -* when accessing a module namespace we create a KVStore with embedded prefix; -* that prefix will be compressed only when accessing and managing `SS`. +- each module has its own namespace; +- when accessing a module namespace we create a KVStore with embedded prefix; +- that prefix will be compressed only when accessing and managing `SS`. We need to assure that the codes won't change. We can fix the mapping in a static variable (provided by an app) or SS state under a special key. @@ -240,21 +286,21 @@ We change the storage layout of the state machine, a storage hard fork and netwo ### Positive -* Decoupling state from state commitment introduce better engineering opportunities for further optimizations and better storage patterns. -* Performance improvements. -* Joining SMT based camp which has wider and proven adoption than IAVL. Example projects which decided on SMT: Ethereum2, Diem (Libra), Trillan, Tezos, Celestia. -* Multistore removal fixes a longstanding issue with the current MultiStore design. -* Simplifies merkle proofs - all modules, except IBC, have only one pass for merkle proof. +- Decoupling state from state commitment introduces better engineering opportunities for further optimizations and better storage patterns. +- Performance improvements. +- Joining SMT based camp which has wider and more proven adoption than IAVL. Example projects which decided on SMT: Ethereum2, Diem (Libra), Trillan, Tezos, Celestia. +- Multistore removal fixes a longstanding issue with the current MultiStore design. +- Simplifies merkle proofs - all modules, except IBC, have only one pass for merkle proof. ### Negative -* Storage migration -* LL SMT doesn't support pruning - we will need to add and test that functionality. -* `SS` keys will have an overhead of a key prefix. This doesn't impact `SC` because all keys in `SC` have same size (they are hashed). +- Storage migration +- LL SMT doesn't support pruning - we will need to add and test that functionality. +- `SS` keys will have an overhead of a key prefix. This doesn't impact `SC` because all keys in `SC` have same size (they are hashed). ### Neutral -* Deprecating IAVL, which is one of the core proposals of Cosmos Whitepaper. +- Deprecating IAVL, which is one of the core proposals of Cosmos Whitepaper. ## Alternative designs @@ -274,16 +320,16 @@ Use of RDBMS instead of simple KV store for state. Use of RDBMS will require a C ### Off Chain Store -We were discussing use case where modules can use a support database, which is not automatically committed. Module will responsible for having a sound storage model and can optionally use the feature discussed in __Committing to an object without saving it_ section. +We were discussing use case where modules can use a support database, which is not automatically committed. Module will be responsible for having a sound storage model and can optionally use the feature discussed in \__Committing to an object without saving it_ section. ## References -* [IAVL What's Next?](https://github.com/cosmos/cosmos-sdk/issues/7100) -* [IAVL overview](https://docs.google.com/document/d/16Z_hW2rSAmoyMENO-RlAhQjAG3mSNKsQueMnKpmcBv0/edit#heading=h.yd2th7x3o1iv) of it's state v0.15 -* [State commitments and storage report](https://paper.dropbox.com/published/State-commitments-and-storage-review--BDvA1MLwRtOx55KRihJ5xxLbBw-KeEB7eOd11pNrZvVtqUgL3h) -* [Celestia (LazyLedger) SMT](https://github.com/lazyledger/smt) -* Facebook Diem (Libra) SMT [design](https://developers.diem.com/papers/jellyfish-merkle-tree/2021-01-14.pdf) -* [Trillian Revocation Transparency](https://github.com/google/trillian/blob/master/docs/papers/RevocationTransparency.pdf), [Trillian Verifiable Data Structures](https://github.com/google/trillian/blob/master/docs/papers/VerifiableDataStructures.pdf). -* Design and implementation [discussion](https://github.com/cosmos/cosmos-sdk/discussions/8297). -* [How to Upgrade IBC Chains and their Clients](https://github.com/cosmos/ibc-go/blob/main/docs/ibc/upgrades/quick-guide.md) -* [ADR-40 Effect on IBC](https://github.com/cosmos/ibc-go/discussions/256) +- [IAVL What's Next?](https://github.com/cosmos/cosmos-sdk/issues/7100) +- [IAVL overview](https://docs.google.com/document/d/16Z_hW2rSAmoyMENO-RlAhQjAG3mSNKsQueMnKpmcBv0/edit#heading=h.yd2th7x3o1iv) of it's state v0.15 +- [State commitments and storage report](https://paper.dropbox.com/published/State-commitments-and-storage-review--BDvA1MLwRtOx55KRihJ5xxLbBw-KeEB7eOd11pNrZvVtqUgL3h) +- [Celestia (LazyLedger) SMT](https://github.com/lazyledger/smt) +- Facebook Diem (Libra) SMT [design](https://developers.diem.com/papers/jellyfish-merkle-tree/2021-01-14.pdf) +- [Trillian Revocation Transparency](https://github.com/google/trillian/blob/master/docs/papers/RevocationTransparency.pdf), [Trillian Verifiable Data Structures](https://github.com/google/trillian/blob/master/docs/papers/VerifiableDataStructures.pdf). +- Design and implementation [discussion](https://github.com/cosmos/cosmos-sdk/discussions/8297). +- [How to Upgrade IBC Chains and their Clients](https://github.com/cosmos/ibc-go/blob/main/docs/ibc/upgrades/quick-guide.md) +- [ADR-40 Effect on IBC](https://github.com/cosmos/ibc-go/discussions/256)