Skip to content

feat: bigint circuit to compile #1639

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

Open
wants to merge 4 commits into
base: feat/new-execution
Choose a base branch
from

Conversation

arayikhalatyan
Copy link
Contributor

Implemented e1 and e3 for HeapBranch, Heap, and VecHeap adapters. Updated the Bigint circuit correspondingly. Had to make some changes in the interfaces of rv32im Steps. In particular

  • Changed Reads type ([u8; N], [u8; N]) into Into<[[u8;N];2]> and Writes type [u8; N] into From<[[u8;N];1]>. This change corresponds to what we used to do with the previous integration API in order to make the interfaces to match.

  • Got rid of TraceAdapterContext in a lot of places. This is because the same Step can be using different AdapterSteps that require different TraceContexts. Or even the AdapterStep might require a TraceContext that the Step doesn't have. The easy solution was to implement AdapterSteps in a similar way as in the previous integration API. That is, added the necessary fields to the AdapterStep structs. I am thinking maybe deleting the TraceContext from the interface makes sense. I am not sure if there is a better way to do this

Important Note: the tests don't run right now because a lot of the read/write operations are done in address space 2 with block size 32 but currently only block size 4 is supported by the memory.

Resolves INT-3980

Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

Hmm we need to discuss this: it is not ideal for a Step to contain a Chip because ideally Step should be more stateless. Maybe @shuklaayush has a view here

@jonathanpwang
Copy link
Contributor

seems like we do need to store some type of reference inside the AdapterStep struct. For now we'll use smart pointer, but Arayi left a comment that it's not ideal and we should use directly reference if possible in the future

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.

2 participants