Fix RLP.decode

src/utils/eth_transaction.cairo

@@ -28,30 +28,41 @@ namespace EthTransaction {
     ) -> model.EthTransaction* {
         // see https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md
         alloc_locals;
-        let (items: RLP.Item*) = alloc();
-        RLP.decode(items, tx_data_len, tx_data);
+        let (tx_items: RLP.Item*) = alloc();
+        RLP.decode(tx_items, tx_data_len, tx_data);
 
-        // the tx is a list of fields, hence first level RLP decoding
-        // is a single item, which is indeed the sought list
-        assert [items].is_list = TRUE;
-        let sub_items_len = [items].data_len;
-        let sub_items = cast([items].data, RLP.Item*);
+        assert [tx_items].is_list = TRUE;
+        let items_len = [tx_items].data_len;
+        let items = cast([tx_items].data, RLP.Item*);
 
-        let nonce = Helpers.bytes_to_felt(sub_items[0].data_len, sub_items[0].data);
-        let gas_price = Helpers.bytes_to_felt(sub_items[1].data_len, sub_items[1].data);
-        let gas_limit = Helpers.bytes_to_felt(sub_items[2].data_len, sub_items[2].data);
+        // Pre eip-155 txs have 6 fields, post eip-155 txs have 9 fields
+        // We check for both cases here, and do the remaining ones in the next if block
+        assert items[0].is_list = FALSE;
+        assert items[1].is_list = FALSE;
+        assert items[2].is_list = FALSE;
+        assert items[3].is_list = FALSE;
+        assert items[4].is_list = FALSE;
+        assert items[5].is_list = FALSE;
+
+        let nonce = Helpers.bytes_to_felt(items[0].data_len, items[0].data);
+        let gas_price = Helpers.bytes_to_felt(items[1].data_len, items[1].data);
+        let gas_limit = Helpers.bytes_to_felt(items[2].data_len, items[2].data);
         let destination = Helpers.try_parse_destination_from_bytes(
-            sub_items[3].data_len, sub_items[3].data
+            items[3].data_len, items[3].data
         );
-        let amount = Helpers.bytes_to_uint256(sub_items[4].data_len, sub_items[4].data);
-        let payload_len = sub_items[5].data_len;
-        let payload = sub_items[5].data;
+        let amount = Helpers.bytes_to_uint256(items[4].data_len, items[4].data);
+        let payload_len = items[5].data_len;
+        let payload = items[5].data;
 
         // pre eip-155 txs have 6 fields, post eip-155 txs have 9 fields
-        if (sub_items_len == 6) {
+        if (items_len == 6) {
             tempvar chain_id = 0;
         } else {
-            let chain_id = Helpers.bytes_to_felt(sub_items[6].data_len, sub_items[6].data);
+            assert items_len = 9;
+            assert items[6].is_list = FALSE;
+            assert items[7].is_list = FALSE;
+            assert items[8].is_list = FALSE;
+            let chain_id = Helpers.bytes_to_felt(items[6].data_len, items[6].data);
         }
         let chain_id = [ap - 1];
 
@@ -80,25 +91,37 @@ namespace EthTransaction {
     ) -> model.EthTransaction* {
         alloc_locals;
 
-        let (items: RLP.Item*) = alloc();
-        RLP.decode(items, tx_data_len - 1, tx_data + 1);
-        let sub_items_len = [items].data_len;
-        let sub_items = cast([items].data, RLP.Item*);
+        let (tx_items: RLP.Item*) = alloc();
+        RLP.decode(tx_items, tx_data_len - 1, tx_data + 1);
+
+        assert [tx_items].is_list = TRUE;
+        let items_len = [tx_items].data_len;
+        let items = cast([tx_items].data, RLP.Item*);
+
+        assert items_len = 8;
+        assert items[0].is_list = FALSE;
+        assert items[1].is_list = FALSE;
+        assert items[2].is_list = FALSE;
+        assert items[3].is_list = FALSE;
+        assert items[4].is_list = FALSE;
+        assert items[5].is_list = FALSE;
+        assert items[6].is_list = FALSE;
+        assert items[7].is_list = TRUE;
 
-        let chain_id = Helpers.bytes_to_felt(sub_items[0].data_len, sub_items[0].data);
-        let nonce = Helpers.bytes_to_felt(sub_items[1].data_len, sub_items[1].data);
-        let gas_price = Helpers.bytes_to_felt(sub_items[2].data_len, sub_items[2].data);
-        let gas_limit = Helpers.bytes_to_felt(sub_items[3].data_len, sub_items[3].data);
+        let chain_id = Helpers.bytes_to_felt(items[0].data_len, items[0].data);
+        let nonce = Helpers.bytes_to_felt(items[1].data_len, items[1].data);
+        let gas_price = Helpers.bytes_to_felt(items[2].data_len, items[2].data);
+        let gas_limit = Helpers.bytes_to_felt(items[3].data_len, items[3].data);
         let destination = Helpers.try_parse_destination_from_bytes(
-            sub_items[4].data_len, sub_items[4].data
+            items[4].data_len, items[4].data
         );
-        let amount = Helpers.bytes_to_uint256(sub_items[5].data_len, sub_items[5].data);
-        let payload_len = sub_items[6].data_len;
-        let payload = sub_items[6].data;
+        let amount = Helpers.bytes_to_uint256(items[5].data_len, items[5].data);
+        let payload_len = items[6].data_len;
+        let payload = items[6].data;
 
         let (access_list: felt*) = alloc();
         let access_list_len = parse_access_list(
-            access_list, sub_items[7].data_len, cast(sub_items[7].data, RLP.Item*)
+            access_list, items[7].data_len, cast(items[7].data, RLP.Item*)
         );
         tempvar tx = new model.EthTransaction(
             signer_nonce=nonce,
@@ -125,30 +148,38 @@ namespace EthTransaction {
     ) -> model.EthTransaction* {
         alloc_locals;
 
-        let (items: RLP.Item*) = alloc();
-        RLP.decode(items, tx_data_len - 1, tx_data + 1);
-        // the tx is a list of fields, hence first level RLP decoding
-        // is a single item, which is indeed the sought list
-        assert [items].is_list = TRUE;
-        let sub_items_len = [items].data_len;
-        let sub_items = cast([items].data, RLP.Item*);
+        let (tx_items: RLP.Item*) = alloc();
+        RLP.decode(tx_items, tx_data_len - 1, tx_data + 1);
 
-        let chain_id = Helpers.bytes_to_felt(sub_items[0].data_len, sub_items[0].data);
-        let nonce = Helpers.bytes_to_felt(sub_items[1].data_len, sub_items[1].data);
-        let max_priority_fee_per_gas = Helpers.bytes_to_felt(
-            sub_items[2].data_len, sub_items[2].data
-        );
-        let max_fee_per_gas = Helpers.bytes_to_felt(sub_items[3].data_len, sub_items[3].data);
-        let gas_limit = Helpers.bytes_to_felt(sub_items[4].data_len, sub_items[4].data);
+        assert [tx_items].is_list = TRUE;
+        let items_len = [tx_items].data_len;
+        let items = cast([tx_items].data, RLP.Item*);
+
+        assert items_len = 9;
+        assert items[0].is_list = FALSE;
+        assert items[1].is_list = FALSE;
+        assert items[2].is_list = FALSE;
+        assert items[3].is_list = FALSE;
+        assert items[4].is_list = FALSE;
+        assert items[5].is_list = FALSE;
+        assert items[6].is_list = FALSE;
+        assert items[7].is_list = FALSE;
+        assert items[8].is_list = TRUE;
+
+        let chain_id = Helpers.bytes_to_felt(items[0].data_len, items[0].data);
+        let nonce = Helpers.bytes_to_felt(items[1].data_len, items[1].data);
+        let max_priority_fee_per_gas = Helpers.bytes_to_felt(items[2].data_len, items[2].data);
+        let max_fee_per_gas = Helpers.bytes_to_felt(items[3].data_len, items[3].data);
+        let gas_limit = Helpers.bytes_to_felt(items[4].data_len, items[4].data);
         let destination = Helpers.try_parse_destination_from_bytes(
-            sub_items[5].data_len, sub_items[5].data
+            items[5].data_len, items[5].data
         );
-        let amount = Helpers.bytes_to_uint256(sub_items[6].data_len, sub_items[6].data);
-        let payload_len = sub_items[7].data_len;
-        let payload = sub_items[7].data;
+        let amount = Helpers.bytes_to_uint256(items[6].data_len, items[6].data);
+        let payload_len = items[7].data_len;
+        let payload = items[7].data;
         let (access_list: felt*) = alloc();
         let access_list_len = parse_access_list(
-            access_list, sub_items[8].data_len, cast(sub_items[8].data, RLP.Item*)
+            access_list, items[8].data_len, cast(items[8].data, RLP.Item*)
         );
         tempvar tx = new model.EthTransaction(
             signer_nonce=nonce,

src/utils/rlp.cairo

@@ -68,7 +68,7 @@ namespace RLP {
     // @param data The RLP encoded data.
     // @param items The pointer to the next free cell in the list of items decoded.
     // @return items_len The number of items decoded.
-    func decode{range_check_ptr}(items: Item*, data_len: felt, data: felt*) -> felt {
+    func decode_raw{range_check_ptr}(items: Item*, data_len: felt, data: felt*) -> felt {
         alloc_locals;
 
         if (data_len == 0) {
@@ -80,7 +80,7 @@ namespace RLP {
         if (rlp_type == 1) {
             // Case list
             let (sub_items: Item*) = alloc();
-            let sub_items_len = decode(items=sub_items, data_len=len, data=data + offset);
+            let sub_items_len = decode_raw(items=sub_items, data_len=len, data=data + offset);
             assert [items] = Item(data_len=sub_items_len, data=cast(sub_items, felt*), is_list=1);
             tempvar range_check_ptr = range_check_ptr;
         } else {
@@ -91,14 +91,22 @@ namespace RLP {
         }
         tempvar items = items + Item.SIZE;
 
-        let total_item_len = len + offset;
-        let is_lt_input = is_nn(data_len + 1 - total_item_len);
-        if (is_lt_input != FALSE) {
-            let items_len = decode(
-                items=items, data_len=data_len - total_item_len, data=data + total_item_len
-            );
-            return 1 + items_len;
+        let remaining_data_len = data_len - len - offset;
+        let items_len = decode_raw(
+            items=items, data_len=remaining_data_len, data=data + offset + len
+        );
+        return 1 + items_len;
+    }
+
+    func decode{range_check_ptr}(items: Item*, data_len: felt, data: felt*) {
+        alloc_locals;
+        let (rlp_type, offset, len) = decode_type(data_len=data_len, data=data);
+        local extra_bytes = data_len - offset - len;
+        with_attr error_message("RLP string ends with {extra_bytes} superfluous bytes") {
+            assert extra_bytes = 0;
         }
-        return 1;
+        let items_len = decode_raw(items=items, data_len=data_len, data=data);
+        assert items_len = 1;
+        return ();
     }
 }

tests/conftest.py

@@ -3,9 +3,11 @@
 import os
 
 import pytest
+from dotenv import load_dotenv
 from hypothesis import Verbosity, settings
 from starkware.cairo.lang.instances import LAYOUTS
 
+load_dotenv()
 logging.getLogger("asyncio").setLevel(logging.ERROR)
 logger = logging.getLogger()
 

tests/fixtures/starknet.py

@@ -234,9 +234,6 @@ def _factory(entrypoint, **kwargs) -> list:
         output_stem = Path(
             f"{output_stem[:160]}_{int(time_ns())}_{md5(output_stem.encode()).digest().hex()[:8]}"
         )
-        logger.info(
-            f"Test {request.node.path.stem}_{entrypoint}_{displayed_args} artifacts saved at: {output_stem}"
-        )
         if request.config.getoption("profile_cairo"):
             tracer_data = TracerData(
                 program=cairo_program,

tests/src/utils/test_eth_transaction.py

@@ -10,15 +10,25 @@
 class TestEthTransaction:
 
     class TestDecodeTransaction:
+
+        async def test_should_raise_with_list_items(self, cairo_run):
+            transaction = {
+                "nonce": 0,
+                "gasPrice": 234567897654321,
+                "gas": 2_000_000,
+                "to": "0xF0109fC8DF283027b6285cc889F5aA624EaC1F55",
+                "value": ["000000000"],
+                "data": b"",
+            }
+            with cairo_error():
+                cairo_run("test__decode", data=list(encode(list(transaction.values()))))
+
         @pytest.mark.parametrize("transaction", TRANSACTIONS)
         async def test_should_decode_all_transactions_types(
             self, cairo_run, transaction
         ):
             encoded_unsigned_tx = rlp_encode_signed_data(transaction)
-            decoded_tx = cairo_run(
-                "test__decode",
-                data=list(encoded_unsigned_tx),
-            )
+            decoded_tx = cairo_run("test__decode", data=list(encoded_unsigned_tx))
 
             expected_data = (
                 "0x" + transaction["data"].hex()

tests/src/utils/test_rlp.cairo

@@ -5,7 +5,7 @@ from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin
 from starkware.cairo.common.alloc import alloc
 from starkware.cairo.common.memcpy import memcpy
 
-func test__decode{range_check_ptr}(output_ptr: felt*) {
+func test__decode{range_check_ptr}() -> RLP.Item* {
     alloc_locals;
     // Given
     tempvar data_len: felt;
@@ -19,15 +19,10 @@ func test__decode{range_check_ptr}(output_ptr: felt*) {
     let (local items: RLP.Item*) = alloc();
     RLP.decode(items, data_len, data);
 
-    %{
-        from tests.utils.hints import flatten_rlp_list
-        # The cairo functions returns a single RLP list of size 1 containing the decoded objects.
-        flatten_rlp_list(ids.items.address_, 1, ids.output_ptr, memory, segments)
-    %}
-    return ();
+    return items;
 }
 
-func test__decode_type{range_check_ptr}(output_ptr: felt*) {
+func test__decode_type{range_check_ptr}() -> (felt, felt, felt) {
     alloc_locals;
     // Given
     tempvar data_len: felt;
@@ -41,31 +36,5 @@ func test__decode_type{range_check_ptr}(output_ptr: felt*) {
     let (type, offset, len) = RLP.decode_type(data_len, data);
 
     // Then
-    assert [output_ptr] = type;
-    assert [output_ptr + 1] = offset;
-    assert [output_ptr + 2] = len;
-
-    return ();
-}
-
-func test__decode_transaction{range_check_ptr}(output_ptr: felt*) {
-    alloc_locals;
-    // Given
-    tempvar data_len: felt;
-    let (data) = alloc();
-    %{
-        ids.data_len = len(program_input["data"])
-        segments.write_arg(ids.data, program_input["data"])
-    %}
-
-    // When
-    let (local items: RLP.Item*) = alloc();
-    RLP.decode(items, data_len, data);
-
-    %{
-        from tests.utils.hints import flatten_rlp_list
-        # The cairo functions returns a single RLP list of size 1 containing the decoded objects.
-        flatten_rlp_list(ids.items.address_, 1, ids.output_ptr, memory, segments)
-    %}
-    return ();
+    return (type, offset, len);
 }