-
Notifications
You must be signed in to change notification settings - Fork 332
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
Query old blocks for proposals in CLI #598
Conversation
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.
Approach is good! See comment about errors.Is
not working, not sure the reason though.
After I make the fix, the query kvcli q proposal 1
returns
pub_proposal:
title: Proposal to Test Voting
description: This proposal is for testing voting functionality.
id: 1
committee_id: 1
deadline: 0001-01-01T00:00:00Z
While the query kvcli q proposal 1 --height 42400
returns
title: Proposal to Test Voting
description: This proposal is for testing voting functionality.
id: 1
committee_id: 1
deadline: 2020-06-18T03:49:58.778856944Z
The deadline is lost because the message is submitted without a deadline and it is calculated based on the blocktime in (Keeper).SubmitProposal
. You could reconstruct it by parsing the TxResponse
object and:
- Querying the committee
ProposalDuration
based onProposal.CommitteeID
- Querying the block time based on
TxResponse.Height
- Adding the
ProposalDuration
to the block time.
x/committee/client/common/query.go
Outdated
return proposal, nil | ||
} | ||
|
||
if err != nil && !errors.Is(err, types.ErrUnknownProposal) { |
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.
did some testing: errors.Is(err, types.ErrUnknownProposal)
does not return true even if the error is proposal not found
. Not sure why exactly.
Replacing with if err != nil && !strings.Contains(err.Error(), "proposal not found") {
works although I'm not sure that's the best approach
@fedekunze any ideas here?
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.
Updated to use the fix. I wonder if this is due to serialization?
Thanks for the review and info on calculating the deadline 👍 I'm not sure if it's the best approach, but I didn't see a better way to get the block time for a given height without using the tendermind rpcclient. I also updated the rest client to use the new method, which is why |
x/committee/client/common/query.go
Outdated
return deadline, err | ||
} | ||
|
||
res, height, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryCommittee), bz) |
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.
I'm getting the deadline calculated relative to the current block time. I believe the variable height
that was passed in to the function is getting overwritten here. I think you want to set the cliCtx
to `cliCtx.WithHeight(height) and then do the query.
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.
tACK 💥
* move file to query.go (we are adding functionality so specific name doesn't fit anymore) * Add tx search for proposals in cli query proposal * add rest support, height support for rest api, and add go doc string * add in deadline calculation * update changelog Co-authored-by: Kevin Davis <kjydavis3@gmail.com>
* move file to query.go (we are adding functionality so specific name doesn't fit anymore) * Add tx search for proposals in cli query proposal * add rest support, height support for rest api, and add go doc string * add in deadline calculation * update changelog Co-authored-by: Kevin Davis <kjydavis3@gmail.com>
No description provided.