Add disable_process_reveal_deadlines decorator and reveal_deadlines_setting meta tag

[hwwhww]

Issue

Fix #2015

Solution

1. Add disable_process_reveal_deadlines decorator to disable phase 1 process_reveal_deadlines function.
2. Add reveal_deadlines_setting meta tag: if reveal_deadlines_setting is 1, disable the process_reveal_deadlines function.

TODOs

• Add @disable_process_reveal_deadlines to some custody game tests.
• TBD: for the correctness of protocol, I'm inclined to have process_reveal_deadlines default to enabled. However, process_reveal_deadlines may affect some more tests' state balances and make it difficult to debug. We may want to disable it in more irrelevant test cases.

[hwwhww]

It's still unacceptable slow to generate these test vectors. I suggest applying @with_configs([MINIMAL]) (#2016) to these test cases, i.e., don't generate mainnet test vectors for now and think about other ways to test custody game with mainnet config.

[djrtwo]

for the correctness of protocol, I'm inclined to have process_reveal_deadlines default to enabled.

I agree

[protolambda]

Mutating the spec during testing is tricky, but possible. See review comment, I think the new disable_process_reveal_deadlines decorator doesn't work as expected.

[protolambda]

Could be a bool, but if we end up having more settings then this is compatible. 👍

[protolambda]

This mutates the spec object. Making it not re-usable. I.e. other tests afterwards will also have a no-operation process_reveal_deadlines. The decorator has to set the old function back, after running the wrapped test, for this to work as expected.

Also beware of returned generators not being evaluated directly; the wrapped function should be evaluated fully before undoing the no-op substitute. A similar thing is done in the BLS decorator, if you need an example. See bls_switch in context.py

[hwwhww]

Thanks for the catch! 👍👍
I forgot that the test generator mode scope != pytest unit test mode scope.

[protolambda]

Thinking long-term, a heavier but imho better approach for testing would be:

• Define an eth2 Spec with a Protocol (py 3.8) for static type checking. (proposed this before, but didn't seem viable at the time)
• Make each spec function isolated: first argument is spec: Spec, to access other functions and variables. No globals or implicit dependencies (yay).
• Ability to mock any part of the spec by passing a special object, that implements the Spec interface (or Protocol in python), and implements it in it's own way. E.g. everything proxied to the real implementation (could be done with inheritance of functions) except the mocked function.
• Solves the phase 0 / 1 compatibility issue. A clear interface would be defined.
• Spec tests would be fully type-checkable, since their is no arbitrary spec module being passed around anymore. It would be a typed spec (Protocol)
• Not just able to mock functions (like making disable_process_reveal_deadlines a no-op), but also able to verify a function was called with specific argument, by wrapping the real function. Could improve testing.

Shall I open an issue for this? I think it's out of scope for this PR, but given the easy mistake with disable_process_reveal_deadlines, and the many benefits, I think it's the way to go.

cc @djrtwo

[hwwhww]

@protolambda

• Define an eth2 Spec with a Protocol (py 3.8) for static type checking. (proposed this before, but didn’t seem viable at the time)
• Make each spec function isolated: first argument is spec: Spec, to access other functions and variables. No globals or implicit dependencies (yay).
• Solves the phase 0 / 1 compatibility issue. A clear interface would be defined.

Yes, I fully support it! 👍 I once tried to implement class Spec (not Protocol version) a long time ago but didn’t have one workable branch due to other higher priorities 🤦‍♀️

Trinity has a similar pattern. From what I can tell, it’s a much better way to handle the forks. The main benefit I see for other teams is that we could use Python super inheritance instead of copying the code and modify a piece of it. Although it’s a bit more Pythonic, IMO we will get a cleaner spec with it.

[protolambda]

LGTM