Merge pull request #14 from citydaoproject/mnorman-issue13

feat: [#13] added Ownable
citydaoproject · May 19, 2022 · efac0c5 · efac0c5
2 parents 73628b5 + b015f65
commit efac0c5
Show file tree

Hide file tree

Showing 8 changed files with 52 additions and 5 deletions.
diff --git a/contracts/ParcelNFT.sol b/contracts/ParcelNFT.sol
@@ -3,6 +3,7 @@
 pragma solidity ^0.8.9;
 
 import '@gnus.ai/contracts-upgradeable-diamond/access/AccessControlUpgradeable.sol';
+import '@gnus.ai/contracts-upgradeable-diamond/access/OwnableUpgradeable.sol';
 import '@gnus.ai/contracts-upgradeable-diamond/proxy/utils/UUPSUpgradeable.sol';
 import '@gnus.ai/contracts-upgradeable-diamond/security/PausableUpgradeable.sol';
 import '@gnus.ai/contracts-upgradeable-diamond/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol';
@@ -70,6 +71,7 @@ contract ParcelNFT is
   ERC721RoyaltyUpgradeable,
   ERC721BatchTransfer,
   AccessControlUpgradeable,
+  OwnableUpgradeable,
   PausableUpgradeable
 {
   struct InitParams {
@@ -91,6 +93,7 @@ contract ParcelNFT is
       initParams.superAdmin = _msgSender();
     }
     _setupRole(Roles.SUPER_ADMIN, initParams.superAdmin);
+    _transferOwnership(initParams.superAdmin);
   }
 
   /**

diff --git a/examples/parcelNFTConstructorParams.js b/examples/parcelNFTConstructorParams.js
@@ -1,8 +1,8 @@
 const { buildParcelNFTInitFunction } = require('../dist/src/contracts/parcelNFT');
 module.exports = [
-  '0xc0cA359c8ce6De21B98fC6c7921a08703f453Fe9',
+  '0xEB667659b19dfc8B6b3b6FaAFaE3b5D7661dcB68',
   buildParcelNFTInitFunction({
-    name: 'ParcelNFT Test 2022-05-07-02',
-    symbol: 'PT050702',
+    name: 'CityDAO Parcel-0',
+    symbol: 'CityDAO-P0',
   }),
 ];
diff --git a/test/contracts/access/access.spec.ts b/test/contracts/access/access.spec.ts
@@ -11,7 +11,7 @@ describe('IAccessControl', () => {
   shouldSupportInterface('IAccessControl', () => createParcelNFT(), ACCESS_CONTROL_INTERFACE_ID);
 });
 
-describe('initialized with zero address', () => {
+describe('SuperUser initialized with zero address', () => {
   it('should set caller as super admin', async () => {
     const parcelNFT = await createParcelNFT({ superAdmin: ZERO_ADDRESS });
     expect(await parcelNFT.hasRole(SUPER_ADMIN_ROLE, INITIALIZER.address)).to.be.true;
@@ -33,7 +33,7 @@ describe('initialized with zero address', () => {
   });
 });
 
-describe('initialized with another address', () => {
+describe('SuperUser initialized with another address', () => {
   it('should set caller as super admin', async () => {
     const parcelNFT = await createParcelNFT({ superAdmin: USER1.address });
     expect(await parcelNFT.hasRole(SUPER_ADMIN_ROLE, USER1.address)).to.be.true;

diff --git a/test/contracts/access/ownable.spec.ts b/test/contracts/access/ownable.spec.ts
@@ -0,0 +1,36 @@
+import { expect } from 'chai';
+import { ZERO_ADDRESS } from '../../../src/constants/accounts';
+import { INITIALIZER, USER1 } from '../../helpers/Accounts';
+import { createParcelNFT } from '../../helpers/contracts/ParcelNFTHelper';
+
+describe('Owner initialized with zero address', () => {
+  it('should set caller as super admin', async () => {
+    const parcelNFT = await createParcelNFT({ superAdmin: ZERO_ADDRESS });
+    expect(await parcelNFT.owner()).to.eq(INITIALIZER.address);
+  });
+});
+
+describe('Owner initialized with another address', () => {
+  it('should set caller as super admin', async () => {
+    const parcelNFT = await createParcelNFT({ superAdmin: USER1.address });
+    expect(await parcelNFT.owner()).to.eq(USER1.address);
+  });
+});
+
+describe('transferOwnership', () => {
+  it('should transfer ownership', async () => {
+    const parcelNFT = await createParcelNFT({ superAdmin: ZERO_ADDRESS });
+    await parcelNFT.transferOwnership(USER1.address);
+
+    expect(await parcelNFT.owner()).to.eq(USER1.address);
+  });
+
+  it('should fail to transfer ownership when not owner', async () => {
+    const parcelNFT = await createParcelNFT({ superAdmin: ZERO_ADDRESS });
+    await expect(parcelNFT.connect(USER1).transferOwnership(USER1.address)).to.be.revertedWith(
+      'caller is not the owner',
+    );
+
+    expect(await parcelNFT.owner()).to.eq(INITIALIZER.address);
+  });
+});
diff --git a/test/contracts/erc721/tokenURI.spec.ts b/test/contracts/erc721/tokenURI.spec.ts
@@ -66,6 +66,7 @@ describe('setTokenURI', () => {
   it('should fail if not called with parcel manager', async () => {
     const parcelNFT = await createParcelNFT();
     await parcelNFT.grantRole(PARCEL_MANAGER_ROLE, INITIALIZER.address);
+    await parcelNFT.transferOwnership(USER1.address);
 
     await setValidClaimPeriod(parcelNFT);
 

diff --git a/test/contracts/pausable/pausable.spec.ts b/test/contracts/pausable/pausable.spec.ts
@@ -19,12 +19,15 @@ describe('pause', () => {
 
   it('should fail if called by non-pauser', async () => {
     const parcelNFT = await createParcelNFT();
+    await parcelNFT.grantRole(PAUSER_ROLE, INITIALIZER.address);
+    await parcelNFT.transferOwnership(USER1.address);
 
     expect(parcelNFT.connect(USER1).pause()).to.be.revertedWith('missing role');
 
     expect(await parcelNFT.paused()).to.be.false;
 
     await parcelNFT.grantRole(PAUSER_ROLE, USER1.address);
+    await parcelNFT.connect(USER1).transferOwnership(USER2.address);
 
     expect(parcelNFT.connect(USER2).pause()).to.be.revertedWith('missing role');
 
@@ -71,6 +74,7 @@ describe('unpause', () => {
   it('should fail if called by non-pauser', async () => {
     const parcelNFT = await createParcelNFT();
     await parcelNFT.grantRole(PAUSER_ROLE, INITIALIZER.address);
+    await parcelNFT.transferOwnership(USER1.address);
 
     await parcelNFT.pause();
 
@@ -79,6 +83,7 @@ describe('unpause', () => {
     expect(await parcelNFT.paused()).to.be.true;
 
     await parcelNFT.grantRole(PAUSER_ROLE, USER1.address);
+    await parcelNFT.connect(USER1).transferOwnership(USER2.address);
 
     expect(parcelNFT.connect(USER2).unpause()).to.be.revertedWith('missing role');
 

diff --git a/test/contracts/proxy/proxy.spec.ts b/test/contracts/proxy/proxy.spec.ts
@@ -31,6 +31,7 @@ describe('createParcelNFT', () => {
     const parcelNFTLogic = await deployParcelNFT();
     const parcelNFT = await createParcelNFT(INITIALIZER, parcelNFTLogic.address);
     await parcelNFT.grantRole(UPGRADER_ROLE, INITIALIZER.address);
+    await parcelNFT.transferOwnership(USER1.address);
 
     const newParcelNFTLogic = await deployParcelNFT();
     await expect(parcelNFT.connect(USER1).upgradeTo(newParcelNFTLogic.address)).to.be.revertedWith('missing role');

diff --git a/test/contracts/royalty/royalty.spec.ts b/test/contracts/royalty/royalty.spec.ts
@@ -25,6 +25,7 @@ describe('setDefaultRoyalty', () => {
 
   it('should fail if not called by parcel manager', async () => {
     const parcelNFT = await createParcelNFT();
+    await parcelNFT.transferOwnership(USER1.address);
 
     await expect(parcelNFT.connect(USER1).setDefaultRoyalty(USER2.address, 200)).to.be.revertedWith('missing role');