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

chore!: migrate to new square version #3742

Merged
merged 8 commits into from
Aug 7, 2024
Merged

Conversation

cmwaters
Copy link
Contributor

This includes the new share version for submitting a verified signer in the blob

@cmwaters cmwaters self-assigned this Aug 5, 2024
Copy link

github-actions bot commented Aug 6, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://celestiaorg.github.io/celestia-app/pr-preview/pr-3742/
on branch gh-pages at 2024-08-06 15:45 UTC

@cmwaters cmwaters marked this pull request as ready for review August 6, 2024 11:59
Copy link
Contributor

coderabbitai bot commented Aug 6, 2024

Walkthrough

Walkthrough

The changes primarily involve a significant update to the go-square library, transitioning from version 1 to version 2. This affects various files across the codebase, particularly in how namespaces and blob transactions are handled. Key modifications include replacing the blob package with the share package, enhancing error handling, and standardizing the creation and processing of blobs and namespaces. Overall, these changes improve the robustness and maintainability of the codebase.

Changes

File(s) Change Summary
app/check_tx.go, app/process_proposal.go, app/validate_txs.go Replaced blob.UnmarshalBlobTx with blobtx.UnmarshalBlobTx, enhancing error handling for blob transactions.
app/errors/*_test.go Updated import paths from namespace to share, reflecting the new structure in the go-square/v2 package.
app/extend_block.go, app/prepare_proposal.go Changed import paths to version 2 of the go-square library and updated function calls to use share.ToBytes.
app/test/*_test.go Modified tests to use share instead of namespace for blob and namespace handling, ensuring compatibility with the new package structure.
pkg/*, test/util/* Updated imports and types from blob and shares to share, including changes in function signatures and internal logic across various files.
go.mod Updated go-square dependency from version 1 to version 2.
specs/* Updated URL references to the new versioning scheme in the documentation.
x/blob/* Updated import paths and usage related to blob transactions, transitioning to the share package.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Blob as Blob Service
    participant Share as Share Service
    participant User as User Interface

    User->>App: Submit Transaction (Transaction Data)
    App->>Blob: Unmarshal Blob Transaction
    Blob-->>App: Return Blob Object or Error
    App->>Share: Process Blob (if valid)
    Share-->>App: Return Share Data or Error
    App-->>User: Response (Success/Error)
Loading
sequenceDiagram
    participant App as Application
    participant Share as Share Service
    participant User as User Interface

    User->>App: Request Namespace
    App->>Share: Create New Namespace
    Share-->>App: Return Namespace Object or Error
    App-->>User: Response (Success/Error)
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (3)
test/util/testfactory/blob.go (3)

31-38: Function GenerateBlobsWithNamespace is not directly tested.

There are no test files directly referencing GenerateBlobsWithNamespace. To ensure the new error handling logic is verified, please add test cases for this function.

  • Add test cases for GenerateBlobsWithNamespace to cover the new error handling logic.
Analysis chain

Improved error handling in GenerateBlobsWithNamespace.

The changes to use share.Blob and handle errors ensure that the function generates valid blobs with namespaces.

However, ensure that all function calls to GenerateBlobsWithNamespace are tested to verify the new error handling logic.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `GenerateBlobsWithNamespace` are tested.

# Test: Search for the function usage. Expect: Ensure test coverage for error handling.
rg --type go -A 5 $'GenerateBlobsWithNamespace'

Length of output: 606


Script:

#!/bin/bash
# Description: Find all occurrences of `GenerateBlobsWithNamespace` in the codebase and check if they are covered by tests.

# Find all occurrences of `GenerateBlobsWithNamespace`
rg --type go 'GenerateBlobsWithNamespace' > function_usages.txt

# Check if these occurrences are within test files or files that are likely to contain tests
rg --type go -l 'GenerateBlobsWithNamespace' | grep -E '_test\.go$' > test_files.txt

# Output the results
echo "Function Usages:"
cat function_usages.txt
echo "Test Files:"
cat test_files.txt

Length of output: 453


60-62: Function GenerateRandomBlobOfShareCount is not used anywhere in the codebase.

The function GenerateRandomBlobOfShareCount is defined but not called or tested in the codebase. Ensure that this function is utilized and properly tested to verify its new logic.

  • File: test/util/testfactory/blob.go
  • Function: GenerateRandomBlobOfShareCount
Analysis chain

Updated to use share.Blob in GenerateRandomBlobOfShareCount.

The changes ensure that the function generates valid random blobs of a specified share count.

However, ensure that all function calls to GenerateRandomBlobOfShareCount are tested to verify the new logic.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `GenerateRandomBlobOfShareCount` are tested.

# Test: Search for the function usage. Expect: Ensure test coverage for the new logic.
rg --type go -A 5 $'GenerateRandomBlobOfShareCount'

Length of output: 618


Script:

#!/bin/bash
# Description: Search for all instances of `GenerateRandomBlobOfShareCount` usage in the codebase.

rg 'GenerateRandomBlobOfShareCount' --type go

Length of output: 236


12-16: Ensure GenerateRandomlySizedBlobs is tested.

The function GenerateRandomlySizedBlobs does not appear to be covered by any tests in the codebase. It is essential to add tests to verify the new error handling logic introduced in this function.

  • Add unit tests for GenerateRandomlySizedBlobs to cover various scenarios, including edge cases and error handling.
Analysis chain

Improved error handling in GenerateRandomlySizedBlobs.

The changes to use share.Blob and handle errors ensure that the function generates valid blobs.

However, ensure that all function calls to GenerateRandomlySizedBlobs are tested to verify the new error handling logic.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `GenerateRandomlySizedBlobs` are tested.

# Test: Search for the function usage. Expect: Ensure test coverage for error handling.
rg --type go -A 5 $'GenerateRandomlySizedBlobs'

Length of output: 463


Script:

#!/bin/bash
# Search for all occurrences of the function `GenerateRandomlySizedBlobs` in the codebase.
rg 'GenerateRandomlySizedBlobs' --type go --context 5

Length of output: 777


Script:

#!/bin/bash
# Search for all test files in the codebase and check if they contain calls to `GenerateRandomlySizedBlobs`.
rg 'GenerateRandomlySizedBlobs' --type go --glob '*_test.go' --context 5

Length of output: 72


Script:

#!/bin/bash
# Search for all occurrences of the function `GenerateRandomlySizedBlobs` in the codebase and trace their usage.
rg 'GenerateRandomlySizedBlobs' --type go --context 10

Length of output: 1186

Copy link
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

Overall LGTM. A few links and comments need to be updated.

@@ -6,8 +6,7 @@ require (
cosmossdk.io/errors v1.0.1
cosmossdk.io/math v1.3.0
github.com/celestiaorg/blobstream-contracts/v3 v3.1.0
github.com/celestiaorg/go-square v1.1.0
github.com/celestiaorg/go-square/merkle v0.0.0-20240117232118-fd78256df076
github.com/celestiaorg/go-square/v2 v2.0.0-rc2
Copy link
Collaborator

Choose a reason for hiding this comment

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

[question] should we cut an official release of go-square and bump to that here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once this is merged confirming that no other changes are necessary I will cut the final release

proto/celestia/core/v1/blob/blob.proto Outdated Show resolved Hide resolved
specs/src/specs/namespace.md Outdated Show resolved Hide resolved
specs/src/specs/shares.md Outdated Show resolved Hide resolved
x/blob/ante/blob_share_decorator_test.go Outdated Show resolved Hide resolved
x/blob/ante/blob_share_decorator_test.go Outdated Show resolved Hide resolved
x/blob/client/cli/payforblob.go Outdated Show resolved Hide resolved
x/blob/client/cli/payforblob.go Outdated Show resolved Hide resolved
@celestia-bot celestia-bot requested a review from a team August 6, 2024 15:23
@celestia-bot celestia-bot requested a review from a team August 6, 2024 15:45
@cmwaters cmwaters merged commit 9523b4c into main Aug 7, 2024
32 of 35 checks passed
@cmwaters cmwaters deleted the cal/integrate-square-v2 branch August 7, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants