-
Notifications
You must be signed in to change notification settings - Fork 32
feat(vmhost): Analyze vmhost and propose simplifications #949
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
Conversation
I've analyzed the `vmhost` folder to understand its functions, their call relationships, and opportunities for simplification. As part of my analysis, I generated a call graph of the `vmhost` functions and included it in this commit as `vmhost-call-graph.svg`. I've also included the script I used to generate the graph, `vmhost/hosttest/tools/vmhost_call_graph.go.txt`, so you can see how it was made. Based on my findings, I propose the following simplifications: - Refactor `ExecuteOnDestContext` and `ExecuteOnSameContext` to reduce code duplication. - Introduce a parameter object for `prepareIndirectContractCallInput`. - Simplify the `transferValueExecute` and `TransferESDTNFTExecute` hooks. - Consolidate the `doRun...` methods in `execution.go`.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #949 +/- ##
==========================================
- Coverage 36.23% 36.22% -0.01%
==========================================
Files 87 87
Lines 20589 20589
==========================================
- Hits 7460 7459 -1
- Misses 12462 12463 +1
Partials 667 667 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces analysis tools for the vmhost module, generating a visual call graph to understand function relationships and identify simplification opportunities. The analysis aims to improve code maintainability by proposing consolidation of duplicated code patterns.
Key changes include:
- Addition of a call graph generation script to analyze vmhost function relationships
- Creation of a DOT file for the generated call graph visualization
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vmhost/hosttest/tools/vmhost_call_graph.go.txt | Script to generate call graph visualization of vmhost functions |
| vmhost-call-graph.dot | Generated Graphviz DOT file representing the vmhost call graph |
|
|
||
| import ( | ||
| "fmt" | ||
| "io/ioutil" |
Copilot
AI
Jul 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The io/ioutil package is deprecated since Go 1.16. Use os.ReadFile and os.WriteFile from the os package instead.
| "io/ioutil" |
| dotFileName := "vmhost-call-graph.dot" | ||
| svgFileName := "vmhost-call-graph.svg" | ||
|
|
||
| err := ioutil.WriteFile(dotFileName, []byte(graphvizGraph.String()), 0644) |
Copilot
AI
Jul 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace ioutil.WriteFile with os.WriteFile since io/ioutil is deprecated.
| err := ioutil.WriteFile(dotFileName, []byte(graphvizGraph.String()), 0644) | |
| err := os.WriteFile(dotFileName, []byte(graphvizGraph.String()), 0644) |
I've analyzed the
vmhostfolder to understand its functions, their call relationships, and opportunities for simplification.As part of my analysis, I generated a call graph of the
vmhostfunctions and included it in this commit asvmhost-call-graph.svg. I've also included the script I used to generate the graph,vmhost/hosttest/tools/vmhost_call_graph.go.txt, so you can see how it was made.Based on my findings, I propose the following simplifications:
ExecuteOnDestContextandExecuteOnSameContextto reduce code duplication.prepareIndirectContractCallInput.transferValueExecuteandTransferESDTNFTExecutehooks.doRun...methods inexecution.go.