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

Turing-incomplete mode #1741

Closed
wants to merge 7 commits into from
Closed

Turing-incomplete mode #1741

wants to merge 7 commits into from

Conversation

erikzhang
Copy link
Member

No description provided.

@erikzhang erikzhang added this to the NEO 3.0 milestone Jul 1, 2020
shargon
shargon previously approved these changes Jul 1, 2020
Co-authored-by: Erik Zhang <erik@neo.org>
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Nice Erik,please wait for @igormcoelho and I to discuss.
neo-project/neo-vm#151 #837

@igormcoelho
Copy link
Contributor

Amazing step @erikzhang , certainly NEO gains a lot from this slight change.

Copy link
Contributor

@igormcoelho igormcoelho left a comment

Choose a reason for hiding this comment

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

Looks nice to me, congratulations!

Copy link
Contributor

@igormcoelho igormcoelho left a comment

Choose a reason for hiding this comment

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

In fact, I just noticed that this proposal will block if-then-else structure, wouldn't this be too limiting @erikzhang?
If only negative offsets on JMP are blocked (and all CALLs), we prevent all kinds of loops/recursions, without destroying the harmless ifs. If you worry about compilers, last time I checked the C# and Python were both generating loops in this format, which is the natural approach... anyway, if following this path, this should be made part of spec for compilers to fulfill.

Copy link
Contributor

@igormcoelho igormcoelho left a comment

Choose a reason for hiding this comment

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

please maintain if-then-else... otherwise it's like the old style push-only verification.

@erikzhang
Copy link
Member Author

In fact, I just noticed that this proposal will block if-then-else structure, wouldn't this be too limiting

Yes, it blocks if-then-else. But it will only be applied on the InvocationScript of witness. The VerificationScript of witness is still Turing-complete.

@igormcoelho
Copy link
Contributor

Yes, it blocks if-then-else. But it will only be applied on the InvocationScript of witness.

Ok, sorry for my misunderstanding. So I fully agree with you @erikzhang, InvocationScript should be highly limited. This is like previous approach of push-only, but written more concisely, I like it.

So, regarding VerificationScript, any perspecting on also leaving it Turing-incomplete? I think it's important (but allowing ifs...).

@roman-khimov
Copy link
Contributor

it will only be applied on the InvocationScript of witness.

Are there any other potential applications? Turing-incompleteness seems to be useful conceptually, but at the same I'm not sure if we still have any problems for it to solve. There is a natural limitation to these scripts which is GAS available for them, do we really care about things they do with their GAS?

Sure there is a question of unpaid failed verifications (like mentioned here: #837 (comment)), but limiting execution engine doesn't completely solve it either (as mentioned here: #789 (comment)), so maybe this question is more easily solved with (set by Policy contract) GAS limit (as was proposed here: #837 (comment)). It can also be enhanced with #1332 which just removes the VM out of the loop for most of transactions (so it can be economically motivated like using this verification type is cheaper than running a VM).

So do we really need this? Are we really solving any problem we have that couldn't be solved in another (probably simpler) way?

@igormcoelho
Copy link
Contributor

So do we really need this? Are we really solving any problem we have that couldn't be solved in another (probably simpler) way?

Yes, we need this, and this is the reason why Bitcoin doesn't have jumps and loops. Ethereum added GAS to resolve this, but they have explicit accounts, so it can be ensured that value will be paid in the end (with the exception of double spends and invalid address count nonces). Since NEO is even more flexible, I really don't see any other way around, because this computational effort hits directly the p2p nodes, which are not paid. The way it is now, there may be many dapplications that could heavily use Verification, but not in a long-term sustainable way. This proposed solution is quite simple (few lines only) and gives much more endurance to the project, while not cutting any feature. It's important to reduce InvocationScript, but I still hope to see some heavy limitations on Verification.

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

As Igor said,hope to see Verification limits as well.

@erikzhang
Copy link
Member Author

I'm stupid, this doesn't solve the problem of VerificationScript.

@erikzhang erikzhang closed this Jul 2, 2020
@erikzhang erikzhang deleted the turing-incomplete branch July 2, 2020 03:25
@erikzhang erikzhang removed this from the NEO 3.0 milestone Jul 2, 2020
@erikzhang
Copy link
Member Author

Replaced by #1745

@vncoelho
Copy link
Member

vncoelho commented Jul 2, 2020

If you are stupid what are we? Ahahahah come on,Erik...ahahah

@igormcoelho
Copy link
Contributor

igormcoelho commented Jul 2, 2020

I'm stupid

Now I'm feeling highly depressed hahaha

this doesn't solve the problem of VerificationScript

It's not true @erikzhang, it partially solves the first part of an invocation... the next step is to limit verification backjumps.

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

Successfully merging this pull request may close these issues.

5 participants