cairo-vm-go
cairo-vm-go copied to clipboard
NethermindEth
Create a structure for Scope value types instead of using interface
Currently the ScopeManager contains a map[string]any to represent a scope, where the key is the name of the variables present in the scope and the value would be the value of such variable. any is being used because we cannot know the type of this value beforehand. As this is used in python code you could get a felt or a bigint, or a mapping, etc.
However, the use of any is not very efficient, so the proposed solution to improve this is to create a struct ScopeValue which will hold the value of the variables. It could look like the following:
type ScopeValue struct {
FeltValue *fp.Element
BigIntValue *Big.Int
}
We should add all possible types in there; it could be done on demand, as we go implementing new hints.
Another idea could be to create a ScopeValueType enum for all these possible types and add a field Type ScopeValueType to the previous struct to know the type of the value without having to check which member is not nil
Hey @fishonamos I've just assigned you to #345 as you also requested . I would like you to finish that one before assigning you to another if that's ok
Hello @cicr99 . I'd like to contribute to this issue, could you please assign it to me?
Hey @fmmesen it's yours!
@danielcdz sorry for replying so late, if you are still interested you're free to pick any open issue you like!
Thanks @rodrigo-pino ! Is there a communication channel for the project?
Hey yes, there is a Telegram group, you will be able to find the link on the Only Dust Hack channel
@rodrigo-pino @cicr99 it's me again sorry for the delay, but if this is something I can help with, I'll be happy to work, this will be my third contribution to the project!
Hey @danielcdz, the solution described in this issue is no longer applicable. As we continued to implement hints, turns out that you could get a lot more of possible types for the variables you want to store in the Scope, which would mean we would have to create a field for each of them in the ScopeValue struct. The idea here is to optimize the current code, and this doesn't seem a like a big improvement, nor an elegant solution. We need to come up with a better idea, so any suggestions are welcome
Hello, @cicr99! I have to clarify my idea and proposal, but I think that this can be an opportunity to use the reflect package from Golang
@cicr99 I was looking at the code and the usages for the ScopeManager and I think using reflect would not optimize much, and we can recur to many changes on the current architecture of the ScopeManager, also, for storing multiple types the best way is to continue using map[string]any, to have an easy way to store and work with numerous types in the scope, on the other side, what I think we can change and I think it is something that we want to avoid is type casting when retrieving a variable from the scope, like here(not directly a type casting but using a specific method to retrieve the value as BigInt), and here, a good way to tackle this is using generics, and creating a method inside scope.go like this one i.e:
// GetVariableAs retrieves a variable from the current scope and asserts its type
func GetVariableAs[T any](sm *ScopeManager, name string) (T, error) {
var zero T // Zero value of the generic type
value, ok := sm.GetVariableValue(name)
if !ok {
return zero, errors.New("variable not found")
}
typedValue, ok := value.(T)
if !ok {
return zero, errors.New("variable has a different type")
}
return typedValue, nil
}
and example of usage will looks like this:
// Retrieve variables with type assertion
fpVar, err := ctx.ScopeManager.GetVariableAs[fp.Element]("fpVar")
if err != nil {
fmt.Println("Error:", err)
} else {
fmt.Println("intVar:", intValue)
}
Doing this will be easy to implement keeping an easy way to store unknown first-hand types in the scope, and optimizing the way to retrieve the values of variables.
I will wait for your response! if this is not a good approach I can keep researching to solve this in the best way!
hey @danielcdz, sorru for the very late answer.
sounds like a nice idea to me. Will make the code cleaner overall but it doesnt improve efficiency at all, it is just some kind of elegant refactoring. You need anyway to check !ok when doing typedValue, ok := value.(T) but there seem to be no other way if we keep using any
@cicr99 what do you think?
just noticed this issue :
https://github.com/NethermindEth/cairo-vm-go/issues/448
which basically suggests the same
@TAdev0 Don't worry, and yes it would be a elegance a better way to retrieve the variables value but if we keep using any, I don't see other way to improve performance, other than adding specific types to the struct