neo icon indicating copy to clipboard operation
neo copied to clipboard

`Assert` and `Safe` methods

Open shargon opened this issue 2 years ago • 3 comments

We have a problem with Asserts and Safe methods. Many projects have created their own version of Assert with notify, but this will also fail. None of them allows a version of Assert with message because both require the same flags CallFlags.AllowNotify.

https://github.com/neo-project/neo-devpack-dotnet/blob/c7b89cd175ca73b5636b5903587e835cfc934fb7/src/Neo.SmartContract.Framework/ExecutionEngine.cs#L29-L34

With System.Runtime.Log:

  • https://github.com/neo-project/neo/blob/26d04a642ac5a1dd1827dabf5602767e0acba25c/src/neo/SmartContract/ApplicationEngine.Runtime.cs#L115

And System.Runtime.Notify:

  • https://github.com/neo-project/neo/blob/26d04a642ac5a1dd1827dabf5602767e0acba25c/src/neo/SmartContract/ApplicationEngine.Runtime.cs#L121

The problem is that Safe methods remove this call flags https://github.com/neo-project/neo/blob/26d04a642ac5a1dd1827dabf5602767e0acba25c/src/neo/SmartContract/ApplicationEngine.cs#L209

So, How are we going to report an error from a safe method?

shargon avatar Jun 28 '22 14:06 shargon

I think that Log should be Safe to call it, because is not possible to have access by the smart contract.

shargon avatar Jun 28 '22 14:06 shargon

bump

ixje avatar Sep 27 '22 12:09 ixje

We can say that there are several related issues here:

  • ASSERT/ABORT not having message parameters
  • Runtime.Log messages are not available from getapplicationlog or invoke* RPC results
  • Runtime.Log not working in safe methods because of missing CallFlags.AllowNotify

If ASSERT/ABORT are to have (message string) parameters we won't have the two latter issues. At the same time adding new opcodes just for messaging purposes is somewhat questionable. So we have logging wrappers (that do Runtime.Log() and then ABORT/ASSERT, working around the first problem), but they have issues as well as already noticed. We have #2783 rejected, neo-project/neo-devpack-dotnet#758 not likely to happen and #2784 hanging for a while.

I think I start to agree with https://github.com/neo-project/neo-devpack-dotnet/pull/758#issuecomment-1179281997 and https://github.com/nspcc-dev/neo-go/issues/2715#issuecomment-1259429917. ASSERTMSG and ABORTMSG can solve all problems and we can even phase out regular ASSERT/ABORT over time.

roman-khimov avatar Sep 27 '22 13:09 roman-khimov