Merge pull request #143 from DIMO-Network/feature/si-2665-admin-funct…

…ion-to-remove-attributes

Feature/si 2665 admin function to remove attributes

Selectors.md

@@ -33,6 +33,7 @@
 | 0xb17b974b | adminCacheDimoStreamrEns() |
 | 0x56936962 | adminChangeParentNode(uint256,address,uint256[]) |
 | 0x3febacab | adminPairAftermarketDevice(uint256,uint256) |
+| 0x5f741f4d | adminRemoveVehicleAttribute(string) |
 | 0xf73a8f04 | renameManufacturers((uint256,string)[]) |
 | 0xff96b761 | transferAftermarketDeviceOwnership(uint256,address) |
 | 0x5c129493 | unclaimAftermarketDeviceNode(uint256[]) |
@@ -53,6 +54,7 @@
 | 0xf6391f5c | RoleRevoked(bytes32,address,address) |
 | 0x8ad18187 | SyntheticDeviceAttributeSetDevAdmin(uint256,string,string) |
 | 0x81741fdd | SyntheticDeviceNodeBurnedDevAdmin(uint256,uint256,address) |
+| 0x9b4bf377 | VehicleAttributeRemoved(string) |
 | 0x1d91e00c | VehicleAttributeSetDevAdmin(uint256,string,string) |
 | 0xb956d027 | VehicleNodeBurnedDevAdmin(uint256,address) |
 

abis/DimoRegistry.json

@@ -452,6 +452,19 @@
         "name": "SyntheticDeviceNodeBurnedDevAdmin",
         "type": "event"
     },
+    {
+        "anonymous": false,
+        "inputs": [
+            {
+                "indexed": false,
+                "internalType": "string",
+                "name": "attribute",
+                "type": "string"
+            }
+        ],
+        "name": "VehicleAttributeRemoved",
+        "type": "event"
+    },
     {
         "anonymous": false,
         "inputs": [
@@ -609,6 +622,19 @@
         "stateMutability": "nonpayable",
         "type": "function"
     },
+    {
+        "inputs": [
+            {
+                "internalType": "string",
+                "name": "attribute",
+                "type": "string"
+            }
+        ],
+        "name": "adminRemoveVehicleAttribute",
+        "outputs": [],
+        "stateMutability": "nonpayable",
+        "type": "function"
+    },
     {
         "inputs": [
             {

contracts/dev/DevAdmin.sol

@@ -70,6 +70,7 @@ contract DevAdmin is AccessControlInternal {
         uint256 vehicleNode,
         address indexed owner
     );
+    event VehicleAttributeRemoved(string attribute);
 
     struct IdManufacturerName {
         uint256 tokenId;
@@ -631,6 +632,21 @@ contract DevAdmin is AccessControlInternal {
         streamRegistry.createStreamWithENS(dimoStreamrEns, "/vehicles/", "{}");
     }
 
+    /**
+     * @notice Admin function remove a vehicle node attribute
+     * @dev Caller must have the DEV_REMOVE_ATTR role
+     */
+    function adminRemoveVehicleAttribute(
+        string calldata attribute
+    ) external onlyRole(DEV_REMOVE_ATTR) {
+        if (
+            AttributeSet.remove(
+                VehicleStorage.getStorage().whitelistedAttributes,
+                attribute
+            )
+        ) emit VehicleAttributeRemoved(attribute);
+    }
+
     /**
      * @dev Internal function to reset vehicle node infos
      * It iterates over all whitelisted attributes to reset each info

contracts/shared/Roles.sol

@@ -49,3 +49,4 @@ bytes32 constant DEV_AD_BURN_ROLE = keccak256("DEV_AD_BURN_ROLE");
 bytes32 constant DEV_SD_BURN_ROLE = keccak256("DEV_SD_BURN_ROLE");
 bytes32 constant DEV_CHANGE_PARENT_NODE = keccak256("DEV_CHANGE_PARENT_NODE");
 bytes32 constant DEV_CACHE_ENS = keccak256("DEV_CACHE_ENS");
+bytes32 constant DEV_REMOVE_ATTR = keccak256("DEV_REMOVE_ATTR");

scripts/data/deployArgs.ts

@@ -34,7 +34,8 @@ export const roles = {
     DEV_AD_BURN_ROLE: _hashRole('DEV_AD_BURN_ROLE'),
     DEV_SD_BURN_ROLE: _hashRole('DEV_SD_BURN_ROLE'),
     DEV_CHANGE_PARENT_NODE: _hashRole('DEV_CHANGE_PARENT_NODE'),
-    DEV_CACHE_ENS: _hashRole('DEV_CACHE_ENS')
+    DEV_CACHE_ENS: _hashRole('DEV_CACHE_ENS'),
+    DEV_REMOVE_ATTR: _hashRole('DEV_REMOVE_ATTR')
   },
   nfts: {
     MINTER_ROLE: _hashRole('MINTER_ROLE'),

test/DevAdmin.test.ts

@@ -2701,4 +2701,39 @@ describe('DevAdmin', function () {
       });
     });
   });
+
+  describe('adminRemoveVehicleAttribute', () => {
+    context('Error handling', () => {
+      it('Should revert if caller does not have DEV_REMOVE_ATTR role', async () => {
+        await expect(
+          devAdminInstance
+            .connect(nonAdmin)
+            .adminRemoveVehicleAttribute(C.mockVehicleAttribute1),
+        ).to.be.rejectedWith(
+          `AccessControl: account ${nonAdmin.address.toLowerCase()} is missing role ${C.DEV_REMOVE_ATTR
+          }`,
+        );
+      });
+    });
+
+    context('Events', () => {
+      it('Should not emit VehicleAttributeRemoved if attribute does not exist', async () => {
+        await expect(
+          devAdminInstance
+            .connect(admin)
+            .adminRemoveVehicleAttribute('UnexistentAttribute')
+        )
+          .to.not.emit(devAdminInstance, 'VehicleAttributeRemoved')
+      });
+      it('Should emit VehicleAttributeRemoved event with correct params', async () => {
+        await expect(
+          devAdminInstance
+            .connect(admin)
+            .adminRemoveVehicleAttribute(C.mockVehicleAttribute1)
+        )
+          .to.emit(devAdminInstance, 'VehicleAttributeRemoved')
+          .withArgs(C.mockVehicleAttribute1);
+      });
+    });
+  });
 });

utils/constants/AccessControl.ts

@@ -97,3 +97,6 @@ export const DEV_CHANGE_PARENT_NODE = ethers.keccak256(
 export const DEV_CACHE_ENS = ethers.keccak256(
   ethers.toUtf8Bytes('DEV_CACHE_ENS')
 );
+export const DEV_REMOVE_ATTR = ethers.keccak256(
+  ethers.toUtf8Bytes('DEV_REMOVE_ATTR')
+);

utils/setup.ts

@@ -64,7 +64,8 @@ export async function grantAdminRoles(
     C.DEV_AD_BURN_ROLE,
     C.DEV_SD_BURN_ROLE,
     C.DEV_CHANGE_PARENT_NODE,
-    C.DEV_CACHE_ENS
+    C.DEV_CACHE_ENS,
+    C.DEV_REMOVE_ATTR
   ];
 
   for (const role of roles) {