[WIP] Refactoring the codebase

[Bhargavasomu]

What was wrong?

The code base can be refactored.
Issue #78

Cute Animal Picture

[Bhargavasomu]

@pipermerriam this is the code I have got till now. The function names and comments are bad as of now. This is because I am focussing on the correctness of the design as of now. The algorithm/design is mentioned in the comments of issue #78 . Please verify it accordingly. Thanks!

[pipermerriam]

I'm having trouble understanding how this will work with a database. Would each of the node classes be given a handle to the database so they can look up values?

[Bhargavasomu]

@pipermerriam yes, I was hoping to include an interface in each of the Node's class which would exclusively deal with the DB. And will change the code as per above suggestion.

• But apart from that, is the code ok?
• Also I was thinking that maybe we could use LevelDB for the DB operations, since it is fast. Any particular reason that I should stick on to the ScratchDB which is used by the current implementation.
• Could you please brief me about HP Encoding, if possible. In specific, I need the answer to my comment here (the HP Encoding part)

[carver]

• Also I was thinking that maybe we could use LevelDB for the DB operations, since it is fast.

The trie implementation should be agnostic to the database. No reason to lock into LevelDB. The current mechanism of passing in the database to init seems fine to me.

Any particular reason that I should stick on to the ScratchDB which is used by the current implementation.

ScratchDB is only used internally to make sure that we don't write out a lot of garbage from intermediate state into the "real" database.

[nobody4t]

• Also I was thinking that maybe we could use LevelDB for the DB operations, since it is fast.

The trie implementation should be agnostic to the database. No reason to lock into LevelDB. The current mechanism of passing in the database to init seems fine to me.

Yes, I agree to this. The implementation should be agnostic to DB. But When I try to make some examples, I have no idea what should I pass as db to trie. From the code, I do not think it is some db instance as I found empty dict is good. But the common db interface is not like that. Especially when to read and write, there are some methods used to achieve this. Like leveldb, this should be done with db.Get and db.Put. So I think before I pass the db parameter to trie, I will always need some kind of wrapping to the db instance. Is that right? If I missed something, please let me know.

Now if I am right, here is the ScrachDB. This is just a wrapper for internal use. Shall we just make it as a wrapper, I mean a real wrapper to wrap the db, and enable a common db interface with it? This is my point.

[carver]

So I think before I pass the db parameter to trie, I will always need some kind of wrapping to the db instance. Is that right?

Yup, that's exactly right. If you just want an in-memory db, then a dict() will do. But if you want any other kind of DB, you'll just build a wrapper that accepts dict-like access, and writes it to your DB. A leveldb example is here: https://github.com/ethereum/py-evm/blob/master/eth/db/backends/level.py You can browse the module and the parent module to see more examples.

Now if I am right, here is the ScrachDB. This is just a wrapper for internal use. Shall we just make it as a wrapper, I mean a real wrapper to wrap the db, and enable a common db interface with it? This is my point.

There is already a generally accepted db interface: a MutableMapping (aka dict-like methods). I don't see any reason to switch the interface.

ScratchDB is built for a very specific internal purpose: to allow squashing a series of changes without creating a bunch of cruft in the trie. What do you mean by "make it ... a real wrapper to wrap the db"? And what does that enable the end-user to do?

[Bhargavasomu]

@pipermerriam @carver I plan on continuing this. But as far as the specs go, to distinguish between Extension Node and Leaf Node, a new variable is used which is the Terminator Flag. Since now we are planning to move to the Object Oriented Approach, this flag is not needed. We could just replace this with an isinstance check. Could you please comment on this and confirm that my view is right?

[carver]

Sounds okay in theory