-
Notifications
You must be signed in to change notification settings - Fork 286
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
Incorporate ParitySharesNamespace and TailPaddingNamespace into IsReserved function #2138
Comments
Agreed, I think it makes sense to have one function (i.e. celestia-app/pkg/namespace/consts.go Lines 54 to 56 in 1b1078c
|
@rootulp Could you please provide some insight into the intended usage of the namespace constants (e.g., Update: |
They are used outside of tests. I expect they will be use externally by clients but I don't think celestia-app strictly needs to export them as long as they are defined in the specs and external clients can re-define them (if they need them).
|
…to IsReserved function (#2194) ## Overview Closes ##2138 This PR also addresses the concerns raised in the [comment](#2138 (comment)) by introducing a distinction between two types of reserved namespaces. Namespaces within the range [0-255] are now identified as "Primary Reserved Namespaces," while any other reserved namespaces beyond this range are categorized as "Non-Primary Reserved Namespaces." Hopefully, this differentiation provides better clarity and organization regarding namespace allocation and usage within the code. ## 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
…to IsReserved function (#2194) Closes ##2138 This PR also addresses the concerns raised in the [comment](#2138 (comment)) by introducing a distinction between two types of reserved namespaces. Namespaces within the range [0-255] are now identified as "Primary Reserved Namespaces," while any other reserved namespaces beyond this range are categorized as "Non-Primary Reserved Namespaces." Hopefully, this differentiation provides better clarity and organization regarding namespace allocation and usage within the code. - [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
…to IsReserved function (#2194) ## Overview Closes ##2138 This PR also addresses the concerns raised in the [comment](#2138 (comment)) by introducing a distinction between two types of reserved namespaces. Namespaces within the range [0-255] are now identified as "Primary Reserved Namespaces," while any other reserved namespaces beyond this range are categorized as "Non-Primary Reserved Namespaces." Hopefully, this differentiation provides better clarity and organization regarding namespace allocation and usage within the code. ## 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 (cherry picked from commit b3b8611) # Conflicts: # pkg/namespace/consts.go # pkg/namespace/namespace.go # specs/src/specs/namespace.md
…to IsReserved function (#2194) ## Overview Closes #celestiaorg/celestia-app#2138 This PR also addresses the concerns raised in the [comment](celestiaorg/celestia-app#2138 (comment)) by introducing a distinction between two types of reserved namespaces. Namespaces within the range [0-255] are now identified as "Primary Reserved Namespaces," while any other reserved namespaces beyond this range are categorized as "Non-Primary Reserved Namespaces." Hopefully, this differentiation provides better clarity and organization regarding namespace allocation and usage within the code. ## 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
…to IsReserved function (#2194) ## Overview Closes #celestiaorg/celestia-app#2138 This PR also addresses the concerns raised in the [comment](celestiaorg/celestia-app#2138 (comment)) by introducing a distinction between two types of reserved namespaces. Namespaces within the range [0-255] are now identified as "Primary Reserved Namespaces," while any other reserved namespaces beyond this range are categorized as "Non-Primary Reserved Namespaces." Hopefully, this differentiation provides better clarity and organization regarding namespace allocation and usage within the code. ## 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
Proposal
The
IsReserved
function currently only checks for namespaces that are less than the maximum reserved namespace. However, according to the specifications, theParitySharesNamespace
andTailPaddingNamespace
are also considered reserved namespaces. Therefore, conceptually, it is expected that theIsReserved
function should take these two namespaces into account when verifying a namespace.Note that the current implementation ensures that
IsParityShares
andIsTailPadding
are called alongside theIsReserved
when needed, however, incorporating these two checks inside theIsReserved
function is a more clear approach.The text was updated successfully, but these errors were encountered: