[DEVSVCS-518] Workflow Registry Contract #14990

eutopian · 2024-10-28T21:28:06Z

Initial dev portion of the workflow registry contract ported from dev-platform

github-actions · 2024-10-28T21:29:26Z

Static analysis results are available

Hey @eutopian, you can view Slither reports in the job summary here or download them as artifact here.
Please check them before merging and make sure you have addressed all issues.

github-actions · 2024-10-29T09:55:13Z

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

cl-sonarqube-production · 2024-10-31T02:34:59Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

https://smartcontract-it.atlassian.net/browse/DEVSVCS-518

shileiwill · 2024-11-08T20:07:41Z

contracts/.prettierignore

@@ -22,6 +22,7 @@ LinkToken.json
 typechain
 **/vendor
 src/v0.8/ccip/**
+src/v0.8/workflow/**


any reason we ignore prettier, i used to rely on prettier to format for me :)

:D we're using forge fmt right now in our repo as it's a bit faster :D

RensR · 2024-11-11T10:06:54Z

contracts/hardhat.config.ts

@@ -132,6 +132,19 @@ let config = {
        version: '0.8.19',
        settings: COMPILER_SETTINGS,
      },
+      'src/v0.8/workflow/dev/WorkflowRegistry.sol': {


Do we need this?

RensR · 2024-11-11T10:08:11Z

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

+
+import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/structs/EnumerableSet.sol";
+
+/// @title DONAccessControl


not sure if a title adds a lot

RensR · 2024-11-11T10:08:26Z

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

+
+  // Struct
+  struct DONPermission {
+    uint32 donID;


missing struct packing comments

RensR · 2024-11-11T10:09:22Z

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

+  error AddressNotAuthorized(uint32 donID, address caller);
+  error DONNotAllowed(uint32 donID);
+
+  /// @notice Updates the allowed status for a single DON


comments should end with a .

RensR · 2024-11-11T10:10:04Z

.github/workflows/solidity-foundry.yml

@@ -39,7 +39,8 @@ jobs:
            { "name": "operatorforwarder", "setup": { "run-coverage": true, "min-coverage": 55.7, "run-gas-snapshot": true, "run-forge-fmt": false }},
            { "name": "shared", "setup": { "run-coverage": true, "extra-coverage-params": "--no-match-path='*CallWithExactGas*' --ir-minimum", "min-coverage": 32.6, "run-gas-snapshot": true, "run-forge-fmt": false }},
            { "name": "transmission", "setup": { "run-coverage": true, "min-coverage": 61.5, "run-gas-snapshot": true, "run-forge-fmt": false }},
-            { "name": "vrf", "setup": { "run-coverage": false, "min-coverage": 98.5, "run-gas-snapshot": false, "run-forge-fmt": false }}
+            { "name": "vrf", "setup": { "run-coverage": false, "min-coverage": 98.5, "run-gas-snapshot": false, "run-forge-fmt": false }},
+            { "name": "workflow", "setup": { "run-coverage": true, "extra-coverage-params": "--ir-minimum", "min-coverage": 60.0, "run-gas-snapshot": false, "run-forge-fmt": false }}


why is forge fmt false?

"min-coverage": 60.0

That's very low. You want this to be well above 90%

RensR · 2024-11-11T11:01:06Z

contracts/src/v0.8/workflow/test/WorkflowRegistryManager.t.sol

+  uint32 private s_chainID = 1;
+
+  function setUp() public {
+    vm.prank(s_owner);


use vm.startPrank to use a prank for multiple calls

RensR · 2024-11-11T11:02:25Z

contracts/src/v0.8/workflow/test/WorkflowRegistryManager.t.sol

+import {WorkflowRegistryManager} from "../dev/WorkflowRegistryManager.sol";
+import {Test} from "forge-std/Test.sol";
+
+contract WorkflowRegistryManagerTest is Test {


Please use the following test name format

test_Description for standard tests. testFuzz_Description for fuzz tests. test_RevertWhen_Condition for tests expecting a revert. testFork_Description for tests that fork from a network.

and create a test file per function to be called. For the folder structure, see #15165 as example.

RensR · 2024-11-11T11:04:29Z

contracts/src/v0.8/workflow/test/WorkflowRegistryManager.t.sol

+
+    // Activate first version
+    s_manager.activateVersion(1);
+    vm.stopPrank();


There's no need to stop the prank

RensR · 2024-11-11T11:04:39Z

contracts/src/v0.8/workflow/test/WorkflowRegistryManager.t.sol

+  function testGetAllVersionsPagination() public {
+    // Add multiple versions
+    vm.startPrank(s_owner);
+    for (uint32 i = 0; i < 5; i++) {


++i, even in tests :)

RensR · 2024-11-11T11:05:36Z

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

+  address private s_owner = address(1);
+  address private s_unauthorizedUser = address(2);
+  address private s_authorizedUser = address(3);
+  bytes32 private s_workflowID1 = keccak256(abi.encodePacked("workflow1"));


I dont think you have to encode it at all

RensR · 2024-11-11T11:07:13Z

The PR isn't forge formatted but also adds its code to prettierignore. This means there is no formatting which is not allowed

RensR · 2024-11-11T11:08:33Z

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

+  using EnumerableSet for EnumerableSet.UintSet;
+
+  // Constants
+  string public constant override typeAndVersion = "WorkflowRegistry 1.0.0";


When under development please use the -dev suffix for versions

eutopian force-pushed the workflow-registry-contract-draft branch from 1e373da to ea6b7ff Compare October 28, 2024 21:29

eutopian force-pushed the workflow-registry-contract-draft branch from af3a3ae to 8cc5835 Compare October 31, 2024 02:13

eutopian marked this pull request as ready for review October 31, 2024 12:30

eutopian requested review from a team as code owners October 31, 2024 12:30

eutopian requested a review from chudilka1 October 31, 2024 12:30

eutopian changed the title ~~workflow registry contract draft~~ workflow registry contract Oct 31, 2024

eutopian force-pushed the workflow-registry-contract-draft branch from 653d06d to 8f37a80 Compare November 1, 2024 14:22

eutopian and others added 11 commits November 8, 2024 13:22

workflow registry contract draft

9d9c744

Added initial Foundry tests and other necessary scripts

9a30ada

CI test matrix fix

944b540

Prettier fixes, Hardhat fixes, add missing generation file

82bb343

Forge fmt and forge coverage with via-ir flag

18a52dc

Ignore workflow from Prettier and use via-ir for coverage check

dd5da66

switch from /** */ to /// in doc comments as set by the solidity tsar

