Olympus DAO contest - aviggiano's results

[1]

• Important address changes should follow a "propose" pattern instead of directly setting variables in order to avoid human errors.
• Missing validation on address changes.

e.g. Kernel.executeAction(Actions,address).target_ on Actions.ChangeExecutor and Actions.ChangeAdmin:

Before

        } else if (action_ == Actions.ChangeExecutor) {
            executor = target_;
        } else if (action_ == Actions.ChangeAdmin) {
            admin = target_;
        }

After

        } else if (action_ == Actions.ChangeExecutor) {
            if(target_ == address(0)) revert Kernel_InvalidAddress();
            proposedExecutor = target_;
        } else if (action_ == Actions.ChangeAdmin) {
            if(target_ == address(0)) revert Kernel_InvalidAddress();
            proposedAdmin = target_;
        }

        // ...

        function acceptProposedExecutor() external {
            if(msg.sender != proposedExecutor) revert Kernel_InvalidExecutor();
            executor = proposedExecutor;
        }
        function acceptProposedAdmin() external {
            if(msg.sender != proposedAdmin) revert Kernel_InvalidAdmin();
            admin = proposedAdmin;
        }

[2] Missing RewardUpdated event on constructor from Heart.sol

Before

    constructor(
        Kernel kernel_,
        IOperator operator_,
        ERC20 rewardToken_,
        uint256 reward_
    ) Policy(kernel_) {
        _operator = operator_;

        active = true;
        lastBeat = block.timestamp;
        rewardToken = rewardToken_;
        reward = reward_;
    }

After

    constructor(
        Kernel kernel_,
        IOperator operator_,
        ERC20 rewardToken_,
        uint256 reward_
    ) Policy(kernel_) {
        _operator = operator_;

        active = true;
        lastBeat = block.timestamp;
        rewardToken = rewardToken_;
        reward = reward_;
        emit RewardUpdated(rewardToken_; reward_);
    }

Gas optimization in src/policies/Governance.sol function endorseProposal(uint256 proposalId_)

Before

        // undo any previous endorsement the user made on these instructions
        uint256 previousEndorsement = userEndorsementsForProposal[proposalId_][msg.sender];
        totalEndorsementsForProposal[proposalId_] -= previousEndorsement;

        // reapply user endorsements with most up-to-date votes
        userEndorsementsForProposal[proposalId_][msg.sender] = userVotes;
        totalEndorsementsForProposal[proposalId_] += userVotes;

// gas reporter
│ endorseProposal                                        ┆ 6874            ┆ 39015  ┆ 30774  ┆ 52674  ┆ 43      │

After

        // undo any previous endorsement the user made on these instructions
        uint256 previousEndorsement = userEndorsementsForProposal[proposalId_][msg.sender];

        // reapply user endorsements with most up-to-date votes
        userEndorsementsForProposal[proposalId_][msg.sender] = userVotes;
        totalEndorsementsForProposal[proposalId_] += userVotes - previousEndorsement;

// gas reporter
│ endorseProposal                                        ┆ 6449            ┆ 38610  ┆ 30349  ┆ 52249  ┆ 43      │