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
ABI formatting: change behaviour of "abi", introduce "abi_python" #995
Conversation
docs/compiling-a-contract.rst
Outdated
@@ -7,7 +7,7 @@ To compile a contract, use: | |||
|
|||
You can also compile to other formats such as ABI using the below format: | |||
:: | |||
vyper -f ['abi', 'json', 'bytecode', 'bytecode_runtime', 'ir'] yourFileName.vy | |||
vyper -f ['python_abi', 'abi', 'bytecode', 'bytecode_runtime', 'ir', 'asm'] yourFileName.vy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while i was at it, I noticed that asm
was missing from the docs as well, i fixed that as well
is there a good way to force couple the code with these docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no easy way that I can think of right away, no.
bin/vyper
Outdated
output_format['abi'] = lambda code: compiler.mk_full_signature(code) | ||
output_format['json'] = lambda code: json.dumps(compiler.mk_full_signature(code)) | ||
output_format['python_abi'] = lambda code: compiler.mk_full_signature(code) | ||
output_format['abi'] = lambda code: json.dumps(compiler.mk_full_signature(code)) | ||
output_format['bytecode'] = lambda code: '0x' + compiler.compile(code).hex() | ||
output_format['bytecode_runtime'] = lambda code: '0x' + compiler.compile(code, bytecode_runtime=True).hex() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by consistency with bytecode_runtime
, a better name may be abi_python
- WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure I agree, however we must leave the old json
. I know for example tools like embark rely on the using json and this will be then an incompatible change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we can introduce 'abi' to be the same as 'json' but we will have to leave json as an alias then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Just note that changing the behaviour of -f abi
will be a slightly incompatible change as well (depending on the syntax-tolerance of whatever parses the resulting ABI).
@kkom the binary has no tests so this is 100%, just see the note on |
@jacqueswww - thanks for the review! I've updated the commit in the PR and the description of the changes as requested. PS: I followed option 2 from https://www.burntfen.com/2015-10-30/how-to-amend-a-commit-on-a-github-pull-request as this is the flow I was used to with Phabricator at Facebook, let me know if this is not your preference in this repo! |
- What I did
Changed the ABI formatting options in the following way:
abi
->python_abi
abi
in addition to the existingjson
option (the latter is now an alias)The rationale is in Issue #989 - vanilla
abi
option should produce the fully standards compliant result.- How I did it
Searched for which lines to change using this query:
https://github.com/ethereum/vyper/search?q=json&unscoped_q=json
- How to verify it
make
.make test
vyper -f [abi, python_abi, json] examples/auctions/simple_open_auction.vy
Running automated tests:
Manually testing the new formatting options:
@jacqueswww - should I write any more end-to-end automated tests for it? I'll defer it to your judgement, but I'd need some pointer for a good way to do it.
- Description for the changelog
-f abi
to produce valid standard JSON ABI output-f abi_python
option for Python-formatted ABI output- Cute Animal Picture