Test: Compute Res Auxiliar Value (#74)

* edited comments todo list

* verify Compute Result Addition via more test results. Minor update on add function on memory_value.go

* Verify Test for ComputeRes Addition

* improve better format for test function

* separate each cases to have own functions

* add Mulitple operand test cases and also Unconstrained, Op1

* wip memory addition

* Fixes add function with felt-to-address addition

* added UpdatePc tests  (#61)

* added UpdatePc tests and fixed a bug in UpdatePc

* Fixed accessing field value without Read()

* fixed failing tests

* small refactor for TestUpdatePcJump

* Chore: Readme hotfix

* separate each cases to have own functions

* Fix as per comments of prev. PR. Fixes issue #56 adding test cases and code change

* integration test done

* minor change on comments

* Remove factorial_compiled.json

* Fix add implementation

---------

Co-authored-by: M. Mahdi Khosravi <mmk1776@gmail.com>
Co-authored-by: Rodrigo <rodrodpino@gmail.com>

pkg/vm/memory/memory_value.go

@@ -26,11 +26,9 @@ func (address *MemoryAddress) Equal(other *MemoryAddress) bool {
 func (address *MemoryAddress) Add(lhs *MemoryAddress, rhs *f.Element) error {
 	lhsOffset := new(f.Element).SetUint64(lhs.Offset)
 	newOffset := new(f.Element).Add(lhsOffset, rhs)
-
 	if !newOffset.IsUint64() {
 		return fmt.Errorf("new offset bigger than uint64: %s", rhs.Text(10))
 	}
-
 	address.SegmentIndex = lhs.SegmentIndex
 	address.Offset = newOffset.Uint64()
 	return nil
@@ -224,10 +222,10 @@ func (mv *MemoryValue) Add(lhs, rhs *MemoryValue) error {
 		}
 		return mv.address.Add(&lhs.address, &rhs.felt)
 	}
-
 	if rhs.IsAddress() {
 		return mv.address.Add(&rhs.address, &lhs.felt)
 	}
+
 	mv.felt.Add(&lhs.felt, &rhs.felt)
 	return nil
 }

pkg/vm/vm.go

@@ -367,14 +367,15 @@ func (vm *VirtualMachine) computeRes(
 			return mem.MemoryValue{}, fmt.Errorf("cannot read op1: %w", err)
 		}
 
+		res := mem.EmptyMemoryValueAs(op0.IsAddress() || op1.IsAddress())
 		if instruction.Res == AddOperands {
-			err = op0.Add(&op0, &op1)
+			err = res.Add(&op0, &op1)
 		} else if instruction.Res == MulOperands {
-			err = op0.Mul(&op0, &op1)
+			err = res.Mul(&op0, &op1)
 		} else {
 			return mem.MemoryValue{}, fmt.Errorf("invalid res flag value: %d", instruction.Res)
 		}
-		return op0, err
+		return res, err
 	}
 }
 

pkg/vm/vm_test.go

@@ -29,7 +29,6 @@ func TestVMCreation(t *testing.T) {
 // - cellDst: with ap and fp (using positive and negative offsets)
 // - cellOp0: with ap and fp (using positive and negative offsets)
 // - cellOp1: all four different outputs (using positive and negative offsets accordingly)
-// - calculate res: verify valid mulitplication and addition. Also verify nil output when correct
 // - update PC: verify all four cases. Besides, when testing relative jump (with or without conditions) that a negative relative address
 // - update AP: verify all posible cases, and when Res is a negative value
 // - update FP: verify all posible cases, and when Res is a negative value
@@ -350,16 +349,35 @@ func TestInferOperandSub(t *testing.T) {
 	assert.Equal(t, expectedOp0Vaue, op0Value)
 }
 
-func TestComputeAddRes(t *testing.T) {
+func TestComputeResUnconstrained(t *testing.T) {
 	vm := defaultVirtualMachine()
-	writeToDataSegment(vm, 0, mem.MemoryValueFromSegmentAndOffset(2, 10)) //op0Cell
-	writeToDataSegment(vm, 1, mem.MemoryValueFromInt(15))                 //op1Cell
-	op0Addr := mem.MemoryAddress{SegmentIndex: ExecutionSegment, Offset: 0}
-	op1Addr := mem.MemoryAddress{SegmentIndex: ExecutionSegment, Offset: 1}
+	instruction := Instruction{Res: Unconstrained}
 
-	instruction := Instruction{
-		Res: AddOperands,
-	}
+	res, err := vm.computeRes(&instruction, nil, nil)
+	require.NoError(t, err)
+	require.False(t, res.Known())
+}
+
+func TestComputeResOp1(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: Op1}
+
+	writeToDataSegment(vm, 3, mem.MemoryValueFromInt(15))
+	op1Addr := mem.MemoryAddress{SegmentIndex: ExecutionSegment, Offset: 3}
+
+	res, err := vm.computeRes(&instruction, nil, &op1Addr)
+	require.NoError(t, err)
+
+	expected := mem.MemoryValueFromInt(15)
+	assert.Equal(t, expected, res)
+}
+
+func TestComputeAddResAddrToFelt(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: AddOperands}
+
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromSegmentAndOffset(2, 10))
+	op1Addr := writeToDataSegment(vm, 8, mem.MemoryValueFromInt(15))
 
 	res, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
 	require.NoError(t, err)
@@ -368,6 +386,132 @@ func TestComputeAddRes(t *testing.T) {
 	assert.Equal(t, expected, res)
 }
 
+func TestComputeAddResFeltToAddr(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: AddOperands}
+
+	op0Addr := writeToDataSegment(vm, 2, mem.MemoryValueFromInt(8))
+	op1Addr := writeToDataSegment(vm, 5, mem.MemoryValueFromSegmentAndOffset(2, 7))
+
+	res, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.NoError(t, err)
+	expected := mem.MemoryValueFromSegmentAndOffset(2, 15)
+	assert.Equal(t, expected, res)
+}
+
+func TestComputeAddResBothAddrs(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: AddOperands}
+
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromSegmentAndOffset(2, 10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromSegmentAndOffset(2, 15))
+
+	_, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.Error(t, err) // Expecting an error since adding two addresses is not allowed
+}
+
+func TestComputeAddResBothFelts(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: AddOperands}
+
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromInt(10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromInt(15))
+
+	res, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.NoError(t, err)
+	expected := mem.MemoryValueFromInt(25)
+	assert.Equal(t, expected, res)
+}
+
+// Felt should be Positive or Negative. Thus four test cases
+func TestComputeMulResPosToPosFelt(t *testing.T) {
+	//Positive Felt to Positive Felt compute
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: MulOperands}
+
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromInt(10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromInt(15))
+
+	res, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.NoError(t, err)
+	expected := mem.MemoryValueFromInt(150)
+	assert.Equal(t, expected, res)
+}
+
+func TestComputeMulResNegToPosFelts(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: MulOperands}
+	//Negative to Positive
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromInt(-10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromInt(15))
+
+	res, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.NoError(t, err)
+	expected := mem.MemoryValueFromInt(-150)
+	assert.Equal(t, expected, res)
+}
+
+func TestComputeMulResPosToNegFelt(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: MulOperands}
+	//Positive to Negative
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromInt(10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromInt(-15))
+
+	res, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.NoError(t, err)
+	expected := mem.MemoryValueFromInt(-150)
+	assert.Equal(t, expected, res)
+}
+
+func TestComputeMulResNegToNegFelt(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: MulOperands}
+	//Netagive to Negative
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromInt(-10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromInt(-15))
+
+	res, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.NoError(t, err)
+	expected := mem.MemoryValueFromInt(150)
+	assert.Equal(t, expected, res)
+}
+
+// Multiplication does not involve addresses
+// three failing cases
+func TestComputeMulResAddrToFelt(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: MulOperands}
+
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromSegmentAndOffset(2, 10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromInt(15))
+
+	_, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.Error(t, err) // Expecting an error since multiplying an address with a felt is not allowed
+}
+
+func TestComputeMulResFeltToAddr(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: MulOperands}
+
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromInt(10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromSegmentAndOffset(2, 15))
+
+	_, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.Error(t, err)
+}
+
+func TestComputeMulResBothAddrs(t *testing.T) {
+	vm := defaultVirtualMachine()
+	instruction := Instruction{Res: MulOperands}
+
+	op0Addr := writeToDataSegment(vm, 3, mem.MemoryValueFromSegmentAndOffset(2, 10))
+	op1Addr := writeToDataSegment(vm, 4, mem.MemoryValueFromSegmentAndOffset(2, 15))
+
+	_, err := vm.computeRes(&instruction, &op0Addr, &op1Addr)
+	require.Error(t, err) // Expecting an error since multiplying two addresses is not allowed
+}
+
 func TestOpcodeAssertionAssertEq(t *testing.T) {
 	vm := defaultVirtualMachine()
 	dstAddr := mem.MemoryAddress{SegmentIndex: ExecutionSegment, Offset: 0}
@@ -526,20 +670,30 @@ func TestUpdateFp(t *testing.T) {
 	assert.Equal(t, vm.Context.Fp, nextFp)
 }
 
-func writeToDataSegment(vm *VirtualMachine, index uint64, value mem.MemoryValue) {
+func writeToDataSegment(vm *VirtualMachine, index uint64, value mem.MemoryValue) mem.MemoryAddress {
 	err := vm.MemoryManager.Memory.Write(ExecutionSegment, index, &value)
 	if err != nil {
 		panic("error in test util: writeToDataSegment")
 	}
+	return mem.MemoryAddress{
+		SegmentIndex: ExecutionSegment,
+		Offset:       index,
+	}
 }
 
 func defaultVirtualMachine() *VirtualMachine {
-	vm, _ := NewVirtualMachine(make([]*f.Element, 0), VirtualMachineConfig{false})
+	vm, err := NewVirtualMachine(make([]*f.Element, 0), VirtualMachineConfig{false})
+	if err != nil {
+		panic(err)
+	}
 	return vm
 }
 
 func defaultVirtualMachineWithBytecode(bytecode []*f.Element) *VirtualMachine {
-	vm, _ := NewVirtualMachine(bytecode, VirtualMachineConfig{false})
+	vm, err := NewVirtualMachine(bytecode, VirtualMachineConfig{false})
+	if err != nil {
+		panic(err)
+	}
 	return vm
 }