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 multiple scripts per "smart contract" - One for verification and one for invocation #919

Closed
lock9 opened this issue Jul 13, 2019 · 11 comments
Labels
Discussion Initial issue state - proposed but not yet accepted Feature Type: Large changes or new features Ledger Module - The ledger is our 'database', this is used to tag changes about how we store information VM New features that affect the Neo Virtual Machine or the Interop layer

Comments

@lock9
Copy link
Contributor

lock9 commented Jul 13, 2019

Hi,

I think that we could divide Verification and Application triggers into two different smart contract files and use the manifest file to link them.
Instead of checking the trigger type, the developer implements the verification script in a different file.

Why:

  • This solves the compiler complexity caused by Mode to disable backward jumps on Neo AppEngine 3 #837
  • Allow us to reuse verification scripts in multiple applications
  • Makes more sense for the developer (different files = different scripts)
  • Developers can share the code without sharing the verification script source code
@lock9 lock9 changed the title Allow more than one script per "smart contract" - One for verification and one for invocation Allow more multiple scripts per "smart contract" - One for verification and one for invocation Jul 13, 2019
@lock9 lock9 changed the title Allow more multiple scripts per "smart contract" - One for verification and one for invocation Allow multiple scripts per "smart contract" - One for verification and one for invocation Jul 13, 2019
@lock9 lock9 added the Discussion Initial issue state - proposed but not yet accepted label Jul 13, 2019
@igormcoelho
Copy link
Contributor

I think this is complicated, and makes things even harder for developers... I've thought about this some time before... the thing is that Triggers are different ways of calling the same script. If dividing this, we would need to solve other things: how to manage funds from an application script? how to link those two things together?
In the end, it won't help #837, because VerificationScripts are also generated in the same way as Applications, using the same C# compiler (see https://neoresearch.io/smacco). So, in my opinion, this makes things worse.

@lock9
Copy link
Contributor Author

lock9 commented Jul 13, 2019

Hi @igormcoelho,
What is this Smacco thing?

Developers don't understand triggers. Similar to what I said in #894, the trigger is the blockchain point of view, this doesn't help developers understanding the platform.
Also, for me, it doesn't make sense saying that "it is the same script", because, for the smart contract developers, we need code specifically for the verification trigger. So it may be for the blockchain point of view, but again, it is not what the 'customer' sees.

How to manage funds from an application script? How to link those things together?

If we deploy these two scripts separately, the manifest file can reference the script hash of the verification script, and not only that: I believe that in most cases, the scripthash used will be an account script hash, meaning they could use their own accounts instead of having to add this new file or implementing custom logic.

Regarding the compiler, if we adopt #837, we can't compile code that won't run, so we need to adjust it anyway. How are we going to protect the developer from committing a mistake?
Imagine if you deploy a smart contract, do your ICO, and you discover that your verification code is not working? This should be protected at "client level", using the compiler.

@igormcoelho
Copy link
Contributor

#837 is still under discussion, the current format is not good yet. Double script is a possibility, issue is that we have three triggers already, with a possibility of creating a fourth one in the future.

@lock9
Copy link
Contributor Author

lock9 commented Jul 13, 2019

@igormcoelho we could have a map for the multiple trigger types

@lock9
Copy link
Contributor Author

lock9 commented Jul 14, 2019

We could have scripts for json encoding / decoding, referenced in the application manifest.
I mentioned a similar idea in #829. If we can reference multiple scripts, and each of them have a different context (trigger), we can also reference scripts used for json encoding / decoding.
Can't wait to see json support added to neo 😅

This idea also fits well with a "micro-service" architecture, because we can reuse scripts that are already deployed in the blockchain. If people agree with the same "model", they can share the same json serializer/deserializer

@vncoelho
Copy link
Member

vncoelho commented Jul 14, 2019

@lock9, I think it makes things worse and would require Dynamic Invoke between both. I think the complexity for safety would be even more complex as Igor highlighted. Simple addresses would require a connection betweem two scripts.

@lock9
Copy link
Contributor Author

lock9 commented Jul 15, 2019

@vncoelho why dynamic invoke? If it is declared in the manifest, it is not dynamic, right? If we have this disclosed in the manifest, we can have different contexts for these different scripts. Json conversion for example, doesn't need any storage, it is completely stateless!

And regarding the complexity: It is more complex for us, or for the dApp developer? Because I'm thinking like a "smart contract developer". You shouldn't care about complexity having a team like this 😏

@lock9
Copy link
Contributor Author

lock9 commented Jul 16, 2019

@igormcoelho I think this will be needed for #772, isn't it? We are going to need to reference other scripts, won't we?

@igormcoelho
Copy link
Contributor

In fact, the idea is to simply call another contract, as a regular Call. What is really useful for this here, is exception handling. This allows simulating "virtual methods" and "polymorphism"... if such method exists on "base contract", it executes, otherwise it throws (UnknownMethodException). So we catch this "fault" and execute our own method.
Without having this, we can do using ABI. We inspect contract ABI and verify that method exists, before actually invoking it.
To be honest, this is a reasonable approach to solve the problem, I haven't thought of it before ;)

@lock9
Copy link
Contributor Author

lock9 commented Jul 20, 2019

@igormcoelho talking about #772, there are 3 scenarios to consider when using inheritance:

  • We want to run the base code only (no override)
  • We want to run the base code and then our code
  • We just want to run our code

I think that we can add these rules in the manifest file, during compilation (complicated?). What do you think?
We could also use the manifest file to mark if a contract can or cannot update its parent contract (change inheritance). This way we can cover the scenario where we want to support updates in the parent contract, while also allowing the developer to disable this.
We might want to allow to change inheritance if it is inheriting from one of the developer scripts but may want to disable it if we inherit from a native contract.

We can make neo more modular if we can add references to other scripts, to be run in "different contexts". I say this because there are some scripts, can be optimized to be run concurrently, "if they meet some criteria" (like being stateless). All of this can be done using the manifest file!

So:

@erikzhang your opinion is very important to me (for all of us I guess), could you share your ideas about this?

@lock9 lock9 added Ledger Module - The ledger is our 'database', this is used to tag changes about how we store information Feature Type: Large changes or new features VM New features that affect the Neo Virtual Machine or the Interop layer labels Aug 10, 2019
@igormcoelho
Copy link
Contributor

igormcoelho commented Aug 11, 2019

In fact, I have a very different point of view for 772 (inheritance), which is quite simpler in fact... if we support exceptions, we naturally will have inheritance (together with @lightszero compile-based entrypoint approach).
Anyway, the idea of also adding it to the manifest may perhaps help to make this more "automatic". We can discuss here how manifest can help automating this (perhaps invoking it first, then inherited method?)

Another (unanswered) question to me is: do we have non-standard entrypoint methods on Neo3? I don't know it yet.

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 Feature Type: Large changes or new features Ledger Module - The ledger is our 'database', this is used to tag changes about how we store information VM New features that affect the Neo Virtual Machine or the Interop layer
Projects
None yet
Development

No branches or pull requests

4 participants