CVE-2022-31198: Keep track of historical quorum values by Amxx · Pull Request #3561 · OpenZeppelin/openzeppelin-contracts

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

Conversation 4 Commits 11 Checks 8 Files changed

Conversation

Copy link

Contributor

** Amxx commented Jul 18, 2022**

Checkpointing quorum is necessary to avoid quorum changes making old, failed because of quorum, proposals suddenly successful.

PR Checklist

• Tests
• Changelog entry

Amxx and others added 10 commits

Jul 15, 2022

frangio previously approved these changes Jul 19, 2022

Waiting until next week to discuss a more efficient fix.

The issue with this PR is that it increases the cost of voting when the PreventLateQuorum module is used. It seems like this can be fixed by first checking if the latest checkpoint is the one we’re looking for (in quorumNumerator(uint blockNumber)) and in that case avoid the binary search. Generally, it is indeed the latest checkpoint that will be needed for voting. The checkpoint history with binary search is still appropriate for the state function.

Copy link

Contributor Author

** Amxx commented Jul 27, 2022**

I’ve implemented an optimistic lookup, that skips the binary search if the checkpoint we are looking for is the last one. However that only concerns quorumNumerator(uint256). This function is mostly called through

function quorum(uint256 blockNumber) public view virtual override returns (uint256) {
    return (token.getPastTotalSupply(blockNumber) * quorumNumerator(blockNumber)) / quorumDenominator();
}

The token.getPastTotalSupply(blockNumber) call will not be optimized the same way

Copy link

Contributor Author

** Amxx commented Jul 27, 2022**

Not, the parts not covered are mechanism used in cases of migration.

frangio changed the title Keep historical quorum values Keep track of historical quorum values

Jul 27, 2022

frangio pushed a commit that referenced this issue

Jul 27, 2022

Co-authored-by: Francisco Giordano [email protected] (cherry picked from commit 8ea1fc8)

Amxx deleted the fix/quorum branch

Jul 27, 2022

2 participants