215fd65

add more test cases

bca45a0

set paris as evm environment in foundry profile

65be2c0

port additional workflow registry contract work into core

7271593

add changeset

eb6bb8e

eutopian force-pushed the workflow-registry-contract-draft branch from b321eed to 81df9a9 Compare November 8, 2024 18:22

add workflow registry manager contract

4e04532

eutopian force-pushed the workflow-registry-contract-draft branch from 81df9a9 to 4e04532 Compare November 8, 2024 18:38

disable run-forge-fmt in solidity-foundry for workflow

40023d2

https://smartcontract-it.atlassian.net/browse/DEVSVCS-518

eutopian force-pushed the workflow-registry-contract-draft branch from 3ea3eab to 40023d2 Compare November 8, 2024 20:02

shileiwill approved these changes Nov 8, 2024

View reviewed changes

eutopian changed the title ~~workflow registry contract~~ [DEVSVCS-518] Workflow Registry Contract Nov 8, 2024

chainchad approved these changes Nov 8, 2024

View reviewed changes

MStreet3 mentioned this pull request Nov 10, 2024

[CAPPL-222] feat(workflows): adds a secrets syncer for workflow registry #15114

Open

RensR reviewed Nov 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DEVSVCS-518] Workflow Registry Contract #14990

[DEVSVCS-518] Workflow Registry Contract #14990

eutopian commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading

cl-sonarqube-production bot commented Oct 31, 2024

shileiwill Nov 8, 2024

eutopian Nov 8, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR Nov 11, 2024

RensR commented Nov 11, 2024

RensR Nov 11, 2024


		import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/structs/EnumerableSet.sol";

		/// @title DONAccessControl

[DEVSVCS-518] Workflow Registry Contract #14990

Are you sure you want to change the base?

[DEVSVCS-518] Workflow Registry Contract #14990

Conversation

eutopian commented Oct 28, 2024 • edited Loading

github-actions bot commented Oct 28, 2024 • edited Loading

Static analysis results are available

github-actions bot commented Oct 29, 2024 • edited Loading

AER Report: CI Core ran successfully ✅

AER Report: Operator UI CI ran successfully ✅

cl-sonarqube-production bot commented Oct 31, 2024

Quality Gate passed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RensR commented Nov 11, 2024

Choose a reason for hiding this comment

eutopian commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading