Stop reverting on precompiles (#803)

Time spent on this PR: 0.2 ## Pull request type Please check the type of change your PR introduces: - [ ] Bugfix - [x] Feature - [ ] Code style update (formatting, renaming) - [ ] Refactoring (no functional changes, no api changes) - [ ] Build related changes - [ ] Documentation content changes - [ ] Other (please describe): ## What is the current behavior? Some Cairo VM errors in precompiles ## What is the new behavior? Returns a reverted context as in other errors.
kkrt-labs · Nov 8, 2023 · 4c2ef03 · 4c2ef03
1 parent a396881
commit 4c2ef03
Show file tree

Hide file tree

Showing 23 changed files with 172 additions and 246 deletions.
diff --git a/src/kakarot/errors.cairo b/src/kakarot/errors.cairo
@@ -352,7 +352,7 @@ namespace Errors {
         assert [error + 24] = 108;  // l
         assert [error + 25] = 101;  // e
         assert [error + 26] = 32;  // " "
-        assert [error + 27] = address;  //
+        assert [error + 27] = '0' + address;  // convert uint address to str
         return (28, error);
     }
 
@@ -393,7 +393,7 @@ namespace Errors {
         assert [error + 31] = 108;  // l
         assert [error + 32] = 101;  // e
         assert [error + 33] = 32;  //
-        assert [error + 34] = address;  //
+        assert [error + 34] = '0' + address;  //
         return (35, error);
     }
 
@@ -499,4 +499,67 @@ namespace Errors {
         dw 'a';
         dw 's';
     }
+
+    func precompileInputError() -> (error_len: felt, error: felt*) {
+        let (error) = get_label_location(precompile_input_error_message);
+        return (27, error);
+
+        precompile_input_error_message:
+        dw 'P';
+        dw 'r';
+        dw 'e';
+        dw 'c';
+        dw 'o';
+        dw 'm';
+        dw 'p';
+        dw 'i';
+        dw 'l';
+        dw 'e';
+        dw ':';
+        dw ' ';
+        dw 'w';
+        dw 'r';
+        dw 'o';
+        dw 'n';
+        dw 'g';
+        dw ' ';
+        dw 'i';
+        dw 'n';
+        dw 'p';
+        dw 'u';
+        dw 't';
+        dw '_';
+        dw 'l';
+        dw 'e';
+        dw 'n';
+    }
+
+    func precompileFlagError() -> (error_len: felt, error: felt*) {
+        let (error) = get_label_location(precompile_flag_error);
+        return (22, error);
+
+        precompile_flag_error:
+        dw 'P';
+        dw 'r';
+        dw 'e';
+        dw 'c';
+        dw 'o';
+        dw 'm';
+        dw 'p';
+        dw 'i';
+        dw 'l';
+        dw 'e';
+        dw ':';
+        dw ' ';
+        dw 'f';
+        dw 'l';
+        dw 'a';
+        dw 'g';
+        dw ' ';
+        dw 'e';
+        dw 'r';
+        dw 'r';
+        dw 'o';
+        dw 'r';
+    }
 }
diff --git a/src/kakarot/precompiles/blake2f.cairo b/src/kakarot/precompiles/blake2f.cairo
@@ -12,6 +12,7 @@ from starkware.cairo.common.math_cmp import is_nn, is_le
 from starkware.cairo.common.bool import FALSE
 
 // Internal dependencies
+from kakarot.errors import Errors
 from utils.utils import Helpers
 
 // @title Blake2f Precompile related functions.
@@ -34,7 +35,7 @@ namespace PrecompileBlake2f {
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         alloc_locals;
         local rounds_bytes_len = 4;
@@ -45,16 +46,16 @@ namespace PrecompileBlake2f {
         local f_bytes_offset = 212;
 
         // Check input length
-        with_attr error_message(
-                "Kakarot: blake2f failed with incorrect input_len: {input_len} instead of 213") {
-            assert input_len = 213;
+        if (input_len != 213) {
+            let (revert_reason_len, revert_reason) = Errors.precompileInputError();
+            return (revert_reason_len, revert_reason, 0, 1);
         }
 
         // Check the flag
         tempvar f = input[f_bytes_offset];
-        with_attr error_message(
-                "Kakarot: blake2f failed with incorrect flag: {f} instead of 0 or 1") {
-            assert f = f * f;
+        if (f != f * f) {
+            let (revert_reason_len, revert_reason) = Errors.precompileFlagError();
+            return (revert_reason_len, revert_reason, 0, 1);
         }
 
         let rounds = Helpers.load_word(rounds_bytes_len, input);
@@ -81,7 +82,7 @@ namespace PrecompileBlake2f {
         Helpers.split_word_little(compressed[6], word_bytes_len, output + 6 * word_bytes_len);
         Helpers.split_word_little(compressed[7], word_bytes_len, output + 7 * word_bytes_len);
 
-        return (word_bytes_len * 8, output, rounds);
+        return (word_bytes_len * 8, output, rounds, 0);
     }
 }
 

diff --git a/src/kakarot/precompiles/datacopy.cairo b/src/kakarot/precompiles/datacopy.cairo
@@ -31,9 +31,9 @@ namespace PrecompileDataCopy {
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         let (minimum_word_size) = Helpers.minimum_word_count(input_len);
-        return (input_len, input, 3 * minimum_word_size + GAS_COST_DATACOPY);
+        return (input_len, input, 3 * minimum_word_size + GAS_COST_DATACOPY, 0);
     }
 }
diff --git a/src/kakarot/precompiles/ec_recover.cairo b/src/kakarot/precompiles/ec_recover.cairo
@@ -13,6 +13,7 @@ from starkware.cairo.common.cairo_secp.signature import (
 
 // Internal dependencies
 from utils.utils import Helpers
+from kakarot.errors import Errors
 
 // @title EcRecover Precompile related functions.
 // @notice This file contains the logic required to run the ec_recover precompile
@@ -35,21 +36,22 @@ namespace PrecompileEcRecover {
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         alloc_locals;
 
-        with_attr error_message(
-                "EcRecover: received wrong number of bytes in input: {input_len} instead of 4*32") {
-            assert input_len = 4 * 32;
+        if (input_len != 4 * 32) {
+            let (revert_reason_len, revert_reason) = Errors.precompileInputError();
+            return (revert_reason_len, revert_reason, 0, 1);
         }
 
         let hash = Helpers.bytes32_to_bigint(input);
         let v_uint256 = Helpers.bytes32_to_uint256(input + 32);
         let v = Helpers.uint256_to_felt(v_uint256);
 
-        with_attr error_message("EcRecover: Recovery identifier should be either 27 or 28") {
-            assert (v - 27) * (v - 28) = 0;
+        if ((v - 27) * (v - 28) != 0) {
+            let (revert_reason_len, revert_reason) = Errors.precompileFlagError();
+            return (revert_reason_len, revert_reason, 0, 1);
         }
 
         let r = Helpers.bytes32_to_bigint(input + 32 * 2);
@@ -68,6 +70,6 @@ namespace PrecompileEcRecover {
         let (output) = alloc();
         Helpers.split_word(public_address, 32, output);
 
-        return (32, output, GAS_COST_EC_RECOVER);
+        return (32, output, GAS_COST_EC_RECOVER, 0);
     }
 }
diff --git a/src/kakarot/precompiles/ecadd.cairo b/src/kakarot/precompiles/ecadd.cairo
@@ -33,7 +33,7 @@ namespace PrecompileEcAdd {
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         alloc_locals;
 
@@ -53,6 +53,6 @@ namespace PrecompileEcAdd {
         // We fill `output + bytes_x_len` ptr with `bytes_y` elements
         Helpers.fill_array(bytes_y_len, bytes_y, output + bytes_x_len);
 
-        return (G1POINT_BYTES_LEN * 2, output, GAS_COST_EC_ADD);
+        return (G1POINT_BYTES_LEN * 2, output, GAS_COST_EC_ADD, 0);
     }
 }
diff --git a/src/kakarot/precompiles/ecmul.cairo b/src/kakarot/precompiles/ecmul.cairo
@@ -33,7 +33,7 @@ namespace PrecompileEcMul {
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         alloc_locals;
 
@@ -50,6 +50,6 @@ namespace PrecompileEcMul {
         // We fill `output + bytes_x_len` ptr with `bytes_y` elements
         Helpers.fill_array(bytes_y_len, bytes_y, output + bytes_x_len);
 
-        return (G1POINT_BYTES_LEN * 2, output, GAS_COST_EC_MUL);
+        return (G1POINT_BYTES_LEN * 2, output, GAS_COST_EC_MUL, 0);
     }
 }
diff --git a/src/kakarot/precompiles/modexp.cairo b/src/kakarot/precompiles/modexp.cairo
@@ -32,7 +32,7 @@ namespace PrecompileModExpUint256 {
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         alloc_locals;
 
@@ -63,6 +63,6 @@ namespace PrecompileModExpUint256 {
             b_size, m_size, e_size, b, e, m
         );
 
-        return (output_len=bytes_len, output=bytes, gas_used=gas_cost);
+        return (bytes_len, bytes, gas_cost, 0);
     }
 }
diff --git a/src/kakarot/precompiles/precompiles.cairo b/src/kakarot/precompiles/precompiles.cairo
@@ -12,6 +12,7 @@ from starkware.cairo.common.math_cmp import is_le, is_not_zero
 // Internal dependencies
 from kakarot.constants import Constants
 from kakarot.execution_context import ExecutionContext
+from kakarot.errors import Errors
 from kakarot.memory import Memory
 from kakarot.model import model
 from kakarot.precompiles.blake2f import PrecompileBlake2f
@@ -48,9 +49,6 @@ namespace Precompiles {
     ) -> model.ExecutionContext* {
         alloc_locals;
 
-        // Execute the precompile at a given evm_address
-        let (output_len, output, gas_used) = _exec_precompile(evm_address, calldata_len, calldata);
-
         // Build returned execution context
         let stack = Stack.init();
         let memory = Memory.init();
@@ -71,7 +69,12 @@ namespace Precompiles {
         );
         let sub_ctx = ExecutionContext.init(call_context);
         let sub_ctx = ExecutionContext.update_state(sub_ctx, calling_context.state);
-        let sub_ctx = ExecutionContext.stop(sub_ctx, output_len, output, FALSE);
+
+        // Execute the precompile at a given evm_address
+        let (output_len, output, gas_used, reverted) = _exec_precompile(
+            evm_address, calldata_len, calldata
+        );
+        let sub_ctx = ExecutionContext.stop(sub_ctx, output_len, output, reverted);
 
         return sub_ctx;
     }
@@ -88,13 +91,14 @@ namespace Precompiles {
     // @return output_len The output length.
     // @return output The output array.
     // @return gas_used The gas usage of precompile.
+    // @return reverted Whether the precompile ran successfully or not
     func _exec_precompile{
         syscall_ptr: felt*,
         pedersen_ptr: HashBuiltin*,
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(evm_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         // Compute the corresponding offset in the jump table:
         // count 1 for "next line" and 3 steps per precompile evm_address: call, precompile, ret
@@ -143,11 +147,11 @@ namespace Precompiles {
         pedersen_ptr: HashBuiltin*,
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
-    }(evm_address: felt, _input_len: felt, _input: felt*) {
-        with_attr error_message("Kakarot: UnknownPrecompile {evm_address}") {
-            assert 0 = 1;
-        }
-        return ();
+    }(evm_address: felt, _input_len: felt, _input: felt*) -> (
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
+    ) {
+        let (revert_reason_len, revert_reason) = Errors.unknownPrecompile(evm_address);
+        return (revert_reason_len, revert_reason, 0, 1);
     }
 
     // @notice A placeholder for precompile that are not implemented yet.
@@ -160,11 +164,11 @@ namespace Precompiles {
         pedersen_ptr: HashBuiltin*,
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
-    }(evm_address: felt, _input_len: felt, _input: felt*) {
-        with_attr error_message("Kakarot: NotImplementedPrecompile {evm_address}") {
-            assert 0 = 1;
-        }
-        return ();
+    }(evm_address: felt, _input_len: felt, _input: felt*) -> (
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
+    ) {
+        let (revert_reason_len, revert_reason) = Errors.notImplementedPrecompile(evm_address);
+        return (revert_reason_len, revert_reason, 0, 1);
     }
 
     // @notice A placeholder for precompile that are not implemented yet.
@@ -177,10 +181,10 @@ namespace Precompiles {
         pedersen_ptr: HashBuiltin*,
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
-    }(evm_address: felt, _input_len: felt, _input: felt*) {
-        with_attr error_message("Kakarot: NotWhitelistedPrecompile {evm_address}") {
-            assert 0 = 1;
-        }
-        return ();
+    }(evm_address: felt, _input_len: felt, _input: felt*) -> (
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
+    ) {
+        let (revert_reason_len, revert_reason) = Errors.notImplementedPrecompile(evm_address);
+        return (revert_reason_len, revert_reason, 0, 1);
     }
 }
diff --git a/src/kakarot/precompiles/ripemd160.cairo b/src/kakarot/precompiles/ripemd160.cairo
@@ -43,7 +43,7 @@ namespace PrecompileRIPEMD160 {
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         alloc_locals;
         let (local buf: felt*) = alloc();
@@ -76,7 +76,7 @@ namespace PrecompileRIPEMD160 {
 
         // 5. return bytes hash code.
         let (minimum_word_size) = Helpers.minimum_word_count(input_len);
-        return (32, arr_x - 12, 120 * minimum_word_size + GAS_COST_RIPEMD160);
+        return (32, arr_x - 12, 120 * minimum_word_size + GAS_COST_RIPEMD160, 0);
     }
 }
 

diff --git a/src/kakarot/precompiles/sha256.cairo b/src/kakarot/precompiles/sha256.cairo
@@ -43,7 +43,7 @@ namespace PrecompileSHA256 {
         range_check_ptr,
         bitwise_ptr: BitwiseBuiltin*,
     }(_address: felt, input_len: felt, input: felt*) -> (
-        output_len: felt, output: felt*, gas_used: felt
+        output_len: felt, output: felt*, gas_used: felt, reverted: felt
     ) {
         alloc_locals;
 
@@ -82,7 +82,7 @@ namespace PrecompileSHA256 {
             8, hash, 0, hash_bytes_array
         );
         let (minimum_word_size) = Helpers.minimum_word_count(input_len);
-        return (32, hash_bytes_array, 12 * minimum_word_size + GAS_COST_SHA256);
+        return (32, hash_bytes_array, 12 * minimum_word_size + GAS_COST_SHA256, 0);
     }
 }