IgnoreMaxNS leads to false computing maxNs
Closed this issue · 3 comments
I am moving the issues from our audit collaboration repo to the nmt repo, because we decided to report issues here. It would be easier to follow the progress.
Here is the description of the issue about computing maxNs
with n.ignoreMaxNS
set to true that we discussed about.
We are not sure what is the expected behavior when it comes to calculating maxNs
with n.ignoreMaxNS
set to true.
As we understand the expected behavior under these conditions is to choose a maxNs
that is less than n.precomputedMaxNs
if possible. If this is the case there might be an issue with computing maxNs
here, which can lead to setting it to n.precomputedMaxNs
even if it could be set to a namespaceId that is smaller than n.precomputedMaxNs
.
If we assume the following:
n.ignoreMaxNS = true
leftMinNs != n.precomputedMaxNs
rightMaxNs = n.precomputedMaxNs
rightMinNs < n.precomputedMaxNs
rightMinNs>leftMaxNs
.
Than the maxNs
will be set to n.precomputedMaxNs
even if it should be set to rightMinNs
because it is the largest namespaceId smaller than n.precomputedMaxNs
.
Example with numbers (presented also in the image at the end):
Lets assume that following:
- precomputedMaxNs = 10
- ignoreMaxNS = true
- leftNode: left{minNs: 7; maxNs:8 }
- rightNode: right{minNs: 9; maxNs:10 }
Based on this logic in HashNode
the minNs
and `maxNs' for the node that is being calculated will take the following values:
- minNs = 7 (because it is the lowest value)
- maxNs = 10 or the
precomputedMaxNs
(because theelse
branch of the if statement will be executed and there 10 will obviously be chosen even if theright.minNs
is larger thenleft.maxNs
but not equal toprecomputedMaxNs
).
This numerical example with the current behavior and the behavior we expect is graphically presented in the following image.
Here is the link to the issue on the audit repo.
Resolution of this issue should be aligned with the rephrased comment in Reworded explanation of calculating namespace when IgnoreMaxNamespace is set.
This issue has been resolved through the following pull request: link. The PR incorporates specific unit tests that focus on the calculation of namespace ranges for nodes. These calculations are based on the namespaces of their respective left and right children, specifically when the IgnoreMaxNamespace flag is enabled. These additions to the codebase aim to provide a clear understanding of the expected behavior when this flag is turned on.