Skip to content

Commit 30c0a32

Browse files
kronosapienskronosapiens
committed
Respond to review comments II
1 parent 693818c commit 30c0a32

13 files changed

+90
-82
lines changed

‎contracts/colony/Colony.sol

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,9 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain {
365365
}
366366

367367
function installExtension(bytes32 _extensionId, uint256 _version)
368-
public stoppable auth returns (address)
368+
public stoppable auth
369369
{
370-
return IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version);
370+
IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version);
371371
}
372372

373373
function upgradeExtension(address _extension, uint256 _newVersion)
@@ -388,6 +388,12 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain {
388388
IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extension);
389389
}
390390

391+
function migrateToMultiExtension(bytes32 _extensionId)
392+
public stoppable auth
393+
{
394+
IColonyNetwork(colonyNetworkAddress).migrateToMultiExtension(_extensionId);
395+
}
396+
391397
function addDomain(uint256 _permissionDomainId, uint256 _childSkillIndex, uint256 _parentDomainId) public
392398
stoppable
393399
authDomain(_permissionDomainId, _childSkillIndex, _parentDomainId)

‎contracts/colony/ColonyAuthority.sol

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ contract ColonyAuthority is CommonAuthority {
120120
addRoleCapability(ROOT_ROLE, "upgradeExtension(address,uint256)");
121121
addRoleCapability(ROOT_ROLE, "deprecateExtension(address,bool)");
122122
addRoleCapability(ROOT_ROLE, "uninstallExtension(address)");
123+
addRoleCapability(ROOT_ROLE, "migrateToMultiExtension(bytes32)");
123124
addRoleCapability(ARBITRATION_ROLE, "setExpenditureMetadata(uint256,uint256,uint256,string)");
124125
}
125126

‎contracts/colony/ColonyStorage.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes
296296
// Ensure addr is an extension installed in the colony, must check old & new formats
297297
try ColonyExtension(addr).identifier() returns (bytes32 extensionId) {
298298
return (
299-
IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(addr) == address(this) ||
299+
IColonyNetwork(colonyNetworkAddress).getExtensionColony(addr) == address(this) ||
300300
IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == addr
301301
);
302302
} catch {

‎contracts/colony/IColony.sol

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,7 @@ interface IColony is ColonyDataTypes, IRecovery {
263263
/// @notice Install an extension to the colony. Secured function to authorised members.
264264
/// @param extensionId keccak256 hash of the extension name, used as an indentifier
265265
/// @param version The new extension version to install
266-
/// @return extension The address of the extension installation
267-
function installExtension(bytes32 extensionId, uint256 version) external returns (address extension);
266+
function installExtension(bytes32 extensionId, uint256 version) external;
268267

269268
/// @notice Upgrade an extension in a colony. Secured function to authorised members.
270269
/// @param extension The address of the extension installation
@@ -282,6 +281,10 @@ interface IColony is ColonyDataTypes, IRecovery {
282281
/// @param extension The address of the extension installation
283282
function uninstallExtension(address extension) external;
284283

284+
/// @notice Migrate extension bookkeeping to multiExtension. Secured function to authorised members.
285+
/// @param extensionId keccak256 hash of the extension name, used as an indentifier
286+
function migrateToMultiExtension(bytes32 extensionId) external;
287+
285288
/// @notice Add a colony domain, and its respective local skill under skill with id `_parentSkillId`.
286289
/// New funding pot is created and associated with the domain here.
287290
/// @param _permissionDomainId The domainId in which I have the permission to take this action

‎contracts/colonyNetwork/ColonyNetworkDataTypes.sol

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,13 @@ interface ColonyNetworkDataTypes {
114114
/// @param version The version of the extension
115115
event ExtensionAddedToNetwork(bytes32 indexed extensionId, uint256 version);
116116

117-
/// @notice Event logged when an extension is installed in a colony
117+
/// @notice Event logged when an extension is installed in a colony (for v7 and below)
118+
/// @param extensionId The identifier for the extension
119+
/// @param colony The address of the colony
120+
/// @param version The version of the extension
121+
event ExtensionInstalled(bytes32 indexed extensionId, address indexed colony, uint256 version);
122+
123+
/// @notice Event logged when an extension is installed in a colony (for v8 and above)
118124
/// @param extensionId The identifier for the extension
119125
/// @param extension Address of the extension installation
120126
/// @param colony The address of the colony

‎contracts/colonyNetwork/ColonyNetworkExtensions.sol

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,17 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
6060
if (IColony(msg.sender).version() <= 7) {
6161
require(installations[_extensionId][msg.sender] == address(0x0), "colony-network-extension-already-installed");
6262
installations[_extensionId][msg.sender] = address(extension);
63+
64+
emit ExtensionInstalled(_extensionId, address(extension), _version);
6365
} else {
6466
multiInstallations[address(extension)] = msg.sender;
67+
68+
emit ExtensionInstalled(_extensionId, address(extension), msg.sender, _version);
6569
}
6670

6771
extension.setResolver(resolvers[_extensionId][_version]);
6872
ColonyExtension(address(extension)).install(msg.sender);
6973

70-
emit ExtensionInstalled(_extensionId, address(extension), msg.sender, _version);
7174

7275
return address(extension);
7376
}
@@ -160,15 +163,16 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
160163
emit ExtensionUninstalled(_extension, msg.sender);
161164
}
162165

163-
function migrateToMultiExtension(bytes32 _extensionId, address _colony)
166+
function migrateToMultiExtension(bytes32 _extensionId)
164167
public
165168
stoppable
169+
calledByColony
166170
{
167-
require(installations[_extensionId][_colony] != address(0x0), "colony-network-extension-not-installed");
171+
require(installations[_extensionId][msg.sender] != address(0x0), "colony-network-extension-not-installed");
168172

169-
multiInstallations[installations[_extensionId][_colony]] = payable(_colony);
173+
multiInstallations[installations[_extensionId][msg.sender]] = payable(msg.sender);
170174

171-
delete installations[_extensionId][_colony];
175+
delete installations[_extensionId][msg.sender];
172176
}
173177

174178
// Public view functions
@@ -181,7 +185,7 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
181185
return resolvers[_extensionId][_version];
182186
}
183187

184-
function getExtensionMultiInstallation(address _extension)
188+
function getExtensionColony(address _extension)
185189
public
186190
view
187191
returns (address)

‎contracts/colonyNetwork/IColonyNetwork.sol

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -316,8 +316,7 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery {
316316
/// @notice Install an extension in a colony. Can only be called by a Colony.
317317
/// @param extensionId keccak256 hash of the extension name, used as an indentifier
318318
/// @param version Version of the extension to install
319-
/// @return extension The address of the extension installation
320-
function installExtension(bytes32 extensionId, uint256 version) external returns (address extension);
319+
function installExtension(bytes32 extensionId, uint256 version) external;
321320

322321
/// @dev DEPRECATED
323322
/// @notice Upgrade an extension in a colony. Can only be called by a Colony.
@@ -350,10 +349,9 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery {
350349
/// @param extension Address of the extension installation
351350
function uninstallExtension(address extension) external;
352351

353-
/// @notice Migrate extension bookkeeping to multiExtension
352+
/// @notice Migrate extension bookkeeping to multiExtension. Can only be called by a Colony.
354353
/// @param extensionId keccak256 hash of the extension name, used as an indentifier
355-
/// @param colony Address of the colony the extension is installed in
356-
function migrateToMultiExtension(bytes32 extensionId, address colony) external;
354+
function migrateToMultiExtension(bytes32 extensionId) external;
357355

358356
/// @notice Get an extension's resolver.
359357
/// @param extensionId keccak256 hash of the extension name, used as an indentifier
@@ -370,7 +368,7 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery {
370368
/// @notice Get an extension's installed colony.
371369
/// @param extension Address of the extension installation
372370
/// @return colony Address of the colony the extension is installed in
373-
function getExtensionMultiInstallation(address extension) external view returns (address colony);
371+
function getExtensionColony(address extension) external view returns (address colony);
374372

375373
/// @notice Return 1 / the fee to pay to the network. e.g. if the fee is 1% (or 0.01), return 100.
376374
/// @return _feeInverse The inverse of the network fee

‎docs/_Interface_IColony.md

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,11 +1038,6 @@ Install an extension to the colony. Secured function to authorised members.
10381038
|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
10391039
|version|uint256|The new extension version to install
10401040

1041-
**Return Parameters**
1042-
1043-
|Name|Type|Description|
1044-
|---|---|---|
1045-
|extension|address|The address of the extension installation
10461041

10471042
### `lockExpenditure`
10481043

@@ -1160,6 +1155,18 @@ Make a new task in the colony. Secured function to authorised members.
11601155
|_dueDate|uint256|The due date of the task, can set to `0` for no-op
11611156

11621157

1158+
### `migrateToMultiExtension`
1159+
1160+
Migrate extension bookkeeping to multiExtension. Secured function to authorised members.
1161+
1162+
1163+
**Parameters**
1164+
1165+
|Name|Type|Description|
1166+
|---|---|---|
1167+
|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
1168+
1169+
11631170
### `mintTokens`
11641171

11651172
Mint `_wad` amount of colony tokens. Secured function to authorised members.

‎docs/_Interface_IColonyNetwork.md

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -347,40 +347,40 @@ Returns the address of the ENSRegistrar for the Network.
347347
|---|---|---|
348348
|address|address|The address the ENSRegistrar resolves to
349349

350-
### `getExtensionInstallation`
350+
### `getExtensionColony`
351351

352-
Get an extension's installation.
352+
Get an extension's installed colony.
353353

354354

355355
**Parameters**
356356

357357
|Name|Type|Description|
358358
|---|---|---|
359-
|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
360-
|colony|address|Address of the colony the extension is installed in
359+
|extension|address|Address of the extension installation
361360

362361
**Return Parameters**
363362

364363
|Name|Type|Description|
365364
|---|---|---|
366-
|installation|address|The address of the installed extension
365+
|colony|address|Address of the colony the extension is installed in
367366

368-
### `getExtensionMultiInstallation`
367+
### `getExtensionInstallation`
369368

370-
Get an extension's installed colony.
369+
Get an extension's installation.
371370

372371

373372
**Parameters**
374373

375374
|Name|Type|Description|
376375
|---|---|---|
377-
|extension|address|Address of the extension installation
376+
|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
377+
|colony|address|Address of the colony the extension is installed in
378378

379379
**Return Parameters**
380380

381381
|Name|Type|Description|
382382
|---|---|---|
383-
|colony|address|Address of the colony the extension is installed in
383+
|installation|address|The address of the installed extension
384384

385385
### `getExtensionResolver`
386386

@@ -693,11 +693,6 @@ Install an extension in a colony. Can only be called by a Colony.
693693
|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
694694
|version|uint256|Version of the extension to install
695695

696-
**Return Parameters**
697-
698-
|Name|Type|Description|
699-
|---|---|---|
700-
|extension|address|The address of the extension installation
701696

702697
### `isColony`
703698

@@ -735,15 +730,14 @@ Reverse lookup a username from an address.
735730

736731
### `migrateToMultiExtension`
737732

738-
Migrate extension bookkeeping to multiExtension
733+
Migrate extension bookkeeping to multiExtension. Can only be called by a Colony.
739734

740735

741736
**Parameters**
742737

743738
|Name|Type|Description|
744739
|---|---|---|
745740
|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
746-
|colony|address|Address of the colony the extension is installed in
747741

748742

749743
### `punishStakers`

‎test/contracts-network/colony-network-extensions.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -196,21 +196,21 @@ contract("Colony Network Extensions", (accounts) => {
196196

197197
let colonyAddress;
198198

199-
colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extension.address);
199+
colonyAddress = await colonyNetwork.getExtensionColony(extension.address);
200200
expect(colonyAddress).to.equal(ethers.constants.AddressZero);
201201

202202
// Set up `installations` mapping in the old style
203-
const slot = soliditySha3(`0x000000000000000000000000${colony.address.slice(2)}`, soliditySha3(TEST_EXTENSION, 39));
204-
const value = `0x000000000000000000000000${extension.address.slice(2)}`;
203+
const slot = soliditySha3(ethers.utils.hexZeroPad(colony.address, 32), soliditySha3(TEST_EXTENSION, 39));
204+
const value = ethers.utils.hexZeroPad(extension.address, 32);
205205
await editableColonyNetwork.setStorageSlot(slot, value);
206206

207207
let extensionAddress;
208208
extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address);
209209
expect(extensionAddress).to.not.equal(ethers.constants.AddressZero);
210210

211-
await colonyNetwork.migrateToMultiExtension(TEST_EXTENSION, colony.address);
211+
await colony.migrateToMultiExtension(TEST_EXTENSION);
212212

213-
colonyAddress = await colonyNetwork.getExtensionMultiInstallation(extension.address);
213+
colonyAddress = await colonyNetwork.getExtensionColony(extension.address);
214214
expect(colonyAddress).to.equal(colony.address);
215215

216216
extensionAddress = await colonyNetwork.getExtensionInstallation(TEST_EXTENSION, colony.address);
@@ -221,8 +221,8 @@ contract("Colony Network Extensions", (accounts) => {
221221
const version7Colony = await Version7.new(colonyNetwork.address);
222222

223223
// Add version7Colony to _isColony mapping
224-
const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19);
225-
const value = `0x0000000000000000000000000000000000000000000000000000000000000001`;
224+
const slot = soliditySha3(ethers.utils.hexZeroPad(version7Colony.address, 32), 19);
225+
const value = ethers.utils.zeroPad(1, 32);
226226
await editableColonyNetwork.setStorageSlot(slot, value);
227227

228228
await version7Colony.installExtension(TEST_EXTENSION, 1);
@@ -297,8 +297,8 @@ contract("Colony Network Extensions", (accounts) => {
297297
const version7Colony = await Version7.new(colonyNetwork.address);
298298

299299
// Add version7Colony to _isColony mapping
300-
const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19);
301-
const value = `0x0000000000000000000000000000000000000000000000000000000000000001`;
300+
const slot = soliditySha3(ethers.utils.hexZeroPad(version7Colony.address, 32), 19);
301+
const value = ethers.utils.zeroPad(1, 32);
302302
await editableColonyNetwork.setStorageSlot(slot, value);
303303

304304
await version7Colony.installExtension(TEST_EXTENSION, 1);
@@ -346,8 +346,8 @@ contract("Colony Network Extensions", (accounts) => {
346346
const version7Colony = await Version7.new(colonyNetwork.address);
347347

348348
// Add version7Colony to _isColony mapping
349-
const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19);
350-
const value = `0x0000000000000000000000000000000000000000000000000000000000000001`;
349+
const slot = soliditySha3(ethers.utils.hexZeroPad(version7Colony.address, 32), 19);
350+
const value = ethers.utils.zeroPad(1, 32);
351351
await editableColonyNetwork.setStorageSlot(slot, value);
352352

353353
await version7Colony.installExtension(TEST_EXTENSION, 1);
@@ -397,8 +397,8 @@ contract("Colony Network Extensions", (accounts) => {
397397
const version7Colony = await Version7.new(colonyNetwork.address);
398398

399399
// Add version7Colony to _isColony mapping
400-
const slot = soliditySha3(`0x000000000000000000000000${version7Colony.address.slice(2)}`, 19);
401-
const value = `0x0000000000000000000000000000000000000000000000000000000000000001`;
400+
const slot = soliditySha3(ethers.utils.hexZeroPad(version7Colony.address, 32), 19);
401+
const value = ethers.utils.zeroPad(1, 32);
402402
await editableColonyNetwork.setStorageSlot(slot, value);
403403

404404
await version7Colony.installExtension(TEST_EXTENSION, 1);

0 commit comments

Comments
 (0)