search

NethermindEth / cairo-vm-go

A virtual machine for Cairo written in Go
MIT License
79 stars 49 forks source link

[bug]: implement correctly `UsortVerifyMultiplicityBody` hint #398

Closed TAdev0 closed 3 months ago

TAdev0 commented 3 months ago

Resolves #391

This PR rectifies UsortVerifyMultiplicityBody hint previously wrongly implemented. I also added more tests, always with a descending list as input (required).

note that current_pos does not need to be assigned in scope , i also rectified this. current_scope is calculated for each iteration with positions.pop() and is not required in scope. Also, i added a reassignement of positions in the scope because it was missing, positions needs to be updated in scope otherwise the array will ne be emptied in the recursive call, this will cause an infinite loop when calling usort

TAdev0 commented 3 months ago

Like SquashDictInnerNextKey , if we want tests to be really good, and check the value in the scope of the array of int64 elements positions after popping, i guess we should add a Case in varValueInScopeEquals for arrays of int64.

MaksymMalicki commented 3 months ago

This looks legit, the utility that you have mentioned is neccessary. I think you can do it in this PR as it is close to merging.

TAdev0 commented 3 months ago

@MaksymMalicki comments addressed! should be good to go I used again Reflect library in test. I also modified the usage of int64 for felt for last_pos and the array.