Fix/range check (keep-starknet-strange#202)

* range check builtin

* range check runner

* memory validation rule

* range check and validation rule

* fixes

* format

* fixed

* some fixes

* try

* numBits()

* clean

* clean

* range check usage

* testing

* move rangeCheckValidation outside struct

* some tests added

* clean

* dust bunny

* fix runInstruction

* getFelt bug, some resolves for issues

* refactor to tuple

* fix hash

* Revert "fix hash"

This reverts commit 13be3fa.

* tuple test refactor

* fix test

* testing

* reset

* will test hash on other pr

---------

Co-authored-by: lanaivina <[email protected]>
Co-authored-by: Thomas Coratger <[email protected]>

src/math/fields/fields.zig

@@ -145,6 +145,21 @@ pub fn Field(
             return ret;
         }
 
+        /// Get the min number of bits needed to field element.
+        ///
+        /// Returns number of bits neeeded.
+        pub fn numBits(self: Self) u64 {
+            const nmself = self.fromMontgomery();
+            var num_bits: u64 = 0;
+            for (0..4) |i| {
+                if (nmself[3 - i] != 0) {
+                    num_bits = (4 - i) * @bitSizeOf(u64) - @clz(nmself[3 - i]);
+                    break;
+                }
+            }
+            return num_bits;
+        }
+
         /// Check if the field element is lexicographically largest.
         ///
         /// Determines whether the field element is larger than half of the field's modulus.

src/math/fields/starknet.zig

@@ -13,6 +13,13 @@ pub const Felt252 = fields.Field(
 const expect = std.testing.expect;
 const expectEqual = std.testing.expectEqual;
 
+test "Felt252 testing for field numBits()" {
+    try expectEqual(@as(u64, 1), Felt252.fromInteger(1).numBits());
+    try expectEqual(@as(u64, 4), Felt252.fromInteger(10).numBits());
+    try expectEqual(@as(u64, 252), Felt252.fromInteger(1).neg().numBits());
+    try expectEqual(@as(u64, 0), Felt252.fromInteger(0).numBits());
+}
+
 test "Felt252 fromInteger" {
     try expectEqual(
         Felt252{ .fe = .{

src/vm/builtins/builtin_runner/range_check.zig

@@ -8,6 +8,7 @@ const relocatable = @import("../../memory/relocatable.zig");
 const Error = @import("../../error.zig");
 const validation_rule = @import("../../memory/memory.zig").validation_rule;
 const Memory = @import("../../memory/memory.zig").Memory;
+const memoryFile = @import("../../memory/memory.zig");
 const range_check_instance_def = @import("../../types/range_check_instance_def.zig");
 
 const CELLS_PER_RANGE_CHECK = range_check_instance_def.CELLS_PER_RANGE_CHECK;
@@ -216,45 +217,51 @@ pub const RangeCheckBuiltinRunner = struct {
         return pointer;
     }
 
-    /// Creates Validation Rules ArrayList
+    /// Creates Validation Rule in Memory
     ///
     /// # Parameters
     ///
     /// - `memory`: A `Memory` pointer of validation rules segment index.
-    /// - `address`: A `Relocatable` pointer to the validation rule.
     ///
-    /// # Returns
+    /// # Modifies
     ///
-    /// An `ArrayList(Relocatable)` containing the rules address
-    /// verification fails.
-    pub fn rangeCheckValidationRule(memory: *Memory, address: Relocatable) ?[]const Relocatable {
-        const num = ((memory.get(address) catch {
-            return null;
-        }) orelse {
-            return null;
-        }).tryIntoFelt() catch {
-            return null;
-        };
-
-        if (@bitSizeOf(u256) - @clz(num.toInteger()) <= N_PARTS * INNER_RC_BOUND_SHIFT) {
-            // TODO: unit tests
-            return &[_]Relocatable{address};
-        } else {
-            return null;
-        }
+    /// - `memory`: Adds validation rule to `memory`.
+    pub fn addValidationRule(self: *const Self, memory: *Memory) !void {
+        try memory.addValidationRule(@intCast(self.base), rangeCheckValidationRule);
     }
 
-    /// Creates Validation Rule in Memory
+    /// Returns the min and max values of range check
     ///
     /// # Parameters
     ///
-    /// - `memory`: A `Memory` pointer of validation rules segment index.
+    /// - `memory`: A `Memory` pointer.
     ///
-    /// # Modifies
+    /// # Returns
     ///
-    /// - `memory`: Adds validation rule to `memory`.
-    pub fn addValidationRule(self: *const Self, memory: *Memory) !void {
-        try memory.addValidationRule(@intCast(self.base), rangeCheckValidationRule);
+    /// - An `Array`containing the min and max of range check.
+    pub fn getRangeCheckUsage(self: *Self, memory: *Memory) ?std.meta.Tuple(&.{ usize, usize }) {
+        if (memory.data.capacity == 0) {
+            return null;
+        }
+        const rc_segment = memory.data.items[self.base];
+        var rc_bounds = if (rc_segment.capacity > 0) [_]usize{ std.math.maxInt(usize), std.math.minInt(usize) } else return null;
+
+        for (rc_segment.items) |cell| {
+            var cellFelt = cell.?.maybe_relocatable.tryIntoFelt() catch null;
+            const cellBytes = cellFelt.?.toBytes();
+            var j: usize = 0;
+            while (j < 32) : (j += 2) {
+                const tempVal = @as(u16, cellBytes[j + 1]) << 8 | @as(u16, cellBytes[j]);
+
+                if (@as(usize, @intCast(tempVal)) < rc_bounds[0]) {
+                    rc_bounds[0] = @as(usize, @intCast(tempVal));
+                }
+                if (@as(usize, @intCast(tempVal)) > rc_bounds[1]) {
+                    rc_bounds[1] = @as(usize, @intCast(tempVal));
+                }
+            }
+        }
+        return .{ rc_bounds[0], rc_bounds[1] };
     }
 
     pub fn deduceMemoryCell(
@@ -269,6 +276,29 @@ pub const RangeCheckBuiltinRunner = struct {
     }
 };
 
+/// Creates Validation Rules ArrayList
+///
+/// # Parameters
+///
+/// - `memory`: A `Memory` pointer of validation rules segment index.
+/// - `address`: A `Relocatable` pointer to the validation rule.
+///
+/// # Returns
+///
+/// An `ArrayList(Relocatable)` containing the rules address
+/// verification fails.
+pub fn rangeCheckValidationRule(memory: *Memory, address: Relocatable) MemoryError![]const Relocatable {
+    const num = memory.getFelt(address) catch |err| switch (err) {
+        error.MemoryOutOfBounds => return MemoryError.RangeCheckGetError,
+        error.ExpectedInteger => return MemoryError.RangecheckNonInt,
+    };
+
+    if (num.numBits() <= N_PARTS * INNER_RC_BOUND_SHIFT) {
+        return &[_]Relocatable{address};
+    } else {
+        return MemoryError.RangeCheckNumberOutOfBounds;
+    }
+}
 test "initialize segments for range check" {
 
     // given
@@ -296,3 +326,148 @@ test "used instances" {
         try builtin.getUsedInstances(memory_segment_manager),
     );
 }
+
+test "Range Check: get usage for range check" {
+
+    // given
+    var builtin = RangeCheckBuiltinRunner.new(8, 8, true);
+    const allocator = std.testing.allocator;
+    var seg = try MemorySegmentManager.init(allocator);
+    defer seg.deinit();
+
+    try memoryFile.setUpMemory(seg.memory, std.testing.allocator, .{
+        .{ .{ 0, 0 }, .{1} },
+        .{ .{ 0, 1 }, .{2} },
+        .{ .{ 0, 2 }, .{3} },
+        .{ .{ 0, 3 }, .{4} },
+    });
+    defer seg.memory.deinitData(std.testing.allocator);
+    const res = builtin.getRangeCheckUsage(seg.memory);
+    try std.testing.expectEqual(@as(std.meta.Tuple(&.{ usize, usize }), .{ 0, 4 }), res.?);
+}
+
+test "Range Check: another successful check of usage for range check" {
+
+    // given
+    var builtin = RangeCheckBuiltinRunner.new(8, 8, true);
+    const allocator = std.testing.allocator;
+    var seg = try MemorySegmentManager.init(allocator);
+    defer seg.deinit();
+
+    try memoryFile.setUpMemory(seg.memory, std.testing.allocator, .{
+        .{ .{ 0, 0 }, .{1465218365} },
+        .{ .{ 0, 1 }, .{2134570341} },
+        .{ .{ 0, 2 }, .{31349610736} },
+        .{ .{ 0, 3 }, .{413468326585859} },
+    });
+    defer seg.memory.deinitData(std.testing.allocator);
+    const res = builtin.getRangeCheckUsage(seg.memory);
+    // assert
+    try std.testing.expectEqual(@as(std.meta.Tuple(&.{ usize, usize }), .{ 0, 62821 }), res.?);
+}
+
+test "Range Check: get usage for range check should be null" {
+
+    // given
+    var builtin = RangeCheckBuiltinRunner.new(8, 8, true);
+    const allocator = std.testing.allocator;
+    var seg = try MemorySegmentManager.init(allocator);
+    defer seg.deinit();
+
+    try memoryFile.setUpMemory(seg.memory, std.testing.allocator, .{});
+    defer seg.memory.deinitData(std.testing.allocator);
+
+    //const expected: ?[2]usize = null;
+    const expected: ?std.meta.Tuple(&.{ usize, usize }) = null;
+    // assert
+    try std.testing.expectEqual(expected, builtin.getRangeCheckUsage(seg.memory));
+}
+
+test "Range Check: validation rule should be empty" {
+
+    // given
+    var builtin = RangeCheckBuiltinRunner.new(8, 8, true);
+    const allocator = std.testing.allocator;
+    var mem = try MemorySegmentManager.init(allocator);
+    defer mem.deinit();
+
+    _ = builtin.getRangeCheckUsage(mem.memory);
+    // assert
+    try std.testing.expectEqual(
+        @as(usize, 0),
+        builtin.base,
+    );
+}
+
+test "Range Check: validation rule should return Relocatable in array successfully" {
+
+    // given
+    const allocator = std.testing.allocator;
+    var mem = try MemorySegmentManager.init(allocator);
+    defer mem.deinit();
+
+    const seg = mem.addSegment();
+    _ = try seg;
+
+    const relo = Relocatable.new(0, 1);
+    try mem.memory.set(std.testing.allocator, relo, MaybeRelocatable.fromFelt(Felt252.zero()));
+    defer mem.memory.deinitData(std.testing.allocator);
+
+    const result = try rangeCheckValidationRule(mem.memory, relo);
+    // assert
+    try std.testing.expectEqual(relo, result[0]);
+}
+
+test "Range Check: validation rule should return erorr out of bounds" {
+    // given
+    const allocator = std.testing.allocator;
+    var mem = try MemorySegmentManager.init(allocator);
+    defer mem.deinit();
+
+    const seg = mem.addSegment();
+    _ = try seg;
+
+    const relo = Relocatable.new(0, 1);
+    try mem.memory.set(std.testing.allocator, relo, MaybeRelocatable.fromFelt(Felt252.fromInteger(10).neg()));
+    defer mem.memory.deinitData(std.testing.allocator);
+
+    const result = rangeCheckValidationRule(mem.memory, relo);
+    // assert
+    try std.testing.expectError(MemoryError.RangeCheckNumberOutOfBounds, result);
+}
+
+test "Range Check: validation rule should return erorr non int" {
+    // given
+    const allocator = std.testing.allocator;
+    var mem = try MemorySegmentManager.init(allocator);
+    defer mem.deinit();
+
+    const seg = mem.addSegment();
+    _ = try seg;
+
+    const relo = Relocatable.new(0, 1);
+    try mem.memory.set(std.testing.allocator, relo, MaybeRelocatable.fromSegment(0, 2));
+    defer mem.memory.deinitData(std.testing.allocator);
+
+    const result = rangeCheckValidationRule(mem.memory, relo);
+    // assert
+    try std.testing.expectError(MemoryError.RangecheckNonInt, result);
+}
+
+test "Range Check: validation rule should return erorr address not in memory" {
+    // given
+    const allocator = std.testing.allocator;
+    var mem = try MemorySegmentManager.init(allocator);
+    defer mem.deinit();
+
+    const seg = mem.addSegment();
+    _ = try seg;
+
+    const relo = Relocatable.new(0, 1);
+    try mem.memory.set(std.testing.allocator, relo, MaybeRelocatable.fromFelt(Felt252.zero()));
+    defer mem.memory.deinitData(std.testing.allocator);
+
+    const result = rangeCheckValidationRule(mem.memory, Relocatable.new(0, 2));
+    // assert
+    try std.testing.expectError(MemoryError.RangeCheckGetError, result);
+}

src/vm/core.zig

@@ -297,9 +297,9 @@ pub const CairoVM = struct {
         );
 
         const OFFSET_BITS: u32 = 16;
-        const off_0 = instruction.off_0 + (@as(i16, 1) << (OFFSET_BITS - 1));
-        const off_1 = instruction.off_1 + (@as(i16, 1) << (OFFSET_BITS - 1));
-        const off_2 = instruction.off_2 + (@as(i16, 1) << (OFFSET_BITS - 1));
+        const off_0 = if (instruction.off_0 < 0) 0 else instruction.off_0 + (@as(i16, 1) << (OFFSET_BITS - 1));
+        const off_1 = if (instruction.off_0 < 0) 0 else instruction.off_1 + (@as(i16, 1) << (OFFSET_BITS - 1));
+        const off_2 = if (instruction.off_0 < 0) 0 else instruction.off_2 + (@as(i16, 1) << (OFFSET_BITS - 1));
 
         const limits = self.rc_limits orelse .{ off_0, off_0 };
         self.rc_limits = .{ @min(limits[0], off_0, off_1, off_2), @max(limits[1], off_0, off_1, off_2) };

src/vm/error.zig

@@ -53,6 +53,12 @@ pub const MemoryError = error{
     GetRangeMemoryGap,
     /// Math error
     Math,
+    /// Range Check Number is out of bounds
+    RangeCheckNumberOutOfBounds,
+    /// Range Check found a non int
+    RangecheckNonInt,
+    /// Range Check get error
+    RangeCheckGetError,
 };
 
 /// Reepresents different error conditions that occur in the built-in runners.

src/vm/memory/memory.zig

@@ -21,7 +21,7 @@ const MemorySegmentManager = @import("./segments.zig").MemorySegmentManager;
 const RangeCheckBuiltinRunner = @import("../builtins/builtin_runner/range_check.zig").RangeCheckBuiltinRunner;
 
 // Function that validates a memory address and returns a list of validated adresses
-pub const validation_rule = *const fn (*Memory, Relocatable) ?[]const Relocatable;
+pub const validation_rule = *const fn (*Memory, Relocatable) anyerror![]const Relocatable;
 
 pub const MemoryCell = struct {
     /// Represents a memory cell that holds relocation information and access status.
@@ -430,7 +430,7 @@ pub const Memory = struct {
                 else => error.ExpectedInteger,
             };
         } else {
-            return error.ExpectedInteger;
+            return error.MemoryOutOfBounds;
         }
     }
 
@@ -533,11 +533,9 @@ pub const Memory = struct {
     pub fn validateMemoryCell(self: *Self, address: Relocatable) !void {
         if (self.validation_rules.get(@intCast(address.segment_index))) |rule| {
             if (!self.validated_addresses.contains(address)) {
-                if (rule(self, address)) |list| {
-                    _ = list;
-                    // TODO: debug rangeCheckValidationRule to be able to push list here again
-                    try self.validated_addresses.addAddresses(&[_]Relocatable{address});
-                }
+                const list = try rule(self, address);
+                const firstElement = list[0];
+                try self.validated_addresses.addAddresses(&[_]Relocatable{firstElement});
             }
         }
     }
@@ -875,17 +873,13 @@ test "Memory: validate memory cell" {
 
     try segments.memory.validateMemoryCell(Relocatable.new(0, 1));
     // null case
-    try segments.memory.validateMemoryCell(Relocatable.new(0, 7));
     defer segments.memory.deinitData(std.testing.allocator);
 
     try expectEqual(
         true,
         segments.memory.validated_addresses.contains(Relocatable.new(0, 1)),
     );
-    try expectEqual(
-        false,
-        segments.memory.validated_addresses.contains(Relocatable.new(0, 7)),
-    );
+    try expectError(MemoryError.RangeCheckGetError, segments.memory.validateMemoryCell(Relocatable.new(0, 7)));
 }
 
 test "Memory: validate memory cell segment index not in validation rules" {