feat: Add support for multiple chain ids in EpochChainsInfo API #369
Conversation
gusin13
changed the base branch from
BM-603-api-add-new-rpc-queries-finalized
to
dev
gusin13
force-pushed
the
BM-606-optimize-epoch-chains-info-rpc
branch
from
3d268ea
to
ed2aa8a
Compare
test/e2e/e2e_test.go
Outdated
| s.FailNow("Epoch 2 should be finalized") | ||
| } | ||
| // Check we have epoch info for opposing chain and some basic assertions | ||
| epochChainsInfo, err := nonValidatorNode.QueryEpochChainsInfo(3, []string{initialization.ChainBID}) |
There was a problem hiding this comment.
| epochChainsInfo, err := nonValidatorNode.QueryEpochChainsInfo(3, []string{initialization.ChainBID}) | |
| epochChainsInfo, err := nonValidatorNode.QueryEpochChainsInfo(endEpoch, []string{initialization.ChainBID}) |
x/zoneconcierge/client/cli/query.go
Outdated
| return cmd | ||
| } | ||
|
|
||
| func CmdChainsInfo() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "chains-info <chain-ids>", | ||
| Short: "retrieves the latest info for a list of chains with given IDs", | ||
| Short: "returns the latest info for a given list of chains", |
There was a problem hiding this comment.
Here we changed "retrieves" to "returns", but the method below still has "retrieves". tbh I prefer the "retrieves" but np with both, but let's be consistent
There was a problem hiding this comment.
sure np, have changed back to retrieve
x/zoneconcierge/client/cli/query.go
Outdated
| var epoch uint64 | ||
| _, err := fmt.Sscanf(args[0], "%d", &epoch) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
| var epoch uint64 | |
| _, err := fmt.Sscanf(args[0], "%d", &epoch) | |
| if err != nil { | |
| return err | |
| } | |
| epoch, err := strconv.ParseUint(args[0], 10, 64) | |
| if err != nil { | |
| return err | |
| } |
| for i := uint64(0); i < numReqs; i++ { | ||
| resp, err := zcKeeper.EpochChainInfo(ctx, &zctypes.QueryEpochChainInfoRequest{EpochNum: epochNumList[i], ChainId: czChain.ChainID}) | ||
| for epoch, info := range epochToChainInfo { | ||
| resp, err := zcKeeper.EpochChainsInfo(ctx, &zctypes.QueryEpochChainsInfoRequest{EpochNum: epoch, ChainIds: []string{info.chainID}}) |
There was a problem hiding this comment.
Now that we have the capability to query for multiple chains we should take advantage of it. This test looks like the convertion of the 1-chainID case to work with the new syntax. Let's test the fact that we can query many different chains at the same time. Let's also test that we handle duplicates properly and reject stuff that's over the request limit.
There was a problem hiding this comment.
sure, have added more tests now.
Description
This PR is part of optimizations done to improve babylon-api performance.
Explorer currently calls
EpochChainInfo(/babylon/zoneconcierge/v1/chain_info/{chain_id}/epochs/{epoch_num}) multiple times for each chain. This PR replacesEpochChainInfowithEpochChainsInfowhich can ingest a range of chain ids for a given epoch and retrieve epoch info for multiple chains at once.Testing
Added fuzz , e2e tests and cli query.