feat: Blacklist attributes not to forward to ContractExt methods

[mooori]

Background

Smart contract plugins like near-plugins use attribute macros to modify contract methods.

For example, access_control_any in the following snippet injects code that panics if the caller is not a grantee of Role::A or Role::B:

#[near_bindgen]
impl Contract {
    #[access_control_any(roles(Role::LevelA, Role::LevelC))]
    pub fn restricted_method(&mut self) { /* */ }
}

Problem

#[near_bindgen] will create a struct ContractExt with:

#[near_bindgen]
impl ContractExt {
    // _All_ attributes of Contract::restricted_method are forwarded here.
    //
    // We want to avoid forwarding plugin attributes (i.e. access_control_any),
    // as it may not make sense and lead to compilation errors.
    #[access_control_any(roles(Role::LevelA, Role::LevelC))]
    pub fn restricted_method(self) -> near_sdk::Promise { /* */ }
}

Proposed solution

Make #[near_bindgen] accept a blacklist of attributes which it should not forward to methods of the automatically generated *Ext type. This PR adds blacklist_ext_fn_attrs for this purpose:

#[near_bindgen(blacklist_ext_fn_attrs(access_control_any))]
impl Contract {
    #[access_control_any(roles(Role::LevelA, Role::LevelC))]
    pub fn restricted_method(&mut self) { /* */ }
}

Implementation overview

• Add MacroConfig which allows configuring the behavior of macros.
• Pass it down to where the corresponding ContractExt methods are created.
• Forward only attributes which are not blacklisted.

Backwards compatibility

This should be backwards compatible since:

• Currently any tokens following the name of near_bindgen are ignored (ref. Remove unused tokens in compilation test #951).
• After this change, usage of #[near_bindgen] without attributes will result in using the default value of MacroConfig which does not alter behavior.

Thoughts / Question

Maybe function attributes shouldn't be forwarded to Ext methods by default? In that case, it should be possible to turn the blacklist feature proposed here into a whitelist feature. Though that would be a breaking change.

[mooori]

Another open PR that's about to use attributes with #[near_bindgen] is #934. A possible way to handle multiple attributes would be:

use darling::util::PathList;
use darling::FromMeta;

#[derive(Default, FromMeta, Clone, Debug)]
pub struct MacroConfig {
    pub events: Option<EventsConfig>,
    pub blacklist_ext_fn_attrs: Option<PathList>,
}

pub struct EventsConfig {
    standard: String,
    another_config_value: T, // darling supports many types
}

Using darling the attributes passed in the following use cases can be parsed into MacroConfig:

#[near_bindgen(events(standard = "nepXXX", another_config_value = "..."))]
pub enum MyEvents { /* ... */ }


#[near_bindgen(blacklist_ext_fn_attrs(some_plugin_attr))]
impl Contract { /* ... */ }

[austinabell]

Seems fine. A little unfortunate to add a dependency like this for something that doesn't affect the vast majority, but this seems reasonable if it enables more tooling.

None of the _Ext methods should require the attributes in general, possibly an oversight there. I'm not sure what developers would prefer in general.

I can't think of any attribute that a developer would want to keep, seems like there might be another option of just removing the attributes and if developers mention breakages we could do something like this? Just seems like a bit of an unintuitive pattern to have the #[near_bindgen(blacklist_ext_fn_attrs(attr1, attr2))] pattern. @itegulov wdyt?

Also, one other thought is that blocklist naming would be more clear, just as an alternative to consider.

[mooori]

I can't think of any attribute that a developer would want to keep, seems like there might be another option of just removing the attributes and if developers mention breakages we could do something like this?

Yes, also blacklisting is an extra step that has to be taken by contract developers that use plugin attributes. So not having to do this because attributes aren't forwarded to _Ext methods would be preferred, from that point of view.

[mooori]

Closing this since it shouldn't be required anymore now that #959 is merged.

If there'll ever be a need to allow contracts dev to whitelist some attributes to forward to _Ext methods, the blacklist feature proposed here could be turned into a whitelist feature.