Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Migrate multi-packed keccak witness assignment to Value<F> API #942

Closed
CPerezz opened this issue Nov 28, 2022 · 2 comments
Closed

Migrate multi-packed keccak witness assignment to Value<F> API #942

CPerezz opened this issue Nov 28, 2022 · 2 comments
Labels
crate-keccak Issues related to the keccak workspace member T-refactor Type: cleanup/refactor T-tech-debt Type: tech-debt generated or cleaned up wontfix This will not be worked on

Comments

@CPerezz
Copy link
Member

CPerezz commented Nov 28, 2022

I made an effort to migrate Part<F> and PartValue<F> and the witness assignment/generation in general to use the Value<F> struct from halo2 API in #925.
During the process, I found several problems on the path that discouraged me from continuing with the task although I think is doable without changes to the Halo2 Value API.

  1. It feels really tricky to extract the byte representation from a Value. It feels like if you're cheating to the API constantly (using .map() to gain access to the underlying F so that you can do extra things with it such as decompose it into bytes actually).
  2. Since the struct doesn't implement PartialEq it's challenging to perform some sanity checks.
  3. IIRC, XOR/AND are not implemented for Value. And that also complicates certain things as you need again to nest multiple map()s to perform the operations with the underlying F's.

For these reasons, right now extract_field function was added. And the goal is to get ridd of it fixing the witness assignment/generation to be done with the Value<F> API.

@CPerezz CPerezz added T-refactor Type: cleanup/refactor crate-keccak Issues related to the keccak workspace member T-tech-debt Type: tech-debt generated or cleaned up labels Nov 28, 2022
@CPerezz
Copy link
Member Author

CPerezz commented Feb 6, 2023

Until HI-LOW is not implemented. This won't be addressed.

@CPerezz CPerezz added the wontfix This will not be worked on label Feb 6, 2023
@ChihChengLiang
Copy link
Collaborator

We can create a new issue after the Hi-low is completed.

RainFallsSilent pushed a commit to dompute/zkevm-circuits that referenced this issue Sep 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-keccak Issues related to the keccak workspace member T-refactor Type: cleanup/refactor T-tech-debt Type: tech-debt generated or cleaned up wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants