refactor: onlyManager to canSetUpdateOperator

decentraland · Apr 23, 2019 · 80c4205 · 80c4205
1 parent 8d1869f
commit 80c4205
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 32 deletions.
diff --git a/contracts/estate/EstateRegistry.sol b/contracts/estate/EstateRegistry.sol
@@ -39,10 +39,10 @@ contract EstateRegistry is Migratable, IEstateRegistry, ERC721Token, ERC721Recei
     _;
   }
 
-  modifier onlyManager(uint256 estateId) {
+  modifier canSetUpdateOperator(uint256 estateId) {
     address owner = ownerOf(estateId);
     require(
-      isApprovedOrOwner(msg.sender, estateId) || updateManager[owner][msg.sender], 
+      isApprovedOrOwner(msg.sender, estateId) || updateManager[owner][msg.sender],
       "unauthorized user"
     );
     _;
@@ -174,25 +174,31 @@ contract EstateRegistry is Migratable, IEstateRegistry, ERC721Token, ERC721Recei
     updateManager[_owner][_operator] = _approved;
 
     emit UpdateManager(
-      _owner, 
+      _owner,
       _operator,
       msg.sender,
       _approved
     );
-  } 
+  }
 
-  function setUpdateOperator(uint256 estateId, address operator) public onlyManager(estateId) {
+  function setUpdateOperator(
+    uint256 estateId,
+    address operator
+  )
+    public
+    canSetUpdateOperator(estateId)
+  {
     updateOperator[estateId] = operator;
     emit UpdateOperator(estateId, operator);
-  }  
+  }
 
   function setLandUpdateOperator(
-    uint256 estateId, 
-    uint256 landId, 
+    uint256 estateId,
+    uint256 landId,
     address operator
-  ) 
-    public 
-    onlyManager(estateId)
+  )
+    public
+    canSetUpdateOperator(estateId)
   {
     require(landIdEstate[landId] == estateId, "The LAND is not part of the Estate");
     registry.setUpdateOperator(landId, operator);
@@ -335,8 +341,8 @@ contract EstateRegistry is Migratable, IEstateRegistry, ERC721Token, ERC721Recei
     }
   }
 
-  function transferFrom(address _from, address _to, uint256 _tokenId) 
-  public 
+  function transferFrom(address _from, address _to, uint256 _tokenId)
+  public
   {
     updateOperator[_tokenId] = address(0);
     super.transferFrom(_from, _to, _tokenId);
@@ -474,11 +480,11 @@ contract EstateRegistry is Migratable, IEstateRegistry, ERC721Token, ERC721Recei
   }
 
   function _isLandUpdateAuthorized(
-    address operator, 
-    uint256 estateId, 
+    address operator,
+    uint256 estateId,
     uint256 landId
-  ) 
-    internal returns (bool) 
+  )
+    internal returns (bool)
   {
     return _isUpdateAuthorized(operator, estateId) || registry.updateOperator(landId) == operator;
   }

diff --git a/contracts/land/LANDRegistry.sol b/contracts/land/LANDRegistry.sol
@@ -32,7 +32,7 @@ contract LANDRegistry is Storage, Ownable, FullAssetRegistry, ILANDRegistry {
 
   modifier onlyDeployer() {
     require(
-      msg.sender == proxyOwner || authorizedDeploy[msg.sender], 
+      msg.sender == proxyOwner || authorizedDeploy[msg.sender],
       "This function can only be called by an authorized deployer"
     );
     _;
@@ -48,18 +48,18 @@ contract LANDRegistry is Storage, Ownable, FullAssetRegistry, ILANDRegistry {
 
   modifier onlyUpdateAuthorized(uint256 tokenId) {
     require(
-      msg.sender == _ownerOf(tokenId) || 
+      msg.sender == _ownerOf(tokenId) ||
       _isAuthorized(msg.sender, tokenId) ||
       _isUpdateAuthorized(msg.sender, tokenId),
       "msg.sender is not authorized to update"
     );
     _;
   }
 
-  modifier onlyManager(uint256 tokenId) {
+  modifier canSetUpdateOperator(uint256 tokenId) {
     address owner = _ownerOf(tokenId);
     require(
-      _isAuthorized(msg.sender, tokenId) || updateManager[owner][msg.sender], 
+      _isAuthorized(msg.sender, tokenId) || updateManager[owner][msg.sender],
       "unauthorized user"
     );
     _;
@@ -76,7 +76,7 @@ contract LANDRegistry is Storage, Ownable, FullAssetRegistry, ILANDRegistry {
   function _isUpdateAuthorized(address operator, uint256 assetId) internal view returns (bool) {
     address owner = _ownerOf(assetId);
 
-    return owner == operator  || 
+    return owner == operator  ||
       updateOperator[assetId] == operator ||
       updateManager[owner][operator];
   }
@@ -92,7 +92,7 @@ contract LANDRegistry is Storage, Ownable, FullAssetRegistry, ILANDRegistry {
   function forbidDeploy(address beneficiary) external onlyProxyOwner {
     require(beneficiary != address(0), "invalid address");
     require(authorizedDeploy[beneficiary], "address is already forbidden");
-    
+
     authorizedDeploy[beneficiary] = false;
     emit DeployForbidden(msg.sender, beneficiary);
   }
@@ -310,7 +310,13 @@ contract LANDRegistry is Storage, Ownable, FullAssetRegistry, ILANDRegistry {
     }
   }
 
-  function setUpdateOperator(uint256 assetId, address operator) external onlyManager(assetId) {
+  function setUpdateOperator(
+    uint256 assetId,
+    address operator
+  )
+    external
+    canSetUpdateOperator(assetId)
+  {
     updateOperator[assetId] = operator;
     emit UpdateOperator(assetId, operator);
   }
@@ -332,12 +338,12 @@ contract LANDRegistry is Storage, Ownable, FullAssetRegistry, ILANDRegistry {
     updateManager[_owner][_operator] = _approved;
 
     emit UpdateManager(
-      _owner, 
+      _owner,
       _operator,
       msg.sender,
       _approved
     );
-  } 
+  }
 
   //
   // Estate generation

diff --git a/full/EstateRegistry.sol b/full/EstateRegistry.sol
@@ -1068,7 +1068,7 @@ contract EstateRegistry is Migratable, IEstateRegistry, ERC721Token, ERC721Recei
     _;
   }
 
-  modifier onlyManager(uint256 estateId) {
+  modifier canSetUpdateOperator(uint256 estateId) {
     address owner = ownerOf(estateId);
     require(
       isApprovedOrOwner(msg.sender, estateId) || updateManager[owner][msg.sender], 
@@ -1210,7 +1210,7 @@ contract EstateRegistry is Migratable, IEstateRegistry, ERC721Token, ERC721Recei
     );
   } 
 
-  function setUpdateOperator(uint256 estateId, address operator) public onlyManager(estateId) {
+  function setUpdateOperator(uint256 estateId, address operator) public canSetUpdateOperator(estateId) {
     updateOperator[estateId] = operator;
     emit UpdateOperator(estateId, operator);
   }  
@@ -1221,7 +1221,7 @@ contract EstateRegistry is Migratable, IEstateRegistry, ERC721Token, ERC721Recei
     address operator
   ) 
     public 
-    onlyManager(estateId)
+    canSetUpdateOperator(estateId)
   {
     require(landIdEstate[landId] == estateId, "The LAND is not part of the Estate");
     registry.setUpdateOperator(landId, operator);

diff --git a/full/LANDRegistry.sol b/full/LANDRegistry.sol
@@ -867,7 +867,7 @@ interface ILANDRegistry {
   function updateLandData(int x, int y, string data) external;
   function updateManyLandData(int[] x, int[] y, string data) external;
 
-  // Authorize an updateManager to update data on any parcel
+  // Authorize an updateManager to manage parcel data
   function setUpdateManager(address _owner, address _operator, bool _approved) external;
 
   // Events
@@ -951,7 +951,7 @@ contract LANDRegistry is Storage, Ownable, FullAssetRegistry, ILANDRegistry {
     _;
   }
 
-  modifier onlyManager(uint256 tokenId) {
+  modifier canSetUpdateOperator(uint256 tokenId) {
     address owner = _ownerOf(tokenId);
     require(
       _isAuthorized(msg.sender, tokenId) || updateManager[owner][msg.sender], 
@@ -1205,7 +1205,7 @@ contract LANDRegistry is Storage, Ownable, FullAssetRegistry, ILANDRegistry {
     }
   }
 
-  function setUpdateOperator(uint256 assetId, address operator) external onlyManager(assetId) {
+  function setUpdateOperator(uint256 assetId, address operator) external canSetUpdateOperator(assetId) {
     updateOperator[assetId] = operator;
     emit UpdateOperator(assetId, operator);
   }