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

Feature verification state machine #84

Conversation

localhuman
Copy link
Contributor

What is this change?

This PR changes the core IVerifiable.VerifyScripts() method which is implemented in neo/Core/Helper.cs, and makes a small change to the smart contract StateMachine so that it can optionally commit only storage api data.

What does it fix?

Currently, there is no way for a smart contract to send assets ( Neo or NeoGas, for example) to another address other than through the Runtime.Notify observer mechanism. An ideal solution for this is to allow addresses to attempt to withdraw assets from a smart contract address. The mechanism which verifies whether an address can withdraw is currently implemented in the VerifyScripts() method mentioned above.

The current issue with the above approach is that a smart contract has no access to blockchain data or storage api data while performing the verification routine, so a smart contract similar to the following would not work:

       public static Object Main(string operation, params object[] args)
        {

            if (Runtime.Trigger == TriggerType.Verification)
            {

                Transaction tx = (Transaction)ExecutionEngine.ScriptContainer;
                TransactionOutput[] outputs = tx.GetOutputs();
                ulong value = 0;
                byte[] receiver = ExecutionEngine.ExecutingScriptHash;

                foreach (TransactionOutput output in outputs)
                {
                    if (output.ScriptHash == receiver)
                    {
                        value += (ulong)output.Value;
                    }
                }

                StorageContext context = Storage.CurrentContext;

                BigInteger receivable = Storage.Get(context, receiver).AsBigInteger();

                if ( value <= receivable)
                {
                    BigInteger new_balance = receivable - value;
                    Storage.Put(context, receiver, new_balance);
                      
                    return true;
                }

                return false;
            }
     }

The purpose of this change-set is to allow for an example similar to the above to work.

Considerations

  • Performance is a consideration when adding the ability to allow verification contracts to access blockchain related data. Due to this, this proposed implementation only gives access to blockchain related date when the verification script that is being executed is not a standard verification script. The current determination of whether a verification script is standard is based on length.
  • Backwards compatibility is maintained
  • No major changes to the current implementation should be necessary to allow this.

Details

  • Allow Verification contracts read-access to blockchain related account, asset, validator, contract, and storage data.
  • Allow Verification contracts write access to storage data
  • Only 'non-standard' verification scripts are given access to blockchain read/write data.

@localhuman localhuman added the Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. label Oct 17, 2017
Copy link

@Ejhfast Ejhfast left a comment

Choose a reason for hiding this comment

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

This looks good to me at a high level, but someone familiar with the C# code should also review. Is it possible we could find a stronger condition than checking for length? In addition to length, the standard verification script should have other set properties as well, right?

Copy link
Member

@erikzhang erikzhang left a comment

Choose a reason for hiding this comment

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

The executions of the verification functions are order-independent, but if the state modification is allowed in the validation functions, the transactions will lose the ability to be validate concurrently. So we disabled all state modification APIs in verification functions.
In addition, it is difficult to ensure that all nodes perform verification functions in the same order.

@canesin
Copy link
Contributor

canesin commented Oct 18, 2017

@localhuman
Copy link
Contributor Author

I see the difficulty you have mentioned.

Are the transactions ordered by the time they are verified via this method?
https://github.com/neo-project/neo/blob/master/neo/Consensus/ConsensusService.cs#L37

Would it be possible to only perform this 'enhanced' level of verification when a node is calling tx.Verify from the method listed above ( or another place in consensus ) ?

@erikzhang
Copy link
Member

Every node will call Transaction.Verify() before relaying it.

@localhuman
Copy link
Contributor Author

What should be the next steps in implementing this feature?
neo-project/proposals#15

@notatestuser
Copy link

Read-only access to storage would be a great start. Can we get this for now?

@localhuman
Copy link
Contributor Author

It has been brought to my attention the read only Storage.Get is currently available in verification contracts. Will close this PR as we should be able to achieve what we need for now.

@localhuman localhuman closed this Oct 19, 2017
Thacryba pushed a commit to simplitech/neo that referenced this pull request Dec 12, 2019
Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
* Create index.md

* Create toc.yml

* Update toc.yml

* Update index.md

* Update toc.yml

* Create white-paper.md

* Update white-paper.md

* Update white-paper.md

* Update white-paper.md

* Update white-paper.md

* Update toc.yml

* Update toc.yml

* de-de (#9)

* Added getting-started.md for docs and sc folders, as well as introduction.md for sc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Type - Changes that may affect performance, usability or add new features to existing modules.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants