Add ability to override owner #3
Conversation
Needed for our incoming Deployer contract
|
Please add a test |
|
@makoto I've added tests |
migrations/2_deploy_contracts.js
Outdated
| @@ -39,7 +39,7 @@ module.exports = function(deployer) { | |||
| deployer | |||
| .then(() => { | |||
| console.log([name, deposit,limitOfParticipants, coolingPeriod, encryption].join(',')); | |||
| return deployer.deploy(Conference, name, deposit,limitOfParticipants, coolingPeriod, encryption); | |||
| return deployer.deploy(Conference, name, deposit,limitOfParticipants, coolingPeriod, encryption, owner); | |||
There was a problem hiding this comment.
Probably no need to pass owner here?
There was a problem hiding this comment.
Or pass 0x to use the default value (= msg.sender). By doing so we can get rid of owner variable altogether as we are also getting rid of ENS contracts from migration
| ) public { | ||
| if (_owner != address(0)) { | ||
| owner = _owner; | ||
| } |
There was a problem hiding this comment.
What do you think of getting rid of https://github.com/noblocknoparty/blockparty-contracts/blob/1a6f355b5134fc9133a447a0010e09753aca2b20/contracts/zeppelin/ownership/Ownable.sol#L21
and specify owner here explicitly like this.
if (_owner != address(0)) {
owner = _owner;
}else{
owner = msg.sender;
}
If inheritance works like Ownable#constructor => Party#constructor then we are setting value to owner twice (and I am guessing this is how owner is falling back to msg.owner if 0 address is passed ). Doing this way, we only set it once and may potentially save gas cost as well as we don't have to worry about how inheritance order works.
|
@makoto Made fixes. |
Needed for our incoming Deployer contract