Neo: NEO2: allow CheckWitness to work when using invokescript

[gsmachado]

Summary

Users get very annoyed that invokescript does not work as expected when "CheckWitness" is used. This prevents, for example, SDKs or application to estimate GAS of a certain call to a contract -- without implementing the whole NEO VM.
It's required to make that work for NEO2, and not only for NEO3.

Do you have any solution you want to propose?

The solution is to merge the following pull request, which, by the way, is already merged to the master branch: https://github.com/neo-project/neo/pull/335
It's required to merge to the NEO2 branch and, thereafter, make a NEO2 release.

Details

This problem has a high negative usability impact on NEO2.

[igormcoelho]

Is it tested?
I agree this can be merged,but I strongly disagree this may affect customer satisfaction... RPC is not part of Neo protocol,its a service,in this case, provided by C#. People can easily implement this rpc feature in any other language or sdk.... Please,lets be independent of non protocol/core features of C# implementation.

[shargon]

Agree with do this

[igormcoelho]

@gsmachado a true solution to this issues will involve state computation on light wallets. But I agree with a workaround

[gsmachado]

@igormcoelho well, the thing is: maybe the "invokescript" can be implemented on SDKs, etc., sure... I agree. I also agree that a "real solution" should be on light wallets (which, you know, I'm very interested to implement). And, I also agree that we must make a clear distinction between JSON-RPC and core functionalities. We are in sync. 👍

However, you should agree with me that if NEO (or, C# implementation) provides the "invokescript", it should just work. For all the cases. Whatever script it takes. No matter what. Otherwise, if it just works for some cases, depending on the input, or with bugs, that's disguising for users... and it demonstrates that the functionality is not well-polished -- which ultimately impacts on users/customers, application, whatever.

That's why I asked to merge that to NEO 2 and make a release of it. I think that's reasonable. 😄

[igormcoelho]

I agree we are in full sync :+1: that's exactly the point,this feature will never work correctly. Ok, we can fix for CheckSig, but what about all other syscalls... I just want to express to Neo users that this is not an easy feature, and "not done by lazyness". To do this correctly we would need to adopt all the things we talked on Shanghai,including important processing/verification capabilities for light wallets. We've discussed this in many other issues already, the real solutions (but they are hard).

So,I'm not (at all) against passing this fix, but this won't completely fix the real problem (and users should be aware of that, specially neo devs here). We need to do a bigger step in that.

[gsmachado]

hmmm...
@igormcoelho then, if not all syscalls work with that JSON-RPC method (invoke*), I'm in favor to remove this feature completely. At least, then, it's not confusing for the users and we don't have to maintain that anymore. What do you think about that?

[igormcoelho]

I really think it's not RPC to define execution prices, but user wallet,or dapp wallet... let's see what other members think. (in the meanwhile, I suggest we merge that code, few lines only, but keep discussion...)

[lock9]

@gsmachado sorry for the long wait. I'm trying to test this for the last 2 days, but it seems that setting up a new computer with all neo code bases is not that easy :(
It should be tested by tomorrow.