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

Fix support for callable coders #76

Merged
merged 2 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions eth_abi/abi.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,13 @@ def is_encodable(typ, arg):
encoder.validate_value(arg)
except EncodingError:
return False
else:
return True
except AttributeError:
try:
encoder(arg)
except EncodingError:
return False

return True


# Decodes a single base datum
Expand Down
2 changes: 1 addition & 1 deletion eth_abi/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,4 @@ def from_type_str(cls, type_str, registry): # pragma: no cover
Used by ``ABIRegistry`` to get an appropriate encoder or decoder
instance for the given type string and type registry.
"""
raise NotImplementedError('Must implement `from_type_str`')
raise cls()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is confusing to me.

Copy link
Contributor Author

@davesque davesque Jun 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for catching that. I intended for that to be return cls() and I believe my thinking was that it would make some of the examples in the registry documentation more simple but then I just went with slightly more complex examples. I'll make a PR to revert it right away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR created for this: #78

4 changes: 2 additions & 2 deletions eth_abi/decoding.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,11 @@ def __init__(self, **kwargs):
super().__init__(**kwargs)

self.decoders = tuple(
HeadTailDecoder(tail_decoder=d) if d.is_dynamic else d
HeadTailDecoder(tail_decoder=d) if getattr(d, 'is_dynamic', False) else d
for d in self.decoders
)

self.is_dynamic = any(d.is_dynamic for d in self.decoders)
self.is_dynamic = any(getattr(d, 'is_dynamic', False) for d in self.decoders)

def validate(self):
super().validate()
Expand Down
9 changes: 6 additions & 3 deletions eth_abi/encoding.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class TupleEncoder(BaseEncoder):
def __init__(self, **kwargs):
super().__init__(**kwargs)

self.is_dynamic = any(e.is_dynamic for e in self.encoders)
self.is_dynamic = any(getattr(e, 'is_dynamic', False) for e in self.encoders)

def validate(self):
super().validate()
Expand All @@ -95,15 +95,18 @@ def validate_value(self, value):
)

for item, encoder in zip(value, self.encoders):
encoder.validate_value(item)
try:
encoder.validate_value(item)
except AttributeError:
encoder(item)

def encode(self, values):
self.validate_value(values)

raw_head_chunks = []
tail_chunks = []
for value, encoder in zip(values, self.encoders):
if encoder.is_dynamic:
if getattr(encoder, 'is_dynamic', False):
raw_head_chunks.append(None)
tail_chunks.append(encoder(value))
else:
Expand Down
Empty file.
99 changes: 99 additions & 0 deletions tests/test_integration/test_custom_registrations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
from eth_abi.abi import (
decode_single,
encode_single,
)
from eth_abi.decoding import (
BaseDecoder,
)
from eth_abi.encoding import (
BaseEncoder,
)
from eth_abi.exceptions import (
DecodingError,
EncodingError,
)
from eth_abi.registry import (
registry,
)

NULL_ENCODING = b'\x00' * 32


def encode_null(x):
if x is not None:
raise EncodingError('Unsupported value')

return NULL_ENCODING


def decode_null(stream):
if stream.read(32) != NULL_ENCODING:
raise DecodingError('Not enough data or wrong data')

return None


class EncodeNull(BaseEncoder):
word_width = None

@classmethod
def from_type_str(cls, type_str, registry):
word_width = int(type_str[4:])
return cls(word_width=word_width)

def encode(self, value):
self.validate_value(value)
return NULL_ENCODING * self.word_width

def validate_value(self, value):
if value is not None:
raise EncodingError('Unsupported value')


class DecodeNull(BaseDecoder):
word_width = None

@classmethod
def from_type_str(cls, type_str, registry):
word_width = int(type_str[4:])
return cls(word_width=word_width)

def decode(self, stream):
byts = stream.read(32 * self.word_width)
if byts != NULL_ENCODING * self.word_width:
raise DecodingError('Not enough data or wrong data')

return None


def test_register_and_use_callables():
registry.register('null', encode_null, decode_null)

assert encode_single('null', None) == NULL_ENCODING
assert decode_single('null', NULL_ENCODING) is None

encoded_tuple = encode_single('(int,null)', (1, None))

assert encoded_tuple == b'\x00' * 31 + b'\x01' + NULL_ENCODING
assert decode_single('(int,null)', encoded_tuple) == (1, None)

registry.unregister('null')
Copy link
Collaborator

@carver carver Jun 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unregister being necessary in the test is a hint to me that the API could be improved. The ideal I would look for is the pattern of initializing an object in a fixture, and then not worrying about state from one test polluting another test. That's awkward with this global registry singleton. Something like a make_registry() method that initializes a new registry with all the defaults would enables us to do:

@pytest.fixture
def registry():
    return make_registry()

Then we don't have to worry about unregistering.

Since there are other more pressing issues, maybe just capture the idea into an issue and we can come back to it later. (if you like it)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about making a fixture but just decided it was an okay hack for the time being. Also, I felt like it would be useful to test the functionality by calling encode_single and decode_single to make it more like an integration test instead of querying the registry directly and using the result of that. I'll make an issue for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue created: #77

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be wrapped in a try/finally, otherwise it's going to leave the registry polluted when this test fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR created for this: #79



def test_register_and_use_coder_classes():
registry.register(
lambda x: x.startswith('null'),
EncodeNull,
DecodeNull,
label='null',
)

assert encode_single('null2', None) == NULL_ENCODING * 2
assert decode_single('null2', NULL_ENCODING * 2) is None

encoded_tuple = encode_single('(int,null2)', (1, None))

assert encoded_tuple == b'\x00' * 31 + b'\x01' + NULL_ENCODING * 2
assert decode_single('(int,null2)', encoded_tuple) == (1, None)

registry.unregister('null')