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.

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

Check ordering of namespace among child nodes in HashNode function #129

Closed
andrijamitrovic23 opened this issue Mar 13, 2023 · 0 comments · Fixed by #187
Closed

Check ordering of namespace among child nodes in HashNode function #129

andrijamitrovic23 opened this issue Mar 13, 2023 · 0 comments · Fixed by #187
Assignees
Labels
audit Issue uncovered during Informal audit

Comments

@andrijamitrovic23
Copy link
Collaborator

This testCase submits children nodes with invalid namespace ordering to HashNode function. The left child node has minNs larger than the maxNs of the same node, and in addition to that leftMinNs is even larger then the rightMinNs. The test will pass because HashNode function panics (in validateSiblingsNamespaceOrder) only if the rightMinNs is less than leftMaxNs.

Evan if the before mentioned test case is totally invalid based on the fact that the Push function will guarantee that the namespace ids will be ordered, it raises two questions or two paths for this issue:

  1. Should this test case be removed?
  2. Should validateSiblingsNamespaceOrder check the ordering of leftMinNs, leftMaxNs, rightMinNs and rightMaxNs among children nodes?

I would suggest that the 'validateSiblingsNamespaceOrder' checks the ordering of the children namespaces just to be more robust, but the decisions is on the development team.

@rootulp rootulp added the audit Issue uncovered during Informal audit label Mar 29, 2023
@staheri14 staheri14 self-assigned this May 1, 2023
staheri14 added a commit that referenced this issue May 4, 2023
## Overview
Closes #129.

## Checklist
- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [x] Visual proof for any user facing features like CLI or
documentation updates
- [x] Linked issues closed with keywords
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit Issue uncovered during Informal audit
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants