ensure that workflow id is unique

contracts/src/v0.8/workflow/dev/WorkflowRegistry.sol

@@ -43,6 +43,8 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
   /// @dev Mapping to track workflows by secretsURL hash (owner + secretsURL).
   /// This is used to find all workflows that have the same secretsURL when a force secrets update event is requested.
   mapping(bytes32 secretsURLHash => EnumerableSet.Bytes32Set workflowKeys) private s_secretsHashToWorkflows;
+  /// @dev Keep track of all workflowIDs to ensure uniqueness.
+  mapping(bytes32 workflowID => bool inUse) private s_usedWorkflowIDs;
 
   /// @dev List of all authorized EOAs/contracts allowed to access this contract's state functions. All view functions are open access.
   EnumerableSet.AddressSet private s_authorizedAddresses;
@@ -203,13 +205,15 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
   ) external registryNotLocked {
     _validatePermissions(donID, msg.sender);
     _validateWorkflowName(bytes(workflowName).length);
-    _validateWorkflowMetadata(workflowID, bytes(binaryURL).length, bytes(configURL).length, bytes(secretsURL).length);
+    _validateWorkflowURLs(bytes(binaryURL).length, bytes(configURL).length, bytes(secretsURL).length);
 
     bytes32 workflowKey = computeHashKey(msg.sender, workflowName);
     if (s_workflows[workflowKey].owner != address(0)) {
       revert WorkflowAlreadyRegistered();
     }
 
+    _requireUniqueWorkflowID(workflowID);
+
     // Create new workflow entry
     s_workflows[workflowKey] = WorkflowMetadata({
       workflowID: workflowID,
@@ -272,7 +276,7 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
     string calldata configURL,
     string calldata secretsURL
   ) external registryNotLocked {
-    _validateWorkflowMetadata(newWorkflowID, bytes(binaryURL).length, bytes(configURL).length, bytes(secretsURL).length);
+    _validateWorkflowURLs(bytes(binaryURL).length, bytes(configURL).length, bytes(secretsURL).length);
 
     WorkflowMetadata storage workflow = _getWorkflowFromStorage(msg.sender, workflowKey);
 
@@ -295,6 +299,12 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
       revert WorkflowContentNotUpdated();
     }
 
+    // Ensure the new workflowID is unique
+    _requireUniqueWorkflowID(newWorkflowID);
+
+    // Free the old workflowID
+    _releaseWorkflowID(currentWorkflowID);
+
     // Update all fields that have changed and the relevant sets
     workflow.workflowID = newWorkflowID;
     if (!sameBinaryURL) {
@@ -387,6 +397,9 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
       revert AddressNotAuthorized(msg.sender);
     }
 
+    // Release the workflowID for reuse
+    _releaseWorkflowID(workflow.workflowID);
+
     // Remove the workflow from the owner and DON mappings
     s_ownerWorkflowKeys[msg.sender].remove(workflowKey);
     s_donWorkflowKeys[workflow.donID].remove(workflowKey);
@@ -508,6 +521,28 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
     return workflow;
   }
 
+  /// @notice Ensures the given workflowID is unique and marks it as used.
+  /// @param workflowID The workflowID to validate and consume.
+  function _requireUniqueWorkflowID(
+    bytes32 workflowID
+  ) internal {
+    if (workflowID == bytes32(0)) revert InvalidWorkflowID();
+
+    if (s_usedWorkflowIDs[workflowID]) {
+      revert WorkflowIDAlreadyExists();
+    }
+
+    s_usedWorkflowIDs[workflowID] = true;
+  }
+
+  /// @notice Frees up a previously used workflowID, allowing it to be reused.
+  /// @param workflowID The workflowID to release.
+  function _releaseWorkflowID(
+    bytes32 workflowID
+  ) internal {
+    s_usedWorkflowIDs[workflowID] = false;
+  }
+
   // ================================================================
   // |                       Workflow Queries                       |
   // ================================================================
@@ -636,16 +671,12 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
   // |                          Validation                          |
   // ================================================================
 
-  /// @dev Internal function to validate the metadata for a workflow.
-  /// @param workflowID The unique identifier for the workflow.
-  function _validateWorkflowMetadata(
-    bytes32 workflowID,
+  /// @dev Internal function to validate the urls for a workflow.
+  function _validateWorkflowURLs(
     uint256 binaryURLLength,
     uint256 configURLLength,
     uint256 secretsURLLength
   ) internal pure {
-    if (workflowID == bytes32(0)) revert InvalidWorkflowID();
-
     if (binaryURLLength > MAX_URL_LENGTH) {
       revert URLTooLong(binaryURLLength, MAX_URL_LENGTH);
     }

contracts/src/v0.8/workflow/test/WorkflowRegistry/WorkflowRegistry.registerWorkflow.t.sol

@@ -138,6 +138,35 @@ contract WorkflowRegistry_registerWorkflow is WorkflowRegistrySetup {
     );
   }
 
+  // whenTheCallerIsAnAuthorizedAddress whenTheRegistryIsNotLocked whenTheDonIDIsAllowed
+  function test_RevertWhen_TheWorkflowIDIsAlreadyInUsedByAnotherWorkflow() external {
+    vm.startPrank(s_authorizedAddress);
+
+    // Register a valid workflow first
+    s_registry.registerWorkflow(
+      s_validWorkflowName,
+      s_validWorkflowID,
+      s_allowedDonID,
+      WorkflowRegistry.WorkflowStatus.ACTIVE,
+      s_validBinaryURL,
+      s_validConfigURL,
+      s_validSecretsURL
+    );
+
+    vm.expectRevert(WorkflowRegistry.WorkflowIDAlreadyExists.selector);
+    s_registry.registerWorkflow(
+      "ValidWorkflow2",
+      s_validWorkflowID,
+      s_allowedDonID,
+      WorkflowRegistry.WorkflowStatus.ACTIVE,
+      s_validBinaryURL,
+      s_validConfigURL,
+      s_validSecretsURL
+    );
+
+    vm.stopPrank();
+  }
+
   // whenTheCallerIsAnAuthorizedAddress whenTheRegistryIsNotLocked whenTheDonIDIsAllowed
   function test_RevertWhen_TheWorkflowNameIsAlreadyUsedByTheOwner() external {
     vm.startPrank(s_authorizedAddress);

contracts/src/v0.8/workflow/test/WorkflowRegistry/WorkflowRegistry.registerWorkflow.tree

@@ -18,6 +18,8 @@ WorkflowRegistry.registerWorkflow
             │   └── it should revert
             ├── when the workflowID is invalid
             │   └── it should revert
+            ├── when the workflowID is already in used by another workflow
+            │   └── it should revert
             ├── when the workflow name is already used by the owner
             │   └── it should revert
             └── when the workflow inputs are all valid

contracts/src/v0.8/workflow/test/WorkflowRegistry/WorkflowRegistry.updateWorkflow.t.sol

@@ -158,6 +158,31 @@ contract WorkflowRegistry_updateWorkflow is WorkflowRegistrySetup {
     s_registry.updateWorkflow(s_validWorkflowKey, bytes32(0), s_validBinaryURL, s_validConfigURL, s_newValidSecretsURL);
   }
 
+  // whenTheCallerIsAnAuthorizedAddress whenTheRegistryIsNotLocked whenTheDonIDIsAllowed whenTheCallerIsTheWorkflowOwner
+  function test_RevertWhen_TheWorkflowIDIsAlreadyInUsedByAnotherWorkflow() external {
+    // Register a workflow first
+    _registerValidWorkflow();
+
+    // Register another workflow with another workflow ID
+    vm.prank(s_authorizedAddress);
+    s_registry.registerWorkflow(
+      "ValidWorkflow2",
+      s_newValidWorkflowID,
+      s_allowedDonID,
+      WorkflowRegistry.WorkflowStatus.ACTIVE,
+      s_validBinaryURL,
+      s_validConfigURL,
+      s_validSecretsURL
+    );
+
+    // Update the workflow with a workflow ID that is already in use by another workflow.
+    vm.prank(s_authorizedAddress);
+    vm.expectRevert(WorkflowRegistry.WorkflowIDAlreadyExists.selector);
+    s_registry.updateWorkflow(
+      s_validWorkflowKey, s_newValidWorkflowID, s_validBinaryURL, s_validConfigURL, s_newValidSecretsURL
+    );
+  }
+
   // whenTheCallerIsAnAuthorizedAddress whenTheRegistryIsNotLocked whenTheDonIDIsAllowed whenTheCallerIsTheWorkflowOwner
   function test_WhenTheWorkflowInputsAreAllValid() external {
     // Register a workflow first.

contracts/src/v0.8/workflow/test/WorkflowRegistry/WorkflowRegistry.updateWorkflow.tree

@@ -25,6 +25,8 @@ WorkflowRegistry.updateWorkflow
                 │   └── it should revert
                 ├── when the workflowID is invalid
                 │   └── it should revert
+                ├── when the workflowID is already in used by another workflow
+                │   └── it should revert
                 └── when the workflow inputs are all valid
                     ├── it should update the existing workflow in s_workflows with the new values
                     ├── it should emit {WorkflowUpdatedV1}