Remove abi.encodePacked

[axic]

trafficstars

abi.encodePacked was introduced in 0.4.22 as a backwards compatibility measure for the irregular "packed" encoding.

Since then we have introduced bytes.concat (#10903) and learned that most projects (rightly) do not rely on the packed encoding, but only concatenation. I suggest we drop abi.encodePacked in the next breaking release.

(We could consider introducing a message with #11508 if we want to test the waters 😅 , but would rather refrain from this.)

[chriseth]

Which types can be used with encodePacked that cannot be used with bytes.concat?

[charles-cooper]

Which types can be used with encodePacked that cannot be used with bytes.concat?

Arrays. https://docs.soliditylang.org/en/v0.8.6/abi-spec.html.

What is the effect of this proposal on event index encoding?

[chriseth]

Event index encoding will not change, since we only plan to remove abi.encodePacked from the language.

Is packed encoding of arrays used in the real world?

[e-nikolov]

Is packed encoding of arrays used in the real world?

One place where it can be used is for EIP-712 hashes - https://eips.ethereum.org/EIPS/eip-712#definition-of-encodedata.

For example if there is a struct that contains an array:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

contract MultiTransferDemo {
    struct MultiTransfer {
        uint256 id;
        Transfer[] transfers;
    }

    struct Transfer {
        uint256 amount;
        address recepient;
    }

    function hash(MultiTransfer memory mt) public pure returns (bytes32) {
        return
            keccak256(
                abi.encode(
                    keccak256(
                        "MultiTransfer(uint256 id,Transfer[] transfers)Transfer(uint256 amount,address recepient)"
                    ),
                    mt.id,
                    hashTransfers(mt.transfers)
                )
            );
    }

    function hashTransfers(Transfer[] memory transfers)
        public
        pure
        returns (bytes32)
    {
        bytes32[] memory hashedTransfers = new bytes32[](transfers.length);
        for (uint256 i = 0; i < transfers.length; i++) {
            hashedTransfers[i] = hashTransfer(transfers[i]);
        }
        return keccak256(abi.encodePacked(hashedTransfers));
    }

    function hashTransfer(Transfer memory transfer)
        public
        pure
        returns (bytes32)
    {
        return
            keccak256(
                abi.encode(
                    keccak256("Transfer(uint256 amount,address recepient)"),
                    transfer.amount,
                    transfer.recepient
                )
            );
    }
}

It can also be implemented via bytes.concat, but I'm not sure which version has better performance:

    function hashTransfers2(Transfer[] memory transfers)
        public
        pure
        returns (bytes32)
    {
        bytes memory encodedTransfers = new bytes(0);
        for (uint256 i = 0; i < transfers.length; i++) {
            encodedTransfers = bytes.concat(
                encodedTransfers,
                hashTransfer(transfers[i])
            );
        }
        return keccak256(encodedTransfers);
    }

[maurelian]

Which types can be used with encodePacked that cannot be used with bytes.concat?

As a real world example, this use of encodePacked():

https://github.com/Anish-Agnihotri/merkle-airdrop-starter/blob/83d6c65d00a10b40e0abdd84a5afa692553a9e72/contracts/src/MerkleClaimERC20.sol#L66

Would need to use casting to do the same thing:

keccak256(bytes.concat(bytes20(a),bytes32(value)));

Would it be considered more 'idiomatic' to use bytes.concat and cast from address and uintX?

[cameel]

I think that address is actually something that should be allowed as input in bytes.concat(). It does have an unambiguous binary representation and an explicit cast to bytes20 should not be needed.

[fredlacs]

Curious on the current thinking around this. Is this still being considered?

[ekpyron]

Current plan is to indeed remove abi.encodePacked as soon as https://github.com/ethereum/solidity/issues/11593 is done and it's possible to reimplement this generically in user code :-).

[cameel]

#11593 is this issue :)

[ekpyron]

#11593 is this issue :)

Ah, sorry, I meant https://github.com/ethereum/solidity/issues/13321 :-).

[axic]

I think that address is actually something that should be allowed as input in bytes.concat(). It does have an unambiguous binary representation and an explicit cast to bytes20 should not be needed.

@cameel I think the real solution is #11284 and not adding non-bytes types to bytes.concat.

[cameel]

I think it's a solution for the general case but I also wouldn't mind easing on the overly verbose conversions in select cases where they don't add anything. Even with some nicer conversion from #11284 this would be more verbose than it needs to be for clarity.

[PaulRBerg]

What's your latest advice on how to convert an address to bytes32?

I'm still relying on abi.encodePacked since it's clearer than bytes.concat(bytes20(addr)).

[CJ42]

I think abi.encodePacked is very convenient for concatenation. Especially when concatenating multiple types that are not all bytesN or bytes.

For instance, to concatenate like: abi.encodePacked(address, bytesN, uint256, …)

[exqlnet]

The most common problem i have met is that how can i do this using other languages? such as js/golang?