From 57483be7ecf533e3571a9e01d2e771a0c0db40b4 Mon Sep 17 00:00:00 2001 From: oxlime <93354898+oxlime@users.noreply.github.com> Date: Mon, 4 Dec 2023 10:24:13 -0600 Subject: [PATCH] Fix/range check (#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 13be3fa1b5ab97a4ccdbf298dc61759915a0fbb5. * tuple test refactor * fix test * testing * reset * will test hash on other pr --------- Co-authored-by: lanaivina <31368580+lana-shanghai@users.noreply.github.com> Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> --- src/math/fields/fields.zig | 15 ++ src/math/fields/starknet.zig | 7 + .../builtins/builtin_runner/range_check.zig | 227 ++++++++++++++++-- src/vm/core.zig | 6 +- src/vm/error.zig | 6 + src/vm/memory/memory.zig | 18 +- 6 files changed, 238 insertions(+), 41 deletions(-) diff --git a/src/math/fields/fields.zig b/src/math/fields/fields.zig index bc5b270f..04bc0f19 100644 --- a/src/math/fields/fields.zig +++ b/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. diff --git a/src/math/fields/starknet.zig b/src/math/fields/starknet.zig index 354586b9..70778412 100644 --- a/src/math/fields/starknet.zig +++ b/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 = .{ diff --git a/src/vm/builtins/builtin_runner/range_check.zig b/src/vm/builtins/builtin_runner/range_check.zig index 059e1935..6980205c 100644 --- a/src/vm/builtins/builtin_runner/range_check.zig +++ b/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); +} diff --git a/src/vm/core.zig b/src/vm/core.zig index deb666d3..1cf3dc59 100644 --- a/src/vm/core.zig +++ b/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) }; diff --git a/src/vm/error.zig b/src/vm/error.zig index 52971631..1c3db5f7 100644 --- a/src/vm/error.zig +++ b/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. diff --git a/src/vm/memory/memory.zig b/src/vm/memory/memory.zig index 8954bc16..53de5c64 100644 --- a/src/vm/memory/memory.zig +++ b/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" {