Fix/gas refund on set vote #185
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…iguration of voteGas and maxGasPrice
…ed test related to voteGas config
|
So we are going with the assumption that gas will be around that cost regardless of chain? |
That's the assumption, yes. If that's not correct, then gas consumption must be calculated every time the function is called. |
rossneilson
previously approved these changes
Jun 28, 2022
AugustoL
requested changes
Jun 28, 2022
|
@AugustoL done! |
AugustoL
requested changes
Jun 29, 2022
| @@ -179,7 +179,7 @@ contract BaseERC20Guild { | |||
| require(_proposalTime > 0, "ERC20Guild: proposal time has to be more than 0"); | |||
| require(_lockTime >= _proposalTime, "ERC20Guild: lockTime has to be higher or equal to proposalTime"); | |||
| require(_votingPowerForProposalExecution > 0, "ERC20Guild: voting power for execution has to be more than 0"); | |||
| require(_voteGas <= 100000, "ERC20Guild: vote gas has to be equal or lower than 100.000"); // 100.000 is 85% of the gas amount to execute setVote | |||
| require(_voteGas <= 117000, "ERC20Guild: vote gas has to be equal or lower than 117.000"); | |||
There was a problem hiding this comment.
Remove the . in 117.000 please, use the plain amount 117000, we never used . for numbers and it might be confussing. Also americans use that for decimals, and , for units.
AugustoL
approved these changes
Jun 30, 2022
rossneilson
approved these changes
Jul 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to issue #159, and DXD-11 in the audit.
Fixes
setVoteexploit whenvoteGasamount is set above the actual gas spent for the function call, enabling voters to get refunded more than what they spent to call the function.It sets a hard limit of 100.000 gas (~85% of the actual 117.000 gas needed to execute
setVote()).