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 calling script hash to pass CheckWitness #1924

Closed
roman-khimov opened this issue Sep 10, 2020 · 0 comments · Fixed by #1925
Closed

Allow calling script hash to pass CheckWitness #1924

roman-khimov opened this issue Sep 10, 2020 · 0 comments · Fixed by #1925
Labels
discussion Initial issue state - proposed but not yet accepted
Milestone

Comments

@roman-khimov
Copy link
Contributor

Summary or problem description
This idea was briefly mentioned at least in #902 (comment) and #1108, but it never got enough attention for some reason.

Any contract (script) is also technically an account (that is just a script hash) and we have lots of places where accounts are being used. Other contracts provide methods that take accounts as parameters and allow to do something for account. Of course they usually only allow to do this something if there is a witness provided, so we have CheckWitness interop function that checks transaction's witnesses. This works fine for standard contracts, but for deployed contracts it's not that easy.

Some standards like NEP-5 recommend (with SHOULD) checking for calling script hash along with regular witness checks which looks like this:

        protected virtual bool Transfer(ApplicationEngine engine, UInt160 from, UInt160 to, BigInteger amount)
        {
...
            if (!from.Equals(engine.CallingScriptHash) && !engine.CheckWitnessInternal(from))
                return false;

And allows for calling contract to transfer some assets from its account to some other account just by invoking the approriate method. But there can be other methods like

        private bool Vote(ApplicationEngine engine, UInt160 account, ECPoint voteTo)
        {
            if (!engine.CheckWitnessInternal(account)) return false;

That don't do that and they're impossible to use by other contracts with simple invocation. Which creates a situation where contract's account could be used for some things provided for accounts, but not all of them. And the rules are contract- and method-dependent.

Of course after #1800 we could theoretically attach some witness using verify method to transaction, but that requires contract to have this verify method and depending on usage scenario may also be inappropriate.

This is affecting NeoFS now as we want internal chain to be managed by contracts (following regular Neo 3 governance model otherwise), so they'll have internal NEO on their accounts and they'll vote with this NEO. If of course it would be allowed for them to do so.

Do you have any solution you want to propose?
I think we can safely add calling script hash to the list of allowed hashes in the CheckWitness function. This follows widely used NEP-5 behavior for transfers and it will make contract accounts fully functional for any operations using any contracts that call CheckWitness. At the same time it seems to be consistent with the logic of CheckWitness which checks whether the account in question allows contract called to do something. Calling contract expresses this allowance by doing the call, that's the code it has.

This functionality may be restricted with some additional flag (Witnessable) similar to Payable, but I'm not sure it's worth it.

Neo Version

  • Neo 3

Where in the software does this update applies to?

  • Other: Contract execution environment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants