neo-project / proposals

NEO Enhancement Proposals
Creative Commons Attribution 4.0 International
136 stars 113 forks source link

NEP 22: Contract Update Standard #154

Open superboyiii opened 2 years ago

superboyiii commented 2 years ago

Close: https://github.com/neo-project/proposals/issues/135

erikzhang commented 2 years ago

Add _initialize?

roman-khimov commented 2 years ago

_initialize is something different to me, not directly related to deploy/update. Maybe it's in some other document (but I wasn't able to find it on https://docs.neo.org/) or other NEP.

superboyiii commented 2 years ago

@roman-khimov @erikzhang Added. Please review again.

roman-khimov commented 2 years ago

NEP-22:

provide systems with a generalized interaction mechanism for smart contract initial deploy, update and destroy.

I don't see _initialize being a part of this NEP.

erikzhang commented 2 years ago

I don't see _initialize being a part of this NEP.

I'm not sure if it should be part of this PR, but obviously it should be included in a document.

Also, don't forget the verify method.

superboyiii commented 2 years ago

I don't see _initialize being a part of this NEP.

I'm not sure if it should be part of this PR, but obviously it should be included in a document.

Also, don't forget the verify method.

Added.

superboyiii commented 2 years ago

@erikzhang @roman-khimov Is it OK?

roman-khimov commented 1 year ago

It's a method soup now to me. I'd rather have separate NEPs for _initialize, _deploy and verify (three NEPs, seems to be perfectly fine to me, they are different unrelated things) and one for update/destroy. I want to be able to specify "NEP-XYZ" in SupportedStandards and expect all the tooling around to understand that this contract has standardized update/destroy method pair that can be interacted with. "NEP-22" doesn't give this meaning right now.

ixje commented 1 year ago

_initialize is something different to me, not directly related to deploy/update. Maybe it's in some other document (but I wasn't able to find it on https://docs.neo.org/) or other NEP.

I can kind of see the relationship. update can call _deploy which in turn can call _initialize again. Ideally there would be a flow diagram included to make it clear. verify on the other hand seems totally unrelated and worthy of a separate document if we're splitting anyway

roman-khimov commented 1 year ago

I can kind of see the relationship

Is it related? Probably so. Can be it be used without all the other things? Sure it is. Can all the other things in this proposal be used without _initialize? Sure they are. We can have as many standards as we need and they can be related, but different standards for different things. NEP-XYZ in SupportedStandards should have some specific meaning. And SupportedStandards can have as many NEP-X, NEP-Y, NEP-Z as needed.

EdgeDLT commented 1 year ago

And SupportedStandards can have as many NEP-X, NEP-Y, NEP-Z as needed.

I agree in principle with the idea of having independent standards. But I suspect there are many others like myself who also have a mental block at the future thought of these long lists of numbers with no meaning. Maybe we could package them together under a standard group, but at that point, is it that much different from what we have now?

ixje commented 1 year ago

NEP-XYZ in SupportedStandards should have some specific meaning. And SupportedStandards can have as many NEP-X, NEP-Y, NEP-Z as needed.

I'm not against finer grained NEPs. Perhaps a See Also section pointing to strongly related NEPs can help here.

Ultimately the SupportedStandards first need to be checked prior to deploy or it's pretty much useless. There's quite a few contracts out there "claiming" (probably more like forgetting to change) a certain standard support and not even exposing the right methods, let alone returning the correct data.

roman-khimov commented 8 months ago

My take on it is still the same, three NEPs here would be much better than a single one:

It's a method soup now to me. I'd rather have separate NEPs for _initialize, _deploy and verify (three NEPs, seems to be perfectly fine to me, they are different unrelated things) and one for update/destroy. I want to be able to specify "NEP-XYZ" in SupportedStandards and expect all the tooling around to understand that this contract has standardized update/destroy method pair that can be interacted with. "NEP-22" doesn't give this meaning right now.

This one was supposed to be about update/destroy initially.

superboyiii commented 5 months ago

@shargon done

Jim8y commented 5 months ago

#154 (comment)

I can't imagine how to use this NEP in its current form. It needs a split.

@roman-khimov this one can be the most general description of base methods, as for specific method that should have a seperate NEP, we can create another NEPs for them, like Verify, no need to complete them in one NEP.

superboyiii commented 5 months ago

#154 (comment)

I can't imagine how to use this NEP in its current form. It needs a split.

It's a guideline proposal for someone doesn't know where to start. It doesn't mean that you must implement all of these. In fact, you can even don't apply any of these, but it can remind these people some important points when starting to write neo smart contract.

roman-khimov commented 5 months ago

Let's get back to #135 that started all of this for a moment. What people really wanted there is a convention for methods that contract authors can use to have some common way to update/destroy contracts. In this sense it's somewhat similar to NEP-11/17/24. And this is only about two methods: update and destroy, so that I wouldn't want to do

func obnovlyashka(data Array)

to update my contract (because 3rd-party tools couldn't easily create an update transaction for me in this case, unlike when they deal with a compliant update).

But what we also have here now are:

At least we need to move verify out. It'd be perfect to have a NEP for it, but it's a separate matter. I would also move _deploy out since it has a broader definition (not just updates, but deployments as well, works irrespective of how your exposed-from-contract update methods are called), then this NEP could just refer to it.

igormcoelho commented 4 months ago

According to recent discussions, I agree that update and destroy are the most important here to be standardized.

AnnaShaleva commented 4 months ago

Totally agree with Roman, we should adjust this NEP to standardize a single update method. The rest of methods standards should be moved to a separate granular NEPs.

cschuchardt88 commented 4 months ago

Should update method have a data parameter? Same for destroy. This has been requested from others in the pass to have one added to neo-cli, including neo-express.

superboyiii commented 2 months ago

@roman-khimov @shargon @cschuchardt88 @Jim8y Now I changed it to Contract Update Standard. I will submit destroy standard as well.

superboyiii commented 2 months ago

@shargon @roman-khimov Done.