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

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

Closed
lock9 opened this issue Nov 7, 2023 · 20 comments
Closed

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

lock9 opened this issue Nov 7, 2023 · 20 comments
Labels
enhancement New feature or request

Comments

@lock9
Copy link
Contributor

lock9 commented Nov 7, 2023

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.

@Ashuaidehao Ashuaidehao added the enhancement New feature or request label Nov 8, 2023
@ixje
Copy link
Contributor

ixje commented Nov 8, 2023

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
Copy link
Contributor Author

lock9 commented Nov 8, 2023

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
Copy link
Contributor

ixje commented Nov 8, 2023

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
Copy link
Contributor Author

lock9 commented Nov 8, 2023

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
Copy link
Contributor

ixje commented Nov 8, 2023

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 🤔

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
Copy link
Contributor Author

lock9 commented Nov 8, 2023

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

2023-11-08.13-15-42.mp4

@ixje
Copy link
Contributor

ixje commented Nov 8, 2023

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
Copy link
Contributor Author

lock9 commented Nov 8, 2023

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
Copy link
Contributor Author

lock9 commented Nov 8, 2023

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

@ixje
Copy link
Contributor

ixje commented Nov 8, 2023

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

@cschuchardt88
Copy link
Member

cschuchardt88 commented Nov 9, 2023

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
Copy link
Contributor

ixje commented Nov 9, 2023

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
Copy link
Member

When that needs to be fixed than. 😸

@cschuchardt88
Copy link
Member

cschuchardt88 commented Nov 12, 2023

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
Copy link
Contributor Author

lock9 commented Nov 13, 2023

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

@cschuchardt88
Copy link
Member

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 neo-project/neo-modules#841

@lock9
Copy link
Contributor Author

lock9 commented Nov 13, 2023

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
Copy link
Member

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
Copy link
Member

cschuchardt88 commented May 12, 2024

@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
Copy link
Member

Resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants