feat(iavlx): membership and nonmembership proofs #25579
Conversation
github-actions
bot
added
C:x/staking
C:CLI
C:x/gov
C:Keys
|
@technicallyty your pull request is missing a changelog! |
github-actions
bot
removed
C:x/staking
C:CLI
C:x/gov
C:Keys
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return nil, err | ||
| } | ||
| if rightNode != nil { | ||
| rightKey, err := rightNode.Key() |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 6 hours ago
The best way to fix this problem is to properly check and handle the error from the call to rightNode.Key(). Specifically, after line 137, add an error check: if err is not nil, return the error, mirroring how errors are treated after similar lines above (like for leftNode.Key() and others). This ensures any error from Key() does not go unnoticed, aligning with good Go error-handling practice.
- Change required in file
iavlx/immutable_tree.go, lines 137–139:- After assigning
rightKey, err := rightNode.Key(), check iferris non-nil, and return as appropriate.
- After assigning
- No new imports or definitions are needed; simply add the error check.
| @@ -135,7 +135,9 @@ | ||
| } | ||
| if rightNode != nil { | ||
| rightKey, err := rightNode.Key() | ||
|
|
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if rightKey != nil { | ||
| nonexist.Right, err = createExistenceProof(root, rightKey) | ||
| if err != nil { |
aaronc
left a comment
aaronc
left a comment
There was a problem hiding this comment.
I didn't look at the implementation logic in thorough detail yet, but I see there are a few tests. It would be nice to have some tests that generate lots of random cases like the rapid tests we have with TestIAVLXSims. Maybe we could even integrate some proof tests there?
Also for full integration, we need the ABCI Query methods implemented like this:
cosmos-sdk/store/iavl/store.go
Line 315 in d38a914
iavlx/commit_tree.go
Outdated
| return NewImmutableTree(rootPtr), nil | ||
| } | ||
|
|
||
| func (c *CommitTree) GetImmutableImpl(version int64) (*ImmutableTree, error) { |
There was a problem hiding this comment.
Let's just change the return type for the existing GetImmutable method to *ImmutableTree
iavlx/proof.go
Outdated
| // NextIndex returns the index that would be assigned to the key. | ||
| // This method assumes the key does not exist. | ||
| // Callers are expected to check that t.Has(key) == false. | ||
| func NextIndex(node Node, key []byte) (int64, error) { |
There was a problem hiding this comment.
| func NextIndex(node Node, key []byte) (int64, error) { | |
| func nextIndex(node Node, key []byte) (int64, error) { |
Description
Closes: #25573
Implements membership and non-membership proofs on IAVLX tree