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

feature/add-tvm-opcodes-list #342

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

zerefwth
Copy link

@zerefwth zerefwth commented May 31, 2023

What is this PR?

Origin: everscale-org/bounties#32
This PR contains a workflow code pipeline for the list of all TVM opcodes.

Changes

Including:

  • A folder at src/arch/ that contains markdown generation logic, also can be rendered at the docs website
  • CSV file: contain the aggregated information from instructions.csv, tvm.tex(or initial TVM paper), and Everscale Extended Instructions, with an additional column that represented tvm_assembler.
  • GitHub workflow file that auto writes the .md after there are push changes to the CSV

Progress

The opcodes collecting job is done. Also fixed the missing description of some opcodes.

Problems:

  • Still haven't tested the gas calculation of some new opcodes.
  • Still haven't figured out the tvm_asm of some complex opcodes. We may need to discuss this more.

Question?

  • What is the layout visual of the expanded column I should handle? I've implemented a single column

@zerefwth zerefwth changed the title feature/add-collected-opcodes feature/add-tvm-opcodes-list Jun 1, 2023
ilyar added a commit to everscale-org/preview that referenced this pull request Jun 1, 2023
@ilyar ilyar marked this pull request as draft June 1, 2023 23:16
@ilyar ilyar added for Developer Improvements or additions to documentation enhancement New feature or request architecture labels Jun 1, 2023
@ilyar
Copy link
Contributor

ilyar commented Jun 1, 2023

@zerefwth
Copy link
Author

zerefwth commented Jun 2, 2023

@ilyar Hello, I've added the collapsed column.

Copy link
Contributor

@ilyar ilyar left a comment

Choose a reason for hiding this comment

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

The current result looks very unfinished, especially when compared to the current reference outcome (https://github.com/ton-blockchain/docs/tree/master/docs/smart-contracts/tvm-instructions). The expected result should be no worse than the reference and should take into account the specific features of Everscale, which involve the division of the current virtual machine opcodes into at least two main categories: core opcodes used in the main network and additional opcodes for the specialized Gosh network. Additionally, considering an alternative to Python in favor of a similar npm package should be explored.

ilyar added a commit to everscale-org/preview that referenced this pull request Jun 2, 2023
@ilyar
Copy link
Contributor

ilyar commented Jun 3, 2023

@qwadratic
Copy link
Contributor

qwadratic commented Jun 3, 2023

@gadillacer

  • the page lacks general rules of how the gas is calculated.
  • I see some empty lines in the generated table
  • I see that the rightmost column is mostly empty. and for some opcodes, it duplicates the content from the desc column. (screenshot attached)
image

To describe the general rules of gas calculation, you can get some insight from the original specification by Telegram: https://ton.org/tvm.pdf
Appendix A, section 1:

image

Also it would be nice if you double-check if the implementation keeps these rules. If not, please outline the differences as well.

@zerefwth
Copy link
Author

zerefwth commented Jun 4, 2023

@ilyar seems like I misunderstood something from the beginning.

@qwadratic

  • The data from the expanded column is just an example.

@zerefwth
Copy link
Author

zerefwth commented Jul 3, 2023

@ilyar I've updated a bit according to your requests. Check pls.

Hope you can provide me next step to do after that.

@zerefwth zerefwth requested a review from ilyar July 4, 2023 10:55
Copy link
Contributor

@ilyar ilyar left a comment

Choose a reason for hiding this comment

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

it’s hard for me to tell you exactly what needs to be changed this process is creative at the moment the result turned out worse than the existing reference, you yourself can see it on the final render, thanks for trying to do it

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

Successfully merging this pull request may close these issues.

None yet

4 participants