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

Resolve as BigInt3 structure #346

Closed
cicr99 opened this issue Apr 10, 2024 · 6 comments · Fixed by #354
Closed

Resolve as BigInt3 structure #346

cicr99 opened this issue Apr 10, 2024 · 6 comments · Fixed by #354
Assignees
Labels
Duration: under a day Should only take a few hours good first issue Good for newcomers

Comments

@cicr99
Copy link
Contributor

cicr99 commented Apr 10, 2024

Currently we use the following logic for resolving BigInt3 structures:

valMemoryValues, err := hinter.GetConsecutiveValues(vm, valAddr, int16(3))
if err != nil {
    return err
}

// [d0, d1, d2]
var valValues [3]*fp.Element

for i := 0; i < 3; i++ {
    valValue, err := valMemoryValues[i].FieldElement()
    if err != nil {
    	return err
    }
    valValues[i] = valValue
}

valValues is the argument that should be passed down to the pack functionality.

As this logic is repeated in the code several times, it would be beneficial to separate it in its own functionality, something like ResolveAsBigInt3 passing the starting address of the structure in memory

@cicr99 cicr99 added good first issue Good for newcomers Duration: under a day Should only take a few hours labels Apr 10, 2024
@MoigeMatino
Copy link
Contributor

Would like to take this on if that's okay.

@MoigeMatino
Copy link
Contributor

@cicr99 Is there a specific file where this function should be stored?

@cicr99
Copy link
Contributor Author

cicr99 commented Apr 15, 2024

Hey @MoigeMatino sure! I just assigned you to it. This functionality should go into the hintrunner/utils folder as it's used by several hints. I would say to add it secp_utils.go file. Wdyt @har777 , since you've been working on that?

@har777
Copy link
Contributor

har777 commented Apr 15, 2024

Yeah thats the right file imo.

@MoigeMatino
Copy link
Contributor

@har777 @cicr99 adding function ResolveAsBigInt3 which calls hinter.GetConsecutiveValues(needs to import hinter) to hintrunner/utils/secp_utils.go results in a circular import because GetConsecutiveValues is from the hinter package. Would appreciate help in resolving this 🙏🏾, new to this.

@MoigeMatino
Copy link
Contributor

Here's the draft PR: #354

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duration: under a day Should only take a few hours good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants