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

Querier Approach for Conditional Clients #5310

Closed
3 tasks
srdtrk opened this issue Dec 5, 2023 · 4 comments · Fixed by #5821
Closed
3 tasks

Querier Approach for Conditional Clients #5310

srdtrk opened this issue Dec 5, 2023 · 4 comments · Fixed by #5821
Assignees
Labels
02-client 08-wasm type: feature New features, sub-features or integrations

Comments

@srdtrk
Copy link
Member

srdtrk commented Dec 5, 2023

Credit to @colin-axner for writing #5112 and @webmaster128 for the helpful discussions on the idea

Summary

This proposal introduces a querier-based mechanism in ibc-go to enable conditional clients, based on the requirements and workflows outlined in #5112. It specifically enables light clients to query other light clients through gRPC, offering a non-intrusive solution. A key feature of this proposal is the introduction of a non-state-modifying proof verification query within the 02-client module, which streamlines the process of implementing conditional clients, particularly within the 08-wasm module. The document details necessary modifications to both the 02-client and 08-wasm modules and includes a proof of concept branch (outdated 08-wasm). Additionally, the proposal contrasts this approach with an alternative method based on a gRPC message server, to provide a comprehensive view of potential gRPC based solutions.

Problem Definition

Issue #5112 introduced the concept of conditional execution in light clients, dependent on the state of another light client. Traditional light client modules in our framework lack the capability to execute code conditionally based on the state of other modules. While we have extended the functionality in the 08-wasm module (see #5192) by allowing chains to provide a custom querier, a more comprehensive solution is needed to facilitate conditional execution between light clients.

At present, the interaction between light client modules and the core 02-client module is unidirectional: the 02-client can only send information to light clients without receiving any queries in return. This proposal aims to enable light clients to query other light clients (or even modules if enabled) via the GRPCQueryRouter in BaseApp.

The 02-client already includes a query gRPC service in its keeper, which enables querying light client states. A gap in the current model is the lack of queries for proof verification. To address this, we propose introducing a non-state-modifying proof verification query in the 02-client module. This enhancement will reduce the engineering complexity and dependencies involved in building conditional clients.
Here is an example workflow to illustrate the proposed solution based on #5112:

  1. Packet is sent on an optimistic rollup.
  2. State header for this execution is submitted to Celestia.
  3. Relayer updates the Celestia client (this is a non-conditional client type such as 07-tendermint) on the counterparty chain (say Osmosis).
  4. 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 conditional client.
  5. Conditional client on Osmosis sends a QueryVerifyMembershipProofRequest for the inclusion of the state header on Celestia's client.
  6. If the result of the query is true, the conditional client performs the proof verification on the inclusion of the packet commitment in the state header of the conditional client.
  7. If the proof succeeds, the fraud window begins.
  8. The rest of the execution is as described in #5112.

Proposal

02-client modifications

Our approach introduces a new query, VerifyMembershipProof, to the 02-client module. Note that this query is marked as module_query_safe to highlight that it can be safely called from within the state machine. This requires that we ensure that the query is deterministic and has its gas usage tracked.

This proposal only introduces the VerifyMembershipProof query because it wasn't clear if other queries would be needed. We can add more queries in the same manner if needed.

// Query provides defines the gRPC querier service
service Query {
  // .. other queries

  // VerifyMembershipProof queries an IBC light client to verify a proof of a commitment
  rpc VerifyMembershipProof(QueryVerifyMembershipProofRequest) returns (QueryVerifyMembershipProofResponse) {
    option (cosmos.query.v1.module_query_safe) = true;
    option (google.api.http) = {
            post: "/ibc/core/client/v1/verify_membership_proof"
            body: "*"
        };
  }
}

// QueryVerifyMembershipProofRequest is the request type for the Query/VerifyMembershipProof RPC method
message QueryVerifyMembershipProofRequest {
  string client_id = 1;
  bytes proof = 2;
  ibc.core.commitment.v1.MerklePath merkle_path = 3;
  ibc.core.client.v1.Height proof_height = 4;
  bytes value = 5;
  // optional time delay
  uint64 time_delay = 6;
  // optional block delay
  uint64 block_delay = 7;
}

// QueryVerifyMembershipProofResponse is the response type for the Query/VerifyMembershipProof RPC method
message QueryVerifyMembershipProofResponse {
  bool result = 1;
}

In implementing this, a new gRPC query handler is added to the 02-client module:

// VerifyMembershipProof implements the Query/VerifyMembershipProof gRPC method
func (k Keeper) VerifyMembershipProof(c context.Context, req *types.QueryVerifyMembershipProofRequest) (*types.QueryVerifyMembershipProofResponse, error) {
	if req == nil {
		return nil, status.Error(codes.InvalidArgument, "empty request")
	}

	if err := host.ClientIdentifierValidator(req.ClientId); err != nil {
		return nil, status.Error(codes.InvalidArgument, err.Error())
	}

	if req.MerklePath == nil {
		return nil, status.Error(codes.InvalidArgument, "empty merkle path")
	}

	if req.Proof == nil {
		return nil, status.Error(codes.InvalidArgument, "empty proof")
	}

	if req.ProofHeight == nil {
		return nil, status.Error(codes.InvalidArgument, "empty proof height")
	}

	if req.Value == nil {
		return nil, status.Error(codes.InvalidArgument, "empty value")
	}

	ctx := sdk.UnwrapSDKContext(c)
	// cache the context to ensure clientState.VerifyMembership does not change state
	cachedCtx, _ := ctx.CacheContext()

	// make sure we charge the higher level context even on panic
	defer func() {
		ctx.GasMeter().ConsumeGas(cachedCtx.GasMeter().GasConsumed(), "verify membership query")
	}()

	clientState, found := k.GetClientState(cachedCtx, req.ClientId)
	if !found {
		return nil, status.Error(
			codes.NotFound,
			errorsmod.Wrap(types.ErrClientNotFound, req.ClientId).Error(),
		)
	}

	result := true
	if err := clientState.VerifyMembership(ctx, k.ClientStore(cachedCtx, req.ClientId), k.cdc, req.ProofHeight, req.TimeDelay, req.BlockDelay, req.Proof, req.MerklePath, req.Value); err != nil {
		result = false
	}

	return &types.QueryVerifyMembershipProofResponse{
		Result: result,
	}, nil
}

08-wasm modifications

Queriers have already been introduced to 08-wasm as a part of #5325. Thus, the only change that needs to be done is to enable this query type by default in /modules/light-clients/08-wasm/types/querier.go

// AcceptListStargateQuerier allows all queries that are in the provided accept list.
// This function returns protobuf encoded responses in bytes.
func AcceptListStargateQuerier(acceptedQueries []string) func(sdk.Context, *wasmvmtypes.StargateQuery) ([]byte, error) {
+	defaultAcceptList := []string{
+		"/ibc.core.client.v1.Query/VerifyMembershipProof",
+	}
+
	return func(ctx sdk.Context, request *wasmvmtypes.StargateQuery) ([]byte, error) {
		// A default list of accepted queries can be added here.
-		// accepted = append(defaultAcceptList, accepted...)
+		acceptedQueries = append(defaultAcceptList, acceptedQueries...)

		isAccepted := slices.Contains(acceptedQueries, request.Path)
		if !isAccepted {
			return nil, wasmvmtypes.UnsupportedRequest{Kind: fmt.Sprintf("'%s' path is not allowed from the contract", request.Path)}
		}

		route := ibcwasm.GetQueryRouter().Route(request.Path)
		if route == nil {
			return nil, wasmvmtypes.UnsupportedRequest{Kind: fmt.Sprintf("No route to query '%s'", request.Path)}
		}

		res, err := route(ctx, &abci.RequestQuery{
			Data: request.Data,
			Path: request.Path,
		})
		if err != nil {
			return nil, err
		}
		if res == nil || res.Value == nil {
			return nil, wasmvmtypes.InvalidResponse{Err: "Query response is empty"}
		}

		return res.Value, nil
	}
}

Other clients

Clients that don't need to depend on other light clients can continue to be implemented as they are now.
For those requiring inter-client dependencies, 02-client's query server offers a streamlined method to interact through gRPC or by directly accessing the 02-client keeper.

Pros and Cons

A proof of concept implementation of this proposal is in the serdar/poc-querier-conditional-clients branch. (Outdated with respect to 08-wasm modifications, but not outdated for 02-client modifications)

Pros

  • This does not require any changes to the wasm client contracts.
  • CosmWasm developers gain access to a familiar interface for querying light clients.
  • Our approach allows querying any module (if enabled), not just 02-client, enhancing flexibility.
  • Safety. Conditional clients cannot modify the state of other light clients.
  • VerifyMembershipProof facilitates proof verification through gRPC by any on-chain or off-chain entity.
  • Contract developers can run the query mid contract execution, instead of having to receive a cosmwasm reply.

