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

Efficiency of NeoToken VoterRewardPerCommittee records #2815

Closed
devhawk opened this issue Sep 21, 2022 · 2 comments · Fixed by #2841
Closed

Efficiency of NeoToken VoterRewardPerCommittee records #2815

devhawk opened this issue Sep 21, 2022 · 2 comments · Fixed by #2841
Labels
Discussion Initial issue state - proposed but not yet accepted

Comments

@devhawk
Copy link
Contributor

devhawk commented Sep 21, 2022

NeoToken contract use of Prefix_VoterRewardPerCommittee seems pretty inefficient.

  • NeoToken.PostPersist creates a new Prefix_VoterRewardPerCommittee for every committee member with votes every time the committee is refreshed. Since the committee size == the epoch block count, this means we are generating an average of one new VoterRewardPerCommittee every block. This comes out to about 275 records per member per day
  • NeoToken.CheckCandidate will find and delete all Prefix_VoterRewardPerCommittee records for a candidate who is not registered and has no votes. For a candidate that had been a committee member for a while but is dropping out of the race, this could end up deleting 10,000s of records.
  • Once the GAS for a given block has been claimed, this record is no longer needed but continues to take up space in the data store.
  • Retrieving 10000s of records via StateService is not feasible, so the [StateServiceStore'](https://github.com/ngdenterprise/neo-blockchaintoolkit-library/blob/master/src/bctklib/persistence/StateServiceStore.cs) can't provide these records. So offline / trace / branch scenarios that use StateServiceStore` can't rely on committee rewards as part of unclaimed gas calculations.
@devhawk devhawk added the Discussion Initial issue state - proposed but not yet accepted label Sep 21, 2022
@shargon
Copy link
Member

shargon commented Sep 22, 2022

Once the GAS for a given block has been claimed, this record is no longer needed but continues to take up space in the data store.

Which one?, NeoAccountState also stores the VoteTo.

@roman-khimov
Copy link
Contributor

Once the GAS for a given block has been claimed, this record is no longer needed but continues to take up space in the data store.

It'd be nice to delete them, but I don't think we know when all of the GAS has been claimed. It's only when all voting NEO accounts have BalanceHeight of more than some block height we can drop older data.

The other two options we have probably are:

  • store batches of these values (which at least will give us somewhat less number of KV pair, but at the expense of more complicated logic that deals with them).
  • store just the latest VoterRewardPerCommittee in some static per-member key and store initial VoterRewardPerCommittee in the account data (we know the value when GAS is distributed, it's easy to save it)

The second option may be interesting to evaluate, even though it makes per-voter data somewhat larger we no longer need to store full history of these values and it'll be updated in a natural way, so storage-wise it'd probably be about the same. It's also easy to cache the latest VoterRewardPerCommittee, so GAS distribution for voters should be much faster with this storage scheme (no DB seeks at all). And we'll have less keys in the DB in general which is always good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants