refactor: remove go-da + fix golangcilint - #54
Conversation
WalkthroughThis set of changes updates the project’s configuration, dependencies, and code organization. The Changes
Sequence Diagram(s)sequenceDiagram
participant StartNode
participant DAClient as jsonrpc.NewClient
participant RollkitDA as rollkitda.NewDAClient
StartNode->>DAClient: Initialize DA client (jsonrpc.NewClient)
DAClient-->>StartNode: Return DA client or error
StartNode->>RollkitDA: Pass &daClient.DA to NewDAClient
RollkitDA-->>StartNode: DA integration complete
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
p2p/errors.go (1)
1-5: Add package‑level sentinel error
IntroducingErrNotReadyprovides a clear, reusable sentinel error for the tx gossiper readiness check. Consider adding a Go doc comment aboveErrNotReadyto satisfy Go’s exported identifier documentation conventions and to explain its intended use.rpc/rpc.go (2)
604-613: Flag this TODO for future implementation.The
nolint:staticcheckdirective is appropriate for now, but make sure to track this TODO to implement transaction proof generation in the future.Consider adding an issue to track these unimplemented proof functions for future implementation.
681-687: Flag this TODO for future implementation.Similar to the earlier instance, this
nolint:staticcheckdirective suppresses warnings about unimplemented proof generation. Consider creating a unified approach to implement both these proofs together.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.golangci.yml(1 hunks)Makefile(1 hunks)README.md(4 hunks)adapter/adapter.go(1 hunks)adapter/store.go(1 hunks)go.mod(1 hunks)p2p/errors.go(1 hunks)p2p/gossip.go(1 hunks)rpc/json/handler.go(1 hunks)rpc/json/service.go(1 hunks)rpc/json/ws.go(1 hunks)rpc/rpc.go(6 hunks)rpc/utils.go(1 hunks)server/start.go(4 hunks)server/tmp_adapter.go(0 hunks)server/utils.go(1 hunks)
💤 Files with no reviewable changes (1)
- server/tmp_adapter.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
rpc/rpc.go (1)
p2p/errors.go (1)
ErrNotReady(5-5)
🔇 Additional comments (24)
adapter/store.go (1)
10-10: Consistent import grouping
A blank line was added between the proto import and the internalstorepackage to clearly separate third‑party imports from internal imports. This aligns with the project's import grouping conventions.server/utils.go (1)
10-11: Maintain import grouping consistency
The blank line before the internalconfigimport cleanly separates external dependencies from local packages, improving readability and aligning with the other import reorganizations in this PR.go.mod (1)
8-10: Update and removal of obsolete dependencies
The module requirements forrollkit,rollkit/core, androllkit/dahave been bumped to the new commit and thego-darequirement has been removed. Please rungo mod tidyto ensure there are no leftover references togo-daand that the module graph is clean.README.md (4)
10-10: Improve list spacing
A blank line was added before the bullet list under “This adapter connects various components…” to enhance readability.
64-64: Enhance section separation
Adding a blank line before the “Dependencies” list visually separates sections and improves markdown structure.
232-232: Specify code block language
Changing the Project Structure snippet to use “```bash” enables proper syntax highlighting in supported renderers.
248-248: Ensure newline at EOF
Adding the missing newline at the end of the file adheres to POSIX text file conventions and avoids potential diffs in future edits.server/start.go (3)
34-37: Import restructuring improves organizationThe imports have been reorganized to replace
go-dawith direct usage of Rollkit's DA package. This aligns well with the PR objective of removing the go-da dependency.
370-374: DA client initialization simplifiedThe DA client creation has been refactored to use
jsonrpc.NewClientdirectly instead of usinggoda.NewClient. The error handling has also been improved with a more descriptive error message.
377-377: Temporary adapter removalThe code no longer uses a temporary adapter between different DA client interfaces. Instead, it directly passes
&daClient.DAto therollkitda.NewDAClient, which simplifies the codebase..golangci.yml (3)
1-1: Updated golangci-lint to version 2The configuration now explicitly specifies version "2", which aligns with current golangci-lint standards.
7-17: Improved linter configuration structureThe revive linter rules are now properly organized under
linters.settings, which follows the correct structure for golangci-lint v2.
18-32: Added formatter configurationsThe addition of the
gciformatter and specific configurations for bothgoimportsandgciwill ensure consistent code formatting, particularly for import organization.The defined sections and custom ordering for imports will create a standardized pattern:
- Standard library imports
- External dependencies
- Rollkit-specific imports
- Project-specific imports
Makefile (1)
39-39: Simplified linting processThe lint target now only runs
golangci-lint run --fix, which streamlines the linting process. This change aligns with the updated golangci-lint configuration.Note that markdown linting is still available in the separate
fmttarget, while the automated workflow now focuses on Go code linting and fixing.p2p/gossip.go (1)
3-11: Import statement reorderingThe import of
cosmossdk.io/loghas been moved above the libp2p-related imports, which improves code organization. This is a minor change that follows standard Go code style practices.rpc/json/ws.go (1)
8-8: Import reorganization looks good.The import statement for the log package has been moved to improve organization.
rpc/json/service.go (1)
13-13: Import cleanup looks good.The log package import has been moved up to be grouped with other imports, removing a duplicate import.
rpc/json/handler.go (1)
15-15: Import reorganization looks good.The log package import has been moved to a better position for consistency with other files.
adapter/adapter.go (1)
12-13: Import cleanup looks good.The imports have been reorganized, improving code organization and removing duplicate imports.
Also applies to: 22-23
rpc/utils.go (1)
102-106: Fix for transaction and data hash assignment.The changes correctly assign transactions to
abciBlock.Txsinstead ofabciBlock.Data.Txsand the data hash toabciBlock.DataHashinstead ofabciBlock.Header.DataHash, which aligns with the proper structure of thecmtypes.Blocktype.This fixes potential issues with data not being stored in the expected fields.
rpc/rpc.go (4)
13-38: Import organization looks clean and clear.Nice job organizing and grouping the imports properly. The separation between standard library imports, third-party libraries, and internal packages improves readability.
515-515: Good use of centralized error constant.Replacing the hardcoded error with the predefined
execp2p.ErrNotReadyconstant fromp2p/errors.goimproves code maintainability and consistency.
721-721: Good variable scope limitation.Limiting the scope of the
endvariable to the pagination block is a good practice. It prevents variable leakage and potential bugs.
869-871: Clean package-level adjustment for peer info.Updated to use
p2p.DefaultNodeInfoinstead of the previous type, maintaining consistent usage of the p2p package throughout the codebase.
3 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores