From 7c598e7abd8478858ed9ae7cace00f37b7133dac Mon Sep 17 00:00:00 2001 From: Tanapol Hongsuwan Date: Wed, 24 Jan 2024 15:44:19 +0700 Subject: [PATCH 1/5] revise transferOwner in Event.sol --- circom | 1 + hardhat/contracts/Event.sol | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 160000 circom diff --git a/circom b/circom new file mode 160000 index 00000000..ccc8cd74 --- /dev/null +++ b/circom @@ -0,0 +1 @@ +Subproject commit ccc8cd743c38ff4d24f6c8b68a8ad7b3093705db diff --git a/hardhat/contracts/Event.sol b/hardhat/contracts/Event.sol index bf7dc1f2..5963b35b 100644 --- a/hardhat/contracts/Event.sol +++ b/hardhat/contracts/Event.sol @@ -194,6 +194,44 @@ contract EventManager is OwnableUpgradeable { return _groups; } + function transferOwner( + uint256 _groupId, + address _newOwnerAddress + ) external whenNotPaused { + bool isOwner = false; + require( + _newOwnerAddress != address(0), + "New owner address is blank" + ); + + for (uint256 i = 0; i < ownGroupIds[msg.sender].length; i++) { + if (ownGroupIds[msg.sender][i] == _groupId) { + isOwner = true; + break; + } + } + require(isOwner, "Caller is not the owner of the group"); + + for (uint256 i = 0; i < groups.length; i++) { + if (groups[i].groupId == _groupId) { + groups[i].ownerAddress = _newOwnerAddress; + break; + } + } + + // Update the mapping of ownGroupIds for the new owner + ownGroupIds[_newOwnerAddress].push(_groupId); + + // Remove the groupId from the list for the current owner + for (uint256 i = 0; i < ownGroupIds[msg.sender].length; i++) { + if (ownGroupIds[msg.sender][i] == _groupId) { + ownGroupIds[msg.sender][i] = ownGroupIds[msg.sender][ownGroupIds[msg.sender].length - 1]; + ownGroupIds[msg.sender].pop(); + break; + } + } + } + function createEventRecord( uint256 _groupId, string memory _name, From 8ddb126766ad2540c74647e1c4f08674c5dd2aad Mon Sep 17 00:00:00 2001 From: Tanapol Hongsuwan Date: Wed, 31 Jan 2024 10:34:15 +0900 Subject: [PATCH 2/5] revised Event.sol --- hardhat/contracts/Event.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hardhat/contracts/Event.sol b/hardhat/contracts/Event.sol index 5963b35b..e0bb9179 100644 --- a/hardhat/contracts/Event.sol +++ b/hardhat/contracts/Event.sol @@ -194,7 +194,7 @@ contract EventManager is OwnableUpgradeable { return _groups; } - function transferOwner( + function transferGroupOwner( uint256 _groupId, address _newOwnerAddress ) external whenNotPaused { From 3e4f60fd3646b243069bf5b3f8cb6621434a2120 Mon Sep 17 00:00:00 2001 From: Tanapol Hongsuwan Date: Wed, 31 Jan 2024 19:38:18 +0900 Subject: [PATCH 3/5] added GetOwnTransfer test code in EventManager.ts --- hardhat/test/EventManager.ts | 48 +++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/hardhat/test/EventManager.ts b/hardhat/test/EventManager.ts index 1066b3bd..690b7751 100644 --- a/hardhat/test/EventManager.ts +++ b/hardhat/test/EventManager.ts @@ -39,9 +39,10 @@ describe("EventManager", function () { let participant1: SignerWithAddress; let participant2: SignerWithAddress; let relayer: SignerWithAddress; + let newOwner: SignerWithAddress; before(async () => { - [organizer, participant1, participant2, relayer] = + [organizer, participant1, participant2, relayer, newOwner] = await ethers.getSigners(); const SecretPhraseVerifierFactory = await ethers.getContractFactory( "SecretPhraseVerifier" @@ -698,6 +699,51 @@ describe("EventManager", function () { }); }); + describe("GetOwnTransfer", function () { + let eventManager: EventManager; + before(async () => { + const eventManagerContractFactory = await ethers.getContractFactory("EventManager"); + const deployedEventManagerContract = await upgrades.deployProxy( + eventManagerContractFactory, + [ + organizer.address, + relayer.address, + 250000, + 1000000, + operationController.address + ], + { initializer: "initialize" } + ); + eventManager = deployedEventManagerContract as EventManager; + await eventManager.deployed(); + await eventManager.setMintNFTAddr(mintNFT.address); + await mintNFT.setEventManagerAddr(eventManager.address); + + const createGroupTx = await eventManager.connect(organizer).createGroup("transferGroup"); + await createGroupTx.wait(); + + const createdGroups = await eventManager.getGroups(); + expect(createdGroups.some(group => group.name === "transferGroup")).to.be.true; + }); + + it("Should transfer group ownership", async function () { + const groups = await eventManager.getGroups(); + const group = groups.find(group => group.name === "transferGroup"); + expect(group, "Group not found").to.exist; + + const groupId = group!.groupId; + + const transferTx = await eventManager.connect(organizer).transferGroupOwner(groupId, newOwner.address); + await transferTx.wait(); + + const updatedGroups = await eventManager.getGroups(); + const updatedGroup = updatedGroups.find(group => group.groupId.eq(groupId)); + + expect(updatedGroup, "Updated group not found").to.exist; + expect(updatedGroup!.ownerAddress).to.equal(newOwner.address); + }); + }); + describe("Role", async () => { let eventManager: EventManager; From 30a945114359d710565f0f5619e176c2d23c6aa4 Mon Sep 17 00:00:00 2001 From: Tanapol Hongsuwan Date: Sun, 4 Feb 2024 21:42:30 +0900 Subject: [PATCH 4/5] added transfer frontend (processing) --- .../src/components/molecules/EventGroupTab.tsx | 17 +++++++++++++++++ frontend/src/locales/en.ts | 1 + frontend/src/locales/ja.ts | 1 + 3 files changed, 19 insertions(+) diff --git a/frontend/src/components/molecules/EventGroupTab.tsx b/frontend/src/components/molecules/EventGroupTab.tsx index d4ccaeae..c363995d 100644 --- a/frontend/src/components/molecules/EventGroupTab.tsx +++ b/frontend/src/components/molecules/EventGroupTab.tsx @@ -27,6 +27,8 @@ const EventGroupTab: FC = ({ group }) => { return 1; case router.asPath.includes("role"): return 2; + case router.asPath.includes("transfer"): + return 3; default: break; } @@ -81,6 +83,21 @@ const EventGroupTab: FC = ({ group }) => { {t.RBAC_EDIT_COLLABORATORS} )} + {(group.ownerAddress === address) && ( + + router.push(`/event-groups/${router.query.eventgroupid}/transfer`) + } + > + {t.EVENT_GROUP_TAB_TRANSFER} + + )} ); diff --git a/frontend/src/locales/en.ts b/frontend/src/locales/en.ts index 927fc271..0a025962 100644 --- a/frontend/src/locales/en.ts +++ b/frontend/src/locales/en.ts @@ -43,6 +43,7 @@ export default { // Event group tab EVENT_GROUP_TAB_EVENTS: "Event List", EVENT_GROUP_TAB_LEADERS: "Leader Board", + EVENT_GROUP_TAB_TRANSFER: "Transfer Owner", // Event group leaders EVENT_GROUP_LEADERS_RANK: "RANK", EVENT_GROUP_LEADERS_ADDRESS: "ADDRESS", diff --git a/frontend/src/locales/ja.ts b/frontend/src/locales/ja.ts index f39d40d9..6c6942dc 100644 --- a/frontend/src/locales/ja.ts +++ b/frontend/src/locales/ja.ts @@ -43,6 +43,7 @@ export default { // Event group tab EVENT_GROUP_TAB_EVENTS: "イベント一覧", EVENT_GROUP_TAB_LEADERS: "リーダーボード", + EVENT_GROUP_TAB_TRANSFER: "権限譲与", // Event group leaders EVENT_GROUP_LEADERS_RANK: "ランク", EVENT_GROUP_LEADERS_ADDRESS: "アドレス", From 532e9fca984fd0959d95bd7c58a88d2f8ebf2153 Mon Sep 17 00:00:00 2001 From: yu23ki14 Date: Wed, 7 Feb 2024 09:01:16 +0900 Subject: [PATCH 5/5] refactor and add test --- circom | 1 - hardhat/contracts/Event.sol | 43 ++++++++++++--------- hardhat/test/EventManager.ts | 73 +++++++++++++++++++++++++----------- 3 files changed, 76 insertions(+), 41 deletions(-) delete mode 160000 circom diff --git a/circom b/circom deleted file mode 160000 index ccc8cd74..00000000 --- a/circom +++ /dev/null @@ -1 +0,0 @@ -Subproject commit ccc8cd743c38ff4d24f6c8b68a8ad7b3093705db diff --git a/hardhat/contracts/Event.sol b/hardhat/contracts/Event.sol index e0bb9179..9c4a20cc 100644 --- a/hardhat/contracts/Event.sol +++ b/hardhat/contracts/Event.sol @@ -61,6 +61,11 @@ contract EventManager is OwnableUpgradeable { private memberRolesByGroupId; mapping(uint256 => address[]) private memberAddressesByGroupId; + modifier onlyGroupOwner(uint256 _groupId) { + require(_isGroupOwner(_groupId, msg.sender), "You have no permission"); + _; + } + modifier onlyAdminAccess(uint256 _groupId) { require( _hasAdminAccess(_groupId, msg.sender), @@ -116,6 +121,11 @@ contract EventManager is OwnableUpgradeable { event CreateGroup(address indexed owner, uint256 groupId); event CreateEvent(address indexed owner, uint256 eventId); + event TransferGroupOwner( + address indexed prevOwner, + address indexed newOwner, + uint256 groupId + ); // Currently, reinitializer(3) was executed as constructor. function initialize( @@ -197,20 +207,8 @@ contract EventManager is OwnableUpgradeable { function transferGroupOwner( uint256 _groupId, address _newOwnerAddress - ) external whenNotPaused { - bool isOwner = false; - require( - _newOwnerAddress != address(0), - "New owner address is blank" - ); - - for (uint256 i = 0; i < ownGroupIds[msg.sender].length; i++) { - if (ownGroupIds[msg.sender][i] == _groupId) { - isOwner = true; - break; - } - } - require(isOwner, "Caller is not the owner of the group"); + ) external whenNotPaused onlyGroupOwner(_groupId) { + require(_newOwnerAddress != address(0), "New owner address is blank"); for (uint256 i = 0; i < groups.length; i++) { if (groups[i].groupId == _groupId) { @@ -219,17 +217,26 @@ contract EventManager is OwnableUpgradeable { } } - // Update the mapping of ownGroupIds for the new owner ownGroupIds[_newOwnerAddress].push(_groupId); - // Remove the groupId from the list for the current owner for (uint256 i = 0; i < ownGroupIds[msg.sender].length; i++) { if (ownGroupIds[msg.sender][i] == _groupId) { - ownGroupIds[msg.sender][i] = ownGroupIds[msg.sender][ownGroupIds[msg.sender].length - 1]; + ownGroupIds[msg.sender][i] = ownGroupIds[msg.sender][ + ownGroupIds[msg.sender].length - 1 + ]; ownGroupIds[msg.sender].pop(); break; } } + + emit TransferGroupOwner(msg.sender, _newOwnerAddress, _groupId); + } + + function _isGroupOwner( + uint256 _groupId, + address _address + ) private view returns (bool) { + return groups[_groupId - 1].ownerAddress == _address; } function createEventRecord( @@ -435,7 +442,7 @@ contract EventManager is OwnableUpgradeable { require(_groupId > 0 && _groupId <= groups.length, "Invalid groupId"); return - groups[_groupId - 1].ownerAddress == _address || + _isGroupOwner(_groupId, _address) || _hasRole(_groupId, _address, ADMIN_ROLE); } diff --git a/hardhat/test/EventManager.ts b/hardhat/test/EventManager.ts index 690b7751..fe92030e 100644 --- a/hardhat/test/EventManager.ts +++ b/hardhat/test/EventManager.ts @@ -88,7 +88,8 @@ describe("EventManager", function () { relayer.address, 250000, 1000000, - operationController.address], + operationController.address, + ], { initializer: "initialize", } @@ -488,7 +489,7 @@ describe("EventManager", function () { relayer.address, 500000, 1000000, - operationController.address + operationController.address, ], { initializer: "initialize", @@ -543,7 +544,7 @@ describe("EventManager", function () { relayer.address, 500000, 1000000, - operationController.address + operationController.address, ], { initializer: "initialize", @@ -618,7 +619,7 @@ describe("EventManager", function () { relayer.address, 250000, 1000000, - operationController.address + operationController.address, ], { initializer: "initialize", @@ -699,10 +700,12 @@ describe("EventManager", function () { }); }); - describe("GetOwnTransfer", function () { + describe("Transfer Owner", function () { let eventManager: EventManager; before(async () => { - const eventManagerContractFactory = await ethers.getContractFactory("EventManager"); + const eventManagerContractFactory = await ethers.getContractFactory( + "EventManager" + ); const deployedEventManagerContract = await upgrades.deployProxy( eventManagerContractFactory, [ @@ -710,7 +713,7 @@ describe("EventManager", function () { relayer.address, 250000, 1000000, - operationController.address + operationController.address, ], { initializer: "initialize" } ); @@ -718,30 +721,56 @@ describe("EventManager", function () { await eventManager.deployed(); await eventManager.setMintNFTAddr(mintNFT.address); await mintNFT.setEventManagerAddr(eventManager.address); - - const createGroupTx = await eventManager.connect(organizer).createGroup("transferGroup"); + + const createGroupTx = await eventManager + .connect(organizer) + .createGroup("transferGroup"); await createGroupTx.wait(); - + const createdGroups = await eventManager.getGroups(); - expect(createdGroups.some(group => group.name === "transferGroup")).to.be.true; + expect(createdGroups.some((group) => group.name === "transferGroup")).to + .be.true; }); - + it("Should transfer group ownership", async function () { const groups = await eventManager.getGroups(); - const group = groups.find(group => group.name === "transferGroup"); - expect(group, "Group not found").to.exist; - + const group = groups.find((group) => group.name === "transferGroup"); const groupId = group!.groupId; - - const transferTx = await eventManager.connect(organizer).transferGroupOwner(groupId, newOwner.address); + + const transferTx = await eventManager + .connect(organizer) + .transferGroupOwner(groupId, newOwner.address); await transferTx.wait(); - + expect(transferTx) + .to.emit(eventManager, "TransferOwner") + .withArgs(groupId, organizer.address, newOwner.address); + const updatedGroups = await eventManager.getGroups(); - const updatedGroup = updatedGroups.find(group => group.groupId.eq(groupId)); - + const updatedGroup = updatedGroups.find((group) => + group.groupId.eq(groupId) + ); + expect(updatedGroup, "Updated group not found").to.exist; expect(updatedGroup!.ownerAddress).to.equal(newOwner.address); - }); + + const ownGroups = await eventManager.getOwnGroups(newOwner.address); + expect(ownGroups.some((group) => group.groupId.eq(groupId))).to.be.true; + const ownGroups2 = await eventManager.getOwnGroups(organizer.address); + expect(ownGroups2.every((group) => !group.groupId.eq(groupId))).to.be + .true; + }); + + it("Should revert if not group owner", async function () { + const groups = await eventManager.getGroups(); + const group = groups.find((group) => group.name === "transferGroup"); + const groupId = group!.groupId; + + await expect( + eventManager + .connect(participant1) + .transferGroupOwner(groupId, newOwner.address) + ).to.be.revertedWith("You have no permission"); + }); }); describe("Role", async () => { @@ -758,7 +787,7 @@ describe("EventManager", function () { relayer.address, 250000, 1000000, - operationController.address + operationController.address, ], { initializer: "initialize",