Merkle tree misc fixes #47
Conversation
adlerjohn
added
bug
|
I think I'll relegate making nice diagrams and the Merkle multiproof format to issues for later, since they're not critical atm. |
| ```C++ | ||
| node.n_min = d.namespaceID | ||
| node.n_max = d.namespaceID | ||
| node.v = h(0x00, serialize(d)) |
There was a problem hiding this comment.
One last nitpick: You implicitly assuming that data d is a structure as well namely struct{namespaceID, raw_data}. And that last line is still ambiguous to what gets hashed in the end: if h(0x00, d.raw_data) or h(0x00, d.namespaceID||d.raw_data)) (or whatever the result of serialize(d) is).
Note: as far as I understand both h(0x00, d.raw_data) or h(0x00, d.namespaceID||d.raw_data)) correctly define a NMT but I think we should be explicit here.
There was a problem hiding this comment.
Technically, the serialization of shares is defined as a special case: https://github.com/lazyledger/lazyledger-specs/blob/adlerjohn-merkle_tree_fixes/specs/data_structures.md#share-serialization.
Shares canonically serialized using only the raw share data, i.e.
serialize(share) = serialize(share.rawData).
There was a problem hiding this comment.
Ah I missed that. But that doesn't address that you implicitly assume that d has to have a structure (namely two fields) but I guess the important part is covered in the spec (how the shares end up in the tree ...).
| @@ -228,100 +229,129 @@ Merkle trees are used to authenticate various pieces of data across the LazyLedg | |||
|
|
|||
| ## Binary Merkle Tree | |||
|
|
|||
| Binary Merkle trees are constructed in the usual fashion, with leaves being hashed once to get leaf node values and internal node values being the hash of the concatenation of their children. The specific mechanism for hashing leaves for leaf nodes and children for internal nodes may be different (see: [annotated Merkle trees](#annotated-merkle-tree)), but for plain binary Merkle trees are the same. | |||
| Binary Merkle trees are constructed in the same fashion as described in [Certificate Transparency (RFC-6962)](https://tools.ietf.org/html/rfc6962). Leaves are hashed once to get leaf node values and internal node values are the hash of the concatenation of their children (either leaf nodes or other internal nodes). | |||
There was a problem hiding this comment.
It would be helpful to specify how it's the same as RFC 6962. Specifically, that the tree is unbalanced when there isn't 2^x leaves. The next sentence is describing properties of all Merkle trees.
| ``` | ||
|
|
||
| If a compact Merkle root is needed, the root level (which consists of root fields and a root value) can be hashed once. | ||
| Note that rather than duplicating the last node if there are an odd number of nodes (the [Bitcoin design](https://github.com/bitcoin/bitcoin/blob/5961b23898ee7c0af2626c46d5d70e80136578d3/src/consensus/merkle.cpp#L9-L43)), trees are allowed to be imbalanced. In other words, the height of each leaf may be different. For an example, see Section 2.1.3 of [Certificate Transparency (RFC-6962)](https://tools.ietf.org/html/rfc6962). |
Rendered: