neo-project / neo-express

Neo Private Net optimized for development scenarios
MIT License
35 stars 37 forks source link

Return logs when using the 'show tx' command #309

Closed lock9 closed 4 months ago

lock9 commented 11 months ago

Problem: Neo Express returns a JSON transaction when you use the show command. However, it doesn't return the 'logs'. The problem is that developers often use the 'print' (log) method to help them debug their code. Notifications are being included in the response but not the logs.

Solution: Include the logs in the response.

ixje commented 11 months ago

ApplicationLogs are based on the ApplicationExecuted class, which don't include logs and are not persisted (and thus cannot be retrieved). They are shown in the console when executing which is where they should be able to read them.

I don't think logs should be persisted, that's not the purpose of a log as defined in the core project. We have notifications for that (which are persisted). For debugging they should really look at the neo-express console or better, use neo-debugger.

lock9 commented 11 months ago

Hi @ixje , I understand that 'technically speaking it doesn't make much sense'. However, users already do this. Instead of using a log, they use 'notify' instead. Can a new plugin be added to enable this feature? Most people I saw prefer prints over debugging. Tyler included

ixje commented 11 months ago

Hi @ixje , I understand that 'technically speaking it doesn't make much sense'. However, users already do this. Instead of using a log, they use 'notify' instead. Can a new plugin be added to enable this feature? Most people I saw prefer prints over debugging. Tyler included

I understand that a lot of people for better or worse use print statements for debugging. Why is looking at the neo-express console for the log statements insufficient?

lock9 commented 11 months ago

It's because we are not looking at the neo-express console. We are examining the transaction.

One of the biggest problems in the developer experience is that the state needs to persist across multiple calls. We built a new VS Code extension that solves that. However, one thing we still need is the print statements. We are using 'notify' for now.

The main issue is that the behavior differs when debugging vs running the transaction. This makes the development experience confusing. You just debugged and saw the print. Now you invoke the method, and it doesn't show the print? Prints are often used to see if some 'path' is being followed (like an if). The person would debug it and see that the print appeared. Then, they run a transaction... and the print doesn't appear. This makes them think that the contract needed to be updated or for some other reason, does not represent the current state of the code.

ixje commented 11 months ago

One of the biggest problems in the developer experience is that the state needs to persist across multiple calls.

Can you elaborate what this means? I thought you were debugging code :thinking:

The main issue is that the behavior differs when debugging vs running the transaction. This makes the development experience confusing. You just debugged and saw the print. Now you invoke the method, and it doesn't show the print?

It sounds like it does but they're looking in the wrong place where it prints

lock9 commented 11 months ago

I'm not debugging. I'm just running the code. Please check the video.

https://github.com/neo-project/neo-express/assets/38396062/4360bbc4-22b9-4ff1-a64f-3d0c288b7131

ixje commented 11 months ago

I don't know what this plugin is or which old plugin it replaces. Does it replace neo-visual-tracker? Where are argument specified if a function takes arguments?

lock9 commented 11 months ago

I don't know what this plugin is or which old plugin it replaces. Does it replace neo-visual-tracker?

It's not a plugin; it's a new VS Code extension. The plugin I refer to is one to enable the 'log' calls to be stored in a transaction as 'debug' logs. The Visual Tracker isn't maintained anymore. I've heard that NGD will maintain the debugger and neo-express.

This extension is not meant to be a visual tracker but to facilitate user onboarding.

Where are argument specified if a function takes arguments?

It shows a window: image

In this example, I'm calling the balanceOf method. Callbacks like _deploy are called too. This allows the user to build a smart contract without leaving the code editor. It's not perfect. That is why it wasn't released yet. I'm sure you are going to like it :)

lock9 commented 11 months ago

Here is the Ghost Market NEP-11 example using a tweak to enable prints: image

ixje commented 11 months ago

It looks interesting :eyes: I guess neo-express could include logs with some effort if that makes sense to many people.

cschuchardt88 commented 11 months ago

I'm not debugging. I'm just running the code. Please check the video.

2023-11-08.13-15-42.mp4

That isn't the right way to do it. That print is output for the console. The compiler for python should be erroring on that. In C# you can't do Console.Writeline which would be the same thing. There is no console output on the blockchain.

You should be using Runtime.Log that is how you print or Console.Writeline

ixje commented 11 months ago

You should be using Runtime.Log that is how you print or Console.Writeline

A print statement in a Python smart contract is translated to a System.Runtime.Log syscall in the VM.

cschuchardt88 commented 11 months ago

When that needs to be fixed than. 😸

cschuchardt88 commented 11 months ago

This is what i have so far let me know if it's suitable for you. This is going to require to update RPC Server plugin. I dont think that will happen, Because this is an interface change. But we will see.

{
  "transaction": {
    "hash": "0xb9203e4f28c5e07eef0bc5e225a2603cc7f4ba0bba8ba04f048750ac7f8ba590",
    "size": 205,
    "version": 0,
    "nonce": 640966548,
    "sender": "NbGJ2PMcd7DeVYCnAf3neEuEqgLdaGX6rP",
    "sysfee": "2645280",
    "netfee": "1188520",
    "validuntilblock": 5876,
    "signers": [
      {
        "account": "0xf114fa11a19cb02299010fe5a5a0518179ee61a8",
        "scopes": "CalledByEntry"
      }
    ],
    "attributes": [],
    "script": "DAVDaHJpcxHAHwwIc2F5SGVsbG8MFAIXhEkqUKHxhCwG7sF2B8byqmlBQWJ9W1I=",
    "witnesses": [
      {
        "invocation": "DEDejoY2ixnHQmejF6Yc6NsnIrHOHRMGk4l5mDvO/FYKhSMcYR/KeU51nTncemMJEPAm98UI9cVptQldeVvqikew",
        "verification": "DCECJJjt3o9js9i1pEOyfB7alrHtZ+v7Pu1Xx736tD2VS3dBVuezJw=="
      }
    ]
  },
  "application-log": {
    "txid": "0xb9203e4f28c5e07eef0bc5e225a2603cc7f4ba0bba8ba04f048750ac7f8ba590",
    "executions": [
      {
        "trigger": "Application",
        "vmstate": "HALT",
        "exception": null,
        "gasconsumed": "2645280",
        "stack": [
          {
            "type": "ByteString",
            "value": "SGVsbG8sIENocmlz"
          }
        ],
        "notifications": [],
        "logs": [
          {
            "contract": "0x4169aaf2c60776c1ee062c84f1a1502a49841702",
            "message": "Hello, Chris"
          }
        ]
      }
    ]
  }
}
lock9 commented 11 months ago

Thanks a lot @cschuchardt88 , this will solve the issue!

cschuchardt88 commented 11 months ago

Looks like they are going to allow the change. But after monorepo is built. Now neo-express does use it own custom ApplicationLog plugin, but i thought it would be better to add to mainnet. Reason being is that RPCClient needs to be updated for neo-express to parse json.

See https://github.com/neo-project/neo-modules/issues/841

lock9 commented 11 months ago

Will it work if we compile it locally? That will do for us, at least for now. We are using the standalone version of Neo Express

cschuchardt88 commented 11 months ago

If you are talking about neo-express you can use https://github.com/cschuchardt88/neo-express/tree/issue-309 which only works with offline mode.

cschuchardt88 commented 5 months ago

@lock9 neo v3.7.0 has this added in now for ApplicationLog plugin. So we have to update neo-express to use the new 3.7.2 version of the plugin. Must put in DebugMode in order to see it.

cschuchardt88 commented 4 months ago

Resolved