feat: add LastValsetRequestBeforeNonce rpc call
#1325
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1325 +/- ##
==========================================
- Coverage 49.79% 48.26% -1.54%
==========================================
Files 76 77 +1
Lines 4374 4397 +23
==========================================
- Hits 2178 2122 -56
- Misses 2014 2098 +84
+ Partials 182 177 -5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -31,6 +31,15 @@ service Query { | |||
| returns (QueryLatestAttestationNonceResponse) { | |||
| option (google.api.http).get = "/qgb/v1/attestations/nonce/latest"; | |||
| } | |||
| // LastValsetRequestBeforeNonce Queries last Valset request before nonce. | |||
There was a problem hiding this comment.
What does Valset stand for? is it validator set? can we please add the full term in the description as well
There was a problem hiding this comment.
Yes, It's a type that describes validator sets:
celestia-app/proto/celestia/qgb/v1/types.proto
Lines 17 to 29 in 4f9deb0
| @@ -130,7 +130,7 @@ func normalizeValidatorPower(rawPower uint64, totalValidatorPower cosmosmath.Int | |||
|
|
|||
| // GetLastValsetBeforeNonce returns the previous valset before the provided `nonce`. | |||
| // the `nonce` can be a valset, but this method will return the valset before it. | |||
| // If the provided nonce is 1. It will return an error. Because, there is no valset before nonce 1. | |||
| // If the provided nonce is 1, it will return an error, because, there is no valset before nonce 1. | |||
| func (k Keeper) GetLastValsetBeforeNonce(ctx sdk.Context, nonce uint64) (*types.Valset, error) { | |||
There was a problem hiding this comment.
Shouldn't the second argument be *QueryLastValsetRequestBeforeNonceRequest? Or where does the actual query get called to
There was a problem hiding this comment.
That's the keeper. It's called in the querier here:
celestia-app/x/qgb/keeper/query_valset.go
Lines 13 to 16 in 4f9deb0
That change is just a docs change that Sanaz spotted and I fixed it in this PR (instead of opening a new one)
There was a problem hiding this comment.
so if I'm understanding correctly, the querier was already defined, we just missed the proto definitions somewhere along the line?
do we need some basic unit tests to ensure basic functionality? like, perhaps we could include this in this test, or in a similar one, where we spin up a node and query using grpc
Yes 100%
Good idea 👍 #1343 |
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> ## Overview <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. --> This RPC call was left out somewhere when adapting the state for V2. This PR introduces it back. ## Checklist <!-- Please complete the checklist to ensure that the PR is ready to be reviewed. IMPORTANT: PRs should be left in Draft until the below checklist is completed. --> - [ ] New and updated code has appropriate documentation - [ ] New and updated code has new and/or updated testing - [ ] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords
Overview
This RPC call was left out somewhere when adapting the state for V2. This PR introduces it back.
Checklist