Skip to content

dumpnef intakes manifest to find method starts#1293

Merged
shargon merged 5 commits intoneo-project:masterfrom
Hecate2:dumpnef-with-manifest
Feb 12, 2025
Merged

dumpnef intakes manifest to find method starts#1293
shargon merged 5 commits intoneo-project:masterfrom
Hecate2:dumpnef-with-manifest

Conversation

@Hecate2
Copy link
Contributor

@Hecate2 Hecate2 commented Feb 8, 2025

Useful for deployed contracts without debugInfo

Jim8y
Jim8y previously approved these changes Feb 8, 2025
/// <param name="debugInfo">The debug information. If available, provides source code comments in the dumped assembly.</param>
/// <param name="manifest">The contract manifest. Provides method start comments. Not necessary if debugInfo is available</param>
/// <returns></returns>
public static string GenerateDumpNef(NefFile nef, JToken? debugInfo, ContractManifest? manifest = null)
Copy link
Member

Choose a reason for hiding this comment

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

We should have a unit test for this method, I don't know well the final output, because maybe we need to makes different between if the method comes from manifest or from debugInfo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should have a unit test for this method, I don't know well the final output, because maybe we need to makes different between if the method comes from manifest or from debugInfo

nccs YourContract.csproj --assembly or nccs YourContract.nef --assembly generates the dumpnef text file. The content is just assembly instructions with comments. The method name from debugInfo is much longer and more accurate, because it contains the namespace and class names from the source code. For example, # Method Start Neo.Compiler.CSharp.TestContracts.Contract_WriteInTry.Write(). Method names from manifest is much more concise, like # Method Start totalSupply

Copy link
Member

@shargon shargon Feb 8, 2025

Choose a reason for hiding this comment

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

Maybe we add a new entry for the manifest?
# Public Method: xxxx

Copy link
Contributor Author

@Hecate2 Hecate2 Feb 8, 2025

Choose a reason for hiding this comment

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

Maybe we add a new entry for the manifest? # Public Method: xxxx

I am still thinking about this. Do we need to include both # Method Start and # Public Method, or only one of them?
I think my original codes are the most reliable😂😭. People dumpnef because they need an assembly document commented well by source codes, and the debugInfo has to be correct for this. If the debugInfo is wrong, a user can try again without the debugInfo.
Also there exists a compilation pattern moving the public method entry point in the manifest to somewhere else (without moving the address in the debugInfo), in order to initialize the non-static variables in the contract class. With this pattern the manifest is even less reliable than the debugInfo, because the user has more difficulty finding the actual method implementation in the dumpnef document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we add a new entry for the manifest? # Public Method: xxxx

I think it is good if we write entry address keys from manifest, if it is not written by the debugInfo.

Copy link
Member

Choose a reason for hiding this comment

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

@Hecate2 make the changes and I will approve it, but it would be nice to have the expected result somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will try to have some nice tests.

Co-authored-by: Hecate2 <2474101468@qq.com>
nan01ab
nan01ab previously approved these changes Feb 10, 2025
@Hecate2
Copy link
Contributor Author

Hecate2 commented Feb 11, 2025

Still struggling to balance the restriction and correctness of tests

@Hecate2
Copy link
Contributor Author

Hecate2 commented Feb 12, 2025

Tests ready. Difficult to include more strict tests.

@shargon shargon merged commit 3dedbac into neo-project:master Feb 12, 2025
3 checks passed
Jim8y added a commit that referenced this pull request Mar 10, 2025
* master:
  Update Copyright date (#1299)
  Add NotaryAssisted transaction attribute type (#983)
  Update submodule (#1296)
  dumpnef intakes manifest to find method starts (#1293)
  Optimize (#1294)
  CatchOnlySystemExceptionAnalyzer (#1292)
  analyze write in nested try (#1291)
Jim8y pushed a commit that referenced this pull request Aug 3, 2025
* dumpnef intakes manifest to find method starts
* Update src/Neo.Compiler.CSharp/Optimizer/DumpNef.cs
* Update src/Neo.Compiler.CSharp/Optimizer/DumpNef.cs
Co-authored-by: Hecate2 <2474101468@qq.com>
* DumpNef test
---------
Co-authored-by: Shargon <shargon@gmail.com>
Jim8y pushed a commit that referenced this pull request Aug 18, 2025
* dumpnef intakes manifest to find method starts

* Update src/Neo.Compiler.CSharp/Optimizer/DumpNef.cs

* Update src/Neo.Compiler.CSharp/Optimizer/DumpNef.cs

Co-authored-by: Hecate2 <2474101468@qq.com>

* DumpNef test

---------

Co-authored-by: Shargon <shargon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants