Add debug mode to the applicationlog to get log info #842
Conversation
|
@cschuchardt88 just finished coding, not tested yet, but you can take a look. Will finish it tomorrow. |
|
Still need to update RpcClient |
|
Why only during debug? It seems reasonable to get this information always |
This changes the response message size and processing time, without a solid test, i dare not add it directly in fear of DOS attacks. So, debug mode first. But we can also go straightforward to add it for production env if you think it is OK. |
|
In this case, test the potential DOS first would be good. And leave as not default for now. |
|
@cschuchardt88 this will not be merged until we have a monorepo. |
| { | ||
| if (!Settings.Default.Debug) return; | ||
|
|
||
| var tx = ((Transaction)args.ScriptContainer).Hash; |
There was a problem hiding this comment.
It can be not only a transaction, right? E.g. block for OnPersist and PostPersist execution results.
There was a problem hiding this comment.
It can be not only a transaction, right? E.g. block for OnPersist and PostPersist execution results.
I think it can only be transaction, but i might be wrong. May you please confirm @shargon @roman-khimov
| ["contract"] = args.ScriptHash.ToString(), | ||
| ["message"] = args.Message | ||
| }; | ||
| logList?.Add(logJson); |
There was a problem hiding this comment.
Why do you need ? here? logList is always non-null.
There was a problem hiding this comment.
Why do you need
?here?logListis always non-null.
It is possible since in the else block, the assignment might be null.
| @@ -18,13 +18,16 @@ internal class Settings | |||
| public uint Network { get; } | |||
| public int MaxStackSize { get; } | |||
|
|
|||
| public bool Debug { get; } | |||
There was a problem hiding this comment.
Are we planning to enable this setting in public networks?
There was a problem hiding this comment.
We could, after solid test and monorepo. but now this is only to answer the need of @cschuchardt88 on neo-express.
| if (Settings.Default.Debug) | ||
| ApplicationEngine.Log += ApplicationEngine_Log; |
There was a problem hiding this comment.
It's a nice feature. How about adding the same ability to add logs to execution result for invokescript, invokefunction and etc. in debug mode? This change is more invasive and requires changing the core library as far, but it might be useful for someone.
There was a problem hiding this comment.
Closes #841
This pr focus on adding a debug mode to the applicationlog plugin such that developers can get the transaction execution log via the rpc interface.
What has changed?
Will this impact other tools?
No