Implementing INVALID opcode #757
Conversation
execution/evm/vm.go
Outdated
| @@ -48,6 +48,7 @@ var ( | |||
| ErrDataStackUnderflow = errors.New("Data stack underflow") | |||
| ErrInvalidContract = errors.New("Invalid contract") | |||
| ErrNativeContractCodeCopy = errors.New("Tried to copy native contract code") | |||
| ErrInvalidOpcode = errors.New("Invalid opcode") | |||
There was a problem hiding this comment.
This has the potential to read as 'an opcode we don't know about', when it means 'abort the execution'. I'd be tempted to suggest: ErrExecutionAborted = errors.New("Execution aborted").
execution/evm/vm_test.go
Outdated
| 0x00, 0x00, 0x00, PUSH1, 0x00, MSTORE, PUSH1, 0x0E, PUSH1, 0x00, INVALID) | ||
|
|
||
| output, err := ourVm.Call(account1, account2, bytecode, []byte{}, 0, &gas) | ||
| assert.Error(t, err, "Expected invalid opcode error") |
There was a problem hiding this comment.
Let's assert.Equal(t, ErrInvalidOpcode, err) here seeing as we have declared error values.
|
Could you also try rebasing on develop I have a feeling that might be cause of integration test failures. |
| @@ -1078,8 +1082,8 @@ func (vm *VM) call(caller acm.Account, callee acm.MutableAccount, code, input [] | |||
| case STATICCALL, SHL, SHR, SAR, RETURNDATASIZE, RETURNDATACOPY: | |||
| return nil, fmt.Errorf("%s not yet implemented", op.Name()) | |||
| default: | |||
| vm.Debugf("(pc) %-3v Invalid opcode %X\n", pc, op) | |||
| return nil, fmt.Errorf("invalid opcode %X", op) | |||
| vm.Debugf("(pc) %-3v Unknown opcode %X\n", pc, op) | |||
There was a problem hiding this comment.
👍 I prefer 'unknown' here
Signed-off-by: smblucker <[email protected]>
|
@silasdavis Should I always be rebasing against develop? For the other opcodes, I have been rebasing against new-opcodes2 without any issues. |
|
I didn't realise your merge base was a feature branch here. I've re-merge-based this to develop and this is now good to go. We'll need to make sure we merge changes on newopcodes-* to develop. |
…code-issue601 Implementing INVALID opcode
These changes implement the invalid opcode; these changes attempt to resolve issue here:
#601