Underflow in generate_seed for GENESIS_EPOCH

[mkalinin]

Issue

Encountered in v0.6.0.

With epoch = 0 (GENESIS_EPOCH) generate_seed passes epoch = GENESIS_EPOCH - 1 value to get_randao_mix:

...
    return hash(
        get_randao_mix(state, epoch - MIN_SEED_LOOKAHEAD) +
...

With epoch = -1 get_randao_mix throws an assertion error as condition
epoch <= get_current_epoch(state) gets violated.

[JustinDrake]

With epoch = -1 get_randao_mix throws an assertion error as condition
epoch <= get_current_epoch(state) gets violated.

epoch <= get_current_epoch(state) corresponds to -1 <= 0 which is not violated. Still, we shouldn't underflow.

@djrtwo I remember you saying that in Python we can do some magic to the minus operator - so that if a - b is ever negative we assert False. That will allow the executable spec to automatically find those nasties, especially when fuzz testing comes along.

[djrtwo]

epoch <= get_current_epoch(state) corresponds to -1 <= 0 which is not violated. Still, we shouldn't underflow.

violated when it's actually uints instead of python's int type. Anything less than 0 is an underflow and will likely be an issue in strongly typed languages.

@djrtwo I remember you saying that in Python we can do some magic to the minus operator - so that if a - b is ever negative we assert False

yes, it is possible to monkeypatch the - operator. Might be useful in this case.

---

nice find @mkalinin, I think it likely appropriate to put a min of genesis_epoch on that call to get_randao_mix.

The actual assert in get_randao_mix is going to underflow as well with get_current_epoch(state) - LATEST_RANDAO_MIXES_LENGTH.

I'll clean up these two and another function in a PR now

[djrtwo]

addressed in #1027

[mkalinin]

yes, it is possible to monkeypatch the - operator. Might be useful in this case.

I like this idea. Should be useful for all uint64 operators that can turn into either an overflow or underflow and would help to discover such cases across the spec on the early stage. Cause, I bet spec tests passed well in spite of this bug as by behaviour uint64 is a signed type.

[JustinDrake]

Fixed in #1027 and created a new issue for monkey-patching the minus operator.