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

ABI formatting: change behaviour of "abi", introduce "abi_python" #995

Merged
merged 1 commit into from Aug 22, 2018
Merged

ABI formatting: change behaviour of "abi", introduce "abi_python" #995

merged 1 commit into from Aug 22, 2018

Conversation

kkom
Copy link
Contributor

@kkom kkom commented Aug 15, 2018

- What I did

Changed the ABI formatting options in the following way:

  • Python formatted output (single quotes, uppercase True and False): abi -> python_abi
  • standard JSON output: accept abi in addition to the existing json 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

  1. Checkout my branch and make.
  2. Run automated tests: make test
  3. Manually check new and old formatting options: vyper -f [abi, python_abi, json] examples/auctions/simple_open_auction.vy

Running automated tests:

(vyper-venv) Highgarden:vyper kkom$ make test
python setup.py test

(...)

---------- coverage: platform darwin, python 3.6.5-final-0 -----------
Name                                     Stmts   Miss  Cover
------------------------------------------------------------
vyper/__init__.py                            7      0   100%
vyper/compile_lll.py                       248      2    99%
vyper/compiler.py                           34      3    91%
vyper/exceptions.py                         44      0   100%
vyper/functions/__init__.py                  1      0   100%
vyper/functions/functions.py               291      5    98%
vyper/functions/signature.py                81      2    98%
vyper/opcodes.py                             7      0   100%
vyper/optimizer.py                          83     14    83%
vyper/parser/__init__.py                     0      0   100%
vyper/parser/context.py                     49      0   100%
vyper/parser/expr.py                       467     21    96%
vyper/parser/lll_node.py                   179     16    91%
vyper/parser/parser.py                     682     20    97%
vyper/parser/parser_utils.py               169     12    93%
vyper/parser/s_expressions.py               34      1    97%
vyper/parser/stmt.py                       360     14    96%
vyper/premade_contracts.py                   5      0   100%
vyper/server.py                              0      0   100%
vyper/signatures/__init__.py                 0      0   100%
vyper/signatures/event_signature.py         56      4    93%
vyper/signatures/function_signature.py     117      3    97%
vyper/types/__init__.py                      1      0   100%
vyper/types/convert.py                      50      2    96%
vyper/types/types.py                       228      6    97%
vyper/utils.py                             101      5    95%
------------------------------------------------------------
TOTAL                                     3294    130    96%
Coverage HTML written to dir htmlcov


=========================================================================================== 857 passed in 216.19 seconds ===========================================================================================

Manually testing the new formatting options:

(vyper-venv) Highgarden:vyper kkom$ vyper -f abi_python examples/auctions/simple_open_auction.vy 
[{'name': '__init__', 'outputs': [], 'inputs': [{'type': 'address', 'name': '_beneficiary'}, {'type': 'uint256', 'name': '_bidding_time'}], 'constant': False, 'payable': False, 'type': 'constructor'}, {'name': 'bid', 'outputs': [], 'inputs': [], 'constant': False, 'payable': True, 'type': 'function', 'gas': 106241}, {'name': 'end_auction', 'outputs': [], 'inputs': [], 'constant': False, 'payable': False, 'type': 'function', 'gas': 71101}, {'name': 'beneficiary', 'outputs': [{'type': 'address', 'name': 'out'}], 'inputs': [], 'constant': True, 'payable': False, 'type': 'function', 'gas': 543}, {'name': 'auction_start', 'outputs': [{'type': 'uint256', 'name': 'out'}], 'inputs': [], 'constant': True, 'payable': False, 'type': 'function', 'gas': 573}, {'name': 'auction_end', 'outputs': [{'type': 'uint256', 'name': 'out'}], 'inputs': [], 'constant': True, 'payable': False, 'type': 'function', 'gas': 603}, {'name': 'highest_bidder', 'outputs': [{'type': 'address', 'name': 'out'}], 'inputs': [], 'constant': True, 'payable': False, 'type': 'function', 'gas': 633}, {'name': 'highest_bid', 'outputs': [{'type': 'uint256', 'name': 'out'}], 'inputs': [], 'constant': True, 'payable': False, 'type': 'function', 'gas': 663}, {'name': 'ended', 'outputs': [{'type': 'bool', 'name': 'out'}], 'inputs': [], 'constant': True, 'payable': False, 'type': 'function', 'gas': 693}]
(vyper-venv) Highgarden:vyper kkom$ vyper -f abi examples/auctions/simple_open_auction.vy 
[{"name": "__init__", "outputs": [], "inputs": [{"type": "address", "name": "_beneficiary"}, {"type": "uint256", "name": "_bidding_time"}], "constant": false, "payable": false, "type": "constructor"}, {"name": "bid", "outputs": [], "inputs": [], "constant": false, "payable": true, "type": "function", "gas": 106241}, {"name": "end_auction", "outputs": [], "inputs": [], "constant": false, "payable": false, "type": "function", "gas": 71101}, {"name": "beneficiary", "outputs": [{"type": "address", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 543}, {"name": "auction_start", "outputs": [{"type": "uint256", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 573}, {"name": "auction_end", "outputs": [{"type": "uint256", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 603}, {"name": "highest_bidder", "outputs": [{"type": "address", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 633}, {"name": "highest_bid", "outputs": [{"type": "uint256", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 663}, {"name": "ended", "outputs": [{"type": "bool", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 693}]
(vyper-venv) Highgarden:vyper kkom$ vyper -f json examples/auctions/simple_open_auction.vy 
[{"name": "__init__", "outputs": [], "inputs": [{"type": "address", "name": "_beneficiary"}, {"type": "uint256", "name": "_bidding_time"}], "constant": false, "payable": false, "type": "constructor"}, {"name": "bid", "outputs": [], "inputs": [], "constant": false, "payable": true, "type": "function", "gas": 106241}, {"name": "end_auction", "outputs": [], "inputs": [], "constant": false, "payable": false, "type": "function", "gas": 71101}, {"name": "beneficiary", "outputs": [{"type": "address", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 543}, {"name": "auction_start", "outputs": [{"type": "uint256", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 573}, {"name": "auction_end", "outputs": [{"type": "uint256", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 603}, {"name": "highest_bidder", "outputs": [{"type": "address", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 633}, {"name": "highest_bid", "outputs": [{"type": "uint256", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 663}, {"name": "ended", "outputs": [{"type": "bool", "name": "out"}], "inputs": [], "constant": true, "payable": false, "type": "function", "gas": 693}]

@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

  • Change the behaviour of -f abi to produce valid standard JSON ABI output
  • Introduce -f abi_python option for Python-formatted ABI output

- Cute Animal Picture

octopus

@@ -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
Copy link
Contributor Author

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?

Copy link
Contributor

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()
Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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).

@jacqueswww
Copy link
Contributor

@kkom the binary has no tests so this is 100%, just see the note on json.

@kkom
Copy link
Contributor Author

kkom commented Aug 15, 2018

@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!

@kkom kkom changed the title change the ABI formatting options to "abi" & "python_abi" ABI formatting: introduce "abi_python", change behaviour of "abi" Aug 15, 2018
@kkom kkom changed the title ABI formatting: introduce "abi_python", change behaviour of "abi" ABI formatting: change behaviour of "abi", introduce "abi_python" Aug 15, 2018
@jacqueswww jacqueswww merged commit b808042 into vyperlang:master Aug 22, 2018
@kkom kkom deleted the abi-output-commands branch August 26, 2018 15:37
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

2 participants