-
Notifications
You must be signed in to change notification settings - Fork 140
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
EVM: Fix data copying logic #1021
Comments
I think we might want to put the brakes for insane behaviour for the sake of spec compliance. I might as well have a sane interpreter that is not exactly EVM at the rough edges than go out of my way to support insane behaviour. Just my EUR0.02. |
We need to match the EVM, especially in cases like this, because people will likely be relying on this behavior. It's actually pretty convenient and important. E.g., if a contract reverts, it won't return anything, but the return value/offset still need to be correct. In general, we need to default to "whatever the EVM does". We can differ, but we need to know and understand why the EVM behaves the way it does first and need to understand the repercussions. Otherwise, https://en.wiktionary.org/wiki/Chesterton's_fence. |
Ok, so, some history: Bascially, this is considered a mistake, but there are at least some compilers relying on it for optimizations.
While serpent itself is deprecated, I'm not sure if we're in a position to make a judgment call here and change the behavior. |
Ok, so, I've been listening to the meeting. Relevant section https://youtu.be/aGeGvZ5uS8s?t=3785 It really sounds like nobody should actually use this behavior. This behavior also explains (cc @mriise) the insane right-pad behavior in precompiles. We'll discuss this in the huddle tomorrow, but I'm up for at least considering dropping this. If we did:
What I really don't want to do is to leave this in an inconsistent state. If we're going to enforce memory bounds, we need to be consistent. |
Ok, nope, we're not touching this: Yeah, that's the solidity compiler abusing the fact that calldatacopy returns infinite zeros after the calldata to zero memory. |
- CALL: truncates the return value, but doesn't zero fill. - CALLDATA, EXTCODECOPY, CODECOPY: treat input as if it were followed by infinite zeros. - RETURNDATACOPY: explicitly forbids out-of-bounds reads. We might be able to change the behavior of EXTCODECOPY and CODECOPY to match RETURNDATACOPY, but we _can't_ change CALLDATA as solidity abuses this feature for zeroing memory. So we're just going to match what the EVM does because it's safer (and because we need to implement that behavior _anyways_ for CALLDATA). fixes filecoin-project/ref-fvm#1021 fixes filecoin-project/ref-fvm#1024
- CALL: truncates the return value, but doesn't zero fill. - CALLDATA, EXTCODECOPY, CODECOPY: treat input as if it were followed by infinite zeros. - RETURNDATACOPY: explicitly forbids out-of-bounds reads. We might be able to change the behavior of EXTCODECOPY and CODECOPY to match RETURNDATACOPY, but we _can't_ change CALLDATA as solidity abuses this feature for zeroing memory. So we're just going to match what the EVM does because it's safer (and because we need to implement that behavior _anyways_ for CALLDATA). fixes filecoin-project/ref-fvm#1021 fixes filecoin-project/ref-fvm#1024
- CALL: truncates the return value, but doesn't zero fill. - CALLDATA, EXTCODECOPY, CODECOPY: treat input as if it were followed by infinite zeros. - RETURNDATACOPY: explicitly forbids out-of-bounds reads. We might be able to change the behavior of EXTCODECOPY and CODECOPY to match RETURNDATACOPY, but we _can't_ change CALLDATA as solidity abuses this feature for zeroing memory. So we're just going to match what the EVM does because it's safer (and because we need to implement that behavior _anyways_ for CALLDATA). fixes filecoin-project/ref-fvm#1021 fixes filecoin-project/ref-fvm#1024
The EVM generally treats memory as "infinite", limited only by gas.
E.g., if a user asks for bytes 5-15 out of a 10 byte return value, the EVM will:
If a user asks for, say, bytes starting at
2^200
, the EVM will not fail. It'll just return a bunch of zeros.We need to fix:
And probably quite a few other opcodes to follow this behavior.
The text was updated successfully, but these errors were encountered: