-
Notifications
You must be signed in to change notification settings - Fork 62
Add Chain Queries for the Insurance Module #314
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
Conversation
WalkthroughSupport for the Insurance module has been added to the chain client, including four new query methods for insurance funds and redemptions. Mock implementations for testing and four example programs demonstrating these new queries were also introduced. No existing logic was changed; only new capabilities and usage examples were added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChainClient
participant InsuranceQueryClient
User->>ChainClient: FetchInsuranceFund(ctx, marketId)
ChainClient->>InsuranceQueryClient: QueryInsuranceFund(request)
InsuranceQueryClient-->>ChainClient: QueryInsuranceFundResponse
ChainClient-->>User: QueryInsuranceFundResponse
User->>ChainClient: FetchInsuranceFunds(ctx)
ChainClient->>InsuranceQueryClient: QueryInsuranceFunds(request)
InsuranceQueryClient-->>ChainClient: QueryInsuranceFundsResponse
ChainClient-->>User: QueryInsuranceFundsResponse
User->>ChainClient: FetchPendingRedemptions(ctx, marketId, address)
ChainClient->>InsuranceQueryClient: QueryPendingRedemptions(request)
InsuranceQueryClient-->>ChainClient: QueryPendingRedemptionsResponse
ChainClient-->>User: QueryPendingRedemptionsResponse
User->>ChainClient: FetchEstimatedRedemptions(ctx, marketId, address)
ChainClient->>InsuranceQueryClient: QueryEstimatedRedemptions(request)
InsuranceQueryClient-->>ChainClient: QueryEstimatedRedemptionsResponse
ChainClient-->>User: QueryEstimatedRedemptionsResponse
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
🪧 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
|
4999acb to
c30d992
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
examples/chain/insurance/1_InsuranceFund/example.go (1)
50-51: Ignored marshal errorSee prior comment – handle
json.MarshalIndenterror.examples/chain/insurance/3_PendingRedemptions/example.go (1)
67-68: Ignored marshal errorHandle the possible marshal failure.
examples/chain/insurance/4_EstimatedRedemptions/example.go (1)
67-68: Ignored marshal errorHandle marshal error.
🧹 Nitpick comments (11)
examples/chain/insurance/2_InsuranceFunds/example.go (3)
16-20: Consider propagating the initialization error rather than panickingUsing
panicin an example is acceptable, but propagating or logging the error (e.g. withlog.Fatalor returning an error frommain) is safer and sets a better pattern for production code.
31-32: Missing context cancellationYou create a background context without cancellation. For long-running queries or CLI tools, prefer:
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel()This prevents the process from hanging indefinitely if the endpoint stalls.
50-51: Ignored marshal error
json.MarshalIndentcan fail (e.g. due to invalid UTF-8). Handle the error instead of discarding it:-str, _ := json.MarshalIndent(res, "", " ") +str, marshalErr := json.MarshalIndent(res, "", " ") +if marshalErr != nil { + log.Fatal(marshalErr) +}examples/chain/insurance/1_InsuranceFund/example.go (2)
16-20: Return/handle initialization error instead of panickingSame reasoning as in the previous example: favour
log.Fatalor proper error handling.
43-46: Hard-coded market IDHard-coding a market ID limits reusability. Accept it as a CLI flag or env var to make the sample more generally useful.
examples/chain/insurance/3_PendingRedemptions/example.go (1)
58-60: Missing context timeout / cancellationSame suggestion as in other examples – wrap with
context.WithTimeoutor similar.examples/chain/insurance/4_EstimatedRedemptions/example.go (1)
58-60: Missing context timeout / cancellationAdd timeout or cancellation for robustness.
client/chain/chain.go (4)
42-47: Import block ordering breaks existing conventionThroughout this file the third-party Injective packages are grouped alphabetically; inserting
insurancetypesbetweenexchangev2typesandpermissionstypesviolates that local ordering rule and will causegoimportsto reshuffle the block on the next run.- exchangev2types "github.com/InjectiveLabs/sdk-go/chain/exchange/types/v2" - insurancetypes "github.com/InjectiveLabs/sdk-go/chain/insurance/types" - permissionstypes "github.com/InjectiveLabs/sdk-go/chain/permissions/types" + exchangev2types "github.com/InjectiveLabs/sdk-go/chain/exchange/types/v2" + permissionstypes "github.com/InjectiveLabs/sdk-go/chain/permissions/types" + insurancetypes "github.com/InjectiveLabs/sdk-go/chain/insurance/types"Run
goimports/go fmtto keep the tree tidy.
472-477: Missing godoc comments for new interface methods
ChainClientis exported; every new exported method therefore needs a top-level docstring to satisfygolint/go vet(golangci-lintwill flag this). Please add concise comments that start with the method name, e.g.:// FetchInsuranceFund returns insurance fund information for the given market ID.Same applies to the three other methods added here.
530-533: Unprotected field increases struct’s public surface
insuranceQueryClientis added directly tochainClientand is publicly accessible to any code in this package.
Consider making it unexported (it already starts with a lowercase letter) and adding a short comment so future maintainers know why it exists, similar to other query-client fields above.// insuranceQueryClient handles all Insurance module gRPC queries. insuranceQueryClient insurancetypes.QueryClient
3706-3738: Consider validating input parameters & documenting historic-height usageThese helpers accept
marketId/addressstrings and forward them verbatim. A couple of defensive checks would save a round-trip and clearer errors:+ if marketId == "" { + return nil, errors.New("marketId cannot be empty") + } + if address == "" { + return nil, errors.New("address cannot be empty") + }Also, historic queries are enabled via the
x-cosmos-block-heightGRPC metadata. A brief doc comment explaining that callers can pass a context containing that metadata (e.g. viametadata.AppendToOutgoingContext) would make these helpers self-explanatory.Functionally everything mirrors other modules – good job.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
client/chain/chain.go(5 hunks)client/chain/chain_test_support.go(2 hunks)examples/chain/insurance/1_InsuranceFund/example.go(1 hunks)examples/chain/insurance/2_InsuranceFunds/example.go(1 hunks)examples/chain/insurance/3_PendingRedemptions/example.go(1 hunks)examples/chain/insurance/4_EstimatedRedemptions/example.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
examples/chain/insurance/4_EstimatedRedemptions/example.go (6)
client/common/network.go (1)
LoadNetwork(53-202)client/chain/keys.go (1)
InitCosmosKeyring(28-164)client/chain/context.go (1)
NewClientContext(129-215)client/chain/chain.go (1)
NewChainClient(539-663)client/common/options.go (1)
OptionGasPrices(36-47)client/constants.go (1)
DefaultGasPriceWithDenom(6-6)
client/chain/chain_test_support.go (1)
chain/insurance/types/query.pb.go (12)
QueryInsuranceFundResponse(166-168)QueryInsuranceFundResponse(172-172)QueryInsuranceFundResponse(173-175)QueryInsuranceFundsResponse(250-252)QueryInsuranceFundsResponse(256-256)QueryInsuranceFundsResponse(257-259)QueryPendingRedemptionsResponse(450-452)QueryPendingRedemptionsResponse(456-456)QueryPendingRedemptionsResponse(457-459)QueryEstimatedRedemptionsResponse(350-352)QueryEstimatedRedemptionsResponse(356-356)QueryEstimatedRedemptionsResponse(357-359)
client/chain/chain.go (3)
chain/insurance/types/query.pb.go (26)
QueryInsuranceFundResponse(166-168)QueryInsuranceFundResponse(172-172)QueryInsuranceFundResponse(173-175)QueryInsuranceFundsResponse(250-252)QueryInsuranceFundsResponse(256-256)QueryInsuranceFundsResponse(257-259)QueryPendingRedemptionsResponse(450-452)QueryPendingRedemptionsResponse(456-456)QueryPendingRedemptionsResponse(457-459)QueryEstimatedRedemptionsResponse(350-352)QueryEstimatedRedemptionsResponse(356-356)QueryEstimatedRedemptionsResponse(357-359)QueryClient(658-672)NewQueryClient(678-680)QueryInsuranceFundRequest(119-122)QueryInsuranceFundRequest(126-126)QueryInsuranceFundRequest(127-129)QueryInsuranceFundsRequest(212-213)QueryInsuranceFundsRequest(217-217)QueryInsuranceFundsRequest(218-220)QueryPendingRedemptionsRequest(396-399)QueryPendingRedemptionsRequest(403-403)QueryPendingRedemptionsRequest(404-406)QueryEstimatedRedemptionsRequest(296-299)QueryEstimatedRedemptionsRequest(303-303)QueryEstimatedRedemptionsRequest(304-306)client/common/api_request_assistant.go (1)
ExecuteCall(13-28)chain/insurance/types/insurance.pb.go (3)
InsuranceFund(82-110)InsuranceFund(114-114)InsuranceFund(115-117)
🔇 Additional comments (2)
client/chain/chain_test_support.go (1)
1213-1229: Nil-pointer safetyReturning
&insurancetypes.QueryInsuranceFundResponse{}when the caller expectsresponse.Fund != nilcan lead to nil-pointer checks throughout the codebase. Populate minimal empty structs where appropriate or document thatFundmay be nil.client/chain/chain.go (1)
639-640: Initialisation follows pattern – LGTMThe new query client is created in the same way as the other module clients and before
subaccountToNonceinitialisation; no issues spotted.
| userAddress, _, err := chainclient.InitCosmosKeyring( | ||
| os.Getenv("HOME")+"/.injectived", | ||
| "injectived", | ||
| "file", | ||
| "inj-user", | ||
| "12345678", | ||
| "5d386fbdbf11f1141010f81a46b40f94887367562bd33b452bbaa6ce1cd1381e", // keyring will be used if pk not provided | ||
| false, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Private key & passphrase committed to source
Embedding a plaintext passphrase ("12345678") and a raw private key exposes test funds and promotes insecure habits. Read credentials from environment variables or a secure store instead.
- "12345678",
- "5d386fbd...1381e",
+ os.Getenv("INJ_KEYRING_PASSPHRASE"),
+ os.Getenv("INJ_PRIV_KEY"),🤖 Prompt for AI Agents
In examples/chain/insurance/3_PendingRedemptions/example.go around lines 22 to
30, the passphrase and private key are hardcoded as plaintext strings, which is
insecure. Replace these hardcoded values by reading the passphrase and private
key from environment variables or a secure secrets store. Update the code to
fetch these credentials dynamically at runtime instead of embedding them
directly in the source code.
| userAddress, _, err := chainclient.InitCosmosKeyring( | ||
| os.Getenv("HOME")+"/.injectived", | ||
| "injectived", | ||
| "file", | ||
| "inj-user", | ||
| "12345678", | ||
| "5d386fbdbf11f1141010f81a46b40f94887367562bd33b452bbaa6ce1cd1381e", // keyring will be used if pk not provided | ||
| false, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Private key & passphrase committed to source
Same security concern as in the PendingRedemptions example; move secrets to environment variables.
🤖 Prompt for AI Agents
In examples/chain/insurance/4_EstimatedRedemptions/example.go around lines 22 to
30, the private key and passphrase are hardcoded in the source code, which is a
security risk. Remove the hardcoded private key and passphrase and instead read
them from environment variables using os.Getenv. Update the InitCosmosKeyring
call to use these environment variables for the private key and passphrase
values.
| // Insurance module | ||
|
|
||
| func (c *MockChainClient) FetchInsuranceFund(ctx context.Context, marketId string) (*insurancetypes.QueryInsuranceFundResponse, error) { | ||
| return &insurancetypes.QueryInsuranceFundResponse{}, nil | ||
| } | ||
|
|
||
| func (c *MockChainClient) FetchInsuranceFunds(ctx context.Context) (*insurancetypes.QueryInsuranceFundsResponse, error) { | ||
| return &insurancetypes.QueryInsuranceFundsResponse{}, nil | ||
| } | ||
|
|
||
| func (c *MockChainClient) FetchPendingRedemptions(ctx context.Context, marketId string, address string) (*insurancetypes.QueryPendingRedemptionsResponse, error) { | ||
| return &insurancetypes.QueryPendingRedemptionsResponse{}, nil | ||
| } | ||
|
|
||
| func (c *MockChainClient) FetchEstimatedRedemptions(ctx context.Context, marketId string, address string) (*insurancetypes.QueryEstimatedRedemptionsResponse, error) { | ||
| return &insurancetypes.QueryEstimatedRedemptionsResponse{}, nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Mock does not allow configurable responses for new insurance queries
Other mock methods (e.g. FetchChainSpotMarkets) pop predefined responses from slices, enabling deterministic tests. The four newly added insurance stubs always return empty structs, preventing unit tests from asserting specific data.
Consider extending MockChainClient with response queues similar to existing patterns:
type MockChainClient struct {
...
+ InsuranceFundResponses []*insurancetypes.QueryInsuranceFundResponse
+ InsuranceFundsResponses []*insurancetypes.QueryInsuranceFundsResponse
+ PendingRedemptionsResponses []*insurancetypes.QueryPendingRedemptionsResponse
+ EstimatedRedemptionsResponses []*insurancetypes.QueryEstimatedRedemptionsResponse
}and pop from those slices inside each method.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In client/chain/chain_test_support.go around lines 1213 to 1229, the new
insurance-related mock methods always return empty structs, which prevents tests
from asserting specific data. To fix this, add response slices or queues to the
MockChainClient struct for each insurance query type, then modify each method to
pop and return the next predefined response from the corresponding slice. This
will allow tests to set up deterministic, configurable mock responses similar to
existing mock methods.
This PR adds support for querying the Insurance module via the chain gRPC API.
While we already expose Insurance data through the Exchange API, it lacks support for historic queries (i.e., queries by block height). This addition enables clients to query Insurance module state at any given block height using standard Cosmos SDK gRPC query methods.
Summary by CodeRabbit