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

feat: allowing subscribe to new blocks with namespace #3098

Closed

Conversation

moldis
Copy link
Contributor

@moldis moldis commented Jan 12, 2024

Closes #2737

This PR allows to subscribe to blob by multiple namespaces.

  • Added new function Subscribe, which contains list of Namespaces.
  • Added unit-tests to test function above.
  • Updated RPC methods with function above.

@github-actions github-actions bot added the external Issues created by non node team members label Jan 12, 2024
@distractedm1nd
Copy link
Collaborator

Thank you for taking this on!

I don't think blob Subscribe should return a map of height -> map of namespace -> blob list. This would mean sending all the blobs for all previous heights every block.

I think it would make more sense to return something like a

type BlobsubResponse struct {
          height uint64
          blobsByNamespace BlobsByNamespace
}


Subscribe func(context.Context, []share.Namespace) (<-chan BlobsubResponse, error) `perm:"read"`

I don't like the blobsubresponse name yet but the structure should probably be the same. Thoughts @renaynay, @walldiss ?

@moldis
Copy link
Contributor Author

moldis commented Jan 12, 2024

Thank you for taking this on!

I don't think blob Subscribe should return a map of height -> map of namespace -> blob list. This would mean sending all the blobs for all previous heights every block.

I think it would make more sense to return something like a

type BlobsubResponse struct {
          height uint64
          blobsByNamespace BlobsByNamespace
}


Subscribe func(context.Context, []share.Namespace) (<-chan BlobsubResponse, error) `perm:"read"`

I don't like the blobsubresponse name yet but the structure should probably be the same. Thoughts @renaynay, @walldiss ?

Thanks, good idea. Now looks too overloaded with map, will change it to this structure

blob/service.go Outdated Show resolved Hide resolved
@moldis moldis changed the title WIP feat: allowing subscribe to new blocks with namespace feat: allowing subscribe to new blocks with namespace Jan 13, 2024
@moldis moldis marked this pull request as ready for review January 13, 2024 07:17
@distractedm1nd
Copy link
Collaborator

Closing in favor of #3539

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external Issues created by non node team members
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: allow subscribing to new blocks
4 participants