Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Generic call' to return bytes instead of boolean #434

Closed
axic opened this issue Mar 18, 2016 · 12 comments
Closed

'Generic call' to return bytes instead of boolean #434

axic opened this issue Mar 18, 2016 · 12 comments
Assignees
Labels
breaking change ⚠️ language design :rage4: Any changes to the language, e.g. new features

Comments

@axic
Copy link
Member

axic commented Mar 18, 2016

Currently the call() with a method signature and parameters returns a boolean stating the success of execution (true if successful).

I think it would make more sense to return (or to have a version of call which does) the raw ABI bytes.

@chriseth
Copy link
Contributor

The reason call does not return the raw ABI bytes is because there is no way in the EVM to find out how much the called contract actually returned - in fact the caller has to "guess" or "know" that number and reserve the memory in advance.

With inline assembly, you can implement a version of call that returns a given number of bytes.

@axic
Copy link
Member Author

axic commented Mar 29, 2016

Well actually there's a way you have proposed https://github.com/ethereum/EIPs/blob/master/EIPS/eip-5.md - would it make sense having this open until that is implemented?

@chriseth
Copy link
Contributor

chriseth commented Aug 5, 2016

Question: should we re-throw on failure or somehow report the failure condition (e.g. by returning (bytes, bool))?

@axic
Copy link
Member Author

axic commented Jul 9, 2017

This is now possible with returndatasize/returndatacopy.

I am not entirely sure what would be the best method, but perhaps a tuple (bool, bytes) would be the best for the next breaking release and once multiple return values are supported by CALL (in order to distinguish between revert) I'd consider introducing either (uint, bytes) or (enum, bytes).

@clesaege
Copy link

clesaege commented Jan 1, 2018

I had opened #3368. But let's move the discussion there as it seems quite related.

I think we must make this change non-breaking by creating a new function making a raw call.
The way I see it working would be
var (x, y) = callReturn[uint, string](bytes4(keccak256("fun(uint256)")), 5778));

We could also reserve the first return to indicate if the call succeeded, so it could be used like
var (succ, x, y) = callReturn[uint, string](bytes4(keccak256("fun(uint256)")), 5778));
Where succ is a boolean being true in case of success.

@chriseth
Copy link
Contributor

chriseth commented Mar 1, 2018

@axic do we want to have this for 0.5.0?

@axic
Copy link
Member Author

axic commented Mar 2, 2018

I'd be happy but it may be too many changes (from the users' perspective).

Also it will be more useful once the abi.decode helper is introduced, which we haven't even started yet, so realistically I'd say to postpone it to 0.6.0 assuming it will come strictly <= 6 months after 0.5.0 :)

@chriseth
Copy link
Contributor

chriseth commented Mar 5, 2018

I agree. In addition to that, we wanted to deprecate the use of low-level features that do not go all the way down the rabbit hole. So I think a generic abi.decode in connection with the call via inline assembly should be the way to go.

@kosecki123
Copy link

Given abi.decode and abi.encode availability, what would be very useful is something like

callReturn(bytes) returns (bool, bytes)

input bytes would be encoded with abi.encode
output bool is to signal failure
output bytes are returned abi encoded bytes ready to be decoded with abi.decode

@axic axic added the language design :rage4: Any changes to the language, e.g. new features label Jul 28, 2018
@axic
Copy link
Member Author

axic commented Jul 28, 2018

I think we must make this change non-breaking by creating a new function making a raw call.

The original issue statement should be extended that this should apply to all low-level calls: call, delegatecall and staticcall.

In that case we'd need to come up with 3 new names, which seems a bit too much namespace pollution.

@chriseth
Copy link
Contributor

Since staticall is close to useless without access to the return value, and since we now have abi.decode and the change in semantics of call, we might also take this in for 0.5.0.

@ekpyron
Copy link
Member

ekpyron commented Aug 15, 2018

I vote for including it in 0.5.0.

@ekpyron ekpyron self-assigned this Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ language design :rage4: Any changes to the language, e.g. new features
Projects
None yet
Development

No branches or pull requests

5 participants