Feat/add missing explorer endpoints #274
Conversation
…d also example scripts for the newly supported endpoints
|
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.62.2)level=warning msg="[lintersdb] The name "gas" is deprecated. The linter has been renamed to: gosec." WalkthroughThe pull request introduces extensive modifications to the Injective blockchain explorer client, particularly focusing on the restructuring of the Changes
Sequence DiagramsequenceDiagram
participant Client
participant ExplorerService
participant Blockchain
Client->>ExplorerService: FetchContractTxs()
ExplorerService->>Blockchain: Query Contract Transactions
Blockchain-->>ExplorerService: Return Transaction Data
ExplorerService-->>Client: Provide Transaction Details
Client->>ExplorerService: FetchValidators()
ExplorerService->>Blockchain: Retrieve Validator List
Blockchain-->>ExplorerService: Return Validator Information
ExplorerService-->>Client: Provide Validator Details
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
client/explorer/explorer.go (1)
Line range hint
1-15: Address import formatting issuesThe
go-importscheck failed and modified the file, indicating that the import statements' formatting does not comply with the project's conventions. Please rungoimportsor your IDE's formatter to fix the import order and formatting issues.🧰 Tools
🪛 GitHub Actions: pre-commit
[error] go-imports check failed and modified the file
🧹 Nitpick comments (13)
examples/explorer/18_GetValidators/example.go (1)
32-33: Handle potential error fromjson.MarshalIndentCurrently, the error returned by
json.MarshalIndentis being ignored. It's good practice to handle this error to ensure robustness and to catch unexpected issues during JSON marshalling.Apply this diff to handle the error:
- str, _ := json.MarshalIndent(response, "", " ") + str, err := json.MarshalIndent(response, "", " ") + if err != nil { + log.Fatalf("Failed to marshal JSON: %v", err) + }examples/explorer/21_Relayers/example.go (1)
35-36: Handle potential error fromjson.MarshalIndentThe error returned by
json.MarshalIndentis being ignored. Handling this error will make your code more robust and help catch unexpected issues during JSON marshalling.Apply this diff to handle the error:
- str, _ := json.MarshalIndent(response, "", " ") + str, err := json.MarshalIndent(response, "", " ") + if err != nil { + log.Fatalf("Failed to marshal JSON: %v", err) + }examples/explorer/16_GetContractTxs/example.go (3)
23-24: Consider extracting timeout duration as a constantThe 10-second timeout is a magic number. Consider defining it as a named constant at package level for better maintainability and reusability across examples.
+const defaultTimeout = 10 * time.Second + func main() { // ... - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout)
26-32: Improve documentation for example contract addressThe comment should provide more context about what type of contract this address represents and where users can find valid contract addresses for testing.
- // Example contract address (replace with an actual contract address) + // Example contract address for testing + // Replace with a valid smart contract address from the Injective testnet + // You can find test contract addresses at: https://testnet.explorer.injective.network/Also, consider extracting the limit as a named constant:
+const defaultTxLimit = 10 + req := &explorerPB.GetContractTxsRequest{ Address: contractAddress, - Limit: 10, // Fetch 10 transactions + Limit: defaultTxLimit, }
46-47: Handle JSON marshaling errorThe error from
json.MarshalIndentis ignored. While unlikely to fail for valid responses, it's best practice to handle all errors in example code.- str, _ := json.MarshalIndent(response, "", " ") - fmt.Print(string(str)) + str, err := json.MarshalIndent(response, "", " ") + if err != nil { + log.Fatalf("Failed to marshal response: %v", err) + } + fmt.Println(string(str))examples/explorer/19_GetValidator/example.go (1)
26-28: Improve documentation for example validator addressThe comment should provide more context about validator addresses and where to find them.
- // Example validator address (replace with an actual validator address) + // Example validator address for testing + // Replace with a valid validator address from the Injective testnet + // Format: injvaloper1<hex> (45 characters) + // You can find test validator addresses at: https://testnet.explorer.injective.network/validatorsexamples/explorer/22_GetBankTransfers/example.go (3)
24-25: Align timeout duration with other examplesThis example uses a 60-second timeout while others use 10 seconds. Consider using the same timeout duration across all examples for consistency unless there's a specific reason for the longer timeout.
27-31: Improve documentation for example sender address and consider request optionsThe sender address lacks documentation, and the limit comment could be more informative.
+ // Example sender address for testing + // Replace with a valid account address from the Injective testnet + // You can find test accounts at: https://testnet.explorer.injective.network/accounts req := &explorerPB.GetBankTransfersRequest{ Senders: []string{"inj17xpfvakm2amg962yls6f84z3kell8c5l6s5ye9"}, - Limit: 5, // Limit number of transfers + Limit: 5, // Maximum number of transfers to fetch }Also consider documenting other available request options that users can explore:
// Available request options: // - Recipients: []string - Filter by recipient addresses // - Skip: uint64 - Number of records to skip // - StartTime: *timestamp.Timestamp - Filter by start time // - EndTime: *timestamp.Timestamp - Filter by end time
1-40: Consider creating a shared examples package for common utilitiesAll examples share similar initialization, timeout handling, and JSON formatting code. Consider creating a shared package to reduce duplication.
This could include:
- Common timeout constants
- Helper functions for client initialization
- JSON formatting utilities with proper error handling
- Documentation templates for addresses
Would you like me to provide a detailed implementation for this shared package?
examples/explorer/17_GetContractTxsV2/example.go (4)
15-22: Consider making network configuration configurable.While the implementation is correct, hardcoding the network values ("testnet", "lb") limits the example's flexibility. Consider accepting these as command-line arguments or environment variables to make the example more versatile.
+import ( + "os" + "github.com/InjectiveLabs/sdk-go/client/common" +) func main() { - network := common.LoadNetwork("testnet", "lb") + networkType := os.Getenv("NETWORK_TYPE") + if networkType == "" { + networkType = "testnet" // default to testnet + } + networkEndpoint := os.Getenv("NETWORK_ENDPOINT") + if networkEndpoint == "" { + networkEndpoint = "lb" // default to lb + } + network := common.LoadNetwork(networkType, networkEndpoint)
27-38: Add documentation about the contract address format and pagination.A few suggestions to improve this section:
- Add a comment explaining the expected format of Injective contract addresses
- Consider implementing pagination handling for production use cases
- Validate the contract address format before making the API call
-// Example contract address (replace with an actual contract address) +// Example contract address in Injective bech32 format with 'inj' prefix +// For production use, implement pagination using the Skip parameter +// and validate the address format using sdk.AccAddress contractAddress := "inj1ady3s7whq30l4fx8sj3x6muv5mx4dfdlcpv8n7" +// Validate address format +if !strings.HasPrefix(contractAddress, "inj1") { + log.Fatal("Invalid address format: must start with 'inj1'") +} req := &explorerPB.GetContractTxsV2Request{ Address: contractAddress, - Limit: 10, // Fetch 10 transactions + Limit: 10, // Maximum number of transactions to fetch per request + Skip: 0, // Skip parameter for pagination }
40-49: Improve error handling and logging structure.The response handling could be improved in several ways:
- Handle JSON marshaling errors
- Use structured logging for better output parsing
- Consider adding transaction type information to the output
-fmt.Printf("\n\n") -fmt.Println("Full response:") -str, _ := json.MarshalIndent(response, "", " ") -fmt.Print(string(str)) +fmt.Println("\nFull response:") +str, err := json.MarshalIndent(response, "", " ") +if err != nil { + log.Fatalf("Failed to marshal response: %v", err) +} +fmt.Println(string(str))
1-49: Good example implementation with room for production enhancements.This example effectively demonstrates the basic usage of the
FetchContractTxsV2endpoint. For production use, consider:
- Adding retry logic for network failures
- Implementing rate limiting
- Adding metrics/tracing
- Including proper cleanup of resources in case of panics
Would you like me to provide an enhanced version of this example with these production-ready features?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
client/explorer/explorer.go(4 hunks)examples/explorer/16_GetContractTxs/example.go(1 hunks)examples/explorer/17_GetContractTxsV2/example.go(1 hunks)examples/explorer/18_GetValidators/example.go(1 hunks)examples/explorer/19_GetValidator/example.go(1 hunks)examples/explorer/20_GetValidatorUptime/example.go(1 hunks)examples/explorer/21_Relayers/example.go(1 hunks)examples/explorer/22_GetBankTransfers/example.go(1 hunks)examples/explorer/5_TxsRequest/example.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/explorer/5_TxsRequest/example.go
🧰 Additional context used
🪛 GitHub Actions: pre-commit
client/explorer/explorer.go
[warning] 35-35: Interface method parameter 'marketIds' should be named 'marketIDs' according to naming convention
[error] go-imports check failed and modified the file
🔇 Additional comments (3)
examples/explorer/19_GetValidator/example.go (1)
35-36: Handle JSON marshaling errorSame issue as in GetContractTxs example.
examples/explorer/17_GetContractTxsV2/example.go (2)
1-13: LGTM! Clean and well-organized imports.The imports are properly organized following Go conventions, with standard library imports grouped separately from external dependencies.
24-25: LGTM! Proper context handling with timeout.The context implementation with a reasonable timeout and deferred cancellation follows Go best practices.
client/explorer/explorer.go
Outdated
| GetWasmCodes(ctx context.Context, req *explorerPB.GetWasmCodesRequest) (*explorerPB.GetWasmCodesResponse, error) | ||
| GetWasmCodeByID(ctx context.Context, req *explorerPB.GetWasmCodeByIDRequest) (*explorerPB.GetWasmCodeByIDResponse, error) | ||
| GetWasmContracts(ctx context.Context, req *explorerPB.GetWasmContractsRequest) (*explorerPB.GetWasmContractsResponse, error) | ||
| GetWasmContractByAddress(ctx context.Context, req *explorerPB.GetWasmContractByAddressRequest) (*explorerPB.GetWasmContractByAddressResponse, error) | ||
| GetCW20Balance(ctx context.Context, req *explorerPB.GetCw20BalanceRequest) (*explorerPB.GetCw20BalanceResponse, error) | ||
| FetchRelayers(ctx context.Context, marketIds []string) (*explorerPB.RelayersResponse, error) |
There was a problem hiding this comment.
Fix inconsistent parameter naming in 'FetchRelayers' method
The parameter marketIds should be renamed to marketIDs to follow Go naming conventions and to ensure consistency with the method implementation and usage. This change will also resolve the pipeline warning.
Apply this diff to fix the parameter name:
- FetchRelayers(ctx context.Context, marketIds []string) (*explorerPB.RelayersResponse, error)
+ FetchRelayers(ctx context.Context, marketIDs []string) (*explorerPB.RelayersResponse, error)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FetchRelayers(ctx context.Context, marketIds []string) (*explorerPB.RelayersResponse, error) | |
| FetchRelayers(ctx context.Context, marketIDs []string) (*explorerPB.RelayersResponse, error) |
🧰 Tools
🪛 GitHub Actions: pre-commit
[error] go-imports check failed and modified the file
| var marketIds []string | ||
|
|
||
| response, err := explorerClient.FetchRelayers(ctx, marketIds) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename variable marketIds to marketIDs to follow Go naming conventions
Per Go naming conventions, acronyms like "ID" should be capitalized. Renaming marketIds to marketIDs ensures consistency and readability.
Apply this diff to rename the variable and update the function call:
- var marketIds []string
+ var marketIDs []string
- response, err := explorerClient.FetchRelayers(ctx, marketIds)
+ response, err := explorerClient.FetchRelayers(ctx, marketIDs)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var marketIds []string | |
| response, err := explorerClient.FetchRelayers(ctx, marketIds) | |
| var marketIDs []string | |
| response, err := explorerClient.FetchRelayers(ctx, marketIDs) |
| explorerClient, err := explorer.NewExplorerClient(network) | ||
| if err != nil { | ||
| log.Fatalf("Failed to create explorer client: %v", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
Add missing defer for client.Close()
Resource cleanup is incomplete. Add defer explorerClient.Close() after client creation to ensure proper cleanup.
if err != nil {
log.Fatalf("Failed to create explorer client: %v", err)
}
+ defer explorerClient.Close()Committable suggestion skipped: line range outside the PR's diff.
| validatorAddress := "injvaloper1..." | ||
|
|
There was a problem hiding this comment.
Replace invalid validator address placeholder
The current placeholder "injvaloper1..." is invalid and won't compile if users try to run the example without modification. Consider using a valid testnet validator address as a placeholder.
- validatorAddress := "injvaloper1..."
+ // Example validator address for testing
+ // Replace with a valid validator address from the Injective testnet
+ validatorAddress := "injvaloper1kk523rsm9pey740cx4plalp40009ncs0wrchfe"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| validatorAddress := "injvaloper1..." | |
| // Example validator address for testing | |
| // Replace with a valid validator address from the Injective testnet | |
| validatorAddress := "injvaloper1kk523rsm9pey740cx4plalp40009ncs0wrchfe" | |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
client/explorer/explorer.go (3)
13-40: Consider reorganizing and potentially splitting the interface.The interface is growing large and combines multiple responsibilities (blocks, transactions, validators, contracts, etc.). Consider:
- Splitting into smaller, focused interfaces (e.g.,
BlockExplorer,TxExplorer,ValidatorExplorer).- Standardizing method naming conventions (currently mixing
FetchandGetprefixes).Example reorganization:
type BlockExplorer interface { GetBlocks(ctx context.Context) (*explorerPB.GetBlocksResponse, error) GetBlock(ctx context.Context, blockHeight string) (*explorerPB.GetBlockResponse, error) StreamBlocks(ctx context.Context) (explorerPB.InjectiveExplorerRPC_StreamBlocksClient, error) } type ValidatorExplorer interface { FetchValidators(ctx context.Context) (*explorerPB.GetValidatorsResponse, error) FetchValidator(ctx context.Context, address string) (*explorerPB.GetValidatorResponse, error) FetchValidatorUptime(ctx context.Context, address string) (*explorerPB.GetValidatorUptimeResponse, error) } // Additional interfaces for other responsibilities...
Line range hint
77-81: Update logger service name to match package.The logger's service name still references "exchangeClient" which should be updated to "explorerClient" for consistency with the package rename.
logger: log.WithFields(log.Fields{ "module": "sdk-go", - "svc": "exchangeClient", + "svc": "explorerClient", }),
147-191: Add input validation and improve error handling.The new methods would benefit from:
- Input validation before making RPC calls
- More informative error wrapping
Example improvement for
FetchValidator:func (c *explorerClient) FetchValidator( ctx context.Context, address string, ) (*explorerPB.GetValidatorResponse, error) { + if address == "" { + return nil, errors.New("validator address cannot be empty") + } + req := &explorerPB.GetValidatorRequest{ Address: address, } res, err := common.ExecuteCall(ctx, c.network.ExplorerCookieAssistant, c.explorerClient.GetValidator, req) if err != nil { - return &explorerPB.GetValidatorResponse{}, err + return &explorerPB.GetValidatorResponse{}, errors.Wrap(err, "failed to fetch validator") } return res, nil }Also applies to: 307-361
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/explorer/explorer.go(3 hunks)
🔇 Additional comments (2)
client/explorer/explorer.go (2)
1-12: LGTM! Package name change aligns with functionality.The package name change from
exchangetoexplorerbetter reflects the module's purpose and responsibilities.
333-348: Fix inconsistent parameter naming in 'FetchRelayers' method.The parameter
marketIDsshould be renamed to match Go naming conventions.
Summary by CodeRabbit
New Features
Refactor
ExplorerClientinterface.exchangetoexplorer.Documentation