New Contribution Scheme Analysis Diff. Report

New Contributor Scheme Review
November 23, 2022

Summary

3AC has done a full comparison between the old and new Contribution Scheme contract that controls funds outflows and REP distribution from the DXdao avatar.

The contracts are obtained from their respective code submitted to Etherscan.

Verdict

While the new contract has some additional changes. These cannot be used in the current implementation. Hence, we deem the new contract to be safe. Yet, a better approach would have been to “patch” the existing deployed contract without adding additional Gov 1.5 methods - making it easier to audit and track changes.

File Comparison

The following (24) contracts/libraries exist in the current Contribution Scheme:

// File @daostack/infra/contracts/votingMachines/IntVoteInterface.sol
// File openzeppelin-solidity/contracts/token/ERC20/IERC20.sol
// File @daostack/infra/contracts/votingMachines/VotingMachineCallbacksInterface.sol
// File openzeppelin-solidity/contracts/ownership/Ownable.sol
// File @daostack/infra/contracts/Reputation.sol
// File openzeppelin-solidity/contracts/math/SafeMath.sol
// File openzeppelin-solidity/contracts/token/ERC20/ERC20.sol
// File openzeppelin-solidity/contracts/token/ERC20/ERC20Burnable.sol
// File contracts/controller/DAOToken.sol
// File openzeppelin-solidity/contracts/utils/Address.sol
// File contracts/libs/SafeERC20.sol
// File contracts/controller/Avatar.sol
// File contracts/universalSchemes/UniversalSchemeInterface.sol
// File contracts/globalConstraints/GlobalConstraintInterface.sol
// File contracts/controller/ControllerInterface.sol
// File contracts/universalSchemes/UniversalScheme.sol
// File openzeppelin-solidity/contracts/cryptography/ECDSA.sol
// File @daostack/infra/contracts/libs/RealMath.sol
// File @daostack/infra/contracts/votingMachines/ProposalExecuteInterface.sol
// File openzeppelin-solidity/contracts/math/Math.sol
// File @daostack/infra/contracts/votingMachines/GenesisProtocolLogic.sol
// File @daostack/infra/contracts/votingMachines/GenesisProtocol.sol
// File contracts/votingMachines/VotingMachineCallbacks.sol
// File contracts/universalSchemes/ContributionReward.sol

The following (25) contracts/libraries exist in the new Contribution Scheme:

// File contracts/daostack/votingMachines/IntVoteInterface.sol
// File openzeppelin-solidity/contracts/token/ERC20/IERC20.sol@v2.4.0
// File contracts/daostack/votingMachines/VotingMachineCallbacksInterface.sol
// File openzeppelin-solidity/contracts/GSN/Context.sol@v2.4.0
// File openzeppelin-solidity/contracts/ownership/Ownable.sol@v2.4.0
// File contracts/daostack/controller/Reputation.sol
// File openzeppelin-solidity/contracts/math/SafeMath.sol@v2.4.0
// File openzeppelin-solidity/contracts/token/ERC20/ERC20.sol@v2.4.0
// File openzeppelin-solidity/contracts/token/ERC20/ERC20Burnable.sol@v2.4.0
// File contracts/daostack/controller/DAOToken.sol
// File openzeppelin-solidity/contracts/utils/Address.sol@v2.4.0
// File contracts/daostack/libs/SafeERC20.sol
// File contracts/daostack/controller/Avatar.sol
// File contracts/daostack/universalSchemes/UniversalSchemeInterface.sol
// File contracts/daostack/globalConstraints/GlobalConstraintInterface.sol
// File contracts/daostack/controller/ControllerInterface.sol
// File contracts/daostack/universalSchemes/UniversalScheme.sol
// File contracts/daostack/libs/RealMath.sol
// File contracts/daostack/votingMachines/ProposalExecuteInterface.sol
// File openzeppelin-solidity/contracts/math/Math.sol@v2.4.0
// File contracts/daostack/votingMachines/GenesisProtocolLogic.sol
// File openzeppelin-solidity/contracts/cryptography/ECDSA.sol@v2.4.0
// File contracts/daostack/votingMachines/GenesisProtocol.sol
// File contracts/daostack/votingMachines/VotingMachineCallbacks.sol
// File contracts/daostack/universalSchemes/ContributionReward.sol

The new contract has one extra contract Context from OpenZepplin and is eventually inherited by Ownable from the same package.

Contracts Changed

In all contracts, uint and int have been changed to uint256 and int256 respectively. This is most likely from using a linter such as Solhint.

IERC20

The new contract reorders the methods and adds proper comments.

Reputation

This contract is imported as an interface. The new interface/contract has mintMultiple method. The current deployed Reputation contract does not have the mentioned method. Hence, we can conclude it is safe. However, 3AC also suggests using interfaces instead of importing the entire as a lean way to interact with external contracts.

VotingMachineCallbacksInterface

The new contract updates the compiler version from ^0.5.4 to 0.5.17.

Ownable

The new contract extends Context which has two methods _msgSender and _msgData. The final Context replaces all msg.sender with _msgSender().

Address

The new library isContract logic has changed to check codehash and size.

ECDSA

The order has changed in the flattened output has changed.

ContributionReward

validateProposalParams was changed to prevent submitting proposals with multiple periods.

4 Likes

Great post, detailed description of the changes that were added cause the new ContirbutionReward was fixed on github where we have new code and we use tools as solhint and flattener scripts.

To see the source code locally and compare it to the new ContributionReward scheme:

  1. Get dxvote on this commit bf3776a0581828b59bcd5c0e0d8464a52d2ac293: GitHub - DXgovernance/dxdao-contracts at bf3776a0581828b59bcd5c0e0d8464a52d2ac293 (latest commit of the PR that merged the changes in the ContributionReward)
  2. Install all dependencies with yarn
  3. Run ./scripts/sourcify.sh contracts/daostack/universalSchemes/ContributionReward.sol
  4. Go to .temp folder and you will see the ContributionReward.sol, this is the contract that was deployed on mainnet and gnosis chain.
  5. You can diff check with the contract deployed on mainnet ContributionReward | Address 0x9d2bc77ad766259721ba3c9033c417b87a3540d0 | Etherscan using https://www.diffchecker.com

In my diff check I saw only the order of imports (the ECDSA contract imported was imported earlier on the one I did locally), and the comment of the verification message added by etherscan
dxdao-contracts/ContributionReward.sol at bf3776a0581828b59bcd5c0e0d8464a52d2ac293 · DXgovernance/dxdao-contracts · GitHub and this is the source code of the ContributionReward contract, (new one).

These instructions were shared with the rest of dxdao contributors two days ago.

1 Like