.github/workflows/sast-linters.yml (2)

Line range hint 82-87: Consider using an array for excluded files.

The current file exclusion logic could be made more maintainable by using an array of excluded files.

-            # Skip this workflow file
-            if [ "$file" == ".github/workflows/sast-linters.yml" ] || [ "$file" == "changelog.md" ]; then
-              echo "Skipping nosec check for $file"
-              continue
-            fi
+            # Define excluded files
+            excluded_files=(".github/workflows/sast-linters.yml" "changelog.md")
+            if [[ " ${excluded_files[@]} " =~ " ${file} " ]]; then
+              echo "Skipping nosec check for excluded file: $file"
+              continue
+            fi

---

Line range hint 143-145: Enhance error reporting for #nosec validation.

The regex pattern and error message could be more descriptive to help developers understand and fix issues.

-          echo "$DIFF" | grep -P '#nosec(?!(\sG\d{3}))(?![^\s\t])([\s\t]*|$)' && echo "nosec without specified rule found!" && exit 1 || exit 0
+          # Detect #nosec annotations that don't specify a rule (e.g., G401)
+          if echo "$DIFF" | grep -P '#nosec(?!(\sG\d{3}))(?![^\s\t])([\s\t]*|$)'; then
+            echo "ERROR: Found #nosec annotations without specific rule numbers."
+            echo "Please specify the exact rule(s) being suppressed (e.g., #nosec G401)."
+            exit 1
+          fi

pkg/ticker/ticker.go (1)

186-186: Fix typo in function comment.

There's a typo in the comment: "secodns" should be "seconds".

-// DurationFromUint64Seconds converts uint64 of secodns to time.Duration.
+// DurationFromUint64Seconds converts uint64 of seconds to time.Duration.

zetaclient/chains/ton/observer/inbound.go (1)

Line range hint 16-18: Enhance documentation for MaxTransactionsPerTick

While the constant's purpose is clear, consider adding more detailed documentation explaining:

• The rationale behind the 100 transaction limit
• Performance implications
• What happens to remaining transactions

Consider updating the comment to:

-// MaxTransactionsPerTick is the maximum number of transactions to process on a ticker
+// MaxTransactionsPerTick limits the number of transactions processed per tick to 100
+// to prevent processing delays and ensure consistent performance. Remaining transactions
+// will be processed in subsequent ticks.

pkg/contracts/ton/gateway.go (1)

127-127: Enhance documentation for security annotations

While the #nosec G115 annotations are correctly placed where type safety is guaranteed by the code structure, it would be beneficial to document the reasoning in more detail. This helps future maintainers understand why these cases are safe.

Consider expanding the comments to explain the safety guarantees:

-// #nosec G115 always in range
+// #nosec G115 -- Safe: op value is validated by the switch statement below which handles unknown cases

Also applies to: 142-142, 151-151

cmd/zetaclientd/p2p_diagnostics.go (2)

Line range hint 158-217: Enhance P2P communication robustness and maintainability.

Several improvements can be made to the peer communication implementation:

1. Define message buffer size as a constant:

+const maxMessageSize = 1024 // Maximum size of P2P diagnostic messages
-	buf := make([]byte, 1024)
+	buf := make([]byte, maxMessageSize)

2. Use defer for stream closure consistently:

 	stream, err := host.NewStream(context.Background(), peer.ID, protocol.ID(ProtocolID))
 	if err != nil {
 		startLogger.Error().Err(err).Msgf("fail to create stream to peer %s", peer)
 		continue
 	}
+	defer func() {
+		if err := stream.Close(); err != nil {
+			startLogger.Warn().Err(err).Msgf("fail to close stream to peer %s", peer)
+		}
+	}()

3. Define a structured message format:

type DiagnosticMessage struct {
    Round    int    `json:"round"`
    FromID   string `json:"from_id"`
    ToID     string `json:"to_id"`
}

These changes will improve code maintainability and reduce error handling complexity.

---

Line range hint 218-229: Consider implementing metrics for P2P diagnostics.

The diagnostic results could be exposed as metrics to enable monitoring and alerting.

Consider adding the following metrics:

• Peer discovery success rate
• Ping-pong latency
• Number of active peers
• Message exchange success rate

This would enable better observability of the P2P network health.

precompiles/staking/staking.go (1)

Line range hint 441-447: Clean up disabled methods implementation.

The disabled methods contain unreachable code after the return statement. While this code is currently commented with //nolint:govet, it would be cleaner to either:

1. Remove the unreachable code until the methods are re-enabled, or
2. Comment out the return statement and keep the implementation if re-enabling is imminent.

Also, consider expanding the comment to include a brief reason for the disabling:

-// Disabled until further notice, check https://github.com/zeta-chain/node/issues/3005.
+// These staking operations are temporarily disabled due to [brief reason].
+// Track re-enabling at https://github.com/zeta-chain/node/issues/3005.

Also applies to: 476-482, 511-517

rpc/backend/node_info.go (1)

84-87: LGTM! Consider enhancing the security comment.

The security comments addressing gosec G115 are appropriate since block heights in blockchain context are indeed always positive. However, consider making the comment more descriptive for future maintainers.

Consider enhancing the comment to:

-// #nosec G115 block height always positive
+// #nosec G115 block height is guaranteed to be positive in blockchain context

rpc/types/utils.go (1)

136-153: LGTM! Consider adding documentation about value guarantees.

The added security comments correctly suppress gosec G115 warnings for values that are guaranteed to be positive in the blockchain context. However, it would be beneficial to add a brief documentation comment explaining why these values are guaranteed to be positive, making it clearer for future maintainers.

Consider adding a documentation comment at the start of the FormatBlock function:

// FormatBlock creates an ethereum block from a tendermint header and ethereum-formatted
// transactions.
// Note: The following fields are guaranteed to be positive:
// - block height: Blockchain heights start from 0 and only increment
// - size: Block size is calculated from actual data
// - gasLimit: Gas limits are configured as positive values
// - timestamp: Block timestamps are Unix timestamps (seconds since epoch)

zetaclient/chains/bitcoin/observer/inbound.go (1)

253-254: LGTM! Consider adding a clarifying comment.

The gosec suppression is justified as the block height in Bitcoin is guaranteed to be positive by the protocol design. Consider adding a brief comment explaining this invariant for future maintainers.

-	// #nosec G115 block height always positive
+	// #nosec G115 Bitcoin block height is guaranteed to be positive by protocol design
 	if !ob.IsBlockConfirmed(uint64(blockVb.Height)) {

zetaclient/chains/evm/observer/outbound.go (1)

Line range hint 1-453: Consider architectural improvements

Several architectural improvements could enhance the codebase:

1. Move configuration values (timeouts, retry limits) to a central configuration
2. Add comprehensive unit tests for error conditions
3. Consider implementing circuit breakers for external calls
4. Add metrics collection for monitoring outbound transaction processing

zetaclient/chains/evm/observer/inbound.go (1)

41-41: LGTM! Consider adding a comment about ticker duration.

The change to DurationFromUint64Seconds improves clarity. Consider adding a brief comment explaining the expected duration range for better maintainability.

+// Convert InboundTicker to duration, expected range: 1s to 1h
 interval := ticker.DurationFromUint64Seconds(ob.ChainParams().InboundTicker)

Configuration used: .coderabbit.yaml
Review profile: CHILL