From bee7b6a2cdcab5fc6e4420e01f282f40a1830cba Mon Sep 17 00:00:00 2001 From: "chalex.eth" <43524913+chalex-eth@users.noreply.github.com> Date: Wed, 9 Oct 2024 22:33:37 +0200 Subject: [PATCH] Add check for valid appId --- contracts/lcov.info | 99 ++++++++++++++++++------------- contracts/src/TaskRegistry.sol | 14 +++-- contracts/test/TaskRegistry.t.sol | 14 +++++ 3 files changed, 81 insertions(+), 46 deletions(-) diff --git a/contracts/lcov.info b/contracts/lcov.info index 318e545..6ded31a 100644 --- 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 index 8dd0f89..d8617ad 100644 --- 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 index ded565c..52025db 100644 --- 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));