Near-One / near-plugins

Implementation of common patterns used for NEAR smart contracts.
Creative Commons Zero v1.0 Universal
27 stars 12 forks source link

Add top-level attributes to describe whole `impl` blocks #109

Closed bepro-bot closed 10 months ago

bepro-bot commented 1 year ago

Right now we use attributes such as #[pause] and #[only] that can only be used at the level function. In some implementations, different impl sections are grouped with different access requirements but similar for every function in the same impl. For those cases, it makes sense to have current attributes to work at a top level.

sgoia commented 10 months ago

could you be more specific within the code please? maybe pointing out specific files to look at for reference and where the refactory is needed, checked the code a few times and it is still not clear.

Karkunow commented 10 months ago

@mooori hey! could you please answer the question above? or maybe just mention someone who can explain more? thanks!

mooori commented 10 months ago

Currently using near-plugins functionality requires attributes on functions, for instance:

// Example 1
impl Contract {
    #[access_control_any(roles(Role::Tagger))]  // <-- here
    pub fn add_tag(tag: String, proposal_id: String) { }

    #[access_control_any(roles(Role::Tagger))] // <-- here
    pub fn remove_tag(tag: String, proposal_id: String) { }
}

As I understand it, the proposal is adding support for near-plugins attributes on impl blocks. The attributes are then forwarded to every function in that impl block. Then above would be equivalent to:

// Example 2
#[access_control_any(roles(Role::Tagger))]  // <-- here
impl Contract {
    pub fn add_tag(tag: String, proposal_id: String) { }

    pub fn remove_tag(tag: String, proposal_id: String) { }
}

Pro: Less typing for developers and fewer repetitive attributes in the code.

Cons:

I would lean towards not supporting this feature as I think cons outweight pros. It would be interesting to know how others feel about this. @birchmd do you have an opinion on this?

mooori commented 10 months ago

In addition, to support this feature we would need to implement rules to resolve collisions between attributes on impl blocks and attributes on functions. Consider the following example:

// Using `only` provided by the `Ownable` plugin.
#[only(self]
impl Contract {
    #[only(owner)]
    pub fn reset_data() { }
}

It is not obvious whether reset_data should be callable only by self, only by owner or by both of them. Developers then would need to learn the rules established to resolve such collisions.

birchmd commented 10 months ago

I agree with the concerns raised by @mooori about this feature. @sgoia @Karkunow please do not invest effort into this task for now as it will likely be marked as a "won't do"

sgoia commented 10 months ago

@birchmd thank you for the feedback, I was thinking about suggesting something similar as it does not make too much sense considering added complexity.

vzctl commented 10 months ago

Thank you for your interest, we are closing this issue as "won't do".

TouchstoneTheDev commented 9 months ago

plz close the bounty also

TouchstoneTheDev commented 7 months ago

@bepro-bot All the bounties are closed but not updated on bepro website