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: optionally disable metadata in bytecode #3107

Merged

Conversation

emilianobonassi
Copy link
Contributor

@emilianobonassi emilianobonassi commented Sep 26, 2022

What I did

Optionally disable metadata in bytecode

How I did it

Added the flag --no-bytecode-metadata to the cli and pass the parameters in the respective components

How to verify it

Compile a contract with and without the flag. Bytecode produced will include or not the metadata (e.g. vyper signature) at the end

Commit message

feat: optionally disable metadata in bytecode

Description for the changelog

cli: optionally disable metadata in bytecode with --no-bytecode-metadata

Cute Animal Picture

image

@@ -967,7 +967,7 @@ def adjust_pc_maps(pc_maps, ofst):
return ret


def assembly_to_evm(assembly, pc_ofst=0, insert_vyper_signature=False):
def assembly_to_evm(assembly, pc_ofst=0, insert_vyper_signature=False, disable_vyper_signature=False):
Copy link
Member

Choose a reason for hiding this comment

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

insert_vyper_signature and disable_vyper_signature both being arguments is a little weird. it should be the callers responsibility to decide what the value here should be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, i agree, this was a short term resolution to avoid refactoring due to the recursive pattern L1011

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

looks good, left a comment about one signature, and also i think the option should be named something like --no-signature, or, --no-bytecode-metadata

@emilianobonassi
Copy link
Contributor Author

thanks @charles-cooper

renamed option to --no-bytecode-metadata more future proof and universal. left a comment for the tactical solution implemented. updated description.

@emilianobonassi emilianobonassi changed the title feat: optionally disable vyper signature in bytecode feat: optionally disable metadata in bytecode Sep 26, 2022
@fubuloubu fubuloubu enabled auto-merge (squash) September 26, 2022 21:59
@fubuloubu fubuloubu merged commit 42fa120 into vyperlang:master Sep 26, 2022
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.

None yet

3 participants