fix: 🐛 check factory owner from unarchive, refactor duplication

Author: Victor Wiebe

src/contract_wrappers/registries/__tests__/module_registry_wrapper.test.ts

@@ -322,6 +322,16 @@ describe('ModuleRegistryWrapper', () => {
     test('should successfully call to unverifyModule', async () => {
       // Owner Address expected
       const expectedOwnerResult = '0x0123456789012345678901234567890123456789';
+
+      // Setup mocked owner from module factory contract
+      const moduleFactoryAddress = '0x2222222222222222222222222222222222222222';
+      when(mockedContractFactory.getModuleFactoryContract(moduleFactoryAddress)).thenResolve(
+          instance(mockedModuleFactoryContract),
+      );
+      const mockedModuleFactoryOwnerMethod = mock(MockedCallMethod);
+      when(mockedModuleFactoryOwnerMethod.callAsync()).thenResolve(expectedOwnerResult);
+      when(mockedModuleFactoryContract.owner).thenReturn(instance(mockedModuleFactoryOwnerMethod));
+
       // Mocked method
       const mockedOwnerMethod = mock(MockedCallMethod);
       // Stub the method
@@ -390,6 +400,9 @@ describe('ModuleRegistryWrapper', () => {
       verify(mockedContract.owner).once();
       verify(mockedOwnerMethod.callAsync()).once();
       verify(mockedWrapper.getAvailableAddressesAsync()).once();
+      verify(mockedContractFactory.getModuleFactoryContract(moduleFactoryAddress)).once();
+      verify(mockedModuleFactoryOwnerMethod.callAsync()).once();
+      verify(mockedModuleFactoryContract.owner).once();
     });
   });
 

src/contract_wrappers/registries/module_registry_wrapper.ts

@@ -245,14 +245,7 @@ export default class ModuleRegistryWrapper extends ContractWrapper {
     assert.isETHAddressHex('moduleFactory', params.moduleFactory);
     await this.checkModuleNotPausedOrOwner();
     await this.checkModuleRegistered(params.moduleFactory);
-    const callerAddress = await this.getCallerAddress(undefined);
-    const owner = await this.owner();
-    const factoryOwner = await (await this.moduleFactoryContract(params.moduleFactory)).owner.callAsync();
-    assert.assert(
-      functionsUtils.checksumAddressComparision(callerAddress, owner) ||
-        functionsUtils.checksumAddressComparision(callerAddress, factoryOwner),
-      'Calling address must be owner or factory owner ',
-    );
+    await this.checkIsOwnerOrModuleFactoryOwner(params.moduleFactory);
     return (await this.contract).removeModule.sendTransactionAsync(
       params.moduleFactory,
       params.txData,
@@ -273,7 +266,7 @@ export default class ModuleRegistryWrapper extends ContractWrapper {
 
   public unverifyModule = async (params: ModuleFactoryParams) => {
     assert.isETHAddressHex('moduleFactory', params.moduleFactory);
-    await this.checkMsgSenderIsOwner();
+    await this.checkIsOwnerOrModuleFactoryOwner(params.moduleFactory);
     await this.checkModuleRegistered(params.moduleFactory);
     return (await this.contract).unverifyModule.sendTransactionAsync(
       params.moduleFactory,
@@ -470,7 +463,18 @@ export default class ModuleRegistryWrapper extends ContractWrapper {
     assert.assert(
       !(await this.isPaused()) ||
         functionsUtils.checksumAddressComparision(await this.owner(), await this.getCallerAddress(undefined)),
-      'Contract should not be Paused',
+      'Check contract is not be paused or method has not been called by owner',
     );
   };
+
+  private checkIsOwnerOrModuleFactoryOwner = async(moduleFactoryAddress: string) => {
+    const callerAddress = await this.getCallerAddress(undefined);
+    const owner = await this.owner();
+    const factoryOwner = await (await this.moduleFactoryContract(moduleFactoryAddress)).owner.callAsync();
+    assert.assert(
+        functionsUtils.checksumAddressComparision(callerAddress, owner) ||
+        functionsUtils.checksumAddressComparision(callerAddress, factoryOwner),
+        'Calling address must be owner or factory owner ',
+    );
+  }
 }