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

Fix raw call outsize #1977

Merged
merged 4 commits into from
May 17, 2020

Conversation

iamdefinitelyahuman
Copy link
Contributor

@iamdefinitelyahuman iamdefinitelyahuman commented May 17, 2020

What I did

Do not return empty calldata in raw_call. Fixes #1976

How I did it

Compare RETURNDATASIZE to outsize and copy the smaller number of bytes to memory.

How to verify it

Run tests. I added / updated some cases. I also had to adjust some failing ones in test_wallet where the spirit of the test was that the call should succeed, but there was a check that it returned a non-empty value. Because of this change the value is now empty.

Cute Animal Picture

image

@codecov-io
Copy link

codecov-io commented May 17, 2020

Codecov Report

Merging #1977 into master will increase coverage by 0.58%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1977      +/-   ##
==========================================
+ Coverage   86.58%   87.17%   +0.58%     
==========================================
  Files          58       58              
  Lines        6636     6637       +1     
  Branches     1677     1677              
==========================================
+ Hits         5746     5786      +40     
+ Misses        569      536      -33     
+ Partials      321      315       -6     
Impacted Files Coverage Δ
vyper/functions/functions.py 92.34% <100.00%> (+5.06%) ⬆️
vyper/ast/nodes.py 94.58% <0.00%> (+1.73%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7498013...d06ef9b. Read the comment docs.

tests/parser/functions/test_raw_call.py Outdated Show resolved Hide resolved
@fubuloubu
Copy link
Member

@charles-cooper could you take a look at the proposed change and let us know if there's something we're missing about how raw_call works?

@charles-cooper
Copy link
Member

I don't really get the purpose of the proposed change.

@charles-cooper
Copy link
Member

Ok I think I understand it now. It's important for variable length return data. I think it is a good change, but using the name outsize is confusing because it suggests the length of the output will be that size. We could rename it to bufsize or max_outsize.

@iamdefinitelyahuman
Copy link
Contributor Author

I like max_outsize, it's more more clear imo.

@charles-cooper
Copy link
Member

Can't figure out how to approve from mobile. But, approved on my end.

@fubuloubu fubuloubu merged commit 1ebce95 into vyperlang:master May 17, 2020
@iamdefinitelyahuman iamdefinitelyahuman deleted the fix-raw_call-outsize branch May 19, 2020 17:09
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.

raw_call returns empty calldata
4 participants