EIP-2315: update jumpsub gascost and add back testcases #2669
Conversation
| | 4 | RETURNSUB | 2 | [] | [ 8] | | ||
| | 9 | STOP | 0 | [] | [] | | ||
|
|
||
| Consumed gas: `26` |
There was a problem hiding this comment.
It was because it executed a BEGINSUB. See https://github.com/ethereum/EIPs/pull/2656/files#diff-4568ebb9120e779b93e457d3926c14d6L152 . The changes made it not do the BEGINSUB. And after that commit, Greg did some changes and it got reverted back to 26 again.
There was a problem hiding this comment.
@holiman Is this 2 gas cost for RETURNSUB ok? In the specification says VeryLow that AFAIK is 3
There was a problem hiding this comment.
Oh my god. Yeah 2 is base.
I don't see why RETURNSUB should be so cheap. I'd actually prefer it to be same as JUMPSUB -- or, more specifically, that cost of(JUMPSUB+RETURNSUB) == cost of (JUMP + JUMP). Which currently would put it at mid == 8
There was a problem hiding this comment.
JUMPSUB inserts into returnstack, whereas RETURNSUB pops one. The popped value does not need to be validated (but a length-check is needed on the returnstack)
There was a problem hiding this comment.
Yes, what I meant is that JUMPSUB inserts into the return stack (needs to check for stack overflow) and RETUNRSUB needs to pop from the stack (needs to check for stack underflow).
Neither of them validate the content nor does JUMP, but JUMP is not growing any stacks.
There was a problem hiding this comment.
How about moving it here: https://ethereum-magicians.org/t/eip-2315-simple-subroutines-for-the-evm/3941/64
There was a problem hiding this comment.
Right. I'd compare JUMPSUB with JUMPI. JUMPI pops an item from a stack, does a check, then a jump. It costs 10.
So one might argue that both JUMPSUB and RETURNSUB should be 10. But having RETURNSUB at 8 would also be sensible, IMO. It might be good to not make subroutines too expensive, so that optimizers replace it all with jumps in the end anyway... Actually, since RETURNSUB does not have to validate the destination (check the opcode at destination + check if destination is code), it could be lower, so the two together are 16.
There was a problem hiding this comment.
Right, I was in the mindset JUMPSUB analysis happens prior to execution, but strictly looking at the spec, it is not. Following that logic indeed RETURNSUB should be cheaper as it does no validation.
However even if both would be set at 10 and thus higher than JUMP, they would still be cheaper given the return address doesn't need to be pushed and no need for stack rotation if there are return values, which is the case for current "subroutines via jump".
This PR
BEGINSUBwas present,JUMPSUBto8.I'm marking this as draft since otherwise the trigger-happy autobot will merge it. Can I please get a thumb up from:
@gcolvin , @ordian (or @sorpaas , or @vorot93 ?), @makt
Just want to make sure everyone is on the same page here...