Skip to content
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

zetacore : Delete Ballots after distribution #942

Open
Tracked by #350 ...
kingpinXD opened this issue Aug 9, 2023 · 10 comments · May be fixed by #2863
Open
Tracked by #350 ...

zetacore : Delete Ballots after distribution #942

kingpinXD opened this issue Aug 9, 2023 · 10 comments · May be fixed by #2863

Comments

@kingpinXD
Copy link
Contributor

kingpinXD commented Aug 9, 2023

Ballots can be deleted from the state after they have matured to save space on the state.db

  • Delete matured ballots
  • Add an event when the ballot gets deleted, which can be indexed and used to query the list of voters if needed for debugging
@ws4charlie
Copy link
Contributor

Maybe we should not delete the ballots as the finalized ballots help us prevent double spending (by blocking the same ballot being created again)

@kingpinXD
Copy link
Contributor Author

We do have the finalized inbounds check to stop ballot creation
https://github.com/zeta-chain/zeta-node/blob/db2c6f3ff7998c1f5f8500e853500a5d9d46adf9/x/crosschain/keeper/msg_server_vote_inbound_tx.go#L95-L97

@kingpinXD kingpinXD changed the title emissions : Delete Ballots after distribution zetacore : Delete Ballots after distribution Aug 28, 2024
@ws4charlie
Copy link
Contributor

We do have the finalized inbounds check to stop ballot creation https://github.com/zeta-chain/zeta-node/blob/db2c6f3ff7998c1f5f8500e853500a5d9d46adf9/x/crosschain/keeper/msg_server_vote_inbound_tx.go#L95-L97

Sounds great. It will be safe then. We can go ahead to clean up ballots after distribution.

@kingpinXD
Copy link
Contributor Author

kingpinXD commented Aug 28, 2024

Deleting older ballots would remove the possibility of querying ballots older than 100 blocks (current athens maturity period )

This can affect current way of operations

  1. We wouldn't be able to query older ballots to see if directly if a particular observer is consistently missing votes
  2. We allow observers to vote on ballots to receive rewards , they would not be able to vote on ballots once deleted

Proposed solutions
When deleting a ballot, we can emit a new event containing the list of observers who have added their votes to the ballot.
This should be enough, as we expect every observer to cast their vote within the maturity period (casting after that is useless as they would not get any rewards )
Additionally, we can consider increasing the maturity blocks, if we expect observers to be more than 100 blocks late in adding their votes

  • The delete ballot event would be part of the BlockBeginner and not a tx_result.

  • We can also consider removing the Ballot_Created if it is not being consumed right now

Changes required

  • The explorer/indexer should be able to index these deleted ballot events so that they can be easily searched. This should be very similar to existing emissions events such as
    /zetachain.zetacore.emissions.internal.ObserverEmissions
    /zetachain.zetacore.emissions.internal.BlockEmissions

cc @zeta-chain/fullstack , @zeta-chain/devops

@GMaiolo
Copy link

GMaiolo commented Aug 28, 2024

If these changes come within /cosmos/tx/v1beta1/txs when querying for a specific block event, then we don't need to update anything on the indexer side 👍

@kingpinXD
Copy link
Contributor Author

I don't think it would be within /cosmos/tx/v1beta1/txs , as its not part of a tx but part of begin block instead . (Not 100% sure though )

Can you send me an example of how we index the two events I mentioned?

@GMaiolo
Copy link

GMaiolo commented Sep 9, 2024

If it's not within /cosmos/tx/v1beta1/txs, and if it's not EVM (therefore not going through Blockscout) we're not indexing it currently

@kingpinXD
Copy link
Contributor Author

@GMaiolo We should be indexing BeginBlock and EndBlock events as well. In case we are not doing it now , it would be a good idea to prioritize this shortly

@kingpinXD
Copy link
Contributor Author

Not entirely sure how the indexer works but the Begin Block and End Block events are part of the block result
https://zetachain-rpc.lavenderfive.com/block_results?height=4799111

@kingpinXD kingpinXD linked a pull request Sep 11, 2024 that will close this issue
5 tasks
@lumtis lumtis closed this as completed Nov 24, 2024
@lumtis lumtis reopened this Nov 24, 2024
@kingpinXD
Copy link
Contributor Author

Deleting older ballots would remove the possibility of querying ballots older than 100 blocks (current athens maturity period )

This can affect current way of operations

  1. We wouldn't be able to query older ballots to see if directly if a particular observer is consistently missing votes
  2. We allow observers to vote on ballots to receive rewards , they would not be able to vote on ballots once deleted

Proposed solutions When deleting a ballot, we can emit a new event containing the list of observers who have added their votes to the ballot. This should be enough, as we expect every observer to cast their vote within the maturity period (casting after that is useless as they would not get any rewards ) Additionally, we can consider increasing the maturity blocks, if we expect observers to be more than 100 blocks late in adding their votes

  • The delete ballot event would be part of the BlockBeginner and not a tx_result.
  • We can also consider removing the Ballot_Created if it is not being consumed right now

Changes required

  • The explorer/indexer should be able to index these deleted ballot events so that they can be easily searched. This should be very similar to existing emissions events such as
    /zetachain.zetacore.emissions.internal.ObserverEmissions
    /zetachain.zetacore.emissions.internal.BlockEmissions

cc @zeta-chain/fullstack , @zeta-chain/devops

The delete ballot event is not mandatory , but we can add it if needed
However if someone can confirm that it is not needed at all , and that we are good with the ballot stats for the last 100 blocks , I would like to remove it to save space in the data directory

@GMaiolo @lumtis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants