Skip to content

Check constancy when calling to external function#1480

Merged
jacqueswww merged 7 commits into
vyperlang:masterfrom
charles-cooper:external_constancy_check
Aug 24, 2019
Merged

Check constancy when calling to external function#1480
jacqueswww merged 7 commits into
vyperlang:masterfrom
charles-cooper:external_constancy_check

Conversation

@charles-cooper

Copy link
Copy Markdown
Member

What I did

Fix #1468 , #1377

How I did it

The compiler was not checking constancy of external calls inside of a constant context. However, it was correctly generating static calls, so calls to modifying functions would fail at runtime when the callee tried to modify state (per https://github.com/ethereum/EIPs/blob/d95612c8206b7e225bec5dc34c4b019c9531e44d/EIPS/eip-214.md). This adds a check when compiling external calls that they cannot be modifying within a constant context.

How to verify it

Description for the changelog

Add constancy check for external calls

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@jacqueswww jacqueswww left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add a test in /syntax/ and it's good to go.

Comment thread vyper/parser/external_call.py
@charles-cooper

Copy link
Copy Markdown
Member Author

Note to self: we decided at last meeting to add an assert_no_modify (or something) to the language and so this PR needs to be expanded to add that. Also I need to fix the tests

@charles-cooper

Copy link
Copy Markdown
Member Author

@jacqueswww should be ready to go

@charles-cooper

Copy link
Copy Markdown
Member Author

pinging @fubuloubu @davesque for review

@fubuloubu

Copy link
Copy Markdown
Contributor

Need to add some docs on assert_modifiable but I don't see why this cannot be merged now.

@jacqueswww jacqueswww left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@jacqueswww jacqueswww merged commit 40ee664 into vyperlang:master Aug 24, 2019
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.

Bug: can't assert result of external calls to modifiable functions

3 participants