search

ethereum / cbc-casper

GNU Affero General Public License v3.0
229 stars 44 forks source link

[WIP] Feat/state lang #157

Closed naterush closed 6 years ago

naterush commented 6 years ago

ADDS:

naterush commented 6 years ago

@djrtwo fixed the tests, but not sure the way I'm testing currently makes the most sense. In many of the tests, loop through all the protocols and test each of them. This seemed like the most reasonable strategy, rather than passing each protocol as a parameter to the tests.

There might be some better pytest method of doing this, which I'll look into, but let me know if you have any thoughts.

naterush commented 6 years ago

Also, I'm not totally comfortable with how the TestingLang and StateLang are split up. Maybe it makes sense to have a "translate" function, which takes a command like SJ and totally removes it (by turning it into a string of S). I think this might be a more pure way of doing things.

naterush commented 6 years ago

The above requires a reverse mapping of messages => message_names. I could also implement a two-way dictionary (something like https://stackoverflow.com/questions/1456373/two-way-reverse-map). Alternatively, could use something like https://pypi.python.org/pypi/bidict. For now, going to just keep a reverse mapping.

naterush commented 6 years ago

@djrtwo your comments about the complicated inheritance, as well as the translation hell I was dealing with, led me removing the TestingLang and adding RR and SJ directly to the StateLang.

While this is a less pure (maybe), it makes things a lot less complicated in both respects above. Furthermore, this StatLang can contains the unimplemented function stubs for assertions that were found in the TestLang, and we now we just have a single level of inheritance for implementing those (which I think is unavoidable).

naterush commented 6 years ago

@djrtwo addressed most of the comments above, with the exception of parameterizing protocols in the way you describe (seemed to make things more complicated, but maybe I'm missing something).

Would appreciate a final review, and then good to merge :)

djrtwo commented 6 years ago

excellent. merging