-
Notifications
You must be signed in to change notification settings - Fork 293
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
Make distinct go modules for the /pkg
directory and celestia-app modules in /x
#2570
Labels
refactor
optional label for items that are related to implementation work and do not change functionality
Comments
evan-forbes
added
refactor
optional label for items that are related to implementation work and do not change functionality
go.mod
labels
Sep 22, 2023
cmwaters
added a commit
that referenced
this issue
Oct 16, 2023
Ref: #2570 This PR creates a `pkg/blob` where the protos for `Blob` and `BlobTx` is created. The intention is that this becomes it's own go.mod to avoid needing to depend on `celestia-core` and our `cosmos-sdk` fork. Other common structures like namespace should do the same so we can achieve some cannonical representation of the structs between node and core/app The reason for creating the types here instead of in `celestia-core` is so that only `celestia-app` becomes a multi go.mod repo as opposed to either. Additionally, `Blob` and `BlobTx` were never part of the upstream CometBFT. I also made the decision to have a single representation of blobs rather than a proto and go native. The reason being is one of simplification and performance. With large blobs, copying the data across to another type can lead to a noticeable impact. Having a single struct however does have the dowside of potential coversion errors from uint32 to uint8 so data needs to be validated before it can be used (this is also the same as before). This PR also ports over the proofs. I have put this all under a common/v1 proto package.
Note: this should be easier to do for |
cmwaters
added a commit
to celestiaorg/go-square
that referenced
this issue
Dec 14, 2023
Ref: celestiaorg/celestia-app#2570 This PR creates a `pkg/blob` where the protos for `Blob` and `BlobTx` is created. The intention is that this becomes it's own go.mod to avoid needing to depend on `celestia-core` and our `cosmos-sdk` fork. Other common structures like namespace should do the same so we can achieve some cannonical representation of the structs between node and core/app The reason for creating the types here instead of in `celestia-core` is so that only `celestia-app` becomes a multi go.mod repo as opposed to either. Additionally, `Blob` and `BlobTx` were never part of the upstream CometBFT. I also made the decision to have a single representation of blobs rather than a proto and go native. The reason being is one of simplification and performance. With large blobs, copying the data across to another type can lead to a noticeable impact. Having a single struct however does have the dowside of potential coversion errors from uint32 to uint8 so data needs to be validated before it can be used (this is also the same as before). This PR also ports over the proofs. I have put this all under a common/v1 proto package.
This seems close-able because we extracted https://github.com/celestiaorg/go-square . Note: go-square doesn't include the |
0xchainlover
pushed a commit
to celestia-org/celestia-app
that referenced
this issue
Aug 1, 2024
Ref: celestiaorg/celestia-app#2570 This PR creates a `pkg/blob` where the protos for `Blob` and `BlobTx` is created. The intention is that this becomes it's own go.mod to avoid needing to depend on `celestia-core` and our `cosmos-sdk` fork. Other common structures like namespace should do the same so we can achieve some cannonical representation of the structs between node and core/app The reason for creating the types here instead of in `celestia-core` is so that only `celestia-app` becomes a multi go.mod repo as opposed to either. Additionally, `Blob` and `BlobTx` were never part of the upstream CometBFT. I also made the decision to have a single representation of blobs rather than a proto and go native. The reason being is one of simplification and performance. With large blobs, copying the data across to another type can lead to a noticeable impact. Having a single struct however does have the dowside of potential coversion errors from uint32 to uint8 so data needs to be validated before it can be used (this is also the same as before). This PR also ports over the proofs. I have put this all under a common/v1 proto package.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
refactor
optional label for items that are related to implementation work and do not change functionality
In order to help downstream teams avoid dependency hell, we should begin using go modules for some of our packages similar to what most rust projects do.
We should at least do this for the
/pkg
and/x
directories, but could go further by including a go.mod in each subdirectory of/x
.ref celestiaorg/rsmt2d#248 + a bunch of issues in node and rollkit
The text was updated successfully, but these errors were encountered: