EVM-C: better error reporting in evm_call_fn

[axic]

We could consider how much of the evm_result / evm_result_code should/could be shared with evm_call_fn.

• Check against EIP 5: Gas costs for return values EIPs#8.
• Move the result_release() function to evm_result (as a pointer to function) to allow VM compositing and results from different VMs. It will work like C++ virtual destructor.
• Use evm_result as the output of evm_call_fn().

[chfast]

I can propose to extend the return value of evm_call_fn to pair of result code and gas left. The output is passed differently so not point to having it there.

The error codes would be: "call depth limit reached", "insufficient funds" + codes we have in evm_result_code already.

However, I'm not convinced that would provide any addition value to the VM. What is the VM supposed to do with this error code? The user already knows about it because it provides it. Moreover, calls are in general a big mess, because it is not clear who (the VM or the user) is responsible to check which call preconditions.

[axic]

I can propose to extend the return value of evm_call_fn to pair of result code and gas left.

I think this would be a good trade off as a start.

However, I'm not convinced that would provide any addition value to the VM. What is the VM supposed to do with this error code?

In eWASM it is possible to return these error codes back to the contract.

[chfast]

Can't you use output for passing errors?

[axic]

I would rather not make it any more messy than it already is.

[chfast]

I was thinking a bit more about this issue.

How about returning evm_result and leaving output copy to the VM instead of the user / the API wrapper. That would be very consistent with the evm_execute.

However, there is one drawback: evm_result points external resources and they must be released. There is no problem if the evm_result comes from the same VM, but it can also be produced by other VM implementation.

[chfast]

I will need to proceed with this one now, because it turned out the current design might be not enough.

What do you think about putting the pointer to result_release() function in the evm_result struct itself. The releasing would look like:

evm_result result = ...
result.release(&result);

It will work like virtual destructor in C++.

[axic]

@chfast should be fine.

Does it needs to be specific for call vs. tx and single release cannot be reused?

[chfast]

Single release will not work if you want to compose VMs. E.g. in one VM you have a call result coming from some other VM.

[axic]

@chfast I see, it's fine having it in the struct.

[chfast]

How the evm_result should work for CREATE?

[chfast]

Returning evm_result should be perfect for ethereum/EIPs#8.

[axic]

How the evm_result should work for CREATE?

This was added 5936db8.

@chfast is anything left to be done on this issue?

[chfast]

After this change Python gave up on this struct :). And there are a lots of issues in go. But last night I had an idea to split the CREATE to "code eval" and "create account" parts. It might work.

[axic]

Are they having an issue with unions? Wouldn't a simple solution like removing the union and just having those 3 as members of the result struct work?

[chfast]

It would, but I'm trying to keep the structs within 64 bytes to fit in the cache line.

[axic]

Since create_address is ultimately uint8_t bytes[20]; you could have a simple helper for consumers of evm.h to convert output_data to create_address.

[chfast]

I'm about to make the last change in the EVM-C by changing

int64_t evm_call_fn(struct evm_env* env, const struct evm_message* msg, uint8_t* output, size_t output_size);

to

void evm_call_fn(struct evm_result* result, struct evm_env* env, const struct evm_message* msg);

In case of CREATE, the host application should allocate 20 bytes of dynamic memory to pass the address of the created account.

But I need #112 first.

[chfast]

This is addressed in #118.
But it creates a little weird combination of status codes (SUCCESS, FAILURE, REVERT) and output data. Sometimes output comes from EVM spec, sometimes it contains additional error message from an EVM implementation. So handling the result struct becomes tricky because you have to decide what kinds of outputs are for end users and what for EVM subcalls.

[axic]

@chfast I think we could consider this finished now?