ibc-go
ibc-go copied to clipboard
Further reduce function arguments for 08-wasm contract calling methods.
Basically, this comment https://github.com/cosmos/ibc-go/pull/6103#discussion_r1566519731. Noticed that we have a couple of ways of accessing client store/state:
- Grab store with
storeProvider
and usetypes.GetClientState
to grab client state. - Use the client keeper stored on the wasm keeper to access these (requiring a cast to
WasmClientState
)
Current pattern for most of the light client module impls is:
- grab store
- grab wasm client state
- create payload
- call sudo/query/migrate etc
Seems like these arguments can just be constructed inside the methods we call instead of passing them as args. Open this issue here to keep track, would be nice to address before v9.
For Admin Use
- [ ] Not duplicate issue
- [ ] Appropriate labels applied
- [ ] Appropriate contributors tagged/assigned
Haven't tried doing the refactor currently, unsure if something might pop up and make this not possible.
Seems like these arguments can just be constructed inside the methods we call instead of passing them as args. Open this issue here to keep track, would be nice to address before v9.
are you suggesting we do something more like below, and get the store and client state at the keeper level?
l.keeper.WasmQuery(ctx, clientID, payload)
yup! I think that for light clients with a keeper that can hold references to a clientKeeper
, the store provider can not be used? I'm unsure if this was the end goal you and Colin had in mind (add internal keepers to lcm's for other clients and just use them to access needed state).
Off the top of my head, I'm thinking we could potentially replace the wasm keeper's clientKeeper entirely with store provider? Would need to look at it more and come back with a fresh answer
edit: If I remember correctly, the only reason wasm keeper has this ref to client keeper is for contract migrations, right?
the only reason wasm keeper has this ref to client keeper is for contract migrations
yea! That's where I encountered its usage. Though would be nice before v9 I guess it isn't as pressing since these are mostly called internally for the time being. We could wait until we see what we do for other light clients and just normalize the way we do things across the board.