-
Notifications
You must be signed in to change notification settings - Fork 370
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
Make the same hotfix proposal executable with unique salts #2357
Changes from 6 commits
1f7b8e6
640c70a
13b73e6
a74ca9d
d2f8559
dad6c4c
6b3c834
dd1d2e2
45d05ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -655,6 +655,7 @@ contract Governance is | |
* @param hash The abi encoded keccak256 hash of the hotfix transaction(s) to be whitelisted. | ||
*/ | ||
function approveHotfix(bytes32 hash) external { | ||
require(!hotfixes[hash].executed, "hotfix already executed"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this a modifier? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixing in #2340 |
||
require(msg.sender == approver, "Not approver"); | ||
hotfixes[hash].approved = true; | ||
emit HotfixApproved(hash); | ||
|
@@ -674,6 +675,7 @@ contract Governance is | |
* @param hash The abi encoded keccak256 hash of the hotfix transaction(s) to be whitelisted. | ||
*/ | ||
function whitelistHotfix(bytes32 hash) external { | ||
require(!hotfixes[hash].executed, "hotfix already executed"); | ||
hotfixes[hash].whitelisted[msg.sender] = true; | ||
emit HotfixWhitelisted(hash, msg.sender); | ||
} | ||
|
@@ -683,6 +685,7 @@ contract Governance is | |
* @param hash The hash of the hotfix to be prepared. | ||
*/ | ||
function prepareHotfix(bytes32 hash) external { | ||
require(!hotfixes[hash].executed, "hotfix already executed"); | ||
require(isHotfixPassing(hash), "hotfix not whitelisted by 2f+1 validators"); | ||
uint256 epoch = getEpochNumber(); | ||
require(hotfixes[hash].preparedEpoch < epoch, "hotfix already prepared for this epoch"); | ||
|
@@ -696,15 +699,17 @@ contract Governance is | |
* @param destinations The destination addresses of the proposed transactions. | ||
* @param data The concatenated data to be included in the proposed transactions. | ||
* @param dataLengths The lengths of each transaction's data. | ||
* @param salt Secret associated with hotfix which guarantees uniqueness of hash. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Does this need to be secret? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
* @dev Reverts if hotfix is already executed, not approved, or not prepared for current epoch. | ||
*/ | ||
function executeHotfix( | ||
uint256[] calldata values, | ||
address[] calldata destinations, | ||
bytes calldata data, | ||
uint256[] calldata dataLengths | ||
uint256[] calldata dataLengths, | ||
bytes32 salt | ||
) external { | ||
bytes32 hash = keccak256(abi.encode(values, destinations, data, dataLengths)); | ||
bytes32 hash = keccak256(abi.encode(values, destinations, data, dataLengths, salt)); | ||
|
||
(bool approved, bool executed, uint256 preparedEpoch) = getHotfixRecord(hash); | ||
require(!executed, "hotfix already executed"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be logged in case something goes wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto