-
Notifications
You must be signed in to change notification settings - Fork 19
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
Document code section type malleability #176
Comments
The malleability only goes as far as the particular RETF section in question, when seen in isolation. In order for the container to validate the callers of such section need to be different (satisfy the 0-127 stack items when calling). Does this observation make it less of an issue? |
Yes, these are clearly different functions from the point of view of stack validation. Even for RETF in isolation, one case requires 0 values on stack, and another one 127 values.
How would validation figure out that this is "artificially increased" numbers and not function legitimately taking N inputs, doing some transformations on them and returning N outputs? |
The main argument is still that no sane compiler/user would diverge from using the "minimal" type. And it goes along other similar restrictions: no unused code sections, no unreferenced subcontainers, etc... The validation rule seems easy to implement: you have to track if any of the instructions reached the stack height of 0. The prototype implementation with some EOF validation tests corrected: ethereum/evmone#1122. The EEST failures caused by the change:
You can note that all the failing/updated tests have |
Some observations: It means for legitimately unused arguments (e.g. callback implementation not caring about some of arguments) compilers will have to insert additional POP instructions. This goes a bit against the relaxed handling of terminating instructions, where no extra POPs are needed. Actually one SWAPN/DUPN/EXCHANGE would be sufficient instead of series of POPs, that's what I would probably do in some of those faling tests. |
If there are legitimate use cases for it, I'm happy to take it back. I guess you indicated it already: function pointer, virtual method? However, they are not directly implementable in EOFv1. |
I think it can be like compile-time polymorphism: you link with some library code, that requires callback with certain name and signature to be implemented? |
The callback would still be expected to clear the stack of those arguments so it would naturally already have to POP them. Hm, I guess that's not true if it's a non-returning callback... What is the concrete issue caused by malleability? |
I've tried to find a good use case justifying not adding this validation rule. I explored the function pointers in solidity because these feature enforces a specific function signature on a group of functions. contract Storage {
function fa(uint256 arg) private pure returns(uint256) {
return arg;
}
function fb(uint256) private pure returns(uint256) {
return 0;
}
function test(bool cond, uint256 x) external pure returns(uint256) {
function(uint256) pure returns(uint256) ptr = fa;
if (cond)
ptr = fb;
return ptr(x);
}
} In the example we have two functions At first, the best implementation of such function seems to be a code section of type However, you can notice that in EOF there is no "indirect call" instruction (an instruction calling a function by taking its index from the stack). So the implementation of the function pointer invocation must be implemented by In other words, solidity function type don't have to match the EOF function type. |
Great example. The argument changes a little with #134 though, right? The code section type would be |
I might be wrong, but I think the argument is the same. The "best implementation" would be |
I talked with Solidity and they convinced me not to do this. Although this is technically doable in the compiler it has some level of annoyance.
This is very nice observation about |
There was some more discussion after the comments quoted above so I'll clarify that our stance on this is actually ambivalent. We later realized that this would be much simpler to satisfy than we assumed at first. I.e. it's just a matter of setting the right input/output number in the header and does not require altering the code at all (neither the code section nor the call site; no On the other hand, the points quoted by @chfast still stand, so it does have some minor downsides too. We don't really have a strong opinion one way or the other. BTW, all of this assumes that we're talking only about the case of returning functions. For non-returning ones it does not seem like a good idea, but @chfast said that that idea was dropped. |
will be resolved in ethereum/EIPs#9382 |
Having a returning code section, e.g.
RETF
, its natural type isinputs=0, output=0, max_stack_height=0
. However, you can artificially increase these numbers, e.g.inputs=127, output=127, max_stack_height=127
, and still have a valid type (with some additional runtime limitation for the caller).This this in the spec by requiring the values to be minimal.
The text was updated successfully, but these errors were encountered: