Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'pc' should be changed to 'program_counter' in the __slots__ of CodeStream class #2109

Closed
imtypist opened this issue May 16, 2023 · 1 comment · Fixed by #2115
Closed

'pc' should be changed to 'program_counter' in the __slots__ of CodeStream class #2109

imtypist opened this issue May 16, 2023 · 1 comment · Fixed by #2115

Comments

@imtypist
Copy link

imtypist commented May 16, 2023

pc should be changed to program_counter in the __slots__ of CodeStream class. Although it does not raise errors (since program_counter is inherited from CodeStreamAPI), I think should correct it.

class CodeStream(CodeStreamAPI):
    __slots__ = [
        "_length_cache",
        "_raw_code_bytes",
        "invalid_positions",
        "valid_positions",
        "pc",
    ]

    logger = logging.getLogger("eth.vm.CodeStream")

    def __init__(self, code_bytes: bytes) -> None:
        validate_is_bytes(code_bytes, title="CodeStream bytes")
        # in order to avoid method overhead when setting/accessing pc, we no longer
        # fence the pc (Program Counter) into 0 <= pc <= len(code_bytes).
        # We now let it float free.
        # NOTE: Setting pc to a negative value has undefined behavior.
        self.program_counter = 0
        self._raw_code_bytes = code_bytes
        self._length_cache = len(code_bytes)
        self.invalid_positions: Set[int] = set()
        self.valid_positions: Set[int] = set()

https://github.com/ethereum/py-evm/blob/8aa9eb51944b4dcf6b365eb032ac674fed933c09/eth/vm/code_stream.py#LL21C1-L42C47

@pacrob
Copy link
Collaborator

pacrob commented Jun 5, 2023

Good catch, thanks @imtypist ! PR incoming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants