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

Add Logs for CallFlags.ReadOnly #2963

Closed
cschuchardt88 opened this issue Nov 13, 2023 · 23 comments
Closed

Add Logs for CallFlags.ReadOnly #2963

cschuchardt88 opened this issue Nov 13, 2023 · 23 comments
Labels
discussion Initial issue state - proposed but not yet accepted

Comments

@cschuchardt88
Copy link
Member

cschuchardt88 commented Nov 13, 2023

Summary or problem description
Currently you can not emit logs (Runtime.Log) with [Safe] (CallFlags.ReadOnly). I don't understand the reason behind it. However we should allow this to come through. One good reason is for debugging. Would hate to have to remove [Safe] from all my contract's; just to debug something and than remember to add it back after testing or debugging.

Neo Version

  • Neo 3.6.0

Where in the software does this update applies to?

  • Compiler
  • CLI
  • VM
@cschuchardt88 cschuchardt88 added the discussion Initial issue state - proposed but not yet accepted label Nov 13, 2023
@cschuchardt88 cschuchardt88 changed the title Add Logs in CallFlags.ReadOnly Add Logs for CallFlags.ReadOnly Nov 13, 2023
@Jim8y
Copy link
Contributor

Jim8y commented Nov 13, 2023

I would suggest to add debug feature for contracts, such that we can have tools enabled in the testnet but disabled for the mainnet. But need to be carefully designed to ensure behavior consistancy cross networks.

@shargon
Copy link
Member

shargon commented Nov 13, 2023

I don't understand the reason behind it.

The logs checks for storage permissions and safe does not have this permission.

@roman-khimov
Copy link
Contributor

This was discussed once upon a time in #2783.

@Jim8y
Copy link
Contributor

Jim8y commented Nov 13, 2023

This was discussed once upon a time in #2783.

This one i remember.

@shargon
Copy link
Member

shargon commented Nov 13, 2023

We can close this if is solved, now you can use assert with message in Safe methods.

@lock9
Copy link
Contributor

lock9 commented Nov 13, 2023

@shargon how can we solve this issue here:
neo-project/neo-express#309

We need to be able to view the logs. Any alternative?

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Nov 13, 2023

@lock9
Copy link
Contributor

lock9 commented Nov 14, 2023

So, we don't need this? Or the log feature won't work on safe methods? 🤔

@cschuchardt88
Copy link
Member Author

Won't work on safe methods, Unless you use

ExecutionEngine.Assert(true, "Hello World!");

Is what they are arguing. However this shouldn't be the case.

@shargon so above code emits Runtime.Log?

@shargon
Copy link
Member

shargon commented Nov 14, 2023

No, it will fault the execution, and the message will be in the fault Exception.

@cschuchardt88
Copy link
Member Author

Well can we have this than?

I would suggest to add debug feature for contracts, such that we can have tools enabled in the testnet but disabled for the mainnet. But need to be carefully designed to ensure behavior consistancy cross networks.

Its is impossible to debug [Safe] contracts. neo-express has same behavior. Which lies the problem.

@shargon
Copy link
Member

shargon commented Nov 14, 2023

I think that neo-express show the exception if it happens

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Nov 14, 2023

Yes but we still have this issue neo-project/neo-express#309, which is my reason behind doing this issue. Watch the video

@lock9
Copy link
Contributor

lock9 commented Nov 15, 2023

What alternatives do we have to return the logs @shargon ? It's only for development mode. Can we do this in a less intrusive way? like using a plugin?

@shargon
Copy link
Member

shargon commented Nov 15, 2023

No logs alternative in Safe mode...

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Nov 15, 2023

@shargon one thing you can do is create a custom ApplicationEngineProvider and do it that way. And blockchain-toolkit-library has one or is it neo-express; i forget which one.

Unless it's in the neo-vm? The log blocking code.

@shargon
Copy link
Member

shargon commented Nov 15, 2023

Why use a different one in neo-express, it will show you things that can't be replicated in the real network.

@cschuchardt88
Copy link
Member Author

Well i don't want to allow logs in neo-express because when they deploy to mainnet or testnet with log in safe methods will error.

@lock9
Copy link
Contributor

lock9 commented Nov 16, 2023

No logs alternative in Safe mode...

Why do you want to make me cry?

I worry that it can be intrusive to update the contract manifest to remove safe from methods.

Well i don't want to allow logs in neo-express because when they deploy to mainnet or testnet with log in safe methods will error.

Afaik, compilers work on debug and release mode. We could update the manifest to make it non-safe if they compile it in debug mode. I'm not sure if this is the best solution but it would work

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Nov 16, 2023

You can always create a neo-express xunit test. But it does require C#.

@lock9
Copy link
Contributor

lock9 commented Nov 17, 2023

This will need to be handled by the compilers / tools. I think we can close this issue.

@Jim8y
Copy link
Contributor

Jim8y commented Dec 29, 2023

Close this issue since we will not implement.

@Jim8y Jim8y closed this as completed Dec 29, 2023
@cschuchardt88
Copy link
Member Author

cschuchardt88 commented May 12, 2024

@Jim8y This was implemented. RPC emits LogEventArgs in ApplicationLog plugin now with release +3.7.0.

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
Projects
None yet
Development

No branches or pull requests

5 participants