Add EIP: Pragmatic stack manipulation tools #9501
Conversation
EIPS/pragmatic-swap-dup.md
Outdated
| instructions. Failure to follow this calling convention will result in an | ||
| out-of-gas error. If `N` is zero, fail with out-of-gas error. | ||
|
|
||
| `SWAPN` pops `N` from the stack and swaps the new top stack element with the |
There was a problem hiding this comment.
PUSH1 N SWAPN take 3 bytes while the intermediate could also be done here: SWAPN N which takes two bytes and does not have this extra check "previous opcode must be PUSH1". Why do we want the PUSH1 SWAPN order and not SWAPN N (thus the opcode is defined by two bytes?). This can essentially be added as "just another PUSH1 opcode" to the jumpdest analysis logic for the jumpdest validation.
There was a problem hiding this comment.
Because adding an immediate to legacy code has historically caused disagreement since it would alter the behavior of contracts already deployed.
EIPS/pragmatic-swap-dup.md
Outdated
| `EXCHANGE` pops `X` from the stack and will swap the stack element at index | ||
| `N-1` with the stack element at `M-1`. |
There was a problem hiding this comment.
| `EXCHANGE` pops `X` from the stack and will swap the stack element at index | |
| `N-1` with the stack element at `M-1`. | |
| `EXCHANGE` pops `X` from the stack and will swap the stack element at index `N-1` with the stack element at `M-1`. |
EIPS/pragmatic-swap-dup.md
Outdated
| Since `SWAP1` and `DUP1` operate on the top of the stack, it seems fitting that | ||
| `EXCHANGE(1, 2)` operate on the `top` and `top-1`. |
There was a problem hiding this comment.
| Since `SWAP1` and `DUP1` operate on the top of the stack, it seems fitting that | |
| `EXCHANGE(1, 2)` operate on the `top` and `top-1`. | |
| Since `SWAP1` and `DUP1` operate on the top of the stack, it seems fitting that `EXCHANGE(1, 2)` operate on the `top` and `top-1`. |
| Both operations take a single argument from the stack, `N`. This argument must | ||
| be provided by a `PUSH1` operation immediately preceding the `SWAPN` and `DUPN` | ||
| instructions. Failure to follow this calling convention will result in an | ||
| out-of-gas error. If `N` is zero, fail with out-of-gas error. |
There was a problem hiding this comment.
Wouldn't it be easier to view N after this number has been popped from the stack? This removes the check to see if 0 is being pushed, and it also allows for one extra stack item to be addressed. Maybe this is the reason that 0 is removed here though, it is to not allow PUSH0 DUPN/SWAPN?
There was a problem hiding this comment.
Ah ok, and banning N=0 also ensures we keep the same style of names, so DUP0/SWAP0 is not a thing. Makes sense to ban 0 (although for practical reasons could make sense also to allow N=0 and start from there)
|
|
||
| The `EXCHANGE` instruction takes a single argument from the stack `X` and | ||
| deconstructs it into two operands, `N` and `M`. `N` is `X >> 4` and `M` is | ||
| `X & 0x0F`. The argument `X` must be provided by a `PUSH2` operation immediately |
There was a problem hiding this comment.
If the PUSH2 and not PUSH1 is intended then this it should be written somewhere that N can have values 0-0x0fff, otherwise this might look like it should be PUSH1.
|
|
||
| ### Gas costs | ||
|
|
||
| All operations cost `3` gas. Preceding push operations are charged separately |
There was a problem hiding this comment.
This might be too low because there are checks for the previous operation being a PUSH1/PUSH2, bitwise operations on EXCHANGE, and the check that the PUSHd values are not 0, plus the actual stack operations.
|
let know when EIP is ready for review w.r.t. draft acceptance |
|
The commit 80c988c (as a parent of 6c3d554) contains errors. |
|
@g11tech g2g |
| ## Security Considerations | ||
|
|
||
| When verifying the preceding `PUSH` operations, client implementers must ensure | ||
| that the preceding bytes are not part of a longer segment of push data (e.g. |
There was a problem hiding this comment.
34 byte code look back; 3 gas seems very low?
|
|
||
| ## Abstract | ||
|
|
||
| Add `SWAP17`-`SWAP24`, `DUP17` - `DUP24`, `SWAPN`, `DUPN`, and `EXCHANGE` |
There was a problem hiding this comment.
Why burn 17 opcodes for the fixed values and add 2 dynamic values?
Adding a stack arg based approach for swap, dup, and exchange.