Define a macro for extending CW721 execute/query messages

[larry0x]

The problem

cw721::ExecuteMsg defines a minimum set of execute methods that all CW-721 variants are expected to implement. However, projects often want to extend it by adding their own methods. For example, cw721-base adds "mint" and "burn" methods. The process for adding these methods is rather cumbersome, for example:

// cw721-base/src/msg.rs
pub enum Cw721ExecuteMsg {
    // below are all copy-pasted from cw721::ExecuteMsg
    TransferNft { .. },
    SendNft { .. },
    Approve { .. },
    Revoke { .. },
    ApproveAll { .. },

    // only the following are new methods added by cw721-base
    RevokeAll { .. },
    Burn { .. }
}

Imo, there are two drawbacks with this approach:

• A lot of copy-pasting is needed (very verbose)
• It is not enforced that the execute message implements all variants required by CW721 specs (What if, say, TransferNft is omitted by mistake? Preferably we want an error to be thrown at compile time to alert the developer that they forgot something.)

Proposed solution

@0xekez showed me it is possible to "autofill" an enum with some pre-defined variants by a macro, which is an approach already used by DAO DAO: https://github.com/DA0-DA0/dao-contracts/blob/main/packages/cwd-macros/src/lib.rs

I propose we create a #[cw721_execute] macro that if used the following way:

use cw721::cw721_execute;

#[cw721_execute]
enum ExecuteMsg {
    Foo,
    Bar,
}

Will transform the enum to:

#[cw_serde]
enum ExecuteMsg {
    TransferNft { .. },
    SendNft { .. },
    Approve { .. },
    Revoke { .. },
    ApproveAll { .. },
    Foo,
    Bar,
}

This prevents the developer to have to copy all the CW721 enum variants over, and enforces all variants are included. Additionally, #[cw_serde] is automatically applied.

In the cw721 crate, the execute message can simply be defined as:

#[cw721_execute]
pub enum ExecuteMsg {}

Further suggestion

It'd be even better if the following traits can be automatically implemented as well:

impl From<cw721::ExecuteMsg> for ExecuteMsg {
    fn from(msg: cw721::ExecuteMsg) -> Self {
        match msg {
            cw721::ExecuteMsg::TransferNft { .. } => Self::TransferNft { .. },
            // ...
        }
    }
}

impl TryFrom<ExecuteMsg> for cw721::ExecuteMsg {
    type Error = String; // or other error type we prefer
    fn try_from(msg: ExecuteMsg) -> Result<Self, Self::Error> {
        match msg {
            ExecuteMsg::TransferNft { .. } => Ok(cw721::ExecuteMsg::TransferNft { .. }),
            // ...
            ExecuteMsg::Foo => Err("`cw721::ExecuteMsg` does not have variant `Foo`".to_string()),
            ExecuteMsg::Bar => Err("`cw721::ExecuteMsg` does not have variant `Bar`".to_string()),
        }
    }
}

This makes it much easier to implement execute entry point:

#[entry_point]
pub fn execute(
    deps: DepsMut,
    env: Env,
    info: MessageInfo,
    msg: ExecuteMsg,
) -> Result<Response, ContractError> {
    match msg {
        // For custom execute methods, dispatch to the appropriate
        // handler functions
        ExecuteMsg::Foo => execute_foo(deps, env, info),
        ExecuteMsg::Bar => execute_bar(deps, env, info),

        // For generic cw721 execute methods,
        // instead of writing a bunch of matching statements,
        // simply cast `ExecuteMsg` into the parent message type
        // and dispatch to the parent contract.
        msg => {
            let parent = Cw721Contract::<Empty, Empty>::default();
            parent.execute(deps, env, info, msg.try_into()?)
        },
    }
}

Notes

• If adopted, this change will deprecate cw721::ExecuteMsg::Extension method as it is no longer needed.

[JakeHartnell]

Love this. :heart:

Everyone hates the extension pattern, would allow us to simplify a lot of stuff. :joy:

[shanev]

This greatly simplifies things for the developer that knows what's going on under the hood. But what about a noob dev that doesn't?

[webmaster128]

Great to see this solution by @0xekez / the DAO DAO team. Some thoughts:

• Procedural macros come with significat maintenance overhead. They require you to ship an extra crate. But regular macros don't seem to allow adding cases to enums.
• #[cw721_execute] is a bit short. I'd lean towards something like #[add_cw721_execute_cases]
• The risk of collisions is real, which would be avoided by nesting instead of inlining
• @hashedone and @jawoznia are working on something like this in the Sylvia contract framework but I don't know the technical details of that
• This is cool stuff