Refactor stack manipulation in evm #460
Conversation
There was a problem hiding this comment.
Could you explain the changes in the LOG opcode?
I am otherwise through and would actually give this a relatively quick go, all looks good and transparently traceable to me and all tests are passing.
Since changes here are pretty extensive I will leave this open a bit more for others to comment.
| @@ -158,7 +159,7 @@ module.exports = function (opts, cb) { | |||
| pc: runState.programCounter, | |||
| gasLeft: runState.gasLeft, | |||
| opcode: lookupOpInfo(opCode, true, self.emitFreeLogs), | |||
| stack: runState.stack, | |||
There was a problem hiding this comment.
Changed for backwards compatibility, ok.
| // check for stack underflows | ||
| if (runState.stack.length < opInfo.in) { | ||
| return cb(new VmError(ERROR.STACK_UNDERFLOW)) | ||
| } |
There was a problem hiding this comment.
This has been moved to the respective push method, ok.
|
|
||
| if ((runState.stack.length - opInfo.in + opInfo.out) > 1024) { | ||
| return cb(new VmError(ERROR.STACK_OVERFLOW)) | ||
| } |
| @@ -192,5 +192,5 @@ module.exports = function (op, full, freeLogs) { | |||
| } | |||
| } | |||
|
|
|||
| return {name: opcode, opcode: op, fee: fee, in: code[2], out: code[3], dynamic: code[4], async: code[5]} | |||
| return {name: opcode, opcode: op, fee: fee, dynamic: code[2], async: code[3]} | |||
| } | ||
|
|
||
| this._store.push(value) | ||
| } |
| @@ -272,9 +351,11 @@ module.exports = { | |||
| runState.memory.write(memOffset, dataLength, data) | |||
| }, | |||
| @@ -354,76 +445,90 @@ module.exports = { | |||
| runState.memory.write(memOffset, length, data) | |||
| }, | |||
| @@ -454,7 +559,8 @@ module.exports = { | |||
| }) | |||
| }) | |||
| }, | |||
| var topIndex = runState.stack.length - 1 | ||
| var tmp = runState.stack[topIndex] | ||
| runState.stack[topIndex] = runState.stack[swapIndex] | ||
| runState.stack[swapIndex] = tmp |
There was a problem hiding this comment.
These two have dedicated functions in the new stack class, ok.
| @@ -1091,7 +1199,8 @@ function makeCall (runState, callOptions, localOpts, cb) { | |||
| runState.contract.nonce = new BN(runState.contract.nonce).subn(1) | |||
| } | |||
|
|
|||
| cb(null, new BN(results.vm.exception)) | |||
| runState.stack.push(new BN(results.vm.exception)) | |||
| cb(null) | |||
| } | |||
| } | |||
| } | |||
|
Previously, it was specified in the opcodes file, how many items should be popped off the stack given each variant of |
This PR introduces a new
Stackclass for the evm, and moves all the boundary-checking logic, anddupandswapmethods to it. it also removes the number of inputs off stack, and results on stack from the opcodes list as they're no longer necessary.