Add abi.encode and abi.encodePacked #2980

axic · 2017-09-27T12:17:11Z

Fixes #1707. Continuation of #2851 (managed to overwrite that branch with something else)

Add the global abi with the following four functions:

abi.encode(...) -> bytes
abi.encodePacked(...) -> bytes
abi.encodeWithSelector(bytes4, ...) -> bytes
abi.encodeWithSignature(string, ...) -> bytes

abi.encode ABI-encodes the provided arguments according to their types. abi.encodePacked performs packed encoding. abi.encodeWithSelector(a, x, ...) acts like abi.encode(x, ...) but prepends the four bytes in a. abi.encodeWithSignature(a, x, ...) is identical to abi.encode(bytes4(keccak256(a)), x, ...).

TODO:

test that the selector functions properly type check the first parameter
test encoding interleaved with other memory operations
test that encoding functions are non-pure if they arguments are non-pure

axic · 2017-09-27T12:17:29Z

Needs tests with more types.

axic · 2017-09-27T12:36:09Z

test/libsolidity/SolidityEndToEndTest.cpp

+			}
+			function g(uint8 a) {
+				bytes request = abi.encode(
+					bytes4(keccak256(abi.encodePacked("f(uint8,string)"))),


This made me realise we could offer abi.encodeSelector(string) as a shorthand for this. Though having it a library seems better, too bad abi cannot be extended by libraries (yet).

Yes, we do want that, but perhaps encodeWithSelector

There are two options. I was going with:

abi.encode( abi.encodeSelector("f(uint8,string)"), a, "He" );

Otherwise we can have three functions with varargs:

abi.encode

abi.encodeWithSelector (should enforce the first parameter being a string type ...)

abi.encodePacked

Unfortunately it does need the third version, otherwise it would be packed.

This means for encodeWithSelector we need to enforce the first parameter to be string (to be hashed) or bytes4 (already hashed).

chriseth · 2017-09-27T12:37:58Z

test/libsolidity/SolidityEndToEndTest.cpp

+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+	ABI_CHECK(callContractFunction("g(uin8)"), encodeArgs(u256(42)));


Missing a t in uint8

Also the encodeArgs call has to be moved to an argument to callContractFunction, or, even shorter:
ABI_CHECK(callContractFunction("g(uint8)", 42), bytes());

axic · 2017-09-27T13:41:41Z

Need to add docs.

axic · 2017-11-29T23:03:48Z

The current API is as follows:

abi.encode() -> bytes
abi.encodePacked() -> bytes
abi.encodeWithSelector(, ) -> bytes

The reason for encodeWithSelector is to have an (memory-)optimised version of manually concatenating encodePacked("f(uint8,string)") .. encode(uint8(1), string("hello world")).

axic · 2017-12-12T04:56:40Z

@chriseth do you agree with API proposed above? If so I should finish this PR.

chriseth · 2017-12-12T09:36:25Z

Yes, fine with the API. For encodeWithSelector, we should also allow users to pass the 4-bytes-selector.

axic · 2017-12-12T10:13:11Z

The (so far) final API we have agreed on:

abi.encode(...) -> bytes
abi.encodePacked(...) -> bytes
abi.encodeWithSelector(bytes4, ...) -> bytes
abi.encodeWithSelector(string, ...) -> bytes

Plus raise a error if encodeWithSelector's first argument is a literal which can be implicitly converted to both bytes4 and string (it is inconclusive).

protinam · 2018-03-02T06:54:01Z

I would find this extremely useful; at the moment the combination of Solidity's stack size limitation and the lack of manual packed encoding is leading to a lot of inefficiencies in the Wyvern Exchange's contracts, - e.g. https://github.com/ProjectWyvern/wyvern-ethereum/blob/b31d60ae58ff251c236f9f95b499b24f26a4a78c/contracts/exchange/ExchangeCore.sol#L223

Anything I can help with to move this along?

chriseth · 2018-03-02T10:03:50Z

@axic what is still to be done here?

chriseth · 2018-03-16T18:28:36Z

Let's get this done!

chriseth · 2018-03-17T12:58:48Z

Rebased.

VoR0220 · 2018-03-17T20:53:21Z

Correct me if I'm wrong, but would this be serializing the bytes into the proper format and have an implicit "require" if the data cannot be formatted into the desired function signature?

chriseth · 2018-03-19T12:38:23Z

@VoR0220 the format is specified by the type of the arguments. There are some types that cannot be serialized (mappings are an example) in which case you would get a compile-time error.

…deWithSignature.

chriseth · 2018-04-11T22:12:35Z

@axic

Plus raise a error if encodeWithSelector's first argument is a literal which can be implicitly converted to both bytes4 and string (it is inconclusive).

Do we have a test case for this?

This is not relevant anymore, because we split the single overloaded function into two different functions.

chriseth · 2018-04-11T22:13:26Z

Fixed everything.

axic · 2018-04-12T14:03:26Z

test/libsolidity/syntaxTests/tight_packing_literals_fine.sol

+        return keccak256(uint8(1));
+    }
+    function g() pure public returns (bytes) {
+        return abi.encode(1);


This isn't tight packing? Should instead have the same 5 methods as tight_packing_literals.sol

The idea was that these are the "good" cases that are variations of tight_pcaking_literals.sol. Perhaps the naming of the file is bad.

axic · 2018-04-12T14:04:49Z

I am sorry its still missing a test for struct encoding with abi.*.

I'd just do keccak256(abi.encode(s)), keccak256(abi.encodeWithSelector(0x12345678, s)), keccak256(abi.encodeWithSignature("f((uint,uint))",s)) and a failure case for abi.encodePacked(s) where s is a struct.

axic · 2018-04-12T14:08:48Z

This is not relevant anymore, because we split the single overloaded function into two different functions.

Ah right, that's clear then 😉

chriseth · 2018-04-12T14:17:28Z

@axic are you talking about syntax or semantics tests for encoding structs?

axic · 2018-04-12T14:17:47Z

encodePacked(s) is a syntax, the rest is semantics

chriseth · 2018-04-12T14:18:14Z

Ok! I did not add semantics tests because they should be the same as the regular encoder tests, but I will add some.

chriseth · 2018-04-12T14:18:53Z

syntax tests for encodePacked is in specialFunctions/abi_encode_structs.sol and specialFunctions/abi_encode_structs_abiv2.sol.

axic · 2018-04-12T14:20:10Z

syntax tests for encodePacked is in specialFunctions/abi_encode_structs.sol and specialFunctions/abi_encode_structs_abiv2.sol.

Sorry missed that, that should be fine.

Only semantics missing then.

chriseth · 2018-04-12T14:20:13Z

Hm, and abi.encode for structs also has a semantics test. I will add structs to the encodeWithSelector and encodeWithSignature tests.

axic · 2018-04-12T14:21:34Z

It has a couple, but couldn't find any for structs.

axic · 2018-04-12T14:42:06Z

test/libsolidity/SolidityEndToEndTest.cpp

+	expectation =
+		encodeArgs(0x20, 4 + 0x120) +
+		bytes{0x7c, 0x79, 0x30, 0x02} +
+		encodeArgs(u256(-1), 0x60, u256(3), 0x1234567, 0x60, 0x1234, 38, "Lorem ipsum dolor sit ethereum........") +


I think 0x1234567 is missing the last nibble 8.

that is s.a, whose value is 0x1234567.

Ah right, in this test case x is a stray value.

oh yeah, do you want me to change it?

If you want. It would be cleaner but I'm not adamant. Actually, lets keep it, once we have detection for side-effect free statements it will be detected :)

axic · 2018-04-12T14:42:34Z

test/libsolidity/SolidityEndToEndTest.cpp

+	expectation =
+		encodeArgs(0x20, 4 + 0x120) +
+		bytes{0x12, 0x34, 0x56, 0x78} +
+		encodeArgs(u256(-1), 0x60, u256(3), 0x1234567, 0x60, 0x1234, 38, "Lorem ipsum dolor sit ethereum........") +


axic · 2018-04-12T15:24:27Z

@chriseth it looks good to me. I cannot approve since I am the one opening the PR. I trust the ABI expectations are correct, but couldn't review all of them manually.

ProphetDaniel · 2018-04-12T22:30:39Z

I'm working with ERC827 standard (ethereum/EIPs#827) and there is an interesting functionality I need to implement that depends on call method with a parameter.

transfer( _to, _value, bytes4(keccak256("setCounter()")).toBytes());
// The following doesn't work out.
// transfer( _to, _value, bytes4(keccak256("setCounter(uint16)"),_color).toBytes());

where toBytes() comes from the following StringsAndBytes.sol library:

library StringsAndBytes{
    /* bytes32 (fixed-size array) to bytes (dynamically-sized array) */
    function toBytes(bytes4 _bytes4) internal pure returns (bytes){
        // string memory str = string(_bytes32);
        // TypeError: Explicit type conversion not allowed from "bytes32" to "string storage pointer"
        bytes memory bytesArray = new bytes(4);
        for (uint256 i; i < 4; i++) {
            bytesArray[i] = _bytes4[i];
        }
        return bytesArray;
    }//
}

And the token contract is:

pragma solidity ^0.4.21;
import "ERC827Token.sol";
import "StringsAndBytes.sol";

contract SpecialToken is ERC827Token{
    using StringsAndBytes for bytes4;
    // Name of the token
    string constant public name = "SpecialToken";
    // Token abbreviation
    string constant public symbol = "SpT";
    // Decimal places
    uint8 constant public decimals = 18;

    function transferThis(address _to, uint256 _value, uint16 _color) {
        transfer( _to, _value, bytes4(keccak256("setCounter()")).toBytes()); // works
        _to.call(bytes4(keccak256("setCounter(uint16)")), 4); // works
        // transfer( _to, _value, bytes4(keccak256("setCounter(uint16)"),_color).toBytes()); // does not work
    }
}
contract ColoredCoin{
    event coinColor (uint16 color);

    function setCounter(){
        emit coinColor(3);
    }
    function setCounter(uint16 _color){
        emit coinColor(_color);
    }
}

Should I just await abi.encodeWithSignature or is there an alternative implementation that I can already start using?

axic · 2018-04-13T01:10:51Z

I think this PR isn't the proper location to discuss what alternatives are for a given programming task in Solidity. This feature should be part of the next release, which is out hopefully very soon (days?). However one must note, that it can easily take weeks for the frameworks (such as truffle) to pick a new version up.

axic force-pushed the abi-api branch 3 times, most recently from c8c8553 to 6ad5c08 Compare September 27, 2017 12:34

axic commented Sep 27, 2017

View reviewed changes

chriseth reviewed Sep 27, 2017

View reviewed changes

axic force-pushed the abi-api branch 4 times, most recently from efa36c1 to c86fcc9 Compare September 27, 2017 12:52

axic mentioned this pull request Oct 1, 2017

[WIP] Prevent block.blockhash from being used without calling it #2995

Closed

axic force-pushed the abi-api branch from cc7396a to 23dd59e Compare October 5, 2017 11:35

axic force-pushed the abi-api branch from 23dd59e to 0293db6 Compare October 18, 2017 11:10

LogvinovLeon mentioned this pull request Nov 30, 2017

[WIP] Add eth_signTypedData as a standard for machine-verifiable and human-readable typed data signing with Ethereum keys ethereum/EIPs#712

Merged

protinam mentioned this pull request Mar 5, 2018

Manually pack Order struct to bytes to avoid unnecessary keccak256 ProjectWyvern/wyvern-ethereum#9

Closed

chriseth force-pushed the abi-api branch from 0293db6 to f504cf2 Compare March 17, 2018 12:58

axic and others added 4 commits April 11, 2018 22:32

Add abi.encode, abi.encodePacked, abi.encodeWithSelector and abi.enco…

d56acb6

…deWithSignature.

Tests.

44c0d7c

Tests for view and pure.

c4a6a63

Check partial function parameters if rest is arbitrary.

7343c40

chriseth force-pushed the abi-api branch from f253f1d to 5c6a6ae Compare April 11, 2018 22:13

chriseth mentioned this pull request Apr 11, 2018

Investigate bytes cleanup #3867

Closed

axic commented Apr 12, 2018

View reviewed changes

Allow struct encoding with new encoder.

75b8828

chriseth force-pushed the abi-api branch from 5c6a6ae to 75b8828 Compare April 12, 2018 14:37

axic commented Apr 12, 2018

View reviewed changes

chriseth approved these changes Apr 12, 2018

View reviewed changes

chriseth merged commit 44416d1 into develop Apr 12, 2018

axic deleted the abi-api branch April 13, 2018 01:10

Add abi.encode and abi.encodePacked #2980

Add abi.encode and abi.encodePacked #2980

Conversation

axic commented Sep 27, 2017 • edited by chriseth Loading

axic commented Sep 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriseth Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

axic commented Sep 27, 2017

axic commented Nov 29, 2017

axic commented Dec 12, 2017

chriseth commented Dec 12, 2017

axic commented Dec 12, 2017

protinam commented Mar 2, 2018 • edited Loading

chriseth commented Mar 2, 2018

chriseth commented Mar 16, 2018

chriseth commented Mar 17, 2018

VoR0220 commented Mar 17, 2018

chriseth commented Mar 19, 2018

chriseth commented Apr 11, 2018

chriseth commented Apr 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axic commented Apr 12, 2018

axic commented Apr 12, 2018

chriseth commented Apr 12, 2018

axic commented Apr 12, 2018

chriseth commented Apr 12, 2018

chriseth commented Apr 12, 2018

axic commented Apr 12, 2018

chriseth commented Apr 12, 2018

axic commented Apr 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axic commented Apr 12, 2018

ProphetDaniel commented Apr 12, 2018 • edited Loading

axic commented Apr 13, 2018

axic commented Sep 27, 2017 •

edited by chriseth

Loading

chriseth Sep 27, 2017 •

edited

Loading

protinam commented Mar 2, 2018 •

edited

Loading

ProphetDaniel commented Apr 12, 2018 •

edited

Loading