#[contractimpl] also copies the procedural macros for the helper functions

[ozgunozerk]

What did you do?

Tried to annotate a smart contract function with a custom macro, and it failed miserably due to how #[contractimpl] macro expands.

More background: we are writing a pausable extension for smart contracts, and we wanted to be able to annotate the functions with #[when_paused] and #[when_not_paused] instead of polluting the function body.

Our macro requires the first parameter of the annotated function to be of type &Env or Env.

And here is the relevant snippet of our code:

#[contractimpl]
impl ExampleContract {
    pub fn __constructor(e: Env, owner: Address) {
        e.storage().instance().set(&DataKey::Owner, &owner);
        e.storage().instance().set(&DataKey::Counter, &0);
    }
    
    /* redacted */

    #[when_paused] // HERE!
    pub fn emergency_reset(e: Env) {
        e.storage().instance().set(&DataKey::Counter, &0);
    }
}

So far so good...

However, when this macro is put to action inside #[contractimpl], the below errors occur.

I was able to expand the #[contractimpl] macro and see the root of the error. Below are the relevant snippets that is generated by the #[contractimpl] expansion (helper functions on related to emergency_reset function):

    impl ExampleContract {
        #[allow(non_snake_case)]
        pub const fn spec_xdr_emergency_reset() -> [u8; 36usize] {
            *b"\0\0\0\0\0\0\0\0\0\0\0\x0femergency_reset\0\0\0\0\0\0\0\0\0"
        }
    }

        pub fn emergency_reset(&self) -> () {
            use core::ops::Not;
            use soroban_sdk::{IntoVal, FromVal};
            let res = self.env.invoke_contract(
                &self.address,
                &{ soroban_sdk::Symbol::new(&self.env, "emergency_reset") },
                ::soroban_sdk::Vec::new(&self.env),
            );
            res
        }

        pub fn try_emergency_reset(
            &self,
        ) -> Result<
            Result<(), <() as soroban_sdk::TryFromVal<soroban_sdk::Env, soroban_sdk::Val>>::Error>,
            Result<soroban_sdk::Error, soroban_sdk::InvokeError>,
        > {
            use soroban_sdk::{IntoVal, FromVal};
            let res = self.env.try_invoke_contract(
                &self.address,
                &{ soroban_sdk::Symbol::new(&self.env, "emergency_reset") },
                ::soroban_sdk::Vec::new(&self.env),
            );
            res
        }

TL;DR

Problem

I think I know the problem already (I've dealt with procedural macros myself). #[contractimpl] macro copies the function name, and creates the necessary helper functions. However, during the process, it keeps the macro annotations for these copies as well. And I think, no macros should be preserved for these copies.

Solution

#[contractimpl] macro should not keep the macros that are annotating the function for the generated helper functions, it should only preserve the macro for the original function.

[leighmcculloch]

Thank you for the detailed report, and identifying the problem. I agree, the contractimpl macro copying not just the function name, but it's annotations/attributes, is problematic.

Part of the reason the attributes were copied was because sometimes folks will name their function something non-standard and quieten clippy by placing a specific attribute on the function. Without copying the attribute to all the generate functions, clippy can warn on the generated code. But there are other ways to address this issue. The generated code could have generous allow attributes to prevent low risk warnings from showing up.

Iirc another reason the attributes were copied was to copy docs as well. We might be able to simply extract only docs attributes though and copy them over.

In any case, the contractimpl macro should stop copying the attributes, so it is more composable. 👍🏻