Cosmos-sdk: Governance API for votes on a proposal returns `null` after voting period closes

Created on 22 Nov 2018  路  12Comments  路  Source: cosmos/cosmos-sdk

Summary of Bug

The Governance API for votes on a proposal (/gov/proposals/:id/votes) returns null instead of the Votes that were associated with the proposal after the voting period closes.

This is unexpected.

It would be expected that the voting period has no effect on the JSON payload returned by a call to /gov/proposals/:id/votes, and that the call reliably returns the votes cast against a proposal whenever it is called.

Steps to Reproduce

  1. Create a Proposal
  2. Make a Vote against a Proposal
  3. Call the Governance API to retrieve the Votes against a Proposal, see that you get a valid JSON payload as a response.
  4. Wait for the Voting period to close.
  5. Make the same call you made in step 3, except this time you will receive null as a response.

For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned
UX bug gov

Most helpful comment

Regarding not pruning, yes, exactly what @ValarDragon said. It's not feasible to keep stuff in state, just because clients want to be able to read it. The LCD at the current moment, is based off of queriers who query state at a specific block height, which is why if you need to pass in a block height at which the votes were still in state if the proposal's voting period has ended (you can get the time at which it ended, and then use the last block height before that time. I think it would be fair to add to each proposal, the height at which it ended, so you don't have to calculate it just off of the time).

Now, the question is, do you want the LCD to do all that fanciness itself, or leave this logic up to the client (wallet or explorer or whatever). Here's how I see it:

Pros of doing this logic in the LCD:

  • Simpler for clients

Cons of doing it in the LCD:

  • What do you do if the node the LCD is talking to is "super-pruning"? As in it doesn't have any history whatsoever, other than the latest block. It won't be able to do this logic, and we need to add these errors and edge case handling for all these different scenarios into the LCD logic.
  • I actually want to move towards a model where the developers don't write LCDs and CLIs themselves, but rather they are auto-generated based off of the queriers.

All 12 comments

I'm shocked that this does not work. Is this expected behavior @sunnya97?

After the votes have been counted and tallied, they are pruned from state.

You can get the TallyResult by using the Tally endpoint, but to get the actual individual votes themselves, you're going to need to query a past block height, during the voting period.

Can LCD not handle this behind the scenes? We currently are scanning and storing limited info for every block, but as I understand it that is not what the Cosmos team considers ideal or necessary for projects like Hubble (which this issue is ultimately related to). We have plans to prune our store of old blocks at some point, and this would put a distinct extra caveat on that -- making sure not to prune a block we may need to have later in order to find votes for a proposal.

Conceptually, it seems to me that the LCD should handle this transparently and just always return the most recent vote set when querying /gov/proposals/:id/votes. The 'wording', if you will, of that endpoint suggests that's what will happen since it does not take a block height or anything as argument as far as I know. I would expect to just get a high level response of 'the votes' at this endpoint and not have to take into account implementation details (like pruning) and a bunch of other intricacies such as the voting period and what blocks would have been part of that period, etc.

@sunnya97 What's the rationale for pruning? Seems like a minimal amount of state space to save at the increase of a much worse scanning requirement?

but to get the actual individual votes themselves, you're going to need to query a past block height, during the voting period.

@sunnya97 clients should be able to query a proposal votes at all times, same with deposits. CC: @faboweb

I agree with the previous posters. Let's design the API so it behaves like users and devs expect it to behave.

@sunnya97 What's the rationale for pruning? Seems like a minimal amount of state space to save at the increase of a much worse scanning requirement?

Not at all. We anticipate far more than just the validators to be submitting votes. I don't see how not pruning votes from state would even be an option. State is only meant for things that need to be persisted for all perpetuity, and should be _quite_ costly. We are planning on one day incorporating storage rent so that it costs money _per block_ to have things in state.

The indexer is meant to handle things like this. We can make the API for the LCD perhaps use the tx indexer in this case.

We could use tags for something like this, right?

Regarding not pruning, yes, exactly what @ValarDragon said. It's not feasible to keep stuff in state, just because clients want to be able to read it. The LCD at the current moment, is based off of queriers who query state at a specific block height, which is why if you need to pass in a block height at which the votes were still in state if the proposal's voting period has ended (you can get the time at which it ended, and then use the last block height before that time. I think it would be fair to add to each proposal, the height at which it ended, so you don't have to calculate it just off of the time).

Now, the question is, do you want the LCD to do all that fanciness itself, or leave this logic up to the client (wallet or explorer or whatever). Here's how I see it:

Pros of doing this logic in the LCD:

  • Simpler for clients

Cons of doing it in the LCD:

  • What do you do if the node the LCD is talking to is "super-pruning"? As in it doesn't have any history whatsoever, other than the latest block. It won't be able to do this logic, and we need to add these errors and edge case handling for all these different scenarios into the LCD logic.
  • I actually want to move towards a model where the developers don't write LCDs and CLIs themselves, but rather they are auto-generated based off of the queriers.

@sunnya97 @alexanderbez @ValarDragon what do you think about doing a tx search with tags when the proposal is not active (i.e Passed or Rejected) ? In that case the user will still be able to query the endpoint and have the same logic + the votes/deposits will be pruned from the state.

The only thing we'd need to add will be keeping only the last vote of the voter and tossing the rest from the search results. In the case of the deposits we'd have to sum the amounts deposited.

I think generally that is what we've been suggesting is it not? Regarding tossing the vote -- why? Isn't it just a proposalID => votes tag?

The LCD at the current moment, is based off of queriers who query state at a specific block height, which is why if you need to pass in a block height at which the votes were still in state if the proposal's voting period has ended (you can get the time at which it ended, and then use the last block height before that time.

I don't see any way to use LCD to get the block height for a given time. How would you attempt this right now, given the voting period close time? Or is it expected you would just manually scan the blocks until you find the block preceding the vote closing time?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

fedekunze picture fedekunze  路  3Comments

cwgoes picture cwgoes  路  3Comments

ValarDragon picture ValarDragon  路  3Comments

hendrikhofstadt picture hendrikhofstadt  路  3Comments

fedekunze picture fedekunze  路  3Comments