From 250c34e7b1cf4c5acb8eceb5a000865aa5ccf433 Mon Sep 17 00:00:00 2001 From: gadicer Date: Tue, 14 Nov 2023 10:29:08 +0200 Subject: [PATCH 01/14] add parametric_contracts comment --- security/certora/confs/verifyVotingStrategy_unittests.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/certora/confs/verifyVotingStrategy_unittests.conf b/security/certora/confs/verifyVotingStrategy_unittests.conf index 6c953f1..2a3d441 100644 --- a/security/certora/confs/verifyVotingStrategy_unittests.conf +++ b/security/certora/confs/verifyVotingStrategy_unittests.conf @@ -21,6 +21,8 @@ "verify": "VotingStrategy:security/certora/specs/VotingStrategy_unittests.spec", "optimistic_loop": true, "loop_iter": "3", // Needs 2 for isTokenSlotAccepted (A_AAVE uses 2 slots) + //"parametric_contracts":["VotingStrategy" + //], "solc": "solc8.19", "msg": "VotingStrategy tests" } From 15d4ab70b52e3bb31d064457396b8408cc634da5 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Tue, 14 Nov 2023 15:41:36 +0200 Subject: [PATCH 02/14] add mutants for payloads --- .../scripts/verifyPayloadsController.sh | 80 +++++ .../payloads/PayloadsControllerCore-1.sol | 333 ++++++++++++++++++ .../payloads/PayloadsControllerCore-2.sol | 333 ++++++++++++++++++ .../payloads/PayloadsControllerCore-3.sol | 333 ++++++++++++++++++ .../payloads/PayloadsControllerCore-4.sol | 333 ++++++++++++++++++ .../payloads/PayloadsControllerCore-5.sol | 333 ++++++++++++++++++ security/certora/tests/payloads/REPORT.txt | 50 +++ 7 files changed, 1795 insertions(+) create mode 100644 security/certora/scripts/verifyPayloadsController.sh create mode 100644 security/certora/tests/payloads/PayloadsControllerCore-1.sol create mode 100644 security/certora/tests/payloads/PayloadsControllerCore-2.sol create mode 100644 security/certora/tests/payloads/PayloadsControllerCore-3.sol create mode 100644 security/certora/tests/payloads/PayloadsControllerCore-4.sol create mode 100644 security/certora/tests/payloads/PayloadsControllerCore-5.sol create mode 100644 security/certora/tests/payloads/REPORT.txt diff --git a/security/certora/scripts/verifyPayloadsController.sh b/security/certora/scripts/verifyPayloadsController.sh new file mode 100644 index 0000000..b165625 --- /dev/null +++ b/security/certora/scripts/verifyPayloadsController.sh @@ -0,0 +1,80 @@ + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule payload_maximal_access_level_gt_action_access_level no_late_cancel state_cant_decrease no_transition_beyond_state_gt_3 no_transition_beyond_state_variable_gt_3 no_queue_after_expiration empty_actions_if_out_of_bound_payload expirationTime_equal_to_createAt_plus_EXPIRATION_DELAY empty_actions_iff_uninitialized null_access_level_if_out_of_bound_payload null_creator_and_zero_expiration_time_if_out_of_bound_payload empty_actions_only_if_uninitialized_payload executor_access_level_within_range consecutiveIDs empty_actions_if_uninitialized_payload queued_before_expiration_delay payload_grace_period_eq_global_grace_period null_access_level_only_if_out_of_bound_payload null_state_variable_if_out_of_bound_payload created_in_the_past executedAt_is_zero_before_executed queued_after_created executed_after_queue queuedAt_is_zero_before_queued no_early_cancellation guardian_can_cancel executed_when_in_queued_state execute_before_delay__maximumAccessLevelRequired action_immutable_fixed_size_fields initialized_payload_fields_are_immutable payload_fields_immutable_after_createPayload method_reachability \ + --msg "1: many rules" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule executor_exists_if_action_not_null \ + --msg "2: executor_exists_if_action_not_null" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule executor_exists_only_if_action_not_null \ + --msg "3: executor_exists_only_if_action_not_null" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule payload_delay_within_range \ + --msg "4: payload_delay_within_range" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule delay_of_executor_of_max_access_level_within_range \ + --msg "5: delay_of_executor_of_max_access_level_within_range" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule nonempty_actions \ + --msg "6: nonempty_actions" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule executor_exists_iff_action_not_null \ + --msg "7: executor_exists_iff_action_not_null" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule null_access_level_iff_state_is_none \ + --msg "8: null_access_level_iff_state_is_none" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule executor_of_maximumAccessLevelRequired_exists \ + --msg "9: executor_of_maximumAccessLevelRequired_exists" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule executor_of_maximumAccessLevelRequired_exists_after_createPayload \ + --msg "10: executor_of_maximumAccessLevelRequired_exists_after_createPayload" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule action_access_level_isnt_null_after_createPayload \ + --msg "11: action_access_level_isnt_null_after_createPayload" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule executor_exists_after_createPayload \ + --msg "12: executor_exists_after_createPayload" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule action_callData_immutable \ + --msg "13: action_callData_immutable" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule action_signature_immutable \ + --msg "14: action_signature_immutable" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule action_immutable_check_only_fixed_size_fields \ + --msg "15: action_immutable_check_only_fixed_size_fields" + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/payloads/verifyPayloadsController.conf \ + --rule zero_executedAt_if_not_executed \ + --msg "16: zero_executedAt_if_not_executed" diff --git a/security/certora/tests/payloads/PayloadsControllerCore-1.sol b/security/certora/tests/payloads/PayloadsControllerCore-1.sol new file mode 100644 index 0000000..b289d75 --- /dev/null +++ b/security/certora/tests/payloads/PayloadsControllerCore-1.sol @@ -0,0 +1,333 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.8; + +import {OwnableWithGuardian} from 'solidity-utils/contracts/access-control/OwnableWithGuardian.sol'; +import {Rescuable, IRescuable} from 'solidity-utils/contracts/utils/Rescuable.sol'; +import {IERC20} from 'solidity-utils/contracts/oz-common/interfaces/IERC20.sol'; +import {Initializable} from 'solidity-utils/contracts/transparent-proxy/Initializable.sol'; +import {SafeERC20} from 'solidity-utils/contracts/oz-common/SafeERC20.sol'; +import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; + +import {IPayloadsControllerCore, PayloadsControllerUtils} from './interfaces/IPayloadsControllerCore.sol'; +import {IExecutor} from './interfaces/IExecutor.sol'; +import {Errors} from '../libraries/Errors.sol'; + +/** + * @title PayloadsControllerCore + * @author BGD Labs + * @notice this contract contains the logic to create and execute a payload. + * @dev To execute a created payload, the payload id must be bridged from governance chain. + * @dev The methods to update the contract configuration are callable only by owner. Owner being the registered + lvl1 Executor. So to update the PayloadsController configuration, a proposal will need to pass in gov chain and + be executed by the appropriate executor. + */ +abstract contract PayloadsControllerCore is + OwnableWithGuardian, + Rescuable, + IPayloadsControllerCore, + Initializable +{ + using SafeCast for uint256; + using SafeERC20 for IERC20; + + // should be always set with respect to the proposal flow duration + // for example: voting takes 5 days + 2 days for bridging + 3 days for cooldown + 2 days of safety gap + // then expirationDelay should be not less then 12 days. + // As expiration delay of proposal is 30 days, payload needs to be able to live longer as its created before and + // will be executed after. + // so nobody should be able to set expiration delay less then that + uint40 public constant EXPIRATION_DELAY = 35 days; + + /// @inheritdoc IPayloadsControllerCore + uint40 public constant GRACE_PERIOD = 7 days; + + uint40 internal _payloadsCount; + + // stores the executor configuration for every lvl of access control + mapping(PayloadsControllerUtils.AccessControl => ExecutorConfig) + internal _accessLevelToExecutorConfig; + + mapping(uint40 => Payload) internal _payloads; + + /// @inheritdoc IPayloadsControllerCore + function MIN_EXECUTION_DELAY() public view virtual returns (uint40) { + return 1 days; + } + + /// @inheritdoc IPayloadsControllerCore + function MAX_EXECUTION_DELAY() public view virtual returns (uint40) { + return 10 days; + } + + function initialize( + address owner, + address guardian, + UpdateExecutorInput[] calldata executors + ) external initializer { + require(executors.length != 0, Errors.SHOULD_BE_AT_LEAST_ONE_EXECUTOR); + + _updateExecutors(executors); + + _updateGuardian(guardian); + _transferOwnership(owner); + } + + /// @inheritdoc IPayloadsControllerCore + function createPayload( + ExecutionAction[] calldata actions + ) external returns (uint40) { + require(actions.length != 0, Errors.INVALID_EMPTY_TARGETS); + + //uint40 payloadId = _payloadsCount++; //ORIG + uint40 payloadId = _payloadsCount; // MUTANT + uint40 creationTime = uint40(block.timestamp); + Payload storage newPayload = _payloads[payloadId]; + newPayload.creator = msg.sender; + newPayload.state = PayloadState.Created; + newPayload.createdAt = creationTime; + newPayload.expirationTime = creationTime + EXPIRATION_DELAY; + newPayload.gracePeriod = GRACE_PERIOD; + + PayloadsControllerUtils.AccessControl maximumAccessLevelRequired; + for (uint256 i = 0; i < actions.length; i++) { + require(actions[i].target != address(0), Errors.INVALID_ACTION_TARGET); + require( + actions[i].accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_ACTION_ACCESS_LEVEL + ); + require( + _accessLevelToExecutorConfig[actions[i].accessLevel].executor != + address(0), + Errors.EXECUTOR_WAS_NOT_SPECIFIED_FOR_REQUESTED_ACCESS_LEVEL + ); + + newPayload.actions.push(actions[i]); + + if (actions[i].accessLevel > maximumAccessLevelRequired) { + maximumAccessLevelRequired = actions[i].accessLevel; + } + } + newPayload.maximumAccessLevelRequired = maximumAccessLevelRequired; + ExecutorConfig + memory maxRequiredExecutorConfig = _accessLevelToExecutorConfig[ + maximumAccessLevelRequired + ]; + newPayload.delay = maxRequiredExecutorConfig.delay; + + emit PayloadCreated( + payloadId, + msg.sender, + actions, + maximumAccessLevelRequired + ); + return payloadId; + } + + /// @inheritdoc IPayloadsControllerCore + function executePayload(uint40 payloadId) external payable { + Payload storage payload = _payloads[payloadId]; + + require( + _getPayloadState(payload) == PayloadState.Queued, + Errors.PAYLOAD_NOT_IN_QUEUED_STATE + ); + + uint256 executionTime = payload.queuedAt + payload.delay; + require(block.timestamp > executionTime, Errors.TIMELOCK_NOT_FINISHED); + + payload.state = PayloadState.Executed; + payload.executedAt = uint40(block.timestamp); + + for (uint256 i = 0; i < payload.actions.length; i++) { + ExecutionAction storage action = payload.actions[i]; + IExecutor executor = IExecutor( + _accessLevelToExecutorConfig[action.accessLevel].executor + ); + + executor.executeTransaction{value: action.value}( + action.target, + action.value, + action.signature, + action.callData, + action.withDelegateCall + ); + } + + emit PayloadExecuted(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function cancelPayload(uint40 payloadId) external onlyGuardian { + Payload storage payload = _payloads[payloadId]; + + PayloadState payloadState = _getPayloadState(payload); + require( + payloadState < PayloadState.Executed && + payloadState >= PayloadState.Created, + Errors.PAYLOAD_NOT_IN_THE_CORRECT_STATE + ); + payload.state = PayloadState.Cancelled; + payload.cancelledAt = uint40(block.timestamp); + + emit PayloadCancelled(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function updateExecutors( + UpdateExecutorInput[] calldata executors + ) external onlyOwner { + _updateExecutors(executors); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadById( + uint40 payloadId + ) external view returns (Payload memory) { + Payload memory payload = _payloads[payloadId]; + payload.state = _getPayloadState(payload); + return payload; + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadState( + uint40 payloadId + ) external view returns (PayloadState) { + return _getPayloadState(_payloads[payloadId]); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadsCount() external view returns (uint40) { + return _payloadsCount; + } + + /// @inheritdoc IPayloadsControllerCore + function getExecutorSettingsByAccessControl( + PayloadsControllerUtils.AccessControl accessControl + ) external view returns (ExecutorConfig memory) { + return _accessLevelToExecutorConfig[accessControl]; + } + + /// @inheritdoc IRescuable + function whoCanRescue() + public + view + override(IRescuable, Rescuable) + returns (address) + { + return owner(); + } + + receive() external payable {} + + /** + * @notice method to get the current state of a payload + * @param payload object with all pertinent payload information + * @return current state of the payload + */ + function _getPayloadState( + Payload memory payload + ) internal view returns (PayloadState) { + PayloadState state = payload.state; + if (state == PayloadState.None || state >= PayloadState.Executed) { + return state; + } + + if ( + (state == PayloadState.Created && + block.timestamp >= payload.expirationTime) || + (state == PayloadState.Queued && + block.timestamp >= + payload.queuedAt + payload.delay + payload.gracePeriod) + ) { + return PayloadState.Expired; + } + + return state; + } + + /** + * @notice method to queue a payload + * @param payloadId id of the payload that needs to be queued + * @param accessLevel access level used for the proposal voting + * @param proposalVoteActivationTimestamp proposal vote activation timestamp in seconds + * @dev this method will be called when a payload is bridged from governance chain + */ + function _queuePayload( + uint40 payloadId, + PayloadsControllerUtils.AccessControl accessLevel, + uint40 proposalVoteActivationTimestamp + ) internal { + Payload storage payload = _payloads[payloadId]; + require( + _getPayloadState(payload) == PayloadState.Created, + Errors.PAYLOAD_NOT_IN_CREATED_STATE + ); + + // by allowing >= it enables the proposal to use a higher level of voting configuration + // than the one set by the payload actions + require( + accessLevel >= payload.maximumAccessLevelRequired, + Errors.INVALID_PROPOSAL_ACCESS_LEVEL + ); + // this checks that the payload has been created before the proposal vote started. + // this ensures that the voters where able to check the content of the payload they voted on + require( + proposalVoteActivationTimestamp > payload.createdAt, + Errors.PAYLOAD_NOT_CREATED_BEFORE_PROPOSAL + ); + + payload.state = PayloadState.Queued; + payload.queuedAt = uint40(block.timestamp); + + emit PayloadQueued(payloadId); + } + + /** + * @notice add new executor configs + * @param executors array of UpdateExecutorInput with needed executor configurations + */ + function _updateExecutors(UpdateExecutorInput[] memory executors) internal { + for (uint256 i = 0; i < executors.length; i++) { + UpdateExecutorInput memory newExecutorConfig = executors[i]; + + require( + newExecutorConfig.executorConfig.executor != address(0), + Errors.INVALID_EXECUTOR_ADDRESS + ); + + require( + newExecutorConfig.accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_EXECUTOR_ACCESS_LEVEL + ); + + require( + newExecutorConfig.executorConfig.delay >= MIN_EXECUTION_DELAY() && + newExecutorConfig.executorConfig.delay <= MAX_EXECUTION_DELAY(), + Errors.INVALID_EXECUTOR_DELAY + ); + + // check that the new executor is not already being used in a different level + PayloadsControllerUtils.AccessControl levelToCheck = newExecutorConfig + .accessLevel == PayloadsControllerUtils.AccessControl.Level_1 + ? PayloadsControllerUtils.AccessControl.Level_2 + : PayloadsControllerUtils.AccessControl.Level_1; + require( + _accessLevelToExecutorConfig[levelToCheck].executor != + newExecutorConfig.executorConfig.executor, + Errors.EXECUTOR_ALREADY_SET_IN_DIFFERENT_LEVEL + ); + + _accessLevelToExecutorConfig[ + newExecutorConfig.accessLevel + ] = newExecutorConfig.executorConfig; + + emit ExecutorSet( + newExecutorConfig.accessLevel, + newExecutorConfig.executorConfig.executor, + newExecutorConfig.executorConfig.delay + ); + } + } +} diff --git a/security/certora/tests/payloads/PayloadsControllerCore-2.sol b/security/certora/tests/payloads/PayloadsControllerCore-2.sol new file mode 100644 index 0000000..88a2188 --- /dev/null +++ b/security/certora/tests/payloads/PayloadsControllerCore-2.sol @@ -0,0 +1,333 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.8; + +import {OwnableWithGuardian} from 'solidity-utils/contracts/access-control/OwnableWithGuardian.sol'; +import {Rescuable, IRescuable} from 'solidity-utils/contracts/utils/Rescuable.sol'; +import {IERC20} from 'solidity-utils/contracts/oz-common/interfaces/IERC20.sol'; +import {Initializable} from 'solidity-utils/contracts/transparent-proxy/Initializable.sol'; +import {SafeERC20} from 'solidity-utils/contracts/oz-common/SafeERC20.sol'; +import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; + +import {IPayloadsControllerCore, PayloadsControllerUtils} from './interfaces/IPayloadsControllerCore.sol'; +import {IExecutor} from './interfaces/IExecutor.sol'; +import {Errors} from '../libraries/Errors.sol'; + +/** + * @title PayloadsControllerCore + * @author BGD Labs + * @notice this contract contains the logic to create and execute a payload. + * @dev To execute a created payload, the payload id must be bridged from governance chain. + * @dev The methods to update the contract configuration are callable only by owner. Owner being the registered + lvl1 Executor. So to update the PayloadsController configuration, a proposal will need to pass in gov chain and + be executed by the appropriate executor. + */ +abstract contract PayloadsControllerCore is + OwnableWithGuardian, + Rescuable, + IPayloadsControllerCore, + Initializable +{ + using SafeCast for uint256; + using SafeERC20 for IERC20; + + // should be always set with respect to the proposal flow duration + // for example: voting takes 5 days + 2 days for bridging + 3 days for cooldown + 2 days of safety gap + // then expirationDelay should be not less then 12 days. + // As expiration delay of proposal is 30 days, payload needs to be able to live longer as its created before and + // will be executed after. + // so nobody should be able to set expiration delay less then that + uint40 public constant EXPIRATION_DELAY = 35 days; + + /// @inheritdoc IPayloadsControllerCore + uint40 public constant GRACE_PERIOD = 7 days; + + uint40 internal _payloadsCount; + + // stores the executor configuration for every lvl of access control + mapping(PayloadsControllerUtils.AccessControl => ExecutorConfig) + internal _accessLevelToExecutorConfig; + + mapping(uint40 => Payload) internal _payloads; + + /// @inheritdoc IPayloadsControllerCore + function MIN_EXECUTION_DELAY() public view virtual returns (uint40) { + return 1 days; + } + + /// @inheritdoc IPayloadsControllerCore + function MAX_EXECUTION_DELAY() public view virtual returns (uint40) { + return 10 days; + } + + function initialize( + address owner, + address guardian, + UpdateExecutorInput[] calldata executors + ) external initializer { + require(executors.length != 0, Errors.SHOULD_BE_AT_LEAST_ONE_EXECUTOR); + + _updateExecutors(executors); + + _updateGuardian(guardian); + _transferOwnership(owner); + } + + /// @inheritdoc IPayloadsControllerCore + function createPayload( + ExecutionAction[] calldata actions + ) external returns (uint40) { + require(actions.length != 0, Errors.INVALID_EMPTY_TARGETS); + + uint40 payloadId = _payloadsCount++; + uint40 creationTime = uint40(block.timestamp); + Payload storage newPayload = _payloads[payloadId]; + newPayload.creator = msg.sender; + newPayload.state = PayloadState.Created; + newPayload.createdAt = creationTime; + newPayload.expirationTime = creationTime + EXPIRATION_DELAY; + newPayload.gracePeriod = GRACE_PERIOD; + + PayloadsControllerUtils.AccessControl maximumAccessLevelRequired; + for (uint256 i = 0; i < actions.length; i++) { + require(actions[i].target != address(0), Errors.INVALID_ACTION_TARGET); + require( + actions[i].accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_ACTION_ACCESS_LEVEL + ); + require( + _accessLevelToExecutorConfig[actions[i].accessLevel].executor != + address(0), + Errors.EXECUTOR_WAS_NOT_SPECIFIED_FOR_REQUESTED_ACCESS_LEVEL + ); + + newPayload.actions.push(actions[i]); + + if (actions[i].accessLevel > maximumAccessLevelRequired) { + maximumAccessLevelRequired = actions[i].accessLevel; + } + } + newPayload.maximumAccessLevelRequired = maximumAccessLevelRequired; + ExecutorConfig + memory maxRequiredExecutorConfig = _accessLevelToExecutorConfig[ + maximumAccessLevelRequired + ]; + newPayload.delay = maxRequiredExecutorConfig.delay; + + emit PayloadCreated( + payloadId, + msg.sender, + actions, + maximumAccessLevelRequired + ); + return payloadId; + } + + /// @inheritdoc IPayloadsControllerCore + function executePayload(uint40 payloadId) external payable { + Payload storage payload = _payloads[payloadId]; + + require( + _getPayloadState(payload) == PayloadState.Queued, + Errors.PAYLOAD_NOT_IN_QUEUED_STATE + ); + + uint256 executionTime = payload.queuedAt + payload.delay; + require(block.timestamp > executionTime, Errors.TIMELOCK_NOT_FINISHED); + + payload.state = PayloadState.Executed; + payload.executedAt = uint40(block.timestamp); + + for (uint256 i = 0; i < payload.actions.length; i++) { + //ExecutionAction storage action = payload.actions[i]; // ORIG + ExecutionAction storage action = payload.actions[0]; // MUTANT + IExecutor executor = IExecutor( + _accessLevelToExecutorConfig[action.accessLevel].executor + ); + + executor.executeTransaction{value: action.value}( + action.target, + action.value, + action.signature, + action.callData, + action.withDelegateCall + ); + } + + emit PayloadExecuted(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function cancelPayload(uint40 payloadId) external onlyGuardian { + Payload storage payload = _payloads[payloadId]; + + PayloadState payloadState = _getPayloadState(payload); + require( + payloadState < PayloadState.Executed && + payloadState >= PayloadState.Created, + Errors.PAYLOAD_NOT_IN_THE_CORRECT_STATE + ); + payload.state = PayloadState.Cancelled; + payload.cancelledAt = uint40(block.timestamp); + + emit PayloadCancelled(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function updateExecutors( + UpdateExecutorInput[] calldata executors + ) external onlyOwner { + _updateExecutors(executors); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadById( + uint40 payloadId + ) external view returns (Payload memory) { + Payload memory payload = _payloads[payloadId]; + payload.state = _getPayloadState(payload); + return payload; + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadState( + uint40 payloadId + ) external view returns (PayloadState) { + return _getPayloadState(_payloads[payloadId]); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadsCount() external view returns (uint40) { + return _payloadsCount; + } + + /// @inheritdoc IPayloadsControllerCore + function getExecutorSettingsByAccessControl( + PayloadsControllerUtils.AccessControl accessControl + ) external view returns (ExecutorConfig memory) { + return _accessLevelToExecutorConfig[accessControl]; + } + + /// @inheritdoc IRescuable + function whoCanRescue() + public + view + override(IRescuable, Rescuable) + returns (address) + { + return owner(); + } + + receive() external payable {} + + /** + * @notice method to get the current state of a payload + * @param payload object with all pertinent payload information + * @return current state of the payload + */ + function _getPayloadState( + Payload memory payload + ) internal view returns (PayloadState) { + PayloadState state = payload.state; + if (state == PayloadState.None || state >= PayloadState.Executed) { + return state; + } + + if ( + (state == PayloadState.Created && + block.timestamp >= payload.expirationTime) || + (state == PayloadState.Queued && + block.timestamp >= + payload.queuedAt + payload.delay + payload.gracePeriod) + ) { + return PayloadState.Expired; + } + + return state; + } + + /** + * @notice method to queue a payload + * @param payloadId id of the payload that needs to be queued + * @param accessLevel access level used for the proposal voting + * @param proposalVoteActivationTimestamp proposal vote activation timestamp in seconds + * @dev this method will be called when a payload is bridged from governance chain + */ + function _queuePayload( + uint40 payloadId, + PayloadsControllerUtils.AccessControl accessLevel, + uint40 proposalVoteActivationTimestamp + ) internal { + Payload storage payload = _payloads[payloadId]; + require( + _getPayloadState(payload) == PayloadState.Created, + Errors.PAYLOAD_NOT_IN_CREATED_STATE + ); + + // by allowing >= it enables the proposal to use a higher level of voting configuration + // than the one set by the payload actions + require( + accessLevel >= payload.maximumAccessLevelRequired, + Errors.INVALID_PROPOSAL_ACCESS_LEVEL + ); + // this checks that the payload has been created before the proposal vote started. + // this ensures that the voters where able to check the content of the payload they voted on + require( + proposalVoteActivationTimestamp > payload.createdAt, + Errors.PAYLOAD_NOT_CREATED_BEFORE_PROPOSAL + ); + + payload.state = PayloadState.Queued; + payload.queuedAt = uint40(block.timestamp); + + emit PayloadQueued(payloadId); + } + + /** + * @notice add new executor configs + * @param executors array of UpdateExecutorInput with needed executor configurations + */ + function _updateExecutors(UpdateExecutorInput[] memory executors) internal { + for (uint256 i = 0; i < executors.length; i++) { + UpdateExecutorInput memory newExecutorConfig = executors[i]; + + require( + newExecutorConfig.executorConfig.executor != address(0), + Errors.INVALID_EXECUTOR_ADDRESS + ); + + require( + newExecutorConfig.accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_EXECUTOR_ACCESS_LEVEL + ); + + require( + newExecutorConfig.executorConfig.delay >= MIN_EXECUTION_DELAY() && + newExecutorConfig.executorConfig.delay <= MAX_EXECUTION_DELAY(), + Errors.INVALID_EXECUTOR_DELAY + ); + + // check that the new executor is not already being used in a different level + PayloadsControllerUtils.AccessControl levelToCheck = newExecutorConfig + .accessLevel == PayloadsControllerUtils.AccessControl.Level_1 + ? PayloadsControllerUtils.AccessControl.Level_2 + : PayloadsControllerUtils.AccessControl.Level_1; + require( + _accessLevelToExecutorConfig[levelToCheck].executor != + newExecutorConfig.executorConfig.executor, + Errors.EXECUTOR_ALREADY_SET_IN_DIFFERENT_LEVEL + ); + + _accessLevelToExecutorConfig[ + newExecutorConfig.accessLevel + ] = newExecutorConfig.executorConfig; + + emit ExecutorSet( + newExecutorConfig.accessLevel, + newExecutorConfig.executorConfig.executor, + newExecutorConfig.executorConfig.delay + ); + } + } +} diff --git a/security/certora/tests/payloads/PayloadsControllerCore-3.sol b/security/certora/tests/payloads/PayloadsControllerCore-3.sol new file mode 100644 index 0000000..896d0c7 --- /dev/null +++ b/security/certora/tests/payloads/PayloadsControllerCore-3.sol @@ -0,0 +1,333 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.8; + +import {OwnableWithGuardian} from 'solidity-utils/contracts/access-control/OwnableWithGuardian.sol'; +import {Rescuable, IRescuable} from 'solidity-utils/contracts/utils/Rescuable.sol'; +import {IERC20} from 'solidity-utils/contracts/oz-common/interfaces/IERC20.sol'; +import {Initializable} from 'solidity-utils/contracts/transparent-proxy/Initializable.sol'; +import {SafeERC20} from 'solidity-utils/contracts/oz-common/SafeERC20.sol'; +import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; + +import {IPayloadsControllerCore, PayloadsControllerUtils} from './interfaces/IPayloadsControllerCore.sol'; +import {IExecutor} from './interfaces/IExecutor.sol'; +import {Errors} from '../libraries/Errors.sol'; + +/** + * @title PayloadsControllerCore + * @author BGD Labs + * @notice this contract contains the logic to create and execute a payload. + * @dev To execute a created payload, the payload id must be bridged from governance chain. + * @dev The methods to update the contract configuration are callable only by owner. Owner being the registered + lvl1 Executor. So to update the PayloadsController configuration, a proposal will need to pass in gov chain and + be executed by the appropriate executor. + */ +abstract contract PayloadsControllerCore is + OwnableWithGuardian, + Rescuable, + IPayloadsControllerCore, + Initializable +{ + using SafeCast for uint256; + using SafeERC20 for IERC20; + + // should be always set with respect to the proposal flow duration + // for example: voting takes 5 days + 2 days for bridging + 3 days for cooldown + 2 days of safety gap + // then expirationDelay should be not less then 12 days. + // As expiration delay of proposal is 30 days, payload needs to be able to live longer as its created before and + // will be executed after. + // so nobody should be able to set expiration delay less then that + uint40 public constant EXPIRATION_DELAY = 35 days; + + /// @inheritdoc IPayloadsControllerCore + uint40 public constant GRACE_PERIOD = 7 days; + + uint40 internal _payloadsCount; + + // stores the executor configuration for every lvl of access control + mapping(PayloadsControllerUtils.AccessControl => ExecutorConfig) + internal _accessLevelToExecutorConfig; + + mapping(uint40 => Payload) internal _payloads; + + /// @inheritdoc IPayloadsControllerCore + function MIN_EXECUTION_DELAY() public view virtual returns (uint40) { + return 1 days; + } + + /// @inheritdoc IPayloadsControllerCore + function MAX_EXECUTION_DELAY() public view virtual returns (uint40) { + return 10 days; + } + + function initialize( + address owner, + address guardian, + UpdateExecutorInput[] calldata executors + ) external initializer { + require(executors.length != 0, Errors.SHOULD_BE_AT_LEAST_ONE_EXECUTOR); + + _updateExecutors(executors); + + _updateGuardian(guardian); + _transferOwnership(owner); + } + + /// @inheritdoc IPayloadsControllerCore + function createPayload( + ExecutionAction[] calldata actions + ) external returns (uint40) { + require(actions.length != 0, Errors.INVALID_EMPTY_TARGETS); + + uint40 payloadId = _payloadsCount++; + uint40 creationTime = uint40(block.timestamp); + Payload storage newPayload = _payloads[payloadId]; + newPayload.creator = msg.sender; + newPayload.state = PayloadState.Created; + newPayload.createdAt = creationTime; + newPayload.expirationTime = creationTime + EXPIRATION_DELAY; + newPayload.gracePeriod = GRACE_PERIOD; + + PayloadsControllerUtils.AccessControl maximumAccessLevelRequired; + for (uint256 i = 0; i < actions.length; i++) { + require(actions[i].target != address(0), Errors.INVALID_ACTION_TARGET); + require( + actions[i].accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_ACTION_ACCESS_LEVEL + ); + require( + _accessLevelToExecutorConfig[actions[i].accessLevel].executor != + address(0), + Errors.EXECUTOR_WAS_NOT_SPECIFIED_FOR_REQUESTED_ACCESS_LEVEL + ); + + newPayload.actions.push(actions[i]); + + if (actions[i].accessLevel > maximumAccessLevelRequired) { + maximumAccessLevelRequired = actions[i].accessLevel; + } + } + newPayload.maximumAccessLevelRequired = maximumAccessLevelRequired; + ExecutorConfig + memory maxRequiredExecutorConfig = _accessLevelToExecutorConfig[ + maximumAccessLevelRequired + ]; + newPayload.delay = maxRequiredExecutorConfig.delay; + + emit PayloadCreated( + payloadId, + msg.sender, + actions, + maximumAccessLevelRequired + ); + return payloadId; + } + + /// @inheritdoc IPayloadsControllerCore + function executePayload(uint40 payloadId) external payable { + Payload storage payload = _payloads[payloadId]; + + require( + _getPayloadState(payload) == PayloadState.Queued, + Errors.PAYLOAD_NOT_IN_QUEUED_STATE + ); + + uint256 executionTime = payload.queuedAt + payload.delay; + require(block.timestamp > executionTime, Errors.TIMELOCK_NOT_FINISHED); + + payload.state = PayloadState.Executed; + payload.executedAt = uint40(block.timestamp); + + for (uint256 i = 0; i < payload.actions.length; i++) { + ExecutionAction storage action = payload.actions[i]; + IExecutor executor = IExecutor( + _accessLevelToExecutorConfig[action.accessLevel].executor + ); + + executor.executeTransaction{value: action.value}( + action.target, + action.value, + action.signature, + action.callData, + action.withDelegateCall + ); + } + + emit PayloadExecuted(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function cancelPayload(uint40 payloadId) external onlyGuardian { + Payload storage payload = _payloads[payloadId]; + + PayloadState payloadState = _getPayloadState(payload); + require( + payloadState < PayloadState.Executed && + payloadState >= PayloadState.Created, + Errors.PAYLOAD_NOT_IN_THE_CORRECT_STATE + ); + //payload.state = PayloadState.Cancelled; // ORIG + payload.state = PayloadState.Queued; // MUTANT + payload.cancelledAt = uint40(block.timestamp); + + emit PayloadCancelled(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function updateExecutors( + UpdateExecutorInput[] calldata executors + ) external onlyOwner { + _updateExecutors(executors); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadById( + uint40 payloadId + ) external view returns (Payload memory) { + Payload memory payload = _payloads[payloadId]; + payload.state = _getPayloadState(payload); + return payload; + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadState( + uint40 payloadId + ) external view returns (PayloadState) { + return _getPayloadState(_payloads[payloadId]); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadsCount() external view returns (uint40) { + return _payloadsCount; + } + + /// @inheritdoc IPayloadsControllerCore + function getExecutorSettingsByAccessControl( + PayloadsControllerUtils.AccessControl accessControl + ) external view returns (ExecutorConfig memory) { + return _accessLevelToExecutorConfig[accessControl]; + } + + /// @inheritdoc IRescuable + function whoCanRescue() + public + view + override(IRescuable, Rescuable) + returns (address) + { + return owner(); + } + + receive() external payable {} + + /** + * @notice method to get the current state of a payload + * @param payload object with all pertinent payload information + * @return current state of the payload + */ + function _getPayloadState( + Payload memory payload + ) internal view returns (PayloadState) { + PayloadState state = payload.state; + if (state == PayloadState.None || state >= PayloadState.Executed) { + return state; + } + + if ( + (state == PayloadState.Created && + block.timestamp >= payload.expirationTime) || + (state == PayloadState.Queued && + block.timestamp >= + payload.queuedAt + payload.delay + payload.gracePeriod) + ) { + return PayloadState.Expired; + } + + return state; + } + + /** + * @notice method to queue a payload + * @param payloadId id of the payload that needs to be queued + * @param accessLevel access level used for the proposal voting + * @param proposalVoteActivationTimestamp proposal vote activation timestamp in seconds + * @dev this method will be called when a payload is bridged from governance chain + */ + function _queuePayload( + uint40 payloadId, + PayloadsControllerUtils.AccessControl accessLevel, + uint40 proposalVoteActivationTimestamp + ) internal { + Payload storage payload = _payloads[payloadId]; + require( + _getPayloadState(payload) == PayloadState.Created, + Errors.PAYLOAD_NOT_IN_CREATED_STATE + ); + + // by allowing >= it enables the proposal to use a higher level of voting configuration + // than the one set by the payload actions + require( + accessLevel >= payload.maximumAccessLevelRequired, + Errors.INVALID_PROPOSAL_ACCESS_LEVEL + ); + // this checks that the payload has been created before the proposal vote started. + // this ensures that the voters where able to check the content of the payload they voted on + require( + proposalVoteActivationTimestamp > payload.createdAt, + Errors.PAYLOAD_NOT_CREATED_BEFORE_PROPOSAL + ); + + payload.state = PayloadState.Queued; + payload.queuedAt = uint40(block.timestamp); + + emit PayloadQueued(payloadId); + } + + /** + * @notice add new executor configs + * @param executors array of UpdateExecutorInput with needed executor configurations + */ + function _updateExecutors(UpdateExecutorInput[] memory executors) internal { + for (uint256 i = 0; i < executors.length; i++) { + UpdateExecutorInput memory newExecutorConfig = executors[i]; + + require( + newExecutorConfig.executorConfig.executor != address(0), + Errors.INVALID_EXECUTOR_ADDRESS + ); + + require( + newExecutorConfig.accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_EXECUTOR_ACCESS_LEVEL + ); + + require( + newExecutorConfig.executorConfig.delay >= MIN_EXECUTION_DELAY() && + newExecutorConfig.executorConfig.delay <= MAX_EXECUTION_DELAY(), + Errors.INVALID_EXECUTOR_DELAY + ); + + // check that the new executor is not already being used in a different level + PayloadsControllerUtils.AccessControl levelToCheck = newExecutorConfig + .accessLevel == PayloadsControllerUtils.AccessControl.Level_1 + ? PayloadsControllerUtils.AccessControl.Level_2 + : PayloadsControllerUtils.AccessControl.Level_1; + require( + _accessLevelToExecutorConfig[levelToCheck].executor != + newExecutorConfig.executorConfig.executor, + Errors.EXECUTOR_ALREADY_SET_IN_DIFFERENT_LEVEL + ); + + _accessLevelToExecutorConfig[ + newExecutorConfig.accessLevel + ] = newExecutorConfig.executorConfig; + + emit ExecutorSet( + newExecutorConfig.accessLevel, + newExecutorConfig.executorConfig.executor, + newExecutorConfig.executorConfig.delay + ); + } + } +} diff --git a/security/certora/tests/payloads/PayloadsControllerCore-4.sol b/security/certora/tests/payloads/PayloadsControllerCore-4.sol new file mode 100644 index 0000000..c351994 --- /dev/null +++ b/security/certora/tests/payloads/PayloadsControllerCore-4.sol @@ -0,0 +1,333 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.8; + +import {OwnableWithGuardian} from 'solidity-utils/contracts/access-control/OwnableWithGuardian.sol'; +import {Rescuable, IRescuable} from 'solidity-utils/contracts/utils/Rescuable.sol'; +import {IERC20} from 'solidity-utils/contracts/oz-common/interfaces/IERC20.sol'; +import {Initializable} from 'solidity-utils/contracts/transparent-proxy/Initializable.sol'; +import {SafeERC20} from 'solidity-utils/contracts/oz-common/SafeERC20.sol'; +import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; + +import {IPayloadsControllerCore, PayloadsControllerUtils} from './interfaces/IPayloadsControllerCore.sol'; +import {IExecutor} from './interfaces/IExecutor.sol'; +import {Errors} from '../libraries/Errors.sol'; + +/** + * @title PayloadsControllerCore + * @author BGD Labs + * @notice this contract contains the logic to create and execute a payload. + * @dev To execute a created payload, the payload id must be bridged from governance chain. + * @dev The methods to update the contract configuration are callable only by owner. Owner being the registered + lvl1 Executor. So to update the PayloadsController configuration, a proposal will need to pass in gov chain and + be executed by the appropriate executor. + */ +abstract contract PayloadsControllerCore is + OwnableWithGuardian, + Rescuable, + IPayloadsControllerCore, + Initializable +{ + using SafeCast for uint256; + using SafeERC20 for IERC20; + + // should be always set with respect to the proposal flow duration + // for example: voting takes 5 days + 2 days for bridging + 3 days for cooldown + 2 days of safety gap + // then expirationDelay should be not less then 12 days. + // As expiration delay of proposal is 30 days, payload needs to be able to live longer as its created before and + // will be executed after. + // so nobody should be able to set expiration delay less then that + uint40 public constant EXPIRATION_DELAY = 35 days; + + /// @inheritdoc IPayloadsControllerCore + uint40 public constant GRACE_PERIOD = 7 days; + + uint40 internal _payloadsCount; + + // stores the executor configuration for every lvl of access control + mapping(PayloadsControllerUtils.AccessControl => ExecutorConfig) + internal _accessLevelToExecutorConfig; + + mapping(uint40 => Payload) internal _payloads; + + /// @inheritdoc IPayloadsControllerCore + function MIN_EXECUTION_DELAY() public view virtual returns (uint40) { + return 1 days; + } + + /// @inheritdoc IPayloadsControllerCore + function MAX_EXECUTION_DELAY() public view virtual returns (uint40) { + return 10 days; + } + + function initialize( + address owner, + address guardian, + UpdateExecutorInput[] calldata executors + ) external initializer { + require(executors.length != 0, Errors.SHOULD_BE_AT_LEAST_ONE_EXECUTOR); + + _updateExecutors(executors); + + _updateGuardian(guardian); + _transferOwnership(owner); + } + + /// @inheritdoc IPayloadsControllerCore + function createPayload( + ExecutionAction[] calldata actions + ) external returns (uint40) { + require(actions.length != 0, Errors.INVALID_EMPTY_TARGETS); + + uint40 payloadId = _payloadsCount++; + uint40 creationTime = uint40(block.timestamp); + Payload storage newPayload = _payloads[payloadId]; + newPayload.creator = msg.sender; + newPayload.state = PayloadState.Created; + newPayload.createdAt = creationTime; + newPayload.expirationTime = creationTime + EXPIRATION_DELAY; + newPayload.gracePeriod = GRACE_PERIOD; + + PayloadsControllerUtils.AccessControl maximumAccessLevelRequired; + for (uint256 i = 0; i < actions.length; i++) { + require(actions[i].target != address(0), Errors.INVALID_ACTION_TARGET); + require( + actions[i].accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_ACTION_ACCESS_LEVEL + ); + require( + _accessLevelToExecutorConfig[actions[i].accessLevel].executor != + address(0), + Errors.EXECUTOR_WAS_NOT_SPECIFIED_FOR_REQUESTED_ACCESS_LEVEL + ); + + newPayload.actions.push(actions[i]); + + if (actions[i].accessLevel > maximumAccessLevelRequired) { + maximumAccessLevelRequired = actions[i].accessLevel; + } + } + newPayload.maximumAccessLevelRequired = maximumAccessLevelRequired; + ExecutorConfig + memory maxRequiredExecutorConfig = _accessLevelToExecutorConfig[ + maximumAccessLevelRequired + ]; + newPayload.delay = maxRequiredExecutorConfig.delay; + + emit PayloadCreated( + payloadId, + msg.sender, + actions, + maximumAccessLevelRequired + ); + return payloadId; + } + + /// @inheritdoc IPayloadsControllerCore + function executePayload(uint40 payloadId) external payable { + Payload storage payload = _payloads[payloadId]; + + require( + _getPayloadState(payload) == PayloadState.Queued, + Errors.PAYLOAD_NOT_IN_QUEUED_STATE + ); + + uint256 executionTime = payload.queuedAt + payload.delay; + require(block.timestamp > executionTime, Errors.TIMELOCK_NOT_FINISHED); + + payload.state = PayloadState.Executed; + payload.executedAt = uint40(block.timestamp); + + for (uint256 i = 0; i < payload.actions.length; i++) { + ExecutionAction storage action = payload.actions[i]; + IExecutor executor = IExecutor( + _accessLevelToExecutorConfig[action.accessLevel].executor + ); + + executor.executeTransaction{value: action.value}( + action.target, + action.value, + action.signature, + action.callData, + action.withDelegateCall + ); + } + + emit PayloadExecuted(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function cancelPayload(uint40 payloadId) external onlyGuardian { + Payload storage payload = _payloads[payloadId]; + + PayloadState payloadState = _getPayloadState(payload); + require( + payloadState < PayloadState.Executed && + payloadState >= PayloadState.Created, + Errors.PAYLOAD_NOT_IN_THE_CORRECT_STATE + ); + payload.state = PayloadState.Cancelled; + payload.cancelledAt = uint40(block.timestamp); + + emit PayloadCancelled(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function updateExecutors( + UpdateExecutorInput[] calldata executors + ) external onlyOwner { + _updateExecutors(executors); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadById( + uint40 payloadId + ) external view returns (Payload memory) { + Payload memory payload = _payloads[payloadId]; + payload.state = _getPayloadState(payload); + return payload; + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadState( + uint40 payloadId + ) external view returns (PayloadState) { + return _getPayloadState(_payloads[payloadId]); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadsCount() external view returns (uint40) { + return _payloadsCount; + } + + /// @inheritdoc IPayloadsControllerCore + function getExecutorSettingsByAccessControl( + PayloadsControllerUtils.AccessControl accessControl + ) external view returns (ExecutorConfig memory) { + return _accessLevelToExecutorConfig[accessControl]; + } + + /// @inheritdoc IRescuable + function whoCanRescue() + public + view + override(IRescuable, Rescuable) + returns (address) + { + return owner(); + } + + receive() external payable {} + + /** + * @notice method to get the current state of a payload + * @param payload object with all pertinent payload information + * @return current state of the payload + */ + function _getPayloadState( + Payload memory payload + ) internal view returns (PayloadState) { + PayloadState state = payload.state; + if (state == PayloadState.None || state >= PayloadState.Executed) { + return state; + } + + if ( + (state == PayloadState.Created && + block.timestamp >= payload.expirationTime) || + (state == PayloadState.Queued && + block.timestamp >= + payload.queuedAt + payload.delay + payload.gracePeriod) + ) { + //return PayloadState.Expired; // ORIG + return PayloadState.None; // MUTANT + } + + return state; + } + + /** + * @notice method to queue a payload + * @param payloadId id of the payload that needs to be queued + * @param accessLevel access level used for the proposal voting + * @param proposalVoteActivationTimestamp proposal vote activation timestamp in seconds + * @dev this method will be called when a payload is bridged from governance chain + */ + function _queuePayload( + uint40 payloadId, + PayloadsControllerUtils.AccessControl accessLevel, + uint40 proposalVoteActivationTimestamp + ) internal { + Payload storage payload = _payloads[payloadId]; + require( + _getPayloadState(payload) == PayloadState.Created, + Errors.PAYLOAD_NOT_IN_CREATED_STATE + ); + + // by allowing >= it enables the proposal to use a higher level of voting configuration + // than the one set by the payload actions + require( + accessLevel >= payload.maximumAccessLevelRequired, + Errors.INVALID_PROPOSAL_ACCESS_LEVEL + ); + // this checks that the payload has been created before the proposal vote started. + // this ensures that the voters where able to check the content of the payload they voted on + require( + proposalVoteActivationTimestamp > payload.createdAt, + Errors.PAYLOAD_NOT_CREATED_BEFORE_PROPOSAL + ); + + payload.state = PayloadState.Queued; + payload.queuedAt = uint40(block.timestamp); + + emit PayloadQueued(payloadId); + } + + /** + * @notice add new executor configs + * @param executors array of UpdateExecutorInput with needed executor configurations + */ + function _updateExecutors(UpdateExecutorInput[] memory executors) internal { + for (uint256 i = 0; i < executors.length; i++) { + UpdateExecutorInput memory newExecutorConfig = executors[i]; + + require( + newExecutorConfig.executorConfig.executor != address(0), + Errors.INVALID_EXECUTOR_ADDRESS + ); + + require( + newExecutorConfig.accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_EXECUTOR_ACCESS_LEVEL + ); + + require( + newExecutorConfig.executorConfig.delay >= MIN_EXECUTION_DELAY() && + newExecutorConfig.executorConfig.delay <= MAX_EXECUTION_DELAY(), + Errors.INVALID_EXECUTOR_DELAY + ); + + // check that the new executor is not already being used in a different level + PayloadsControllerUtils.AccessControl levelToCheck = newExecutorConfig + .accessLevel == PayloadsControllerUtils.AccessControl.Level_1 + ? PayloadsControllerUtils.AccessControl.Level_2 + : PayloadsControllerUtils.AccessControl.Level_1; + require( + _accessLevelToExecutorConfig[levelToCheck].executor != + newExecutorConfig.executorConfig.executor, + Errors.EXECUTOR_ALREADY_SET_IN_DIFFERENT_LEVEL + ); + + _accessLevelToExecutorConfig[ + newExecutorConfig.accessLevel + ] = newExecutorConfig.executorConfig; + + emit ExecutorSet( + newExecutorConfig.accessLevel, + newExecutorConfig.executorConfig.executor, + newExecutorConfig.executorConfig.delay + ); + } + } +} diff --git a/security/certora/tests/payloads/PayloadsControllerCore-5.sol b/security/certora/tests/payloads/PayloadsControllerCore-5.sol new file mode 100644 index 0000000..e56c451 --- /dev/null +++ b/security/certora/tests/payloads/PayloadsControllerCore-5.sol @@ -0,0 +1,333 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.8; + +import {OwnableWithGuardian} from 'solidity-utils/contracts/access-control/OwnableWithGuardian.sol'; +import {Rescuable, IRescuable} from 'solidity-utils/contracts/utils/Rescuable.sol'; +import {IERC20} from 'solidity-utils/contracts/oz-common/interfaces/IERC20.sol'; +import {Initializable} from 'solidity-utils/contracts/transparent-proxy/Initializable.sol'; +import {SafeERC20} from 'solidity-utils/contracts/oz-common/SafeERC20.sol'; +import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; + +import {IPayloadsControllerCore, PayloadsControllerUtils} from './interfaces/IPayloadsControllerCore.sol'; +import {IExecutor} from './interfaces/IExecutor.sol'; +import {Errors} from '../libraries/Errors.sol'; + +/** + * @title PayloadsControllerCore + * @author BGD Labs + * @notice this contract contains the logic to create and execute a payload. + * @dev To execute a created payload, the payload id must be bridged from governance chain. + * @dev The methods to update the contract configuration are callable only by owner. Owner being the registered + lvl1 Executor. So to update the PayloadsController configuration, a proposal will need to pass in gov chain and + be executed by the appropriate executor. + */ +abstract contract PayloadsControllerCore is + OwnableWithGuardian, + Rescuable, + IPayloadsControllerCore, + Initializable +{ + using SafeCast for uint256; + using SafeERC20 for IERC20; + + // should be always set with respect to the proposal flow duration + // for example: voting takes 5 days + 2 days for bridging + 3 days for cooldown + 2 days of safety gap + // then expirationDelay should be not less then 12 days. + // As expiration delay of proposal is 30 days, payload needs to be able to live longer as its created before and + // will be executed after. + // so nobody should be able to set expiration delay less then that + uint40 public constant EXPIRATION_DELAY = 35 days; + + /// @inheritdoc IPayloadsControllerCore + uint40 public constant GRACE_PERIOD = 7 days; + + uint40 internal _payloadsCount; + + // stores the executor configuration for every lvl of access control + mapping(PayloadsControllerUtils.AccessControl => ExecutorConfig) + internal _accessLevelToExecutorConfig; + + mapping(uint40 => Payload) internal _payloads; + + /// @inheritdoc IPayloadsControllerCore + function MIN_EXECUTION_DELAY() public view virtual returns (uint40) { + return 1 days; + } + + /// @inheritdoc IPayloadsControllerCore + function MAX_EXECUTION_DELAY() public view virtual returns (uint40) { + return 10 days; + } + + function initialize( + address owner, + address guardian, + UpdateExecutorInput[] calldata executors + ) external initializer { + require(executors.length != 0, Errors.SHOULD_BE_AT_LEAST_ONE_EXECUTOR); + + _updateExecutors(executors); + + _updateGuardian(guardian); + _transferOwnership(owner); + } + + /// @inheritdoc IPayloadsControllerCore + function createPayload( + ExecutionAction[] calldata actions + ) external returns (uint40) { + require(actions.length != 0, Errors.INVALID_EMPTY_TARGETS); + + uint40 payloadId = _payloadsCount++; + uint40 creationTime = uint40(block.timestamp); + Payload storage newPayload = _payloads[payloadId]; + newPayload.creator = msg.sender; + newPayload.state = PayloadState.Created; + newPayload.createdAt = creationTime; + newPayload.expirationTime = creationTime + EXPIRATION_DELAY; + newPayload.gracePeriod = GRACE_PERIOD; + + PayloadsControllerUtils.AccessControl maximumAccessLevelRequired; + for (uint256 i = 0; i < actions.length; i++) { + require(actions[i].target != address(0), Errors.INVALID_ACTION_TARGET); + require( + actions[i].accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_ACTION_ACCESS_LEVEL + ); + require( + _accessLevelToExecutorConfig[actions[i].accessLevel].executor != + address(0), + Errors.EXECUTOR_WAS_NOT_SPECIFIED_FOR_REQUESTED_ACCESS_LEVEL + ); + + newPayload.actions.push(actions[i]); + + if (actions[i].accessLevel > maximumAccessLevelRequired) { + maximumAccessLevelRequired = actions[i].accessLevel; + } + } + newPayload.maximumAccessLevelRequired = maximumAccessLevelRequired; + ExecutorConfig + memory maxRequiredExecutorConfig = _accessLevelToExecutorConfig[ + maximumAccessLevelRequired + ]; + newPayload.delay = maxRequiredExecutorConfig.delay; + + emit PayloadCreated( + payloadId, + msg.sender, + actions, + maximumAccessLevelRequired + ); + return payloadId; + } + + /// @inheritdoc IPayloadsControllerCore + function executePayload(uint40 payloadId) external payable { + Payload storage payload = _payloads[payloadId]; + + require( + _getPayloadState(payload) == PayloadState.Queued, + Errors.PAYLOAD_NOT_IN_QUEUED_STATE + ); + + uint256 executionTime = payload.queuedAt + payload.delay; + require(block.timestamp > executionTime, Errors.TIMELOCK_NOT_FINISHED); + + payload.state = PayloadState.Executed; + payload.executedAt = uint40(block.timestamp); + + for (uint256 i = 0; i < payload.actions.length; i++) { + ExecutionAction storage action = payload.actions[i]; + IExecutor executor = IExecutor( + _accessLevelToExecutorConfig[action.accessLevel].executor + ); + + executor.executeTransaction{value: action.value}( + action.target, + action.value, + action.signature, + action.callData, + action.withDelegateCall + ); + } + + emit PayloadExecuted(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function cancelPayload(uint40 payloadId) external onlyGuardian { + Payload storage payload = _payloads[payloadId]; + + PayloadState payloadState = _getPayloadState(payload); + require( + payloadState < PayloadState.Executed && + payloadState >= PayloadState.Created, + Errors.PAYLOAD_NOT_IN_THE_CORRECT_STATE + ); + payload.state = PayloadState.Cancelled; + payload.cancelledAt = uint40(block.timestamp); + + emit PayloadCancelled(payloadId); + } + + /// @inheritdoc IPayloadsControllerCore + function updateExecutors( + UpdateExecutorInput[] calldata executors + ) external onlyOwner { + _updateExecutors(executors); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadById( + uint40 payloadId + ) external view returns (Payload memory) { + Payload memory payload = _payloads[payloadId]; + payload.state = _getPayloadState(payload); + return payload; + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadState( + uint40 payloadId + ) external view returns (PayloadState) { + return _getPayloadState(_payloads[payloadId]); + } + + /// @inheritdoc IPayloadsControllerCore + function getPayloadsCount() external view returns (uint40) { + return _payloadsCount; + } + + /// @inheritdoc IPayloadsControllerCore + function getExecutorSettingsByAccessControl( + PayloadsControllerUtils.AccessControl accessControl + ) external view returns (ExecutorConfig memory) { + return _accessLevelToExecutorConfig[accessControl]; + } + + /// @inheritdoc IRescuable + function whoCanRescue() + public + view + override(IRescuable, Rescuable) + returns (address) + { + return owner(); + } + + receive() external payable {} + + /** + * @notice method to get the current state of a payload + * @param payload object with all pertinent payload information + * @return current state of the payload + */ + function _getPayloadState( + Payload memory payload + ) internal view returns (PayloadState) { + PayloadState state = payload.state; + if (state == PayloadState.None || state >= PayloadState.Executed) { + return state; + } + + if ( + (state == PayloadState.Created && + block.timestamp >= payload.expirationTime) || + (state == PayloadState.Queued && + block.timestamp >= + payload.queuedAt + payload.delay + payload.gracePeriod) + ) { + return PayloadState.Expired; + } + + return state; + } + + /** + * @notice method to queue a payload + * @param payloadId id of the payload that needs to be queued + * @param accessLevel access level used for the proposal voting + * @param proposalVoteActivationTimestamp proposal vote activation timestamp in seconds + * @dev this method will be called when a payload is bridged from governance chain + */ + function _queuePayload( + uint40 payloadId, + PayloadsControllerUtils.AccessControl accessLevel, + uint40 proposalVoteActivationTimestamp + ) internal { + Payload storage payload = _payloads[payloadId]; + require( + _getPayloadState(payload) == PayloadState.Created, + Errors.PAYLOAD_NOT_IN_CREATED_STATE + ); + + // by allowing >= it enables the proposal to use a higher level of voting configuration + // than the one set by the payload actions + require( + accessLevel >= payload.maximumAccessLevelRequired, + Errors.INVALID_PROPOSAL_ACCESS_LEVEL + ); + // this checks that the payload has been created before the proposal vote started. + // this ensures that the voters where able to check the content of the payload they voted on + require( + proposalVoteActivationTimestamp > payload.createdAt, + Errors.PAYLOAD_NOT_CREATED_BEFORE_PROPOSAL + ); + + payload.state = PayloadState.Queued; + payload.queuedAt = uint40(block.timestamp); + + emit PayloadQueued(payloadId); + } + + /** + * @notice add new executor configs + * @param executors array of UpdateExecutorInput with needed executor configurations + */ + function _updateExecutors(UpdateExecutorInput[] memory executors) internal { + //for (uint256 i = 0; i < executors.length; i++) { // ORIG + for (uint256 i = 0; i < 1; i++) { // MUTANT + UpdateExecutorInput memory newExecutorConfig = executors[i]; + + require( + newExecutorConfig.executorConfig.executor != address(0), + Errors.INVALID_EXECUTOR_ADDRESS + ); + + require( + newExecutorConfig.accessLevel > + PayloadsControllerUtils.AccessControl.Level_null, + Errors.INVALID_EXECUTOR_ACCESS_LEVEL + ); + + require( + newExecutorConfig.executorConfig.delay >= MIN_EXECUTION_DELAY() && + newExecutorConfig.executorConfig.delay <= MAX_EXECUTION_DELAY(), + Errors.INVALID_EXECUTOR_DELAY + ); + + // check that the new executor is not already being used in a different level + PayloadsControllerUtils.AccessControl levelToCheck = newExecutorConfig + .accessLevel == PayloadsControllerUtils.AccessControl.Level_1 + ? PayloadsControllerUtils.AccessControl.Level_2 + : PayloadsControllerUtils.AccessControl.Level_1; + require( + _accessLevelToExecutorConfig[levelToCheck].executor != + newExecutorConfig.executorConfig.executor, + Errors.EXECUTOR_ALREADY_SET_IN_DIFFERENT_LEVEL + ); + + _accessLevelToExecutorConfig[ + newExecutorConfig.accessLevel + ] = newExecutorConfig.executorConfig; + + emit ExecutorSet( + newExecutorConfig.accessLevel, + newExecutorConfig.executorConfig.executor, + newExecutorConfig.executorConfig.delay + ); + } + } +} diff --git a/security/certora/tests/payloads/REPORT.txt b/security/certora/tests/payloads/REPORT.txt new file mode 100644 index 0000000..227b414 --- /dev/null +++ b/security/certora/tests/payloads/REPORT.txt @@ -0,0 +1,50 @@ +verifyPayloadsController.conf +============================= + +Mutations: +--------- +1. DETECTED +Changed file: PayloadsControllerCore.sol ==> PayloadsControllerCore-1.sol +The change: PayloadsControllerCore:81: + orig: + uint40 payloadId = _payloadsCount++; + mutant: + uint40 payloadId = _payloadsCount; + + + +2. TIME-OUT (most jobs) +Changed file: PayloadsControllerCore.sol ==> PayloadsControllerCore-2.sol +The change: PayloadsControllerCore:142: + orig: + ExecutionAction storage action = payload.actions[i]; + mutant: + ExecutionAction storage action = payload.actions[0]; + + +3. UNDETECTED +Changed file: PayloadsControllerCore.sol ==> PayloadsControllerCore-3.sol +The change: PayloadsControllerCore:169: + orig: + payload.state = PayloadState.Cancelled; + mutant: + payload.state = PayloadState.Queued; + + +4. UNDETECTED +Changed file: PayloadsControllerCore.sol ==> PayloadsControllerCore-4.sol +The change: PayloadsControllerCore:242: + orig: + return PayloadState.Expired; + mutant: + return PayloadState.None; + + +5. UNDETECTED +Changed file: PayloadsControllerCore.sol ==> PayloadsControllerCore-5.sol +The change: PayloadsControllerCore:290: + orig: + for (uint256 i = 0; i < executors.length; i++) { + mutant: + for (uint256 i = 0; i < 1; i++) { + From 275692638a9f925461f6dba35b82b5d678ed6bff Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Tue, 14 Nov 2023 18:46:46 +0200 Subject: [PATCH 03/14] end of 14.11 --- security/certora/scripts/voting-chain.sh | 89 +++++++++++++++++++ .../certora/tests/REPORT-power-strategy.txt | 2 +- security/certora/tests/REPORT-voting.txt | 3 - 3 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 security/certora/scripts/voting-chain.sh diff --git a/security/certora/scripts/voting-chain.sh b/security/certora/scripts/voting-chain.sh new file mode 100644 index 0000000..0b065a8 --- /dev/null +++ b/security/certora/scripts/voting-chain.sh @@ -0,0 +1,89 @@ + + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyLegality.conf \ + --rule createdVoteHasNonZeroHash \ + --msg "1: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyLegality.conf \ + --rule onlyValidProposalCanChangeTally \ + --msg "2: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyLegality.conf \ + --rule legalVote \ + --msg "3: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyLegality.conf \ + --rule votedPowerIsImmutable method_reachability \ + --msg "4: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyMisc.conf \ + --msg "5: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyPower_summary.conf \ + --msg "6: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_config.conf \ + --rule createdProposalHasRoots \ + --msg "7: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_config.conf \ + --rule startedProposalHasConfig \ + --msg "8: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_config.conf \ + --rule proposalHasNonzeroDuration configIsImmutable newProposalUnusedId method_reachability \ + --msg "9: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_states.conf \ + --rule proposalImmutability \ + --msg "10: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_states.conf \ + --rule startsStrictlyBeforeEnds \ + --msg "11: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_states.conf \ + --rule startsBeforeEnds \ + --msg "12: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_states.conf \ + --rule startedProposalHasConfig \ + --msg "13: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_states.conf \ + --rule proposalMethodStateTransitionCompliance \ + --msg "14: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyProposal_states.conf \ + --rule proposalIdIsImmutable proposalHasNonzeroDuration proposalTimeStateTransitionCompliance proposalLegalStates method_reachability \ + --msg "15: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyVoting_and_tally.conf \ + --rule voteUpdatesTally \ + --msg "16: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyVoting_and_tally.conf \ + --rule cannot_vote_twice_with_submitVoteSingleProofAsRepresentative_and_submitVote \ + --msg "17: " + +certoraRun --fe_version latest --send_only --disable_auto_cache_key_gen \ + security/certora/confs/voting/verifyVoting_and_tally.conf \ + --rule voteTallyChangedOnlyByVoting onlyVoteCanChangeResult votingTallyCanOnlyIncrease strangerVoteUnchanged otherProposalUnchanged otherVoterUntouched cannot_vote_twice_with_submitVote_and_submitVoteAsRepresentative cannot_vote_twice_with_submitVoteAsRepresentative_and_submitVote method_reachability \ + --msg "18: " diff --git a/security/certora/tests/REPORT-power-strategy.txt b/security/certora/tests/REPORT-power-strategy.txt index 1846402..8c87454 100644 --- a/security/certora/tests/REPORT-power-strategy.txt +++ b/security/certora/tests/REPORT-power-strategy.txt @@ -10,7 +10,7 @@ Questions: Mutations: --------- -1. UNDETECTED +1. UNDETECTED (one rule got Time-Out) Changed file: BaseVotingStrategy.sol ==> BaseVotingStrategy-1.sol The change: BaseVotingStrategy.sol:76: orig: diff --git a/security/certora/tests/REPORT-voting.txt b/security/certora/tests/REPORT-voting.txt index bdab6da..22e0ee6 100644 --- a/security/certora/tests/REPORT-voting.txt +++ b/security/certora/tests/REPORT-voting.txt @@ -46,8 +46,6 @@ The change: VotingMachineWithProofs.sol::222 mutant: return _proposals[proposalId+1].votes[user]; -Suggestion for rules that can catch it: - 4. DETECTED Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-4.sol @@ -67,5 +65,4 @@ The change: VotingMachineWithProofs.sol::288 mutant: proposalListLength - skip - i -Suggestion for rules that can catch it: From cc76f8810f0d8a51e596d4f2a694c3d076fec9d7 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 10:15:24 +0200 Subject: [PATCH 04/14] update report --- security/certora/tests/REPORT-power-strategy.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/security/certora/tests/REPORT-power-strategy.txt b/security/certora/tests/REPORT-power-strategy.txt index 8c87454..554d29d 100644 --- a/security/certora/tests/REPORT-power-strategy.txt +++ b/security/certora/tests/REPORT-power-strategy.txt @@ -35,7 +35,8 @@ Suggestion for rule that can catch it: - We have the rule: invalidTokenRefused(...). We can add its analogue validTokenAccepted(...). -3. DETECTED +3. DETECTED VVV +By: certora-review-mainnet.yml::verifyGovernancePowerStrategy.conf Changed file: GovernancePowerStrategy.sol ==> GovernancePowerStrategy-3.sol The change: GovernancePowerStrategy.sol::25: orig: @@ -46,7 +47,7 @@ The change: GovernancePowerStrategy.sol::25: Found by: powerlessCompliance -4. UNDETECTED +4. UNDETECTED VVV Changed file: GovernancePowerStrategy.sol ==> GovernancePowerStrategy-4.sol The change: GovernancePowerStrategy.sol::53: orig: From 30fdfd911a0831315508289fbc6afbe6b56a41b4 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 10:17:39 +0200 Subject: [PATCH 05/14] voting chain mutant 1 --- src/contracts/BaseVotingStrategy.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contracts/BaseVotingStrategy.sol b/src/contracts/BaseVotingStrategy.sol index eb8dd60..e131ac9 100644 --- a/src/contracts/BaseVotingStrategy.sol +++ b/src/contracts/BaseVotingStrategy.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: BUSL-1.1 +// SPDX-License-Identifier: MIT pragma solidity ^0.8.0; import {IBaseVotingStrategy} from '../interfaces/IBaseVotingStrategy.sol'; @@ -73,7 +73,7 @@ abstract contract BaseVotingStrategy is IBaseVotingStrategy { function getVotingAssetList() public pure returns (address[] memory) { address[] memory votingAssets = new address[](3); - votingAssets[0] = AAVE(); + votingAssets[0] = A_AAVE(); //AAVE(); votingAssets[1] = STK_AAVE(); votingAssets[2] = A_AAVE(); From 52eda7f85cc9b820c158bf67d118a52e598db4ba Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 10:38:19 +0200 Subject: [PATCH 06/14] fixing --- src/contracts/BaseVotingStrategy.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contracts/BaseVotingStrategy.sol b/src/contracts/BaseVotingStrategy.sol index e131ac9..eb8dd60 100644 --- a/src/contracts/BaseVotingStrategy.sol +++ b/src/contracts/BaseVotingStrategy.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.0; import {IBaseVotingStrategy} from '../interfaces/IBaseVotingStrategy.sol'; @@ -73,7 +73,7 @@ abstract contract BaseVotingStrategy is IBaseVotingStrategy { function getVotingAssetList() public pure returns (address[] memory) { address[] memory votingAssets = new address[](3); - votingAssets[0] = A_AAVE(); //AAVE(); + votingAssets[0] = AAVE(); votingAssets[1] = STK_AAVE(); votingAssets[2] = A_AAVE(); From 442be5d0b98e29c48cfd40e92754ebd2af55d778 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 12:00:04 +0200 Subject: [PATCH 07/14] change yml trigger on PR --- .../certora-review-execution-chain.yml | 4 +- .github/workflows/certora-review-mainnet.yml | 4 +- .../workflows/certora-review-voting-chain.yml | 2 +- .../certora/tests/BaseVotingStrategy-6.sol | 116 ++++++++++++++++++ .../certora/tests/REPORT-power-strategy.txt | 3 +- 5 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 security/certora/tests/BaseVotingStrategy-6.sol diff --git a/.github/workflows/certora-review-execution-chain.yml b/.github/workflows/certora-review-execution-chain.yml index 1ec7db0..2f17733 100644 --- a/.github/workflows/certora-review-execution-chain.yml +++ b/.github/workflows/certora-review-execution-chain.yml @@ -6,7 +6,7 @@ on: - main pull_request: branches: - - main + - certora workflow_dispatch: @@ -64,4 +64,4 @@ jobs: - verifyPayloadsController.conf --rule action_signature_immutable - verifyPayloadsController.conf --rule action_immutable_check_only_fixed_size_fields - verifyPayloadsController.conf --rule zero_executedAt_if_not_executed - \ No newline at end of file + diff --git a/.github/workflows/certora-review-mainnet.yml b/.github/workflows/certora-review-mainnet.yml index 72031de..01fa64a 100644 --- a/.github/workflows/certora-review-mainnet.yml +++ b/.github/workflows/certora-review-mainnet.yml @@ -7,7 +7,7 @@ on: - main pull_request: branches: - - main + - certora workflow_dispatch: @@ -63,4 +63,4 @@ jobs: - verifyGovernance.conf --rule cannot_queue_when_voting_portal_unapproved only_owner_can_set_voting_config_witness only_owner_can_set_voting_config single_state_transition_per_block_non_creator_witness - verifyGovernance.conf --rule single_state_transition_per_block_non_creator_non_guardian state_cant_decrease no_state_transitions_beyond_3 immutable_voting_portal insufficient_proposition_power_time_elapsed_tight_witness - verifyGovernance.conf --rule insufficient_proposition_power_allow_time_elapse insufficient_proposition_power proposal_after_voting_portal_invalidate - \ No newline at end of file + diff --git a/.github/workflows/certora-review-voting-chain.yml b/.github/workflows/certora-review-voting-chain.yml index ba6caa5..058a063 100644 --- a/.github/workflows/certora-review-voting-chain.yml +++ b/.github/workflows/certora-review-voting-chain.yml @@ -7,7 +7,7 @@ on: - main pull_request: branches: - - main + - certora workflow_dispatch: diff --git a/security/certora/tests/BaseVotingStrategy-6.sol b/security/certora/tests/BaseVotingStrategy-6.sol new file mode 100644 index 0000000..b55e4bc --- /dev/null +++ b/security/certora/tests/BaseVotingStrategy-6.sol @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +import {IBaseVotingStrategy} from '../interfaces/IBaseVotingStrategy.sol'; +import {Errors} from './libraries/Errors.sol'; + +//import {AaveV3EthereumAssets} from 'aave-address-book/AaveV3Ethereum.sol'; + +/** + * @title BaseVotingStrategy + * @author BGD Labs + * @notice This contract contains the base logic of a voting strategy, being on governance chain or voting machine chain. + */ +abstract contract BaseVotingStrategy is IBaseVotingStrategy { + function AAVE() public pure virtual returns (address) { + return 0x7Fc66500c84A76Ad7e9c93437bFc5Ac33E2DDaE9; + } + + function STK_AAVE() public pure virtual returns (address) { + return 0x4da27a545c0c5B758a6BA100e3a049001de870f5; + } + + function A_AAVE() public pure virtual returns (address) { + return 0xA700b4eB416Be35b2911fd5Dee80678ff64fF6C9; + } + + uint128 public constant BASE_BALANCE_SLOT = 0; + uint128 public constant A_AAVE_BASE_BALANCE_SLOT = 52; + uint128 public constant A_AAVE_DELEGATED_STATE_SLOT = 64; + + /// @dev on the constructor we get all the voting assets and emit the different asset configurations + constructor() { + address[] memory votingAssetList = getVotingAssetList(); + + // Check that voting strategy at least has one asset + require(votingAssetList.length != 0, Errors.NO_VOTING_ASSETS); + + for (uint256 i = 0; i < votingAssetList.length; i++) { + for (uint256 j = i + 1; j < votingAssetList.length; j++) { + require( + votingAssetList[i] != votingAssetList[j], + Errors.REPEATED_STRATEGY_ASSET + ); + } + VotingAssetConfig memory votingAssetConfig = getVotingAssetConfig( + votingAssetList[i] + ); + + require( + votingAssetConfig.storageSlots.length > 0, + Errors.EMPTY_ASSET_STORAGE_SLOTS + ); + + for (uint256 k = 0; k < votingAssetConfig.storageSlots.length; k++) { + for ( + uint256 l = k + 1; + l < votingAssetConfig.storageSlots.length; + l++ + ) { + require( + votingAssetConfig.storageSlots[k] != + votingAssetConfig.storageSlots[l], + Errors.REPEATED_STRATEGY_ASSET_SLOT + ); + } + } + + emit VotingAssetAdd(votingAssetList[i], votingAssetConfig.storageSlots); + } + } + + /// @inheritdoc IBaseVotingStrategy + function getVotingAssetList() public pure returns (address[] memory) { + address[] memory votingAssets = new address[](3); + + votingAssets[0] = AAVE(); + votingAssets[1] = STK_AAVE(); + votingAssets[2] = A_AAVE(); + + return votingAssets; + } + + /// @inheritdoc IBaseVotingStrategy + function getVotingAssetConfig( + address asset + ) public pure returns (VotingAssetConfig memory) { + VotingAssetConfig memory votingAssetConfig; + + if (asset == AAVE() || asset == STK_AAVE()) { + votingAssetConfig.storageSlots = new uint128[](1); + votingAssetConfig.storageSlots[0] = BASE_BALANCE_SLOT; + } else if (asset == A_AAVE()) { + votingAssetConfig.storageSlots = new uint128[](2); + votingAssetConfig.storageSlots[0] = BASE_BALANCE_SLOT; + votingAssetConfig.storageSlots[1] = A_AAVE_DELEGATED_STATE_SLOT; + } else { + return votingAssetConfig; + } + + return votingAssetConfig; + } + + /// @inheritdoc IBaseVotingStrategy + function isTokenSlotAccepted( + address token, + uint128 slot + ) external pure returns (bool) { + VotingAssetConfig memory votingAssetConfig = getVotingAssetConfig(token); + for (uint256 i = 0; i < votingAssetConfig.storageSlots.length; i++) { + if (slot == votingAssetConfig.storageSlots[i]) { + return true; + } + } + return false; + } +} diff --git a/security/certora/tests/REPORT-power-strategy.txt b/security/certora/tests/REPORT-power-strategy.txt index 554d29d..a3fbac5 100644 --- a/security/certora/tests/REPORT-power-strategy.txt +++ b/security/certora/tests/REPORT-power-strategy.txt @@ -23,7 +23,8 @@ Suggestion for rules that can catch it: - For every voting token T, and every user U that does not delegate, if the balance of U in T is increased then the power of U increased. -2. UNDETECTED +2. DETECTED VVV +By certora-review-mainnet.yml::verifyVotingStrategy_unittests.conf Changed file: BaseVotingStrategy.sol ==> BaseVotingStrategy-2.sol The change: BaseVotingStrategy.sol:109: orig: From 4e51ee402ec5a2faf4cd6392dbb00480d8d66681 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 12:01:46 +0200 Subject: [PATCH 08/14] voting chain mutant 6 --- src/contracts/BaseVotingStrategy.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contracts/BaseVotingStrategy.sol b/src/contracts/BaseVotingStrategy.sol index eb8dd60..b55e4bc 100644 --- a/src/contracts/BaseVotingStrategy.sol +++ b/src/contracts/BaseVotingStrategy.sol @@ -91,7 +91,7 @@ abstract contract BaseVotingStrategy is IBaseVotingStrategy { votingAssetConfig.storageSlots[0] = BASE_BALANCE_SLOT; } else if (asset == A_AAVE()) { votingAssetConfig.storageSlots = new uint128[](2); - votingAssetConfig.storageSlots[0] = A_AAVE_BASE_BALANCE_SLOT; + votingAssetConfig.storageSlots[0] = BASE_BALANCE_SLOT; votingAssetConfig.storageSlots[1] = A_AAVE_DELEGATED_STATE_SLOT; } else { return votingAssetConfig; From 56dd86152d58eec569bf29782ed5824383c2be80 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 12:16:07 +0200 Subject: [PATCH 09/14] move files --- security/certora/tests/REPORT-voting.txt | 10 +++++----- ...WithProofs-4.sol => VotingMachineWithProofs-10.sol} | 0 ...WithProofs-5.sol => VotingMachineWithProofs-11.sol} | 0 ...eWithProofs-1.sol => VotingMachineWithProofs-7.sol} | 0 ...eWithProofs-2.sol => VotingMachineWithProofs-8.sol} | 0 ...eWithProofs-3.sol => VotingMachineWithProofs-9.sol} | 0 6 files changed, 5 insertions(+), 5 deletions(-) rename security/certora/tests/{VotingMachineWithProofs-4.sol => VotingMachineWithProofs-10.sol} (100%) rename security/certora/tests/{VotingMachineWithProofs-5.sol => VotingMachineWithProofs-11.sol} (100%) rename security/certora/tests/{VotingMachineWithProofs-1.sol => VotingMachineWithProofs-7.sol} (100%) rename security/certora/tests/{VotingMachineWithProofs-2.sol => VotingMachineWithProofs-8.sol} (100%) rename security/certora/tests/{VotingMachineWithProofs-3.sol => VotingMachineWithProofs-9.sol} (100%) diff --git a/security/certora/tests/REPORT-voting.txt b/security/certora/tests/REPORT-voting.txt index 22e0ee6..6617150 100644 --- a/security/certora/tests/REPORT-voting.txt +++ b/security/certora/tests/REPORT-voting.txt @@ -18,7 +18,7 @@ Questions/Comments: Mutations: --------- 1. UNDETECTED -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-1.sol +Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-7.sol The change: VotingMachineWithProofs.sol:88: orig: return _bridgedVotes[voter][proposalId]; @@ -29,7 +29,7 @@ Suggestion for rules that can catch it: 2. DETECTED (by sanity rule) -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-2.sol +Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-8.sol The change: VotingMachineWithProofs.sol:197: orig: assetFound = true; @@ -39,7 +39,7 @@ The change: VotingMachineWithProofs.sol:197: Suggestion for rules that can catch it: 3. DETECTED -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-3.sol +Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-9.sol The change: VotingMachineWithProofs.sol::222 orig: return _proposals[proposalId].votes[user]; @@ -48,7 +48,7 @@ The change: VotingMachineWithProofs.sol::222 4. DETECTED -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-4.sol +Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-10.sol The change: VotingMachineWithProofs.sol::250 orig: Proposal storage proposal = _proposals[proposalId]; @@ -58,7 +58,7 @@ The change: VotingMachineWithProofs.sol::250 Suggestion for rules that can catch it: 5. UNDETECTED -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-5.sol +Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-11.sol The change: VotingMachineWithProofs.sol::288 orig: proposalListLength - skip - i - 1 diff --git a/security/certora/tests/VotingMachineWithProofs-4.sol b/security/certora/tests/VotingMachineWithProofs-10.sol similarity index 100% rename from security/certora/tests/VotingMachineWithProofs-4.sol rename to security/certora/tests/VotingMachineWithProofs-10.sol diff --git a/security/certora/tests/VotingMachineWithProofs-5.sol b/security/certora/tests/VotingMachineWithProofs-11.sol similarity index 100% rename from security/certora/tests/VotingMachineWithProofs-5.sol rename to security/certora/tests/VotingMachineWithProofs-11.sol diff --git a/security/certora/tests/VotingMachineWithProofs-1.sol b/security/certora/tests/VotingMachineWithProofs-7.sol similarity index 100% rename from security/certora/tests/VotingMachineWithProofs-1.sol rename to security/certora/tests/VotingMachineWithProofs-7.sol diff --git a/security/certora/tests/VotingMachineWithProofs-2.sol b/security/certora/tests/VotingMachineWithProofs-8.sol similarity index 100% rename from security/certora/tests/VotingMachineWithProofs-2.sol rename to security/certora/tests/VotingMachineWithProofs-8.sol diff --git a/security/certora/tests/VotingMachineWithProofs-3.sol b/security/certora/tests/VotingMachineWithProofs-9.sol similarity index 100% rename from security/certora/tests/VotingMachineWithProofs-3.sol rename to security/certora/tests/VotingMachineWithProofs-9.sol From 9ce72327f4d9ede0e6869f8368679e496865f8ad Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 12:38:06 +0200 Subject: [PATCH 10/14] fixing --- security/certora/tests/REPORT-voting.txt | 34 +- .../tests/VotingMachineWithProofs-10.sol | 500 ------------------ .../tests/VotingMachineWithProofs-11.sol | 500 ------------------ .../tests/VotingMachineWithProofs-7.sol | 237 +++++---- .../tests/VotingMachineWithProofs-8.sol | 500 ------------------ .../tests/VotingMachineWithProofs-9.sol | 500 ------------------ src/contracts/BaseVotingStrategy.sol | 2 +- 7 files changed, 154 insertions(+), 2119 deletions(-) delete mode 100644 security/certora/tests/VotingMachineWithProofs-10.sol delete mode 100644 security/certora/tests/VotingMachineWithProofs-11.sol delete mode 100644 security/certora/tests/VotingMachineWithProofs-8.sol delete mode 100644 security/certora/tests/VotingMachineWithProofs-9.sol diff --git a/security/certora/tests/REPORT-voting.txt b/security/certora/tests/REPORT-voting.txt index 6617150..dc19205 100644 --- a/security/certora/tests/REPORT-voting.txt +++ b/security/certora/tests/REPORT-voting.txt @@ -17,38 +17,18 @@ Questions/Comments: Mutations: --------- -1. UNDETECTED -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-7.sol -The change: VotingMachineWithProofs.sol:88: - orig: - return _bridgedVotes[voter][proposalId]; - mutant: - return _bridgedVotes[voter][proposalId+1]; - -Suggestion for rules that can catch it: - - -2. DETECTED (by sanity rule) -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-8.sol -The change: VotingMachineWithProofs.sol:197: - orig: - assetFound = true; - mutant: - assetFound = false; - -Suggestion for rules that can catch it: -3. DETECTED -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-9.sol -The change: VotingMachineWithProofs.sol::222 +7. DETECTED +Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-7.sol +The change: VotingMachineWithProofs.sol::319 orig: return _proposals[proposalId].votes[user]; mutant: return _proposals[proposalId+1].votes[user]; -4. DETECTED -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-10.sol +8. DETECTED +Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-8.sol The change: VotingMachineWithProofs.sol::250 orig: Proposal storage proposal = _proposals[proposalId]; @@ -57,8 +37,8 @@ The change: VotingMachineWithProofs.sol::250 Suggestion for rules that can catch it: -5. UNDETECTED -Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-11.sol +9. UNDETECTED +Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-9.sol The change: VotingMachineWithProofs.sol::288 orig: proposalListLength - skip - i - 1 diff --git a/security/certora/tests/VotingMachineWithProofs-10.sol b/security/certora/tests/VotingMachineWithProofs-10.sol deleted file mode 100644 index 355fa56..0000000 --- a/security/certora/tests/VotingMachineWithProofs-10.sol +++ /dev/null @@ -1,500 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -import {Ownable} from 'solidity-utils/contracts/oz-common/Ownable.sol'; -import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; -import {StateProofVerifier} from './libs/StateProofVerifier.sol'; -import {IVotingMachineWithProofs, IDataWarehouse, IVotingStrategy} from './interfaces/IVotingMachineWithProofs.sol'; -import {IBaseVotingStrategy} from '../../interfaces/IBaseVotingStrategy.sol'; -import {Errors} from '../libraries/Errors.sol'; -import {SlotUtils} from '../libraries/SlotUtils.sol'; -import {EIP712} from 'openzeppelin-contracts/contracts/utils/cryptography/EIP712.sol'; -import {ECDSA} from 'openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol'; - -/** - * @title VotingMachineWithProofs - * @author BGD Labs - * @notice this contract contains the logic to vote on a bridged proposal. It uses registered proofs to calculate the - voting power of the users. Once the voting is finished it will send the results back to the governance chain. - * @dev Abstract contract that is implemented on VotingMachine contract - */ -abstract contract VotingMachineWithProofs is - IVotingMachineWithProofs, - EIP712, - Ownable -{ - using SafeCast for uint256; - - string public constant VOTING_ASSET_WITH_SLOT_RAW = - 'VotingAssetWithSlot(address underlyingAsset,uint128 slot)'; - - /// @inheritdoc IVotingMachineWithProofs - bytes32 public constant VOTE_SUBMITTED_TYPEHASH = - keccak256( - abi.encodePacked( - 'SubmitVote(uint256 proposalId,address voter,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', - VOTING_ASSET_WITH_SLOT_RAW - ) - ); - - /// @inheritdoc IVotingMachineWithProofs - bytes32 public constant VOTING_ASSET_WITH_SLOT_TYPEHASH = - keccak256(abi.encodePacked(VOTING_ASSET_WITH_SLOT_RAW)); - - /// @inheritdoc IVotingMachineWithProofs - string public constant NAME = 'Aave Voting Machine'; - - /// @inheritdoc IVotingMachineWithProofs - IVotingStrategy public immutable VOTING_STRATEGY; - - /// @inheritdoc IVotingMachineWithProofs - IDataWarehouse public immutable DATA_WAREHOUSE; - - // (proposalId => proposal information) stores the information of the proposals - mapping(uint256 => Proposal) internal _proposals; - - // (proposalId => proposal vote configuration) stores the configuration for voting on each proposal - mapping(uint256 => ProposalVoteConfiguration) - internal _proposalsVoteConfiguration; - - // saves the ids of the proposals that have been bridged for a vote. - uint256[] internal _proposalsVoteConfigurationIds; - - // (voter => proposalId => voteInfo) stores the information for the bridged votes - mapping(address => mapping(uint256 => BridgedVote)) internal _bridgedVotes; - - /** - * @param votingStrategy address of the new VotingStrategy contract - */ - constructor(IVotingStrategy votingStrategy) Ownable() EIP712(NAME, 'V1') { - require( - address(votingStrategy) != address(0), - Errors.INVALID_VOTING_STRATEGY - ); - VOTING_STRATEGY = votingStrategy; - DATA_WAREHOUSE = votingStrategy.DATA_WAREHOUSE(); - } - - /// @inheritdoc IVotingMachineWithProofs - function DOMAIN_SEPARATOR() public view returns (bytes32) { - return _domainSeparatorV4(); - } - - /// @inheritdoc IVotingMachineWithProofs - function getBridgedVoteInfo( - uint256 proposalId, - address voter - ) external view returns (BridgedVote memory) { - return _bridgedVotes[voter][proposalId]; - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalVoteConfiguration( - uint256 proposalId - ) external view returns (ProposalVoteConfiguration memory) { - return _proposalsVoteConfiguration[proposalId]; - } - - /// @inheritdoc IVotingMachineWithProofs - function startProposalVote(uint256 proposalId) external returns (uint256) { - ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ - proposalId - ]; - require( - voteConfig.l1ProposalBlockHash != bytes32(0), - Errors.MISSING_PROPOSAL_BLOCK_HASH - ); - Proposal storage newProposal = _proposals[proposalId]; - - require( - _getProposalState(newProposal) == ProposalState.NotCreated, - Errors.PROPOSAL_VOTE_ALREADY_CREATED - ); - - VOTING_STRATEGY.hasRequiredRoots(voteConfig.l1ProposalBlockHash); - - uint40 startTime = _getCurrentTimeRef(); - uint40 endTime = startTime + voteConfig.votingDuration; - - newProposal.id = proposalId; - newProposal.creationBlockNumber = block.number; - newProposal.startTime = startTime; - newProposal.endTime = endTime; - - emit ProposalVoteStarted( - proposalId, - voteConfig.l1ProposalBlockHash, - startTime, - endTime - ); - - return proposalId; - } - - /// @inheritdoc IVotingMachineWithProofs - function submitVoteBySignature( - uint256 proposalId, - address voter, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs, - uint8 v, - bytes32 r, - bytes32 s - ) external { - bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( - votingBalanceProofs.length - ); - for (uint256 i = 0; i < votingBalanceProofs.length; i++) { - underlyingAssetsWithSlotHashes[i] = keccak256( - abi.encode( - VOTING_ASSET_WITH_SLOT_TYPEHASH, - votingBalanceProofs[i].underlyingAsset, - votingBalanceProofs[i].slot - ) - ); - } - - bytes32 digest = _hashTypedDataV4( - keccak256( - abi.encode( - VOTE_SUBMITTED_TYPEHASH, - proposalId, - voter, - support, - keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) - ) - ) - ); - address signer = ECDSA.recover(digest, v, r, s); - - require(signer == voter && signer != address(0), Errors.INVALID_SIGNATURE); - _submitVote(signer, proposalId, support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function settleVoteFromPortal( - uint256 proposalId, - address voter, - VotingBalanceProof[] calldata votingBalanceProofs - ) external { - BridgedVote memory bridgedVote = _bridgedVotes[voter][proposalId]; - - require( - bridgedVote.votingAssetsWithSlot.length == votingBalanceProofs.length, - Errors.INVALID_NUMBER_OF_PROOFS_FOR_VOTING_TOKENS - ); - - // check that the proofs are of the voter assets - for (uint256 i = 0; i < bridgedVote.votingAssetsWithSlot.length; i++) { - bool assetFound; - for (uint256 j = 0; j < votingBalanceProofs.length; j++) { - if ( - votingBalanceProofs[j].underlyingAsset == - bridgedVote.votingAssetsWithSlot[i].underlyingAsset && - votingBalanceProofs[j].slot == - bridgedVote.votingAssetsWithSlot[i].slot - ) { - assetFound = true; - break; - } - } - - require(assetFound, Errors.PROOFS_NOT_FOR_VOTING_TOKENS); - } - - _submitVote(voter, proposalId, bridgedVote.support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function submitVote( - uint256 proposalId, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs - ) external { - _submitVote(msg.sender, proposalId, support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function getUserProposalVote( - address user, - uint256 proposalId - ) external view returns (Vote memory) { - return _proposals[proposalId].votes[user]; - } - - /// @inheritdoc IVotingMachineWithProofs - function closeAndSendVote(uint256 proposalId) external { - Proposal storage proposal = _proposals[proposalId]; - require( - _getProposalState(proposal) == ProposalState.Finished, - Errors.PROPOSAL_VOTE_NOT_FINISHED - ); - - proposal.votingClosedAndSentBlockNumber = block.number; - proposal.votingClosedAndSentTimestamp = _getCurrentTimeRef(); - - uint256 forVotes = proposal.forVotes; - uint256 againstVotes = proposal.againstVotes; - - proposal.sentToGovernance = true; - - _sendVoteResults(proposalId, forVotes, againstVotes); - - emit ProposalResultsSent(proposalId, forVotes, againstVotes); - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalById( - uint256 proposalId - ) external view returns (ProposalWithoutVotes memory) { - Proposal storage proposal = _proposals[0]; - ProposalWithoutVotes memory proposalWithoutVotes = ProposalWithoutVotes({ - id: proposalId, - startTime: proposal.startTime, - endTime: proposal.endTime, - creationBlockNumber: proposal.creationBlockNumber, - forVotes: proposal.forVotes, - againstVotes: proposal.againstVotes, - votingClosedAndSentBlockNumber: proposal.votingClosedAndSentBlockNumber, - votingClosedAndSentTimestamp: proposal.votingClosedAndSentTimestamp, - sentToGovernance: proposal.sentToGovernance - }); - - return proposalWithoutVotes; - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalState( - uint256 proposalId - ) external view returns (ProposalState) { - return _getProposalState(_proposals[proposalId]); - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalsVoteConfigurationIds( - uint256 skip, - uint256 size - ) external view returns (uint256[] memory) { - uint256 proposalListLength = _proposalsVoteConfigurationIds.length; - if (proposalListLength == 0 || proposalListLength <= skip) { - return new uint256[](0); - } else if (proposalListLength < size + skip) { - size = proposalListLength - skip; - } - - uint256[] memory ids = new uint256[](size); - for (uint256 i = 0; i < size; i++) { - ids[i] = _proposalsVoteConfigurationIds[ - proposalListLength - skip - i - 1 - ]; - } - return ids; - } - - /** - * @notice method to cast a vote on a proposal specified by its id - * @param voter address with the voting power - * @param proposalId id of the proposal on which the vote will be cast - * @param support boolean indicating if the vote is in favor or against the proposal - * @param votingBalanceProofs list of objects containing the information necessary to vote using the tokens - allowed on the voting strategy. - * @dev A vote does not need to use all the tokens allowed, can be a subset - */ - function _submitVote( - address voter, - uint256 proposalId, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs - ) internal { - Proposal storage proposal = _proposals[proposalId]; - require( - _getProposalState(proposal) == ProposalState.Active, - Errors.PROPOSAL_VOTE_NOT_IN_ACTIVE_STATE - ); - - Vote storage vote = proposal.votes[voter]; - require(vote.votingPower == 0, Errors.PROPOSAL_VOTE_ALREADY_EXISTS); - - ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ - proposalId - ]; - - uint256 votingPower; - StateProofVerifier.SlotValue memory balanceVotingPower; - for (uint256 i = 0; i < votingBalanceProofs.length; i++) { - for (uint256 j = i + 1; j < votingBalanceProofs.length; j++) { - require( - votingBalanceProofs[i].slot != votingBalanceProofs[j].slot || - votingBalanceProofs[i].underlyingAsset != - votingBalanceProofs[j].underlyingAsset, - Errors.VOTE_ONCE_FOR_ASSET - ); - } - - balanceVotingPower = DATA_WAREHOUSE.getStorage( - votingBalanceProofs[i].underlyingAsset, - voteConfig.l1ProposalBlockHash, - SlotUtils.getAccountSlotHash(voter, votingBalanceProofs[i].slot), - votingBalanceProofs[i].proof - ); - - require(balanceVotingPower.exists, Errors.USER_BALANCE_DOES_NOT_EXISTS); - - if (balanceVotingPower.value != 0) { - votingPower += IVotingStrategy(address(VOTING_STRATEGY)).getVotingPower( - votingBalanceProofs[i].underlyingAsset, - votingBalanceProofs[i].slot, - balanceVotingPower.value, - voteConfig.l1ProposalBlockHash - ); - } - } - require(votingPower != 0, Errors.USER_VOTING_BALANCE_IS_ZERO); - - if (support) { - proposal.forVotes += votingPower.toUint128(); - } else { - proposal.againstVotes += votingPower.toUint128(); - } - - vote.support = support; - vote.votingPower = votingPower.toUint248(); - - emit VoteEmitted(proposalId, voter, support, votingPower); - } - - /** - * @notice method to send the voting results on a proposal back to L1 - * @param proposalId id of the proposal to send the voting result to L1 - * @dev This method should be implemented to trigger the bridging flow - */ - function _sendVoteResults( - uint256 proposalId, - uint256 forVotes, - uint256 againstVotes - ) internal virtual; - - /** - * @notice method to get the state of a proposal specified by its id - * @param proposal the proposal to retrieve the state of - * @return the state of the proposal - */ - function _getProposalState( - Proposal storage proposal - ) internal view returns (ProposalState) { - if (proposal.endTime == 0) { - return ProposalState.NotCreated; - } else if (_getCurrentTimeRef() <= proposal.endTime) { - return ProposalState.Active; - } else if (proposal.sentToGovernance) { - return ProposalState.SentToGovernance; - } else { - return ProposalState.Finished; - } - } - - /** - * @notice method to get the timestamp of a block casted to uint40 - * @return uint40 block timestamp - */ - function _getCurrentTimeRef() internal view returns (uint40) { - return uint40(block.timestamp); - } - - /** - * @notice method that registers a proposal configuration and creates the voting if it can. If not it will register the - the configuration for later creation. - * @param proposalId id of the proposal bridged to start the vote on - * @param blockHash hash of the block on L1 when the proposal was activated for voting - * @param votingDuration duration in seconds of the vote - */ - function _createBridgedProposalVote( - uint256 proposalId, - bytes32 blockHash, - uint24 votingDuration - ) internal { - require( - blockHash != bytes32(0), - Errors.INVALID_VOTE_CONFIGURATION_BLOCKHASH - ); - require( - votingDuration > 0, - Errors.INVALID_VOTE_CONFIGURATION_VOTING_DURATION - ); - require( - _proposalsVoteConfiguration[proposalId].l1ProposalBlockHash == bytes32(0), - Errors.PROPOSAL_VOTE_CONFIGURATION_ALREADY_BRIDGED - ); - - _proposalsVoteConfiguration[proposalId] = IVotingMachineWithProofs - .ProposalVoteConfiguration({ - votingDuration: votingDuration, - l1ProposalBlockHash: blockHash - }); - _proposalsVoteConfigurationIds.push(proposalId); - - bool created; - try this.startProposalVote(proposalId) { - created = true; - } catch (bytes memory) {} - - emit ProposalVoteConfigurationBridged( - proposalId, - blockHash, - votingDuration, - created - ); - } - - /** - * @notice method that registers a vote on a proposal from a specific voter, contained in a bridged message - from governance chain - * @param proposalId id of the proposal bridged to start the vote on - * @param voter address that wants to emit the vote - * @param support indicates if vote is in favor or against the proposal - * @param votingAssetsWithSlot list of token addresses with base slots that the voter will use for voting - */ - function _registerBridgedVote( - uint256 proposalId, - address voter, - bool support, - VotingAssetWithSlot[] memory votingAssetsWithSlot - ) internal { - // It also only allows to register the vote when proposal is active. - // To retry to register a vote (after it fails) the message will need to be retried from the cross chain controller - require( - _getProposalState(_proposals[proposalId]) == ProposalState.Active, - Errors.PROPOSAL_VOTE_CAN_NOT_BE_REGISTERED - ); - require(voter != address(0), Errors.INVALID_VOTER); - require(votingAssetsWithSlot.length > 0, Errors.NO_BRIDGED_VOTING_ASSETS); - require( - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.length == 0, - Errors.VOTE_ALREADY_BRIDGED - ); - _bridgedVotes[voter][proposalId].support = support; - for (uint256 i = 0; i < votingAssetsWithSlot.length; i++) { - require( - IBaseVotingStrategy(address(VOTING_STRATEGY)).isTokenSlotAccepted( - votingAssetsWithSlot[i].underlyingAsset, - votingAssetsWithSlot[i].slot - ), - Errors.INVALID_BRIDGED_VOTING_TOKEN - ); - for (uint256 j = i + 1; j < votingAssetsWithSlot.length; j++) { - require( - votingAssetsWithSlot[j].underlyingAsset != - votingAssetsWithSlot[i].underlyingAsset || - votingAssetsWithSlot[j].slot != votingAssetsWithSlot[i].slot, - Errors.BRIDGED_REPEATED_ASSETS - ); - } - - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.push( - votingAssetsWithSlot[i] - ); - } - - emit VoteBridged(proposalId, voter, support, votingAssetsWithSlot); - } -} diff --git a/security/certora/tests/VotingMachineWithProofs-11.sol b/security/certora/tests/VotingMachineWithProofs-11.sol deleted file mode 100644 index 1ca90ca..0000000 --- a/security/certora/tests/VotingMachineWithProofs-11.sol +++ /dev/null @@ -1,500 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -import {Ownable} from 'solidity-utils/contracts/oz-common/Ownable.sol'; -import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; -import {StateProofVerifier} from './libs/StateProofVerifier.sol'; -import {IVotingMachineWithProofs, IDataWarehouse, IVotingStrategy} from './interfaces/IVotingMachineWithProofs.sol'; -import {IBaseVotingStrategy} from '../../interfaces/IBaseVotingStrategy.sol'; -import {Errors} from '../libraries/Errors.sol'; -import {SlotUtils} from '../libraries/SlotUtils.sol'; -import {EIP712} from 'openzeppelin-contracts/contracts/utils/cryptography/EIP712.sol'; -import {ECDSA} from 'openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol'; - -/** - * @title VotingMachineWithProofs - * @author BGD Labs - * @notice this contract contains the logic to vote on a bridged proposal. It uses registered proofs to calculate the - voting power of the users. Once the voting is finished it will send the results back to the governance chain. - * @dev Abstract contract that is implemented on VotingMachine contract - */ -abstract contract VotingMachineWithProofs is - IVotingMachineWithProofs, - EIP712, - Ownable -{ - using SafeCast for uint256; - - string public constant VOTING_ASSET_WITH_SLOT_RAW = - 'VotingAssetWithSlot(address underlyingAsset,uint128 slot)'; - - /// @inheritdoc IVotingMachineWithProofs - bytes32 public constant VOTE_SUBMITTED_TYPEHASH = - keccak256( - abi.encodePacked( - 'SubmitVote(uint256 proposalId,address voter,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', - VOTING_ASSET_WITH_SLOT_RAW - ) - ); - - /// @inheritdoc IVotingMachineWithProofs - bytes32 public constant VOTING_ASSET_WITH_SLOT_TYPEHASH = - keccak256(abi.encodePacked(VOTING_ASSET_WITH_SLOT_RAW)); - - /// @inheritdoc IVotingMachineWithProofs - string public constant NAME = 'Aave Voting Machine'; - - /// @inheritdoc IVotingMachineWithProofs - IVotingStrategy public immutable VOTING_STRATEGY; - - /// @inheritdoc IVotingMachineWithProofs - IDataWarehouse public immutable DATA_WAREHOUSE; - - // (proposalId => proposal information) stores the information of the proposals - mapping(uint256 => Proposal) internal _proposals; - - // (proposalId => proposal vote configuration) stores the configuration for voting on each proposal - mapping(uint256 => ProposalVoteConfiguration) - internal _proposalsVoteConfiguration; - - // saves the ids of the proposals that have been bridged for a vote. - uint256[] internal _proposalsVoteConfigurationIds; - - // (voter => proposalId => voteInfo) stores the information for the bridged votes - mapping(address => mapping(uint256 => BridgedVote)) internal _bridgedVotes; - - /** - * @param votingStrategy address of the new VotingStrategy contract - */ - constructor(IVotingStrategy votingStrategy) Ownable() EIP712(NAME, 'V1') { - require( - address(votingStrategy) != address(0), - Errors.INVALID_VOTING_STRATEGY - ); - VOTING_STRATEGY = votingStrategy; - DATA_WAREHOUSE = votingStrategy.DATA_WAREHOUSE(); - } - - /// @inheritdoc IVotingMachineWithProofs - function DOMAIN_SEPARATOR() public view returns (bytes32) { - return _domainSeparatorV4(); - } - - /// @inheritdoc IVotingMachineWithProofs - function getBridgedVoteInfo( - uint256 proposalId, - address voter - ) external view returns (BridgedVote memory) { - return _bridgedVotes[voter][proposalId]; - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalVoteConfiguration( - uint256 proposalId - ) external view returns (ProposalVoteConfiguration memory) { - return _proposalsVoteConfiguration[proposalId]; - } - - /// @inheritdoc IVotingMachineWithProofs - function startProposalVote(uint256 proposalId) external returns (uint256) { - ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ - proposalId - ]; - require( - voteConfig.l1ProposalBlockHash != bytes32(0), - Errors.MISSING_PROPOSAL_BLOCK_HASH - ); - Proposal storage newProposal = _proposals[proposalId]; - - require( - _getProposalState(newProposal) == ProposalState.NotCreated, - Errors.PROPOSAL_VOTE_ALREADY_CREATED - ); - - VOTING_STRATEGY.hasRequiredRoots(voteConfig.l1ProposalBlockHash); - - uint40 startTime = _getCurrentTimeRef(); - uint40 endTime = startTime + voteConfig.votingDuration; - - newProposal.id = proposalId; - newProposal.creationBlockNumber = block.number; - newProposal.startTime = startTime; - newProposal.endTime = endTime; - - emit ProposalVoteStarted( - proposalId, - voteConfig.l1ProposalBlockHash, - startTime, - endTime - ); - - return proposalId; - } - - /// @inheritdoc IVotingMachineWithProofs - function submitVoteBySignature( - uint256 proposalId, - address voter, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs, - uint8 v, - bytes32 r, - bytes32 s - ) external { - bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( - votingBalanceProofs.length - ); - for (uint256 i = 0; i < votingBalanceProofs.length; i++) { - underlyingAssetsWithSlotHashes[i] = keccak256( - abi.encode( - VOTING_ASSET_WITH_SLOT_TYPEHASH, - votingBalanceProofs[i].underlyingAsset, - votingBalanceProofs[i].slot - ) - ); - } - - bytes32 digest = _hashTypedDataV4( - keccak256( - abi.encode( - VOTE_SUBMITTED_TYPEHASH, - proposalId, - voter, - support, - keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) - ) - ) - ); - address signer = ECDSA.recover(digest, v, r, s); - - require(signer == voter && signer != address(0), Errors.INVALID_SIGNATURE); - _submitVote(signer, proposalId, support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function settleVoteFromPortal( - uint256 proposalId, - address voter, - VotingBalanceProof[] calldata votingBalanceProofs - ) external { - BridgedVote memory bridgedVote = _bridgedVotes[voter][proposalId]; - - require( - bridgedVote.votingAssetsWithSlot.length == votingBalanceProofs.length, - Errors.INVALID_NUMBER_OF_PROOFS_FOR_VOTING_TOKENS - ); - - // check that the proofs are of the voter assets - for (uint256 i = 0; i < bridgedVote.votingAssetsWithSlot.length; i++) { - bool assetFound; - for (uint256 j = 0; j < votingBalanceProofs.length; j++) { - if ( - votingBalanceProofs[j].underlyingAsset == - bridgedVote.votingAssetsWithSlot[i].underlyingAsset && - votingBalanceProofs[j].slot == - bridgedVote.votingAssetsWithSlot[i].slot - ) { - assetFound = true; - break; - } - } - - require(assetFound, Errors.PROOFS_NOT_FOR_VOTING_TOKENS); - } - - _submitVote(voter, proposalId, bridgedVote.support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function submitVote( - uint256 proposalId, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs - ) external { - _submitVote(msg.sender, proposalId, support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function getUserProposalVote( - address user, - uint256 proposalId - ) external view returns (Vote memory) { - return _proposals[proposalId].votes[user]; - } - - /// @inheritdoc IVotingMachineWithProofs - function closeAndSendVote(uint256 proposalId) external { - Proposal storage proposal = _proposals[proposalId]; - require( - _getProposalState(proposal) == ProposalState.Finished, - Errors.PROPOSAL_VOTE_NOT_FINISHED - ); - - proposal.votingClosedAndSentBlockNumber = block.number; - proposal.votingClosedAndSentTimestamp = _getCurrentTimeRef(); - - uint256 forVotes = proposal.forVotes; - uint256 againstVotes = proposal.againstVotes; - - proposal.sentToGovernance = true; - - _sendVoteResults(proposalId, forVotes, againstVotes); - - emit ProposalResultsSent(proposalId, forVotes, againstVotes); - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalById( - uint256 proposalId - ) external view returns (ProposalWithoutVotes memory) { - Proposal storage proposal = _proposals[proposalId]; - ProposalWithoutVotes memory proposalWithoutVotes = ProposalWithoutVotes({ - id: proposalId, - startTime: proposal.startTime, - endTime: proposal.endTime, - creationBlockNumber: proposal.creationBlockNumber, - forVotes: proposal.forVotes, - againstVotes: proposal.againstVotes, - votingClosedAndSentBlockNumber: proposal.votingClosedAndSentBlockNumber, - votingClosedAndSentTimestamp: proposal.votingClosedAndSentTimestamp, - sentToGovernance: proposal.sentToGovernance - }); - - return proposalWithoutVotes; - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalState( - uint256 proposalId - ) external view returns (ProposalState) { - return _getProposalState(_proposals[proposalId]); - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalsVoteConfigurationIds( - uint256 skip, - uint256 size - ) external view returns (uint256[] memory) { - uint256 proposalListLength = _proposalsVoteConfigurationIds.length; - if (proposalListLength == 0 || proposalListLength <= skip) { - return new uint256[](0); - } else if (proposalListLength < size + skip) { - size = proposalListLength - skip; - } - - uint256[] memory ids = new uint256[](size); - for (uint256 i = 0; i < size; i++) { - ids[i] = _proposalsVoteConfigurationIds[ - proposalListLength - skip - i - ]; - } - return ids; - } - - /** - * @notice method to cast a vote on a proposal specified by its id - * @param voter address with the voting power - * @param proposalId id of the proposal on which the vote will be cast - * @param support boolean indicating if the vote is in favor or against the proposal - * @param votingBalanceProofs list of objects containing the information necessary to vote using the tokens - allowed on the voting strategy. - * @dev A vote does not need to use all the tokens allowed, can be a subset - */ - function _submitVote( - address voter, - uint256 proposalId, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs - ) internal { - Proposal storage proposal = _proposals[proposalId]; - require( - _getProposalState(proposal) == ProposalState.Active, - Errors.PROPOSAL_VOTE_NOT_IN_ACTIVE_STATE - ); - - Vote storage vote = proposal.votes[voter]; - require(vote.votingPower == 0, Errors.PROPOSAL_VOTE_ALREADY_EXISTS); - - ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ - proposalId - ]; - - uint256 votingPower; - StateProofVerifier.SlotValue memory balanceVotingPower; - for (uint256 i = 0; i < votingBalanceProofs.length; i++) { - for (uint256 j = i + 1; j < votingBalanceProofs.length; j++) { - require( - votingBalanceProofs[i].slot != votingBalanceProofs[j].slot || - votingBalanceProofs[i].underlyingAsset != - votingBalanceProofs[j].underlyingAsset, - Errors.VOTE_ONCE_FOR_ASSET - ); - } - - balanceVotingPower = DATA_WAREHOUSE.getStorage( - votingBalanceProofs[i].underlyingAsset, - voteConfig.l1ProposalBlockHash, - SlotUtils.getAccountSlotHash(voter, votingBalanceProofs[i].slot), - votingBalanceProofs[i].proof - ); - - require(balanceVotingPower.exists, Errors.USER_BALANCE_DOES_NOT_EXISTS); - - if (balanceVotingPower.value != 0) { - votingPower += IVotingStrategy(address(VOTING_STRATEGY)).getVotingPower( - votingBalanceProofs[i].underlyingAsset, - votingBalanceProofs[i].slot, - balanceVotingPower.value, - voteConfig.l1ProposalBlockHash - ); - } - } - require(votingPower != 0, Errors.USER_VOTING_BALANCE_IS_ZERO); - - if (support) { - proposal.forVotes += votingPower.toUint128(); - } else { - proposal.againstVotes += votingPower.toUint128(); - } - - vote.support = support; - vote.votingPower = votingPower.toUint248(); - - emit VoteEmitted(proposalId, voter, support, votingPower); - } - - /** - * @notice method to send the voting results on a proposal back to L1 - * @param proposalId id of the proposal to send the voting result to L1 - * @dev This method should be implemented to trigger the bridging flow - */ - function _sendVoteResults( - uint256 proposalId, - uint256 forVotes, - uint256 againstVotes - ) internal virtual; - - /** - * @notice method to get the state of a proposal specified by its id - * @param proposal the proposal to retrieve the state of - * @return the state of the proposal - */ - function _getProposalState( - Proposal storage proposal - ) internal view returns (ProposalState) { - if (proposal.endTime == 0) { - return ProposalState.NotCreated; - } else if (_getCurrentTimeRef() <= proposal.endTime) { - return ProposalState.Active; - } else if (proposal.sentToGovernance) { - return ProposalState.SentToGovernance; - } else { - return ProposalState.Finished; - } - } - - /** - * @notice method to get the timestamp of a block casted to uint40 - * @return uint40 block timestamp - */ - function _getCurrentTimeRef() internal view returns (uint40) { - return uint40(block.timestamp); - } - - /** - * @notice method that registers a proposal configuration and creates the voting if it can. If not it will register the - the configuration for later creation. - * @param proposalId id of the proposal bridged to start the vote on - * @param blockHash hash of the block on L1 when the proposal was activated for voting - * @param votingDuration duration in seconds of the vote - */ - function _createBridgedProposalVote( - uint256 proposalId, - bytes32 blockHash, - uint24 votingDuration - ) internal { - require( - blockHash != bytes32(0), - Errors.INVALID_VOTE_CONFIGURATION_BLOCKHASH - ); - require( - votingDuration > 0, - Errors.INVALID_VOTE_CONFIGURATION_VOTING_DURATION - ); - require( - _proposalsVoteConfiguration[proposalId].l1ProposalBlockHash == bytes32(0), - Errors.PROPOSAL_VOTE_CONFIGURATION_ALREADY_BRIDGED - ); - - _proposalsVoteConfiguration[proposalId] = IVotingMachineWithProofs - .ProposalVoteConfiguration({ - votingDuration: votingDuration, - l1ProposalBlockHash: blockHash - }); - _proposalsVoteConfigurationIds.push(proposalId); - - bool created; - try this.startProposalVote(proposalId) { - created = true; - } catch (bytes memory) {} - - emit ProposalVoteConfigurationBridged( - proposalId, - blockHash, - votingDuration, - created - ); - } - - /** - * @notice method that registers a vote on a proposal from a specific voter, contained in a bridged message - from governance chain - * @param proposalId id of the proposal bridged to start the vote on - * @param voter address that wants to emit the vote - * @param support indicates if vote is in favor or against the proposal - * @param votingAssetsWithSlot list of token addresses with base slots that the voter will use for voting - */ - function _registerBridgedVote( - uint256 proposalId, - address voter, - bool support, - VotingAssetWithSlot[] memory votingAssetsWithSlot - ) internal { - // It also only allows to register the vote when proposal is active. - // To retry to register a vote (after it fails) the message will need to be retried from the cross chain controller - require( - _getProposalState(_proposals[proposalId]) == ProposalState.Active, - Errors.PROPOSAL_VOTE_CAN_NOT_BE_REGISTERED - ); - require(voter != address(0), Errors.INVALID_VOTER); - require(votingAssetsWithSlot.length > 0, Errors.NO_BRIDGED_VOTING_ASSETS); - require( - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.length == 0, - Errors.VOTE_ALREADY_BRIDGED - ); - _bridgedVotes[voter][proposalId].support = support; - for (uint256 i = 0; i < votingAssetsWithSlot.length; i++) { - require( - IBaseVotingStrategy(address(VOTING_STRATEGY)).isTokenSlotAccepted( - votingAssetsWithSlot[i].underlyingAsset, - votingAssetsWithSlot[i].slot - ), - Errors.INVALID_BRIDGED_VOTING_TOKEN - ); - for (uint256 j = i + 1; j < votingAssetsWithSlot.length; j++) { - require( - votingAssetsWithSlot[j].underlyingAsset != - votingAssetsWithSlot[i].underlyingAsset || - votingAssetsWithSlot[j].slot != votingAssetsWithSlot[i].slot, - Errors.BRIDGED_REPEATED_ASSETS - ); - } - - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.push( - votingAssetsWithSlot[i] - ); - } - - emit VoteBridged(proposalId, voter, support, votingAssetsWithSlot); - } -} diff --git a/security/certora/tests/VotingMachineWithProofs-7.sol b/security/certora/tests/VotingMachineWithProofs-7.sol index 5733aa4..6052784 100644 --- a/security/certora/tests/VotingMachineWithProofs-7.sol +++ b/security/certora/tests/VotingMachineWithProofs-7.sol @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: MIT +// SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.0; import {Ownable} from 'solidity-utils/contracts/oz-common/Ownable.sol'; @@ -25,6 +25,10 @@ abstract contract VotingMachineWithProofs is { using SafeCast for uint256; + /// @inheritdoc IVotingMachineWithProofs + uint256 public constant REPRESENTATIVES_SLOT = 9; + + /// @inheritdoc IVotingMachineWithProofs string public constant VOTING_ASSET_WITH_SLOT_RAW = 'VotingAssetWithSlot(address underlyingAsset,uint128 slot)'; @@ -37,6 +41,15 @@ abstract contract VotingMachineWithProofs is ) ); + /// @inheritdoc IVotingMachineWithProofs + bytes32 public constant VOTE_SUBMITTED_BY_REPRESENTATIVE_TYPEHASH = + keccak256( + abi.encodePacked( + 'SubmitVoteAsRepresentative(uint256 proposalId,address voter,address representative,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', + VOTING_ASSET_WITH_SLOT_RAW + ) + ); + /// @inheritdoc IVotingMachineWithProofs bytes32 public constant VOTING_ASSET_WITH_SLOT_TYPEHASH = keccak256(abi.encodePacked(VOTING_ASSET_WITH_SLOT_RAW)); @@ -50,6 +63,9 @@ abstract contract VotingMachineWithProofs is /// @inheritdoc IVotingMachineWithProofs IDataWarehouse public immutable DATA_WAREHOUSE; + /// @inheritdoc IVotingMachineWithProofs + address public immutable GOVERNANCE; + // (proposalId => proposal information) stores the information of the proposals mapping(uint256 => Proposal) internal _proposals; @@ -60,19 +76,22 @@ abstract contract VotingMachineWithProofs is // saves the ids of the proposals that have been bridged for a vote. uint256[] internal _proposalsVoteConfigurationIds; - // (voter => proposalId => voteInfo) stores the information for the bridged votes - mapping(address => mapping(uint256 => BridgedVote)) internal _bridgedVotes; - /** * @param votingStrategy address of the new VotingStrategy contract + * @param governance address of the governance contract on ethereum */ - constructor(IVotingStrategy votingStrategy) Ownable() EIP712(NAME, 'V1') { + constructor( + IVotingStrategy votingStrategy, + address governance + ) Ownable() EIP712(NAME, 'V1') { require( address(votingStrategy) != address(0), Errors.INVALID_VOTING_STRATEGY ); + require(governance != address(0), Errors.VM_INVALID_GOVERNANCE_ADDRESS); VOTING_STRATEGY = votingStrategy; DATA_WAREHOUSE = votingStrategy.DATA_WAREHOUSE(); + GOVERNANCE = governance; } /// @inheritdoc IVotingMachineWithProofs @@ -80,14 +99,6 @@ abstract contract VotingMachineWithProofs is return _domainSeparatorV4(); } - /// @inheritdoc IVotingMachineWithProofs - function getBridgedVoteInfo( - uint256 proposalId, - address voter - ) external view returns (BridgedVote memory) { - return _bridgedVotes[voter][proposalId+1]; - } - /// @inheritdoc IVotingMachineWithProofs function getProposalVoteConfiguration( uint256 proposalId @@ -113,6 +124,8 @@ abstract contract VotingMachineWithProofs is VOTING_STRATEGY.hasRequiredRoots(voteConfig.l1ProposalBlockHash); + _checkRepresentationRoots(voteConfig.l1ProposalBlockHash); + uint40 startTime = _getCurrentTimeRef(); uint40 endTime = startTime + voteConfig.votingDuration; @@ -172,46 +185,130 @@ abstract contract VotingMachineWithProofs is } /// @inheritdoc IVotingMachineWithProofs - function settleVoteFromPortal( + function submitVote( uint256 proposalId, - address voter, + bool support, VotingBalanceProof[] calldata votingBalanceProofs ) external { - BridgedVote memory bridgedVote = _bridgedVotes[voter][proposalId]; + _submitVote(msg.sender, proposalId, support, votingBalanceProofs); + } - require( - bridgedVote.votingAssetsWithSlot.length == votingBalanceProofs.length, - Errors.INVALID_NUMBER_OF_PROOFS_FOR_VOTING_TOKENS + /// @inheritdoc IVotingMachineWithProofs + function submitVoteAsRepresentativeBySignature( + uint256 proposalId, + address voter, + address representative, + bool support, + bytes memory proofOfRepresentation, + VotingBalanceProof[] calldata votingBalanceProofs, + SignatureParams memory signatureParams + ) external { + bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( + votingBalanceProofs.length ); + for (uint256 i = 0; i < votingBalanceProofs.length; i++) { + underlyingAssetsWithSlotHashes[i] = keccak256( + abi.encode( + VOTING_ASSET_WITH_SLOT_TYPEHASH, + votingBalanceProofs[i].underlyingAsset, + votingBalanceProofs[i].slot + ) + ); + } - // check that the proofs are of the voter assets - for (uint256 i = 0; i < bridgedVote.votingAssetsWithSlot.length; i++) { - bool assetFound; - for (uint256 j = 0; j < votingBalanceProofs.length; j++) { - if ( - votingBalanceProofs[j].underlyingAsset == - bridgedVote.votingAssetsWithSlot[i].underlyingAsset && - votingBalanceProofs[j].slot == - bridgedVote.votingAssetsWithSlot[i].slot - ) { - assetFound = true; - break; - } - } + bytes32 digest = _hashTypedDataV4( + keccak256( + abi.encode( + VOTE_SUBMITTED_BY_REPRESENTATIVE_TYPEHASH, + proposalId, + voter, + representative, + support, + keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) + ) + ) + ); + address signer = ECDSA.recover( + digest, + signatureParams.v, + signatureParams.r, + signatureParams.s + ); - require(assetFound, Errors.PROOFS_NOT_FOR_VOTING_TOKENS); - } + require( + signer == representative && signer != address(0), + Errors.INVALID_SIGNATURE + ); - _submitVote(voter, proposalId, bridgedVote.support, votingBalanceProofs); + _submitVoteAsRepresentative( + proposalId, + support, + voter, + representative, + proofOfRepresentation, + votingBalanceProofs + ); } /// @inheritdoc IVotingMachineWithProofs - function submitVote( + function submitVoteAsRepresentative( uint256 proposalId, bool support, + address voter, + bytes memory proofOfRepresentation, VotingBalanceProof[] calldata votingBalanceProofs ) external { - _submitVote(msg.sender, proposalId, support, votingBalanceProofs); + _submitVoteAsRepresentative( + proposalId, + support, + voter, + msg.sender, + proofOfRepresentation, + votingBalanceProofs + ); + } + + /** + * @notice Function to register the vote of user as its representative + * @param proposalId id of the proposal + * @param support boolean, true = vote for, false = vote against + * @param voter the voter address + * @param representative address of the voter representative + * @param proofOfRepresentation proof that can validate that msg.sender is the voter representative + * @param votingBalanceProofs list of voting assets proofs + */ + function _submitVoteAsRepresentative( + uint256 proposalId, + bool support, + address voter, + address representative, + bytes memory proofOfRepresentation, + VotingBalanceProof[] calldata votingBalanceProofs + ) internal { + require(voter != address(0), Errors.INVALID_VOTER); + bytes32 l1ProposalBlockHash = _proposalsVoteConfiguration[proposalId] + .l1ProposalBlockHash; + + bytes32 slot = SlotUtils.getRepresentativeSlotHash( + voter, + block.chainid, + REPRESENTATIVES_SLOT + ); + StateProofVerifier.SlotValue memory storageData = DATA_WAREHOUSE.getStorage( + GOVERNANCE, + l1ProposalBlockHash, + slot, + proofOfRepresentation + ); + + address storedRepresentative = address(uint160(storageData.value)); + + require( + representative == storedRepresentative && representative != address(0), + Errors.CALLER_IS_NOT_VOTER_REPRESENTATIVE + ); + + _submitVote(voter, proposalId, support, votingBalanceProofs); } /// @inheritdoc IVotingMachineWithProofs @@ -219,7 +316,7 @@ abstract contract VotingMachineWithProofs is address user, uint256 proposalId ) external view returns (Vote memory) { - return _proposals[proposalId].votes[user]; + return _proposals[proposalId+1].votes[user]; } /// @inheritdoc IVotingMachineWithProofs @@ -291,6 +388,16 @@ abstract contract VotingMachineWithProofs is return ids; } + function _checkRepresentationRoots( + bytes32 l1ProposalBlockHash + ) internal view { + require( + DATA_WAREHOUSE.getStorageRoots(GOVERNANCE, l1ProposalBlockHash) != + bytes32(0), + Errors.MISSING_REPRESENTATION_ROOTS + ); + } + /** * @notice method to cast a vote on a proposal specified by its id * @param voter address with the voting power @@ -445,56 +552,4 @@ abstract contract VotingMachineWithProofs is created ); } - - /** - * @notice method that registers a vote on a proposal from a specific voter, contained in a bridged message - from governance chain - * @param proposalId id of the proposal bridged to start the vote on - * @param voter address that wants to emit the vote - * @param support indicates if vote is in favor or against the proposal - * @param votingAssetsWithSlot list of token addresses with base slots that the voter will use for voting - */ - function _registerBridgedVote( - uint256 proposalId, - address voter, - bool support, - VotingAssetWithSlot[] memory votingAssetsWithSlot - ) internal { - // It also only allows to register the vote when proposal is active. - // To retry to register a vote (after it fails) the message will need to be retried from the cross chain controller - require( - _getProposalState(_proposals[proposalId]) == ProposalState.Active, - Errors.PROPOSAL_VOTE_CAN_NOT_BE_REGISTERED - ); - require(voter != address(0), Errors.INVALID_VOTER); - require(votingAssetsWithSlot.length > 0, Errors.NO_BRIDGED_VOTING_ASSETS); - require( - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.length == 0, - Errors.VOTE_ALREADY_BRIDGED - ); - _bridgedVotes[voter][proposalId].support = support; - for (uint256 i = 0; i < votingAssetsWithSlot.length; i++) { - require( - IBaseVotingStrategy(address(VOTING_STRATEGY)).isTokenSlotAccepted( - votingAssetsWithSlot[i].underlyingAsset, - votingAssetsWithSlot[i].slot - ), - Errors.INVALID_BRIDGED_VOTING_TOKEN - ); - for (uint256 j = i + 1; j < votingAssetsWithSlot.length; j++) { - require( - votingAssetsWithSlot[j].underlyingAsset != - votingAssetsWithSlot[i].underlyingAsset || - votingAssetsWithSlot[j].slot != votingAssetsWithSlot[i].slot, - Errors.BRIDGED_REPEATED_ASSETS - ); - } - - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.push( - votingAssetsWithSlot[i] - ); - } - - emit VoteBridged(proposalId, voter, support, votingAssetsWithSlot); - } } diff --git a/security/certora/tests/VotingMachineWithProofs-8.sol b/security/certora/tests/VotingMachineWithProofs-8.sol deleted file mode 100644 index e69c395..0000000 --- a/security/certora/tests/VotingMachineWithProofs-8.sol +++ /dev/null @@ -1,500 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -import {Ownable} from 'solidity-utils/contracts/oz-common/Ownable.sol'; -import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; -import {StateProofVerifier} from './libs/StateProofVerifier.sol'; -import {IVotingMachineWithProofs, IDataWarehouse, IVotingStrategy} from './interfaces/IVotingMachineWithProofs.sol'; -import {IBaseVotingStrategy} from '../../interfaces/IBaseVotingStrategy.sol'; -import {Errors} from '../libraries/Errors.sol'; -import {SlotUtils} from '../libraries/SlotUtils.sol'; -import {EIP712} from 'openzeppelin-contracts/contracts/utils/cryptography/EIP712.sol'; -import {ECDSA} from 'openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol'; - -/** - * @title VotingMachineWithProofs - * @author BGD Labs - * @notice this contract contains the logic to vote on a bridged proposal. It uses registered proofs to calculate the - voting power of the users. Once the voting is finished it will send the results back to the governance chain. - * @dev Abstract contract that is implemented on VotingMachine contract - */ -abstract contract VotingMachineWithProofs is - IVotingMachineWithProofs, - EIP712, - Ownable -{ - using SafeCast for uint256; - - string public constant VOTING_ASSET_WITH_SLOT_RAW = - 'VotingAssetWithSlot(address underlyingAsset,uint128 slot)'; - - /// @inheritdoc IVotingMachineWithProofs - bytes32 public constant VOTE_SUBMITTED_TYPEHASH = - keccak256( - abi.encodePacked( - 'SubmitVote(uint256 proposalId,address voter,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', - VOTING_ASSET_WITH_SLOT_RAW - ) - ); - - /// @inheritdoc IVotingMachineWithProofs - bytes32 public constant VOTING_ASSET_WITH_SLOT_TYPEHASH = - keccak256(abi.encodePacked(VOTING_ASSET_WITH_SLOT_RAW)); - - /// @inheritdoc IVotingMachineWithProofs - string public constant NAME = 'Aave Voting Machine'; - - /// @inheritdoc IVotingMachineWithProofs - IVotingStrategy public immutable VOTING_STRATEGY; - - /// @inheritdoc IVotingMachineWithProofs - IDataWarehouse public immutable DATA_WAREHOUSE; - - // (proposalId => proposal information) stores the information of the proposals - mapping(uint256 => Proposal) internal _proposals; - - // (proposalId => proposal vote configuration) stores the configuration for voting on each proposal - mapping(uint256 => ProposalVoteConfiguration) - internal _proposalsVoteConfiguration; - - // saves the ids of the proposals that have been bridged for a vote. - uint256[] internal _proposalsVoteConfigurationIds; - - // (voter => proposalId => voteInfo) stores the information for the bridged votes - mapping(address => mapping(uint256 => BridgedVote)) internal _bridgedVotes; - - /** - * @param votingStrategy address of the new VotingStrategy contract - */ - constructor(IVotingStrategy votingStrategy) Ownable() EIP712(NAME, 'V1') { - require( - address(votingStrategy) != address(0), - Errors.INVALID_VOTING_STRATEGY - ); - VOTING_STRATEGY = votingStrategy; - DATA_WAREHOUSE = votingStrategy.DATA_WAREHOUSE(); - } - - /// @inheritdoc IVotingMachineWithProofs - function DOMAIN_SEPARATOR() public view returns (bytes32) { - return _domainSeparatorV4(); - } - - /// @inheritdoc IVotingMachineWithProofs - function getBridgedVoteInfo( - uint256 proposalId, - address voter - ) external view returns (BridgedVote memory) { - return _bridgedVotes[voter][proposalId]; - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalVoteConfiguration( - uint256 proposalId - ) external view returns (ProposalVoteConfiguration memory) { - return _proposalsVoteConfiguration[proposalId]; - } - - /// @inheritdoc IVotingMachineWithProofs - function startProposalVote(uint256 proposalId) external returns (uint256) { - ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ - proposalId - ]; - require( - voteConfig.l1ProposalBlockHash != bytes32(0), - Errors.MISSING_PROPOSAL_BLOCK_HASH - ); - Proposal storage newProposal = _proposals[proposalId]; - - require( - _getProposalState(newProposal) == ProposalState.NotCreated, - Errors.PROPOSAL_VOTE_ALREADY_CREATED - ); - - VOTING_STRATEGY.hasRequiredRoots(voteConfig.l1ProposalBlockHash); - - uint40 startTime = _getCurrentTimeRef(); - uint40 endTime = startTime + voteConfig.votingDuration; - - newProposal.id = proposalId; - newProposal.creationBlockNumber = block.number; - newProposal.startTime = startTime; - newProposal.endTime = endTime; - - emit ProposalVoteStarted( - proposalId, - voteConfig.l1ProposalBlockHash, - startTime, - endTime - ); - - return proposalId; - } - - /// @inheritdoc IVotingMachineWithProofs - function submitVoteBySignature( - uint256 proposalId, - address voter, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs, - uint8 v, - bytes32 r, - bytes32 s - ) external { - bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( - votingBalanceProofs.length - ); - for (uint256 i = 0; i < votingBalanceProofs.length; i++) { - underlyingAssetsWithSlotHashes[i] = keccak256( - abi.encode( - VOTING_ASSET_WITH_SLOT_TYPEHASH, - votingBalanceProofs[i].underlyingAsset, - votingBalanceProofs[i].slot - ) - ); - } - - bytes32 digest = _hashTypedDataV4( - keccak256( - abi.encode( - VOTE_SUBMITTED_TYPEHASH, - proposalId, - voter, - support, - keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) - ) - ) - ); - address signer = ECDSA.recover(digest, v, r, s); - - require(signer == voter && signer != address(0), Errors.INVALID_SIGNATURE); - _submitVote(signer, proposalId, support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function settleVoteFromPortal( - uint256 proposalId, - address voter, - VotingBalanceProof[] calldata votingBalanceProofs - ) external { - BridgedVote memory bridgedVote = _bridgedVotes[voter][proposalId]; - - require( - bridgedVote.votingAssetsWithSlot.length == votingBalanceProofs.length, - Errors.INVALID_NUMBER_OF_PROOFS_FOR_VOTING_TOKENS - ); - - // check that the proofs are of the voter assets - for (uint256 i = 0; i < bridgedVote.votingAssetsWithSlot.length; i++) { - bool assetFound; - for (uint256 j = 0; j < votingBalanceProofs.length; j++) { - if ( - votingBalanceProofs[j].underlyingAsset == - bridgedVote.votingAssetsWithSlot[i].underlyingAsset && - votingBalanceProofs[j].slot == - bridgedVote.votingAssetsWithSlot[i].slot - ) { - assetFound = false; - break; - } - } - - require(assetFound, Errors.PROOFS_NOT_FOR_VOTING_TOKENS); - } - - _submitVote(voter, proposalId, bridgedVote.support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function submitVote( - uint256 proposalId, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs - ) external { - _submitVote(msg.sender, proposalId, support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function getUserProposalVote( - address user, - uint256 proposalId - ) external view returns (Vote memory) { - return _proposals[proposalId].votes[user]; - } - - /// @inheritdoc IVotingMachineWithProofs - function closeAndSendVote(uint256 proposalId) external { - Proposal storage proposal = _proposals[proposalId]; - require( - _getProposalState(proposal) == ProposalState.Finished, - Errors.PROPOSAL_VOTE_NOT_FINISHED - ); - - proposal.votingClosedAndSentBlockNumber = block.number; - proposal.votingClosedAndSentTimestamp = _getCurrentTimeRef(); - - uint256 forVotes = proposal.forVotes; - uint256 againstVotes = proposal.againstVotes; - - proposal.sentToGovernance = true; - - _sendVoteResults(proposalId, forVotes, againstVotes); - - emit ProposalResultsSent(proposalId, forVotes, againstVotes); - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalById( - uint256 proposalId - ) external view returns (ProposalWithoutVotes memory) { - Proposal storage proposal = _proposals[proposalId]; - ProposalWithoutVotes memory proposalWithoutVotes = ProposalWithoutVotes({ - id: proposalId, - startTime: proposal.startTime, - endTime: proposal.endTime, - creationBlockNumber: proposal.creationBlockNumber, - forVotes: proposal.forVotes, - againstVotes: proposal.againstVotes, - votingClosedAndSentBlockNumber: proposal.votingClosedAndSentBlockNumber, - votingClosedAndSentTimestamp: proposal.votingClosedAndSentTimestamp, - sentToGovernance: proposal.sentToGovernance - }); - - return proposalWithoutVotes; - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalState( - uint256 proposalId - ) external view returns (ProposalState) { - return _getProposalState(_proposals[proposalId]); - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalsVoteConfigurationIds( - uint256 skip, - uint256 size - ) external view returns (uint256[] memory) { - uint256 proposalListLength = _proposalsVoteConfigurationIds.length; - if (proposalListLength == 0 || proposalListLength <= skip) { - return new uint256[](0); - } else if (proposalListLength < size + skip) { - size = proposalListLength - skip; - } - - uint256[] memory ids = new uint256[](size); - for (uint256 i = 0; i < size; i++) { - ids[i] = _proposalsVoteConfigurationIds[ - proposalListLength - skip - i - 1 - ]; - } - return ids; - } - - /** - * @notice method to cast a vote on a proposal specified by its id - * @param voter address with the voting power - * @param proposalId id of the proposal on which the vote will be cast - * @param support boolean indicating if the vote is in favor or against the proposal - * @param votingBalanceProofs list of objects containing the information necessary to vote using the tokens - allowed on the voting strategy. - * @dev A vote does not need to use all the tokens allowed, can be a subset - */ - function _submitVote( - address voter, - uint256 proposalId, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs - ) internal { - Proposal storage proposal = _proposals[proposalId]; - require( - _getProposalState(proposal) == ProposalState.Active, - Errors.PROPOSAL_VOTE_NOT_IN_ACTIVE_STATE - ); - - Vote storage vote = proposal.votes[voter]; - require(vote.votingPower == 0, Errors.PROPOSAL_VOTE_ALREADY_EXISTS); - - ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ - proposalId - ]; - - uint256 votingPower; - StateProofVerifier.SlotValue memory balanceVotingPower; - for (uint256 i = 0; i < votingBalanceProofs.length; i++) { - for (uint256 j = i + 1; j < votingBalanceProofs.length; j++) { - require( - votingBalanceProofs[i].slot != votingBalanceProofs[j].slot || - votingBalanceProofs[i].underlyingAsset != - votingBalanceProofs[j].underlyingAsset, - Errors.VOTE_ONCE_FOR_ASSET - ); - } - - balanceVotingPower = DATA_WAREHOUSE.getStorage( - votingBalanceProofs[i].underlyingAsset, - voteConfig.l1ProposalBlockHash, - SlotUtils.getAccountSlotHash(voter, votingBalanceProofs[i].slot), - votingBalanceProofs[i].proof - ); - - require(balanceVotingPower.exists, Errors.USER_BALANCE_DOES_NOT_EXISTS); - - if (balanceVotingPower.value != 0) { - votingPower += IVotingStrategy(address(VOTING_STRATEGY)).getVotingPower( - votingBalanceProofs[i].underlyingAsset, - votingBalanceProofs[i].slot, - balanceVotingPower.value, - voteConfig.l1ProposalBlockHash - ); - } - } - require(votingPower != 0, Errors.USER_VOTING_BALANCE_IS_ZERO); - - if (support) { - proposal.forVotes += votingPower.toUint128(); - } else { - proposal.againstVotes += votingPower.toUint128(); - } - - vote.support = support; - vote.votingPower = votingPower.toUint248(); - - emit VoteEmitted(proposalId, voter, support, votingPower); - } - - /** - * @notice method to send the voting results on a proposal back to L1 - * @param proposalId id of the proposal to send the voting result to L1 - * @dev This method should be implemented to trigger the bridging flow - */ - function _sendVoteResults( - uint256 proposalId, - uint256 forVotes, - uint256 againstVotes - ) internal virtual; - - /** - * @notice method to get the state of a proposal specified by its id - * @param proposal the proposal to retrieve the state of - * @return the state of the proposal - */ - function _getProposalState( - Proposal storage proposal - ) internal view returns (ProposalState) { - if (proposal.endTime == 0) { - return ProposalState.NotCreated; - } else if (_getCurrentTimeRef() <= proposal.endTime) { - return ProposalState.Active; - } else if (proposal.sentToGovernance) { - return ProposalState.SentToGovernance; - } else { - return ProposalState.Finished; - } - } - - /** - * @notice method to get the timestamp of a block casted to uint40 - * @return uint40 block timestamp - */ - function _getCurrentTimeRef() internal view returns (uint40) { - return uint40(block.timestamp); - } - - /** - * @notice method that registers a proposal configuration and creates the voting if it can. If not it will register the - the configuration for later creation. - * @param proposalId id of the proposal bridged to start the vote on - * @param blockHash hash of the block on L1 when the proposal was activated for voting - * @param votingDuration duration in seconds of the vote - */ - function _createBridgedProposalVote( - uint256 proposalId, - bytes32 blockHash, - uint24 votingDuration - ) internal { - require( - blockHash != bytes32(0), - Errors.INVALID_VOTE_CONFIGURATION_BLOCKHASH - ); - require( - votingDuration > 0, - Errors.INVALID_VOTE_CONFIGURATION_VOTING_DURATION - ); - require( - _proposalsVoteConfiguration[proposalId].l1ProposalBlockHash == bytes32(0), - Errors.PROPOSAL_VOTE_CONFIGURATION_ALREADY_BRIDGED - ); - - _proposalsVoteConfiguration[proposalId] = IVotingMachineWithProofs - .ProposalVoteConfiguration({ - votingDuration: votingDuration, - l1ProposalBlockHash: blockHash - }); - _proposalsVoteConfigurationIds.push(proposalId); - - bool created; - try this.startProposalVote(proposalId) { - created = true; - } catch (bytes memory) {} - - emit ProposalVoteConfigurationBridged( - proposalId, - blockHash, - votingDuration, - created - ); - } - - /** - * @notice method that registers a vote on a proposal from a specific voter, contained in a bridged message - from governance chain - * @param proposalId id of the proposal bridged to start the vote on - * @param voter address that wants to emit the vote - * @param support indicates if vote is in favor or against the proposal - * @param votingAssetsWithSlot list of token addresses with base slots that the voter will use for voting - */ - function _registerBridgedVote( - uint256 proposalId, - address voter, - bool support, - VotingAssetWithSlot[] memory votingAssetsWithSlot - ) internal { - // It also only allows to register the vote when proposal is active. - // To retry to register a vote (after it fails) the message will need to be retried from the cross chain controller - require( - _getProposalState(_proposals[proposalId]) == ProposalState.Active, - Errors.PROPOSAL_VOTE_CAN_NOT_BE_REGISTERED - ); - require(voter != address(0), Errors.INVALID_VOTER); - require(votingAssetsWithSlot.length > 0, Errors.NO_BRIDGED_VOTING_ASSETS); - require( - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.length == 0, - Errors.VOTE_ALREADY_BRIDGED - ); - _bridgedVotes[voter][proposalId].support = support; - for (uint256 i = 0; i < votingAssetsWithSlot.length; i++) { - require( - IBaseVotingStrategy(address(VOTING_STRATEGY)).isTokenSlotAccepted( - votingAssetsWithSlot[i].underlyingAsset, - votingAssetsWithSlot[i].slot - ), - Errors.INVALID_BRIDGED_VOTING_TOKEN - ); - for (uint256 j = i + 1; j < votingAssetsWithSlot.length; j++) { - require( - votingAssetsWithSlot[j].underlyingAsset != - votingAssetsWithSlot[i].underlyingAsset || - votingAssetsWithSlot[j].slot != votingAssetsWithSlot[i].slot, - Errors.BRIDGED_REPEATED_ASSETS - ); - } - - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.push( - votingAssetsWithSlot[i] - ); - } - - emit VoteBridged(proposalId, voter, support, votingAssetsWithSlot); - } -} diff --git a/security/certora/tests/VotingMachineWithProofs-9.sol b/security/certora/tests/VotingMachineWithProofs-9.sol deleted file mode 100644 index de7025d..0000000 --- a/security/certora/tests/VotingMachineWithProofs-9.sol +++ /dev/null @@ -1,500 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -import {Ownable} from 'solidity-utils/contracts/oz-common/Ownable.sol'; -import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; -import {StateProofVerifier} from './libs/StateProofVerifier.sol'; -import {IVotingMachineWithProofs, IDataWarehouse, IVotingStrategy} from './interfaces/IVotingMachineWithProofs.sol'; -import {IBaseVotingStrategy} from '../../interfaces/IBaseVotingStrategy.sol'; -import {Errors} from '../libraries/Errors.sol'; -import {SlotUtils} from '../libraries/SlotUtils.sol'; -import {EIP712} from 'openzeppelin-contracts/contracts/utils/cryptography/EIP712.sol'; -import {ECDSA} from 'openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol'; - -/** - * @title VotingMachineWithProofs - * @author BGD Labs - * @notice this contract contains the logic to vote on a bridged proposal. It uses registered proofs to calculate the - voting power of the users. Once the voting is finished it will send the results back to the governance chain. - * @dev Abstract contract that is implemented on VotingMachine contract - */ -abstract contract VotingMachineWithProofs is - IVotingMachineWithProofs, - EIP712, - Ownable -{ - using SafeCast for uint256; - - string public constant VOTING_ASSET_WITH_SLOT_RAW = - 'VotingAssetWithSlot(address underlyingAsset,uint128 slot)'; - - /// @inheritdoc IVotingMachineWithProofs - bytes32 public constant VOTE_SUBMITTED_TYPEHASH = - keccak256( - abi.encodePacked( - 'SubmitVote(uint256 proposalId,address voter,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', - VOTING_ASSET_WITH_SLOT_RAW - ) - ); - - /// @inheritdoc IVotingMachineWithProofs - bytes32 public constant VOTING_ASSET_WITH_SLOT_TYPEHASH = - keccak256(abi.encodePacked(VOTING_ASSET_WITH_SLOT_RAW)); - - /// @inheritdoc IVotingMachineWithProofs - string public constant NAME = 'Aave Voting Machine'; - - /// @inheritdoc IVotingMachineWithProofs - IVotingStrategy public immutable VOTING_STRATEGY; - - /// @inheritdoc IVotingMachineWithProofs - IDataWarehouse public immutable DATA_WAREHOUSE; - - // (proposalId => proposal information) stores the information of the proposals - mapping(uint256 => Proposal) internal _proposals; - - // (proposalId => proposal vote configuration) stores the configuration for voting on each proposal - mapping(uint256 => ProposalVoteConfiguration) - internal _proposalsVoteConfiguration; - - // saves the ids of the proposals that have been bridged for a vote. - uint256[] internal _proposalsVoteConfigurationIds; - - // (voter => proposalId => voteInfo) stores the information for the bridged votes - mapping(address => mapping(uint256 => BridgedVote)) internal _bridgedVotes; - - /** - * @param votingStrategy address of the new VotingStrategy contract - */ - constructor(IVotingStrategy votingStrategy) Ownable() EIP712(NAME, 'V1') { - require( - address(votingStrategy) != address(0), - Errors.INVALID_VOTING_STRATEGY - ); - VOTING_STRATEGY = votingStrategy; - DATA_WAREHOUSE = votingStrategy.DATA_WAREHOUSE(); - } - - /// @inheritdoc IVotingMachineWithProofs - function DOMAIN_SEPARATOR() public view returns (bytes32) { - return _domainSeparatorV4(); - } - - /// @inheritdoc IVotingMachineWithProofs - function getBridgedVoteInfo( - uint256 proposalId, - address voter - ) external view returns (BridgedVote memory) { - return _bridgedVotes[voter][proposalId]; - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalVoteConfiguration( - uint256 proposalId - ) external view returns (ProposalVoteConfiguration memory) { - return _proposalsVoteConfiguration[proposalId]; - } - - /// @inheritdoc IVotingMachineWithProofs - function startProposalVote(uint256 proposalId) external returns (uint256) { - ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ - proposalId - ]; - require( - voteConfig.l1ProposalBlockHash != bytes32(0), - Errors.MISSING_PROPOSAL_BLOCK_HASH - ); - Proposal storage newProposal = _proposals[proposalId]; - - require( - _getProposalState(newProposal) == ProposalState.NotCreated, - Errors.PROPOSAL_VOTE_ALREADY_CREATED - ); - - VOTING_STRATEGY.hasRequiredRoots(voteConfig.l1ProposalBlockHash); - - uint40 startTime = _getCurrentTimeRef(); - uint40 endTime = startTime + voteConfig.votingDuration; - - newProposal.id = proposalId; - newProposal.creationBlockNumber = block.number; - newProposal.startTime = startTime; - newProposal.endTime = endTime; - - emit ProposalVoteStarted( - proposalId, - voteConfig.l1ProposalBlockHash, - startTime, - endTime - ); - - return proposalId; - } - - /// @inheritdoc IVotingMachineWithProofs - function submitVoteBySignature( - uint256 proposalId, - address voter, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs, - uint8 v, - bytes32 r, - bytes32 s - ) external { - bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( - votingBalanceProofs.length - ); - for (uint256 i = 0; i < votingBalanceProofs.length; i++) { - underlyingAssetsWithSlotHashes[i] = keccak256( - abi.encode( - VOTING_ASSET_WITH_SLOT_TYPEHASH, - votingBalanceProofs[i].underlyingAsset, - votingBalanceProofs[i].slot - ) - ); - } - - bytes32 digest = _hashTypedDataV4( - keccak256( - abi.encode( - VOTE_SUBMITTED_TYPEHASH, - proposalId, - voter, - support, - keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) - ) - ) - ); - address signer = ECDSA.recover(digest, v, r, s); - - require(signer == voter && signer != address(0), Errors.INVALID_SIGNATURE); - _submitVote(signer, proposalId, support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function settleVoteFromPortal( - uint256 proposalId, - address voter, - VotingBalanceProof[] calldata votingBalanceProofs - ) external { - BridgedVote memory bridgedVote = _bridgedVotes[voter][proposalId]; - - require( - bridgedVote.votingAssetsWithSlot.length == votingBalanceProofs.length, - Errors.INVALID_NUMBER_OF_PROOFS_FOR_VOTING_TOKENS - ); - - // check that the proofs are of the voter assets - for (uint256 i = 0; i < bridgedVote.votingAssetsWithSlot.length; i++) { - bool assetFound; - for (uint256 j = 0; j < votingBalanceProofs.length; j++) { - if ( - votingBalanceProofs[j].underlyingAsset == - bridgedVote.votingAssetsWithSlot[i].underlyingAsset && - votingBalanceProofs[j].slot == - bridgedVote.votingAssetsWithSlot[i].slot - ) { - assetFound = true; - break; - } - } - - require(assetFound, Errors.PROOFS_NOT_FOR_VOTING_TOKENS); - } - - _submitVote(voter, proposalId, bridgedVote.support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function submitVote( - uint256 proposalId, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs - ) external { - _submitVote(msg.sender, proposalId, support, votingBalanceProofs); - } - - /// @inheritdoc IVotingMachineWithProofs - function getUserProposalVote( - address user, - uint256 proposalId - ) external view returns (Vote memory) { - return _proposals[proposalId+1].votes[user]; - } - - /// @inheritdoc IVotingMachineWithProofs - function closeAndSendVote(uint256 proposalId) external { - Proposal storage proposal = _proposals[proposalId]; - require( - _getProposalState(proposal) == ProposalState.Finished, - Errors.PROPOSAL_VOTE_NOT_FINISHED - ); - - proposal.votingClosedAndSentBlockNumber = block.number; - proposal.votingClosedAndSentTimestamp = _getCurrentTimeRef(); - - uint256 forVotes = proposal.forVotes; - uint256 againstVotes = proposal.againstVotes; - - proposal.sentToGovernance = true; - - _sendVoteResults(proposalId, forVotes, againstVotes); - - emit ProposalResultsSent(proposalId, forVotes, againstVotes); - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalById( - uint256 proposalId - ) external view returns (ProposalWithoutVotes memory) { - Proposal storage proposal = _proposals[proposalId]; - ProposalWithoutVotes memory proposalWithoutVotes = ProposalWithoutVotes({ - id: proposalId, - startTime: proposal.startTime, - endTime: proposal.endTime, - creationBlockNumber: proposal.creationBlockNumber, - forVotes: proposal.forVotes, - againstVotes: proposal.againstVotes, - votingClosedAndSentBlockNumber: proposal.votingClosedAndSentBlockNumber, - votingClosedAndSentTimestamp: proposal.votingClosedAndSentTimestamp, - sentToGovernance: proposal.sentToGovernance - }); - - return proposalWithoutVotes; - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalState( - uint256 proposalId - ) external view returns (ProposalState) { - return _getProposalState(_proposals[proposalId]); - } - - /// @inheritdoc IVotingMachineWithProofs - function getProposalsVoteConfigurationIds( - uint256 skip, - uint256 size - ) external view returns (uint256[] memory) { - uint256 proposalListLength = _proposalsVoteConfigurationIds.length; - if (proposalListLength == 0 || proposalListLength <= skip) { - return new uint256[](0); - } else if (proposalListLength < size + skip) { - size = proposalListLength - skip; - } - - uint256[] memory ids = new uint256[](size); - for (uint256 i = 0; i < size; i++) { - ids[i] = _proposalsVoteConfigurationIds[ - proposalListLength - skip - i - 1 - ]; - } - return ids; - } - - /** - * @notice method to cast a vote on a proposal specified by its id - * @param voter address with the voting power - * @param proposalId id of the proposal on which the vote will be cast - * @param support boolean indicating if the vote is in favor or against the proposal - * @param votingBalanceProofs list of objects containing the information necessary to vote using the tokens - allowed on the voting strategy. - * @dev A vote does not need to use all the tokens allowed, can be a subset - */ - function _submitVote( - address voter, - uint256 proposalId, - bool support, - VotingBalanceProof[] calldata votingBalanceProofs - ) internal { - Proposal storage proposal = _proposals[proposalId]; - require( - _getProposalState(proposal) == ProposalState.Active, - Errors.PROPOSAL_VOTE_NOT_IN_ACTIVE_STATE - ); - - Vote storage vote = proposal.votes[voter]; - require(vote.votingPower == 0, Errors.PROPOSAL_VOTE_ALREADY_EXISTS); - - ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ - proposalId - ]; - - uint256 votingPower; - StateProofVerifier.SlotValue memory balanceVotingPower; - for (uint256 i = 0; i < votingBalanceProofs.length; i++) { - for (uint256 j = i + 1; j < votingBalanceProofs.length; j++) { - require( - votingBalanceProofs[i].slot != votingBalanceProofs[j].slot || - votingBalanceProofs[i].underlyingAsset != - votingBalanceProofs[j].underlyingAsset, - Errors.VOTE_ONCE_FOR_ASSET - ); - } - - balanceVotingPower = DATA_WAREHOUSE.getStorage( - votingBalanceProofs[i].underlyingAsset, - voteConfig.l1ProposalBlockHash, - SlotUtils.getAccountSlotHash(voter, votingBalanceProofs[i].slot), - votingBalanceProofs[i].proof - ); - - require(balanceVotingPower.exists, Errors.USER_BALANCE_DOES_NOT_EXISTS); - - if (balanceVotingPower.value != 0) { - votingPower += IVotingStrategy(address(VOTING_STRATEGY)).getVotingPower( - votingBalanceProofs[i].underlyingAsset, - votingBalanceProofs[i].slot, - balanceVotingPower.value, - voteConfig.l1ProposalBlockHash - ); - } - } - require(votingPower != 0, Errors.USER_VOTING_BALANCE_IS_ZERO); - - if (support) { - proposal.forVotes += votingPower.toUint128(); - } else { - proposal.againstVotes += votingPower.toUint128(); - } - - vote.support = support; - vote.votingPower = votingPower.toUint248(); - - emit VoteEmitted(proposalId, voter, support, votingPower); - } - - /** - * @notice method to send the voting results on a proposal back to L1 - * @param proposalId id of the proposal to send the voting result to L1 - * @dev This method should be implemented to trigger the bridging flow - */ - function _sendVoteResults( - uint256 proposalId, - uint256 forVotes, - uint256 againstVotes - ) internal virtual; - - /** - * @notice method to get the state of a proposal specified by its id - * @param proposal the proposal to retrieve the state of - * @return the state of the proposal - */ - function _getProposalState( - Proposal storage proposal - ) internal view returns (ProposalState) { - if (proposal.endTime == 0) { - return ProposalState.NotCreated; - } else if (_getCurrentTimeRef() <= proposal.endTime) { - return ProposalState.Active; - } else if (proposal.sentToGovernance) { - return ProposalState.SentToGovernance; - } else { - return ProposalState.Finished; - } - } - - /** - * @notice method to get the timestamp of a block casted to uint40 - * @return uint40 block timestamp - */ - function _getCurrentTimeRef() internal view returns (uint40) { - return uint40(block.timestamp); - } - - /** - * @notice method that registers a proposal configuration and creates the voting if it can. If not it will register the - the configuration for later creation. - * @param proposalId id of the proposal bridged to start the vote on - * @param blockHash hash of the block on L1 when the proposal was activated for voting - * @param votingDuration duration in seconds of the vote - */ - function _createBridgedProposalVote( - uint256 proposalId, - bytes32 blockHash, - uint24 votingDuration - ) internal { - require( - blockHash != bytes32(0), - Errors.INVALID_VOTE_CONFIGURATION_BLOCKHASH - ); - require( - votingDuration > 0, - Errors.INVALID_VOTE_CONFIGURATION_VOTING_DURATION - ); - require( - _proposalsVoteConfiguration[proposalId].l1ProposalBlockHash == bytes32(0), - Errors.PROPOSAL_VOTE_CONFIGURATION_ALREADY_BRIDGED - ); - - _proposalsVoteConfiguration[proposalId] = IVotingMachineWithProofs - .ProposalVoteConfiguration({ - votingDuration: votingDuration, - l1ProposalBlockHash: blockHash - }); - _proposalsVoteConfigurationIds.push(proposalId); - - bool created; - try this.startProposalVote(proposalId) { - created = true; - } catch (bytes memory) {} - - emit ProposalVoteConfigurationBridged( - proposalId, - blockHash, - votingDuration, - created - ); - } - - /** - * @notice method that registers a vote on a proposal from a specific voter, contained in a bridged message - from governance chain - * @param proposalId id of the proposal bridged to start the vote on - * @param voter address that wants to emit the vote - * @param support indicates if vote is in favor or against the proposal - * @param votingAssetsWithSlot list of token addresses with base slots that the voter will use for voting - */ - function _registerBridgedVote( - uint256 proposalId, - address voter, - bool support, - VotingAssetWithSlot[] memory votingAssetsWithSlot - ) internal { - // It also only allows to register the vote when proposal is active. - // To retry to register a vote (after it fails) the message will need to be retried from the cross chain controller - require( - _getProposalState(_proposals[proposalId]) == ProposalState.Active, - Errors.PROPOSAL_VOTE_CAN_NOT_BE_REGISTERED - ); - require(voter != address(0), Errors.INVALID_VOTER); - require(votingAssetsWithSlot.length > 0, Errors.NO_BRIDGED_VOTING_ASSETS); - require( - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.length == 0, - Errors.VOTE_ALREADY_BRIDGED - ); - _bridgedVotes[voter][proposalId].support = support; - for (uint256 i = 0; i < votingAssetsWithSlot.length; i++) { - require( - IBaseVotingStrategy(address(VOTING_STRATEGY)).isTokenSlotAccepted( - votingAssetsWithSlot[i].underlyingAsset, - votingAssetsWithSlot[i].slot - ), - Errors.INVALID_BRIDGED_VOTING_TOKEN - ); - for (uint256 j = i + 1; j < votingAssetsWithSlot.length; j++) { - require( - votingAssetsWithSlot[j].underlyingAsset != - votingAssetsWithSlot[i].underlyingAsset || - votingAssetsWithSlot[j].slot != votingAssetsWithSlot[i].slot, - Errors.BRIDGED_REPEATED_ASSETS - ); - } - - _bridgedVotes[voter][proposalId].votingAssetsWithSlot.push( - votingAssetsWithSlot[i] - ); - } - - emit VoteBridged(proposalId, voter, support, votingAssetsWithSlot); - } -} diff --git a/src/contracts/BaseVotingStrategy.sol b/src/contracts/BaseVotingStrategy.sol index b55e4bc..8073e65 100644 --- a/src/contracts/BaseVotingStrategy.sol +++ b/src/contracts/BaseVotingStrategy.sol @@ -88,7 +88,7 @@ abstract contract BaseVotingStrategy is IBaseVotingStrategy { if (asset == AAVE() || asset == STK_AAVE()) { votingAssetConfig.storageSlots = new uint128[](1); - votingAssetConfig.storageSlots[0] = BASE_BALANCE_SLOT; + votingAssetConfig.storageSlots[0] = A_AAVE_BASE_BALANCE_SLOT; } else if (asset == A_AAVE()) { votingAssetConfig.storageSlots = new uint128[](2); votingAssetConfig.storageSlots[0] = BASE_BALANCE_SLOT; From 596d17d00677b8f42c4b1b19e1f0ab6ea68a7a0d Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 12:43:31 +0200 Subject: [PATCH 11/14] fixing mutants --- security/certora/tests/REPORT-voting.txt | 6 +- .../tests/VotingMachineWithProofs-8.sol | 555 ++++++++++++++++++ .../tests/VotingMachineWithProofs-9.sol | 555 ++++++++++++++++++ 3 files changed, 1113 insertions(+), 3 deletions(-) create mode 100644 security/certora/tests/VotingMachineWithProofs-8.sol create mode 100644 security/certora/tests/VotingMachineWithProofs-9.sol diff --git a/security/certora/tests/REPORT-voting.txt b/security/certora/tests/REPORT-voting.txt index dc19205..2585989 100644 --- a/security/certora/tests/REPORT-voting.txt +++ b/security/certora/tests/REPORT-voting.txt @@ -29,9 +29,9 @@ The change: VotingMachineWithProofs.sol::319 8. DETECTED Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-8.sol -The change: VotingMachineWithProofs.sol::250 +The change: VotingMachineWithProofs.sol::324 orig: - Proposal storage proposal = _proposals[proposalId]; + Proposal storage proposal = _proposals[proposalId]; mutant: Proposal storage proposal = _proposals[0]; @@ -39,7 +39,7 @@ Suggestion for rules that can catch it: 9. UNDETECTED Changed file: VotingMachineWithProofs.sol ==> VotingMachineWithProofs-9.sol -The change: VotingMachineWithProofs.sol::288 +The change: VotingMachineWithProofs.sol::385 orig: proposalListLength - skip - i - 1 mutant: diff --git a/security/certora/tests/VotingMachineWithProofs-8.sol b/security/certora/tests/VotingMachineWithProofs-8.sol new file mode 100644 index 0000000..268b3b6 --- /dev/null +++ b/security/certora/tests/VotingMachineWithProofs-8.sol @@ -0,0 +1,555 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +import {Ownable} from 'solidity-utils/contracts/oz-common/Ownable.sol'; +import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; +import {StateProofVerifier} from './libs/StateProofVerifier.sol'; +import {IVotingMachineWithProofs, IDataWarehouse, IVotingStrategy} from './interfaces/IVotingMachineWithProofs.sol'; +import {IBaseVotingStrategy} from '../../interfaces/IBaseVotingStrategy.sol'; +import {Errors} from '../libraries/Errors.sol'; +import {SlotUtils} from '../libraries/SlotUtils.sol'; +import {EIP712} from 'openzeppelin-contracts/contracts/utils/cryptography/EIP712.sol'; +import {ECDSA} from 'openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol'; + +/** + * @title VotingMachineWithProofs + * @author BGD Labs + * @notice this contract contains the logic to vote on a bridged proposal. It uses registered proofs to calculate the + voting power of the users. Once the voting is finished it will send the results back to the governance chain. + * @dev Abstract contract that is implemented on VotingMachine contract + */ +abstract contract VotingMachineWithProofs is + IVotingMachineWithProofs, + EIP712, + Ownable +{ + using SafeCast for uint256; + + /// @inheritdoc IVotingMachineWithProofs + uint256 public constant REPRESENTATIVES_SLOT = 9; + + /// @inheritdoc IVotingMachineWithProofs + string public constant VOTING_ASSET_WITH_SLOT_RAW = + 'VotingAssetWithSlot(address underlyingAsset,uint128 slot)'; + + /// @inheritdoc IVotingMachineWithProofs + bytes32 public constant VOTE_SUBMITTED_TYPEHASH = + keccak256( + abi.encodePacked( + 'SubmitVote(uint256 proposalId,address voter,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', + VOTING_ASSET_WITH_SLOT_RAW + ) + ); + + /// @inheritdoc IVotingMachineWithProofs + bytes32 public constant VOTE_SUBMITTED_BY_REPRESENTATIVE_TYPEHASH = + keccak256( + abi.encodePacked( + 'SubmitVoteAsRepresentative(uint256 proposalId,address voter,address representative,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', + VOTING_ASSET_WITH_SLOT_RAW + ) + ); + + /// @inheritdoc IVotingMachineWithProofs + bytes32 public constant VOTING_ASSET_WITH_SLOT_TYPEHASH = + keccak256(abi.encodePacked(VOTING_ASSET_WITH_SLOT_RAW)); + + /// @inheritdoc IVotingMachineWithProofs + string public constant NAME = 'Aave Voting Machine'; + + /// @inheritdoc IVotingMachineWithProofs + IVotingStrategy public immutable VOTING_STRATEGY; + + /// @inheritdoc IVotingMachineWithProofs + IDataWarehouse public immutable DATA_WAREHOUSE; + + /// @inheritdoc IVotingMachineWithProofs + address public immutable GOVERNANCE; + + // (proposalId => proposal information) stores the information of the proposals + mapping(uint256 => Proposal) internal _proposals; + + // (proposalId => proposal vote configuration) stores the configuration for voting on each proposal + mapping(uint256 => ProposalVoteConfiguration) + internal _proposalsVoteConfiguration; + + // saves the ids of the proposals that have been bridged for a vote. + uint256[] internal _proposalsVoteConfigurationIds; + + /** + * @param votingStrategy address of the new VotingStrategy contract + * @param governance address of the governance contract on ethereum + */ + constructor( + IVotingStrategy votingStrategy, + address governance + ) Ownable() EIP712(NAME, 'V1') { + require( + address(votingStrategy) != address(0), + Errors.INVALID_VOTING_STRATEGY + ); + require(governance != address(0), Errors.VM_INVALID_GOVERNANCE_ADDRESS); + VOTING_STRATEGY = votingStrategy; + DATA_WAREHOUSE = votingStrategy.DATA_WAREHOUSE(); + GOVERNANCE = governance; + } + + /// @inheritdoc IVotingMachineWithProofs + function DOMAIN_SEPARATOR() public view returns (bytes32) { + return _domainSeparatorV4(); + } + + /// @inheritdoc IVotingMachineWithProofs + function getProposalVoteConfiguration( + uint256 proposalId + ) external view returns (ProposalVoteConfiguration memory) { + return _proposalsVoteConfiguration[proposalId]; + } + + /// @inheritdoc IVotingMachineWithProofs + function startProposalVote(uint256 proposalId) external returns (uint256) { + ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ + proposalId + ]; + require( + voteConfig.l1ProposalBlockHash != bytes32(0), + Errors.MISSING_PROPOSAL_BLOCK_HASH + ); + Proposal storage newProposal = _proposals[proposalId]; + + require( + _getProposalState(newProposal) == ProposalState.NotCreated, + Errors.PROPOSAL_VOTE_ALREADY_CREATED + ); + + VOTING_STRATEGY.hasRequiredRoots(voteConfig.l1ProposalBlockHash); + + _checkRepresentationRoots(voteConfig.l1ProposalBlockHash); + + uint40 startTime = _getCurrentTimeRef(); + uint40 endTime = startTime + voteConfig.votingDuration; + + newProposal.id = proposalId; + newProposal.creationBlockNumber = block.number; + newProposal.startTime = startTime; + newProposal.endTime = endTime; + + emit ProposalVoteStarted( + proposalId, + voteConfig.l1ProposalBlockHash, + startTime, + endTime + ); + + return proposalId; + } + + /// @inheritdoc IVotingMachineWithProofs + function submitVoteBySignature( + uint256 proposalId, + address voter, + bool support, + VotingBalanceProof[] calldata votingBalanceProofs, + uint8 v, + bytes32 r, + bytes32 s + ) external { + bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( + votingBalanceProofs.length + ); + for (uint256 i = 0; i < votingBalanceProofs.length; i++) { + underlyingAssetsWithSlotHashes[i] = keccak256( + abi.encode( + VOTING_ASSET_WITH_SLOT_TYPEHASH, + votingBalanceProofs[i].underlyingAsset, + votingBalanceProofs[i].slot + ) + ); + } + + bytes32 digest = _hashTypedDataV4( + keccak256( + abi.encode( + VOTE_SUBMITTED_TYPEHASH, + proposalId, + voter, + support, + keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) + ) + ) + ); + address signer = ECDSA.recover(digest, v, r, s); + + require(signer == voter && signer != address(0), Errors.INVALID_SIGNATURE); + _submitVote(signer, proposalId, support, votingBalanceProofs); + } + + /// @inheritdoc IVotingMachineWithProofs + function submitVote( + uint256 proposalId, + bool support, + VotingBalanceProof[] calldata votingBalanceProofs + ) external { + _submitVote(msg.sender, proposalId, support, votingBalanceProofs); + } + + /// @inheritdoc IVotingMachineWithProofs + function submitVoteAsRepresentativeBySignature( + uint256 proposalId, + address voter, + address representative, + bool support, + bytes memory proofOfRepresentation, + VotingBalanceProof[] calldata votingBalanceProofs, + SignatureParams memory signatureParams + ) external { + bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( + votingBalanceProofs.length + ); + for (uint256 i = 0; i < votingBalanceProofs.length; i++) { + underlyingAssetsWithSlotHashes[i] = keccak256( + abi.encode( + VOTING_ASSET_WITH_SLOT_TYPEHASH, + votingBalanceProofs[i].underlyingAsset, + votingBalanceProofs[i].slot + ) + ); + } + + bytes32 digest = _hashTypedDataV4( + keccak256( + abi.encode( + VOTE_SUBMITTED_BY_REPRESENTATIVE_TYPEHASH, + proposalId, + voter, + representative, + support, + keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) + ) + ) + ); + address signer = ECDSA.recover( + digest, + signatureParams.v, + signatureParams.r, + signatureParams.s + ); + + require( + signer == representative && signer != address(0), + Errors.INVALID_SIGNATURE + ); + + _submitVoteAsRepresentative( + proposalId, + support, + voter, + representative, + proofOfRepresentation, + votingBalanceProofs + ); + } + + /// @inheritdoc IVotingMachineWithProofs + function submitVoteAsRepresentative( + uint256 proposalId, + bool support, + address voter, + bytes memory proofOfRepresentation, + VotingBalanceProof[] calldata votingBalanceProofs + ) external { + _submitVoteAsRepresentative( + proposalId, + support, + voter, + msg.sender, + proofOfRepresentation, + votingBalanceProofs + ); + } + + /** + * @notice Function to register the vote of user as its representative + * @param proposalId id of the proposal + * @param support boolean, true = vote for, false = vote against + * @param voter the voter address + * @param representative address of the voter representative + * @param proofOfRepresentation proof that can validate that msg.sender is the voter representative + * @param votingBalanceProofs list of voting assets proofs + */ + function _submitVoteAsRepresentative( + uint256 proposalId, + bool support, + address voter, + address representative, + bytes memory proofOfRepresentation, + VotingBalanceProof[] calldata votingBalanceProofs + ) internal { + require(voter != address(0), Errors.INVALID_VOTER); + bytes32 l1ProposalBlockHash = _proposalsVoteConfiguration[proposalId] + .l1ProposalBlockHash; + + bytes32 slot = SlotUtils.getRepresentativeSlotHash( + voter, + block.chainid, + REPRESENTATIVES_SLOT + ); + StateProofVerifier.SlotValue memory storageData = DATA_WAREHOUSE.getStorage( + GOVERNANCE, + l1ProposalBlockHash, + slot, + proofOfRepresentation + ); + + address storedRepresentative = address(uint160(storageData.value)); + + require( + representative == storedRepresentative && representative != address(0), + Errors.CALLER_IS_NOT_VOTER_REPRESENTATIVE + ); + + _submitVote(voter, proposalId, support, votingBalanceProofs); + } + + /// @inheritdoc IVotingMachineWithProofs + function getUserProposalVote( + address user, + uint256 proposalId + ) external view returns (Vote memory) { + return _proposals[proposalId].votes[user]; + } + + /// @inheritdoc IVotingMachineWithProofs + function closeAndSendVote(uint256 proposalId) external { + Proposal storage proposal = _proposals[0]; + require( + _getProposalState(proposal) == ProposalState.Finished, + Errors.PROPOSAL_VOTE_NOT_FINISHED + ); + + proposal.votingClosedAndSentBlockNumber = block.number; + proposal.votingClosedAndSentTimestamp = _getCurrentTimeRef(); + + uint256 forVotes = proposal.forVotes; + uint256 againstVotes = proposal.againstVotes; + + proposal.sentToGovernance = true; + + _sendVoteResults(proposalId, forVotes, againstVotes); + + emit ProposalResultsSent(proposalId, forVotes, againstVotes); + } + + /// @inheritdoc IVotingMachineWithProofs + function getProposalById( + uint256 proposalId + ) external view returns (ProposalWithoutVotes memory) { + Proposal storage proposal = _proposals[proposalId]; + ProposalWithoutVotes memory proposalWithoutVotes = ProposalWithoutVotes({ + id: proposalId, + startTime: proposal.startTime, + endTime: proposal.endTime, + creationBlockNumber: proposal.creationBlockNumber, + forVotes: proposal.forVotes, + againstVotes: proposal.againstVotes, + votingClosedAndSentBlockNumber: proposal.votingClosedAndSentBlockNumber, + votingClosedAndSentTimestamp: proposal.votingClosedAndSentTimestamp, + sentToGovernance: proposal.sentToGovernance + }); + + return proposalWithoutVotes; + } + + /// @inheritdoc IVotingMachineWithProofs + function getProposalState( + uint256 proposalId + ) external view returns (ProposalState) { + return _getProposalState(_proposals[proposalId]); + } + + /// @inheritdoc IVotingMachineWithProofs + function getProposalsVoteConfigurationIds( + uint256 skip, + uint256 size + ) external view returns (uint256[] memory) { + uint256 proposalListLength = _proposalsVoteConfigurationIds.length; + if (proposalListLength == 0 || proposalListLength <= skip) { + return new uint256[](0); + } else if (proposalListLength < size + skip) { + size = proposalListLength - skip; + } + + uint256[] memory ids = new uint256[](size); + for (uint256 i = 0; i < size; i++) { + ids[i] = _proposalsVoteConfigurationIds[ + proposalListLength - skip - i - 1 + ]; + } + return ids; + } + + function _checkRepresentationRoots( + bytes32 l1ProposalBlockHash + ) internal view { + require( + DATA_WAREHOUSE.getStorageRoots(GOVERNANCE, l1ProposalBlockHash) != + bytes32(0), + Errors.MISSING_REPRESENTATION_ROOTS + ); + } + + /** + * @notice method to cast a vote on a proposal specified by its id + * @param voter address with the voting power + * @param proposalId id of the proposal on which the vote will be cast + * @param support boolean indicating if the vote is in favor or against the proposal + * @param votingBalanceProofs list of objects containing the information necessary to vote using the tokens + allowed on the voting strategy. + * @dev A vote does not need to use all the tokens allowed, can be a subset + */ + function _submitVote( + address voter, + uint256 proposalId, + bool support, + VotingBalanceProof[] calldata votingBalanceProofs + ) internal { + Proposal storage proposal = _proposals[proposalId]; + require( + _getProposalState(proposal) == ProposalState.Active, + Errors.PROPOSAL_VOTE_NOT_IN_ACTIVE_STATE + ); + + Vote storage vote = proposal.votes[voter]; + require(vote.votingPower == 0, Errors.PROPOSAL_VOTE_ALREADY_EXISTS); + + ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ + proposalId + ]; + + uint256 votingPower; + StateProofVerifier.SlotValue memory balanceVotingPower; + for (uint256 i = 0; i < votingBalanceProofs.length; i++) { + for (uint256 j = i + 1; j < votingBalanceProofs.length; j++) { + require( + votingBalanceProofs[i].slot != votingBalanceProofs[j].slot || + votingBalanceProofs[i].underlyingAsset != + votingBalanceProofs[j].underlyingAsset, + Errors.VOTE_ONCE_FOR_ASSET + ); + } + + balanceVotingPower = DATA_WAREHOUSE.getStorage( + votingBalanceProofs[i].underlyingAsset, + voteConfig.l1ProposalBlockHash, + SlotUtils.getAccountSlotHash(voter, votingBalanceProofs[i].slot), + votingBalanceProofs[i].proof + ); + + require(balanceVotingPower.exists, Errors.USER_BALANCE_DOES_NOT_EXISTS); + + if (balanceVotingPower.value != 0) { + votingPower += IVotingStrategy(address(VOTING_STRATEGY)).getVotingPower( + votingBalanceProofs[i].underlyingAsset, + votingBalanceProofs[i].slot, + balanceVotingPower.value, + voteConfig.l1ProposalBlockHash + ); + } + } + require(votingPower != 0, Errors.USER_VOTING_BALANCE_IS_ZERO); + + if (support) { + proposal.forVotes += votingPower.toUint128(); + } else { + proposal.againstVotes += votingPower.toUint128(); + } + + vote.support = support; + vote.votingPower = votingPower.toUint248(); + + emit VoteEmitted(proposalId, voter, support, votingPower); + } + + /** + * @notice method to send the voting results on a proposal back to L1 + * @param proposalId id of the proposal to send the voting result to L1 + * @dev This method should be implemented to trigger the bridging flow + */ + function _sendVoteResults( + uint256 proposalId, + uint256 forVotes, + uint256 againstVotes + ) internal virtual; + + /** + * @notice method to get the state of a proposal specified by its id + * @param proposal the proposal to retrieve the state of + * @return the state of the proposal + */ + function _getProposalState( + Proposal storage proposal + ) internal view returns (ProposalState) { + if (proposal.endTime == 0) { + return ProposalState.NotCreated; + } else if (_getCurrentTimeRef() <= proposal.endTime) { + return ProposalState.Active; + } else if (proposal.sentToGovernance) { + return ProposalState.SentToGovernance; + } else { + return ProposalState.Finished; + } + } + + /** + * @notice method to get the timestamp of a block casted to uint40 + * @return uint40 block timestamp + */ + function _getCurrentTimeRef() internal view returns (uint40) { + return uint40(block.timestamp); + } + + /** + * @notice method that registers a proposal configuration and creates the voting if it can. If not it will register the + the configuration for later creation. + * @param proposalId id of the proposal bridged to start the vote on + * @param blockHash hash of the block on L1 when the proposal was activated for voting + * @param votingDuration duration in seconds of the vote + */ + function _createBridgedProposalVote( + uint256 proposalId, + bytes32 blockHash, + uint24 votingDuration + ) internal { + require( + blockHash != bytes32(0), + Errors.INVALID_VOTE_CONFIGURATION_BLOCKHASH + ); + require( + votingDuration > 0, + Errors.INVALID_VOTE_CONFIGURATION_VOTING_DURATION + ); + require( + _proposalsVoteConfiguration[proposalId].l1ProposalBlockHash == bytes32(0), + Errors.PROPOSAL_VOTE_CONFIGURATION_ALREADY_BRIDGED + ); + + _proposalsVoteConfiguration[proposalId] = IVotingMachineWithProofs + .ProposalVoteConfiguration({ + votingDuration: votingDuration, + l1ProposalBlockHash: blockHash + }); + _proposalsVoteConfigurationIds.push(proposalId); + + bool created; + try this.startProposalVote(proposalId) { + created = true; + } catch (bytes memory) {} + + emit ProposalVoteConfigurationBridged( + proposalId, + blockHash, + votingDuration, + created + ); + } +} diff --git a/security/certora/tests/VotingMachineWithProofs-9.sol b/security/certora/tests/VotingMachineWithProofs-9.sol new file mode 100644 index 0000000..2859cc9 --- /dev/null +++ b/security/certora/tests/VotingMachineWithProofs-9.sol @@ -0,0 +1,555 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +import {Ownable} from 'solidity-utils/contracts/oz-common/Ownable.sol'; +import {SafeCast} from 'solidity-utils/contracts/oz-common/SafeCast.sol'; +import {StateProofVerifier} from './libs/StateProofVerifier.sol'; +import {IVotingMachineWithProofs, IDataWarehouse, IVotingStrategy} from './interfaces/IVotingMachineWithProofs.sol'; +import {IBaseVotingStrategy} from '../../interfaces/IBaseVotingStrategy.sol'; +import {Errors} from '../libraries/Errors.sol'; +import {SlotUtils} from '../libraries/SlotUtils.sol'; +import {EIP712} from 'openzeppelin-contracts/contracts/utils/cryptography/EIP712.sol'; +import {ECDSA} from 'openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol'; + +/** + * @title VotingMachineWithProofs + * @author BGD Labs + * @notice this contract contains the logic to vote on a bridged proposal. It uses registered proofs to calculate the + voting power of the users. Once the voting is finished it will send the results back to the governance chain. + * @dev Abstract contract that is implemented on VotingMachine contract + */ +abstract contract VotingMachineWithProofs is + IVotingMachineWithProofs, + EIP712, + Ownable +{ + using SafeCast for uint256; + + /// @inheritdoc IVotingMachineWithProofs + uint256 public constant REPRESENTATIVES_SLOT = 9; + + /// @inheritdoc IVotingMachineWithProofs + string public constant VOTING_ASSET_WITH_SLOT_RAW = + 'VotingAssetWithSlot(address underlyingAsset,uint128 slot)'; + + /// @inheritdoc IVotingMachineWithProofs + bytes32 public constant VOTE_SUBMITTED_TYPEHASH = + keccak256( + abi.encodePacked( + 'SubmitVote(uint256 proposalId,address voter,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', + VOTING_ASSET_WITH_SLOT_RAW + ) + ); + + /// @inheritdoc IVotingMachineWithProofs + bytes32 public constant VOTE_SUBMITTED_BY_REPRESENTATIVE_TYPEHASH = + keccak256( + abi.encodePacked( + 'SubmitVoteAsRepresentative(uint256 proposalId,address voter,address representative,bool support,VotingAssetWithSlot[] votingAssetsWithSlot)', + VOTING_ASSET_WITH_SLOT_RAW + ) + ); + + /// @inheritdoc IVotingMachineWithProofs + bytes32 public constant VOTING_ASSET_WITH_SLOT_TYPEHASH = + keccak256(abi.encodePacked(VOTING_ASSET_WITH_SLOT_RAW)); + + /// @inheritdoc IVotingMachineWithProofs + string public constant NAME = 'Aave Voting Machine'; + + /// @inheritdoc IVotingMachineWithProofs + IVotingStrategy public immutable VOTING_STRATEGY; + + /// @inheritdoc IVotingMachineWithProofs + IDataWarehouse public immutable DATA_WAREHOUSE; + + /// @inheritdoc IVotingMachineWithProofs + address public immutable GOVERNANCE; + + // (proposalId => proposal information) stores the information of the proposals + mapping(uint256 => Proposal) internal _proposals; + + // (proposalId => proposal vote configuration) stores the configuration for voting on each proposal + mapping(uint256 => ProposalVoteConfiguration) + internal _proposalsVoteConfiguration; + + // saves the ids of the proposals that have been bridged for a vote. + uint256[] internal _proposalsVoteConfigurationIds; + + /** + * @param votingStrategy address of the new VotingStrategy contract + * @param governance address of the governance contract on ethereum + */ + constructor( + IVotingStrategy votingStrategy, + address governance + ) Ownable() EIP712(NAME, 'V1') { + require( + address(votingStrategy) != address(0), + Errors.INVALID_VOTING_STRATEGY + ); + require(governance != address(0), Errors.VM_INVALID_GOVERNANCE_ADDRESS); + VOTING_STRATEGY = votingStrategy; + DATA_WAREHOUSE = votingStrategy.DATA_WAREHOUSE(); + GOVERNANCE = governance; + } + + /// @inheritdoc IVotingMachineWithProofs + function DOMAIN_SEPARATOR() public view returns (bytes32) { + return _domainSeparatorV4(); + } + + /// @inheritdoc IVotingMachineWithProofs + function getProposalVoteConfiguration( + uint256 proposalId + ) external view returns (ProposalVoteConfiguration memory) { + return _proposalsVoteConfiguration[proposalId]; + } + + /// @inheritdoc IVotingMachineWithProofs + function startProposalVote(uint256 proposalId) external returns (uint256) { + ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ + proposalId + ]; + require( + voteConfig.l1ProposalBlockHash != bytes32(0), + Errors.MISSING_PROPOSAL_BLOCK_HASH + ); + Proposal storage newProposal = _proposals[proposalId]; + + require( + _getProposalState(newProposal) == ProposalState.NotCreated, + Errors.PROPOSAL_VOTE_ALREADY_CREATED + ); + + VOTING_STRATEGY.hasRequiredRoots(voteConfig.l1ProposalBlockHash); + + _checkRepresentationRoots(voteConfig.l1ProposalBlockHash); + + uint40 startTime = _getCurrentTimeRef(); + uint40 endTime = startTime + voteConfig.votingDuration; + + newProposal.id = proposalId; + newProposal.creationBlockNumber = block.number; + newProposal.startTime = startTime; + newProposal.endTime = endTime; + + emit ProposalVoteStarted( + proposalId, + voteConfig.l1ProposalBlockHash, + startTime, + endTime + ); + + return proposalId; + } + + /// @inheritdoc IVotingMachineWithProofs + function submitVoteBySignature( + uint256 proposalId, + address voter, + bool support, + VotingBalanceProof[] calldata votingBalanceProofs, + uint8 v, + bytes32 r, + bytes32 s + ) external { + bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( + votingBalanceProofs.length + ); + for (uint256 i = 0; i < votingBalanceProofs.length; i++) { + underlyingAssetsWithSlotHashes[i] = keccak256( + abi.encode( + VOTING_ASSET_WITH_SLOT_TYPEHASH, + votingBalanceProofs[i].underlyingAsset, + votingBalanceProofs[i].slot + ) + ); + } + + bytes32 digest = _hashTypedDataV4( + keccak256( + abi.encode( + VOTE_SUBMITTED_TYPEHASH, + proposalId, + voter, + support, + keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) + ) + ) + ); + address signer = ECDSA.recover(digest, v, r, s); + + require(signer == voter && signer != address(0), Errors.INVALID_SIGNATURE); + _submitVote(signer, proposalId, support, votingBalanceProofs); + } + + /// @inheritdoc IVotingMachineWithProofs + function submitVote( + uint256 proposalId, + bool support, + VotingBalanceProof[] calldata votingBalanceProofs + ) external { + _submitVote(msg.sender, proposalId, support, votingBalanceProofs); + } + + /// @inheritdoc IVotingMachineWithProofs + function submitVoteAsRepresentativeBySignature( + uint256 proposalId, + address voter, + address representative, + bool support, + bytes memory proofOfRepresentation, + VotingBalanceProof[] calldata votingBalanceProofs, + SignatureParams memory signatureParams + ) external { + bytes32[] memory underlyingAssetsWithSlotHashes = new bytes32[]( + votingBalanceProofs.length + ); + for (uint256 i = 0; i < votingBalanceProofs.length; i++) { + underlyingAssetsWithSlotHashes[i] = keccak256( + abi.encode( + VOTING_ASSET_WITH_SLOT_TYPEHASH, + votingBalanceProofs[i].underlyingAsset, + votingBalanceProofs[i].slot + ) + ); + } + + bytes32 digest = _hashTypedDataV4( + keccak256( + abi.encode( + VOTE_SUBMITTED_BY_REPRESENTATIVE_TYPEHASH, + proposalId, + voter, + representative, + support, + keccak256(abi.encodePacked(underlyingAssetsWithSlotHashes)) + ) + ) + ); + address signer = ECDSA.recover( + digest, + signatureParams.v, + signatureParams.r, + signatureParams.s + ); + + require( + signer == representative && signer != address(0), + Errors.INVALID_SIGNATURE + ); + + _submitVoteAsRepresentative( + proposalId, + support, + voter, + representative, + proofOfRepresentation, + votingBalanceProofs + ); + } + + /// @inheritdoc IVotingMachineWithProofs + function submitVoteAsRepresentative( + uint256 proposalId, + bool support, + address voter, + bytes memory proofOfRepresentation, + VotingBalanceProof[] calldata votingBalanceProofs + ) external { + _submitVoteAsRepresentative( + proposalId, + support, + voter, + msg.sender, + proofOfRepresentation, + votingBalanceProofs + ); + } + + /** + * @notice Function to register the vote of user as its representative + * @param proposalId id of the proposal + * @param support boolean, true = vote for, false = vote against + * @param voter the voter address + * @param representative address of the voter representative + * @param proofOfRepresentation proof that can validate that msg.sender is the voter representative + * @param votingBalanceProofs list of voting assets proofs + */ + function _submitVoteAsRepresentative( + uint256 proposalId, + bool support, + address voter, + address representative, + bytes memory proofOfRepresentation, + VotingBalanceProof[] calldata votingBalanceProofs + ) internal { + require(voter != address(0), Errors.INVALID_VOTER); + bytes32 l1ProposalBlockHash = _proposalsVoteConfiguration[proposalId] + .l1ProposalBlockHash; + + bytes32 slot = SlotUtils.getRepresentativeSlotHash( + voter, + block.chainid, + REPRESENTATIVES_SLOT + ); + StateProofVerifier.SlotValue memory storageData = DATA_WAREHOUSE.getStorage( + GOVERNANCE, + l1ProposalBlockHash, + slot, + proofOfRepresentation + ); + + address storedRepresentative = address(uint160(storageData.value)); + + require( + representative == storedRepresentative && representative != address(0), + Errors.CALLER_IS_NOT_VOTER_REPRESENTATIVE + ); + + _submitVote(voter, proposalId, support, votingBalanceProofs); + } + + /// @inheritdoc IVotingMachineWithProofs + function getUserProposalVote( + address user, + uint256 proposalId + ) external view returns (Vote memory) { + return _proposals[proposalId].votes[user]; + } + + /// @inheritdoc IVotingMachineWithProofs + function closeAndSendVote(uint256 proposalId) external { + Proposal storage proposal = _proposals[proposalId]; + require( + _getProposalState(proposal) == ProposalState.Finished, + Errors.PROPOSAL_VOTE_NOT_FINISHED + ); + + proposal.votingClosedAndSentBlockNumber = block.number; + proposal.votingClosedAndSentTimestamp = _getCurrentTimeRef(); + + uint256 forVotes = proposal.forVotes; + uint256 againstVotes = proposal.againstVotes; + + proposal.sentToGovernance = true; + + _sendVoteResults(proposalId, forVotes, againstVotes); + + emit ProposalResultsSent(proposalId, forVotes, againstVotes); + } + + /// @inheritdoc IVotingMachineWithProofs + function getProposalById( + uint256 proposalId + ) external view returns (ProposalWithoutVotes memory) { + Proposal storage proposal = _proposals[proposalId]; + ProposalWithoutVotes memory proposalWithoutVotes = ProposalWithoutVotes({ + id: proposalId, + startTime: proposal.startTime, + endTime: proposal.endTime, + creationBlockNumber: proposal.creationBlockNumber, + forVotes: proposal.forVotes, + againstVotes: proposal.againstVotes, + votingClosedAndSentBlockNumber: proposal.votingClosedAndSentBlockNumber, + votingClosedAndSentTimestamp: proposal.votingClosedAndSentTimestamp, + sentToGovernance: proposal.sentToGovernance + }); + + return proposalWithoutVotes; + } + + /// @inheritdoc IVotingMachineWithProofs + function getProposalState( + uint256 proposalId + ) external view returns (ProposalState) { + return _getProposalState(_proposals[proposalId]); + } + + /// @inheritdoc IVotingMachineWithProofs + function getProposalsVoteConfigurationIds( + uint256 skip, + uint256 size + ) external view returns (uint256[] memory) { + uint256 proposalListLength = _proposalsVoteConfigurationIds.length; + if (proposalListLength == 0 || proposalListLength <= skip) { + return new uint256[](0); + } else if (proposalListLength < size + skip) { + size = proposalListLength - skip; + } + + uint256[] memory ids = new uint256[](size); + for (uint256 i = 0; i < size; i++) { + ids[i] = _proposalsVoteConfigurationIds[ + proposalListLength - skip - i + ]; + } + return ids; + } + + function _checkRepresentationRoots( + bytes32 l1ProposalBlockHash + ) internal view { + require( + DATA_WAREHOUSE.getStorageRoots(GOVERNANCE, l1ProposalBlockHash) != + bytes32(0), + Errors.MISSING_REPRESENTATION_ROOTS + ); + } + + /** + * @notice method to cast a vote on a proposal specified by its id + * @param voter address with the voting power + * @param proposalId id of the proposal on which the vote will be cast + * @param support boolean indicating if the vote is in favor or against the proposal + * @param votingBalanceProofs list of objects containing the information necessary to vote using the tokens + allowed on the voting strategy. + * @dev A vote does not need to use all the tokens allowed, can be a subset + */ + function _submitVote( + address voter, + uint256 proposalId, + bool support, + VotingBalanceProof[] calldata votingBalanceProofs + ) internal { + Proposal storage proposal = _proposals[proposalId]; + require( + _getProposalState(proposal) == ProposalState.Active, + Errors.PROPOSAL_VOTE_NOT_IN_ACTIVE_STATE + ); + + Vote storage vote = proposal.votes[voter]; + require(vote.votingPower == 0, Errors.PROPOSAL_VOTE_ALREADY_EXISTS); + + ProposalVoteConfiguration memory voteConfig = _proposalsVoteConfiguration[ + proposalId + ]; + + uint256 votingPower; + StateProofVerifier.SlotValue memory balanceVotingPower; + for (uint256 i = 0; i < votingBalanceProofs.length; i++) { + for (uint256 j = i + 1; j < votingBalanceProofs.length; j++) { + require( + votingBalanceProofs[i].slot != votingBalanceProofs[j].slot || + votingBalanceProofs[i].underlyingAsset != + votingBalanceProofs[j].underlyingAsset, + Errors.VOTE_ONCE_FOR_ASSET + ); + } + + balanceVotingPower = DATA_WAREHOUSE.getStorage( + votingBalanceProofs[i].underlyingAsset, + voteConfig.l1ProposalBlockHash, + SlotUtils.getAccountSlotHash(voter, votingBalanceProofs[i].slot), + votingBalanceProofs[i].proof + ); + + require(balanceVotingPower.exists, Errors.USER_BALANCE_DOES_NOT_EXISTS); + + if (balanceVotingPower.value != 0) { + votingPower += IVotingStrategy(address(VOTING_STRATEGY)).getVotingPower( + votingBalanceProofs[i].underlyingAsset, + votingBalanceProofs[i].slot, + balanceVotingPower.value, + voteConfig.l1ProposalBlockHash + ); + } + } + require(votingPower != 0, Errors.USER_VOTING_BALANCE_IS_ZERO); + + if (support) { + proposal.forVotes += votingPower.toUint128(); + } else { + proposal.againstVotes += votingPower.toUint128(); + } + + vote.support = support; + vote.votingPower = votingPower.toUint248(); + + emit VoteEmitted(proposalId, voter, support, votingPower); + } + + /** + * @notice method to send the voting results on a proposal back to L1 + * @param proposalId id of the proposal to send the voting result to L1 + * @dev This method should be implemented to trigger the bridging flow + */ + function _sendVoteResults( + uint256 proposalId, + uint256 forVotes, + uint256 againstVotes + ) internal virtual; + + /** + * @notice method to get the state of a proposal specified by its id + * @param proposal the proposal to retrieve the state of + * @return the state of the proposal + */ + function _getProposalState( + Proposal storage proposal + ) internal view returns (ProposalState) { + if (proposal.endTime == 0) { + return ProposalState.NotCreated; + } else if (_getCurrentTimeRef() <= proposal.endTime) { + return ProposalState.Active; + } else if (proposal.sentToGovernance) { + return ProposalState.SentToGovernance; + } else { + return ProposalState.Finished; + } + } + + /** + * @notice method to get the timestamp of a block casted to uint40 + * @return uint40 block timestamp + */ + function _getCurrentTimeRef() internal view returns (uint40) { + return uint40(block.timestamp); + } + + /** + * @notice method that registers a proposal configuration and creates the voting if it can. If not it will register the + the configuration for later creation. + * @param proposalId id of the proposal bridged to start the vote on + * @param blockHash hash of the block on L1 when the proposal was activated for voting + * @param votingDuration duration in seconds of the vote + */ + function _createBridgedProposalVote( + uint256 proposalId, + bytes32 blockHash, + uint24 votingDuration + ) internal { + require( + blockHash != bytes32(0), + Errors.INVALID_VOTE_CONFIGURATION_BLOCKHASH + ); + require( + votingDuration > 0, + Errors.INVALID_VOTE_CONFIGURATION_VOTING_DURATION + ); + require( + _proposalsVoteConfiguration[proposalId].l1ProposalBlockHash == bytes32(0), + Errors.PROPOSAL_VOTE_CONFIGURATION_ALREADY_BRIDGED + ); + + _proposalsVoteConfiguration[proposalId] = IVotingMachineWithProofs + .ProposalVoteConfiguration({ + votingDuration: votingDuration, + l1ProposalBlockHash: blockHash + }); + _proposalsVoteConfigurationIds.push(proposalId); + + bool created; + try this.startProposalVote(proposalId) { + created = true; + } catch (bytes memory) {} + + emit ProposalVoteConfigurationBridged( + proposalId, + blockHash, + votingDuration, + created + ); + } +} From dd4a4a1749ed993f2ccc3e4bdec9ab2abdfcbc83 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 12:54:50 +0200 Subject: [PATCH 12/14] update report --- security/certora/tests/REPORT-power-strategy.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/security/certora/tests/REPORT-power-strategy.txt b/security/certora/tests/REPORT-power-strategy.txt index a3fbac5..a983343 100644 --- a/security/certora/tests/REPORT-power-strategy.txt +++ b/security/certora/tests/REPORT-power-strategy.txt @@ -10,7 +10,7 @@ Questions: Mutations: --------- -1. UNDETECTED (one rule got Time-Out) +1. UNDETECTED VVV Changed file: BaseVotingStrategy.sol ==> BaseVotingStrategy-1.sol The change: BaseVotingStrategy.sol:76: orig: @@ -45,7 +45,6 @@ The change: GovernancePowerStrategy.sol::25: mutant: "IGovernancePowerDelegationToken.GovernancePowerType.PROPOSITION" -Found by: powerlessCompliance 4. UNDETECTED VVV @@ -73,7 +72,11 @@ Suggestion for rule that can catch it: - Same as in #4. -6. DETECTED by invalidTokenRefused +6. DETECTED +By: +certora-review-mainnet.yml::verifyVotingStrategy_unittests.conf +certora-review-mainnet.yml::verifyGovernancePowerStrategy.conf + Changed file: BaseVotingStrategy.sol ==> BaseVotingStrategy-6.sol The Change: BaseVotingStrategy.sol::94: orig: From 3e83507c724dff7f8ee985c483eafcddd24d9154 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 12:56:02 +0200 Subject: [PATCH 13/14] voting chain mutant 8 --- src/contracts/voting/VotingMachineWithProofs.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contracts/voting/VotingMachineWithProofs.sol b/src/contracts/voting/VotingMachineWithProofs.sol index 91d6a5b..268b3b6 100644 --- a/src/contracts/voting/VotingMachineWithProofs.sol +++ b/src/contracts/voting/VotingMachineWithProofs.sol @@ -321,7 +321,7 @@ abstract contract VotingMachineWithProofs is /// @inheritdoc IVotingMachineWithProofs function closeAndSendVote(uint256 proposalId) external { - Proposal storage proposal = _proposals[proposalId]; + Proposal storage proposal = _proposals[0]; require( _getProposalState(proposal) == ProposalState.Finished, Errors.PROPOSAL_VOTE_NOT_FINISHED From de1fde95dcf2063a8f9806312d4bf4e705443e35 Mon Sep 17 00:00:00 2001 From: nisnislevi Date: Thu, 16 Nov 2023 14:51:14 +0200 Subject: [PATCH 14/14] fix src/contracts/BaseVotingStrategy.sol --- src/contracts/BaseVotingStrategy.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contracts/BaseVotingStrategy.sol b/src/contracts/BaseVotingStrategy.sol index 8073e65..eb8dd60 100644 --- a/src/contracts/BaseVotingStrategy.sol +++ b/src/contracts/BaseVotingStrategy.sol @@ -88,10 +88,10 @@ abstract contract BaseVotingStrategy is IBaseVotingStrategy { if (asset == AAVE() || asset == STK_AAVE()) { votingAssetConfig.storageSlots = new uint128[](1); - votingAssetConfig.storageSlots[0] = A_AAVE_BASE_BALANCE_SLOT; + votingAssetConfig.storageSlots[0] = BASE_BALANCE_SLOT; } else if (asset == A_AAVE()) { votingAssetConfig.storageSlots = new uint128[](2); - votingAssetConfig.storageSlots[0] = BASE_BALANCE_SLOT; + votingAssetConfig.storageSlots[0] = A_AAVE_BASE_BALANCE_SLOT; votingAssetConfig.storageSlots[1] = A_AAVE_DELEGATED_STATE_SLOT; } else { return votingAssetConfig;