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

feat: add new fuel-asm package #987

Merged
merged 7 commits into from
May 15, 2023
Merged

feat: add new fuel-asm package #987

merged 7 commits into from
May 15, 2023

Conversation

camsjams
Copy link
Contributor

@camsjams camsjams commented May 8, 2023

Closes #332

This fuel-asm package is based off of the Fuel VM implementation, and is the initial implementation to get the ball rolling on support needed to finish a #933

In order to complete #933, we plan to duplicate the idea used in the Rust SDK that is documented in FuelLabs/fuels-rs#746 and implemented in FuelLabs/fuels-rs#848, thus requiring some of the features of fuel-asm which was not available in TypeScript.

Note:

I didn't implement all of the opcodes, here's why:

  1. there are a lot of them
  2. the issue in Improve Vector output support with Vec<> return #933 is time sensitive so I wanted to get through the basics of this support to unblock the Vector changes
  3. I will likely add some more while implementing Improve Vector output support with Vec<> return #933
  4. One can easily add them all, I think its more just time then complexity if using the fuel-asm as a guide

See also:

@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2023

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
95.34% (-0.07% 🔻)
5232/5488
🟢 Branches
84.32% (+0.03% 🔼)
855/1014
🟢 Functions
87.06% (-5.88% 🔻)
955/1097
🟢 Lines
95.33% (-0.08% 🔻)
5003/5248
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / index.ts
100% 100% 100% 100%
🟢
... / constants.ts
100% 100% 100% 100%
🟡
... / opcode.ts
74.16% 100% 13.41% 74.16%
🟢
... / instruction.ts
100% 100% 100% 100%

Test suite run success

885 tests passing in 138 suites.

Report generated by 🧪jest coverage report action from f46b291

Copy link
Member

@arboleya arboleya left a comment

Choose a reason for hiding this comment

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

Interesting PR! 🤘

I'm still going through everything but would like to understand more about the design decisions—first questions that came to mind:

  1. There are standalone functions on the instruction.ts file:
    • Should they all be spread into individual utils/<name>.ts files?
    • Should they become static methods of the Instruction class?
    • Should the current approach be kept? If yes, what's the difference between the static methods and the standalone functions?
  2. Should we break apart the tests from index.test.ts and use more descriptive file names, in accordance with their source counterpart?
  3. Should we worry about how big certain files like opcode.ts will soon become? For example, there could be 2x files per opcode (if applicable):
    • /opcodes/<name>.ts
    • /opcodes/<name>.test.ts

Great stuff! @camsjams

@arboleya arboleya self-requested a review May 11, 2023 11:59
@camsjams camsjams enabled auto-merge (squash) May 15, 2023 05:22
@camsjams camsjams merged commit a17bc14 into master May 15, 2023
@camsjams camsjams deleted the cm/issue-332 branch May 15, 2023 05:52
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.

Create @fuel-ts/asm to support script building using SDK
3 participants