ethereum / execution-specs

Specification for the Execution Layer. Tracking network upgrades.
Creative Commons Zero v1.0 Universal
832 stars 234 forks source link

Strict types needed #793

Open winsvega opened 1 year ago

winsvega commented 1 year ago

I see many places in code rely on Any. thats very unclear what properties I can expect in this Any types. But here I want to point out this problem that I had too:

def read_randao(self, data: Any, t8n: Any) -> None:
        """
        Read the randao from the data.
        """
        self.prev_randao = None
        if t8n.is_after_fork("ethereum.paris"):
            # tf tool might not always provide an
            # even number of nibbles in the randao
            # This could create issues in the
            # hex_to_bytes function
            current_random = data["currentRandom"]
            if current_random.startswith("0x"):
                current_random = current_random[2:]

            if len(current_random) % 2 == 1:
                current_random = "0" + current_random

            self.prev_randao = Bytes32(
                left_pad_zero_bytes(hex_to_bytes(current_random), 32)
            )

this is because there is no type representing hex value. at some point it will make a mess reading and converting this to accurate 0x00 values. you need to store all hex values in prefixed hex string type. that will guarantee that the value is following schema

0x
0x00
0x01  
0x0112

this type class can read from int, string and output the formatted representation

Redidacove commented 7 months ago

Can you plz tell how can I make changes to fix this @winsvega and some places where the change is needed for reference

winsvega commented 7 months ago

Ah this in python uses a different model of exporting structures to json.

Like here: https://github.com/ethereum/execution-spec-tests/blob/00965e684dd23423d4cc4ee9f141ef8cd04f612a/src/ethereum_test_tools/spec/state/types.py#L31

So instead you must be sure all this exports convert types to a propriate format in json.

I would say the fill command need a verification of the generated test. Not just for types in json but also to try to read and execute it on a client.