gildlab / ethgild

Other
2 stars 3 forks source link

protofire upgreadeability #188

Open thedavidmeister opened 2 days ago

thedavidmeister commented 2 days ago

The project employs an upgradability pattern, which introduces a high risk of centralization. In this pattern, the project owner has significant control over the contract's functionality and can make updates at any time. This centralization requires users to place a high level of trust in the project owner, as they have the power to alter the contract's behavior, potentially in ways that could be detrimental to users. To mitigate the risk of centralization, we recommend implementing a Timelock contract for the update functionality. A Timelock contract introduces a delay between the announcement of an upgrade and its execution, providing users with a window of time to review and react to the proposed changes.

thedavidmeister commented 2 days ago

no it doesn't

open zeppelin calls any contract that has initializers instead of constructors "upgradeable" but it really just means "compatible with proxy contracts" which is how clones from the clone factory are implemented, but open zeppelin clones are ERC1167 minimal proxy contracts, which are immutable https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.9/contracts/proxy/Clones.sol#L7 https://eips.ethereum.org/EIPS/eip-1167

the cyclo deploy script uses the clone factory which deploys immutable proxies https://github.com/cyclofinance/cyclo.sol/blob/main/script/Deploy.sol#L56

please don't say the contracts are mutable/upgradeable when this is not true, because immutability and avoiding admin keys is a very important detail that we worked hard to achieve