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

share: introduce Namespace type #2367

Merged
merged 7 commits into from
Jun 21, 2023
Merged

share: introduce Namespace type #2367

merged 7 commits into from
Jun 21, 2023

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented Jun 14, 2023

Mostly copied from the app's Namespace type with multiple modifications. In preparation for #2301.

We want a more straightforward Namespace type from what the app provides:

  • Repsents 1:1 mapping of the serialized form of Namespace [version byte + namespace id]
  • So that it does not allocate on the hot path when getting data by namespace
  • and had simpler json serialization

@Wondertan Wondertan added the kind:feat Attached to feature PRs label Jun 14, 2023
@Wondertan Wondertan requested a review from rootulp June 14, 2023 19:14
@Wondertan Wondertan self-assigned this Jun 14, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 14, 2023

Codecov Report

Merging #2367 (38cc975) into main (30f0616) will decrease coverage by 0.23%.
The diff coverage is 20.89%.

@@            Coverage Diff             @@
##             main    #2367      +/-   ##
==========================================
- Coverage   50.81%   50.58%   -0.23%     
==========================================
  Files         156      157       +1     
  Lines        9860     9927      +67     
==========================================
+ Hits         5010     5022      +12     
- Misses       4410     4463      +53     
- Partials      440      442       +2     
Impacted Files Coverage Δ
share/nid.go 76.92% <ø> (ø)
share/namespace.go 20.89% <20.89%> (ø)

... and 4 files with indirect coverage changes

distractedm1nd
distractedm1nd previously approved these changes Jun 15, 2023
Copy link
Collaborator

@distractedm1nd distractedm1nd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

share/namespace.go Outdated Show resolved Hide resolved
share/namespace.go Show resolved Hide resolved
share/namespace.go Outdated Show resolved Hide resolved
share/namespace.go Outdated Show resolved Hide resolved
share/namespace.go Show resolved Hide resolved
share/namespace.go Outdated Show resolved Hide resolved
share/namespace.go Outdated Show resolved Hide resolved
share/namespace.go Outdated Show resolved Hide resolved
share/namespace.go Outdated Show resolved Hide resolved
share/namespace.go Show resolved Hide resolved
walldiss
walldiss previously approved these changes Jun 16, 2023
Copy link
Member

@walldiss walldiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful

share/namespace.go Outdated Show resolved Hide resolved
share/namespace.go Outdated Show resolved Hide resolved
Copy link
Member

@walldiss walldiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bless

@Wondertan Wondertan enabled auto-merge June 20, 2023 11:24
@Wondertan Wondertan disabled auto-merge June 20, 2023 11:33
@Wondertan Wondertan enabled auto-merge June 20, 2023 16:06
@Wondertan Wondertan added this pull request to the merge queue Jun 20, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jun 20, 2023
@renaynay renaynay added this pull request to the merge queue Jun 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jun 21, 2023
@Wondertan Wondertan added this pull request to the merge queue Jun 21, 2023
@Wondertan Wondertan removed this pull request from the merge queue due to a manual request Jun 21, 2023
@Wondertan Wondertan enabled auto-merge June 21, 2023 15:23
@Wondertan Wondertan added this pull request to the merge queue Jun 21, 2023
Merged via the queue into main with commit 9adb61e Jun 21, 2023
@Wondertan Wondertan deleted the hlib/share/namespace-type branch June 21, 2023 15:32
github-merge-queue bot pushed a commit that referenced this pull request Jun 27, 2023
## Notable Changes
- `namespace.ID` to  `share.Namespace`
- Changes every comment namespace ID mentioning to just namespace. I
renamed every such mention besides in ADRs. I don't want to touch ADRs
here, as they need a more holistic re-review and up-to-date catchup.
	- Uses all the utility methods on the type, where suitable
- Namespace constructor now only creates Blob Namespaces. For other
reserved namespaces, the predefined globals should be used.
- Uses namespace.ValidateDataNamespace everywhere data is requested.
This is guarantees we verify the namespace are 100% valid and forbids
requesting parity and padding namespaces.
- Restricts PFBs for reserved namespaces 
- Reversed the dependency from `share -> share/ipld` to `share/ipld ->
share`
- NewBlobV0 constructor. Similar to NewNamespaceV0
- `sharetest` pkg for share related testing utilities
- `edstest` pkg for eds related testing utilities

## Follow-ups
- `blobtest` pkg to generate node's blob type


## Refs
* Substitutes zombie PR
#2376. I push to the
branch, but GH does not see commits.
* Based on #2367
* Closes #2301
* Closes #2309 
* Blocked on #2256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feat Attached to feature PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants