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.

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

Conditional clients #5112

Closed
3 tasks
colin-axner opened this issue Nov 15, 2023 · 5 comments
Closed
3 tasks

Conditional clients #5112

colin-axner opened this issue Nov 15, 2023 · 5 comments
Labels
02-client 08-wasm type: feature New features, sub-features or integrations
Milestone

Comments

@colin-axner
Copy link
Contributor

Credit to @notbdu for introducing Conditional Clients and @AdityaSripal for providing feedback on the idea

Summary

Conditional clients are light clients which are dependent on another client in order to verify or update state. Conditional clients are essential for integration with modular blockchains which break up consensus and state management, such as roll ups.

The initial idea for Conditional clients was posted on the ibc spec repo. In this proposal, a light client may optionally specify a list of dependencies and the 02-client module would lookup read-only provable stores for each dependency and provide this to the conditional client to perform verification.

As the initial usage of this feature is for integration with roll ups, I will look at how this will be implemented between 08-wasm and native go clients, as 08-wasm will be the primary client used as a conditional client.

Problem

Currently, light clients receive a single provable store they maintain. There's a unidirectional communication channel with 02-client. The 02-client module will call into the light client, without allowing for the light client to call into the 02-client module.

As stated by @notbdu:

Modular blockchains break up a logical blockchain into many constituent parts. To accurately represent these chains and also to account for various types of shared security primitives that are coming up, we need to introduce dependencies between clients.

In the case of optimistic rollups, as outlined by this comment. In order to prove execution (allowing for fraud proofs), you must prove data availability and sequencing.

Using opimistic rollups as an example with Celestia providing data availability and sequencing. A tendermint light client will be used to prove inclusion of state headers submitted to Celestia by the sequencer. In order to protect against a sequencer submitting an invalid header (a header which based on a set of txs which includes an invalid state transition), settlement is required on the sender side (protecting native assets), while misbehaviour/delay periods is required on the receving side (for IBC).

On the receiving side, to proof receipt. An optimistic light client, will need to prove data availiablity and sequencing which will intiate a fraud proof window. It must be capable of proving fraud proofs for invalid state transitions within the fraud proof window. Proving data availability and sequencing are preconditions for starting a fraud proof window as we cannot otherwise guarentee an honest actor will be capable of creating a fraud proof if the data is not available, allowing invalid actions after the fraud proof window completes.

My understanding is that the work flow might look something like this:

  • packet is sent on an optimsitc roll up
  • state header for this executin is submitted to celestia
  • relayer updates celestia client (client type is tendermint) on counterparty chain (say Osmosis)
  • relayer submits 2 proofs to Osmosis, 1 proof for inclusion of the state header on celestia, another proof for the inclusion of the packet commitment in the state header of the optimistic light client (lets call it a conditional client)
  • the conditional client only will fail proof of inclusion if the celestia client fails proof of inclusion
  • proofs succeed, the fraud proof window begins
  • (misbehaviour situation) a fraud proof is submitted to the conditional client
  • (misbehaviour situation) the conditional client verifies the invalid state transition and returns success if the celestia client also verifies the state header in question
  • (misbehaviour situation) the conditional client can be updated as necessary (maybe marking all heights after the proved misbehaviour as invalid)
  • tokens are received in successful case

As we see, to avoid unnecessary engineering complexity, it makes sense to request proof verification from the existing clients, rather than embedding dependency client verification logic into conditional clients. It is also important to note, the conditional client must have access to the value being proved, in this case the state header.

One note, being discussed on this issue are the current limitations of the 02-client design. Specifically, the 02-client module intermixes routing and provable store management, making it very difficult to create a separation of concerns. Future modifications to 02-client will separate these two concerns which will come with numerous benefits to light client development. I will take this future direction into consideration for long term solutions, but for short term solutions I will assume the structure of the 02-client module as it is today.

02-client modifications

The solution proposed in the ibc spec repo is to use the 02-client module to provide read-only access to client stores specified in a dependency list.

Based on the spec changes:

    // get client dependencies (conditional client) 
    dependencies = []
    for (const clientID of clientState.clientDependencies) {
        dependencies.push(provableStore.get(clientStatePath(clientID)))
    }

    clientState.verifyClientMessage(clientMessage, dependencies)

This requires introducing another interface function into the client state inteface. To reduce breakage, we can make it an optional interface:

type ConditionalClient interface {
    ClientDependecies() []string
}

With the following additions to the 02-client module:

    var dependencyStores []sdk.KVStore
    if conditionalClient, ok := clientState.(exported.ConditionalClient); ok {
        for _, dependency := range clientState.ClientDependencies() {
            dependencyStores = append(dependencyStores, k.ClientStore(ctx, dependency)) 
        }
    }

    if err := clientState.VerifyClientMessage(ctx, k.cdc, clientStore, clientMsg, dependencyStores); err != nil {
		return err
    }

I believe this needs to be done for membership and non-membership proofs as well? (please correct me if I am mistaken)

func (k Keeper) VerifyPacketCommitment(  
    ...

    var dependencyStores []sdk.KVStore
    if conditionalClient, ok := clientState.(exported.ConditionalClient); ok {
        for _, dependency := range clientState.ClientDependencies() {
            dependencyStores = append(dependencyStores, k.ClientStore(ctx, dependency)) 
        }
    }
    
    if err := clientState.VerifyMembership(
	ctx, clientStore, k.cdc, height,
	timeDelay, blockDelay,
	proof, merklePath, commitmentBytes,
    ); err != nil {
	return errorsmod.Wrapf(err, "failed packet commitment verification for client (%s)", clientID)
    }

Native go clients

Adoption of this feature in native go clients (if you are writing a conditional client in a go module that connects to ibc-go) is quite straight forward.

Within the conditional client code:

    for i, dependencyStore := range dependencyStores {
        dependecy := getClientState(ctx, cdc, dependencyStore)
        if err := dependency.VerifyClientMessage(ctx, cdc, dependencyStore, clientMsg.DependencyMsgs[i], nil); err != nil {
            return err
        }
    }

    // use proved values for self verification

One note, I am not sure how nested dependencies would work in this situation (I don't feel it's necessary to worry about this at the moment without a specific use case in mind?)

08-wasm

Adoption of this feature in 08-wasm is a bit trickier. Trying to follow the approach of breaking interfaces to provide the conditional client with access to a read-only store is cumbersome. To do this, we would need to create a wrapped store for all contract api's which allows the contract to use a special key to indicate it wants to read from the dependency store.

Another approach, which I prefer at the moment which aslo falls in line with the Actor model specified by CosmWasm is to allow contracts to return back sub-msgs which indicate their dependency msgs.

Example:

    payload := SudoMsg{
	VerifyMembership: &VerifyMembershipMsg{
		Height:           proofHeight,
		DelayTimePeriod:  delayTimePeriod,
		DelayBlockPeriod: delayBlockPeriod,
		Proof:            proof,
		Path:             merklePath,
		Value:            value,
	},
    }
    resp, err := wasmSudo[VerifyMembershipResponse](ctx, cdc, payload, clientStore, &cs)
    if err != nil {
        return err
    }

    // sanity check relayer provided values match expected dependency verification by contract
    // also ensure messages returned are of correct msg type
    // this verification can optionally occur in validation of response in wasmSudo call 
    if err := validadateDependecies[VerifyMembershipResponse](resp.DependencyMsgs, dependencyStores); err != nil {
        return err
    {

    if len(resp.DependencyMsgs) > 0    {
        for i, dependencyStore := range dependencyStores {
            dependecy := getClientState(ctx, cdc, dependencyStore)
            if err := dependency.VerifyClientMessage(ctx, cdc, dependencyStore, resp.DependencyMsgs[i], nil); err != nil {
                return err
            }
        }
    } 

As a note, the general feature we are trying to achieve here is contract communication with other light client modules (native or wasm clients). This approach hasn't been validated by a PoC and still needs to be disuccsed with Confio, so it is subject to change if a nicer solution comes about during discussion.

Pros/Cons

In terms of code complexity, I think this solution is quite minimal. The primary downside I see is that it requires a major version release of ibc-go in order to break the client state interface. This would also require updates to all light client implementations, regardless of their usage of conditional clients. The requirement for interface breakage is not in the conditional client concept itself, but rather in working around the limitations outlined in #5084. Thus the changes made would likely be removed within due time.

Alternative approach

Here I will expand on the concept above by taking into account the future design proposed in #5084

Native go clients

For native go clients, my recommendation would be to wait until the feature has a user. It's my understanding the bulk of conditional clients will be 08-wasm, so I would recommend waiting until we need conditional client support for native go clients.

If you need native clients before #5084 is implemented, I would recommend go with the client state interface breakage approach outlined above.

Otherwise if #5084 is implemented before native client support is demanded, the solution would look like follows:

No modifications to 02-client needed.

Native light client module must embded 02-client router:

    // depependcies can be set/stored within the light client module
    for i, dependency := range dependencies {
        dependencyModule := module.clientRouter.Route[dependency]
        if err := dependencyModule.VerifyClientMessage(ctx, clientMsg.DependencyMsgs[i]); err != nil {
            return err
        }     
    }

    // use proved values for self verification

08-wasm

For the 08-wasm client, the same actor model approach will be required. I will outline a short and long term approach to obtaining the dependency client without modifications to the 02-client module or the ClientState interface.

Short-term solution

Add the 02-client keeper as a global to 08-wasm.

// 08-wasm/internal/ibcwasm/wasm.go
var (
        vm WasmEngine

	// state management
	Schema     collections.Schema
	CodeHashes collections.KeySet[[]byte]

	// CodeHashesKey is the key under which all code hashes are stored
	CodeHashesKey = collections.NewPrefix(0)

        // conditional client support
        clientKeeper ClientKeeper
)

// SetClientKeeper sets the client keeper used by the 08-wasm module.
func SetVM(wasmVM WasmEngine) {
	vm = wasmVM
}

// GetDependencyStore returns the dependency store required for conditional clients.
func GetDependencyStore(ctx sdk.Context, dependency string) sdk.KVStore {
    return clientKeeper.ClientStore(ctx, dependency)
}

Now the 08-wasm logic looks as follows:

    payload := SudoMsg{
	VerifyMembership: &VerifyMembershipMsg{
		Height:           proofHeight,
		DelayTimePeriod:  delayTimePeriod,
		DelayBlockPeriod: delayBlockPeriod,
		Proof:            proof,
		Path:             merklePath,
		Value:            value,
	},
    }
    resp, err := wasmSudo[VerifyMembershipResponse](ctx, cdc, payload, clientStore, &cs)
    if err != nil {
        return err
    }


    // must ensure dependency msgs are valid
    if len(resp.DependencyMsgs) > 0    {
        for _, msg := range resp.DependencyMsgs {
            dependecy := ibcwasm.GetDependencyStore(ctx, msg.Dependency)
            if err := dependency.VerifyClientMessage(ctx, cdc, dependencyStore, msg.ClientMessage, nil); err != nil {
                return err
            }
        }
    } 

Long-term solution

With the addition #5084, we can simply embed the 02-client router into the 08-wasm light client module:

    payload := SudoMsg{
	VerifyMembership: &VerifyMembershipMsg{
		Height:           proofHeight,
		DelayTimePeriod:  delayTimePeriod,
		DelayBlockPeriod: delayBlockPeriod,
		Proof:            proof,
		Path:             merklePath,
		Value:            value,
	},
    }
    resp, err := wasmSudo[VerifyMembershipResponse](ctx, cdc, payload, clientStore, &cs)
    if err != nil {
        return err
    }


    if len(resp.DependencyMsgs) > 0    {
        for _, msg := range resp.DependencyMsgs {
            dependencyModule := module.clientRouter.Route[msg.Dependency]
            if err := dependencyModule.VerifyClientMessage(ctx, cdc, msg.ClientMessage, nil); err != nil {
                return err
            }
        }
    } 

Pros/Cons

The benefit to this alternative approach is that no modifications to ibc-go are required. We can release a v0.2 of 08-wasm using the short term solution and eventually update to the long term solution (with no breaking changes) when it is available. Thus not tying conditional client implementation to a longer term solution, but making it compatible with it.

Conclusion

I think the least disruptive and most impactful solution is adding a global client keeper to 08-wasm, so it can access and dependencies for verification and allowing contracts to return back dependency messages. I would recommend implementing this as soon as possible.

If there becomes a strong requirement for native go client support for conditional clients, break the API and add dependency store management to 02-client.

Once #5084 is implemented, require light client modules to use the 02-client router to obtain access to dependencies.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added needs discussion Issues that need discussion before they can be worked on 08-wasm 02-client labels Nov 15, 2023
@AdityaSripal
Copy link
Member

AdityaSripal commented Nov 20, 2023

I'm in favor of the short term solution, thanks @colin-axner for the write-up 👍

I guess the reason you want to add the full clientKeeper in instead of a QueryRouter is to support solomachine clients as an underlying dependency. Though perhaps its also easier since you don't need to implement query routes. Is there a real use case for solomachines as an underlying client. Especially since this is a short term solution, seems unnecessary to support without a clear purpose for it and we can think about the more general case for the longer term case.

Asking because it came up in our private chat and again in Union wanting the ability to make queries in their clients

@colin-axner
Copy link
Contributor Author

The primary reason to use the client keeper rather than the grpc router is because we would need to add more grpc routes which would require a minor version of ibc-go. As you note, this is a short term solution, so it doesn't make much sense to me to extend the grpc routes without a clear vision of if it is necessary for the long term approach. The grpc querier is built for external users, so repurposing it for inter-chain communication would be a bit of a change

I agree, it doesn't make sense to make extra accommodation for the solo machine as it is unclear if it'll ever be used in such a configuration 👍

Regarding using the wasmvm.Querier. That is a viable route, which I believe will be investigated further on this issue. My only concern with regards to combining this Querier with the client state API is the inability to assert no state changes occur (ensuring it is a pure query action). Off the top of my head, I'm not sure how you could enforce a read-only without breaking the client state interface

@nashqueue
Copy link

nashqueue commented Dec 1, 2023

In favor of this proposal.

My understanding is that the work flow might look something like this:

The path you are describing is one possible path out of many. Remembering to have a good abstraction and not break other options would be a good idea. A good exercise could be to write down different sequence diagrams of rollup types to understand the general concept and how this translates to rollup bridging with IBC. I'm happy to help with that as well.

@crodriguezvega crodriguezvega added the type: feature New features, sub-features or integrations label Dec 4, 2023
@crodriguezvega crodriguezvega moved this to Todo in ibc-go Jan 10, 2024
@crodriguezvega crodriguezvega added this to the v9.0.0 milestone Jan 15, 2024
@crodriguezvega crodriguezvega moved this from Todo 🏃 to In progress 👷 in ibc-go Feb 6, 2024
@colin-axner
Copy link
Contributor Author

This issue will be closed by:

@crodriguezvega
Copy link
Contributor

Closed by #5821 and #5806.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
02-client 08-wasm type: feature New features, sub-features or integrations
Projects
Status: Done 🥳
Development

No branches or pull requests

4 participants