Adding a new db wrapper - TrackedDB

[hwwhww]

What was wrong?

This PR is part of #195
The first step: database wrapper

How was it fixed?

Adding a new database wrapper TrackedDB. TrackedDB has two dicts:

• _writes: keeps track of the nodes that are accessed by TrackedDB.put(key, value)
• _reads: keeps track of that are accessed by TrackedDB.get(key)

Some new class properties of TrackedDB:

• @property def reads(self): returns the whole _reads dict
• @property def writes(self): returns the whole _writes dict

Cute Animal Picture

[pipermerriam]

Have you looked into how this is going to interact with the JournalDB or whether this should be integrated with that class in some way. If an internal call within a transaction touches some state and then those state changes are reverted due to an evm exception, do the touched parts still remain in the read/write list? My assumption is yes, but if not, we need to take that into account.

[pipermerriam]

Can I suggest we move this to it's own module since it's a low level DB wrapper.

I would also suggest we rename this to something more descriptive. Here are some names that come to mind.

• StenographerDB (A stenographer is the person in a courtroom who types everything that people say).
• TrackedDB - tracks changes, touches, etc.
• LedgerDB - keeps a ledger of the db read/write

[hwwhww]

Sure!
In pyethereum, a similar db wrapper is called ListeningDB. IMO StenographerDB is cool but a bit too long. I like TrackedDB.

[hwwhww]

Have you looked into how this is going to interact with the JournalDB or whether this should be integrated with that class in some way. If an internal call within a transaction touches some state and then those state changes are reverted due to an evm exception, do the touched parts still remain in the read/write list? My assumption is yes, but if not, we need to take that into account.

My assumption is that in the stateless client use case, StateDB is more like a disposable container because:

1. With pure function, we can use StateDB more flexibly.
2. We mainly need it for creating the collation witness data. So reads and writes data is no longer need for stateless client.
3. So if transaction fails, we just don't include the reads and writes data of this tx.

p.s. I am writing some more detailed description and will comment on #195

[hwwhww]

I want to leave the modification of ChainDB to other PR, after I actually have some test cases of using pure apply_transaction function. So this PR is only for introducing TrackedDB.

[pipermerriam]

This whole try/except/else block is equivalent to the same code without the try/except/else since except just re-raises.

current_value = self.wrapped_db.get(key)
self._reads[key] = current_value
return current_value

[pipermerriam]

Should this instead be set to something like None to indicate that it's been deleted?

[hwwhww]

Good, using None would be more clear :)
And I think the delete function don't need to return something?
Can I just modify it to:

     def delete(self, key):
        """
        Delete the key and update writes dict.
        """
        self.wrapped_db.delete(key)
        self._writes[key] = None

[pipermerriam]

I'm thinking that we can accomplish the same by just exposing reads as a property.

all_reads = db.get_reads()
single_read = db.get_reads(key=b'cow')
# becomes the following if `reads` is a public property
all_reads = db.reads
single_read = db.reads.get(key=b'cow')

Alternatively, I'd be more comfortable with this API if it only did one of these two things (preferrably returning all reads).

[pipermerriam]

Same thoughts as with the get_reads API

[pipermerriam]

if clear_logs is only used for testing, lets remove it and just manually reset the obj._reads and obj._writes on the db object.

[hwwhww]

1. Followed @pipermerriam 's suggestions:

   • @property def reads(self): returns the whole _reads dict
   • @property def writes(self): returns the whole _writes dict

2. Update the logic in delete

   • Set self._writes[key] = None
   • By the way, it seems during state transition, delete method would only be triggered for SELFDESTRUCT or EIP161 (Spurious Dragon)?

3. Update the logic in exsits

   • Set self._reads[key] = self.wrapped_db.get(key) if self.wrapped_db.exists(key) else None
   • It would call wrapped_db.get(key) twice: one is in self.wrapped_db.get(key), another is in self.wrapped_db.exists(key)). It seems a wasted call, but also keeping the code simple and wrapped.