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

feat: add vyper version bytecode check #1578

Merged
merged 3 commits into from
Jul 12, 2022

Conversation

tserg
Copy link
Contributor

@tserg tserg commented Jul 11, 2022

What I did

Vyper 0.3.4 adds a signature to the bytecode. This causes an error when compiling in Brownie because the source map is empty before the opcodes queue is exhausted in _generate_coverage_data() in brownie/project/compiler/vyper.py.

Related issue: #1566 and vyperlang/vyper#2955

How I did it

Check for the signature when the source map is empty, and exit the while loop.

How to verify it

Checklist

  • I have confirmed that my PR passes all linting checks
  • I have included test cases
  • I have updated the documentation
  • I have added an entry to the changelog

@@ -310,6 +310,10 @@ def _generate_coverage_data(
revert_pc = len(opcodes) + sum(int(i[4:]) - 1 for i in opcodes if i.startswith("PUSH")) - 5

while opcodes:
# ignore vyper version bytecode for >= 0.3.4
if len(source_map) == 0 and opcodes[0] == "PUSH6" and opcodes[1] == "0x767970657283":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think a more general/graceful solution would just be to break if len(source_map) == 0, but i'm not sure what other consequences that would have here

@tserg tserg marked this pull request as ready for review July 12, 2022 03:23
@charles-cooper charles-cooper merged commit 0e0619e into eth-brownie:master Jul 12, 2022
@tserg tserg deleted the feat/vyper_version branch July 13, 2022 04:21
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 this pull request may close these issues.

2 participants