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.

Sign up for GitHub

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

Jump to bottom

Add new ZK opcodes #870

Merged
merged 31 commits into from
Nov 29, 2024
Merged

Add new ZK opcodes #870

merged 31 commits into from
Nov 29, 2024

Conversation

AurelienFT
Copy link
Contributor

@AurelienFT AurelienFT commented Nov 14, 2024 •
edited
Loading

FuelLabs/fuel-specs#615

Checklist

  • Breaking changes are clearly marked as such in the PR description and changelog
  • New behavior is reflected in tests
  • If performance characteristic of an instruction change, update gas costs as well or make a follow-up PR for that
  • The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

  • I have reviewed the code myself
  • I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

[Add or remove entries as needed]

rymnc
rymnc reviewed Nov 18, 2024
View reviewed changes
fuel-asm/src/panic_reason.rs Outdated Show resolved Hide resolved
@AurelienFT AurelienFT self-assigned this Nov 18, 2024
@AurelienFT AurelienFT requested a review from bitzoic November 19, 2024 14:04
@AurelienFT AurelienFT mentioned this pull request Nov 19, 2024
Open
5 tasks
@AurelienFT AurelienFT requested a review from IGI-111 November 19, 2024 18:27
@xgreenx xgreenx requested a review from Dentosal November 27, 2024 23:02
@AurelienFT AurelienFT marked this pull request as ready for review November 28, 2024 13:16
@AurelienFT @Dentosal
Update fuel-vm/src/interpreter/crypto.rs
1741110 
Co-authored-by: Hannes Karppila <2204863+Dentosal@users.noreply.github.com>
@Dentosal
Copy link
Member

I did some polishing in #874, which could be merged into this one

Dentosal
Dentosal requested changes Nov 28, 2024
View reviewed changes
Copy link
Member

@Dentosal Dentosal left a comment

Choose a reason for hiding this comment

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

This is lacking tests for memory boundary violations

fuel-vm/src/interpreter/crypto.rs Show resolved Hide resolved
fuel-vm/src/interpreter/crypto.rs Show resolved Hide resolved
Dentosal and others added 3 commits November 28, 2024 15:03
@Dentosal @AurelienFT
cleanup for add zk opcodes PR (#874)
ff0bbf7 
* Simplify error types

* Use MemoryOverflow instead of ArithmeticOverflow

* Fix inversion letter

---------

Co-authored-by: AurelienFT <aurelien.foucault@epitech.eu>
@AurelienFT
Add some doc about the usage of operation symbols + and *
498395c
@AurelienFT
Add memory overflow tests
5f8b8e8
@AurelienFT
Copy link
Contributor Author

@Dentosal I added new tests for memory overflow and writing writes.

@AurelienFT AurelienFT requested a review from Dentosal November 28, 2024 15:03
@AurelienFT AurelienFT requested a review from rymnc November 29, 2024 16:20
rymnc
rymnc approved these changes Nov 29, 2024
View reviewed changes
Copy link
Member

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

i would prefer if we worked with more kats but it can be a follow up pr.

@AurelienFT AurelienFT added this pull request to the merge queue Nov 29, 2024
Merged via the queue into master with commit e7d5d6f Nov 29, 2024
40 checks passed
@AurelienFT AurelienFT deleted the add_zk_opcodes branch November 29, 2024 21:25
@xgreenx xgreenx mentioned this pull request Dec 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rymnc rymnc rymnc approved these changes

@Dentosal Dentosal Dentosal approved these changes

@bitzoic bitzoic Awaiting requested review from bitzoic

@IGI-111 IGI-111 Awaiting requested review from IGI-111

@xgreenx xgreenx Awaiting requested review from xgreenx xgreenx is a code owner

@MitchTurner MitchTurner Awaiting requested review from MitchTurner MitchTurner is a code owner

@Voxelot Voxelot Awaiting requested review from Voxelot Voxelot is a code owner

Assignees

@AurelienFT AurelienFT

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AurelienFT @Dentosal @rymnc