Skip to content

[**Fixed**] ApplicationLogs Console Printing#3348

Merged
NGDAdmin merged 3 commits intoneo-project:masterfrom
cschuchardt88:fix/applicationlogs-01
Jun 21, 2024
Merged

[**Fixed**] ApplicationLogs Console Printing#3348
NGDAdmin merged 3 commits intoneo-project:masterfrom
cschuchardt88:fix/applicationlogs-01

Conversation

@cschuchardt88
Copy link
Member

@cschuchardt88 cschuchardt88 commented Jun 18, 2024 •
edited
Loading

Description

This PR fixes an issue due to ApplicationLogs and blockchain's lack of knowledge about previous contract states; since they are not stored.

ApplicationLogs when printing information to console for users to lookups; this method uses current manifest (from core/NativeContract) for a contract to map event parameter names to the event by it's parameter index. So if a contract were to update and/or remove this event or add an ambiguous event or change parameter count with the same event name. The information will not be displayed (will crash or be null).

Now instead it will print the event's parameter index, if the above would happen.

No Resync Required!

Type of change

  • Optimization (the change is only an optimization)
  • Style (the change is only a code style for better maintenance or standard purpose)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Jim8y
Jim8y reviewed Jun 19, 2024
View reviewed changes
Copy link
Contributor

@Jim8y Jim8y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if they updated but still have the same param length?

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Jun 19, 2024 •
edited
Loading

Then parameter name will be wrong. There is no tracking for this in core or ApplicationLogs plugin. We don't care about the parameter names, it just a quality of life feature in neo-cli. If you have a solution I would love to know.

roman-khimov
roman-khimov approved these changes Jun 19, 2024
View reviewed changes
Copy link
Contributor

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, follows the already existing "missing contract" logic.

@superboyiii
Copy link
Member

superboyiii commented Jun 21, 2024 •
edited
Loading

@cschuchardt88 Somewhere still hasn't get the latest manifest arguments.
In this example, events still get the initial manifest arguments from contract, so you can see the args for transfer of FLM are arg1,2,3

neo> log tx 0xfc84a6153decbb0171142dfab535e9f53d5ad820c4480d83a5307fba239f337f
Trigger: Application
VM State: HALT
Exception: null
Gas Consumed: 0.14578843
Stack: 
  0: {"type":"Boolean","value":true}
Notifications:

  ScriptHash: 0x48c40d4666f93408be1bef038b6722404d9a4c2a
  Event Name: Transfer
  State Parameters:
    from: {"type":"ByteString","value":"eIgtv7rryQI17Ldq48mV4vwgoLg="}
    to: {"type":"ByteString","value":"7MAgTg6XM6llts9y33c3yLCFWk0="}
    amount: {"type":"Integer","value":"8392000000"}

  ScriptHash: 0xf0151f528127558851b39c2cd8aa47da7418ab28
  Event Name: Transfer
  State Parameters:
    arg1: {"type":"ByteString","value":"7MAgTg6XM6llts9y33c3yLCFWk0="}
    arg2: {"type":"ByteString","value":"eIgtv7rryQI17Ldq48mV4vwgoLg="}
    arg3: {"type":"Integer","value":"1398299720061"}

  ScriptHash: 0x48c40d4666f93408be1bef038b6722404d9a4c2a
  Event Name: Transfer
  State Parameters:
    from: {"type":"ByteString","value":"7MAgTg6XM6llts9y33c3yLCFWk0="}
    to: {"type":"ByteString","value":"oyJrAPWr9bXxnMN64ykd4lk6YKk="}
    amount: {"type":"Integer","value":"4196000"}

  ScriptHash: 0x4d5a85b0c83777df72cfb665a933970e4e20c0ec
  Event Name: Synced
  State Parameters:
    balance0: {"type":"Integer","value":"35128294503880"}
    balance1: {"type":"Integer","value":"5869390309517200"}

  ScriptHash: 0x4d5a85b0c83777df72cfb665a933970e4e20c0ec
  Event Name: Swapped
  State Parameters:
    caller: {"type":"ByteString","value":"I0/XJYzDXXchuDI3tmXX7Mz0cPk="}
    amount0In: {"type":"Integer","value":"8392000000"}
    amount1In: {"type":"Integer","value":"0"}
    amount0Out: {"type":"Integer","value":"0"}
    amount1Out: {"type":"Integer","value":"1398299720061"}
    to: {"type":"ByteString","value":"eIgtv7rryQI17Ldq48mV4vwgoLg="}

In fact, they're:
fbc7ed2c7b0740048aa6db49d0a80a9

@Jim8y
Copy link
Contributor

Jim8y commented Jun 21, 2024

Maybe merge this after the UT, then add a few UT tests for this pr.

@dusmart
Copy link

dusmart commented Jun 21, 2024

@superboyiii seems that it's FLM's problem that they specified their arguments' name like that: https://explorer.onegate.space/contractinfo/0xf0151f528127558851b39c2cd8aa47da7418ab28

@superboyiii
Copy link
Member

superboyiii commented Jun 21, 2024 •
edited
Loading

@superboyiii seems that it's FLM's problem that they specified their arguments' name like that: https://explorer.onegate.space/contractinfo/0xf0151f528127558851b39c2cd8aa47da7418ab28

Oh, I see. That's only event arguments. Yes, their name is non-standard.

@NGDAdmin NGDAdmin merged commit f379dab into neo-project:master Jun 21, 2024
@cschuchardt88
Copy link
Member Author

cschuchardt88 commented Jun 21, 2024 •
edited
Loading

@superboyiii You need to be looking at their events in manifest not the method description.

From neotube.io they match
image

@shargon shargon deleted the fix/applicationlogs-01 branch June 21, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shargon shargon shargon approved these changes

@Jim8y Jim8y Jim8y approved these changes

@roman-khimov roman-khimov roman-khimov approved these changes

+1 more reviewer

@superboyiii superboyiii superboyiii approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Ready to Merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@cschuchardt88 @superboyiii @Jim8y @dusmart @shargon @roman-khimov @NGDAdmin