address review

solidity_contracts/src/L1L2Messaging/MessageConsumerTest.sol → solidity_contracts/src/L1L2Messaging/MessageAppL1.sol

@@ -12,7 +12,7 @@ error InvalidPayload();
    @title Test contract to receive / send messages to starknet.
    @author Glihm https://github.com/glihm/starknet-messaging-dev
 */
-contract MessageConsumerTest {
+contract MessageAppL1 {
 
     //
     IStarknetMessaging private _starknetMessaging;

solidity_contracts/src/L1L2Messaging/MessageSenderL2.sol → solidity_contracts/src/L1L2Messaging/MessageAppL2.sol

@@ -3,7 +3,7 @@ pragma solidity >=0.7.0 <0.9.0;
 
 import "../CairoPrecompiles/CairoLib.sol";
 
-contract MessageSenderL2  {
+contract MessageAppL2  {
     // @notice Sends a message to L1.
     // @dev Uses the Cairo Precompiles mechanism to invoke a the send_message_to_l1 syscall
     function sendMessageToL1(address to, uint128 value) external {

src/kakarot/precompiles/kakarot_precompiles.cairo

@@ -13,8 +13,8 @@ from utils.utils import Helpers
 const CALL_CONTRACT_SOLIDITY_SELECTOR = 0xb3eb2c1b;
 const LIBRARY_CALL_SOLIDITY_SELECTOR = 0x5a9af197;
 
+// TODO: compute acceptable EVM gas values for Cairo execution
 const CAIRO_PRECOMPILE_GAS = 10000;
-
 const CAIRO_MESSAGE_GAS = 5000;
 
 namespace KakarotPrecompiles {
@@ -117,18 +117,24 @@ namespace KakarotPrecompiles {
         let target_address = Helpers.bytes32_to_felt(input);
 
         let data_offset_ptr = input + 32;
-        let data_offset = Helpers.bytes32_to_felt(data_offset_ptr);
-        let data_len_ptr = input + data_offset;
+        let data_len_offset = Helpers.bytes32_to_felt(data_offset_ptr);
+        let data_len_ptr = input + data_len_offset;
 
         // Load input data by packing all
         // If the input data is larger than the size of a felt, it will wrap around the felt size.
         let data_words_len = Helpers.bytes32_to_felt(data_len_ptr);
         let data_bytes_len = data_words_len * 32;
-        let data_ptr = data_len_ptr + 32;
+        let data_offset = data_len_offset + 32;
+        let data_fits_in_input = is_le(data_bytes_len, input_len - data_offset);
+        if (data_fits_in_input == 0) {
+            let (revert_reason_len, revert_reason) = Errors.outOfBoundsRead();
+            return (revert_reason_len, revert_reason, CAIRO_MESSAGE_GAS, Errors.EXCEPTIONAL_HALT);
+        }
+        let data_ptr = input + data_offset;
         let (data_len, data) = Helpers.load_256_bits_array(data_bytes_len, data_ptr);
 
         send_message_to_l1(target_address, data_words_len, data);
         let (output) = alloc();
-        return (0, output, CAIRO_PRECOMPILE_GAS, Errors.EXCEPTIONAL_HALT);
+        return (0, output, CAIRO_MESSAGE_GAS, 0);
     }
 }

tests/end_to_end/L1L2Messaging/conftest.py

@@ -4,7 +4,7 @@
 @pytest.fixture(scope="session")
 def deploy_l1_contract():
     """
-    Fixture to attach a modified web3.contract instance to an already deployed contract_account on an L1 node.
+    Fixture to deploy and attach a modified web3.contract instance to a contract on L1.
     """
 
     from kakarot_scripts.utils.l1 import deploy_on_l1

tests/end_to_end/L1L2Messaging/test_messaging.py

@@ -19,10 +19,10 @@ async def sn_messaging_local(deploy_l1_contract, owner):
     if l1_addresses.get("StarknetMessagingLocal"):
         address = l1_addresses["StarknetMessagingLocal"]["address"]
         if l1_contract_exists(address):
-            return get_l1_contract("L1L2Messaging", "StarknetMessagingLocal", address)
+            return get_l1_contract("starknet", "StarknetMessagingLocal", address)
 
     contract = await deploy_l1_contract(
-        "L1L2Messaging",
+        "starknet",
         "StarknetMessagingLocal",
     )
     l1_addresses.update({"StarknetMessagingLocal": {"address": contract.address}})
@@ -34,7 +34,7 @@ async def sn_messaging_local(deploy_l1_contract, owner):
 async def message_sender_l2(deploy_contract, owner):
     message_sender = await deploy_contract(
         "L1L2Messaging",
-        "MessageSenderL2",
+        "MessageAppL2",
         caller_eoa=owner.starknet_contract,
     )
     return message_sender
@@ -45,7 +45,7 @@ async def message_consumer_test(deploy_l1_contract, sn_messaging_local):
     kakarot_address = get_deployments()["kakarot"]["address"]
     return await deploy_l1_contract(
         "L1L2Messaging",
-        "MessageConsumerTest",
+        "MessageAppL1",
         sn_messaging_local.address,
         kakarot_address,
     )

tests/end_to_end/conftest.py

@@ -260,7 +260,7 @@ async def _factory(address: Union[int, str]):
 @pytest.fixture(scope="session")
 def deploy_contract(default_fee: int):
     """
-    Fixture to attach a modified web3.contract instance to an already deployed contract_account in kakarot.
+    Fixture to deploy and attach a modified web3.contract instance to a contract in kakarot.
     """
 
     from kakarot_scripts.utils.kakarot import deploy

tests/src/kakarot/precompiles/test_precompiles.py

@@ -2,6 +2,7 @@
 from starkware.starknet.public.abi import get_selector_from_name
 
 from tests.utils.constants import (
+    CAIRO_MESSAGE_GAS,
     CAIRO_PRECOMPILE_GAS,
     FIRST_KAKAROT_PRECOMPILE_ADDRESS,
     FIRST_ROLLUP_PRECOMPILE_ADDRESS,
@@ -165,25 +166,69 @@ def test__cairo_precompiles(
 
                 return
 
+        @pytest.mark.parametrize(
+            "address, input_data, to_address, payload, expected_reverted",
+            [
+                (
+                    0x75002,
+                    bytes.fromhex(
+                        f"{0xc0de:064x}"
+                        + f"{0x40:064x}"
+                        + f"{0x01:064x}"
+                        + f"{0x2a:064x}"
+                    ),
+                    0xC0DE,
+                    [0x2A],
+                    False,
+                ),
+                # case with data_len not matching the actual data length
+                (
+                    0x75002,
+                    bytes.fromhex(
+                        f"{0xc0de:064x}"
+                        + f"{0x40:064x}"
+                        + f"{0x01:064x}"
+                        + f"{0x2a:032x}"
+                    ),
+                    0xC0DE,
+                    [],
+                    True,
+                ),
+                # case with input data too short
+                (
+                    0x75002,
+                    bytes.fromhex(f"{0xc0de:064x}" + f"{0x40:064x}"),
+                    0xC0DE,
+                    [],
+                    True,
+                ),
+            ],
+            ids=["ok", "ko_data_len_not_matching_actual_length", "ko_input_too_short"],
+        )
         class TestKakarotMessaging:
-            def test__cairo_message(self, cairo_run):
-                input_data = bytes.fromhex(
-                    f"{0xc0de:064x}"
-                    + f"{0x40:064x}"  # data_offset
-                    + f"{0x01:064x}"  # data_len: 1 word
-                    + f"{0x2a:064x}"  # data: uint256(42)
-                )
-
+            def test__cairo_message(
+                self,
+                cairo_run,
+                address,
+                input_data,
+                to_address,
+                payload,
+                expected_reverted,
+            ):
                 address = 0x75002
                 return_data, reverted, gas_used = cairo_run(
                     "test__precompiles_run",
                     address=address,
                     input=input_data,
-                    caller_address=0,
                 )
+                if expected_reverted:
+                    assert reverted
+                    return
+
                 SyscallHandler.mock_send_message_to_l1.assert_any_call(
-                    to_address=0xC0DE, payload=[0x2A]
+                    to_address=to_address, payload=payload
                 )
+                assert gas_used == CAIRO_MESSAGE_GAS
 
     class TestIsPrecompile:
         @pytest.mark.parametrize(

tests/utils/constants.py

@@ -41,6 +41,7 @@
 LAST_KAKAROT_PRECOMPILE_ADDRESS = 0x75002
 
 CAIRO_PRECOMPILE_GAS = 10000
+CAIRO_MESSAGE_GAS = 5000
 
 MAX_INT = 2**256 - 1
 

tests/utils/syscall_handler.py

@@ -475,12 +475,12 @@ def send_message_to_l1(self, segments, syscall_ptr):
         Record the send_message call in the internal mock object.
 
         Syscall structure is:
-        struct SendMessageToL1SysCall {
-            selector: felt,
-            to_address: felt,
-            payload_size: felt,
-            payload_ptr: felt*,
-        }
+            struct SendMessageToL1SysCall {
+                selector: felt,
+                to_address: felt,
+                payload_size: felt,
+                payload_ptr: felt*,
+            }
         """
         to_address = segments.memory[syscall_ptr + 1]
         payload_size = segments.memory[syscall_ptr + 2]