Verify compute res - Add, Mul, Unconstrained #68
Conversation
pkg/vm/memory/memory_value.go
Outdated
| mv.address, err = mv.address.Add(lhs.address, rhs.felt) | ||
| mv.felt = nil // Felt field is set to nil |
There was a problem hiding this comment.
No need to set felt member as nil since address is already non nil. MemoryValue only has one member non nil at a time. It works as the union of both types.
If felt and address are both non nil, then the MemoryValue type was misused, that means that the error occurred somewhere before. No need to hide it here.
pkg/vm/vm_test.go
Outdated
| @@ -346,34 +348,168 @@ func TestInferOperandSub(t *testing.T) { | |||
| assert.Equal(t, expectedOp0Cell, op0Cell) | |||
| } | |||
|
|
|||
| func TestComputeAddRes(t *testing.T) { | |||
| func TestComputeRes_Unconstrained(t *testing.T) { | |||
There was a problem hiding this comment.
Use CamelCase for defining Tests, update the name to TestComputeResUnconstrained. Please do this to the other tests as well
pkg/vm/vm_test.go
Outdated
| cellOp0 := &mem.Cell{Accessed: true, Value: mem.MemoryValueFromInt(10)} | ||
| cellOp1 := &mem.Cell{Accessed: true, Value: mem.MemoryValueFromInt(15)} |
There was a problem hiding this comment.
For this tests cellOp0 and cellOp1 can be nil since they are not being used by the method being tested
There was a problem hiding this comment.
done. Test goes with Nil as operands, and succeeds.
pkg/vm/vm_test.go
Outdated
| vm := defaultVirtualMachine() | ||
| instruction := Instruction{Res: Op1} | ||
|
|
||
| cellOp0 := &mem.Cell{Accessed: true, Value: mem.MemoryValueFromInt(10)} |
There was a problem hiding this comment.
cellOp0 is not used in this test, so no need to create it to test the function
There was a problem hiding this comment.
done. Cell Operand 0 set to nil, the test succeeds.
pkg/vm/vm_test.go
Outdated
| vm := defaultVirtualMachine() | ||
| instruction := Instruction{Res: AddOperands} | ||
|
|
||
| cellOp0 := &mem.Cell{Accessed: true, Value: mem.MemoryValueFromMemoryAddress(mem.NewMemoryAddress(2, 10))} |
There was a problem hiding this comment.
You can use MemoryValueFromSegmentAndOffset method
pkg/vm/vm_test.go
Outdated
| } | ||
|
|
||
| // Felt should be Positive or Negative. Thus four test cases | ||
| func TestComputeMulRes_PToPFelt(t *testing.T) { |
There was a problem hiding this comment.
| func TestComputeMulRes_PToPFelt(t *testing.T) { | |
| func TestComputeMulResPosToPosFelt(t *testing.T) { |
There was a problem hiding this comment.
Done. Applied to other function names too.
pkg/vm/vm_test.go
Outdated
| instruction := Instruction{Res: MulOperands} | ||
|
|
||
| cellOp0 := &mem.Cell{Accessed: true, Value: mem.MemoryValueFromInt(15)} | ||
| cellOp1 := &mem.Cell{Accessed: true, Value: mem.MemoryValueFromMemoryAddress(mem.NewMemoryAddress(2, 10))} |
There was a problem hiding this comment.
Use MemoryFromSegmentAndOffset
… add function on memory_value.go
jkktom
force-pushed
the
Verify-ComputeRes
branch
from
e996c8a
to
c5d96d1
Compare
Please kindly discard former branch. I changed too much so just restarted.