Don't allow empty string for hex values

[kclowes]

What was wrong?

Because we allow a bytestring or a hexstring to be passed into functions that require bytes types, an empty string was mistakenly being decoded as hex.

Related to Issue #1412

How was it fixed?

Added a check for an empty string in the hex string mixin class. Now an empty hexstring requires a '0x'.

Cute Animal Picture

[carver]

Is this about fulfilling this idea?

But even without the flag, the ValidationError from OP should be raised. +1

It seems strange & difficult to explain in the docs, that these are valid inputs for bytes32:

b''
b'X'
"0x"
"58"
"0x58"

But then "" is invalid. Whatever we decide here definitely belongs in a new section about ABI type-coercion, with examples.

What I was clumsily communicating in my earlier quote was the idea that anything shorter than 32 bytes should be invalid. I thought the extra flag would require all strs, no matter the length, to start with "0x".

I understand the hesitation to make strings shorter than 32 bytes invalid (as backwards incompatible). So, I think both of those changes (require 0x on all strings and invalidate too-short values) could go behind the same flag.

[kclowes]

Is this about fulfilling this idea:

But even without the flag, the ValidationError from OP should be raised. +1

yep, this is the attempt to do that. What I understood from #1412 is that an empty string is an invalid hex value, and therefore passing an empty string into a function that accepts a bytes value is a bug. But yeah, it's weird that other string values can be accepted without the '0x' prefix, as long as they can be decoded to hex.

Whatever we decide here definitely belongs in a new section about ABI type-coercion, with examples.

👍 I'll add that

I understand the hesitation to make strings shorter than 32 bytes invalid (as backwards incompatible). So, I think both of those changes (require 0x on all strings and invalidate too-short values) could go behind the same flag.

So, it seems like the most consistent (and non-breaking) approach here is leaving the default as-is, and adding docs about the validity of different parameters. Then, we can add a flag that, if enabled, requires all hex values to be prefixed with a 0x, and doesn't allow strings without the correct number of bytes. I started down that path earlier and then changed directions. Then, for v6, we can make strict the default. I will add warnings for values that will throw an error in v6.

[kclowes]

Closing in favor of #1419