Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
@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
Merged
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

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
AurelienFT added a commit to FuelLabs/fuel-specs that referenced this pull request Jan 16, 2025
@AurelienFT @rymnc
@Dentosal @rafal-ch
Add ZK opcodes definition (#619)
cca2e8c 
#615

VM PR : FuelLabs/fuel-vm#870

### Before requesting review
- [x] I have reviewed the code myself

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?

---------

Co-authored-by: Aaryamann Challani <43716372+rymnc@users.noreply.github.com>
Co-authored-by: Hannes Karppila <2204863+Dentosal@users.noreply.github.com>
Co-authored-by: Rafał Chabowski <88321181+rafal-ch@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Dentosal Dentosal Dentosal approved these changes

@rymnc rymnc rymnc 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