-
Notifications
You must be signed in to change notification settings - Fork 170
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
zoneconcierge: API for querying headers in a given epoch #261
Changes from 5 commits
fe58599
e254c62
4cc3f9d
2c3de9a
f6b9a11
3f3ea1b
8bc1cdf
494a04d
f2f923f
c743f6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,10 +30,18 @@ service Query { | |
rpc ChainInfo(QueryChainInfoRequest) returns (QueryChainInfoResponse) { | ||
option (google.api.http).get = "/babylon/zoneconcierge/v1/chain_info/{chain_id}"; | ||
} | ||
// EpochChainInfo queries the latest info of a chain in a given epoch of Babylon's view | ||
rpc EpochChainInfo(QueryEpochChainInfoRequest) returns (QueryEpochChainInfoResponse) { | ||
option (google.api.http).get = "/babylon/zoneconcierge/v1/epochs/{epoch_num}/chain_info/{chain_id}"; | ||
} | ||
// ListHeaders queries the headers of a chain in Babylon's view, with pagination support | ||
rpc ListHeaders(QueryListHeadersRequest) returns (QueryListHeadersResponse) { | ||
option (google.api.http).get = "/babylon/zoneconcierge/v1/headers/{chain_id}"; | ||
} | ||
// ListEpochHeaders queries the headers of a chain timestamped in a given epoch of Babylon, with pagination support | ||
rpc ListEpochHeaders(QueryListEpochHeadersRequest) returns (QueryListEpochHeadersResponse) { | ||
option (google.api.http).get = "/babylon/zoneconcierge/v1/epochs/{epoch_num}/headers/{chain_id}"; | ||
} | ||
// FinalizedChainInfo queries the BTC-finalised info of a chain, with proofs | ||
rpc FinalizedChainInfo(QueryFinalizedChainInfoRequest) returns (QueryFinalizedChainInfoResponse) { | ||
option (google.api.http).get = "/babylon/zoneconcierge/v1/finalized_chain_info/{chain_id}"; | ||
|
@@ -68,6 +76,18 @@ message QueryChainInfoResponse { | |
babylon.zoneconcierge.v1.ChainInfo chain_info = 1; | ||
} | ||
|
||
// QueryEpochChainInfoRequest is request type for the Query/EpochChainInfo RPC method. | ||
message QueryEpochChainInfoRequest { | ||
uint64 epoch_num = 1; | ||
string chain_id = 2; | ||
} | ||
|
||
// QueryEpochChainInfoResponse is response type for the Query/EpochChainInfo RPC method. | ||
message QueryEpochChainInfoResponse { | ||
// chain_info is the info of the CZ | ||
babylon.zoneconcierge.v1.ChainInfo chain_info = 1; | ||
} | ||
|
||
// QueryListHeadersRequest is request type for the Query/ListHeaders RPC method. | ||
message QueryListHeadersRequest { | ||
string chain_id = 1; | ||
|
@@ -83,6 +103,18 @@ message QueryListHeadersResponse { | |
cosmos.base.query.v1beta1.PageResponse pagination = 2; | ||
} | ||
|
||
// QueryListEpochHeadersRequest is request type for the Query/ListEpochHeaders RPC method. | ||
message QueryListEpochHeadersRequest { | ||
uint64 epoch_num = 1; | ||
string chain_id = 2; | ||
} | ||
|
||
// QueryListEpochHeadersResponse is response type for the Query/ListEpochHeaders RPC method. | ||
message QueryListEpochHeadersResponse { | ||
// headers is the list of headers | ||
repeated babylon.zoneconcierge.v1.IndexedHeader headers = 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest we have pagination here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was also considering pagination for this API. The problem is that this neither is straightforward nor gives us performance improvements due to the KVStore structure. We are querying a segment in the KVStore and the keys in this KVStore is not consecutive. The non-consecutiveness requires us to find the start header and the end header of this epoch, which is already O(epoch_interval) in the worst case. For the sake of efficiency, maybe we can consider a slightly different API, which only returns a list of heights whose corresponding headers are timestamped in this given epoch. With the list of heights the explorer can query its interested indexed headers from ZoneConcierge individually. I have marked a TODO here. Once we have a decision I will make corresponding changes in a subsequent API. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I also encountered a similar issue with the |
||
} | ||
|
||
// QueryFinalizedChainInfoRequest is request type for the Query/FinalizedChainInfo RPC method. | ||
message QueryFinalizedChainInfoRequest { | ||
// chain_id is the ID of the CZ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package keeper | ||
|
||
import ( | ||
bbn "github.com/babylonchain/babylon/types" | ||
"github.com/babylonchain/babylon/x/zoneconcierge/types" | ||
"github.com/cosmos/cosmos-sdk/store/prefix" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
@@ -19,6 +20,46 @@ func (k Keeper) GetEpochChainInfo(ctx sdk.Context, chainID string, epochNumber u | |
return &chainInfo, nil | ||
} | ||
|
||
// GetEpochHeaders gets the headers timestamped in a given epoch, in the ascending order | ||
func (k Keeper) GetEpochHeaders(ctx sdk.Context, chainID string, epochNumber uint64) ([]*types.IndexedHeader, error) { | ||
headers := []*types.IndexedHeader{} | ||
|
||
// find the last timestamped header of this chain in the epoch | ||
epochChainInfo, err := k.GetEpochChainInfo(ctx, chainID, epochNumber) | ||
if err != nil { | ||
return nil, err | ||
} | ||
// it's possible that this epoch's snapshot is not updated for many epochs | ||
// this implies that this epoch does not timestamp any header for this chain at all | ||
// and the query should return empty | ||
if epochChainInfo.LatestHeader.BabylonEpoch < epochNumber { | ||
return nil, types.ErrEpochHeadersNotFound | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Above you mention that the query should return empty, yet here you return an error. I'm happy with returning an empty set. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel an error gives a more clear message to the function invoker. The comment in L34 is indeed inconsistent though so I deleted it. |
||
} | ||
// now we have the last header in this epoch | ||
headers = append(headers, epochChainInfo.LatestHeader) | ||
|
||
// append all previous headers until reaching the previous epoch | ||
canonicalChainStore := k.canonicalChainStore(ctx, chainID) | ||
lastHeaderKey := sdk.Uint64ToBigEndian(epochChainInfo.LatestHeader.Height) | ||
// NOTE: even in ReverseIterator, start and end should still be specified in ascending order | ||
canonicalChainIter := canonicalChainStore.ReverseIterator(nil, lastHeaderKey) | ||
defer canonicalChainIter.Close() | ||
for ; canonicalChainIter.Valid(); canonicalChainIter.Next() { | ||
var prevHeader types.IndexedHeader | ||
k.cdc.MustUnmarshal(canonicalChainIter.Value(), &prevHeader) | ||
if prevHeader.BabylonEpoch < epochNumber { | ||
// we have reached the previous epoch, break the loop | ||
break | ||
} | ||
headers = append(headers, &prevHeader) | ||
} | ||
|
||
// reverse the list so that it remains ascending order | ||
bbn.Reverse(headers) | ||
|
||
return headers, nil | ||
} | ||
|
||
// recordEpochChainInfo records the chain info for a given epoch number of given chain ID | ||
// where the latest chain info is retrieved from the chain info indexer | ||
func (k Keeper) recordEpochChainInfo(ctx sdk.Context, chainID string, epochNumber uint64) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,8 @@ func FuzzEpochChainInfoIndexer(f *testing.F) { | |
f.Fuzz(func(t *testing.T, seed int64) { | ||
rand.Seed(seed) | ||
|
||
_, babylonChain, czChain, zcKeeper := SetupTest(t) | ||
_, babylonChain, czChain, babylonApp := SetupTest(t) | ||
zcKeeper := babylonApp.ZoneConciergeKeeper | ||
|
||
ctx := babylonChain.GetContext() | ||
hooks := zcKeeper.Hooks() | ||
|
@@ -35,3 +36,40 @@ func FuzzEpochChainInfoIndexer(f *testing.F) { | |
require.Equal(t, numForkHeaders, uint64(len(chainInfo.LatestForks.Headers))) | ||
}) | ||
} | ||
|
||
func FuzzGetEpochHeaders(f *testing.F) { | ||
datagen.AddRandomSeedsToFuzzer(f, 10) | ||
|
||
f.Fuzz(func(t *testing.T, seed int64) { | ||
rand.Seed(seed) | ||
|
||
_, babylonChain, czChain, babylonApp := SetupTest(t) | ||
zcKeeper := babylonApp.ZoneConciergeKeeper | ||
epochingKeeper := babylonApp.EpochingKeeper | ||
|
||
ctx := babylonChain.GetContext() | ||
hooks := zcKeeper.Hooks() | ||
|
||
// enter a random epoch | ||
epochNum := datagen.RandomInt(10) + 1 | ||
for i := uint64(0); i < epochNum; i++ { | ||
epochingKeeper.IncEpoch(ctx) | ||
} | ||
|
||
// invoke the hook a random number of times to simulate a random number of blocks | ||
numHeaders := datagen.RandomInt(100) + 1 | ||
numForkHeaders := datagen.RandomInt(10) + 1 | ||
expectedHeaders, _ := SimulateHeadersAndForksViaHook(ctx, hooks, czChain.ChainID, numHeaders, numForkHeaders) | ||
|
||
// end this epoch so that the chain info is recorded | ||
hooks.AfterEpochEnds(ctx, epochNum) | ||
|
||
// check if the headers are same as expected | ||
headers, err := zcKeeper.GetEpochHeaders(ctx, czChain.ChainID, epochNum) | ||
require.NoError(t, err) | ||
require.Equal(t, len(expectedHeaders), len(headers)) | ||
for i := 0; i < len(expectedHeaders); i++ { | ||
require.Equal(t, expectedHeaders[i].Header.LastCommitHash, headers[i].Hash) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
}) | ||
} |
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.
nit: I suggest the link be
/headers/{chain_id}/epoch/{epoch_num}
as the epoch is a more specific part of a headers query.