diff --git a/src/PalmeraModule.sol b/src/PalmeraModule.sol index c2000b4..53d180a 100644 --- a/src/PalmeraModule.sol +++ b/src/PalmeraModule.sol @@ -290,7 +290,7 @@ contract PalmeraModule is Auth, Helpers { || role == DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY ) { // Update safe/org lead - _safe.lead = user; + _safe.lead = enabled ? user : address(0); } RolesAuthority _authority = RolesAuthority(rolesAuthority); _authority.setUserRole(user, uint8(role), enabled); @@ -1136,6 +1136,7 @@ contract PalmeraModule is Auth, Helpers { /// @param org ID's of the organisation /// @param safeId uint256 of the safe function removeIndexSafe(bytes32 org, uint256 safeId) private { + address _safe = safes[org][safeId].safe; uint256[] storage indexSafeOrg = indexSafe[org]; for (uint256 i; i < indexSafeOrg.length;) { if (indexSafeOrg[i] == safeId) { @@ -1147,6 +1148,17 @@ contract PalmeraModule is Auth, Helpers { ++i; } } + // Remove Safe Address from the Any Safe Lead into the Same Org + mapping(uint256 => DataTypes.Safe) storage _safes = safes[org]; + for (uint256 i; i < indexSafeOrg.length;) { + uint256 _safeId = indexSafeOrg[i]; + if (_safes[_safeId].lead == _safe) { + _safes[_safeId].lead = address(0); + } + unchecked { + ++i; + } + } } /// @notice Private method to remove Org from Array of Hashes of organisations diff --git a/test/PalmeraRolesTest.t.sol b/test/PalmeraRolesTest.t.sol index 6e2bac3..0472323 100644 --- a/test/PalmeraRolesTest.t.sol +++ b/test/PalmeraRolesTest.t.sol @@ -806,8 +806,8 @@ contract PalmeraRolesTest is DeployHelper { function test_SafeLeadModifyOwnersOnly_CannotCanExecuteTransactionOnBehalf() public { - (uint256 rootIdA,,,, uint256 subSafeIdA1,) = - palmeraSafeBuilder.setupTwoRootOrgWithOneSafeAndOneChildEach( + (uint256 rootIdA,,,, uint256 subSafeIdA1,) = palmeraSafeBuilder + .setupTwoRootOrgWithOneSafeAndOneChildEach( orgName, safeA1Name, root2Name, @@ -861,7 +861,9 @@ contract PalmeraRolesTest is DeployHelper { } /// @notice Test to create a root Safe in one onchain org, remove Whole Tree it and after to add another or and verify not preserve any role - function test_AnyRootSafeNotPreserveRootSafeRoleAfterRemoveWholeTree() public { + function test_AnyRootSafeNotPreserveRootSafeRoleAfterRemoveWholeTree() + public + { (uint256 rootIdA,) = palmeraSafeBuilder.setupRootOrgAndOneSafe(orgName, safeA1Name); @@ -909,7 +911,7 @@ contract PalmeraRolesTest is DeployHelper { ); } - /// @notice Test to create a root Safe in one onchain org, disconnect it and after to add another or and verify not preserve any role + /// @notice Test to create a root Safe in one onchain org, disconnect it and after to add another or and verify not preserve any role function test_AnyRootSafeNotPreserveRootSafeRoleAfterDisconnect() public { (uint256 rootIdA, uint256 safeAId) = palmeraSafeBuilder.setupRootOrgAndOneSafe(orgName, safeA1Name); @@ -958,4 +960,285 @@ contract PalmeraRolesTest is DeployHelper { false ); } + + /// @notice Test to create a complex multi Root Org, assign one child Safe like a Safe Lead Role in multiples Safe of OnChain Org and Validate all al removed after disconnect + function test_AnySafeNotPreserveSafeLeadRolesAfterDisconnectinTheSameOrg() + public + { + ( + uint256 rootIdA, + uint256 safeIdA1, + uint256 rootIdB, + uint256 safeIdB1, + uint256 subSafeA1, + uint256 subSafeB1 + ) = palmeraSafeBuilder.setupTwoRootOrgWithOneSafeAndOneChildEach( + orgName, + safeA1Name, + org2Name, + safeBName, + subSafeA1Name, + subSafeB1Name + ); + // Array of Address for the subSafes + address[] memory subSafeAaddr = new address[](6); + uint256[] memory subSafeAid = new uint256[](6); + + // Assig the Address to first two subSafes + subSafeAaddr[0] = palmeraModule.getSafeAddress(rootIdA); + subSafeAaddr[1] = palmeraModule.getSafeAddress(safeIdA1); + subSafeAaddr[2] = palmeraModule.getSafeAddress(subSafeA1); + subSafeAaddr[3] = palmeraModule.getSafeAddress(rootIdB); + subSafeAaddr[4] = palmeraModule.getSafeAddress(safeIdB1); + subSafeAaddr[5] = palmeraModule.getSafeAddress(subSafeB1); + + // Assig the Id to first two subSafes + subSafeAid[0] = rootIdA; + subSafeAid[1] = safeIdA1; + subSafeAid[2] = subSafeA1; + subSafeAid[3] = rootIdB; + subSafeAid[4] = safeIdB1; + subSafeAid[5] = subSafeB1; + + // Address of Root B + address rootBAddr = palmeraModule.getSafeAddress(rootIdB); + + // Assign subSafeB1 as Safe Lead Role in multiples Safe of OnChain Org + for (uint256 i = 0; i < subSafeAaddr.length - 1; i++) { + // Set at least Two Safe Lead Role to safeAIdAddr over rootIdA + vm.startPrank(subSafeAaddr[i <= 2 ? 0 : 3]); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD, subSafeAaddr[5], subSafeAid[i], true + ); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY, + subSafeAaddr[5], + subSafeAid[i], + true + ); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY, + subSafeAaddr[5], + subSafeAid[i], + true + ); + vm.stopPrank(); + // Verify the Roles Setting for subSafeB1 + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], uint8(DataTypes.Role.SAFE_LEAD) + ), + true + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY) + ), + true + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY) + ), + true + ); + assertTrue(palmeraModule.isSafeLead(subSafeAid[i], subSafeAaddr[5])); + } + + // disconnect subSafeB1 from RootIdB + vm.startPrank(rootBAddr); + palmeraModule.disconnectSafe(subSafeB1); + vm.stopPrank(); + + // Verify the Roles Setting for subSafeB1 + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], uint8(DataTypes.Role.SAFE_LEAD) + ), + false + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY) + ), + false + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY) + ), + false + ); + for (uint256 i = 0; i < subSafeAaddr.length - 1; i++) { + (,, address safeLead,,,) = palmeraModule.getSafeInfo(subSafeAid[i]); + assertEq(safeLead, address(0)); + assertFalse( + palmeraModule.isSafeLead(subSafeAid[i], subSafeAaddr[5]) + ); + } + } + + /// @notice Test to create a complex multi Root Org, assign one child Safe like a Safe Lead Role in multiples Safe of OnChain Org and Validate all al removed after disconnect + function test_AnySafeNotPreserveSafeLeadRolesAfterUnSetRole() public { + ( + uint256 rootIdA, + uint256 safeIdA1, + uint256 rootIdB, + uint256 safeIdB1, + uint256 subSafeA1, + uint256 subSafeB1 + ) = palmeraSafeBuilder.setupTwoRootOrgWithOneSafeAndOneChildEach( + orgName, + safeA1Name, + org2Name, + safeBName, + subSafeA1Name, + subSafeB1Name + ); + // Array of Address for the subSafes + address[] memory subSafeAaddr = new address[](6); + uint256[] memory subSafeAid = new uint256[](6); + + // Assig the Address to first two subSafes + subSafeAaddr[0] = palmeraModule.getSafeAddress(rootIdA); + subSafeAaddr[1] = palmeraModule.getSafeAddress(safeIdA1); + subSafeAaddr[2] = palmeraModule.getSafeAddress(subSafeA1); + subSafeAaddr[3] = palmeraModule.getSafeAddress(rootIdB); + subSafeAaddr[4] = palmeraModule.getSafeAddress(safeIdB1); + subSafeAaddr[5] = palmeraModule.getSafeAddress(subSafeB1); + + // Assig the Id to first two subSafes + subSafeAid[0] = rootIdA; + subSafeAid[1] = safeIdA1; + subSafeAid[2] = subSafeA1; + subSafeAid[3] = rootIdB; + subSafeAid[4] = safeIdB1; + subSafeAid[5] = subSafeB1; + + // Address of Root B + address rootBAddr = palmeraModule.getSafeAddress(rootIdB); + + // Assign subSafeB1 as Safe Lead Role in multiples Safe of OnChain Org + for (uint256 i = 0; i < subSafeAaddr.length - 1; i++) { + // Set at least Two Safe Lead Role to safeAIdAddr over rootIdA + vm.startPrank(subSafeAaddr[i <= 2 ? 0 : 3]); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD, subSafeAaddr[5], subSafeAid[i], true + ); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY, + subSafeAaddr[5], + subSafeAid[i], + true + ); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY, + subSafeAaddr[5], + subSafeAid[i], + true + ); + vm.stopPrank(); + // Verify the Roles Setting for subSafeB1 + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], uint8(DataTypes.Role.SAFE_LEAD) + ), + true + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY) + ), + true + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY) + ), + true + ); + assertTrue(palmeraModule.isSafeLead(subSafeAid[i], subSafeAaddr[5])); + } + + // UnAssign subSafeB1 as Safe Lead Role in multiples Safe of OnChain Org + for (uint256 i = 0; i < subSafeAaddr.length - 1; i++) { + // Set at least Two Safe Lead Role to safeAIdAddr over rootIdA + vm.startPrank(subSafeAaddr[i <= 2 ? 0 : 3]); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD, subSafeAaddr[5], subSafeAid[i], false + ); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY, + subSafeAaddr[5], + subSafeAid[i], + false + ); + palmeraModule.setRole( + DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY, + subSafeAaddr[5], + subSafeAid[i], + false + ); + vm.stopPrank(); + // Verify the Roles Setting for subSafeB1 + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], uint8(DataTypes.Role.SAFE_LEAD) + ), + false + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY) + ), + false + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY) + ), + false + ); + assertFalse( + palmeraModule.isSafeLead(subSafeAid[i], subSafeAaddr[5]) + ); + } + + // Verify the Roles Setting for subSafeB1 + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], uint8(DataTypes.Role.SAFE_LEAD) + ), + false + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_MODIFY_OWNERS_ONLY) + ), + false + ); + assertEq( + palmeraRolesContract.doesUserHaveRole( + subSafeAaddr[5], + uint8(DataTypes.Role.SAFE_LEAD_EXEC_ON_BEHALF_ONLY) + ), + false + ); + for (uint256 i = 0; i < subSafeAaddr.length - 1; i++) { + (,, address safeLead,,,) = palmeraModule.getSafeInfo(subSafeAid[i]); + assertEq(safeLead, address(0)); + assertFalse( + palmeraModule.isSafeLead(subSafeAid[i], subSafeAaddr[5]) + ); + } + } }