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

Add exception message to from #140

Merged
merged 3 commits into from
Dec 11, 2018

Conversation

jacqueswww
Copy link
Contributor

What was wrong?

When supplying an invalid from address, one would receive a KeyError.

How was it fixed?

Throws a more useful exception.

Cute Animal Picture

Cute animal picture

Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

Some minor cleanup requested

@@ -471,6 +472,8 @@ def _get_normalized_and_unsigned_evm_transaction(self, transaction, block_number
return evm_transaction

def _get_normalized_and_signed_evm_transaction(self, transaction, block_number='latest'):
if transaction['from'] not in self._key_lookup:
raise ValidationError('"from" key not available, does this account exist?')
Copy link
Member

Choose a reason for hiding this comment

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

This message seems like it could be improved a bit.

No "from" key was provided in the transaction which is required for transaction signing

A bit more concise and hints at how the user can fix it.

Copy link
Contributor Author

@jacqueswww jacqueswww Dec 10, 2018

Choose a reason for hiding this comment

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

I am changing this message to No valid "from" key was provided... the reason is that one can specify a public key, but the the key isn't available to use (this is how I found the error, I specified a public key of a contract) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we wanted to get really fancy, we could list the valid keys and show the actual key, but that shouldn't hold up this PR.

tests/backends/test_pyevm.py Show resolved Hide resolved
tests/backends/test_pyevm.py Outdated Show resolved Hide resolved
@carver carver merged commit 8a538c6 into ethereum:master Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants