Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

Expose leaf node with smt.GetLeaf() #54

Closed
wants to merge 2 commits into from

Conversation

roysc
Copy link
Contributor

@roysc roysc commented Jul 31, 2021

This will allow access to the contents of a leaf node, including the node path and hashed value.

This will support an update to Cosmos SDK ADR-040 by allowing access to the hashed value and path without any redundant hashing.

Note: should we optimize this to allow fetching the leaf node directly rather than descending the tree, as has been done for value lookup?

@codecov-commenter
Copy link

codecov-commenter commented Jul 31, 2021

Codecov Report

Merging #54 (56045a8) into master (2c90766) will decrease coverage by 1.43%.
The diff coverage is 65.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
- Coverage   86.05%   84.61%   -1.44%     
==========================================
  Files           6        6              
  Lines         466      494      +28     
==========================================
+ Hits          401      418      +17     
- Misses         37       43       +6     
- Partials       28       33       +5     
Impacted Files Coverage Δ
smt.go 79.21% <65.38%> (-1.58%) ⬇️
deepsubtree.go 61.11% <0.00%> (-2.36%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c90766...56045a8. Read the comment docs.

Copy link
Member

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

It's not clear to me why this is actually needed. Where in cosmos/cosmos-sdk#9680 specifically is it required to get the leaf node?

smt.go Outdated Show resolved Hide resolved

if bytes.Equal(currentHash, smt.th.placeholder()) {
// We've hit a placeholder value; this is the end.
return nil, nil
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@roysc
Copy link
Contributor Author

roysc commented Aug 1, 2021

The linked PR is just to change the SMT stored value from the h(k, v) to h(v) (where k, v are the state store KV record). Since the decoupled state store will also store a reverse index mapping the h(k) path back to the original k, we want a way to get the path without performing a redundant hash on k. (Though with a change is planned in #40 to map keys directly in the SMT without hashing, in which case that should be unnecessary.)

We also want to use h(v) to index values in IPLD (not my domain, @i-norden could shed more light on that), so likewise, we want access to the leaf to avoid hashing v again.

@adlerjohn
Copy link
Member

we want a way to get the path without performing a redundant hash on k
we want access to the leaf to avoid hashing v again.

I 100% guarantee you it's cheaper to perform single hashes than to execute this function (which will perform log(n) hashes and disk reads). By several orders of magnitude.

smt/smt.go

Lines 103 to 148 in 56045a8

func (smt *SparseMerkleTree) GetLeaf(key []byte) (*LeafNode, error) {
path := smt.th.path(key)
if bytes.Equal(smt.root, smt.th.placeholder()) {
// The tree is empty, return the default value.
return &LeafNode{Path: path, ValueHash: nil}, nil
}
currentHash := smt.root
for i := 0; i < smt.depth(); i++ {
currentData, err := smt.nodes.Get(currentHash)
if err != nil {
return nil, err
} else if smt.th.isLeaf(currentData) {
// We've reached the end. Is this the actual leaf?
p, valueHash := smt.th.parseLeaf(currentData)
if !bytes.Equal(path, p) {
// Nope. Therefore the key is actually empty.
return &LeafNode{Path: p, ValueHash: nil}, nil
}
// Otherwise, yes. Return the value.
return &LeafNode{Path: p, ValueHash: valueHash}, nil
}
leftNode, rightNode := smt.th.parseNode(currentData)
if getBitAtFromMSB(path, i) == right {
currentHash = rightNode
} else {
currentHash = leftNode
}
if bytes.Equal(currentHash, smt.th.placeholder()) {
// We've hit a placeholder value; this is the end.
return nil, nil
}
}
// The following lines of code should only be reached if the path is 256
// nodes high, which should be very unlikely if the underlying hash function
// is collision-resistant.
currentData, err := smt.nodes.Get(currentHash)
if err != nil {
return nil, err
}
_, valueHash := smt.th.parseLeaf(currentData)
return &LeafNode{Path: path, ValueHash: valueHash}, nil
}

@roysc
Copy link
Contributor Author

roysc commented Aug 1, 2021 via email

@liamsi
Copy link
Member

liamsi commented Aug 3, 2021

We also want to use h(v) to index values in IPLD

Can you point me to how you use IPLD? I assume you also store the inner nodes in IPFS, too?

@roysc
Copy link
Contributor Author

roysc commented Aug 13, 2021

@liamsi I believe so but I'm not certain - @i-norden would have more info. To my knowledge what we need is to iterate over the SMT's nodes (leaf and probably internal) and retrieve their contents - in which case, this GetLeaf accessor won't be needed anyway. I will close this PR.

@roysc roysc closed this Aug 13, 2021
@roysc roysc deleted the cosmos-get-leaf branch April 15, 2022 16:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants