-
Notifications
You must be signed in to change notification settings - Fork 3
Description
The Eternal Storage pattern is not used in the way it is designed. According to https://fravoll.github.io/solidity-patterns/eternal_storage.html:
“A separate contract, with the sole purpose of acting as a storage to another contract, is introduced.”
However with ElasticDAO the contracts are derived from EternalModel and thus the EternalModel memory is integrated in the contract:
• contract TokenHolder is EternalModel, ReentryProtection {
• contract Token is EternalModel, ReentryProtection {
• contract Ecosystem is EternalModel, ReentryProtection {
• contract DAO is EternalModel, ReentryProtection {
You can also see the difference in the implementation of the EternalModel functions, they should be external to be callable from another contract:
ElasticDAO EternalModel.sol:
Fravoll EternalModel.sol:
function setUint(bytes32 _key, uint256 _value) internal {
s.uIntStorage[_key] = _value;
}
function setUint(bytes32 _key, uint _value) onlyLatestVersion external {
uIntStorage[_key] = _value;
}
Because the Proxy pattern is already used, the Eternal Storage pattern is not necessary and just complicates the source and uses a lot of gas.
Recommendation:
Choose either the Proxy pattern or the Eternal Storage pattern.
Use the same chosen pattern for allowances (see bugs above).
Definition of Done
- Time boxed to 1 day, check to see gas savings of the following
- DAO struct lives on ElasticDAO
- elasticGovernanceTokenAddress lives on ElasticDAO
- Token struct lives on ElasticGovernanceToken
- TokenHolder struct lives on ElasticGovernanceToken
- mapping(address => TokenHolder) struct lives on ElasticGovernanceToken
- before and after gas estimations for all model related write functions