celestiaorg/nmt

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 the else branch of the if statement will be executed and there 10 will obviously be chosen even if the right.minNs is larger then left.maxNs but not equal to precomputedMaxNs ).

This numerical example with the current behavior and the behavior we expect is graphically presented in the following image.

ignoreMaxNamespace

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.