Add check for valid appId

gizatechxyz · Oct 9, 2024 · bee7b6a · bee7b6a
1 parent 1968884
commit bee7b6a
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 46 deletions.
diff --git a/contracts/lcov.info b/contracts/lcov.info
@@ -1,9 +1,30 @@
 TN:
+SF:src/ClientAppRegistry.sol
+FN:36,ClientAppRegistry.
+FNDA:20,ClientAppRegistry.
+FN:42,ClientAppRegistry.registerClientApp
+FNDA:19,ClientAppRegistry.registerClientApp
+DA:43,1
+BRDA:43,0,0,1
+DA:44,17
+DA:45,17
+DA:46,17
+FN:53,ClientAppRegistry.getClientAppMetadata
+FNDA:2,ClientAppRegistry.getClientAppMetadata
+DA:54,2
+FNF:3
+FNH:3
+LF:5
+LH:5
+BRF:1
+BRH:1
+end_of_record
+TN:
 SF:src/Ownable.sol
 FN:35,Ownable.
-FNDA:17,Ownable.
-DA:36,17
-DA:38,17
+FNDA:44,Ownable.
+DA:36,44
+DA:38,44
 FN:48,Ownable.transferOwnership
 FNDA:3,Ownable.transferOwnership
 DA:49,3
@@ -21,9 +42,9 @@ FNDA:1,Ownable.cancelTransferOwnership
 DA:64,1
 DA:65,1
 FN:68,Ownable.onlyOwner
-FNDA:5,Ownable.onlyOwner
-DA:69,5
-BRDA:69,1,0,2
+FNDA:24,Ownable.onlyOwner
+DA:69,24
+BRDA:69,1,0,3
 FNF:5
 FNH:5
 LF:12
@@ -33,46 +54,44 @@ BRH:2
 end_of_record
 TN:
 SF:src/TaskRegistry.sol
-FN:45,TaskRegistry.
-FNDA:13,TaskRegistry.
-DA:46,13
-DA:47,13
-FN:54,TaskRegistry.setAggregatorNode
+FN:46,TaskRegistry.
+FNDA:20,TaskRegistry.
+DA:47,20
+DA:48,20
+FN:55,TaskRegistry.setAggregatorNode
 FNDA:2,TaskRegistry.setAggregatorNode
-DA:55,1
-FN:58,TaskRegistry.setClientAppRegistry
+DA:56,1
+FN:59,TaskRegistry.setClientAppRegistry
 FNDA:2,TaskRegistry.setClientAppRegistry
-DA:59,1
-FN:66,TaskRegistry.createTask
-FNDA:8,TaskRegistry.createTask
-DA:68,8
-DA:71,8
-BRDA:71,0,0,1
-DA:72,7
-DA:73,7
-DA:74,7
-FN:77,TaskRegistry.respondToTask
+DA:60,1
+FN:67,TaskRegistry.createTask
+FNDA:9,TaskRegistry.createTask
+DA:69,9
+BRDA:69,0,0,1
+DA:72,8
+DA:75,8
+BRDA:75,1,0,1
+DA:76,7
+DA:77,7
+DA:78,7
+FN:81,TaskRegistry.respondToTask
 FNDA:8,TaskRegistry.respondToTask
-DA:79,7
-BRDA:79,1,0,1
-DA:80,6
-BRDA:80,2,0,1
-DA:83,5
-BRDA:83,3,0,1
-DA:84,4
-BRDA:84,4,0,1
-DA:86,3
-DA:87,3
-FN:94,TaskRegistry.onlyAggregatorNode
+DA:83,7
+BRDA:83,2,0,2
+DA:86,5
+BRDA:86,3,0,2
+DA:88,3
+DA:89,3
+FN:96,TaskRegistry.onlyAggregatorNode
 FNDA:8,TaskRegistry.onlyAggregatorNode
-DA:95,8
-BRDA:95,5,0,1
+DA:97,8
+BRDA:97,4,0,1
 FNF:6
 FNH:6
-LF:16
-LH:16
-BRF:6
-BRH:6
+LF:15
+LH:15
+BRF:5
+BRH:5
 end_of_record
 TN:
 SF:test/TestOwnable.t.sol

diff --git a/contracts/src/TaskRegistry.sol b/contracts/src/TaskRegistry.sol
@@ -2,6 +2,7 @@
 pragma solidity 0.8.26;
 
 import {Ownable} from "./Ownable.sol";
+import {IClientAppRegistry} from "./interfaces/IClientAppRegistry.sol";
 
 contract TaskRegistry is Ownable {
     /*//////////////////////////////////////////////////////////////
@@ -17,7 +18,7 @@ contract TaskRegistry is Ownable {
 
     error TaskAlreadyExists();
     error InvalidTaskOperation();
-
+    error InvalidAppId();
     /*//////////////////////////////////////////////////////////////
                               STATE
     //////////////////////////////////////////////////////////////*/
@@ -64,6 +65,9 @@ contract TaskRegistry is Ownable {
     //////////////////////////////////////////////////////////////*/
 
     function createTask(bytes32 appId) external {
+        // Check that appId is registered
+        if (!IClientAppRegistry(clientAppRegistry).isClientApp(appId)) revert InvalidAppId();
+
         // We create a pseudo unique taskId in order to keep track of the task while minimizing gas cost
         bytes32 taskId = keccak256(abi.encode(msg.sender, appId, block.timestamp));
 
@@ -76,12 +80,10 @@ contract TaskRegistry is Ownable {
 
     function respondToTask(bytes32 taskId, TaskStatus status) external onlyAggregatorNode {
         // Check that status is only Completed or Failed
-        if (status == TaskStatus.EMPTY) revert InvalidTaskOperation();
-        if (status == TaskStatus.PENDING) revert InvalidTaskOperation();
+        if (status == TaskStatus.EMPTY || status == TaskStatus.PENDING) revert InvalidTaskOperation();
 
-        // Check that taskId have not been responded to already
-        if (tasks[taskId] == TaskStatus.COMPLETED) revert InvalidTaskOperation();
-        if (tasks[taskId] == TaskStatus.FAILED) revert InvalidTaskOperation();
+        // Check that taskId have not been responded yet
+        if (tasks[taskId] != TaskStatus.PENDING) revert InvalidTaskOperation();
 
         tasks[taskId] = status;
         emit TaskResponded(taskId, status);

diff --git a/contracts/test/TaskRegistry.t.sol b/contracts/test/TaskRegistry.t.sol
@@ -4,10 +4,16 @@ pragma solidity 0.8.26;
 import {TaskRegistry} from "../src/TaskRegistry.sol";
 import {Ownable} from "../src/Ownable.sol";
 import {TestState} from "./TestState.sol";
+import {ClientAppMetadata} from "../src/ClientAppRegistry.sol";
 
 contract TaskRegistryTest is TestState {
     function setUp() public override {
         super.setUp();
+
+        // Register client app
+        bytes32 appId = bytes32(uint256(1));
+        vm.prank(owner);
+        clientAppRegistry.registerClientApp(appId, ClientAppMetadata("Test APP", "Test App", "TEST"));
     }
 
     function testSetAggregatorNode() public {
@@ -44,6 +50,14 @@ contract TaskRegistryTest is TestState {
         taskRegistry.setClientAppRegistry(newClientAppRegistry);
     }
 
+    function testCreateTask_RevertWhen_InvalidAppId() public {
+        bytes32 appId = bytes32(uint256(2));
+
+        vm.prank(user);
+        vm.expectRevert(TaskRegistry.InvalidAppId.selector);
+        taskRegistry.createTask(appId);
+    }
+
     function testCreateTask() public {
         bytes32 appId = bytes32(uint256(1));
         bytes32 taskId = keccak256(abi.encode(user, appId, block.timestamp));