Cons

  • VerifyMembership is technically a state modifying method, but we are using it in a non-state modifying query. This is a bit of a hack.
  • The 08-wasm module might require maintenance for a whitelist of queries or its conversion into a chain parameter.
  • Each time a new query is needed, such as VerifyNonMembershipProof, we need to add a new query handler to the 02-client module.

Pros and Cons

Pros

  • No whitelist required as the message server is always deterministic and gas tracked.
  • Does not require any changes to already deployed contracts.
  • CosmWasm developers gain access to a familiar interface for sending messages to light clients.
  • Our approach allows sending messages to any module, not just 02-client. ( Always enabled )

Cons

  • This is a state machine breaking change for ibc-go.
  • This requires a new entry point to be added to the contracts that depend on other light clients.
  • Each time a new message is needed, such as VerifyNonMembershipProof, we need to add a new message handler to the 02-client module.
  • Each light client needs to be assigned a Cosmos SDK address if they depend on other light clients. This is a bit of a hack.

Conclusion

Considering the advantages and limitations of both the gRPC querier and message server approaches, the querier method appears more suitable for conditional clients in the current context. It offers a non-intrusive, flexible solution compatible with existing systems. However, should there be a future need for comprehensive state-modifying inter-client communication, the message server approach remains a viable option to explore.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@srdtrk srdtrk added needs discussion Issues that need discussion before they can be worked on type: feature New features, sub-features or integrations 08-wasm 02-client labels Dec 5, 2023
@webmaster128
Copy link
Member

Note: Currently, the stargate querier provided in wasmd converts the protobuf encoded query responses to JSON. In our conversation with @webmaster128, we decided that it would be better to return the protobuf encoded responses directly.
Therefore, unlike wasmd, we will return protobuf encoded responses directly in our implementation.

👍 Introducing JSON responses was a mistake we will repair in wasmd as well in the future.

@colin-axner
Copy link
Contributor

colin-axner commented Dec 6, 2023

Excellent investigative work!! 🙏

I'm happy with either approach :)

To add some more color to the general design narrative. As mentioned, the current state of ibc-go creates a uni-directional communication channel where 02-client sends requests to the light client modules. It's desirable to have bi-directional communication (conditional clients being an initial use case). To achieve this, we are establishing two sets of API's:

API for sending requests to light client modules this is what is proposed in #5084. This is yet to be designed, but I'm imagining something similar to the existing client state interface, but a bit more standardized

API for sending requests to 02-client this is what is proposed here. As noted in this issue, there's two sets of API naturally exposed by 02-client. The message server and the grpc query server. As of today, these are intended for user <> ibc-go interaction, but they are reusable for module <> ibc-go interaction. The primary difference between the two is that the grpc query server does perform state changes and generally does not track gas unless marked as module safe and modified to handle gas (as noted in this issue)

This issue outlines that there are two possible flows:

  1. conditional client -> querier (08-wasm) -> 02-client, grpc req -> client router -> light client module API -> return back to contract
  2. conditional client -> sub-msg responses (08-wasm) -> 02-client, msg server -> client router -> light client module API -> return back to 08-wasm -> reply to contract

The first is a read-only query to a state modifying API (state changes discarded) during contract execution. The second is a sub-messages responses execution without an optional reply back to the contract with the message execution result.

The only thing I find odd is that we make a read-only query to a state modifying API, but in 99% of cases, this is a non-state modifying action. Only solo machine modifies state during VerifyMembership and VerifyNonMembership. If we reworked solo machine to somehow not modify state during membership verification then these would be read-only, but simultaneously, that's a bit like treating the ante handler (sig verifier) as a type of query. But I don't think this is a substantial enough reason not to add the grpc routes

I think we will likely be asked to support both approaches and I think it's possible we end up with a VerifyMembership grpc route and a VerifyMembership msg server handler (but that doesn't mean we need to support both now). Once we add a query route or msg service handler, I expect we commit to maintaining it for quite a while to avoid breaking contracts.

@crodriguezvega crodriguezvega added this to the Rollkit integration milestone Jan 10, 2024
@colin-axner colin-axner moved this to Todo in ibc-go Jan 29, 2024
@colin-axner colin-axner removed the needs discussion Issues that need discussion before they can be worked on label Jan 29, 2024
@colin-axner
Copy link
Contributor

We decided to move forward with this approach adding VerifyMembership to the gRPC server. It will explicitly disallow usage of the solo-machine

@srdtrk
Copy link
Member Author

srdtrk commented Jan 29, 2024

I updated the 08-wasm modifications section, accounting for the changes made in #5325

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
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants