Add msg.data environment variable VIP-1181

[skellet0r]

What I did

As per #1181, I added the msg.data environment variable, allowing access to the complete calldata within external functions.

Also updated the docs to include information on the syntax/usage of msg.data.

How I did it

1. Defined the data member in the msg environment variable - vyper/context/environment.py
2. Modified the StatementAnnotationVisitor to attach metadata to the msg.data node depending on usage - vyper/context/validation/annotation.py
3. Added the msg.data branch in the parse_Attribute fn, to return the appropriate LLLNode based on whether usage is in len or slice - vyper/parser/expr.py
4. Modified code validation to throw a StateAccessViolation when msg.data is used inside an internal function - vyper/context/validation/local.py
5. Modified code validation to throw a SyntaxException when msg.data is used outside of len or slice

Similar to before this copies the calldata to an internal variable before it gets sliced, I'm sure in the future we can work on cacheing msg.data but that's a whole 'nother beast. The use of len(msg.data) simply just used the calldatasize opcode and isn't as bad I hope.

How to verify it

I've added a few tests, which can be found in tests/parser/syntax/test_msg_data.py.

Description for the changelog

Added msg.data syntax allowing access to full calldata in external functions

Cute Animal Picture

Closes #1181

[fubuloubu]

I was thinking the implementation would be slightly different, more like this:

[skellet0r]

Ahhh @fubuloubu I like that syntax/implementation better, since it looks like one is explicitly extracting a piece of the calldata, instead of the syntax I used which looks like indexing a reference to calldata. I'll get to work on adjusting 👍

[codecov-io]

Codecov Report

Merging #2343 (806a4c0) into master (84be2b5) will increase coverage by 0.03%.
The diff coverage is 96.66%.

❗ Current head 806a4c0 differs from pull request most recent head 9cf8d76. Consider uploading reports for the commit 9cf8d76 to get more accurate results

@@            Coverage Diff             @@
##           master    #2343      +/-   ##
==========================================
+ Coverage   85.73%   85.77%   +0.03%     
==========================================
  Files          90       90              
  Lines        8765     8795      +30     
  Branches     2091     2099       +8     
==========================================
+ Hits         7515     7544      +29     
  Misses        760      760              
- Partials      490      491       +1     

Impacted Files	Coverage Δ
vyper/context/validation/annotation.py	92.37% <85.71%> (-0.42%)	⬇️
vyper/context/environment.py	100.00% <100.00%> (ø)
vyper/context/validation/local.py	90.76% <100.00%> (+0.36%)	⬆️
vyper/parser/expr.py	77.29% <100.00%> (+0.54%)	⬆️

---

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 84be2b5...9cf8d76. Read the comment docs.

[skellet0r]

Sorry for the force push ... not sure if that's accepted around these parts 👀
I didn't implement the compile-time bounds checking as you commented @fubuloubu, since there are times in which calldatasize will exceed the compile-time expected size. EIP-2771 for example. I also totally missed previously handling len(msg.data) so that's included now, as well as the right syntax slice(msg.data, x, k).

[fubuloubu]

Sorry for the force push ... not sure if that's accepted around these parts

No worries, works for me!

I didn't implement the compile-time bounds checking as you commented @fubuloubu, since there are times in which calldatasize will exceed the compile-time expected size. EIP-2771 for example. I also totally missed previously handling len(msg.data) so that's included now, as well as the right syntax slice(msg.data, x, k).

This should be okay! I think if you try to pull calldata longer than CALLDATASIZE the EVM's behavior is to give you empty data. So, it should be safe. Also, it is okay for CALLDATASIZE to exceed the expected size, the check was that it is at least the expected size to make sure we don't run into empty values unexpectedly (leading to difficult-to-debug behavior).

As such, this PR is a great addition! Thank you!

[skellet0r]

Also, it is okay for CALLDATASIZE to exceed the expected size, the check was that it is at least the expected size to make sure we don't run into empty values unexpectedly.

ahh @fubuloubu I see what you mean, isn't that handled already though? Indirectly through vyper/parser/arg_clamps.py? I guess if you call a function and give it calldata with a length < EXPECTED_CALLDATASIZE, the arg clamps it's effectively just zero those args to the default values for the type?

I think if you try to pull calldata longer than CALLDATASIZE the EVM's behavior is to give you empty data. So, it should be safe

Should I remove that runtime bounds check then? Since like you said, if you slice past the bound of CALLDATASIZE you'll just get null bytes appended to the end. Could just leave as it is, and if any complaints remove the runtime bounds check.

[fubuloubu]

Should I remove that runtime bounds check then?

I prefer to have it. For some reason I misread and thought you hadn't implemented it, but I see now that you did!

[skellet0r]

Awesome! The checks just have to run through one last time and should be good then 🤞

[skellet0r]

What happens if I assign a slice of msg.data to a storage variable?

@iamdefinitelyahuman that's a good test case I didn't include, I added it in, looks like that works as expected, along with the memory position advancement, and assignment to bytes32 throwing a TypeMismatch exception.

[iamdefinitelyahuman]

Great job!

[fubuloubu]

lgtm

[mahsaC28]

does anyone know how can I do this without use msg.data???