3
\$\begingroup\$

As I was streaming I had a brilliant visitor suggest we write a blockchain in Python.

So we did.

Note that there is no validation or voting simulated here.

Here's the results, in package/blockchain.py:

from __future__ import annotations
# https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
from oscrypto.symmetric import aes_cbc_pkcs7_encrypt as encrypt # type: ignore


class Node:
    __slots__ = 'prior_node', 'data', 'hash'
    prior_node: Node
    data: bytes
    hash: bytes

    def __init__(self, prior_node, key, data):
        self.prior_node = prior_node
        self.data = data
        if prior_node is None:
            init_vector = bytes(16)
        else:
            init_vector = _ensure_byte_length(prior_node.hash, 16)
        key = _ensure_byte_length(key, 32)
        self.hash = encrypt(key, data, init_vector)[1]

    def __repr__(self):
        return f'Node<{self.data}\n{self.hash}\n{self.prior_node}>'

def _ensure_byte_length(bytes_, length):
    return bytes(bytes_.ljust(length, b'\x00')[:length])

class Chain:
    __slots__ = 'nodes'
    def __init__(self, key: bytes, data: bytes):
        self.nodes = Node(None, key, data)

    def __repr__(self):
        return f'Chain:\n{self.nodes}'

    def new_node(self, key, data):
        self.nodes = node = Node(self.nodes, key, data)
        return node

    def __len__(self):
        length = 0
        nodes = self.nodes
        while nodes:
            length += 1
            nodes = nodes.prior_node
        return length

def main():
    chain = Chain(b'the key', b'here is a bit of data')
    chain.new_node(b'P@$$w0rd', b'and here is a bit more data')
    chain.new_node(b'hunter2', b'and finally here is some more')
    print(chain)


if __name__ == '__main__':
    main()

And here's a tiny test, tests/test_blockchain.py:

from package.blockchain import Chain


def test_blockchain():
    chain = Chain(b'the key', b'here is a bit of data')
    chain.new_node(b'P@$$w0rd', b'and here is a bit more data')
    chain.new_node(b'hunter2', b'and finally here is some more')
    assert len(chain) == 3

Note that we did require oscrypto and openssl to run this.

Ran coverage with pytest under Python 3.7. Ran with black and mypy as well.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Have you edited this since you ran the code through black? Because black uses " for string literals, and would change your inline comment to be PEP 8 compliant. Also it would add 2 empty lines between your top level functions and classes. \$\endgroup\$
    – Peilonrayz
    Mar 30, 2020 at 1:10
  • 1
    \$\begingroup\$ @Peilonrayz silly me, I'm using nix-build and it's running black on the build directory and not the source... I should have known something was wrong with it when I didn't notice changes... \$\endgroup\$
    – Aaron Hall
    Mar 30, 2020 at 1:24

1 Answer 1

2
\$\begingroup\$
  1. By default mypy doesn't test much. This is because a large part of its philosophy is to allow dynamic and statically typed code at the same time. This is so migrating to mypy is easier and less daunting. Having thousands of errors when you start to port your legacy app is likely to scare a fair few mortals away.

    Please use the --strict flag to have typed Python, rather than hybrid Python.

    When you use the flag and you type all the functions and methods, you'll notice that there's an issue with Node.prior_node. Currently it's assigned the type Node, but we know that's a lie because we have if prior_node is None.

  2. I personally use --ignore-missing-imports rather than ignoring each import, as they quickly build up over time.

  3. Your __repr__ are non standard.

    Called by the repr() built-in function to compute the “official” string representation of an object. If at all possible, this should look like a valid Python expression that could be used to recreate an object with the same value (given an appropriate environment). If this is not possible, a string of the form <...some useful description...> should be returned.

    You probably want to be using __str__.

  4. I find it a little confusing that init_vector is being assigned two different things. It would make more sense if you pass in an empty bytes to _ensure_byte_length.

    prior_hash = b'' if prior_node is None else prior_node.hash
    init_vector = _ensure_byte_length(prior_hash, 16)
    

    You could change the ternary into a getattr.

    prior_hash = getattr(prior_hash, 'hash', b'')
    
  5. I would change Node to a dataclass. Why write code when you can just not?

    This would require moving the hash generation into a class method.

  6. I would define an __iter__ method on the node so that we can easily traverse the chain from any node.

    This makes the __len__ method of Chain really simple and clean.

  7. I'd rename _ensure_byte_length to _pad. The function has two jobs, pad is well known and allows us to have a much shorter function name.

  8. _ensure_byte_length doesn't need the extra bytes call.
  9. The method Chain.add_node is un-Pythonic. In Python it's standard to return nothing from a function with mutations.
from __future__ import annotations

import dataclasses
from typing import Optional, Iterator

from oscrypto.symmetric import aes_cbc_pkcs7_encrypt as encrypt


@dataclasses.dataclass
class Node:
    prev_node: Optional[Node]
    data: bytes
    hash: bytes

    @classmethod
    def build(cls, key: bytes, data: bytes, prev_node: Optional[Node] = None) -> Node:
        prev_hash = b"" if prev_node is None else prev_node.hash
        hash = encrypt(_pad(key, 32), data, _pad(prev_hash, 16))[1]
        return cls(prev_node, data, hash)

    def __iter__(self) -> Iterator[Node]:
        node: Optional[Node] = self
        while node is not None:
            yield node
            node = node.prev_node


def _pad(bytes_: bytes, length: int) -> bytes:
    return bytes_.ljust(length, b"\x00")[:length]


@dataclasses.dataclass
class Chain:
    node: Node

    def add_node(self, key: bytes, data: bytes) -> None:
        self.node = Node.build(key, data, self.node)

    def __len__(self) -> int:
        return sum(1 for _ in self.node)


def main() -> None:
    chain = Chain(Node.build(b"the key", b"here is a bit of data"))
    chain.add_node(b"P@$$w0rd", b"and here is a bit more data")
    chain.add_node(b"hunter2", b"and finally here is some more")
    print(chain)


if __name__ == "__main__":
    main()
\$\endgroup\$
4
  • \$\begingroup\$ 1. thx. 2. thx. 3. you quoted "<...some useful description...>" - I do that, and __repr__ is a fallback for __str__. 4. will consider 5. maybe, what about __new__? 6. thx 7. pad, no, this will truncate - pad doesn't. 8. thx 9. eh, set returns self on mutating. I really don't like the Node.build - will __new__ work? What about __post_init__? docs.python.org/3/library/dataclasses.html#post-init-processing - thanks for putting so much effort into reviewing - +1 \$\endgroup\$
    – Aaron Hall
    Mar 30, 2020 at 3:03
  • \$\begingroup\$ @AaronHall 3. Please re-look at Chain.__repr__. No __repr__ isn't a fallback for __str__ it's a fallback for str. They achieve very different things. 7. Seems pedantic at the cost of nice usage. 9. Right and exceptions define the norm. Whatever. 9.b. New and post may work for you. \$\endgroup\$
    – Peilonrayz
    Mar 30, 2020 at 3:29
  • \$\begingroup\$ @AaronHall 3. I think you failed to read "If this is not possible". 9. I made the mistake of trusting rando's on the internet. No, set doesn't return self. Only the special i-op dunder methods do. \$\endgroup\$
    – Peilonrayz
    Mar 30, 2020 at 14:44
  • \$\begingroup\$ I had the recollection that sets were a source of inconsistency in that area, I wonder where I got that from. \$\endgroup\$
    – Aaron Hall
    Mar 30, 2020 at 15:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.