ethereum / py-ssz

Python implementation of the Simple Serialize encoding and decoding
MIT License
32 stars 22 forks source link

Deserialise seq of bytes into bitlist only if last byte is non null #113

Closed franck44 closed 4 years ago

franck44 commented 4 years ago

What was wrong?

A sequence of bytes ending with 0x00 cannot be a serialisation of a bitlist. As a consequence it should not be possible to deserialise such sequence into a bitlist. The current implementation allows that.

Issue #109

How the issue was uncovered?

This issue was uncovered as part of the formal verification of the Eth2.0 specs using Dafny: issue 4

How was it fixed?

Deserialise into bitlist code; add a test to check that the last byte is not null. If it is raise an exception.

Tests: two tests provided with illegal input for deserialising into biltists and check that the exception is actually raised.

Cute Animal Picture

put a cute animal picture link inside the parentheses

franck44 commented 4 years ago

@ralexstokes @pipermerriam I am not sure how to fix the latest lint error. I don't see any error message apart from a command invocation error. Is the reformatter failing for some reason?

pipermerriam commented 4 years ago

Looks like the linting error is related to the black code formatter that we use. If you run make lint-roll from the root of the project it should run the code formatter and clean this up for you.

ralexstokes commented 4 years ago

separately i think we do want to ensure that deserializing with extra input raises an error, but this is separate from this PR. i'll move to an issue

franck44 commented 4 years ago

Looks like the linting error is related to the black code formatter that we use. If you run make lint-roll from the root of the project it should run the code formatter and clean this up for you.

@pipermerriam Re formatting with black, I have installed and reformatted the files, but still get an error when I run make lint-roll:

franck:~/development/franck44-py-ssz [master]$ black tests/sedes/test_bitlist_serializer.py
All done! ✨ 🍰 ✨
1 file left unchanged.
franck:~/development/franck44-py-ssz [master]$ make lint-roll
isort --recursive ssz tests
/Library/Developer/CommandLineTools/usr/bin/make lint
tox -elint
lint develop-inst-noop: /Users/franck/development/franck44-py-ssz
lint installed: appdirs==1.4.4,attrs==19.3.0,black==19.3b0,click==7.1.2,cytoolz==0.10.1,entrypoints==0.3,eth-hash==0.2.0,eth-typing==2.2.1,eth-utils==1.9.0,flake8==3.7.8,isort==4.3.21,lru-dict==1.1.6,mccabe==0.6.1,pycodestyle==2.5.0,pyflakes==2.1.1,pyrsistent==0.16.0,six==1.14.0,-e git+git@github.com:franck44/py-ssz.git@cda99a64d68ced1ee8c051f4acd87244de5f62b5#egg=ssz,toml==0.10.1,toolz==0.10.0
lint run-test-pre: PYTHONHASHSEED='2896134891'
lint run-test: commands[0] | flake8 /Users/franck/development/franck44-py-ssz/ssz /Users/franck/development/franck44-py-ssz/tests /Users/franck/development/franck44-py-ssz/scripts
lint run-test: commands[1] | black --check /Users/franck/development/franck44-py-ssz/ssz /Users/franck/development/franck44-py-ssz/tests /Users/franck/development/franck44-py-ssz/scripts
would reformat /Users/franck/development/franck44-py-ssz/tests/sedes/test_bitlist_serializer.py
All done! 💥 💔 💥
1 file would be reformatted, 64 files would be left unchanged.
ERROR: InvocationError for command /Users/franck/development/franck44-py-ssz/.tox/lint/bin/black --check ssz tests scripts (exited with code 1)

Any idea what can go wrong? I am not a Python/PyCharm regular user :-(

franck44 commented 4 years ago

@pipermerriam I fixed the lint problem. I think I had not properly completed the setup (activate the venv) and may have used different version of black on my MacOS box/PyCharm.

ralexstokes commented 4 years ago

@franck44 sorry about that! make lint-roll in this case only prints the linting errors, it does not go fix them for you