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

Generate queriers for interfaces #28

Closed
hashedone opened this issue Aug 26, 2022 · 7 comments · Fixed by #141
Closed

Generate queriers for interfaces #28

hashedone opened this issue Aug 26, 2022 · 7 comments · Fixed by #141
Assignees
Milestone

Comments

@hashedone
Copy link
Collaborator

It would be helpful to provide utilities to easily query the contract.

For interfaces:

#[interface]
pub trait Cw1 {
  type Error: From<StdError>;
  
  #[msg(query)]
  fn find_member(&self, ctx: QueryCtx, member: String) -> Result<FindMemberResponse, Self::Error>;
}

// Generated:
pub struct Cw1Querier<'a> {
  contract: &'a Addr,
  querier: &'a cosmwasm_std::Querier,
}

impl<'a> Cw1Querier<'a> {
  pub fn new(querier: &'a cosmwasm_std::Querier, contract: Addr) -> Self { /* ... */ }
  
  pub fn find_member(&self, member: String) -> Result<FindMemberResponse, Self::Error>;
}

Additionally for contracts:

#[contract]
#[messages(cw1 as Cw1)]
pub struct Cw1WhitelistContract { /* ... */ }

#[derive(Serialize, Deserialize)]
pub struct Cw1WhitelistContractProxy(Addr);

impl Cw1WhitelistContractProxy {
  pub fn addr(&self) -> &Addr;
  
  pub fn cw1<'a>(&'a self, querier: &'a cosmwasm_std::Querier) -> Cw1Querier<'a> { /* .. */ }
}

This would allow keeping the contract in storage with semantical contract type, and then directly call queries. The access to underlying address is obviously required (eg. for sending submessages). Also implementing impl From<Cw1WhitelistContractProxy> for Addr should probably happen for free unwrapping.

@ethanfrey
Copy link
Member

This would be a great help to add.
(Also explanation on how to use cargo doc or other tools to see it)

@hashedone hashedone added this to the 0.4 milestone Apr 28, 2023
@ethanfrey
Copy link
Member

Hey @hashedone I wrote some such structs for use with mesh-security, focusing just on calling the traits / interfaces (which is what we are importing).

Check out osmosis-labs/mesh-security#21

I think the code there could totally be auto-generated (I added the last funds: Vec<Coin> argument to show that). Maybe a bit of cleanup on the API, but no need to add yet. I like adding some contract-specific one and a builder for instantiate and all, but that is another level. For now, ensuring we properly call remote contracts that implement some expected interface is a clear and useful use-case and doesn't seem like too much lift.

Let me know if this looks doable.

@hashedone hashedone self-assigned this May 5, 2023
@hashedone
Copy link
Collaborator Author

@ethanfrey I've checked your helpers - they are ok, but in the general case, they might not be as perfect. You create a long-living query with an address inside and then add functions to it, but the problem is - this works only if you know exactly what contract you query, so you lose the idea of interfaces. And there would be a way to achieve this for interfaces - maybe a querier is defined for the interface keeping its address.

But then there is a problem with combining interfaces - maybe I care about two interfaces, but depending on the context (some methods use group and some method uses distribution parts of the contract, some of them maybe both - and the contract has also other things which I don't care about right now).

That is why, in the initial proposal, queriers are temp short-living objects doing all the functionality for a trait or contract.

The task would be split into three small ones:

  • Implement queriers generated for interfaces
  • Implement queriers generated for contracts
  • Add function from interfaces implemented on a contract by extension traits

@hashedone
Copy link
Collaborator Author

The PR is done, but generated structs are oversimplified, so there would be a real problem extending in the future. Let me formalize what should be generated here, splitting it into expected PRs:

PR1 - Remote structure

All contracts should have a type for storing them as remote contracts with some address. I suggest the Remote name. A type like this should be generated for every single interface and contract itself:

pub struct Remote<'a>(Cow<'a, Addr>);

impl Remote<'static> {
  pub fn new(addr: Addr) -> Self {
    todo!()
  }
}

impl Remote<'a> {
  pub fn borrowed(addr: &'a Addr) -> Self {
    todo!()
  }
}

The type does nothing for now, but it exists on all interfaces and contracts, and all helper traits (querier, executors) will be implemented on this.

For contracts, we need some additional things to be generated to make it easy to convert the contract-level remotes to trait-level remotes:

impl<'a> From<&'a Remote<'a>> for counter::Remote<'a> {
  fn from(...) -> Self {
    todo!()
  }
}

Remote here is a remote for the contract, counter::Remote is remote for counter::Counter trait. Then we can get a counter-level remote with from (or into):

let remote = counter::Remote::from(&contract_remote);

Note that the address is kept as Cow<'a, Addr> instead of the owned address. There are two reasons:

  1. When we convert remotes around, we want to clone the address
  2. Sometimes, we don't want to store the remote, but we want to create short-living one from the address; we want to avoid cloning the address in such a scenario.

Note that all the remotes are named the same way - it would vastly improve implementation, but what is more important, it is more clear in fact. The thing is that in Rust use clause can import a module, not a particular entity. The idea is, that all those remotes, queries, mt helpers shall not be imported to the global namespace - the module of a trait/contract should be imported, and instead of prefixing the entity name with the trait/contract name, we "prefix" it with a module on use:

use crate::counter;

fn foo(addr: Addr) {
  let remote = counter::Remote::new(addr);
}

@hashedone
Copy link
Collaborator Author

hashedone commented May 16, 2023

PR2 - queriers

Having a remote wrapper, the queries are to be added. Every "message source" (so contract and interface) should generate a BoundQuerier type which holds a reference to the cosmwasm_std::Querier type:

struct BoundQuerier<'a> {
  addr: &'a Addr,
  querier: &'a cosmwasm_std::Querier,
}

impl Remote<'_> {
  fn querier(&self, querier: &cosmwasm_std::Querier) -> BoundQuerier {
    todo!();
  }
}

The next step is to generate the Querier trait for every message holder:

pub trait Querier {
  fn count(&self, /* all message arguments */) -> /* The result type */;
}

The trait should be implemented on the BoundQuery type for the interface/contract.

This is everything for this PR. It still has an inconvenience known as "how to call methods from traits on the remote for contract". The answer is: create a temporary remote for trait:

let remote = contract::Remote::new(addr);
let resp = counter::Remote::from(&remote).querier(&deps.querier).count()?;

This could be nicer, but we want to minimize the size of PRs - this will be solved in a second.

@hashedone
Copy link
Collaborator Author

PR3 - BoundQeurier conversions

Now we want to generate conversion methods for already-bound queries. Every contract-level should have From conversions generated for all interface-lever bound queries it implements:

impl<'a> From<&'a BoundQuerier<'a>> for counter::BoundQuerier<'a> {
  // ...
}

Now we can bind the early and reuse single binding for contract and trait calls:

let remote = contract::Remote::new(addr);
let querier = remote.querier(&deps.querier);
let resp = counter::BoundQuerier::from(querier)).count()?;

Additionally, sometimes we may want to create a temporary querier for the contract or interface, so we can add additional functions on them (both interface and contract level):

impl BoundQuerier<'_> {
  fn borrowed(&self, querier: cosmwasm_std::Querier) -> Self {
    todo!();
  }
}

So we can now write:

let resp = counter::BoundQuerier(&addr, &deps.querier).count()?;

This is more or less what it pending on PR right now. Still API for calling trait methods is not perfect, but it is the intermediate state.

@hashedone
Copy link
Collaborator Author

PR4 - interface-level Queriers on contract-level BoundQuerier

The final one for this - we want to implement all Querier traits from interfaces contract implements. So the contract level BoundQuerier should implement counter::Querier directly:

impl counter::Querier for BoundQuerier<'_> {
  // ...
}

This finally enables transparent call of the interface-level methods:

use counter::Querier;

fn foo(addr: Addr, deps: Deps) {
  let resp = Remote::borrowed(&addr).querier(&deps.querier).count()?;
}

Note that the Querier trait has to be imported directly - it is fine for traits, it will just allow usage of their methods.

This is everything for queries. Next step would be adding helpers for messages, but it is another issue.

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

Successfully merging a pull request may close this issue.

2 participants