-
Notifications
You must be signed in to change notification settings - Fork 268
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
General Merkle Proof #105
General Merkle Proof #105
Conversation
6c8aad3
to
d9bfcf3
Compare
d9bfcf3
to
d6bb142
Compare
0.9.1 -- compute hash from rangeproof Require Verify() before Verify*() Review fixes from cosmos#75 First commit for generalized merkle proofs Fix imports Bump version to 0.9.3 Return nil on empty tree - bump version to 0.10.0 Add OpDecoders fix test finalize rebase fix test fix toml
d6bb142
to
629dd86
Compare
import ( | ||
"fmt" | ||
|
||
"github.com/tendermint/tendermint/crypto/merkle" |
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.
If IAVL is going to remain its thing/own repo (which I think it should), it would really be nice to not take on more dependencies that couple it tightly to Tendermint at a particular version. Ultimately it would be good to drop dependencies on tendermint altogether if we can spin tmlibs/db out into its own thing see #46.
For ProofOperator
you can just replicate the interface (or even better the subset of the interface you actually use) here - which makes it clearer what this package. For ProofOp
could you just define the type you need here and do a bit of mapping elsewhere when you want to use the protofbuf type?
A little bit of duplication could make this library more lean and loosely coupled.
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.
I agree, but to do after game of stakes.
Could you explain what the overarching goal of this PR is? |
@@ -385,9 +385,9 @@ func (t *ImmutableTree) getRangeProof(keyStart, keyEnd []byte, limit int) (*Rang | |||
pn.Right != nil && !bytes.Equal(pn.Right, node.rightHash) { | |||
|
|||
// We've diverged, so start appending to inners. | |||
pathCount-- | |||
pathCount = -1 |
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 is weird that a change like this doesn't need a change in a test, too.
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.
@mossid can you add a test-case for this?
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.
Made issue #115
@@ -7,7 +7,7 @@ import ( | |||
"strings" | |||
|
|||
"github.com/tendermint/tendermint/crypto/tmhash" | |||
cmn "github.com/tendermint/tendermint/libs/common" | |||
cmn "github.com/tendermint/tmlibs/common" |
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.
Is there a reason this doesn't use github.com/tendermint/tendermint/libs/common
? I mean, if we have to depend on tendermint, than let's use it.
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.
fixed on another PR i'll make.
General Merkle Proof
Depends on: tendermint/tendermint#2298