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

Follow-up: Optimize storage of balance checkpoints in T token contract #20

Merged
merged 2 commits into from
Oct 14, 2021

Conversation

pdyraga
Copy link
Member

@pdyraga pdyraga commented Oct 14, 2021

This PR is a follow-up of #19 and two things are happening here:

  • Cleared up slither warnings making main build green again,
  • Improve checkpoint tests for mint, burn, and burnFrom according to this comment.

I did not look into @vzotova's comment as I was not sure what types the governance contract expects. Leaving it to @cygnusv

dead-code detector on maxSupply and delegate is not correct - these
functions are used in the inheriting code and may be overwritten.

Found and reported two bugs in the detectors in moveVotingPower function.

incorrect-equality detector in writeCheckpoint is wrong as well - this line
needs to look like that because we want to update the value if we are still
in the context of the given block.
We do not need to distinguish self-delegation and delegation to someone
else for checkpoint tests. All we care about is that the delegation
does not affect the result. Also, the current version covers more paths
specific to `getPastTotalSupply`, which is burning from multiple accounts
and burning from just one account.
@pdyraga
Copy link
Member Author

pdyraga commented Oct 14, 2021

Tagging @tomaszslabon as well given that he is working on checkpoints for COV token (coverage pool underwriter token) in keep-network/coverage-pools#199.

@pdyraga
Copy link
Member Author

pdyraga commented Oct 14, 2021

FWIW, I am comfortable with merging with just @cygnusv's approval. I tagged Tomasz so that he is aware of this change but I do not expect him to review.

@cygnusv cygnusv merged commit 6664c73 into main Oct 14, 2021
@cygnusv cygnusv deleted the checkpoints-followup branch October 14, 2021 17:47
@pdyraga pdyraga added this to the solidity/v1.0.0 milestone Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants