Skip to content

Commit

Permalink
Investigate and resolve discrepancies with solidity's abi.encode()
Browse files Browse the repository at this point in the history
- Empty values for dynamic encoders had extra 32-byte zero padding when compared to Solidity's abi.encode() method. This led to correct instructions for the EVM but also required more gas than necessary and ultimately led to differing hashes, etc.

- Add more tests. Decode tuple test values not just as separate list of types but as a tuple type with nested values for encode_abi() tests for tuples.

- Eliminate padding for empty dynamic arrays as well. If an array is static or if it is dynamic and empty do not pad with 32-byte zero padding.
  • Loading branch information
fselmo committed Feb 25, 2022
1 parent 8cd71ae commit 75ead60
Show file tree
Hide file tree
Showing 13 changed files with 248 additions and 115 deletions.
27 changes: 9 additions & 18 deletions eth_abi/encoding.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ def encode(self, values):
)

encoded_value = b''.join(head_chunks + tuple(tail_chunks))

return encoded_value

@parse_tuple_type_str
Expand Down Expand Up @@ -530,16 +529,12 @@ def validate_value(cls, value):
@classmethod
def encode(cls, value):
cls.validate_value(value)
value_length = len(value)

if not value:
padded_value = b'\x00' * 32
else:
padded_value = zpad_right(value, ceil32(len(value)))

encoded_size = encode_uint_256(len(value))
encoded_value = encoded_size + padded_value
encoded_size = encode_uint_256(value_length)
padded_value = zpad_right(value, ceil32(value_length))

return encoded_value
return encoded_size + padded_value

@parse_type_str('bytes')
def from_type_str(cls, abi_type, registry):
Expand Down Expand Up @@ -568,16 +563,12 @@ def encode(cls, value):
cls.validate_value(value)

value_as_bytes = codecs.encode(value, 'utf8')
value_length = len(value_as_bytes)

if not value_as_bytes:
padded_value = b'\x00' * 32
else:
padded_value = zpad_right(value_as_bytes, ceil32(len(value_as_bytes)))

encoded_size = encode_uint_256(len(value_as_bytes))
encoded_value = encoded_size + padded_value
encoded_size = encode_uint_256(value_length)
padded_value = zpad_right(value_as_bytes, ceil32(value_length))

return encoded_value
return encoded_size + padded_value

@parse_type_str('string')
def from_type_str(cls, abi_type, registry):
Expand Down Expand Up @@ -619,7 +610,7 @@ def encode_elements(self, value):
tail_chunks = tuple(item_encoder(i) for i in value)

items_are_dynamic = getattr(item_encoder, 'is_dynamic', False)
if not items_are_dynamic:
if not items_are_dynamic or len(value) == 0:
return b''.join(tail_chunks)

head_length = 32 * len(value)
Expand Down
1 change: 1 addition & 0 deletions newsfragments/158.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reconcile differences in 32-byte padding between eth-abi encoders for dynamic types and Solidity's abi.encode() for 0 or empty values
4 changes: 2 additions & 2 deletions tests/abi/test_decode_abi.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@


@pytest.mark.parametrize(
'type_str,expected,abi_encoding,_',
'type_str,expected,_1,abi_encoding,_2',
CORRECT_TUPLE_ENCODINGS,
)
def test_decode_abi(type_str, expected, abi_encoding, _):
def test_decode_abi(type_str, expected, _1, abi_encoding, _2):
abi_type = parse(type_str)
if abi_type.arrlist is not None:
pytest.skip('ABI coding functions do not support array types')
Expand Down
4 changes: 2 additions & 2 deletions tests/abi/test_decode_single.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@


@pytest.mark.parametrize(
'typ,expected,abi_encoding,_',
'typ,expected,_1,abi_encoding,_2',
CORRECT_SINGLE_ENCODINGS,
)
def test_decode_single(typ, expected, abi_encoding, _):
def test_decode_single(typ, expected, _1, abi_encoding, _2):
actual = decode_single(typ, abi_encoding)
assert actual == expected

Expand Down
55 changes: 49 additions & 6 deletions tests/abi/test_encode_abi.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,62 @@

from ..common.unit import (
CORRECT_TUPLE_ENCODINGS,
words,
)


@pytest.mark.parametrize(
'type_str,python_value,abi_encoding,_',
'tuple_type,python_value,_1,encoded_list_of_types,_2',
CORRECT_TUPLE_ENCODINGS,
)
def test_encode_abi(type_str, python_value, abi_encoding, _):
abi_type = parse(type_str)
def test_encode_abi_as_list_of_types(tuple_type, python_value, _1, encoded_list_of_types, _2):
abi_type = parse(tuple_type)
if abi_type.arrlist is not None:
pytest.skip('ABI coding functions do not support array types')

types = [t.to_type_str() for t in abi_type.components]
# assert different types encoded correctly as a list
# e.g. encode_abi(['bytes32[]', 'uint256'], ([b'a', b'b'], 22))
#
# compare to solidity:
# bytes32 a = 0x6100000000000000000000000000000000000000000000000000000000000000;
# bytes32 b = 0x6200000000000000000000000000000000000000000000000000000000000000;
# bytes32[] arr = [a,b];
# uint256 num = 22;
#
# abi.encode(arr,num);
separated_list_of_types = [t.to_type_str() for t in abi_type.components]
eth_abi_encoded = encode_abi(separated_list_of_types, python_value)
assert eth_abi_encoded == encoded_list_of_types

actual = encode_abi(types, python_value)
assert actual == abi_encoding

@pytest.mark.parametrize(
'tuple_type,python_value,is_dynamic,encoded_list_of_types,_2',
CORRECT_TUPLE_ENCODINGS,
)
def test_encode_abi_as_single_tuple_type(
tuple_type, python_value, is_dynamic, encoded_list_of_types, _2
):
# assert the tuple type is encoded correctly
# e.g. encode_abi(['(bytes32[],uint256)'], [([b'a', b'b'], 22)])
#
# compare to solidity:
# struct TupleExample {
# bytes32[] arg1;
# uint256 arg2;
# }
# bytes32 a = 0x6100000000000000000000000000000000000000000000000000000000000000;
# bytes32 b = 0x6200000000000000000000000000000000000000000000000000000000000000;
# bytes32[] arr = [a,b];
# uint256 num = 22;
#
# abi.encode(TupleExample(arr,num));
eth_abi_encoded = encode_abi([tuple_type], [python_value])

encoded_tuple_type = (
# 32 bytes offset for dynamic tuple types
b''.join([words('20'), encoded_list_of_types]) if is_dynamic

# no offset for static tuples so same encoding as if encoding a list of the types
else encoded_list_of_types
)
assert eth_abi_encoded == encoded_tuple_type
6 changes: 3 additions & 3 deletions tests/abi/test_encode_single.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@


@pytest.mark.parametrize(
'typ,python_value,abi_encoding,_',
'typ,python_value,_1,single_type_encoding,_2',
CORRECT_SINGLE_ENCODINGS,
)
def test_encode_single(typ, python_value, abi_encoding, _):
def test_encode_single(typ, python_value, _1, single_type_encoding, _2):
actual = encode_single(typ, python_value)
assert actual == abi_encoding
assert actual == single_type_encoding
4 changes: 2 additions & 2 deletions tests/abi/test_is_encodable.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@


@pytest.mark.parametrize(
'type_str,python_value,_1,_2',
'type_str,python_value,_1,_2,_3',
CORRECT_SINGLE_ENCODINGS,
)
def test_is_encodable_returns_true(type_str, python_value, _1, _2):
def test_is_encodable_returns_true(type_str, python_value, _1, _2, _3):
assert is_encodable(type_str, python_value)


Expand Down
4 changes: 2 additions & 2 deletions tests/abi/test_is_encodable_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@


@pytest.mark.parametrize(
'type_str,_python_value,_1,_2',
'type_str,_python_value,_1,_2,_3',
CORRECT_SINGLE_ENCODINGS,
)
def test_is_encodable_type_returns_true(type_str, _python_value, _1, _2):
def test_is_encodable_type_returns_true(type_str, _python_value, _1, _2, _3):
assert is_encodable_type(type_str)


Expand Down
Loading

0 comments on commit 75ead60

Please sign in to comment.