nash-io / neo-ico-template

An ICO Template for NEO projects
GNU General Public License v3.0
117 stars 56 forks source link

Allow Verification trigger to be called for all methods #8

Open dicarlo2 opened 6 years ago

dicarlo2 commented 6 years ago

I haven't tested this, but as far as I understand it seems like wallet clients should only construct a transaction that triggers the smart contract during verification when mintTokens is called. Otherwise, it will run verification for mintTokens even though another method is actually the one being called.

Instead, I propose that we change the verification trigger to also check the method invoked and only run the can_exchange verification if that method is mintTokens. This way, clients can always trigger verification which has a few benefits:

localhuman commented 6 years ago

The args passed to a Verificationcontract normally don't include the actual args passed to an Application contract execution (its normally the signature), so there's no reliable way to determine what method is actually being called.

dicarlo2 commented 6 years ago

Wallet clients can be configured to pass those args. Just create a witness where the invocation script pushes the args on the stack (as you would for invoking the smart contract) and the verification script is empty (i.e. a length 0 hex string equivalent). Then NEO will use the associated script hash as the app call (which you can construct such that it is the script hash of the contract): https://github.com/neo-project/neo/blob/f7c4d86b458b0ae11d69214227cee33938a8f7ca/neo/Core/Helper.cs#L65.

localhuman commented 6 years ago

I understand that. What I'm saying is that those args can't be trusted, as you can pass mintTokens in the Verification portion, but that doesn't mean it will match up with what is being passed in to the Application portion.

My preferred approach, which is less overhead on the network, is to only attach the script to be invoked during verification if you plan on doing a mintTokens call. Otherwise, the script shouldn't be attached or run at all for example during a transfer call.

dicarlo2 commented 6 years ago

Ah, good point. But in that case, wouldn't that be an error (bug) by the wallet client? Even if we consider a malicious actor, the same verification should be performed during the Application portion so it would just result in undesirable behavior for the invoker - that is, their transaction would go through but fail during invocation, locking up any assets they transferred in the process.

I can understand the argument that we should attempt to reduce the load on the network - in that case, an alternative approach here would be to add a verify boolean to the ABI NEP so that wallet clients can generically know when to construct the transaction such that Verification portion is run.

To be clear, in the end my goal is that wallet clients shouldn't have to know or special case certain methods for certain smart contracts.

localhuman commented 6 years ago

That makes sense.. it kind of bothers me that mintTokens has become part of a standard and yet its not standardized anywhere 😅. I can definitely understand wanting to have a standardized interface for invocation, especially when relating to tokens. I'll take a look at the ABI NEP

prasreit commented 6 years ago

bitcon