Implement evm_result_outcome

[axic]

This is not meant to be merged in the current form, rather merely to start a discussion.

[axic]

Probably this would need to be renamed if it is only used by evm_call_fn.

[axic]

static_assert(sizeof(evm_result) == 32, "evm_result is too big");

Is that assumption used in the generated code?

[chfast]

Can you explain why this is needed?

[axic]

This is the item from the todo list:

Consider extending evm_result with custom error codes and messages.

cpp-ethereum doesn't really care about any of this, because it chooses not to report it back to the user, but other implementations do have detailed information, mostly as part of the trace RPC call. And who knows, hopefully in the future transactionReceipt could contain some of it.

[chfast]

Having 32/64-bit error code, would that be enough?

[axic]

@chfast: the outcome is a 32bit error code 😃

[axic]

When the cpp-ethereum EVM interpreter is converted to this interface, perhaps it would like to keep the majority of its exceptions:

ETH_SIMPLE_EXCEPTION_VM(BadInstruction);
ETH_SIMPLE_EXCEPTION_VM(BadJumpDestination);
ETH_SIMPLE_EXCEPTION_VM(OutOfGas);
ETH_SIMPLE_EXCEPTION_VM(OutOfStack);
ETH_SIMPLE_EXCEPTION_VM(StackUnderflow);

Also probably a PC field would help.

[chfast]

My question was if only error code is enough.

My view on this is like that:

1. I like error code being in the result. That would simplify logic around gas_left. Previously I wanted to fit the result into 32 bytes, but we must give up on that. 64 bytes are fine.
2. Having additional message attached to the result is not so fine. If the message is dynamically created by a VM it will need to be feed by the user.
3. There should be a pair SUCCESS / FAILURE mandatory to implement, the rest is optional.
4. Adding additional information (like current PC) directly in EVM-C seems to be bad idea. I'd prefer any structure extendable by VM implementation.

[axic]

Previously I wanted to fit the result into 32 bytes, but we must give up on that. 64 bytes are fine.

Why does it need to be a multiple of 32 bytes?

Having additional message attached to the result is not so fine. If the message is dynamically created by a VM it will need to be feed by the user.

I think it is optional from both the VM and the VM consumer's (i.e. an ethereum client) point of view. Can be null.

There should be a pair SUCCESS / FAILURE mandatory to implement, the rest is optional.

So right now, SUCCESS means success, and anything else means failure. The only requirement for a VM is to use SUCCESS or not-SUCCESS. Now that could be cleaned up a bit, maybe with using code ranges or negative numbers:

    EVM_RESULT_SUCCESS = 0,
    EVM_RESULT_OUT_OF_GAS = -1,
    EVM_RESULT_BAD_INSTRUCTION = -2,
    EVM_RESULT_BAD_JUMP_DESTINATION = -3,
    EVM_RESULT_STACK_OVERFLOW = -4,
    EVM_RESULT_STACK_UNDERFLOW = -5,
    EVM_RESULT_EXCEPTION = -6

Do you expect any other positive outcome other than success?

Adding additional information (like current PC) directly in EVM-C seems to be bad idea. I'd prefer any structure extendable by VM implementation.

Do you have a proposal for that structure? I would like to implement provisions for that. Though I think PC is a fairly standard piece of information.

[axic]

@chfast in case of Hera, there could be more exceptions:

• INVALID_IMPORT (this has a reason text, listing the import name)
• INVALID_BYTECODE_FORMAT
• INVALID_BYTECODE_VERSION
• OUT_OF_BOUNDS_MEMORY_ACCESS

[chfast]

Let's do something like that:

EVM_SUCCESS = 0,
EVM_FAILURE = 1,  ///< Generic failure, no reason provided
EVM_OUT_OF_GAS = 2,
EVM_BAD_INSTRUCTION = 3,
...

Let's reserve negative values to implementation defined errors (like LLVM internal error, etc.)

I have not added additional prefixes besides EVM_ to enum values yet. No need for that so far.

[axic]

I have not added additional prefixes besides EVM_ to enum values yet. No need for that so far.

EVM_EXCEPTION would collide.

[chfast]

Let's have EVM_EXCEPTION or EVM_FAILURE here and EVM_CALL_EXCEPTION or EVM_CALL_FAILURE in call callback.

[axic]

@chfast renamed.

I'm wondering whether the last commit is the way to go (>EVM_FAILURE is failure) or introducing a simple macro/inline method would be better which says that an outcome is a failure.

Well this only matters if you anticipate multiple successful codes.

[axic]

Superseded by #83.