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

Allow Verification trigger to be called for all methods #8

Open
dicarlo2 opened this issue Jan 5, 2018 · 6 comments
Open

Allow Verification trigger to be called for all methods #8

dicarlo2 opened this issue Jan 5, 2018 · 6 comments

Comments

@dicarlo2
Copy link

dicarlo2 commented Jan 5, 2018

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:

  • Wallet clients can uniformly invoke the contract in a way that always triggers verification, regardless of the method called
  • Smart contract authors can assume that their smart contract will be invoked during verification for all methods so they can implement automatic rejection of transactions for more than just mintTokens - i.e. this makes it more generalized and not specific to just NEP-5 ICO crowdsales.
@localhuman
Copy link
Member

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
Copy link
Author

dicarlo2 commented Jan 5, 2018

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
Copy link
Member

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
Copy link
Author

dicarlo2 commented Jan 5, 2018

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
Copy link
Member

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
Copy link

prasreit commented Sep 5, 2018

bitcon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants