Skip to content

Commit

Permalink
Merge pull request #300 from AugustoL/fix-dao-final-audit-report
Browse files Browse the repository at this point in the history 
Fixes DAO Final Audit Report
  • Loading branch information
@AugustoL
AugustoL authored Mar 6, 2023
2 parents 1ceeadc + deec1aa commit dc3e2ec
Show file tree
Hide file tree
Showing 28 changed files with 2,510 additions and 1,836 deletions.
45 changes: 12 additions & 33 deletions contracts/dao/DAOController.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ contract DAOController is Initializable {

struct Scheme {
bytes32 paramsHash; // a hash voting parameters of the scheme
bool isRegistered;
bool canManageSchemes;
bool canMakeAvatarCalls;
bool canChangeReputation;
Expand Down Expand Up @@ -61,19 +60,15 @@ contract DAOController is Initializable {
/// @notice Cannot unregister last scheme with manage schemes permission
error DAOController__CannotUnregisterLastSchemeWithManageSchemesPermission();

/// @notice Cannot register a scheme with paramsHash 0
error DAOController__CannotRegisterSchemeWithNullParamsHash();

/// @notice Sender is not the scheme that originally started the proposal
error DAOController__SenderIsNotTheProposer();

/// @notice Sender is not a registered scheme or proposal is not active
error DAOController__SenderIsNotRegisteredOrProposalIsInactive();

/// @dev Verify if scheme is registered
modifier onlyRegisteredScheme() {
if (!schemes[msg.sender].isRegistered) {
revert DAOController__SenderNotRegistered();
}
_;
}
/// @dev Verify if scheme can manage schemes
modifier onlyRegisteringSchemes() {
if (!schemes[msg.sender].canManageSchemes) {
Expand Down Expand Up @@ -111,7 +106,6 @@ contract DAOController is Initializable {
) public initializer {
schemes[scheme] = Scheme({
paramsHash: paramsHash,
isRegistered: true,
canManageSchemes: true,
canMakeAvatarCalls: true,
canChangeReputation: true
Expand All @@ -135,11 +129,15 @@ contract DAOController is Initializable {
bool canManageSchemes,
bool canMakeAvatarCalls,
bool canChangeReputation
) external onlyRegisteredScheme onlyRegisteringSchemes returns (bool success) {
) external onlyRegisteringSchemes returns (bool success) {
Scheme memory scheme = schemes[schemeAddress];

if (paramsHash == bytes32(0)) {
revert DAOController__CannotRegisterSchemeWithNullParamsHash();
}

// Add or change the scheme:
if ((!scheme.isRegistered || !scheme.canManageSchemes) && canManageSchemes) {
if (!scheme.canManageSchemes && canManageSchemes) {
schemesWithManageSchemesPermission = schemesWithManageSchemesPermission + 1;
} else if (scheme.canManageSchemes && !canManageSchemes) {
if (schemesWithManageSchemesPermission <= 1) {
Expand All @@ -150,7 +148,6 @@ contract DAOController is Initializable {

schemes[schemeAddress] = Scheme({
paramsHash: paramsHash,
isRegistered: true,
canManageSchemes: canManageSchemes,
canMakeAvatarCalls: canMakeAvatarCalls,
canChangeReputation: canChangeReputation
Expand All @@ -166,16 +163,11 @@ contract DAOController is Initializable {
* @param schemeAddress The address of the scheme to unregister/delete from `schemes` mapping
* @return success Success of the operation
*/
function unregisterScheme(address schemeAddress)
external
onlyRegisteredScheme
onlyRegisteringSchemes
returns (bool success)
{
function unregisterScheme(address schemeAddress) external onlyRegisteringSchemes returns (bool success) {
Scheme memory scheme = schemes[schemeAddress];

//check if the scheme is registered
if (_isSchemeRegistered(schemeAddress) == false) {
if (scheme.paramsHash == bytes32(0)) {
return false;
}

Expand Down Expand Up @@ -206,7 +198,7 @@ contract DAOController is Initializable {
bytes calldata data,
DAOAvatar avatar,
uint256 value
) external onlyRegisteredScheme onlyAvatarCallScheme returns (bool callSuccess, bytes memory callData) {
) external onlyAvatarCallScheme returns (bool callSuccess, bytes memory callData) {
return avatar.executeCall(to, data, value);
}

Expand Down Expand Up @@ -243,15 +235,6 @@ contract DAOController is Initializable {
reputationToken.transferOwnership(newOwner);
}

/**
* @dev Returns whether a scheme is registered or not
* @param scheme The address of the scheme
* @return isRegistered Whether a scheme is registered or not
*/
function isSchemeRegistered(address scheme) external view returns (bool isRegistered) {
return _isSchemeRegistered(scheme);
}

/**
* @dev Returns scheme paramsHash
* @param scheme The address of the scheme
Expand Down Expand Up @@ -300,10 +283,6 @@ contract DAOController is Initializable {
return schemesWithManageSchemesPermission;
}

function _isSchemeRegistered(address scheme) private view returns (bool) {
return (schemes[scheme].isRegistered);
}

/**
* @dev Function to get reputation token
* @return tokenAddress The reputation token set on controller.initialize
Expand Down
47 changes: 24 additions & 23 deletions contracts/dao/schemes/Scheme.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity ^0.8.17;

import "@openzeppelin/contracts/utils/Address.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "../../utils/PermissionRegistry.sol";
import "../DAOReputation.sol";
import "../DAOAvatar.sol";
Expand All @@ -25,7 +26,7 @@ import "../votingMachine/VotingMachineCallbacks.sol";
* Once the governance process ends on the voting machine the voting machine can execute the proposal winning option.
* If the wining option cant be executed successfully, it can be finished without execution once the maxTimesForExecution time passes.
*/
abstract contract Scheme is VotingMachineCallbacks {
abstract contract Scheme is Initializable, VotingMachineCallbacks {
using Address for address;

enum ProposalState {
Expand Down Expand Up @@ -59,9 +60,6 @@ abstract contract Scheme is VotingMachineCallbacks {

event ProposalStateChange(bytes32 indexed proposalId, uint256 indexed state);

/// @notice Emitted when its initialized twice
error Scheme__CannotInitTwice();

/// @notice Emitted if avatar address is zero
error Scheme__AvatarAddressCannotBeZero();

Expand Down Expand Up @@ -105,11 +103,7 @@ abstract contract Scheme is VotingMachineCallbacks {
address permissionRegistryAddress,
string calldata _schemeName,
uint256 _maxRepPercentageChange
) external {
if (address(avatar) != address(0)) {
revert Scheme__CannotInitTwice();
}

) external initializer {
if (avatarAddress == address(0)) {
revert Scheme__AvatarAddressCannotBeZero();
}
Expand Down Expand Up @@ -176,7 +170,8 @@ abstract contract Scheme is VotingMachineCallbacks {
}

/**
* @dev Execution of proposals, can only be called by the voting machine in which the vote is held.
* @dev Execute the proposal calls for the winning option,
* can only be called by the voting machine in which the vote is held.
* @param proposalId The ID of the voting in the voting machine
* @param winningOption The winning option in the voting machine
* @return success Success of the execution
Expand All @@ -193,13 +188,18 @@ abstract contract Scheme is VotingMachineCallbacks {
}
executingProposal = true;

Proposal memory proposal = proposals[proposalId];
Proposal storage proposal = proposals[proposalId];

if (proposal.state != ProposalState.Submitted) {
revert Scheme__ProposalMustBeSubmitted();
}

if (winningOption > 1) {
if (winningOption == 1) {
proposal.state = ProposalState.Rejected;
emit ProposalStateChange(proposalId, uint256(ProposalState.Rejected));
} else {
proposal.state = ProposalState.Passed;
emit ProposalStateChange(proposalId, uint256(ProposalState.Passed));
uint256 oldRepSupply = getNativeReputationTotalSupply();

permissionRegistry.setERC20Balances();
Expand Down Expand Up @@ -253,7 +253,8 @@ abstract contract Scheme is VotingMachineCallbacks {
}

/**
* @dev Finish a proposal and set the final state in storage
* @dev Finish a proposal and set the final state in storage without any execution.
* The only thing done here is a change in the proposal state in case the proposal was not executed.
* @param proposalId The ID of the voting in the voting machine
* @param winningOption The winning option in the voting machine
* @return success Proposal finish successfully
Expand All @@ -265,18 +266,18 @@ abstract contract Scheme is VotingMachineCallbacks {
returns (bool success)
{
Proposal storage proposal = proposals[proposalId];
if (proposal.state != ProposalState.Submitted) {
revert Scheme__ProposalMustBeSubmitted();
}

if (winningOption == 1) {
proposal.state = ProposalState.Rejected;
emit ProposalStateChange(proposalId, uint256(ProposalState.Rejected));
if (proposal.state == ProposalState.Submitted) {
if (winningOption == 1) {
proposal.state = ProposalState.Rejected;
emit ProposalStateChange(proposalId, uint256(ProposalState.Rejected));
} else {
proposal.state = ProposalState.Passed;
emit ProposalStateChange(proposalId, uint256(ProposalState.Passed));
}
return true;
} else {
proposal.state = ProposalState.Passed;
emit ProposalStateChange(proposalId, uint256(ProposalState.Passed));
return false;
}
return true;
}

/**
Expand Down
Loading

0 comments on commit dc3e2ec

Please sign in to comment.