-
Notifications
You must be signed in to change notification settings - Fork 71
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
Add SmtSpec
for SMT proofs
#57
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
df4bb4f
fix empty branch check
roysc c60c048
add SmtSpec
roysc 05d4f03
add testdata/smt
roysc c02d029
add smt to vector tests
roysc 7a38155
rust add smt spec
roysc ac18e04
rust port smt vector tests
roysc 01524ae
js smt spec
roysc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,26 @@ var TendermintSpec = &ProofSpec{ | |
}, | ||
} | ||
|
||
// SmtSpec constrains the format for SMT proofs (as implemented by github.com/celestiaorg/smt) | ||
var SmtSpec = &ProofSpec{ | ||
LeafSpec: &LeafOp{ | ||
Hash: HashOp_SHA256, | ||
PrehashKey: HashOp_NO_HASH, | ||
PrehashValue: HashOp_SHA256, | ||
Length: LengthOp_NO_PREFIX, | ||
Prefix: []byte{0}, | ||
}, | ||
InnerSpec: &InnerSpec{ | ||
ChildOrder: []int32{0, 1}, | ||
ChildSize: 32, | ||
MinPrefixLength: 1, | ||
MaxPrefixLength: 1, | ||
EmptyChild: make([]byte, 32), | ||
Hash: HashOp_SHA256, | ||
}, | ||
MaxDepth: 256, | ||
} | ||
|
||
// Calculate determines the root hash that matches a given Commitment proof | ||
// by type switching and calculating root based on proof type | ||
// NOTE: Calculate will return the first calculated root in the proof, | ||
|
@@ -216,7 +236,7 @@ func IsLeftMost(spec *InnerSpec, path []*InnerOp) bool { | |
|
||
// ensure every step has a prefix and suffix defined to be leftmost, unless it is a placeholder node | ||
for _, step := range path { | ||
if !hasPadding(step, minPrefix, maxPrefix, suffix) && !leftBranchesAreEmpty(spec, step, 0) { | ||
if !hasPadding(step, minPrefix, maxPrefix, suffix) && !leftBranchesAreEmpty(spec, step) { | ||
return false | ||
} | ||
} | ||
|
@@ -230,7 +250,7 @@ func IsRightMost(spec *InnerSpec, path []*InnerOp) bool { | |
|
||
// ensure every step has a prefix and suffix defined to be rightmost, unless it is a placeholder node | ||
for _, step := range path { | ||
if !hasPadding(step, minPrefix, maxPrefix, suffix) && !rightBranchesAreEmpty(spec, step, int32(last)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good to remove |
||
if !hasPadding(step, minPrefix, maxPrefix, suffix) && !rightBranchesAreEmpty(spec, step) { | ||
return false | ||
} | ||
} | ||
|
@@ -310,12 +330,15 @@ func getPadding(spec *InnerSpec, branch int32) (minPrefix, maxPrefix, suffix int | |
return | ||
} | ||
|
||
// leftBranchesAreEmpty returns true if the padding bytes correspond to all empty children | ||
// on the left side of this branch, ie. it's a valid placeholder on a leftmost path | ||
func leftBranchesAreEmpty(spec *InnerSpec, op *InnerOp, branch int32) bool { | ||
idx := getPosition(spec.ChildOrder, branch) | ||
// leftBranchesAreEmpty returns true if the padding bytes correspond to all empty siblings | ||
// on the left side of a branch, ie. it's a valid placeholder on a leftmost path | ||
func leftBranchesAreEmpty(spec *InnerSpec, op *InnerOp) bool { | ||
idx, err := orderFromPadding(spec, op) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good improvement |
||
if err != nil { | ||
return false | ||
} | ||
// count branches to left of this | ||
leftBranches := idx | ||
leftBranches := int(idx) | ||
if leftBranches == 0 { | ||
return false | ||
} | ||
|
@@ -333,12 +356,15 @@ func leftBranchesAreEmpty(spec *InnerSpec, op *InnerOp, branch int32) bool { | |
return true | ||
} | ||
|
||
// rightBranchesAreEmpty returns true if the padding bytes correspond to all empty children | ||
// on the right side of this branch, ie. it's a valid placeholder on a rightmost path | ||
func rightBranchesAreEmpty(spec *InnerSpec, op *InnerOp, branch int32) bool { | ||
idx := getPosition(spec.ChildOrder, branch) | ||
// rightBranchesAreEmpty returns true if the padding bytes correspond to all empty siblings | ||
// on the right side of a branch, ie. it's a valid placeholder on a rightmost path | ||
func rightBranchesAreEmpty(spec *InnerSpec, op *InnerOp) bool { | ||
idx, err := orderFromPadding(spec, op) | ||
if err != nil { | ||
return false | ||
} | ||
// count branches to right of this one | ||
rightBranches := len(spec.ChildOrder) - 1 - idx | ||
rightBranches := len(spec.ChildOrder) - 1 - int(idx) | ||
if rightBranches == 0 { | ||
return false | ||
} | ||
|
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have a test case (even json file, table test... something generated by a real SMT that we can test against).
Let's verify this works with your SMT proofs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally this was in the cosmos-sdk repo, but I moved it here to keep a single source of truth.
Test cases are here, in the code for the sdk PR. I don't have one with a failed verification yet, but I will add that. Should I add any tests within this repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some basic tests in this repo.
Happy to have it here, just tested here as well