Restructure code to expose the internal hasher:#19
Merged
Conversation
- expose NmtHasher API s.t. it can be used to construct and use NMT Hashsers externally (e.g. for IPLD multihashers) - Add two functions Sha256Namespace8FlaggedLeaf, Sha256Namespace8FlaggedInner whose signature matches regular hash functions that simplify the above even further
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 95.30% 95.39% +0.09%
==========================================
Files 7 7
Lines 298 304 +6
==========================================
+ Hits 284 290 +6
Misses 10 10
Partials 4 4
Continue to review full report at Codecov.
|
liamsi
commented
Feb 13, 2021
Comment on lines
+20
to
+51
| var defaultHasher = NewNmtHasher(sha256.New(), DefaultNamespaceIDLen, true) | ||
|
|
||
| // Sha256Namespace8FlaggedLeaf uses sha256 as a base-hasher, 8 bytes | ||
| // for the namespace IDs and ignores the maximum possible namespace. | ||
| // | ||
| // Sha256Namespace8FlaggedLeaf(namespacedData) results in: | ||
| // ns(rawData) || ns(rawData) || sha256(LeafPrefix || rawData), | ||
| // where rawData is the leaf's data minus the namespace.ID prefix | ||
| // (namely namespacedData[NamespaceLen:]). | ||
| // | ||
| // Note that for the input len(namespacedData) >= DefaultNamespaceIDLen has to hold. | ||
| // If the input does not fulfil this, we will panic. | ||
| // The output will be of length 2*DefaultNamespaceIDLen+sha256.Size = 48 bytes. | ||
| func Sha256Namespace8FlaggedLeaf(namespacedData []byte) []byte { | ||
| return defaultHasher.HashLeaf(namespacedData) | ||
| } | ||
|
|
||
| // Sha256Namespace8FlaggedInner hashes inner nodes to: | ||
| // minNID || maxNID || sha256(NodePrefix || leftRight), where leftRight consists of the full | ||
| // left and right child node bytes, including their respective min and max namespace IDs. | ||
| // Hence, the input has to be of size: | ||
| // 48 = 32 + 8 + 8 = sha256.Size + 2*DefaultNamespaceIDLen bytes. | ||
| // If the input does not fulfil this, we will panic. | ||
| // The output will also be of length 2*DefaultNamespaceIDLen+sha256.Size = 48 bytes. | ||
| func Sha256Namespace8FlaggedInner(leftRight []byte) []byte { | ||
| const flagLen = DefaultNamespaceIDLen * 2 | ||
| sha256Len := defaultHasher.Size() | ||
| left := leftRight[flagLen:+sha256Len] | ||
| right := leftRight[flagLen+sha256Len:] | ||
|
|
||
| return defaultHasher.HashNode(left, right) | ||
| } |
Member
Author
There was a problem hiding this comment.
Adding these 2 methods is the only real change. Everything else is moving existing code from the internal package to the main nmt one, and, deleting some no longer needed code/abstractions.
liamsi
added a commit
to celestiaorg/celestia-core
that referenced
this pull request
Feb 13, 2021
- use hasher from nmt instead of redefining the logic in multihash or here (see celestiaorg/nmt#19) - remove multihash fork and use updated go-verifcid - minor renaming and refactoring
6 tasks
liamsi
added a commit
to celestiaorg/celestia-core
that referenced
this pull request
Feb 13, 2021
- use hasher from nmt instead of redefining the logic in multihash or here (see celestiaorg/nmt#19) - remove multihash fork and use updated go-verifcid - minor renaming and refactoring
tzdybal
approved these changes
Feb 15, 2021
| return n.NamespaceLen | ||
| } | ||
|
|
||
| func NewNmtHasher(baseHasher hash.Hash, nidLen namespace.IDSize, ignoreMaxNamespace bool) *Hasher { |
Contributor
There was a problem hiding this comment.
As this method is in nmt package, it can be named NewHasher.
evan-forbes
approved these changes
Feb 15, 2021
liamsi
added a commit
to celestiaorg/celestia-core
that referenced
this pull request
Feb 19, 2021
* change plugin package * update to new rsmt2d API and update go.mod * Moved (pre)loading the plugin to init() of the plugin package itself * minor clarification with regard to the skipped test * refactor plugin to be more easily usable as a lib * Use cid.Undef and remove obsolete comment * use that lazyledger/go-ipfs fork * use ll go-verifcid and go-ipfs fork in main ll-core package too * update forks to latest version * replace deps in plugin and reorg in ll-core * bump ipfs related versions * Register custom hasher from plugin code instead of in go-multihash fork - use hasher from nmt instead of redefining the logic in multihash or here (see celestiaorg/nmt#19) - remove multihash fork and use updated go-verifcid - minor renaming and refactoring * update nmt and assert that the hashFunc is registered * small test to validate verifcid allows our hasher * actually skip test * use remote instead of local go-ipfs repo * clean up Makefile, remove set-taget, and add comment about building locally (in Readme) and remove ipfs dependency from ll-core module (this will be part of #152 instead) * review feedback: rename to mustRegisterNamespacedCodec * more review feedback: rename to mustRegisterNamespacedCodec and make sure the codec isn't already known * update nmt too * address nits: remove some unnecessary else statements * Update p2p/ipld/README.md Co-authored-by: John Adler <adlerjohn@users.noreply.github.com> * s/defaulLength/defaultLength * Update p2p/ipld/plugin/nodes/nodes.go Co-authored-by: John Adler <adlerjohn@users.noreply.github.com> * Deduplicate nmtHashSize * implement Copy * fix Tree implementation * Address more review comments: - rename share to namespacedLeaf - fix doc comment "When you realize you forgot to Load more https://www.youtube.com/watch?v=8yGfQak-q9M" * Add clarifying comment to `DataSquareRowOrColumnRawInputParser` and rename constant * minor doc improvement Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In celestiaorg/celestia-core#155 we want to register a custom multihash that matches those used by the nmt internally. Instead of duplicating the code for this (as in https://github.com/lazyledger/go-multihash/pull/1), we simply export the API to construct an nmt hasher.
Sha256Namespace8FlaggedLeaf,Sha256Namespace8FlaggedInnerwhose signature matches regular hash functions that simplify the above even furtherTODOs:
Sha256Namespace8FlaggedLeaf,Sha256Namespace8FlaggedInnercloses: #18