Proposal for improving contract interfaces

[hashedone]

For now contract interfaces are defined by structures defining messages (queries/executes/migrations/instantiations) for contracts. On the one hand this seems simple, but writing contracts I found problems with this approach. First of all re usability and extend ability of contracts. If there is cw20-base contract defining some basic implementation of cw20 token, and then there is another contract which is based on it, but just extending it further, then the new contract is typically copying messages from base contract, adding on top of them, and in the implementation basically forward-calling implementation of cw20-base. I find this messy, and difficult to maintain - in case of update of cw20-base, all contracts basing on it should be probably updated, but in reality we technically cannot do that - because someone may just have another contract based on it, and he would not be notified anyhow about changes (unless he reads all changelogs).

My proposal is, to add possibility of more natural way of defining interfaces - as traits. For cw20 contract trait could look like:

#[cw_derive::interface]
trait Cw20Base {
  #[exec]
  fn transfer(&self, ctx: CtxMut, recipient: Addr, amount: u128) -> Result<Response, Error>;
  
  #[exec]
  fn burn(&self, ctx: CtxMut, amount: String) -> Result<Response, Error>;
  
  // Another exec messages
  // ...
  
  #[query]
  fn balance(&self, ctx: Ctx, address: Addr) -> Result<BalanceResponse, Error>;
  
  #[query]
  fn token_info(&self, ctx: Ctx) -> Result<TokenInfoResponse, Error>;
  
  // Another queries
  // ...
}

The whole idea is to provide interface attribute which generates structures for parsing from jsons, and generates dispatching function which takes parsed input and calls proper functions. Also this handles intermediate wrappers like Uint128 which as far as I understand are just for proper parsing, but using them in Rust code itself seems inconvenient. Also taking Addr in interfaces is proper here - dispatcher can know to convert it to validate it with Addr::validate(...). The additional Ctx type is helper providing access to things such as Deps/DepsMut, Env, MessageInfo.

The second part of this would be actually generating implementation structures, like:

#[cw_derive::contract(Cw20Base)]
struct Cw20BaseContract;

#[cw_derive::contract_impl]
impl Cw20BaseContract {
  #[instantiate]
  fn instantiate(&mut self, ctx: CtxMut, name: String, symbol: String, ...)
    -> Result<Response, Error> { todo!() }
  
  #[migrate]
  fn migrate(&mut self, ctx: CtxMut) -> Result<Response, Error> { todo!() }
}

impl Cw20Base for Cw20BaseContract {
  // ...
}

The contract macro takes as arguments all interfaces traits which would be implemented, so additional dispatch function is generated - basically creating intermediate serde deserializable untagged enum, and after deserialization dispatching to proper trait implementation. The downside of this is what happens in case of interface conflicts (same message possible for two interfaces) - but either it can be ignored, then just earlier interface in the list has precedence on handling message, or it is basically possible to detect such cases in generator itself to report an error. The last thing which the contract would take care of is generating entry points for the contracts, which would create Cw20Contract inplace, and call proper function on this.

Just to make whole thing more complete, here is how the contract extending Cw20Base could look like:

#[cw_derive::interface]
trait Cw20Bonding {
  #[exec]
  fn buy(&mut self, ctx: CtxMut) -> Result<Response, Error>;
  
  #[query]
  fn curve_info(&self, ctx: Ctx) -> Result<CurveInfoResponse, Error>;
}

#[cw_derive::contract(Cw20Base, Cw20Bonding)]
// This is generic so mock implementation can be injected. It could be simplified by `Box<dyn Cw20Base>`
// but it gains dynamic dispatch overhead, and even space overhead (in most cases ZST vs fat pointer)
struct Cw20BondingContract<Cw20Base> {
  base: Cw20BaseContract,
}

// Instantiation and migration omitted, but should be similar to base contract

#[cw_derive::forward_impl(
    base;
      transfer, send, increase_allowance, decrease_allowance,
      transfer_from, send_from
)]
impl<Base: Cw20Base> Cw20Base for Cw20BondingContract<Base> {
  // Need to hand implement everything which is not forwarded
  fn burn(&mut self, ctx: CtxMut, amount: u128) -> Result<Response, Error> { todo!() }
  fn burn_from(&mut self, ctx: CtxMut, owner: Addr, amount: u128)
    -> Result<Response, Error> { todo!() }
}

impl<Base: Cw20Base> Cw20Bonding for Cw20BondingContract<Base> {
  // Implementation of `buy` and `curve_info`
}

The most upside of this approach is, that it does not impact interfacing with wasm at all - whole toolset can be even separated crate on top of wasmstd (possibly should be). And upsides of this approach are:

• Contracts are more behavior focused instead of messages focused. This strongly improves testability - on unit test level, the base contract can be substituted with mock implementation (possibly generated with mockall or similar tool). This way instead of testing all contract in the stack (which is good but on another level), only new implementation is tested assuming, as underlying one is correct
• Easier maintenance of extending contracts - when upgrading to new version of base contract, there would be missing functions implementations
• More similarity to solidity framework (maybe not so important, but at the end it makes dev transition easier)
• Possibility to extend it to generate additional helpers (some contracts actually contain helpers performing cross-contracts queries - it can be unified)

Downsides:

• Requires time to develop
• Probably problems I am not (yet) aware of

For now this is just brief idea, to get feedback what ppl think about such approach. If it would be positive, than I could process to implement some PoC (as separated crate - making it part of cosmwasm-std may be some step in future, but is actually not needed, however possibly nice for unification purposes).

Also this draft is basically long term direction how I see interfaces for contracts may look like, proof of concept would be developed in small chunks progressing to some final form. And obviously - there may be difficulties I cannot predict for now (any suggestions would be nice).

[ethanfrey]

Let's look at json schema -> ts bindings auto-get solution, so there is no focus on "ExecuteMsg" rust type
(As a transition, generate what it "would be" into docs.rs)

[hashedone]

Additionally a word about implementing contracts with generic messages, cw1 as an example:

#[cw_derive::interface]
trait Cw1<T = Empty> {
  #[exec]
  fn execute(&self, ctx: CtxMut, msgs: Vec<CosmosMsg<T>>) -> Result<Response, Error>;
}

#[cw_derive::interface]
trait Cw1Subkeys<T = Empty> {
  #[exec]
  fn freeze(&self, ctx: CtxMut) -> Result<Response, Error>;
  
  // More of contract specifics
}

#[cw_derive::contract(Cw1, Cw1Subkeys)]
struct Cw1SubkeysContract;

#[cw_derive::contract_impl]
impl Cw1SubkeysContract {
  #[instantiate]
  fn instantiate(&mut self, ctx: CtxMut, name: String, symbol: String, ...)
    -> Result<Response, Error> { todo!() }

  // Also possibly migration, sudo, and other optional things
}

impl<T> Cw1<T> for Cw1SubkeysContract
  where T: Clone + Debug + PartialEq
{
  // Execute implementation
}

impl<T> Cw1Subkeys<T> for Cw1SubkeysContract
  where T: Clone + Debug + PartialEq + PossiblySomeMoreSpecializedTrait
{
  // All subkeys specific implementation
}

Basically generic traits would generate generic messages. To generate generic instantiate/migrate messages, just specific functions would be generics.

Also @ethanfrey bring an issue, that this solution removes interface structures from code, which might be a problem as they are used by js/go devs to create their own interfaces. Generating js/go interfaces from json schemas might be a thing in future, but it is not now. However I have a solution for that - just generate the code and store it in either .rs not included to module, or 'md (it can be done either by separate command like cargo schema, or as part as normal compilation flow with build.rs). Another, probably better solution is to generate doc comments for the structures, the problem here is - I don't remember how cargo docs handles procedural macros, but I think it is possible (to be investigated).

Also there is an issue of using this approach vs cosmwasm-std (like normal todays approach) - it looks like it would create two ways of creating contracts (everyone loves two standard libraries of D...). The idea is to not make it an issue at all. The proposed cw_derive way would generate struct very similar (or if possible identical) as handwritten ones. Basically cw_derive is designed to be additional layer on top of what is there now, improving some aspects of contracts. Contracts using traditional approach would be able to create contracts structs (and Ctx) inplace, and they should be ZST or transparent structs to not add overhead, also cw_derive contracts may easly call functions defined in traditional way (passing parts of Ctx). Even if there would be missing functionality in cw_derive (let say defining sudo methods), then they still may be defined the old way.

[ethanfrey]

Contracts using traditional approach would be able to create contracts structs (and Ctx) inplace,

Note this will affect cosmwasm-std. If we remove the 3 arguments and try to normalise them into on Ctx. This is a good discussion to have with @webmaster128 but I would favour keeping the multiple argument format we have now, as there are a few different combinations:

(Dep, Env)
(DepMut, Env, Info)
(DepMut, Env)

[hashedone]

This not affect cosmwasm-std at all, as traditional interfaces still will not use context at all - it would be created in-place in dispatcher functions.

Also good find, that there are several combinations of those contextual types, so probably many types might be needed. This is good find, but I have some solutions for that. Basically note, that I never exactly described how Ctx looks like. It is somehow on purpose - I don't think it should be always the same. Basically the intention is, that it is some type, possibly created by contract creator (because possibly it might do some stuff like precomputations), which are required to be createable from set of arguments proper for the case (so for eg. (DepMut, Env, Info) in case of execution messages), probably via implementation some traits. However probably typical generic implementations would be delivered by crate - here I shown Ctx and CtxMut, but maybe better they would be ExecCtx, QueryCtx, MigrationCtx and so on.

There are two reasons why I prefer to couple contextual types:

• Simplify code generation (just always ignore first two arguments when generating msg structure - fist is self, second is context). I was trying to figure out how to even get rid of context and encapsulate contextual things in self, but things get messy on sketch level so I abandoned this direction
• Make code easier to read. Those functions alredy tend to take tons of arguments. Even clippy doesn't like it (I tweaked treshold from 7 to 10 somewhere so clippy is happy because of that). I think that having 2 arguments less in such case wont hurt readability

[ethanfrey]

I was discussing some issue on Discord where a user wanted a way to easily extend cw721-base (NFTs) to include optional extra data. I thought to add extension: Option<T> to the Item, but then this means all functions need to be aware of the proper T.

After a bit of reflection, I realised that we could make a struct that includes the maps (and applies the proper T everywhere) and using execute_xyz and query_xyz methods much like your proposal, but without the macros.

Then a top level #[entry_point] pub fn execute() could just instantiate the struct with some proper T and then call into it.

This would be a good use case to experiment with your concepts, and work with concrete code until we found a pattern we are happy with that we could optimize with macros. I also realise this would make the contracts look very much like the other controller idea. (Which also all need at least one T to set Response).

I would be happy to take on a few refactorings of concrete contracts/controllers to take some concrete steps in this direction and work out the details.

For me, macros only make sense once the desired code is very clear and it is becoming repetitive to write it. But I would start the design without them.

[mradkov]

https://github.com/hackbg/fadroma

[hashedone]

@mradkov nice think to have in mind.

[uint]

This is already being worked on in Sylvia. Closing!