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

Refine merkle tree construction #516

Closed
wants to merge 3 commits into from

Conversation

tbekas
Copy link
Contributor

@tbekas tbekas commented Aug 15, 2023

Resolves #511

##
## where HZ=H(0x0b)
##
## Memory layout is [H0, H1, H2, H3, H4, H5]
Copy link
Contributor Author

@tbekas tbekas Aug 15, 2023

Choose a reason for hiding this comment

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

Adressing the comment by @elcritch from previous PR.

I added some docs to individual methods instead of a module.

@elcritch
Copy link
Contributor

Also I created an issue for the pre-image attack bit: #511. You could probably close it with this PR?

@tbekas
Copy link
Contributor Author

tbekas commented Aug 16, 2023

Also I created an issue for the pre-image attack bit: #511. You could probably close it with this PR?

I just marked this PR as resolving the #511

@dryajov
Copy link
Contributor

dryajov commented Aug 21, 2023

From our sync discussion:

  • We want to store the global multihash codec for the interior nodes as a type field and only store the digest bytes for each node
  • We want to additionally store the CID codec for the merkle root so we can continue using the existing repo as a storage backend
  • We should probably rework the tree to be a full tree, which means that it has to contain an even number of nodes at each level
    • The overhead from that would be one additional hash per level, but it will greatly simplify the logic for calculating the number of nodes, height, offsets, etc. For reference look at the https://en.wikipedia.org/wiki/Binary_tree#Properties_of_binary_trees
    • For serialization purposes, we don't need to store the duplicate/dummy nodes, so the overhead is mostly just in memory

@tbekas
Copy link
Contributor Author

tbekas commented Aug 25, 2023

@dryajov Let me know if the current data model as well as accessors are sufficient. Internally tree is stored just a seq[byte] while on the API MultiHash is used (raw bytes are wrapped into a MultiHash)

MerkleTree* = object
    mcodec: MultiCodec
    digestSize: Natural
    leavesCount: Natural
    nodesBuffer: seq[byte]
    
proc root*(self: MerkleTree): MultiHash

proc leaves*(self: MerkleTree): seq[MultiHash]

@tbekas tbekas requested a review from dryajov August 28, 2023 05:39
@tbekas
Copy link
Contributor Author

tbekas commented Oct 12, 2023

Moved to a different PR

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.

Upgrade Merkle Tree to avoid second-preimage attacks
3 participants