None universal voting machines #757
Conversation
| _stakingToken, | ||
| address(this), | ||
| address(this), | ||
| address(this), |
There was a problem hiding this comment.
If in all our schemes it's all the scheme address 3 times why not just make a scheme parameter for all the 3 instead?
Anyway this seems wrong actually, shouldn't this be the Avatar address, then twice the scheme address, instead of 3 times the scheme?
This comment is for all schemes...
| uint256 fundingGoalDeadline, | ||
| bool rageQuitEnable | ||
| ) = | ||
| abi.decode(_encodedJoinAndQuitParams, (address, uint256, uint256, uint256, uint256, bool)); |
There was a problem hiding this comment.
Why is that? Not that I see an issue, just want to understand why the added complexity here?
There was a problem hiding this comment.
this is needed to solve stack too deep issue.
| bytes32 _voteParamsHash | ||
| DAOFactory _daoFactory, | ||
| address _stakingToken, | ||
| uint64[3] calldata _packageVersion, |
There was a problem hiding this comment.
Maybe we could take that from the Avatar or somewhere else? I don't really like how this is duplicated in all the code... (Also relevant for daofactory)
There was a problem hiding this comment.
the alternative is to always use lasted package for the voting machines.
need to give it more thoughts.
contracts/utils/DAOFactory.sol
Outdated
| //this is here due to "stack too deep issue" | ||
| uint64[3] private packageVersion; | ||
|
|
||
| // orgName - The name of the new organization |
There was a problem hiding this comment.
Delete if not using this
| }, | ||
| "homepage": "https://daostack.io", | ||
| "dependencies": { | ||
| "@daostack/infra-experimental": "0.0.1-rc.16", |
There was a problem hiding this comment.
Let's also bump arc version
contracts/schemes/ArcScheme.sol
Outdated
| address _voteOnBehalf, | ||
| DAOFactory _daoFactory, | ||
| address _stakingToken, | ||
| address _organization, |
There was a problem hiding this comment.
Why is there both _organization and _avatar?
Shouldn't we just have _avatar ?
| address _stakingToken, | ||
| address _organization, | ||
| address _callbacks, | ||
| address _authorizedToPropose, |
There was a problem hiding this comment.
Maybe combine here _callbacks with _authorizedToPropose ?
This is anyway only for our schemes and I don't think we have a use case which needs them to be different.
There was a problem hiding this comment.
there might be a use case for that .
for now lets keep it like that.
| * @param _encodedSetSchemesParams _setSchemes parameters | ||
| * @param _packageVersion package version | ||
| */ | ||
| function _setSchemes ( |
There was a problem hiding this comment.
Each scheme receives DAOFactory, and package version in its init params, which creates a large duplication of data. It might be too complicated to do but it would be better if we could instead of passing it to each scheme we could pass it here for the schemes
This reverts commit 51c72e5.
-DAOFactory : DAO creation in a single transaction .