fix: modify get method in Memory to return null instead of error (

keep-starknet-strange#207)

* modify get method in memory to return null instead of error

* fix tests

src/vm/builtins/bitwise/bitwise.zig

@@ -35,9 +35,7 @@ const BITWISE_INPUT_CELLS_PER_INSTANCE = 2;
 /// # Returns
 /// The felt as an integer.
 fn getValue(address: Relocatable, memory: *Memory) BitwiseError!u256 {
-    const value = memory.get(address) catch {
-        return BitwiseError.InvalidAddressForBitwise;
-    };
+    const value = memory.get(address);
 
     if (value) |v| {
         var felt = v.tryIntoFelt() catch {

src/vm/builtins/builtin_runner/keccak.zig

@@ -346,9 +346,7 @@ pub const KeccakBuiltinRunner = struct {
             const stop_pointer_addr = pointer.subUint(
                 @intCast(1),
             ) catch return RunnerError.NoStopPointer;
-            const stop_pointer = try ((segments.memory.get(
-                stop_pointer_addr,
-            ) catch return RunnerError.NoStopPointer) orelse return RunnerError.NoStopPointer).tryIntoRelocatable();
+            const stop_pointer = try (segments.memory.get(stop_pointer_addr) orelse return RunnerError.NoStopPointer).tryIntoRelocatable();
             if (@as(
                 isize,
                 @intCast(self.base),
@@ -430,9 +428,7 @@ pub const KeccakBuiltinRunner = struct {
             usize,
             @intCast(self.n_input_cells),
         )) |i| {
-            const num = ((memory.get(try first_input_addr.addUint(@intCast(i))) catch {
-                return null;
-            }) orelse return null).tryIntoFelt() catch {
+            const num = (memory.get(try first_input_addr.addUint(@intCast(i))) orelse return null).tryIntoFelt() catch {
                 return RunnerError.BuiltinExpectedInteger;
             };
 

src/vm/builtins/builtin_runner/range_check.zig

@@ -192,9 +192,7 @@ pub const RangeCheckBuiltinRunner = struct {
             const stop_pointer_addr = pointer.subUint(
                 @intCast(1),
             ) catch return RunnerError.NoStopPointer;
-            const stop_pointer = try (segments.memory.get(
-                stop_pointer_addr,
-            ) catch return RunnerError.NoStopPointer).tryIntoRelocatable();
+            const stop_pointer = try (segments.memory.get(stop_pointer_addr)).tryIntoRelocatable();
             if (@as(
                 isize,
                 @intCast(self.base),

src/vm/core.zig

@@ -142,7 +142,7 @@ pub const CairoVM = struct {
     pub fn getRelocatable(
         self: *Self,
         address: Relocatable,
-    ) error{MemoryOutOfBounds}!?MaybeRelocatable {
+    ) ?MaybeRelocatable {
         return self.segments.memory.get(address);
     }
 
@@ -232,9 +232,7 @@ pub const CairoVM = struct {
         // *                    FETCH                                 *
         // ************************************************************
 
-        const encoded_instruction = self.segments.memory.get(self.run_context.pc.*) catch {
-            return CairoVMError.InstructionFetchingFailed;
-        };
+        const encoded_instruction = self.segments.memory.get(self.run_context.pc.*);
 
         // ************************************************************
         // *                    DECODE                                *
@@ -326,19 +324,19 @@ pub const CairoVM = struct {
         op_res.res = null;
 
         op_res.dst_addr = try self.run_context.computeDstAddr(instruction);
-        const dst_op = try self.segments.memory.get(op_res.dst_addr);
+        const dst_op = self.segments.memory.get(op_res.dst_addr);
 
         op_res.op_0_addr = try self.run_context.computeOp0Addr(instruction);
 
         op_res.op_1_addr = try self.run_context.computeOp1Addr(
             instruction,
             op_res.op_0,
         );
-        const op_1_op = try self.segments.memory.get(op_res.op_1_addr);
+        const op_1_op = self.segments.memory.get(op_res.op_1_addr);
 
         // Deduce the operands if they haven't been successfully retrieved from memory.
 
-        if (self.segments.memory.get(op_res.op_0_addr) catch null) |op_0| {
+        if (self.segments.memory.get(op_res.op_0_addr)) |op_0| {
             op_res.op_0 = op_0;
         } else {
             op_res.setOp0(true);

src/vm/core_test.zig

@@ -887,7 +887,7 @@ test "set get value in vm memory" {
 
     // Test checks
     // Verify the value is correctly set to 42.
-    const actual_value = try vm.segments.memory.get(address);
+    const actual_value = vm.segments.memory.get(address);
     const expected_value = value;
     try expectEqual(
         expected_value,
@@ -1548,8 +1548,8 @@ test "CairoVM: getRelocatable without value raises error" {
     defer vm.deinit();
 
     // Test check
-    try expectError(
-        error.MemoryOutOfBounds,
+    try expectEqual(
+        @as(?MaybeRelocatable, null),
         vm.getRelocatable(Relocatable.new(0, 0)),
     );
 }
@@ -1571,7 +1571,7 @@ test "CairoVM: getRelocatable with value should return a MaybeRelocatable" {
     // Test check
     try expectEqual(
         MaybeRelocatable.fromU256(5),
-        (try vm.getRelocatable(Relocatable.new(34, 12))).?,
+        (vm.getRelocatable(Relocatable.new(34, 12))).?,
     );
 }
 
@@ -1913,15 +1913,15 @@ test "CairoVM: InserDeducedOperands should insert operands if set as deduced" {
 
     // Test checks
     try expectEqual(
-        try vm.segments.memory.get(Relocatable.new(1, 0)),
+        vm.segments.memory.get(Relocatable.new(1, 0)),
         dst_val,
     );
     try expectEqual(
-        try vm.segments.memory.get(Relocatable.new(1, 1)),
+        vm.segments.memory.get(Relocatable.new(1, 1)),
         op0_val,
     );
     try expectEqual(
-        try vm.segments.memory.get(Relocatable.new(1, 2)),
+        vm.segments.memory.get(Relocatable.new(1, 2)),
         op1_val,
     );
 }
@@ -1967,16 +1967,16 @@ test "CairoVM: InserDeducedOperands insert operands should not be inserted if no
     try vm.insertDeducedOperands(allocator, test_operands);
 
     // Test checks
-    try expectError(
-        error.MemoryOutOfBounds,
+    try expectEqual(
+        @as(?MaybeRelocatable, null),
         vm.segments.memory.get(Relocatable.new(1, 0)),
     );
-    try expectError(
-        error.MemoryOutOfBounds,
+    try expectEqual(
+        @as(?MaybeRelocatable, null),
         vm.segments.memory.get(Relocatable.new(1, 1)),
     );
-    try expectError(
-        error.MemoryOutOfBounds,
+    try expectEqual(
+        @as(?MaybeRelocatable, null),
         vm.segments.memory.get(Relocatable.new(1, 2)),
     );
 }

src/vm/memory/memory.zig

@@ -382,24 +382,20 @@ pub const Memory = struct {
         data_segment.items[address.offset] = MemoryCell.new(value);
     }
 
-    // Get some value from the memory at the given address.
-    // # Arguments
-    // - `address` - The address to get the value from.
-    // # Returns
-    // The value at the given address.
-    pub fn get(
-        self: *Self,
-        address: Relocatable,
-    ) error{MemoryOutOfBounds}!?MaybeRelocatable {
+    /// Retrieves data at a specified address within a relocatable data structure.
+    ///
+    /// - `self`: The instance of the data structure.
+    /// - `address`: The target address to retrieve data from.
+    /// Returns:
+    /// A `MaybeRelocatable` value if the address is valid; otherwise, returns `null`.
+    pub fn get(self: *Self, address: Relocatable) ?MaybeRelocatable {
         const data = if (address.segment_index < 0) &self.temp_data else &self.data;
         const segment_index: usize = @intCast(if (address.segment_index < 0) -(address.segment_index + 1) else address.segment_index);
 
         const isSegmentIndexValid = address.segment_index < data.items.len;
         const isOffsetValid = isSegmentIndexValid and (address.offset < data.items[segment_index].items.len);
 
-        if (!isSegmentIndexValid or !isOffsetValid) {
-            return CairoVMError.MemoryOutOfBounds;
-        }
+        if (!isSegmentIndexValid or !isOffsetValid) return null;
 
         if (data.items[segment_index].items[@intCast(address.offset)]) |val| {
             return val.maybe_relocatable;
@@ -423,8 +419,8 @@ pub const Memory = struct {
     pub fn getFelt(
         self: *Self,
         address: Relocatable,
-    ) error{ MemoryOutOfBounds, ExpectedInteger }!Felt252 {
-        if (try self.get(address)) |m| {
+    ) error{ ExpectedInteger, MemoryOutOfBounds }!Felt252 {
+        if (self.get(address)) |m| {
             return switch (m) {
                 .felt => |fe| fe,
                 else => error.ExpectedInteger,
@@ -449,8 +445,8 @@ pub const Memory = struct {
     pub fn getRelocatable(
         self: *Self,
         address: Relocatable,
-    ) error{ MemoryOutOfBounds, ExpectedRelocatable }!Relocatable {
-        if (try self.get(address)) |m| {
+    ) error{ExpectedRelocatable}!Relocatable {
+        if (self.get(address)) |m| {
             return switch (m) {
                 .relocatable => |rel| rel,
                 else => error.ExpectedRelocatable,
@@ -697,7 +693,7 @@ pub const Memory = struct {
     ) !std.ArrayList(?MaybeRelocatable) {
         var values = std.ArrayList(?MaybeRelocatable).init(allocator);
         for (0..size) |i| {
-            try values.append(try self.get(try address.addUint(@intCast(i))));
+            try values.append(self.get(try address.addUint(@intCast(i))));
         }
         return values;
     }
@@ -752,7 +748,7 @@ pub const Memory = struct {
         );
         errdefer values.deinit();
         for (0..size) |i| {
-            if (try self.get(try address.addUint(@intCast(i)))) |elem| {
+            if (self.get(try address.addUint(@intCast(i)))) |elem| {
                 try values.append(elem);
             } else {
                 return MemoryError.GetRangeMemoryGap;
@@ -974,52 +970,59 @@ test "memory inner for testing test" {
     );
 }
 
-test "memory get without value raises error" {
-    // ************************************************************
-    // *                 SETUP TEST CONTEXT                       *
-    // ************************************************************
+test "Memory: get method without segment should return null" {
+    // Test setup
     // Initialize an allocator.
     const allocator = std.testing.allocator;
 
     // Initialize a memory instance.
     var memory = try Memory.init(allocator);
     defer memory.deinit();
 
-    // ************************************************************
-    // *                      TEST CHECKS                         *
-    // ************************************************************
+    // Test checks
     // Get a value from the memory at an address that doesn't exist.
-    try expectError(
-        error.MemoryOutOfBounds,
+    try expectEqual(
+        @as(?MaybeRelocatable, null),
         memory.get(Relocatable.new(0, 0)),
     );
 }
 
-test "memory set and get" {
-    // ************************************************************
-    // *                 SETUP TEST CONTEXT                       *
-    // ************************************************************
+test "Memory: get method wit segment but non allocated memory should return null" {
+    // Test setup
     // Initialize an allocator.
     const allocator = std.testing.allocator;
 
     // Initialize a memory instance.
     var memory = try Memory.init(allocator);
     defer memory.deinit();
 
-    // ************************************************************
-    // *                      TEST BODY                           *
-    // ************************************************************
-    const address_1 = Relocatable.new(
-        0,
-        0,
+    // Set a value into the memory.
+    try setUpMemory(
+        memory,
+        std.testing.allocator,
+        .{
+            .{ .{ 0, 0 }, .{1} },
+            .{ .{ 0, 10 }, .{1} },
+        },
     );
-    const value_1 = MaybeRelocatable.fromFelt(starknet_felt.Felt252.one());
+    defer memory.deinitData(std.testing.allocator);
 
-    const address_2 = Relocatable.new(
-        -1,
-        0,
+    // Test checks
+    // Get a value from the memory at an address that doesn't exist.
+    try expectEqual(
+        @as(?MaybeRelocatable, null),
+        memory.get(Relocatable.new(0, 15)),
     );
-    const value_2 = MaybeRelocatable.fromFelt(starknet_felt.Felt252.one());
+}
+
+test "Memory: set and get for both segments and temporary segments should return proper MaybeRelocatable values" {
+    // Test setup
+    // Initialize an allocator.
+    const allocator = std.testing.allocator;
+
+    // Initialize a memory instance.
+    var memory = try Memory.init(allocator);
+    defer memory.deinit();
 
     // Set a value into the memory.
     try setUpMemory(
@@ -1030,19 +1033,17 @@ test "memory set and get" {
             .{ .{ -1, 0 }, .{1} },
         },
     );
-
     defer memory.deinitData(std.testing.allocator);
 
-    // Get the value from the memory.
-    const maybe_value_1 = try memory.get(address_1);
-    const maybe_value_2 = try memory.get(address_2);
-
-    // ************************************************************
-    // *                      TEST CHECKS                         *
-    // ************************************************************
-    // Assert that the value is the expected value.
-    try expect(maybe_value_1.?.eq(value_1));
-    try expect(maybe_value_2.?.eq(value_2));
+    // Test checks
+    try expectEqual(
+        @as(?MaybeRelocatable, MaybeRelocatable.fromU256(1)),
+        memory.get(Relocatable.new(0, 0)),
+    );
+    try expectEqual(
+        @as(?MaybeRelocatable, MaybeRelocatable.fromU256(1)),
+        memory.get(Relocatable.new(-1, 0)),
+    );
 }
 
 test "Memory: get inside a segment without value but inbout should return null" {
@@ -1069,7 +1070,7 @@ test "Memory: get inside a segment without value but inbout should return null"
     // Test check
     try expectEqual(
         @as(?MaybeRelocatable, null),
-        try memory.get(Relocatable.new(1, 3)),
+        memory.get(Relocatable.new(1, 3)),
     );
 }
 
@@ -1125,7 +1126,7 @@ test "validate existing memory for range check within bound" {
     defer memory.deinitData(std.testing.allocator);
 
     // Get the value from the memory.
-    const maybe_value_1 = try memory.get(address_1);
+    const maybe_value_1 = memory.get(address_1);
 
     // ************************************************************
     // *                      TEST CHECKS                         *
@@ -1184,14 +1185,14 @@ test "Memory: getFelt should return ExpectedInteger error if Relocatable instead
     );
 }
 
-test "Memory: getRelocatable should return MemoryOutOfBounds error if no value at the given address" {
+test "Memory: getRelocatable should return ExpectedRelocatable error if no value at the given address" {
     // Test setup
     var memory = try Memory.init(std.testing.allocator);
     defer memory.deinit();
 
     // Test checks
     try expectError(
-        error.MemoryOutOfBounds,
+        error.ExpectedRelocatable,
         memory.getRelocatable(Relocatable.new(0, 0)),
     );
 }