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

public-vm: support all types for SET (u8, ..., u128) #4267

Closed
Tracked by #3383
dbanks12 opened this issue Jan 29, 2024 · 4 comments · Fixed by #4465
Closed
Tracked by #3383

public-vm: support all types for SET (u8, ..., u128) #4267

dbanks12 opened this issue Jan 29, 2024 · 4 comments · Fixed by #4465
Assignees
Labels
C-avm Component: AVM related tickets (aka public VM)

Comments

@dbanks12
Copy link
Contributor

No description provided.

@fcarreiro
Copy link
Contributor

@dbanks12 what was the scope for this ticket? have the binary rep be u128 and cast, or decide the spec on what to do with other sizes?

@dbanks12
Copy link
Contributor Author

dbanks12 commented Feb 1, 2024

Let's do whatever's easiest for now (probably 128 binary rep + cast?). This ticket is basically "get them working with simulator and transpiler", although at the moment Noir doesn't actually tell us what type it's setting, so we might not really be able to flex this with real transpiled code at the moment.

fcarreiro added a commit that referenced this issue Feb 2, 2024
This pull request completes the implementation of the SET instruction
and adds handling for invalid tags. It also includes necessary tests for
the new functionality.

SET takes the given u128, casts it to the provided inTag type, and sets
the memory.

Ref: #4267, #4271.
@fcarreiro
Copy link
Contributor

It's done now on the TS side, assuming a 128 bit constant in the wire format. As I mentioned in other comment, we have a few options for the wire format depending on wether we want fixed-size serialization or variable-size serialization.

  • Fixed: (1) many opcodes, one for each size; (2) one opcode, constant size w/truncation (like now)
  • Variable: I think this can be done by specializing static deserialize in Set. Should not be too difficult. Is this difficult on the circuit side?

cc: @jeanmon

@jeanmon
Copy link
Contributor

jeanmon commented Feb 2, 2024

Current cpp code deserializes SET bytecode deducing the size of the constant from the inTag (fixed size based on tag)

dbanks12 pushed a commit that referenced this issue Feb 6, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Feb 6, 2024
michaelelliot pushed a commit to Swoir/noir_rs that referenced this issue Feb 28, 2024
This pull request completes the implementation of the SET instruction
and adds handling for invalid tags. It also includes necessary tests for
the new functionality.

SET takes the given u128, casts it to the provided inTag type, and sets
the memory.

Ref: AztecProtocol#4267, AztecProtocol#4271.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-avm Component: AVM related tickets (aka public VM)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants