Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: remove RangeProofs #586

Merged
merged 12 commits into from
Oct 19, 2022
Merged

fix: remove RangeProofs #586

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
01890c8
remove ProofRange
cool-develope Oct 11, 2022
8dce332
refactor proof
cool-develope Oct 11, 2022
91ad406
remove unnecessary ones
cool-develope Oct 13, 2022
7db6e22
Merge branch 'master' into 584/remove-proofrange
cool-develope Oct 13, 2022
a87b7b6
small fix
cool-develope Oct 13, 2022
31bab40
test fix
cool-develope Oct 13, 2022
7a7dfca
remove proto
cool-develope Oct 13, 2022
bd9353b
changelog
cool-develope Oct 14, 2022
87cbe24
Merge branch 'master' into 584/remove-proofrange
cool-develope Oct 17, 2022
3f1d232
Update proof_test.go
cool-develope Oct 17, 2022
9c9915c
revert the isExist
cool-develope Oct 18, 2022
fa0d2b3
Merge branch 'master' into 584/remove-proofrange
tac0turtle Oct 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- [#586](https://github.com/cosmos/iavl/pull/586) Remove the `RangeProof` and refactor the ics23_proof to use the internal methods.

## 0.19.3 (October 8, 2022)

- `ProofInner.Hash()` prevents both right and left from both being set. Only one is allowed to be set.
Expand Down
2 changes: 1 addition & 1 deletion basic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ func TestTreeProof(t *testing.T) {
assert.Equal(t, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", hex.EncodeToString(hash))

// should get false for proof with nil root
_, err = tree.GetWithProof([]byte("foo"))
_, err = tree.GetProof([]byte("foo"), true)
require.Error(t, err)

// insert lots of info and store the bytes
Expand Down
4 changes: 2 additions & 2 deletions mutable_tree_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestDelete(t *testing.T) {

require.NoError(t, tree.DeleteVersion(version))

proof, err := tree.GetVersionedWithProof([]byte("k1"), version)
proof, err := tree.GetVersionedProof([]byte("k1"), true, version)
require.EqualError(t, err, ErrVersionDoesNotExist.Error())
require.Nil(t, proof)

Expand All @@ -129,7 +129,7 @@ func TestDelete(t *testing.T) {
require.NoError(t, err)
tree.versions[version] = true

proof, err = tree.GetVersionedWithProof([]byte("k1"), version)
proof, err = tree.GetVersionedProof([]byte("k1"), true, version)
require.Nil(t, err)
require.Equal(t, 0, bytes.Compare([]byte("Fred"), proof.GetExist().Value))
}
Expand Down
16 changes: 6 additions & 10 deletions proof_ics23.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,12 @@ func convertVarIntToBytes(orig int64, buf [binary.MaxVarintLen64]byte) []byte {
return buf[:n]
}

// GetWithProof gets the proof for the given key.
func (t *ImmutableTree) GetWithProof(key []byte) (*ics23.CommitmentProof, error) {
// GetProof gets the proof for the given key.
func (t *ImmutableTree) GetProof(key []byte, isExist bool) (*ics23.CommitmentProof, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Seems weird to me that we would expose the API to take in an isExist bool.

Doesn't it make more sense to just check if key exists in the tree.

If it does, return an existence proof otherwise return a nonexistence proof

if t.root == nil {
return nil, fmt.Errorf("cannot generate the proof with nil root")
}
value, err := t.Get(key)
if err != nil {
return nil, err
}
if value != nil {
if isExist {
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry for not being more specific, I think you can use the Has function

exist, err := t.Has(key)
if err != nil {
    return nil, err
}

if exist {
    return t.GetMembershipProof(key)
}

return t.GetNonMembershipProof(key)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, it's a simple solution but I'd like to add isExist param to be more clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess when the SDK queries for this value, it should be performing the Has query then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, I think it would be related to each use case, here my purpose is to reduce checks in the GetProof function.

return t.GetMembershipProof(key)
}
return t.GetNonMembershipProof(key)
Expand All @@ -202,14 +198,14 @@ func (t *ImmutableTree) VerifyProof(proof *ics23.CommitmentProof, key []byte) (b
return t.VerifyNonMembership(proof, key)
}

// GetVersionedWithProof gets the proof for the given key at the specified version.
func (tree *MutableTree) GetVersionedWithProof(key []byte, version int64) (*ics23.CommitmentProof, error) {
// GetVersionedProof gets the proof for the given key at the specified version.
func (tree *MutableTree) GetVersionedProof(key []byte, isExist bool, version int64) (*ics23.CommitmentProof, error) {
if tree.VersionExists(version) {
t, err := tree.GetImmutable(version)
if err != nil {
return nil, err
}
return t.GetWithProof(key)
return t.GetProof(key, isExist)
}
return nil, ErrVersionDoesNotExist
}
2 changes: 1 addition & 1 deletion proof_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestTreeKeyExistsProof(t *testing.T) {
require.NoError(t, err)

// should get error
_, err = tree.GetWithProof([]byte("foo"))
_, err = tree.GetProof([]byte("foo"), true)
assert.Error(t, err)

// insert lots of info and store the bytes
Expand Down
14 changes: 7 additions & 7 deletions tree_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ func TestVersionedTreeErrors(t *testing.T) {

// Same thing with proof. We get an error because a proof couldn't be
// constructed.
_, err = tree.GetVersionedWithProof([]byte("key"), 404)
_, err = tree.GetVersionedProof([]byte("key"), true, 404)
require.Error(err)
}

Expand Down Expand Up @@ -1217,14 +1217,14 @@ func TestVersionedTreeProofs(t *testing.T) {
iTree, err := tree.GetImmutable(1)
require.NoError(err)

proof, err := tree.GetVersionedWithProof([]byte("k2"), 1)
proof, err := tree.GetVersionedProof([]byte("k2"), true, 1)
require.NoError(err)
require.EqualValues(proof.GetExist().Value, []byte("v1"))
res, err := iTree.VerifyProof(proof, []byte("k2"))
require.NoError(err)
require.True(res)

proof, err = tree.GetVersionedWithProof([]byte("k4"), 1)
proof, err = tree.GetVersionedProof([]byte("k4"), false, 1)
require.NoError(err)
require.EqualValues(proof.GetNonexist().Key, []byte("k4"))
res, err = iTree.VerifyProof(proof, []byte("k4"))
Expand All @@ -1233,14 +1233,14 @@ func TestVersionedTreeProofs(t *testing.T) {

iTree, err = tree.GetImmutable(2)
require.NoError(err)
proof, err = tree.GetVersionedWithProof([]byte("k2"), 2)
proof, err = tree.GetVersionedProof([]byte("k2"), true, 2)
require.NoError(err)
require.EqualValues(proof.GetExist().Value, []byte("v2"))
res, err = iTree.VerifyProof(proof, []byte("k2"))
require.NoError(err)
require.True(res)

proof, err = tree.GetVersionedWithProof([]byte("k1"), 2)
proof, err = tree.GetVersionedProof([]byte("k1"), true, 2)
require.NoError(err)
require.EqualValues(proof.GetExist().Value, []byte("v1"))
res, err = iTree.VerifyProof(proof, []byte("k1"))
Expand All @@ -1249,7 +1249,7 @@ func TestVersionedTreeProofs(t *testing.T) {

iTree, err = tree.GetImmutable(3)
require.NoError(err)
proof, err = tree.GetVersionedWithProof([]byte("k2"), 3)
proof, err = tree.GetVersionedProof([]byte("k2"), false, 3)
require.NoError(err)
require.EqualValues(proof.GetNonexist().Key, []byte("k2"))
res, err = iTree.VerifyProof(proof, []byte("k2"))
Expand Down Expand Up @@ -1316,7 +1316,7 @@ func TestVersionedTreeHash(t *testing.T) {
_, _, err = tree.SaveVersion()
require.NoError(err)

proof, err := tree.GetVersionedWithProof([]byte("I"), 2)
proof, err := tree.GetVersionedProof([]byte("I"), true, 2)
require.NoError(err)
require.EqualValues([]byte("F"), proof.GetExist().Value)
iTree, err := tree.GetImmutable(2)
Expand Down