Computable-Finance / CoFiX

Core Smart Contracts of CoFiX: A Computable Financial Transaction Model. The Future of On-Chain Market Making is Here.
https://cofix.io
GNU General Public License v3.0
54 stars 21 forks source link

for diff review #3

Closed kazamio closed 4 years ago

kazamio commented 4 years ago

Response to CertiK Phase 3 Audit

Exhibit 1 - DAO needed

Discussion: We saw the records of Governance Authority Transfer to Multi-Sig Wallet. Not sure CoFiX will use Multi-Sig Wallet as governor? Or implementing CoFiX DAO and transfer governance to DAO?

The CoFiX DAO is still under design. Currently, we use multi-sig wallet as governor to ensure the security of user assets. Governance ownership will be transferred to the CoFiX DAO in the next stage when the CoFi token is widely distributed. At that point, we will invite CertiK to review the governance model.

Exhibit 2 - Simplifying Existing Code

Consider using a modifier to replace the below same codes existing in many functions.

CoFiStakingRewards and CoFiXVaultForCNode are deployed and running on the main-net now. We could improve the coding style by using onlyGovernance() modifier in the next update.

Exhibit 3 - Gas consumption

Variables like “cofiToken”,”factory”,”genesisBlock” change only once, better to define it as immutable to avoid gas consumption.

The immutable keyword has been used to reduce gas costs before the main-net release.

Exhibit 4 - Declare Events

Several sensitive actions are defined without event declarations. Examples :

  • Functions “setInitCoFiRate”,”setDecayPeriod” and ”setDecayRate”.

CoFiXVaultForCNode is deployed and running on the main-net now. We could add the event in the next update.

Exhibit 5 - Inconsistent coding style

  • in CoFiXVaultForTrader.sol: uint256 public constant RATE_BASE = 1e18;
  • in CoFiXController.sol: uint256 constant public AONE = 1 ether;

Thanks for pointing out. We could improve there in the next update.