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
@adlerjohn adlerjohn requested a review from tzdybal July 11, 2021 19:34
@@ -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.

@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