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

base64_decode opcode from TEALv7 #653

Merged
merged 10 commits into from
Apr 29, 2022
Merged

base64_decode opcode from TEALv7 #653

merged 10 commits into from
Apr 29, 2022

Conversation

sczembor
Copy link
Contributor

@sczembor sczembor commented Apr 8, 2022

Closes #x

Proposed Changes

  • added unit test to test the correctness of the solution
  • added base64_decode opcode functionality

TODO

  • Add unit tests
  • Implement Base64Decode class:
  • map the opcode
  • update CHANGLELOG.MD
  • Add support for tealv7

@sczembor sczembor requested review from robert-zaremba and vuvoth and removed request for robert-zaremba April 8, 2022 13:10
Copy link
Contributor

@vuvoth vuvoth left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! I have few comments.

How about compute cost on this opcode?

Check list:

  • Update opcode construction and logic.
  • Opcode unit test.
  • Add test for opcode on parser.
  • Compute opcode cost. We use it for compute budget of transaction. Logic for compute budget(gas) in method executeWithResult of class runtime/src/interpreter/Interpreter.ts.

Copy link
Contributor

@vuvoth vuvoth left a comment

Choose a reason for hiding this comment

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

Nice work! Preapproved!

@sczembor sczembor merged commit f6f002a into develop Apr 29, 2022
@sczembor sczembor deleted the TEAL_v7_decode_opcode branch April 29, 2022 06:45
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.

3 participants