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: migrate to new calldata #565

Merged
merged 9 commits into from
Oct 17, 2024

Conversation

kp2pml30
Copy link
Contributor

Fixes <no issue>

What

  • implemented new calldata encoding

Why

  • first step for migration to new genvm

Testing done

  • python file is taken from genvm, untested here
  • ts parser is tested to some extent
  • some manual testing was done

Decisions made

Checks

  • I have tested this code
  • I have reviewed my own PR
  • I have created an issue for this PR
  • I have set a descriptive PR title compliant with conventional commits

Reviewing tips

For now there is a problem with "json" input by user for obvious reasons. I can allocate some time to fix them, but I am not great in reactive ui's

User facing release notes

New calldata string representation is slightly different from json:

  • it has bytes type (encoded in text as b#<hex bytes>)
  • it has address type (encoded in text as addr#<64 hex digits>
  • keys can omit "

- add parser for ts
- add encoder for ts
- bring python implementation from genvm
- patch backend for working with binary calldata
@kp2pml30 kp2pml30 mentioned this pull request Oct 15, 2024
4 tasks
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 84.08644% with 81 lines in your changes missing coverage. Please review.

Project coverage is 19.60%. Comparing base (5b647e1) to head (590d0de).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...src/components/Simulator/ConstructorParameters.vue 0.00% 27 Missing ⚠️
...nd/src/components/Simulator/ContractMethodItem.vue 0.00% 18 Missing ⚠️
frontend/src/calldata/encoder.ts 90.85% 15 Missing ⚠️
frontend/src/hooks/useContractQueries.ts 6.66% 14 Missing ⚠️
...d/src/components/Simulator/ContractReadMethods.vue 0.00% 3 Missing ⚠️
frontend/src/calldata/parser.ts 99.13% 2 Missing ⚠️
...ntend/src/components/Simulator/TransactionItem.vue 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #565      +/-   ##
==========================================
+ Coverage   15.35%   19.60%   +4.24%     
==========================================
  Files         112      116       +4     
  Lines        7950     8403     +453     
  Branches      187      274      +87     
==========================================
+ Hits         1221     1647     +426     
- Misses       6653     6680      +27     
  Partials       76       76              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[true, /^,/g, TokenKind.Comma],
[true, /^:/g, TokenKind.Colon],
[true, /^(?!\d)\w\w*/g, TokenKind.Id],
[true, /^'(?:[^']|\\.)*'/g, TokenKind.Str],

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with ''' and containing many repetitions of '&'.
[true, /^:/g, TokenKind.Colon],
[true, /^(?!\d)\w\w*/g, TokenKind.Id],
[true, /^'(?:[^']|\\.)*'/g, TokenKind.Str],
[true, /^"(?:[^"]|\\.)*"/g, TokenKind.Str],

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with '"' and containing many repetitions of '!'.
@@ -42,6 +43,24 @@ def _parse_transaction_data(transaction_data: Transactions) -> dict:
],
}

@staticmethod
def _transaction_data_to_str(data: dict) -> str:
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AgustinRamiroDiaz comment added

@kp2pml30
Copy link
Contributor Author

Actually I found out that probably what we mean by address is 20 bytes and not 32 as in this PR. It is sort of confusing:

  • account address is 20 bytes
  • word size (in eth) is 32 bytes, slot addresses in eth are also 32 bytes

If I fix that right now, will it break js sdk? @cristiam86

@cristiam86
Copy link
Collaborator

Actually I found out that probably what we mean by address is 20 bytes and not 32 as in this PR. It is sort of confusing:

  • account address is 20 bytes
  • word size (in eth) is 32 bytes, slot addresses in eth are also 32 bytes

If I fix that right now, will it break js sdk? @cristiam86

The JS SDK does exactly the same as the simulator front-end so I will adapt it

@cristiam86 cristiam86 merged commit eeb012d into yeagerai:main Oct 17, 2024
8 of 10 checks passed
Copy link
Contributor

🎉 This PR is included in version 0.18.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants