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

Flesh out merkle root construction page #38

Merged
merged 7 commits into from
Oct 26, 2019
Merged

Conversation

s-ben
Copy link
Contributor

@s-ben s-ben commented Oct 14, 2019

Pulled content and diagrams from DCP-0005 to flesh out merkle root construction page.

At the bottom of the page I've copied content from the corresponding page on the Bitcoin devdocs. Update for Decred and reuse?

Copy link
Member

@jholdstock jholdstock left a comment

Choose a reason for hiding this comment

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

I've dropped some comments on this, but fair warning - this is outside my wheelhouse.

Requesting a once-over from @davecgh if you have some time please.

docs/developer-guides/merkle-root-construction.md Outdated Show resolved Hide resolved
docs/developer-guides/merkle-root-construction.md Outdated Show resolved Hide resolved
docs/developer-guides/merkle-root-construction.md Outdated Show resolved Hide resolved
docs/developer-guides/merkle-root-construction.md Outdated Show resolved Hide resolved
docs/developer-guides/merkle-root-construction.md Outdated Show resolved Hide resolved
@jholdstock jholdstock mentioned this pull request Oct 22, 2019
22 tasks
Copy link
Member

@jholdstock jholdstock left a comment

Choose a reason for hiding this comment

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

Content is looking good now. I've added a couple of nit picking comments.

I also notice there are still a few lower case merkles in here, were you going to update all instances to caps? It seems like the correct thing to do

TXIDs and intermediate hashes are always in internal byte order when they're
concatenated, and the resulting merkle root is also in internal byte
order when it's placed in the block header.
This page explains what merkle trees are, how Decred uses them and provides step-by-step instructions for constructing compliant Merkle trees.
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest an oxford comma here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I thought the oxford comma was against the rules now, and have been trying to phase it out of my writing. Just googled it and was happy to see it's still legal in most contexts. It's just a "stylistic" choice, where it improves readability. Woot! Oxford comma added.

https://www.youtube.com/watch?v=P_i1xk07o4g


### Merkle Trees in Decred

Merkle roots are used to create compact summaries of all transactions in a block. These merkle roots can then be queried to quickly and efficiently verify whether or not a a transaction is included in a block.
Copy link
Member

Choose a reason for hiding this comment

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

not a a transaction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


![Merkle Tree Calculation](/img/core-blockchain-concepts/merkle_root_calc.svg)

!!! warning "Warning"
Copy link
Member

Choose a reason for hiding this comment

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

I think this warning should really be moved above the diagram. It feels strange that the last element on the page is a warning box, ie. a high priority message. It should be more prominent imo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Moved.


Merkle roots are used to create compact summaries of all transactions in a block. These merkle roots can then be queried to quickly and efficiently verify whether or not a a transaction is included in a block.

The block header of each block contains the merkle roots of two merkle trees, the `MerkleRoot` tree and `StakeRoot` tree. The `MerkleRoot` tree contains hashes of all regular transaction IDs. The `StakeRoot` tree contains hashes of stake-based transaction IDs (ticket purchases, votes, revocations, etc.).
Copy link
Member

Choose a reason for hiding this comment

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

To avoid the repetition we could say eitherThe header of each block... or Each block header...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice compression. Used

@s-ben
Copy link
Contributor Author

s-ben commented Oct 25, 2019

@jholdstock, comments addressed. Missed merkle tree capitalization was an epic find/replace fail.

Copy link
Member

@jholdstock jholdstock left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks

@jholdstock jholdstock merged commit 22437f8 into decred:master Oct 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants