Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Destroy standard #182

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions nep-X1.mediawiki
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,19 @@ Neo N3 has the native <code>ContractManagement</code> contract that is responsib
{
"name": "destroy",
"safe": false,
"parameters": [],
"parameters": not restricted,
superboyiii marked this conversation as resolved.
Show resolved Hide resolved
"returntype": "Void"
}
</pre>
This method MUST invoke the <code>destroy</code> method of <code>ContractManagement</code> contract when the contract is destroyed.
superboyiii marked this conversation as resolved.
Show resolved Hide resolved
superboyiii marked this conversation as resolved.
Show resolved Hide resolved

<code>destroy</code> method can have multi parameters for any specific purpose since it's an operation only for contract owner.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can avoid this and have a specific (parameterless) interface just like for update. Suppose I want to create a NEP-DESTROY-compatible CLI command, it'd be harder to do this if one contract has parameters and the other does not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we strictly require zero parameters in the standard, or it is just a "SHOULD" requirement?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the practical value of parameters here? I can only imagine moving tokens from contract address before deletion (which might require some address), but likely they'll be sent to owner or this can be handled in a different manner by contract. Conditional (based on parameters) destroy? Not likely to be useful, owner can do anything already, so if he decides to destroy he will do this. So I'd go with MUST here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still relevant, this NEP doesn't make much sense if parameters are not fixed.


This function SHOULD check whether the <code>signer</code> address equals the <code>owner</code> hash of contract to avoid security risks. This function SHOULD use the SYSCALL <code>Neo.Runtime.CheckWitness</code> to verify the <code>signer</code>. Details has been explained in [NEP-30](https://github.com/superboyiii/proposals/blob/upgrade-standard/nep-30.mediawiki).
<code>destroy</code> method SHOULD do checkwitness for contract <code>owner</code> to avoid security risks. This function SHOULD use the SYSCALL <code>Neo.Runtime.CheckWitness</code> to verify the <code>signer</code>. Details has been explained in [NEP-30](https://github.com/superboyiii/proposals/blob/upgrade-standard/nep-30.mediawiki).
superboyiii marked this conversation as resolved.
Show resolved Hide resolved

This function is not allowed to call another contract when executing <code>destroy</code> method of native <code>ContractManagement</code> contract since <code>RequiredCallFlags = CallFlags.States | CallFlags.AllowNotify</code>
It's strongly recommended to execute all codes before calling ContractManagement.Destroy() to ensure they can be executed correctly.
superboyiii marked this conversation as resolved.
Show resolved Hide resolved

<code>destroy</code> method is not allowed to call another contract when executing the <code>destroy</code> method of native <code>ContractManagement</code> contract since <code>RequiredCallFlags = CallFlags.States | CallFlags.AllowNotify</code>
superboyiii marked this conversation as resolved.
Show resolved Hide resolved

superboyiii marked this conversation as resolved.
Show resolved Hide resolved
===Events===

Expand Down