Merge cc30af2 into 86ab654

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

@@ -83,8 +83,8 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
     bytes32 indexed workflowID, address indexed workflowOwner, uint32 indexed donID, string workflowName
   );
   event WorkflowForceUpdateSecretsRequestedV1(address indexed owner, bytes32 secretsURLHash, string workflowName);
-  event RegistryLockedV1(address indexed lockedBy);
-  event RegistryUnlockedV1(address indexed unlockedBy);
+  event RegistryLockedV1(address lockedBy);
+  event RegistryUnlockedV1(address unlockedBy);
 
   error AddressNotAuthorized(address caller);
   error CallerIsNotWorkflowOwner(address caller);
@@ -401,11 +401,11 @@ contract WorkflowRegistry is Ownable2StepMsgSender, ITypeAndVersion {
       s_secretsHashToWorkflows[secretsHash].remove(workflowKey);
     }
 
+    // Emit an event indicating the workflow has been deleted. We need to do this before deleting the workflow from storage.
+    emit WorkflowDeletedV1(workflow.workflowID, sender, donID, workflow.workflowName);
+
     // Delete the workflow metadata from storage
     delete s_workflows[workflowKey];
-
-    // Emit an event indicating the workflow has been deleted
-    emit WorkflowDeletedV1(workflow.workflowID, sender, donID, workflow.workflowName);
   }
 
   /// @notice Requests a force update for workflows that share the same secrets URL.

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

@@ -26,6 +26,10 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
   /// indexing strategy to avoid off-by-one errors.
   mapping(uint32 versionNumber => Version versionInfo) private s_versions;
 
+  /// @notice Maps a combination of address and chain ID to the version number.
+  /// @dev This mapping allows for lookup of the version number for a given address and chain ID.
+  mapping(bytes32 => uint32) private s_versionNumberByAddressAndChainID;
+
   /// @notice The version number of the currently active WorkflowRegistry.
   /// @dev Initialized to 0 to indicate no active version. Updated when a version is activated.
   uint32 private s_activeVersionNumber = 0;
@@ -34,21 +38,18 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
   /// @dev Incremented each time a new version is added. Useful for iterating over all registered versions.
   uint32 private s_latestVersionNumber = 0;
 
-  /// @notice Maps a combination of address and chain ID to the version number.
-  /// @dev This mapping allows for lookup of the version number for a given address and chain ID.
-  mapping(bytes32 => uint32) private s_versionNumberByAddressAndChainID;
-
   // Errors
   error InvalidContractAddress(address invalidAddress);
   error InvalidContractType(address invalidAddress);
   error NoActiveVersionAvailable();
   error NoVersionsRegistered();
   error VersionNotRegistered(uint32 versionNumber);
+  error VersionAlreadyActive(uint32 versionNumber);
   // Events
 
   event VersionAdded(address indexed contractAddress, uint64 chainID, uint32 deployedAt, uint32 version);
-  event VersionActivated(address indexed contractAddress, uint64 chainID, uint32 indexed version);
-  event VersionDeactivated(address indexed contractAddress, uint64 chainID, uint32 indexed version);
+  event VersionActivated(address indexed contractAddress, uint64 chainID, uint32 version);
+  event VersionDeactivated(address indexed contractAddress, uint64 chainID, uint32 version);
 
   // ================================================================
   // |                      Manage Versions                         |
@@ -110,6 +111,11 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
     // Cache the current active version number to reduce storage reads
     uint32 currentActiveVersionNumber = s_activeVersionNumber;
 
+    // Check that the version number is not the same as the current active version number
+    if (currentActiveVersionNumber == versionNumber) {
+      revert VersionAlreadyActive(versionNumber);
+    }
+
     // Emit deactivation event if there is an active version
     if (currentActiveVersionNumber != 0) {
       Version memory currentActive = s_versions[currentActiveVersionNumber];
@@ -178,7 +184,10 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
   /// @param contractAddress The address of the WorkflowRegistry contract.
   /// @param chainID The chain ID of the network where the WorkflowRegistry is deployed.
   /// @return versionNumber The version number associated with the given contract address and chain ID.
-  function getVersionNumber(address contractAddress, uint64 chainID) external view returns (uint32 versionNumber) {
+  function getVersionNumberByContractAddressAndChainID(
+    address contractAddress,
+    uint64 chainID
+  ) external view returns (uint32 versionNumber) {
     _validateContractAddress(contractAddress);
 
     bytes32 key = keccak256(abi.encodePacked(contractAddress, chainID));
@@ -201,10 +210,10 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
 
   /// @notice Retrieves the details of the latest registered WorkflowRegistry version.
   /// @return A `Version` struct containing the details of the latest version.
-  /// @custom:throws NoActiveVersionAvailable if no versions have been registered.
+  /// @custom:throws NoVersionsRegistered if no versions have been registered.
   function getLatestVersion() external view returns (Version memory) {
     uint32 latestVersionNumber = s_latestVersionNumber;
-    if (latestVersionNumber == 0) revert NoActiveVersionAvailable();
+    if (latestVersionNumber == 0) revert NoVersionsRegistered();
     return s_versions[latestVersionNumber];
   }
 
@@ -246,6 +255,9 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
     }
   }
 
+  /// @dev Validates that a given contract address is non-zero and contains code.
+  /// @param _addr The address of the contract to validate.
+  /// @custom:throws InvalidContractAddress if the address is zero or contains no code.
   function _validateContractAddress(
     address _addr
   ) internal view {

contracts/src/v0.8/workflow/mocks/MockContract.sol

@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.24;
+
+contract MockContract {
+  function mockFunction() external pure returns (string memory) {
+    return "Mocked!";
+  }
+}

contracts/src/v0.8/workflow/mocks/MockWorkflowRegistryContract.sol

@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.24;
+
+import {ITypeAndVersion} from "../../shared/interfaces/ITypeAndVersion.sol";
+
+contract MockWorkflowRegistryContract is ITypeAndVersion {
+  string public constant override typeAndVersion = "MockWorkflowRegistryContract 1.0.0";
+}

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

@@ -104,6 +104,12 @@ contract WorkflowRegistry_activateWorkflow is WorkflowRegistrySetup {
       s_validSecretsURL
     );
 
+    // It should emit {WorkflowActivatedV1} when the workflow is activated.
+    vm.expectEmit(true, true, true, true);
+    emit WorkflowRegistry.WorkflowActivatedV1(
+      s_validWorkflowID, s_authorizedAddress, s_allowedDonID, s_validWorkflowName
+    );
+
     // Activate the workflow.
     vm.prank(s_authorizedAddress);
     s_registry.activateWorkflow(s_validWorkflowKey);

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

@@ -5,13 +5,13 @@ WorkflowRegistry.activateWorkflow
     ├── when the caller is not the workflow owner
     │   └── it should revert
     └── when the caller is the workflow owner
-        ├── when the workflow is already paused
+        ├── when the workflow is already active
         │   └── it should revert
         └── when the workflow is paused
             ├── when the donID is not allowed
             │   └── it should revert
             └── when the donID is allowed
-                └── when the caller is not an authorized address
+                ├── when the caller is not an authorized address
                 │   └── it should revert
                 └── when the caller is an authorized address
-                    └── it should activate the workflow
+                    └── it should activate the workflow and emit {WorkflowActivatedV1}

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

@@ -57,6 +57,10 @@ contract WorkflowRegistry_deleteWorkflow is WorkflowRegistrySetup {
       s_registry.getWorkflowMetadata(s_authorizedAddress, s_validWorkflowName);
     assertEq(workflow.workflowName, s_validWorkflowName);
 
+    // It should emit {WorkflowDeletedV1} when the workflow is deleted.
+    vm.expectEmit(true, true, true, true);
+    emit WorkflowRegistry.WorkflowDeletedV1(s_validWorkflowID, s_authorizedAddress, s_allowedDonID, s_validWorkflowName);
+
     // Delete the workflow.
     vm.prank(s_authorizedAddress);
     s_registry.deleteWorkflow(s_validWorkflowKey);
@@ -79,6 +83,10 @@ contract WorkflowRegistry_deleteWorkflow is WorkflowRegistrySetup {
     // Remove the DON from the allowed DONs list.
     _removeDONFromAllowedDONs(s_allowedDonID);
 
+    // It should emit {WorkflowDeletedV1} when the workflow is deleted.
+    vm.expectEmit(true, true, true, true);
+    emit WorkflowRegistry.WorkflowDeletedV1(s_validWorkflowID, s_authorizedAddress, s_allowedDonID, s_validWorkflowName);
+
     // Delete the workflow.
     vm.prank(s_authorizedAddress);
     s_registry.deleteWorkflow(s_validWorkflowKey);

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

@@ -8,5 +8,5 @@ WorkflowRegistry.deleteWorkflow
         ├── when the caller is not an authorized address
         │   └── it should revert
         └── when the caller is an authorized address
-            ├── it should delete the workflow if the donID is not allowed
-            └── it should delete the workflow if the donID is allowed
+            ├── it should delete the workflow if the donID is not allowed and emit {WorkflowDeletedV1}
+            └── it should delete the workflow if the donID is allowed and emit {WorkflowDeletedV1}

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

@@ -31,4 +31,15 @@ contract WorkflowRegistry_getAllAllowedDONs is WorkflowRegistrySetup {
     assertEq(allowedDONs[0], s_allowedDonID);
     assertEq(allowedDONs[1], allowedDonID2);
   }
+
+  function test_WhenTheRegistryIsLocked() external {
+    // Lock the registry
+    vm.prank(s_owner);
+    s_registry.lockRegistry();
+
+    // It should behave the same as when the registry is not locked
+    uint32[] memory allowedDONs = s_registry.getAllAllowedDONs();
+    assertEq(allowedDONs.length, 1);
+    assertEq(allowedDONs[0], s_allowedDonID);
+  }
 }

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

@@ -3,5 +3,7 @@ WorkflowRegistry.getAllAllowedDONs
 │   └── it should return an empty array
 ├── when there is a single allowed DON
 │   └── it should return an array with one element
-└── when there are multiple allowed DONs
-    └── it should return an array with all the allowed DONs
+├── when there are multiple allowed DONs
+│   └── it should return an array with all the allowed DONs
+└── when the registry is locked
+    └── it should behave the same as when the registry is not locked

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

@@ -28,4 +28,15 @@ contract WorkflowRegistrygetAllAuthorizedAddresses is WorkflowRegistrySetup {
     assertEq(authorizedAddresses[0], s_authorizedAddress);
     assertEq(authorizedAddresses[1], s_unauthorizedAddress);
   }
+
+  function test_WhenTheRegistryIsLocked() external {
+    // Lock the registry
+    vm.prank(s_owner);
+    s_registry.lockRegistry();
+
+    // It should behave the same as when the registry is not locked
+    address[] memory authorizedAddresses = s_registry.getAllAuthorizedAddresses();
+    assertEq(authorizedAddresses.length, 1);
+    assertEq(authorizedAddresses[0], s_authorizedAddress);
+  }
 }

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

@@ -3,5 +3,7 @@ WorkflowRegistry.getAllAuthorizedAddresses
 │   └── it should return an empty array
 ├── when there is a single authorized address
 │   └── it should return an array with one element
-└── when there are multiple authorized addresses
-    └── it should return an array with all the authorized addresses
+├── when there are multiple authorized addresses
+│   └── it should return an array with all the authorized addresses
+└── when the registry is locked
+    └── it should behave the same as when the registry is not locked

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

@@ -22,4 +22,23 @@ contract WorkflowRegistry_getWorkflowMetadata is WorkflowRegistrySetup {
     vm.expectRevert(WorkflowRegistry.WorkflowDoesNotExist.selector);
     s_registry.getWorkflowMetadata(s_authorizedAddress, "RandomWorkflowName");
   }
+
+  function test_WhenTheRegistryIsLocked() external {
+    // Register a workflow
+    _registerValidWorkflow();
+
+    // Lock the registry
+    vm.prank(s_owner);
+    s_registry.lockRegistry();
+
+    // It should behave the same as when the registry is not locked
+    WorkflowRegistry.WorkflowMetadata memory metadata =
+      s_registry.getWorkflowMetadata(s_authorizedAddress, s_validWorkflowName);
+
+    assertEq(metadata.workflowName, s_validWorkflowName);
+    assertEq(metadata.workflowID, s_validWorkflowID);
+    assertEq(metadata.binaryURL, s_validBinaryURL);
+    assertEq(metadata.configURL, s_validConfigURL);
+    assertEq(metadata.secretsURL, s_validSecretsURL);
+  }
 }

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

@@ -1,5 +1,7 @@
 WorkflowRegistry.getWorkflowMetadata
 ├── when the workflow exists with the owner and name
 │   └── it returns the correct metadata
-└── when the workflow does not exist
-    └── it reverts with WorkflowDoesNotExist
+├── when the workflow does not exist
+│   └── it reverts with WorkflowDoesNotExist
+└── when the registry is locked
+    └── it should behave the same as when the registry is not locked

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

@@ -123,4 +123,33 @@ contract WorkflowRegistry_getWorkflowMetadataListByDON is WorkflowRegistryWithFi
 
     assertEq(workflows.length, 0);
   }
+
+  function test_WhenTheRegistryIsLocked() external {
+    // Lock the registry
+    vm.prank(s_owner);
+    s_registry.lockRegistry();
+
+    // It should behave the same as when the registry is not locked
+    WorkflowRegistry.WorkflowMetadata[] memory workflows =
+      s_registry.getWorkflowMetadataListByDON(s_allowedDonID, 0, 10);
+
+    assertEq(workflows.length, 3);
+    assertEq(workflows[0].workflowName, s_workflowName1);
+    assertEq(workflows[0].workflowID, s_workflowID1);
+    assertEq(workflows[0].binaryURL, s_binaryURL1);
+    assertEq(workflows[0].configURL, s_configURL1);
+    assertEq(workflows[0].secretsURL, s_secretsURL1);
+
+    assertEq(workflows[1].workflowName, s_workflowName2);
+    assertEq(workflows[1].workflowID, s_workflowID2);
+    assertEq(workflows[1].binaryURL, s_binaryURL2);
+    assertEq(workflows[1].configURL, s_configURL2);
+    assertEq(workflows[1].secretsURL, s_secretsURL2);
+
+    assertEq(workflows[2].workflowName, s_workflowName3);
+    assertEq(workflows[2].workflowID, s_workflowID3);
+    assertEq(workflows[2].binaryURL, s_binaryURL3);
+    assertEq(workflows[2].configURL, s_configURL3);
+    assertEq(workflows[2].secretsURL, s_secretsURL3);
+  }
 }

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

@@ -12,5 +12,7 @@ WorkflowRegistry.getWorkflowMetadataListByDON
 │       └── it returns the correct metadata list
 ├── when the DON has no workflows
 │   └── it returns an empty list
-└── when start is greater than or equal to total workflows
-    └── it returns an empty list
+├── when start is greater than or equal to total workflows
+│   └── it returns an empty list
+└── when the registry is locked
+    └── it should behave the same as when the registry is not locked

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

@@ -5,7 +5,7 @@ import {WorkflowRegistry} from "../../dev/WorkflowRegistry.sol";
 import {WorkflowRegistryWithFixture} from "./WorkflowRegistryWithFixture.t.sol";
 
 contract WorkflowRegistry_getWorkflowMetadataListByOwner is WorkflowRegistryWithFixture {
-  function test_WhenStartIs0_AndLimitIs0() external view {
+  function test_WhenStartIs0() external view {
     WorkflowRegistry.WorkflowMetadata[] memory workflows =
       s_registry.getWorkflowMetadataListByOwner(s_authorizedAddress, 0, 0);
 
@@ -126,4 +126,33 @@ contract WorkflowRegistry_getWorkflowMetadataListByOwner is WorkflowRegistryWith
 
     assertEq(workflows.length, 0);
   }
+
+  function test_WhenTheRegistryIsLocked() external {
+    // Lock the registry
+    vm.prank(s_owner);
+    s_registry.lockRegistry();
+
+    // It should behave the same as when the registry is not locked
+    WorkflowRegistry.WorkflowMetadata[] memory workflows =
+      s_registry.getWorkflowMetadataListByOwner(s_authorizedAddress, 0, 0);
+
+    assertEq(workflows.length, 3);
+    assertEq(workflows[0].workflowName, s_workflowName1);
+    assertEq(workflows[0].workflowID, s_workflowID1);
+    assertEq(workflows[0].binaryURL, s_binaryURL1);
+    assertEq(workflows[0].configURL, s_configURL1);
+    assertEq(workflows[0].secretsURL, s_secretsURL1);
+
+    assertEq(workflows[1].workflowName, s_workflowName2);
+    assertEq(workflows[1].workflowID, s_workflowID2);
+    assertEq(workflows[1].binaryURL, s_binaryURL2);
+    assertEq(workflows[1].configURL, s_configURL2);
+    assertEq(workflows[1].secretsURL, s_secretsURL2);
+
+    assertEq(workflows[2].workflowName, s_workflowName3);
+    assertEq(workflows[2].workflowID, s_workflowID3);
+    assertEq(workflows[2].binaryURL, s_binaryURL3);
+    assertEq(workflows[2].configURL, s_configURL3);
+    assertEq(workflows[2].secretsURL, s_secretsURL3);
+  }
 }

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

@@ -2,7 +2,7 @@ WorkflowRegistry.getWorkflowMetadataListByOwner
 ├── when the owner has workflows
 │   ├── when start is 0
 │   │   └── it returns the correct metadata list
-│   ├── when start is greater than 0 and limit exceeds total
+│   ├── when start is greater than 0
 │   │   └── it returns the correct metadata list
 │   ├── when limit is less than total workflows
 │   │   └── it returns the correct metadata list
@@ -12,5 +12,7 @@ WorkflowRegistry.getWorkflowMetadataListByOwner
 │       └── it returns the correct metadata list
 ├── when the owner has no workflows
 │   └── it returns an empty list
-└── when start is greater than or equal to total workflows
-    └── it returns an empty list
+├── when start is greater than or equal to total workflows
+│   └── it returns an empty list
+└── when the registry is locked
+    └── it should behave the same as when the registry is not locked