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

Fetch values immediately, updates #39

Merged
merged 8 commits into from
Jul 12, 2021

Conversation

adlerjohn
Copy link
Member

Make updates as per #37 (comment).

Fixes #8 #14.

  1. Clean up readme.
  2. Remove Get/HasForRoot.
  3. Fetch values immediately instead of descending the tree.
  4. Add new methods to descend the tree for DSMST since key might not have been added with AddBranch.

@adlerjohn adlerjohn self-assigned this Jul 11, 2021
@adlerjohn adlerjohn linked an issue Jul 11, 2021 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2021

Codecov Report

Merging #39 (49a51bb) into master (03ea407) will decrease coverage by 1.10%.
The diff coverage is 61.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
- Coverage   87.47%   86.36%   -1.11%     
==========================================
  Files           6        6              
  Lines         455      462       +7     
==========================================
+ Hits          398      399       +1     
- Misses         31       36       +5     
- Partials       26       27       +1     
Impacted Files Coverage Δ
deepsubtree.go 63.46% <59.37%> (-6.54%) ⬇️
smt.go 81.33% <100.00%> (+0.93%) ⬆️

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 03ea407...49a51bb. Read the comment docs.

@adlerjohn adlerjohn requested review from liamsi and musalbas July 11, 2021 19:28
smt.go Outdated Show resolved Hide resolved
@adlerjohn adlerjohn requested a review from tzdybal July 11, 2021 19:34
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -59,3 +59,68 @@ func (dsmst *DeepSparseMerkleSubTree) AddBranch(proof SparseMerkleProof, key []b

return nil
}

// GetDescend gets the value of a key from the tree by descending it.
// Use if a key was _not_ previously added with AddBranch, otherwise use Get.
Copy link
Member

Choose a reason for hiding this comment

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

Why would someone try to get a key that wasn't added with AddBranch? In what circumstances would that succeed?

Copy link
Member Author

@adlerjohn adlerjohn Jul 12, 2021

Choose a reason for hiding this comment

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

From #39 (comment)

the code wasn't able to get a key that is expected to have a [non-default] value.

☝️ that exact case. When a key in the original SMT has a non-default value, but wasn't added via AddBranch to the DSMST, then was queried. This happens when a malicious fraud proof doesn't provide enough pre-state. You don't want to return defaultValue in that case because the key doesn't actually have default value; it has a non-default value but that value wasn't proven.

In other words, with an untrusted fraud proof, only GetDescend and HasDescend should be used with the DSMST.

smt.go Outdated Show resolved Hide resolved
@adlerjohn adlerjohn requested a review from musalbas July 12, 2021 17:12
@adlerjohn adlerjohn merged commit 072abf0 into master Jul 12, 2021
@adlerjohn adlerjohn deleted the adlerjohn/update_smt_after_orphan_removal branch July 12, 2021 17:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants