[DEVSVCS-963] Ensure that workflow id is unique (#15582)

* ensure that workflow id is unique

* Update gethwrappers

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>

contracts/gas-snapshots/workflow.gas-snapshot

@@ -17,9 +17,9 @@ WorkflowRegistryManager_getVersion:test_WhenVersionNumberIsRegistered() (gas: 28
 WorkflowRegistryManager_getVersionNumberByContractAddressAndChainID:test_WhenAVersionIsRegisteredForTheContractAddressAndChainIDCombination() (gas: 285022)
 WorkflowRegistryManager_getVersionNumberByContractAddressAndChainID:test_WhenNoVersionIsRegisteredForTheContractAddressAndChainIDCombination() (gas: 286634)
 WorkflowRegistryManager_getVersionNumberByContractAddressAndChainID:test_WhenTheContractAddressIsInvalid() (gas: 284604)
-WorkflowRegistry_activateWorkflow:test_WhenTheCallerIsAnAuthorizedAddress() (gas: 495029)
-WorkflowRegistry_deleteWorkflow:test_WhenTheCallerIsAnAuthorizedAddress_AndTheDonIDIsAllowed() (gas: 403945)
-WorkflowRegistry_deleteWorkflow:test_WhenTheCallerIsAnAuthorizedAddress_AndTheDonIDIsNotAllowed() (gas: 421748)
+WorkflowRegistry_activateWorkflow:test_WhenTheCallerIsAnAuthorizedAddress() (gas: 517416)
+WorkflowRegistry_deleteWorkflow:test_WhenTheCallerIsAnAuthorizedAddress_AndTheDonIDIsAllowed() (gas: 422157)
+WorkflowRegistry_deleteWorkflow:test_WhenTheCallerIsAnAuthorizedAddress_AndTheDonIDIsNotAllowed() (gas: 439960)
 WorkflowRegistry_getAllAllowedDONs:test_WhenTheRegistryIsLocked() (gas: 47473)
 WorkflowRegistry_getAllAllowedDONs:test_WhenTheSetOfAllowedDONsIsEmpty() (gas: 25780)
 WorkflowRegistry_getAllAllowedDONs:test_WhenThereAreMultipleAllowedDONs() (gas: 75437)
@@ -28,9 +28,9 @@ WorkflowRegistry_getAllAuthorizedAddresses:test_WhenTheRegistryIsLocked() (gas:
 WorkflowRegistry_getAllAuthorizedAddresses:test_WhenTheSetOfAuthorizedAddressesIsEmpty() (gas: 26152)
 WorkflowRegistry_getAllAuthorizedAddresses:test_WhenThereAreMultipleAuthorizedAddresses() (gas: 78270)
 WorkflowRegistry_getAllAuthorizedAddresses:test_WhenThereIsASingleAuthorizedAddress() (gas: 16832)
-WorkflowRegistry_getWorkflowMetadata:test_WhenTheRegistryIsLocked() (gas: 519145)
+WorkflowRegistry_getWorkflowMetadata:test_WhenTheRegistryIsLocked() (gas: 541532)
 WorkflowRegistry_getWorkflowMetadata:test_WhenTheWorkflowDoesNotExist() (gas: 17543)
-WorkflowRegistry_getWorkflowMetadata:test_WhenTheWorkflowExistsWithTheOwnerAndName() (gas: 490001)
+WorkflowRegistry_getWorkflowMetadata:test_WhenTheWorkflowExistsWithTheOwnerAndName() (gas: 512388)
 WorkflowRegistry_getWorkflowMetadataListByDON:test_WhenLimitExceedsTotalWorkflows() (gas: 128146)
 WorkflowRegistry_getWorkflowMetadataListByDON:test_WhenLimitIsEqualToTotalWorkflows() (gas: 128035)
 WorkflowRegistry_getWorkflowMetadataListByDON:test_WhenLimitIsLessThanTotalWorkflows() (gas: 90141)
@@ -48,17 +48,17 @@ WorkflowRegistry_getWorkflowMetadataListByOwner:test_WhenStartIsGreaterThanOrEqu
 WorkflowRegistry_getWorkflowMetadataListByOwner:test_WhenTheOwnerHasNoWorkflows() (gas: 14006)
 WorkflowRegistry_getWorkflowMetadataListByOwner:test_WhenTheRegistryIsLocked() (gas: 165968)
 WorkflowRegistry_lockRegistry:test_WhenTheCallerIsTheContractOwner() (gas: 38758)
-WorkflowRegistry_pauseWorkflow:test_WhenTheDonIDIsAllowed_AndTheCallerIsAnAuthorizedAddress() (gas: 494993)
-WorkflowRegistry_pauseWorkflow:test_WhenTheDonIDIsAllowed_AndTheCallerIsAnUnauthorizedAddress() (gas: 502796)
-WorkflowRegistry_pauseWorkflow:test_WhenTheDonIDIsNotAllowed_AndTheCallerIsAnAuthorizedAddress() (gas: 502555)
-WorkflowRegistry_pauseWorkflow:test_WhenTheDonIDIsNotAllowed_AndTheCallerIsAnUnauthorizedAddress() (gas: 506966)
-WorkflowRegistry_registerWorkflow:test_WhenTheWorkflowInputsAreAllValid() (gas: 549769)
-WorkflowRegistry_requestForceUpdateSecrets:test_WhenTheCallerIsAnAuthorizedAddress_AndTheWorkflowIsInAnAllowedDON() (gas: 891242)
-WorkflowRegistry_requestForceUpdateSecrets:test_WhenTheCallerIsAnAuthorizedAddress_AndTheWorkflowIsNotInAnAllowedDON() (gas: 488397)
-WorkflowRegistry_requestForceUpdateSecrets:test_WhenTheCallerIsNotAnAuthorizedAddress() (gas: 486751)
+WorkflowRegistry_pauseWorkflow:test_WhenTheDonIDIsAllowed_AndTheCallerIsAnAuthorizedAddress() (gas: 517380)
+WorkflowRegistry_pauseWorkflow:test_WhenTheDonIDIsAllowed_AndTheCallerIsAnUnauthorizedAddress() (gas: 525183)
+WorkflowRegistry_pauseWorkflow:test_WhenTheDonIDIsNotAllowed_AndTheCallerIsAnAuthorizedAddress() (gas: 524942)
+WorkflowRegistry_pauseWorkflow:test_WhenTheDonIDIsNotAllowed_AndTheCallerIsAnUnauthorizedAddress() (gas: 529353)
+WorkflowRegistry_registerWorkflow:test_WhenTheWorkflowInputsAreAllValid() (gas: 572178)
+WorkflowRegistry_requestForceUpdateSecrets:test_WhenTheCallerIsAnAuthorizedAddress_AndTheWorkflowIsInAnAllowedDON() (gas: 936016)
+WorkflowRegistry_requestForceUpdateSecrets:test_WhenTheCallerIsAnAuthorizedAddress_AndTheWorkflowIsNotInAnAllowedDON() (gas: 510784)
+WorkflowRegistry_requestForceUpdateSecrets:test_WhenTheCallerIsNotAnAuthorizedAddress() (gas: 509138)
 WorkflowRegistry_unlockRegistry:test_WhenTheCallerIsTheContractOwner() (gas: 30325)
 WorkflowRegistry_updateAllowedDONs:test_WhenTheBoolInputIsFalse() (gas: 29739)
 WorkflowRegistry_updateAllowedDONs:test_WhenTheBoolInputIsTrue() (gas: 170296)
 WorkflowRegistry_updateAuthorizedAddresses:test_WhenTheBoolInputIsFalse() (gas: 30278)
 WorkflowRegistry_updateAuthorizedAddresses:test_WhenTheBoolInputIsTrue() (gas: 175515)
-WorkflowRegistry_updateWorkflow:test_WhenTheWorkflowInputsAreAllValid() (gas: 479601)
+WorkflowRegistry_updateWorkflow:test_WhenTheWorkflowInputsAreAllValid() (gas: 515666)

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_workflowIDs;
 
   /// @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
+    s_workflowIDs[currentWorkflowID] = false;
+
     // 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
+    s_workflowIDs[workflow.workflowID] = false;
+
     // Remove the workflow from the owner and DON mappings
     s_ownerWorkflowKeys[msg.sender].remove(workflowKey);
     s_donWorkflowKeys[workflow.donID].remove(workflowKey);
@@ -508,6 +521,20 @@ 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_workflowIDs[workflowID]) {
+      revert WorkflowIDAlreadyExists();
+    }
+
+    s_workflowIDs[workflowID] = true;
+  }
+
   // ================================================================
   // |                       Workflow Queries                       |
   // ================================================================
@@ -636,16 +663,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,32 @@ 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.startPrank(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.expectRevert(WorkflowRegistry.WorkflowIDAlreadyExists.selector);
+    s_registry.updateWorkflow(
+      s_validWorkflowKey, s_newValidWorkflowID, s_validBinaryURL, s_validConfigURL, s_newValidSecretsURL
+    );
+
+    vm.stopPrank();
+  }
+
   // 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}