Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem: No BEP discussing the UTXO implementation #20

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kansi
Copy link
Contributor

@kansi kansi commented Mar 14, 2018

Solution: Write an informational BEP with a brief discussion of current UTXO implementation.

Solution: Write an informational BEP with a brief discussion of current UTXO implementation.
@@ -0,0 +1,19 @@
```
shortname: ?/UTXO-IMPL
name: A short discription of the current UTXO implementation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

description

```

## Description
The UTXO (unspent transaction output) of each transaction is tracked using the a merkle tree. The merkle root of this tree is used as the app hash and retuned to Tendermint during `commit`. Below is a short summary of the UTXO is calculated,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear how all the outputs (which are like floating independent dots) are somehow connected to form a Merkle tree. Maybe draw a picture? They say pictures are worth 1000 words and in this case I think it's true.

```

## Description
The UTXO (unspent transaction output) of each transaction is tracked using the a merkle tree. The merkle root of this tree is used as the app hash and retuned to Tendermint during `commit`. Below is a short summary of the UTXO is calculated,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe link to some articles about Merkle trees.

https://en.wikipedia.org/wiki/Merkle_tree

What about AVL trees, IAVL trees, and IAVL+ trees. I gather that those are supposed to be useful somehow, as an improvement. Is it the rules for adding the next leaf UTXO, where to put it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ttmc I have restrained the context of this BEP to just discussing the current implementation. Also, the leaves of the UTXO merkle tree are added to the list of the existing leaves and the list is sorted after which merkle root is calculated. As explained in the BEP, the leaves are added to the utxo collection and then all the leaves are read back and sorted, after which the root is calculated.

- This list of unspent outputs fetched from the `utxo` collection is considered as the leaves of the UTXO merkle tree. Refer the image below,
- The list of leaves is hashed and sorted after which the merkle root is calculated by pairing adjacent leaves.

![Merkel Tree](https://upload.wikimedia.org/wikipedia/commons/thumb/9/95/Hash_Tree.svg/800px-Hash_Tree.svg.png)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merkle

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🇩🇪 🌲


![Merkel Tree](https://upload.wikimedia.org/wikipedia/commons/thumb/9/95/Hash_Tree.svg/800px-Hash_Tree.svg.png)


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still not clear to me where one puts a new leaf node (UTXO). There are many possible spots to choose from. And it matters. The choice will affect the final hash. It seems that's where the AVL rules come into use. Is this right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, lets take a step back. The unspent outputs or UTXO is stored in the utxo collection in MongoDB (there is no insert order here). When the merkle root is calculated all the leaves are fetched from the utxo collection, the leaves are hashed (lets call this H), then this hash list is sorted (Hs) and the merkle root is caculated by pairing the leaves. So if a new leaf is inserted we don't particularly insert it to specific position in the tree, its position among leaves will be determinted by the sort function which sorts the list of hashes H.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, seems like we are relying on some mysterious sort function. It would be nice to know what it does and how it works. Can we reliably get a nice balanced tree doing this?

Copy link
Contributor Author

@kansi kansi Mar 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ttmc You can have a look at the code here. The list of hashes are sorted and the merkle root is calculated. Since the whole tree is re-built everytime the merkle root is calculated so it would be balanced.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, rebuilding the whole tree from scratch every time will make it balanced, but that's a real waste of computing time and power. It's okay for now, but we need to do it smart in the future, using a proper IAVL+ library. That library should have a method like tree.insert_node(new_node) and then there will be other methods to compute the Merkle root of the tree (i.e. the app hash. That will also be fast, because most of those hash computations were done before and are stored somewhere handy).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ttmc You are right. We need to have some discussions around how to improve this implementation and make it crash recovery friendly.

Solution: Fix spelling
@vrde
Copy link
Contributor

vrde commented Sep 6, 2018

Should we move it to an issue and label it with BEP idea?

@ttmc
Copy link
Contributor

ttmc commented Sep 6, 2018

I will change the label on this pull request from "new BEP" to "BEP idea" but I think it's fine to leave it as a PR, so as to preserve its history in one place.

@ttmc ttmc added BEP idea An idea for a new BEP, seeking feedback and removed new BEP labels Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BEP idea An idea for a new BEP, seeking feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants