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

Add custom query support to wasm module #5192

Closed
2 of 3 tasks
aeryz opened this issue Nov 28, 2023 · 3 comments · Fixed by #5261
Closed
2 of 3 tasks

Add custom query support to wasm module #5192

aeryz opened this issue Nov 28, 2023 · 3 comments · Fixed by #5261

Comments

@aeryz
Copy link
Contributor

aeryz commented Nov 28, 2023

Summary

CosmWasm supports chains to add their custom query implementations. With this feature, chains can implement some functionality natively and call them from the contract by doing a query. This would be a nice to have feature in wasm module as well since the light clients might want to do some expensive operations natively.

Possible Implementation

Add a global querier to 08-wasm/types/vm.go

var (
	Querier       wasmvm.Querier // <--- NEW
	VMGasRegister = NewDefaultWasmGasRegister()
	wasmvmAPI = wasmvm.GoAPI{
		HumanAddress:     humanAddress,
		CanonicalAddress: canonicalAddress,
	}
)

And pass the querier to the VM whenever we call it

ibcwasm.GetVM().Instantiate(
                    checksum,
                    env,
                    msgInfo,
                    msg,
                    newStoreAdapter(clientStore),
                    wasmvmAPI,
                    Querier, // <---- NEW, previously `nil`
                    multipliedGasMeter,
                    gasLimit,
                    costJSONDeserialization,
                  )

Add querier parameter to the constructors

func NewKeeperWithVM(
	cdc codec.BinaryCodec,
	storeService storetypes.KVStoreService,
	clientKeeper types.ClientKeeper,
	authority string,
	vm ibcwasm.WasmEngine,
	querier wasmvm.Querier, // <--- NEW
) Keeper {
	if clientKeeper == nil {
		panic(errors.New("client keeper must be not nil"))
	}
	types.Querier = querier // <--- NEW

  ...
}

How app developers will use this?

Let's say that we have this query defined in the light client:

pub enum CustomQuery {
  Foo {}
}

Devs will just do:

let ret: SomeType = deps.querier.query(&QueryRequest::Custom(CustomQuery::Foo {}))?.try_into()?;

The chain will implement the query:

type CustomQuery struct {
  Foo *QueryFoo `json:"foo,omitempty"`
}

type QueryFoo struct {}

type CustomQueryHandler struct{}

func (h *CustomQueryHandler) GasConsumed() uint64 {
  // ...
}

func (h *CustomQueryHandler) Query(request wasmvmtypes.QueryRequest, gasLimit uint64) ([]byte, error) {
    var customQuery CustomQuery
  	err := json.Unmarshal([]byte(request.Custom), &customQuery)

    // Some expensive operation

    return json.Marshal(ret), nil
}

Also note that I would be happy to create a PR for this.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega
Copy link
Contributor

Thanks for the clear and detailed issue, @aeryz! We discussed this improvement internally and we decided to label it as nice-to-have for 08-wasm/v0.1.0, since we could implement it afterwards and include it in 08-wasm/v0.2.0.

Just some suggestions to build upon your proposal:

We could add an interface in expected_keepers.go for Querier, just as we have for WasmEngine:

type WasmQuerier interface {
  // Query takes a query request, performs the query and returns the response.
  // It takes a gas limit measured in [CosmWasm gas] (aka. wasmvm gas) to ensure
  // the query does not consume more gas than the contract execution has left.
  //
  // [CosmWasm gas]: https://github.com/CosmWasm/cosmwasm/blob/v1.3.1/docs/GAS.md
  Query(request wasmvmtypes.QueryRequest, gasLimit uint64) ([]byte, error)
  // GasConsumed returns the gas that was consumed by the querier during its entire
  // lifetime or by the context in which it was executed in. The absolute gas values
  // must not be used directly as it is undefined what is included in this value. Instead
  // wasmvm will call GasConsumed before and after the query and use the difference
  // as the query's gas usage.
  // Like the gas limit above, this is measured in [CosmWasm gas] (aka. wasmvm gas).
  //
  // [CosmWasm gas]: https://github.com/CosmWasm/cosmwasm/blob/v1.3.1/docs/GAS.md
  GasConsumed() uint64
}

And similarly as we do for with the wasm VM, we could have an internal variable in wasm.go with gettter/setter functions:

var (
  vm      WasmEngine
  querier WasmQuerier // <------ NEW

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

  // ChecksumsKey is the key under which all checksums are stored
  ChecksumsKey = collections.NewPrefix(0)
)

func SetQuerier(wasmQuerier WasmQuerier) {
  querier = wasmQuerier
}

func GetQuerier() WasmQuerier {
  return querier
}

And in the constructor function we would do:

func NewKeeperWithVM(
  cdc codec.BinaryCodec,
  storeService storetypes.KVStoreService,
  clientKeeper types.ClientKeeper,
  authority string,
  vm ibcwasm.WasmEngine,
  querier ibcwasm.WasmQuerier,
) Keeper {
  ...
  ibcwasm.SetVM(vm)
  ibcwasm.SetQuerier(querier) // <------ NEW
  ibcwasm.SetupWasmStoreService(storeService)
  ...

And then wherever we call the VM we pass the querier:

ibcwasm.GetVM().Instantiate(
  checksum,
  env,
  msgInfo,
  msg,
  newStoreAdapter(clientStore),
  wasmvmAPI,
  ibcwasm.GetQuerier(), // <---- NEW, previously `nil`
  multipliedGasMeter,
  gasLimit,
  costJSONDeserialization,
)

How does this look?

Thank you for offering to open a PR for this addition! We can try to include it in the v0.1.0 release if we can merge it and back port it to the release lines before the end of next week. We would like to tag a RC by the end of next week (or early the week after). If it doesn't make the cut, would it be ok for you if we add this in v0.2.0? We just would like to finally draw a line on the scope of the v0.1.0 release and finally release 08-wasm before the end of the year.

@aeryz
Copy link
Contributor Author

aeryz commented Nov 29, 2023

Hey, thanks for the quick response. I am not sure what's the planned timeline for v0.2.0 but it would be so nice for us to have this in the next week's release. If we can't make it to that though, it's fine. I have the PR ready here which also contains a custom query test. I was looking at the branches, should I need to open the PR to the main branch or some other feature branch?

@crodriguezvega
Copy link
Contributor

Great, thank you for writing the code so quickly! I think we still have enough time then to include this in v0.1.0. Please feel free to open a PR (targeting main) at your convenience. Once the PR is merged we will back port it to the release lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants