Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis change set modernizes linting and formatting infrastructure by upgrading Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant golangci-lint v2
participant Codebase
Developer->>GitHub Actions: Push code / open PR
GitHub Actions->>golangci-lint v2: Run lint and format checks
golangci-lint v2->>Codebase: Apply configured linters and formatters
golangci-lint v2-->>GitHub Actions: Report lint/format results
GitHub Actions-->>Developer: Pass/fail status and feedback
sequenceDiagram
participant Code
participant Linter
participant Formatter
Code->>Linter: Source code analysis (enabled linters only)
Code->>Formatter: Apply formatting (gci, gofmt, goimports, golines)
Linter-->>Code: Lint results (issues, warnings, errors)
Formatter-->>Code: Formatted code output
These diagrams illustrate the updated CI workflow and the new formatting/linting pipeline using golangci-lint v2, reflecting the streamlined and modernized approach introduced by these changes. 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
github-actions
bot
added
ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.golangci.yml (1)
38-52: Define enabled linters explicitly.The curated list focuses on high-value checks. Consider adding
errorlintto catch unchecked errors in control flow.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
.github/workflows/sast-linters.yml(1 hunks).golangci.yml(1 hunks)Makefile(2 hunks)app/export.go(1 hunks)cmd/zetacored/root.go(1 hunks)cmd/zetae2e/stress.go(2 hunks)cmd/zetatool/cctx/cctx_details.go(1 hunks)cmd/zetatool/chains/evm.go(1 hunks)contrib/rpctest/main.go(1 hunks)e2e/e2etests/legacy/test_erc20_deposit_refund.go(2 hunks)e2e/runner/solana_gateway_upgrade.go(1 hunks)e2e/runner/ton/faucet.go(1 hunks)rpc/namespaces/ethereum/debug/api.go(1 hunks)scripts/fmt.sh(0 hunks)scripts/gen-spec.go(2 hunks)testutil/simulation/rand_observer.go(1 hunks)x/crosschain/keeper/abort.go(0 hunks)x/crosschain/keeper/cctx.go(5 hunks)x/crosschain/keeper/cctx_gateway_observers.go(1 hunks)x/crosschain/keeper/cctx_orchestrator_validate_outbound.go(1 hunks)x/crosschain/keeper/inbound_tracker.go(1 hunks)x/crosschain/keeper/msg_server_add_inbound_tracker.go(1 hunks)x/crosschain/keeper/msg_server_add_outbound_tracker.go(1 hunks)x/crosschain/types/inbound_parsing.go(1 hunks)x/fungible/keeper/deposits.go(2 hunks)x/lightclient/keeper/block_header.go(1 hunks)x/lightclient/keeper/chain_state.go(1 hunks)x/observer/keeper/chain_params.go(1 hunks)x/observer/keeper/hooks.go(1 hunks)zetaclient/chains/bitcoin/observer/outbound.go(2 hunks)zetaclient/chains/evm/common/cctx.go(1 hunks)zetaclient/chains/evm/observer/v2_inbound.go(1 hunks)
💤 Files with no reviewable changes (2)
- x/crosschain/keeper/abort.go
- scripts/fmt.sh
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
testutil/simulation/rand_observer.goe2e/runner/ton/faucet.gozetaclient/chains/evm/common/cctx.gox/lightclient/keeper/chain_state.gorpc/namespaces/ethereum/debug/api.gox/crosschain/keeper/cctx_gateway_observers.goe2e/e2etests/legacy/test_erc20_deposit_refund.gozetaclient/chains/evm/observer/v2_inbound.gox/crosschain/types/inbound_parsing.gocmd/zetatool/chains/evm.gox/crosschain/keeper/msg_server_add_outbound_tracker.gox/observer/keeper/hooks.gox/crosschain/keeper/inbound_tracker.gox/crosschain/keeper/cctx_orchestrator_validate_outbound.gox/observer/keeper/chain_params.gox/lightclient/keeper/block_header.goapp/export.goe2e/runner/solana_gateway_upgrade.gocontrib/rpctest/main.gox/fungible/keeper/deposits.gocmd/zetatool/cctx/cctx_details.gox/crosschain/keeper/cctx.goscripts/gen-spec.gox/crosschain/keeper/msg_server_add_inbound_tracker.gocmd/zetacored/root.gozetaclient/chains/bitcoin/observer/outbound.gocmd/zetae2e/stress.go
🧬 Code Graph Analysis (6)
zetaclient/chains/evm/common/cctx.go (1)
testutil/sample/crosschain.go (1)
InboundParams(176-191)
e2e/e2etests/legacy/test_erc20_deposit_refund.go (1)
e2e/utils/evm.go (1)
MustWaitForTxReceipt(19-48)
x/crosschain/keeper/inbound_tracker.go (1)
x/crosschain/types/keys.go (1)
InboundTrackerKeyPrefix(63-63)
x/observer/keeper/chain_params.go (1)
x/observer/types/keys.go (1)
AllChainParamsKey(60-60)
x/lightclient/keeper/block_header.go (1)
x/observer/types/keys.go (1)
BlockHeaderKey(74-74)
x/crosschain/keeper/cctx.go (1)
x/crosschain/types/keys.go (2)
KeyPrefix(37-39)CCTXKey(44-44)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: test-sim-nondeterminism / sim
- GitHub Check: test-sim-fullappsimulation / sim
- GitHub Check: test-sim-import-export / sim
- GitHub Check: test-sim-after-import / sim
- GitHub Check: build-zetanode
- GitHub Check: build-and-test
- GitHub Check: lint
- GitHub Check: gosec
- GitHub Check: analyze (go)
- GitHub Check: build
🔇 Additional comments (46)
e2e/e2etests/legacy/test_erc20_deposit_refund.go (2)
129-129: LGTM: Removed unnecessary receipt assignment.The receipt from the approval transaction is not used in subsequent logic, so removing the assignment improves code clarity while preserving the waiting behavior.
144-144: LGTM: Properly scoped receipt variable.Creating a new local variable for the deposit transaction receipt is appropriate since this receipt is actually used for status checking and logging in the following lines.
contrib/rpctest/main.go (1)
155-155: LGTM: Eliminated redundant receipt assignment.The previous pattern immediately overwrote the receipt from
MustWaitForReceiptwith a call tozevmClient.TransactionReceipt. UsingMustWaitForReceiptpurely for its waiting side effect and obtaining the receipt separately with proper error handling is more efficient and clearer.x/crosschain/keeper/cctx_gateway_observers.go (1)
44-47: LGTM! Excellent simplification of boolean assignment.The direct assignment from the function result is more concise and readable than the previous conditional pattern.
x/crosschain/keeper/cctx_orchestrator_validate_outbound.go (1)
56-56: LGTM! Removed unnecessary string formatting.Direct string assignment is more efficient than
fmt.Sprintffor string literals without format specifiers.cmd/zetacored/root.go (1)
323-323: LGTM! Clean boolean assignment simplification.Direct assignment from the comparison expression is more concise and expressive than conditional blocks.
scripts/gen-spec.go (1)
132-132: LGTM! Excellent optimization replacing WriteString+Sprintf pattern.Using
fmt.Fprintfdirectly to the file is more efficient than the intermediate string creation withfmt.Sprintffollowed byWriteString. This reduces memory allocations and improves performance.Also applies to: 137-137, 142-142, 158-158
cmd/zetatool/cctx/cctx_details.go (1)
124-124: Excellent boolean logic simplification.The application of De Morgan's law transforms the negated OR condition into a direct AND condition, significantly improving readability while maintaining identical logic.
testutil/simulation/rand_observer.go (1)
39-40: Perfect elimination of redundant conditional logic.Directly returning the boolean result from
IsNonTombstonedObserverremoves unnecessary intermediate variable assignment and conditional branching, making the code more concise and idiomatic.zetaclient/chains/bitcoin/observer/outbound.go (2)
459-459: Well-executed boolean logic simplification in critical validation.The transformation from negated OR to direct AND logic in the TSS output validation enhances readability while preserving the exact validation semantics for Bitcoin transaction outputs.
518-518: Consistent logic improvement in cancelled transaction validation.The boolean expression simplification aligns with the pattern established in
checkTSSVout, maintaining consistency across the validation functions while improving code clarity.x/observer/keeper/hooks.go (1)
149-153: Clean negation removal with improved formatting.The transformation from negated equality to direct inequality, combined with the added whitespace, significantly enhances the readability of the self-delegation check logic.
e2e/runner/solana_gateway_upgrade.go (1)
115-116: Excellent resource management improvement.The migration from
time.Ticktotime.NewTickerwith proper cleanup prevents resource leaks and follows Go best practices for production code.x/fungible/keeper/deposits.go (2)
163-164: Clean code improvement through else elimination.Removing the unnecessary else block improves readability and follows the early return pattern, making the control flow more straightforward.
179-181: Consistent application of early return pattern.The elimination of the else block maintains consistency with the previous case and reduces nesting depth, enhancing code clarity.
cmd/zetae2e/stress.go (2)
208-210: Elegant simplification of ticker pattern.The
for range ticker.Cconstruct is more idiomatic than explicitselectstatements for simple ticker loops, improving code readability while maintaining identical functionality.
223-286: Comprehensive modernization of ticker handling.The refactoring from
selectstatements tofor range ticker.Csignificantly reduces boilerplate code while preserving all network metrics calculation logic. This pattern is more concise and aligns with modern Go idioms.e2e/runner/ton/faucet.go (1)
12-12: Proper alignment with golangci-lint v2 configuration.Removing the
stylecheckdirective from the nolint comment aligns with the migration to golangci-lint v2, where this linter was removed from the enabled set. The change maintains only relevant suppression directives.rpc/namespaces/ethereum/debug/api.go (1)
134-134: LGTM! Linter directive cleanup aligns with golangci-lint v2 migration.The removal of deprecated
golintandstylecheckdirectives while retainingreviveis correct for the v2 migration. This modernizes the linting configuration without affecting functionality.x/lightclient/keeper/chain_state.go (1)
15-15: Excellent refactoring! Direct key prefix usage eliminates unnecessary complexity.The simplification from
fmt.Sprintfto directtypes.KeyPrefix(types.ChainStateKey)usage improves code clarity and removes redundant string formatting without altering functionality..github/workflows/sast-linters.yml (1)
50-52: LGTM! CI workflow correctly upgraded for golangci-lint v2 migration.The upgrades to
golangci-lint-action@v8andgolangci-lint v2.1.6are essential for the v2 migration and ensure the CI pipeline uses the latest tooling infrastructure.zetaclient/chains/evm/observer/v2_inbound.go (1)
183-183: Excellent simplification! Direct boolean assignment improves code conciseness.The refactoring from an explicit conditional block to direct assignment of
len(event.Payload) > 0follows clean code principles while maintaining identical functionality.x/lightclient/keeper/block_header.go (1)
17-17: Excellent simplification of key prefix construction.The removal of unnecessary
fmt.Sprintfaround the string constanttypes.BlockHeaderKeyimproves code clarity and performance. SinceBlockHeaderKeyis already a string constant, direct usage withtypes.KeyPrefix()is the appropriate approach.cmd/zetatool/chains/evm.go (1)
178-178: Clean boolean assignment simplification.The direct assignment
isCrossChainCall := len(event.Payload) > 0is more concise and idiomatic than an explicit conditional block. This change improves readability while maintaining identical functionality.x/crosschain/keeper/inbound_tracker.go (1)
97-97: Proper elimination of redundant string formatting.The direct usage of
types.KeyPrefix(types.InboundTrackerKeyPrefix)removes unnecessaryfmt.Sprintfoverhead sinceInboundTrackerKeyPrefixis already a string constant. This change aligns with the broader codebase simplification effort.x/crosschain/types/inbound_parsing.go (1)
124-124: Consistent boolean assignment improvement.The direct assignment
isCrossChainCall := len(event.Message) > 0maintains the backward compatibility logic while improving code conciseness. This change aligns with similar simplifications across the EVM-related codebase.x/observer/keeper/chain_params.go (1)
13-13: LGTM: Clean refactoring eliminates unnecessary string formatting overhead.The direct usage of
types.KeyPrefix(types.AllChainParamsKey)is more efficient than wrapping a string constant infmt.Sprintf, improving both readability and performance.Also applies to: 20-20
x/crosschain/keeper/msg_server_add_inbound_tracker.go (1)
31-31: LGTM: Logical correction improves authorization security.The change from
!(isAuthorizedPolicy || isObserver)to!isAuthorizedPolicy && !isObservercorrectly implements the intended authorization logic, ensuring access is denied only when the creator is neither an authorized policy nor an observer.app/export.go (1)
60-60: LGTM: Concise boolean assignment improves readability.Direct assignment of the boolean expression
len(jailAllowedAddrs) > 0is more idiomatic and readable than using a conditional block for simple boolean logic.x/crosschain/keeper/msg_server_add_outbound_tracker.go (1)
62-62: LGTM: Consistent authorization logic correction.The change from
!(isAuthorizedPolicy || isObserver)to!isAuthorizedPolicy && !isObservercorrectly implements the authorization logic and maintains consistency with the identical fix in the inbound tracker message server.Makefile (2)
154-156: Ensure installation instructions align with project standards.The added reminder to install
golangci-lintv2.1.6 locally is helpful. Verify that the pinned version matches the CI configuration and include this step in the developer setup documentation if not already documented.
165-167: Replace custom formatting script with integrated formatter.Invoking
golangci-lint fmtdirectly simplifies the workflow and avoids an external script. Ensure thatgolangci-lint fmtcovers all formatting requirements previously handled bygolines.zetaclient/chains/evm/common/cctx.go (4)
49-54: Remove redundantelseafter early return for clarity.Refactoring
if…return; returnimproves linear control flow. Confirm that all code paths remain covered by tests for the Gas PendingOutbound case.
55-61: Streamline PendingRevert branch by eliminatingelse.The pattern enhances readability. Ensure no unintended fallthrough occurs for the Gas PendingRevert state.
63-69: Simplify ERC20 PendingOutbound logic.Consistent with other cases, the removal of
elsemaintains clarity. Verify behavior via existing integration tests.
70-76: Refactor ERC20 PendingRevert branch for readability.Unwinding the
if-elsereduces nesting. Confirm test coverage for the ERC20 revert scenarios.x/crosschain/keeper/cctx.go (5)
68-71: Simplify boolean condition.Changing
== falseto!is more idiomatic in Go. Good cleanup.
101-102: UseKeyPrefixdirectly instead offmt.Sprintf.Removing
fmt.Sprintfreduces overhead and unused imports.
116-117: Consistent prefix construction for retrieval.Direct use of
types.KeyPrefixaligns with other keeper methods.
130-132: Standardize key prefix in iterator.This change simplifies prefix setup for iteration.
148-150: Condense delete operation with direct prefix.Using
types.KeyPrefixkeeps the code DRY and avoids string formatting..golangci.yml (5)
1-9: Upgrade to golangci-lint v2 schema and Go 1.23.The version bump and disabling of tests in
runare aligned with the new linter defaults. Ensure CI and local installations use Go 1.23.
10-23: Configure formatters for import order and line length.Enabling
gci,gofmt,goimports, andgolineswith deterministic import sections and a 120-character limit is appropriate.
24-35: Exclude generated and test files from formatting.Excluding
.pb.go, test files, and config directories helps avoid noise.
67-82: Refine exclusions for linters.Preset exclusions and path filters are comprehensive. Confirm that new directories (e.g.,
./scripts) are correctly excluded.
83-97: Filter specific staticcheck warnings.Excluding
QF1002,QF1003,SA1019, andQF1008is done intentionally. Validate that these do not mask critical issues.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3960 +/- ##
===========================================
- Coverage 64.18% 64.10% -0.08%
===========================================
Files 474 474
Lines 34803 34838 +35
===========================================
- Hits 22337 22333 -4
- Misses 11438 11479 +41
+ Partials 1028 1026 -2
🚀 New features to boost your workflow:
|
julianrubino
left a comment
julianrubino
left a comment
There was a problem hiding this comment.
lgtm and workflow did run fine
* Migrate linter to V2 * Apply lint * Improve makefile
* feat: migrate to golangci-lint v2 (#3960) * Migrate linter to V2 * Apply lint * Improve makefile * feat(ton): Implement HTTP-RPC based on toncenter v2 (#3929) * Add TON HTTP-RPC client skeleton * RPC WIP [1] * RPC WIP [2] * RPC WIP [3] * Metrics note * Record metrics * Update changelog * feat(zetaclient): integrate TON HTTP-RPC (#3958) * Refactor observer; refactor mocks * Fix signer & unit tests * Drop liteapi * Restore tx hash conversion aliases * Fix E2E, adapt Tongo x RPC * Update wait-for-ton * Move tlb to rpc package * fix e2e tests; simplify ensureLastScannedTx; fix client * rename .rpc to .Endpoint in config * Simplify config * Update changelog * lint * Address PR comments * PR fixes * fix merge conflict * bump linter timeout * use ton localnet v2 * bump ton localnet * bump ton localnet * Bump changelog
4 participants
Summary by CodeRabbit