Skip to content

Commit

Permalink
fix additional registry tests
Browse files Browse the repository at this point in the history
  • Loading branch information
eutopian committed Nov 14, 2024
1 parent a3f4ceb commit 76f21df
Show file tree
Hide file tree
Showing 10 changed files with 232 additions and 183 deletions.
258 changes: 147 additions & 111 deletions contracts/src/v0.8/workflow/dev/WorkflowRegistry.sol

Large diffs are not rendered by default.

20 changes: 15 additions & 5 deletions contracts/src/v0.8/workflow/dev/WorkflowRegistryManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
/// can be active at a time; activating a new version automatically deactivates the currently active one (if any).
/// @param versionNumber The 1-based version number to activate (minimum value is 1).
/// @custom:throws VersionNotRegistered if the `versionNumber` is not valid or not registered.
function activateVersion(uint32 versionNumber) external onlyOwner {
function activateVersion(
uint32 versionNumber
) external onlyOwner {
_activateVersion(versionNumber);
}

Expand Down Expand Up @@ -127,7 +129,9 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
/// @param versionNumber The 1-based version number of the version to retrieve (minimum value is 1).
/// @return A `Version` struct containing the details of the specified version.
/// @custom:throws VersionNotRegistered if the `versionNumber` is not valid or not registered.
function getVersion(uint32 versionNumber) external view returns (Version memory) {
function getVersion(
uint32 versionNumber
) external view returns (Version memory) {
if (versionNumber == 0 || versionNumber > s_latestVersionNumber) {
revert VersionNotRegistered(versionNumber);
}
Expand Down Expand Up @@ -178,7 +182,9 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
/// emits events for both deactivation and activation.
/// @param versionNumber The version number of the version to activate.
/// @custom:throws IndexOutOfBounds if the version number does not exist.
function _activateVersion(uint32 versionNumber) private {
function _activateVersion(
uint32 versionNumber
) private {
// Cache the current active version number to reduce storage reads
uint32 currentActiveVersionNumber = s_activeVersionNumber;

Expand All @@ -203,7 +209,9 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
/// @param contractAddress The address of the contract to validate.
/// @custom:throws InvalidContractAddress if the address is zero or contains no code.
/// @custom:throws InvalidContractType if the contract does not implement typeAndVersion().
function _getTypeAndVersionForContract(address contractAddress) internal view returns (string memory) {
function _getTypeAndVersionForContract(
address contractAddress
) internal view returns (string memory) {
if (!_isNonZeroWithCode(contractAddress)) {
revert InvalidContractAddress(contractAddress);
}
Expand All @@ -215,7 +223,9 @@ contract WorkflowRegistryManager is Ownable2StepMsgSender, ITypeAndVersion {
}
}

function _isNonZeroWithCode(address _addr) internal view returns (bool) {
function _isNonZeroWithCode(
address _addr
) internal view returns (bool) {
return _addr != address(0) && _addr.code.length > 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ contract WorkflowRegistry_activateWorkflow is WorkflowRegistrySetup {
vm.prank(s_owner);
s_registry.lockRegistry();

// Attempt to delete the workflow now after the registry is locked.
// Attempt to activate the workflow now after the registry is locked.
vm.prank(s_authorizedAddress);
vm.expectRevert(WorkflowRegistry.RegistryLocked.selector);
s_registry.activateWorkflow(s_validWorkflowKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ contract WorkflowRegistry_deleteWorkflow is WorkflowRegistrySetup {
s_registry.deleteWorkflow(s_validWorkflowKey);
}

modifier whenTheRegistryIsNotLocked() {
_;
}

// whenTheRegistryIsNotLocked
function test_RevertWhen_TheCallerIsNotTheWorkflowOwner() external {
// Register a workflow first.
Expand All @@ -37,10 +33,6 @@ contract WorkflowRegistry_deleteWorkflow is WorkflowRegistrySetup {
s_registry.deleteWorkflow(s_validWorkflowKey);
}

modifier whenTheCallerIsTheWorkflowOwner() {
_;
}

// whenTheRegistryIsNotLocked whenTheCallerIsTheWorkflowOwner
function test_RevertWhen_TheCallerIsNotAnAuthorizedAddress() external {
// Register the workflow first as an authorized address.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ contract WorkflowRegistry_registerWorkflow is WorkflowRegistrySetup {
vm.expectRevert(
abi.encodeWithSelector(WorkflowRegistry.WorkflowNameTooLong.selector, bytes(s_invalidWorkflowName).length, 64)
);

// vm.expectRevert(abi.encodeWithSelector(WorkflowRegistry.WorkflowNameTooLong.selector, 128, 64));
s_registry.registerWorkflow(
s_invalidWorkflowName,
s_validWorkflowID,
Expand All @@ -76,9 +74,7 @@ contract WorkflowRegistry_registerWorkflow is WorkflowRegistrySetup {
);
}

// whenTheCallerIsAnAuthorizedAddress
// whenTheRegistryIsNotLocked
// whenTheDonIDIsAllowed
// whenTheCallerIsAnAuthorizedAddress whenTheRegistryIsNotLocked whenTheDonIDIsAllowed
function test_RevertWhen_TheBinaryURLIsTooLong() external {
vm.prank(s_authorizedAddress);

Expand Down Expand Up @@ -110,9 +106,7 @@ contract WorkflowRegistry_registerWorkflow is WorkflowRegistrySetup {
);
}

// whenTheCallerIsAnAuthorizedAddress
// whenTheRegistryIsNotLocked
// whenTheDonIDIsAllowed
// whenTheCallerIsAnAuthorizedAddress whenTheRegistryIsNotLocked whenTheDonIDIsAllowed
function test_RevertWhen_TheSecretsURLIsTooLong() external {
vm.prank(s_authorizedAddress);

Expand All @@ -123,7 +117,7 @@ contract WorkflowRegistry_registerWorkflow is WorkflowRegistrySetup {
s_allowedDonID,
WorkflowRegistry.WorkflowStatus.ACTIVE,
s_validBinaryURL,
s_validBinaryURL,
s_validConfigURL,
s_invalidURL
);
}
Expand All @@ -139,7 +133,7 @@ contract WorkflowRegistry_registerWorkflow is WorkflowRegistrySetup {
s_allowedDonID,
WorkflowRegistry.WorkflowStatus.ACTIVE,
s_validBinaryURL,
s_validBinaryURL,
s_validConfigURL,
s_validSecretsURL
);
}
Expand Down Expand Up @@ -239,7 +233,7 @@ contract WorkflowRegistry_registerWorkflow is WorkflowRegistrySetup {
// it should add the url + key to s_secretsHashToWorkflows when the secretsURL is not empty
vm.expectEmit(true, true, false, true);
emit WorkflowRegistry.WorkflowForceUpdateSecretsRequestedV1(
s_validSecretsURL, s_authorizedAddress, s_validWorkflowName
s_authorizedAddress, keccak256(abi.encodePacked(s_authorizedAddress, s_validSecretsURL)), s_validWorkflowName
);

// Call the function that should emit the event
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// SPDX-License-Identifier: MIT
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.24;

import {WorkflowRegistry} from "../../dev/WorkflowRegistry.sol";
Expand All @@ -25,9 +25,6 @@ contract WorkflowRegistry_requestForceUpdateSecrets is WorkflowRegistrySetup {
// Register a workflow first.
_registerValidWorkflow();

// Start recording logs
vm.recordLogs();

// Call the requestForceUpdateSecrets function now on a random URL
vm.prank(s_authorizedAddress);
vm.expectRevert(WorkflowRegistry.WorkflowDoesNotExist.selector);
Expand All @@ -44,7 +41,6 @@ contract WorkflowRegistry_requestForceUpdateSecrets is WorkflowRegistrySetup {
// Start recording logs
vm.recordLogs();

// Call the requestForceUpdateSecrets function now after the registry is locked.
vm.prank(s_authorizedAddress);
s_registry.requestForceUpdateSecrets(s_validSecretsURL);

Expand Down Expand Up @@ -103,29 +99,52 @@ contract WorkflowRegistry_requestForceUpdateSecrets is WorkflowRegistrySetup {
// Register a workflow first.
_registerValidWorkflow();

// Register another workflow with the same owner but different secrets URL.
vm.prank(s_authorizedAddress);
s_registry.registerWorkflow(
"ValidWorkflow2",
keccak256("validWorkflow2"),
s_allowedDonID,
WorkflowRegistry.WorkflowStatus.ACTIVE,
"https://example.com/valid-binary2",
s_validConfigURL,
s_validSecretsURL
);

// Start recording logs
vm.recordLogs();

// Call the requestForceUpdateSecrets function now after the registry is locked.
vm.prank(s_authorizedAddress);
s_registry.requestForceUpdateSecrets(s_validSecretsURL);
// Verify the event emitted with correct details
Vm.Log[] memory entries = vm.getRecordedLogs();
assertEq(entries.length, 1);
assertEq(entries[0].topics[0], keccak256("WorkflowForceUpdateSecretsRequestedV1(string,address,string)"));

// Compare the hash of the expected string with the topic
bytes32 expectedSecretsURLHash = keccak256(abi.encodePacked(s_validSecretsURL));
assertEq(entries[0].topics[1], expectedSecretsURLHash);

// Decode the indexed address
address decodedOwner = abi.decode(abi.encodePacked(entries[0].topics[2]), (address));
assertEq(decodedOwner, s_authorizedAddress);

// Decode the non-indexed data
string memory decodedWorkflowName = abi.decode(entries[0].data, (string));

// Assert the values
assertEq(entries.length, 2);

bytes32 eventSignature = keccak256("WorkflowForceUpdateSecretsRequestedV1(address,bytes32,string)");

// Check the first event
assertEq(entries[0].topics[0], eventSignature);
// Verify owner (indexed)
address decodedAddress = abi.decode(abi.encodePacked(entries[0].topics[1]), (address));
assertEq(decodedAddress, s_authorizedAddress);
// Decode non-indexed parameters (secretsURLHash and workflowName)
(bytes32 decodedSecretsURLHash, string memory decodedWorkflowName) = abi.decode(entries[0].data, (bytes32, string));
// Verify the decoded values
bytes32 expectedSecretsURLHash = keccak256(abi.encodePacked(s_authorizedAddress, s_validSecretsURL));
assertEq(decodedSecretsURLHash, expectedSecretsURLHash);
assertEq(decodedWorkflowName, s_validWorkflowName);

// // Check the second event
assertEq(entries[1].topics[0], eventSignature);
// Verify owner (indexed)
address decodedAddress2 = abi.decode(abi.encodePacked(entries[1].topics[1]), (address));
assertEq(decodedAddress2, s_authorizedAddress);
// Decode non-indexed parameters (secretsURLHash and workflowName)
(bytes32 decodedSecretsURLHash2, string memory decodedWorkflowName2) =
abi.decode(entries[1].data, (bytes32, string));
// Verify the decoded values
bytes32 expectedSecretsURLHash2 = keccak256(abi.encodePacked(s_authorizedAddress, s_validSecretsURL));
assertEq(decodedSecretsURLHash2, expectedSecretsURLHash2);
assertEq(decodedWorkflowName2, "ValidWorkflow2");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ contract WorkflowRegistry_updateWorkflow is WorkflowRegistrySetup {
// It should add the url + key to s_secretsHashToWorkflows when the secretsURL is not empty
vm.expectEmit(true, true, false, true);
emit WorkflowRegistry.WorkflowForceUpdateSecretsRequestedV1(
s_newValidSecretsURL, s_authorizedAddress, s_validWorkflowName
s_authorizedAddress, keccak256(abi.encodePacked(s_authorizedAddress, s_newValidSecretsURL)), s_validWorkflowName
);

// Call the function that should emit the event.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,29 @@ contract WorkflowRegistrySetup is Test {
}

// Helper function to remove an address from the authorized addresses list
function _removeAddressFromAuthorizedAddresses(address addressToRemove) internal {
function _removeAddressFromAuthorizedAddresses(
address addressToRemove
) internal {
address[] memory addressesToRemove = new address[](1);
addressesToRemove[0] = addressToRemove;
vm.prank(s_owner);
s_registry.updateAuthorizedAddresses(addressesToRemove, false);
}

// Helper function to remove a DON from the allowed DONs list
function _removeDONFromAllowedDONs(uint32 donIDToRemove) internal {
function _removeDONFromAllowedDONs(
uint32 donIDToRemove
) internal {
uint32[] memory donIDsToRemove = new uint32[](1);
donIDsToRemove[0] = donIDToRemove;
vm.prank(s_owner);
s_registry.updateAllowedDONs(donIDsToRemove, false);
}

// Helper function to add an address to the authorized addresses list
function _addAddressToAuthorizedAddresses(address addressToAdd) internal {
function _addAddressToAuthorizedAddresses(
address addressToAdd
) internal {
address[] memory addressesToAdd = new address[](1);
addressesToAdd[0] = addressToAdd;
vm.prank(s_owner);
Expand Down
Loading

0 comments on commit 76f21df

Please sign in to comment.